8000 rp2: Crash with hard pin IRQ · Issue #6957 · micropython/micropython · GitHub
[go: up one dir, main page]
Skip to content

rp2: Crash with hard pin IRQ #6957

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
peterhinch opened this issue Feb 24, 2021 · 12 comments
Open

rp2: Crash with hard pin IRQ #6957

peterhinch opened this issue Feb 24, 2021 · 12 comments
Labels
bug port-rp2

Comments

@peterhinch
Copy link
Contributor

Measuring hard IRQ latency with this script:

from machine import Pin, PWM
from time import sleep_ms
import math
# Link 16-17, time 16-18
pin18 = Pin(18, Pin.OUT)

def callback(_):
    pin18(1)  # Trigger
    pin18(0)

pin17 = Pin(17, Pin.IN)
_ = pin17.irq(callback, trigger=Pin.IRQ_RISING, hard=True)

pwm = PWM(Pin(16))
pwm.freq(1000)
pwm.duty_u16(0xffff // 2)

while True:
    y = 0
    for x in range(100):
        y += math.sin(x * 2 * math.pi/100)  # 6.2ms of busywork
    sleep_ms(10)

The usual latency was a creditable 25μs, but I have measured 100μs. It was not practicable to measure a worst case because the the machine suffers a hard crash usually almost immediately. If I comment out the busywork, leaving just the sleep_ms(10), it runs indefinitely with about 20μs latency.
@kevindawson
Copy link
kevindawson commented Apr 29, 2021 •
edited
Loading

from: library/machine.Pin.html

hard if true a hardware interrupt is used. This reduces the delay between the pin change and the handler being called. Hard interrupt handlers may not allocate memory; see Writing interrupt handlers. Not all ports support this argument.

Having read Writing interrupt handlers there is no mention of board/port support
Would I be correct in thinking this is not fully implemented for rp2 as it deadlocks with anything more than test code.

@peterhinch
Copy link
Contributor Author

This bug has not yet been fixed with the latest firmware. I guess it's still early days for rp2.

@robert-hh
Copy link
Contributor
robert-hh commented Apr 29, 2021 •
edited
Loading

I confirm that using hard=TRUE the device deadlocks. That's truly a bug. When enabled with the above example, the GPIO pin fires once, and then the device is unresponsive.

Edit: With soft IRQ the latency is ~35µs, with a range of 30 to 44 µs.

irqlatency

@kevindawson
Copy link

@robert-hh nice image

@robert-hh
Copy link
Contributor

Playing a little bit around with the code I have the hard irq running. I removed the calls for gc.lock() and gc.unlock in mpirq.c. Not safe, but for the given code example it works. So I could get some latency timings, which are not a real improvement over soft irq. The latency is 21 to 49 µs with an avarage of about 28µs. Just 7 µs less that the soft irq. With that numbers, hard irq is hardly worth the restrictions that come along with it. Scope screen shot below.

irqlatency_hard

@peterhinch
Copy link
Contributor Author
peterhinch commented Apr 30, 2021 •
edited
Loading

The problem with soft IRQ's arises when the interrupt happens while a GC is in progress. Latency is then much greater than for a hard IRQ.

@robert-hh
Copy link
Contributor

Looking for another problem, I came along this "lock on hard irq" topic. gc_lock() calls GC_ENTER(), which in term is calling mutex_enter_blocking(). That's a function which according to the specs must not called in an IRQ. Replacing the mutex calls with critsec calls changes the behavior, but I do not know if that is the proper cure. I know too little about GC.

@robert-hh robert-hh mentioned this issue May 2, 2021
Open
@dpgeorge
Copy link
Member
dpgeorge commented May 2, 2021

Thanks @peterhinch for the report. I can reproduce the issue using the code above. The problem is that it's tricky to coordinate handling IRQs with multithreading (which CPU handles the IRQ, and what can each CPU do when one is handling an IRQ?). For now please just use soft IRQs.

@dpgeorge dpgeorge added the bug label May 2, 2021
@robert-hh
Copy link
Contributor
robert-hh commented May 2, 2021 •
edited
Loading

@dpgeorge You have probably seen myabove test of replacing the mutex calls in mpthreadport.c by critsec calls. With that, the test code of Peter works. According to the sdk documentation the mutex calls must not be called in an IRQ context. But memory management, dual threads and IRQ is still an open topic.

Code used for the test:

/*
 * This file is part of the MicroPython project, http://micropython.org/
 *
 * The MIT License (MIT)
 *
 * Copyright (c) 2020-2021 Damien P. George
 *
 * Permission is hereby granted, free of charge, to any person obtaining a copy
 * of this software and associated documentation files (the "Software"), to deal
 * in the Software without restriction, including without limitation the rights
 * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
 * copies of the Software, and to permit persons to whom the Software is
 * furnished to do so, subject to the following conditions:
 *
 * The above copyright notice and this permission notice shall be included in
 * all copies or substantial portions of the Software.
 *
 * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
 * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
 * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
 * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
 * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
 * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
 * THE SOFTWARE.
 */
#ifndef MICROPY_INCLUDED_RP2_MPTHREADPORT_H
#define MICROPY_INCLUDED_RP2_MPTHREADPORT_H

#include "py/mpthread.h"
#include "pico/critical_section.h"

typedef struct critical_section mp_thread_mutex_t;

extern void *core_state[2];

void mp_thread_init(void);
void mp_thread_gc_others(void);

static inline void mp_thread_set_state(struct _mp_state_thread_t *state) {
    core_state[get_core_num()] = state;
}

static inline struct _mp_state_thread_t *mp_thread_get_state(void) {
    return (struct _mp_state_thread_t *)core_state[get_core_num()];
}

static inline void mp_thread_mutex_init(mp_thread_mutex_t *m) {
    critical_section_init(m);
}

static inline int mp_thread_mutex_lock(mp_thread_mutex_t *m, int wait) {
    if (wait) {
        critical_section_enter_blocking(m);
        return 1;
    } else {
        // Only blocking calls are supported by the critsec class, so return 0 in that case
        return 0;
    }
}

static inline void mp_thread_mutex_unlock(mp_thread_mutex_t *m) {
    critical_section_exit(m);
}

#endif // MICROPY_INCLUDED_RP2_MPTHREADPORT_H

@dpgeorge dpgeorge mentioned this issue May 4, 2021
Closed
@dpgeorge
Copy link
Member
dpgeorge commented May 4, 2021

I posted #7217 to try and fix this issue. It also aims to make hard IRQ have less jitter (at the expense of RAM usage, putting more code into RAM so that the external flash access doesn't slow things down). I also noticed that the hard IRQ jitter improves (is less) if the main code just executes while 1: pass.

If you could test it that would be great!

@robert-hh
Copy link
Contributor
robert-hh commented May 4, 2021 •
edited
Loading

Using the previous test script, it fixes this issue. Without any other load than in this test script, the jitter is the same (stddev 3.5µs, mean ~29µs, min 16µs, max 40 µs).

irqlatency_pr7217

Indeed is the jitter smaller if the main code just executes while 1: pass. I see a mean of 17.5µs, with a sdev of .65µs, min ~16µs, max ~20µſ.

@dpgeorge
Copy link
Member
dpgeorge commented May 5, 2021

Using the previous test script, it fixes this issue.

Great, thanks for testing it.

Indeed is the jitter smaller if the main code just executes while 1: pass. I see a mean of 17.5µs, with a sdev of .65µs, min ~16µs, max ~20µſ.

That's about what I see (although my equipment is not as fancy as yours!).

I'm not sure of the exact architecture of the rp2040, but it may be that IRQs are stalled until any outstanding external-SPI-flash requests are complete. So if the code is executing something from flash and the flash must load the machine code, then IRQs must wait until that is complete (even if the IRQ runs from RAM). That's my guess as to why the jitter there with the original script at the top of this issue, but not with while 1: pass.

dpgeorge added a commit to dpgeorge/micropython that referenced this issue May 6, 2021
@dpgeorge
rp2/mpconfigport.h: Enable multiple cores for gc_lock_depth.
8c2f11e 
So that hard IRQs can run correctly.

Fixes issue micropython#6957.

Signed-off-by: Damien George <damien@micropython.org>
dpgeorge added a commit to dpgeorge/micropython that referenced this issue May 6, 2021
@dpgeorge
rp2/mpconfigport.h: Enable multiple cores for gc_lock_depth.
0efea1e 
So that hard IRQs can run correctly.

Fixes issue micropython#6957.

Signed-off-by: Damien George <damien@micropython.org>
@dpgeorge dpgeorge mentioned this issue May 10, 2021
Merged
Wind-stormger pushed a commit to BPI-STEAM/micropython that referenced this issue Oct 13, 2022
@dhalbert
Merge pull request micropython#6957 from weblate/weblate-circuitpytho…
03f02c7 
…n-main

Translations update from Hosted Weblate
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug port-rp2
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

rp2: make GC lock work per CPU dpgeorge/micropython
4 participants
@kevindawson @dpgeorge @peterhinch @robert-hh
0