rp2: use irq_add_shared_handler() for DMA_IRQ_0 instead of fixed isr_dma_0()

ports/raspberrypi/audio_dma.c

@@ -9,18 +9,23 @@
 #include "shared-bindings/audiocore/RawSample.h"
 #include "shared-bindings/audiocore/WaveFile.h"
 #include "shared-bindings/microcontroller/__init__.h"
-#include "bindings/rp2pio/StateMachine.h"
 #include "supervisor/background_callback.h"
 
 #include "py/mpstate.h"
 #include "py/runtime.h"
 
 #include "hardware/irq.h"
-#include "hardware/regs/intctrl.h" // For isr_ macro.
 
 
 #if CIRCUITPY_AUDIOCORE
 
+// audio_dma and rp2pio cooperate on DMA_IRQ_0 using the SDK's shared interrupt
+// handlers. We add our handler when we enable our first channel and remove it
+// when our last channel is disabled. See the DMA IRQ allocation notes in
+// common-hal/microcontroller/__init__.c.
+static void audio_dma_enable_irq(uint channel);
+static void audio_dma_disable_irq(uint channel);
+
 void audio_dma_reset(void) {
     for (size_t channel = 0; channel < NUM_DMA_CHANNELS; channel++) {
         if (MP_STATE_PORT(playing_audio)[channel] == NULL) {
@@ -335,12 +340,10 @@ audio_dma_result audio_dma_setup_playback(
             1, // transaction count
             false); // trigger
     } else {
-        // Clear any latent interrupts so that we don't immediately disable channels.
-        dma_hw->ints0 |= (1 << dma->channel[0]) | (1 << dma->channel[1]);
         // Enable our DMA channels on DMA_IRQ_0 to the CPU. This will wake us up when
         // we're WFI.
-        dma_hw->inte0 |= (1 << dma->channel[0]) | (1 << dma->channel[1]);
-        irq_set_mask_enabled(1 << DMA_IRQ_0, true);
+        audio_dma_enable_irq(dma->channel[0]);
+        audio_dma_enable_irq(dma->channel[1]);
     }
 
     dma->playing_in_progress = true;
@@ -353,16 +356,11 @@ void audio_dma_stop(audio_dma_t *dma) {
     dma->paused = true;
 
     // Disable our interrupts.
-    uint32_t channel_mask = 0;
     if (dma->channel[0] < NUM_DMA_CHANNELS) {
-        channel_mask |= 1 << dma->channel[0];
+        audio_dma_disable_irq(dma->channel[0]);
     }
     if (dma->channel[1] < NUM_DMA_CHANNELS) {
-        channel_mask |= 1 << dma->channel[1];
-    }
-    dma_hw->inte0 &= ~channel_mask;
-    if (!dma_hw->inte0) {
-        irq_set_mask_enabled(1 << DMA_IRQ_0, false);
+        audio_dma_disable_irq(dma->channel[1]);
     }
 
     // Run any remaining audio tasks because we remove ourselves from
@@ -532,34 +530,59 @@ static void dma_callback_fun(void *arg) {
     }
 }
 
-void __not_in_flash_func(isr_dma_0)(void) {
+// Shared DMA_IRQ_0 handler for audio. It acknowledges and services only audio
+// channels, leaving any other channels' interrupts for the other shared
+// handlers (e.g. rp2pio) to acknowledge.
+static void __not_in_flash_func(audio_dma_irq_handler)(void) {
     for (size_t i = 0; i < NUM_DMA_CHANNELS; i++) {
         uint32_t mask = 1 << i;
         if ((dma_hw->ints0 & mask) == 0) {
             continue;
         }
+        audio_dma_t *dma = MP_STATE_PORT(playing_audio)[i];
+        if (dma == NULL) {
+            // Not one of our channels; leave it for another shared handler.
+            continue;
+        }
         // acknowledge interrupt early. Doing so late means that you could lose an
         // interrupt if the buffer is very small and the DMA operation
         // completed by the time callback_add() / dma_complete() returned. This
         // affected PIO continuous write more than audio.
         dma_hw->ints0 = mask;
-        if (MP_STATE_PORT(playing_audio)[i] != NULL) {
-            audio_dma_t *dma = MP_STATE_PORT(playing_audio)[i];
-            // Record all channels whose DMA has completed; they need loading.
-            dma->channels_to_load_mask |= mask;
-            // Disable the channel so that we don't play it without filling it.
-            dma_hw->ch[i].al1_ctrl &= ~DMA_CH0_CTRL_TRIG_EN_BITS;
-            // This is a noop if the callback is already queued.
-            background_callback_add(&dma->callback, dma_callback_fun, (void *)dma);
-        }
-        if (MP_STATE_PORT(background_pio_read)[i] != NULL) {
-            rp2pio_statemachine_obj_t *pio = MP_STATE_PORT(background_pio_read)[i];
-            rp2pio_statemachine_dma_complete_read(pio, i);
-        }
-        if (MP_STATE_PORT(background_pio_write)[i] != NULL) {
-            rp2pio_statemachine_obj_t *pio = MP_STATE_PORT(background_pio_write)[i];
-            rp2pio_statemachine_dma_complete_write(pio, i);
-        }
+        // Record all channels whose DMA has completed; they need loading.
+        dma->channels_to_load_mask |= mask;
+        // Disable the channel so that we don't play it without filling it.
+        dma_hw->ch[i].al1_ctrl &= ~DMA_CH0_CTRL_TRIG_EN_BITS;
+        // This is a noop if the callback is already queued.
+        background_callback_add(&dma->callback, dma_callback_fun, (void *)dma);
+    }
+}
+
+// Channels (bitmask) audio currently has enabled on DMA_IRQ_0. Used to decide
+// when to add/remove our shared interrupt handler.
+static uint32_t audio_dma_irq0_channel_mask = 0;
+
+static void audio_dma_enable_irq(uint channel) {
+    // Clear any latent interrupt so that we don't immediately disable the channel.
+    dma_hw->ints0 = 1u << channel;
+    if (audio_dma_irq0_channel_mask == 0) {
+        irq_add_shared_handler(DMA_IRQ_0, audio_dma_irq_handler,
+            PICO_SHARED_IRQ_HANDLER_DEFAULT_ORDER_PRIORITY);
+    }
+    audio_dma_irq0_channel_mask |= 1u << channel;
+    dma_irqn_set_channel_enabled(0, channel, true);
+    irq_set_enabled(DMA_IRQ_0, true);
+}
+
+static void audio_dma_disable_irq(uint channel) {
+    dma_irqn_set_channel_enabled(0, channel, false);
+    audio_dma_irq0_channel_mask &= ~(1u << channel);
+    if (audio_dma_irq0_channel_mask == 0) {
+        irq_remove_handler(DMA_IRQ_0, audio_dma_irq_handler);
+    }
+    // Turn off the IRQ line entirely once no one (audio or rp2pio) needs it.
+    if (dma_hw->inte0 == 0) {
+        irq_set_enabled(DMA_IRQ_0, false);
     }
 }
 

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

@@ -25,6 +25,22 @@
 #include "hardware/watchdog.h"
 #include "hardware/irq.h"
 
+// DMA interrupt (DMA_IRQ_n) allocation for this port:
+//
+//   DMA_IRQ_0  Shared. audiocore (audio_dma.c) and rp2pio (StateMachine.c) each
+//              register a shared handler with irq_add_shared_handler() and
+//              service only their own DMA channels. Any new code that needs a
+//              DMA completion interrupt should do the same: add a shared handler
+//              on DMA_IRQ_0, check dma_hw->ints0, and acknowledge only its own
+//              channels. Runs at the default IRQ priority and is masked during
+//              flash writes (see common_hal_mcu_disable_interrupts below).
+//   DMA_IRQ_1  Exclusive to picodvi (Framebuffer_RP2040.c / Framebuffer_RP2350.c),
+//              registered with irq_set_exclusive_handler() at the highest
+//              priority. On RP2350 it is deliberately kept enabled during flash
+//              writes via BASEPRI (see below) so the display keeps refreshing.
+//   DMA_IRQ_2  RP2350 only; currently unused / free.
+//   DMA_IRQ_3  RP2350 only; currently unused / free.
+
 void common_hal_mcu_delay_us(uint32_t delay) {
     mp_hal_delay_us(delay);
 }

ports/raspberrypi/common-hal/rp2pio/StateMachine.c

@@ -68,14 +68,69 @@ static void rp2pio_statemachine_set_pull(pio_pinmask_t pull_pin_up, pio_pinmask_
     }
 }
 
+// audio_dma and rp2pio cooperate on DMA_IRQ_0 using the SDK's shared interrupt
+// handlers. We add our handler when we enable our first channel and remove it
+// when our last channel is disabled. See the DMA IRQ allocation notes in
+// common-hal/microcontroller/__init__.c.
+
+// Shared DMA_IRQ_0 handler for rp2pio background reads and writes. It
+// acknowledges and services only its own channels, leaving any other channels'
+// interrupts (e.g. audio) for the other shared handlers to acknowledge.
+static void __not_in_flash_func(rp2pio_dma_irq_handler)(void) {
+    for (size_t i = 0; i < NUM_DMA_CHANNELS; i++) {
+        uint32_t mask = 1 << i;
+        if ((dma_hw->ints0 & mask) == 0) {
+            continue;
+        }
+        rp2pio_statemachine_obj_t *read = MP_STATE_PORT(background_pio_read)[i];
+        rp2pio_statemachine_obj_t *write = MP_STATE_PORT(background_pio_write)[i];
+        if (read == NULL && write == NULL) {
+            // Not one of our channels; leave it for another shared handler.
+            continue;
+        }
+        // Acknowledge the interrupt early; see the comment in audio_dma.c.
+        dma_hw->ints0 = mask;
+        if (read != NULL) {
+            rp2pio_statemachine_dma_complete_read(read, i);
+        }
+        if (write != NULL) {
+            rp2pio_statemachine_dma_complete_write(write, i);
+        }
+    }
+}
+
+// Channels (bitmask) rp2pio currently has enabled on DMA_IRQ_0. Used to decide
+// when to add/remove our shared interrupt handler.
+static uint32_t rp2pio_dma_irq0_channel_mask = 0;
+
+static void rp2pio_dma_enable_irq(uint channel) {
+    // Clear any latent interrupt so that we don't immediately re-trigger.
+    dma_hw->ints0 = 1u << channel;
+    if (rp2pio_dma_irq0_channel_mask == 0) {
+        irq_add_shared_handler(DMA_IRQ_0, rp2pio_dma_irq_handler,
+            PICO_SHARED_IRQ_HANDLER_DEFAULT_ORDER_PRIORITY);
+    }
+    rp2pio_dma_irq0_channel_mask |= 1u << channel;
+    dma_irqn_set_channel_enabled(0, channel, true);
+    irq_set_enabled(DMA_IRQ_0, true);
+}
+
+static void rp2pio_dma_disable_irq(uint channel) {
+    dma_irqn_set_channel_enabled(0, channel, false);
+    rp2pio_dma_irq0_channel_mask &= ~(1u << channel);
+    if (rp2pio_dma_irq0_channel_mask == 0) {
+        irq_remove_handler(DMA_IRQ_0, rp2pio_dma_irq_handler);
+    }
+    // Turn off the IRQ line entirely once no one (audio or rp2pio) needs it.
+    if (dma_hw->inte0 == 0) {
+        irq_set_enabled(DMA_IRQ_0, false);
+    }
+}
+
 static void rp2pio_statemachine_clear_dma_write(int pio_index, int sm) {
     if (SM_DMA_ALLOCATED_WRITE(pio_index, sm)) {
         int channel_write = SM_DMA_GET_CHANNEL_WRITE(pio_index, sm);
-        uint32_t channel_mask_write = 1u << channel_write;
-        dma_hw->inte0 &= ~channel_mask_write;
-        if (!dma_hw->inte0) {
-            irq_set_mask_enabled(1 << DMA_IRQ_0, false);
-        }
+        rp2pio_dma_disable_irq(channel_write);
         MP_STATE_PORT(background_pio_write)[channel_write] = NULL;
         dma_channel_abort(channel_write);
         dma_channel_unclaim(channel_write);
@@ -86,11 +141,7 @@ static void rp2pio_statemachine_clear_dma_write(int pio_index, int sm) {
 static void rp2pio_statemachine_clear_dma_read(int pio_index, int sm) {
     if (SM_DMA_ALLOCATED_READ(pio_index, sm)) {
         int channel_read = SM_DMA_GET_CHANNEL_READ(pio_index, sm);
-        uint32_t channel_mask_read = 1u << channel_read;
-        dma_hw->inte0 &= ~channel_mask_read;
-        if (!dma_hw->inte0) {
-            irq_set_mask_enabled(1 << DMA_IRQ_0, false);
-        }
+        rp2pio_dma_disable_irq(channel_read);
         MP_STATE_PORT(background_pio_read)[channel_read] = NULL;
         dma_channel_abort(channel_read);
         dma_channel_unclaim(channel_read);
@@ -1274,12 +1325,8 @@ bool common_hal_rp2pio_statemachine_background_write(rp2pio_statemachine_obj_t *
 
     common_hal_mcu_disable_interrupts();
 
-    // Acknowledge any previous pending interrupt
-    dma_hw->ints0 |= 1u << channel_write;
     MP_STATE_PORT(background_pio_write)[channel_write] = self;
-    dma_hw->inte0 |= 1u << channel_write;
-
-    irq_set_mask_enabled(1 << DMA_IRQ_0, true);
+    rp2pio_dma_enable_irq(channel_write);
     dma_start_channel_mask(1u << channel_write);
     common_hal_mcu_enable_interrupts();
 
@@ -1438,11 +1485,8 @@ bool common_hal_rp2pio_statemachine_background_read(rp2pio_statemachine_obj_t *s
         false);
 
     common_hal_mcu_disable_interrupts();
-    // Acknowledge any previous pending interrupt
-    dma_hw->ints0 |= 1u << channel_read;
     MP_STATE_PORT(background_pio_read)[channel_read] = self;
-    dma_hw->inte0 |= 1u << channel_read;
-    irq_set_mask_enabled(1 << DMA_IRQ_0, true);
+    rp2pio_dma_enable_irq(channel_read);
     dma_start_channel_mask((1u << channel_read));
     common_hal_mcu_enable_interrupts();
 

tests/circuitpython-manual/rp2pio/README.md

@@ -0,0 +1,44 @@
+# rp2pio + audio DMA shared-IRQ manual tests
+
+These programs exercise the `DMA_IRQ_0` interrupt handlers on the RP2040 /
+RP2350, which are shared between `audiocore`/`audiopwmio` (audio DMA) and
+`rp2pio` (PIO background read/write DMA).
+
+They were written to validate the change in issue #9992 (moving `audio_dma.c`
+off the fixed `isr_dma_0()` linker symbol and onto `irq_add_shared_handler()`,
+with `rp2pio` registering its own shared handler). **They use only public APIs
+that predate that change, so each program should behave identically before and
+after it** — that is the point: they are regression tests, not feature tests.
+
+## What each program covers
+
+| Program | Audio DMA | PIO write DMA | PIO read DMA |
+|---|---|---|---|
+| `pio_background_write.py`        |     | ✓ |   |
+| `audio_dma_shared_irq_write.py`  | ✓   | ✓ |   |
+| `audio_dma_shared_irq_loopback.py` | ✓ | ✓ | ✓ |
+
+Each goes through the background-DMA path (`background_write` /
+`background_read`), which is interrupt-driven. The blocking `write`/`readinto`/
+`write_readinto` calls poll instead and do **not** use the DMA IRQ, so they are
+deliberately not used here. The audio sample is created with
+`single_buffer=False` so audio also uses the interrupt-driven (double-buffered)
+DMA path rather than the no-interrupt single-buffer chaining path.
+
+## Wiring (Raspberry Pi Pico / Pico 2)
+
+- `GP13` — PWM audio output. Connect to an amplifier/speaker, or just leave it;
+  the test does not require you to hear anything, only that playback keeps
+  running.
+- `GP2`  — PIO output pin.
+- `GP3`  — PIO input pin (loopback test only). For a meaningful loopback,
+  connect `GP2` to `GP3` with a jumper. Without the jumper the read still
+  completes (it just reads whatever the floating/pulled pin sees), so the DMA
+  path is still exercised.
+
+## Running
+
+Copy one file at a time to `CIRCUITPY/code.py` and watch the serial console.
+A successful run ends with a `PASS:` line. A hang (no further output) or a
+`RuntimeError`/crash/safe-mode indicates a regression in the shared DMA IRQ
+handling.