Merge pull request #10049 from jepler/keep-dma-irq-enabled-rp2350

raspberrypi: Don't block DMA_IRQ_1 during common_hal_mcu_disable_interrupts

Author: jepler

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

@@ -23,12 +23,14 @@
 #include "src/rp2_common/hardware_sync/include/hardware/sync.h"
 
 #include "hardware/watchdog.h"
+#include "hardware/irq.h"
 
 void common_hal_mcu_delay_us(uint32_t delay) {
     mp_hal_delay_us(delay);
 }
 
 volatile uint32_t nesting_count = 0;
+#ifdef PICO_RP2040
 void common_hal_mcu_disable_interrupts(void) {
     // We don't use save_and_disable_interrupts() from the sdk because we don't want to worry about PRIMASK.
     // This is what we do on the SAMD21 via CMSIS.
@@ -48,6 +50,38 @@ void common_hal_mcu_enable_interrupts(void) {
     __dmb();
     asm volatile ("cpsie i" : : : "memory");
 }
+#else
+#include "src/rp2_common/cmsis/stub/CMSIS/Device/RP2350/Include/RP2350.h"
+#define PICO_ELEVATED_IRQ_PRIORITY (0x60) // between PICO_DEFAULT and PIOCO_HIGHEST_IRQ_PRIORITY
+static uint32_t oldBasePri = 0; // 0 (default) masks nothing, other values mask equal-or-larger priority values
+void common_hal_mcu_disable_interrupts(void) {
+    if (nesting_count == 0) {
+        // We must keep DMA_IRQ_1 (reserved for pico dvi) enabled at all times,
+        // including during flash writes. Do this by setting the priority mask (BASEPRI
+        // register).
+        // grab old base priority
+        oldBasePri = __get_BASEPRI();
+        // and set the new one
+        __set_BASEPRI_MAX(PICO_ELEVATED_IRQ_PRIORITY);
+        __isb(); // Instruction synchronization barrier
+    }
+    nesting_count++;
+}
+
+void common_hal_mcu_enable_interrupts(void) {
+    uint32_t my_interrupts = save_and_disable_interrupts();
+    if (nesting_count == 0) {
+        reset_into_safe_mode(SAFE_MODE_INTERRUPT_ERROR);
+    }
+    nesting_count--;
+    if (nesting_count == 0) {
+        // return to the old priority setting
+        __set_BASEPRI(oldBasePri);
+        __isb(); // Instruction synchronization barrier
+    }
+    restore_interrupts(my_interrupts);
+}
+#endif
 
 static bool next_reset_to_bootloader = false;
 

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

@@ -430,6 +430,7 @@ void common_hal_picodvi_framebuffer_construct(picodvi_framebuffer_obj_t *self,
     dma_hw->inte1 = (1u << self->dma_pixel_channel);
     irq_set_exclusive_handler(DMA_IRQ_1, dma_irq_handler);
     irq_set_enabled(DMA_IRQ_1, true);
+    irq_set_priority(DMA_IRQ_1, PICO_HIGHEST_IRQ_PRIORITY);
 
     bus_ctrl_hw->priority = BUSCTRL_BUS_PRIORITY_DMA_W_BITS | BUSCTRL_BUS_PRIORITY_DMA_R_BITS;
 

ports/raspberrypi/supervisor/port.c

@@ -52,6 +52,10 @@
 #include "pico/bootrom.h"
 #include "hardware/watchdog.h"
 
+#ifdef PICO_RP2350
+#include "src/rp2_common/cmsis/stub/CMSIS/Device/RP2350/Include/RP2350.h"
+#endif
+
 #include "supervisor/shared/serial.h"
 
 #include "tusb.h"
@@ -497,6 +501,7 @@ void port_interrupt_after_ticks(uint32_t ticks) {
 }
 
 void port_idle_until_interrupt(void) {
+    #ifdef PICO_RP2040
     common_hal_mcu_disable_interrupts();
     #if CIRCUITPY_USB_HOST
     if (!background_callback_pending() && !tud_task_event_ready() && !tuh_task_event_ready() && !_woken_up) {
@@ -507,6 +512,31 @@ void port_idle_until_interrupt(void) {
         __WFI();
     }
     common_hal_mcu_enable_interrupts();
+    #else
+    // because we use interrupt priority, don't use
+    // common_hal_mcu_disable_interrupts (because an interrupt masked by
+    // BASEPRI will not occur)
+    uint32_t state = save_and_disable_interrupts();
+
+    // Ensure BASEPRI is at 0...
+    uint32_t oldBasePri = __get_BASEPRI();
+    __set_BASEPRI(0);
+    __isb();
+    #if CIRCUITPY_USB_HOST
+    if (!background_callback_pending() && !tud_task_event_ready() && !tuh_task_event_ready() && !_woken_up) {
+    #else
+    if (!background_callback_pending() && !tud_task_event_ready() && !_woken_up) {
+        #endif
+        __DSB();
+        __WFI();
+    }
+
+    // and restore basepri before reenabling interrupts
+    __set_BASEPRI(oldBasePri);
+    __isb();
+
+    restore_interrupts(state);
+    #endif
 }
 
 /**