Skip to content
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.

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

raspberrypi: Don't block DMA_IRQ_1 during common_hal_mcu_disable_interrupts #10049

Merged
merged 1 commit into from
Feb 12, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions ports/raspberrypi/common-hal/microcontroller/__init__.c
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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;

Expand Down
1 change: 1 addition & 0 deletions ports/raspberrypi/common-hal/picodvi/Framebuffer_RP2350.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
30 changes: 30 additions & 0 deletions ports/raspberrypi/supervisor/port.c
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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) {
Expand All @@ -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
}

/**
Expand Down
Loading