Skip to content

Commit bb52df2

Browse files
committed
raspberrypi: Don't block DMA_IRQ_1 during common_hal_mcu_disable_interrupts
doing so causes the picodvi display on rp2350 to blank, because this irq is handed by cpu0. Instead, use the BASEPRI register so that common_hal_mcu_disable_interrupts masks interrupts with lower priority (higher priority values) than PICO_ELEVATED_IRQ_PRIORITY. This has the effect of masking "regular" interrupts while still letting this priority interrupt occur. port_idle_until_interrupt now needs to directly manipulate the interrupt enable state, because an interrupt masked via BASEPRI also does not cause the WFI instruction to complete. Since I don't know that `nesting_count==0` is a precondition of `port_idle_until_interrupt`, also set and restore BASEPRI before WFI.
1 parent 255eea9 commit bb52df2

File tree

3 files changed

+47
-11
lines changed

3 files changed

+47
-11
lines changed

ports/raspberrypi/common-hal/microcontroller/__init__.c

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -23,30 +23,46 @@
2323
#include "src/rp2_common/hardware_sync/include/hardware/sync.h"
2424

2525
#include "hardware/watchdog.h"
26+
#include "hardware/irq.h"
2627

2728
void common_hal_mcu_delay_us(uint32_t delay) {
2829
mp_hal_delay_us(delay);
2930
}
3031

32+
#ifdef PICO_RP2040
33+
#include "src/rp2_common/cmsis/stub/CMSIS/Device/RP0400/Include/RP0400.h"
34+
#else
35+
#include "src/rp2_common/cmsis/stub/CMSIS/Device/RP2350/Include/RP2350.h"
36+
#endif
3137
volatile uint32_t nesting_count = 0;
38+
#define PICO_ELEVATED_IRQ_PRIORITY (0x60) // between PICO_DEFAULT and PIOCO_HIGHEST_IRQ_PRIORITY
39+
static uint32_t oldBasePri = 0; // 0 (default) masks nothing, other values mask equal-or-larger priority values
3240
void common_hal_mcu_disable_interrupts(void) {
33-
// We don't use save_and_disable_interrupts() from the sdk because we don't want to worry about PRIMASK.
34-
// This is what we do on the SAMD21 via CMSIS.
35-
asm volatile ("cpsid i" : : : "memory");
36-
__dmb();
41+
if (nesting_count == 0) {
42+
// We must keep DMA_IRQ_1 (reserved for pico dvi) enabled at all times,
43+
// including during flash writes. Do this by setting the priority mask (BASEPRI
44+
// register).
45+
// grab old base priority
46+
oldBasePri = __get_BASEPRI();
47+
// and set the new one
48+
__set_BASEPRI_MAX(PICO_ELEVATED_IRQ_PRIORITY);
49+
__isb(); // Instruction synchronization barrier
50+
}
3751
nesting_count++;
3852
}
3953

4054
void common_hal_mcu_enable_interrupts(void) {
55+
uint32_t my_interrupts = save_and_disable_interrupts();
4156
if (nesting_count == 0) {
42-
// reset_into_safe_mode(SAFE_MODE_INTERRUPT_ERROR);
57+
reset_into_safe_mode(SAFE_MODE_INTERRUPT_ERROR);
4358
}
4459
nesting_count--;
45-
if (nesting_count > 0) {
46-
return;
60+
if (nesting_count == 0) {
61+
// return to the old priority setting
62+
__set_BASEPRI(oldBasePri);
63+
__isb(); // Instruction synchronization barrier
4764
}
48-
__dmb();
49-
asm volatile ("cpsie i" : : : "memory");
65+
restore_interrupts(my_interrupts);
5066
}
5167

5268
static bool next_reset_to_bootloader = false;

ports/raspberrypi/common-hal/picodvi/Framebuffer_RP2350.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -430,6 +430,7 @@ void common_hal_picodvi_framebuffer_construct(picodvi_framebuffer_obj_t *self,
430430
dma_hw->inte1 = (1u << self->dma_pixel_channel);
431431
irq_set_exclusive_handler(DMA_IRQ_1, dma_irq_handler);
432432
irq_set_enabled(DMA_IRQ_1, true);
433+
irq_set_priority(DMA_IRQ_1, PICO_HIGHEST_IRQ_PRIORITY);
433434

434435
bus_ctrl_hw->priority = BUSCTRL_BUS_PRIORITY_DMA_W_BITS | BUSCTRL_BUS_PRIORITY_DMA_R_BITS;
435436

ports/raspberrypi/supervisor/port.c

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,12 @@
5252
#include "pico/bootrom.h"
5353
#include "hardware/watchdog.h"
5454

55+
#ifdef PICO_RP2040
56+
#include "src/rp2_common/cmsis/stub/CMSIS/Device/RP0400/Include/RP0400.h"
57+
#else
58+
#include "src/rp2_common/cmsis/stub/CMSIS/Device/RP2350/Include/RP2350.h"
59+
#endif
60+
5561
#include "supervisor/shared/serial.h"
5662

5763
#include "tusb.h"
@@ -497,7 +503,15 @@ void port_interrupt_after_ticks(uint32_t ticks) {
497503
}
498504

499505
void port_idle_until_interrupt(void) {
500-
common_hal_mcu_disable_interrupts();
506+
// because we use interrupt priority, don't use
507+
// common_hal_mcu_disable_interrupts (because an interrupt masked by
508+
// BASEPRI will not occur)
509+
uint32_t state = save_and_disable_interrupts();
510+
511+
// Ensure BASEPRI is at 0...
512+
uint32_t oldBasePri = __get_BASEPRI();
513+
__set_BASEPRI(0);
514+
__isb();
501515
#if CIRCUITPY_USB_HOST
502516
if (!background_callback_pending() && !tud_task_event_ready() && !tuh_task_event_ready() && !_woken_up) {
503517
#else
@@ -506,7 +520,12 @@ void port_idle_until_interrupt(void) {
506520
__DSB();
507521
__WFI();
508522
}
509-
common_hal_mcu_enable_interrupts();
523+
524+
// and restore basepri before reenabling interrupts
525+
__set_BASEPRI(oldBasePri);
526+
__isb();
527+
528+
restore_interrupts(state);
510529
}
511530

512531
/**

0 commit comments

Comments
 (0)