diff --git a/ports/raspberrypi/audio_dma.c b/ports/raspberrypi/audio_dma.c index 03d98ae876a09..36b86cbf8449b 100644 --- a/ports/raspberrypi/audio_dma.c +++ b/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); } } diff --git a/ports/raspberrypi/common-hal/microcontroller/__init__.c b/ports/raspberrypi/common-hal/microcontroller/__init__.c index 49dbc6082b0f5..3d8bd381f5e7b 100644 --- a/ports/raspberrypi/common-hal/microcontroller/__init__.c +++ b/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); } diff --git a/ports/raspberrypi/common-hal/rp2pio/StateMachine.c b/ports/raspberrypi/common-hal/rp2pio/StateMachine.c index af6e98bbc85cc..8e1ea26ab0da6 100644 --- a/ports/raspberrypi/common-hal/rp2pio/StateMachine.c +++ b/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(); diff --git a/tests/circuitpython-manual/rp2pio/README.md b/tests/circuitpython-manual/rp2pio/README.md new file mode 100644 index 0000000000000..de67cf203d2ea --- /dev/null +++ b/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. diff --git a/tests/circuitpython-manual/rp2pio/audio_dma_shared_irq_loopback.py b/tests/circuitpython-manual/rp2pio/audio_dma_shared_irq_loopback.py new file mode 100644 index 0000000000000..ba8ebbb589df8 --- /dev/null +++ b/tests/circuitpython-manual/rp2pio/audio_dma_shared_irq_loopback.py @@ -0,0 +1,85 @@ +"""Audio + rp2pio background write AND read sharing DMA_IRQ_0. + +Exercises all three DMA_IRQ_0 dispatch arms at once: + * audio playback (double-buffered, interrupt-driven), + * a continuous PIO background_write (write-completion interrupts), and + * repeated PIO background_read one-shots (read-completion interrupts). + +The continuous background_write keeps the state machine clocking so the read +side always has data to capture. Each background_read is a one-shot whose +completion is detected by polling `StateMachine.reading`; if that never clears, +the read-completion interrupt is not being serviced (a regression). Audio must +also keep playing throughout. + +Wiring: PWM audio on GP13 (speaker optional), PIO output on GP2, PIO input on +GP3. Jumper GP2 -> GP3 for a true loopback (then the captured data reflects what +was written); without the jumper the read still completes, so the DMA path is +still exercised. +""" + +import array +import math +import time + +import audiocore +import audiopwmio +import board +import rp2pio + +# --- Audio: signed-16 sine wave, double-buffered so it uses the DMA IRQ path. +SAMPLE_RATE = 16000 +SAMPLE_COUNT = 4000 # even count (required for single_buffer=False) +PERIODS = 32 +sine = array.array("h", [0] * SAMPLE_COUNT) +for i in range(SAMPLE_COUNT): + sine[i] = int(math.sin(2 * math.pi * PERIODS * i / SAMPLE_COUNT) * (2**14)) +sample = audiocore.RawSample(sine, sample_rate=SAMPLE_RATE, single_buffer=False) + +audio = audiopwmio.PWMAudioOut(board.GP13) +audio.play(sample, loop=True) +print("audio playing:", audio.playing) + +# --- PIO loopback program (hand assembled): +# .wrap_target +# out pins, 1 ; 0x6001 shift one bit OSR -> output pin (GP2) +# in pins, 1 ; 0x4001 shift one bit input pin (GP3) -> ISR +# .wrap +# auto_pull/auto_push at 32 bits keep both FIFOs (and their DMA) moving. +PROGRAM = array.array("H", [0x6001, 0x4001]) +sm = rp2pio.StateMachine( + PROGRAM, + frequency=2_000_000, + first_out_pin=board.GP2, + first_in_pin=board.GP3, + auto_pull=True, + pull_threshold=32, + out_shift_right=True, + auto_push=True, + push_threshold=32, + in_shift_right=True, +) + +out_buf = array.array("I", [0x0F0F0F0F] * 1024) +in_buf = array.array("I", [0] * 1024) + +# Continuous background write to keep the state machine clocking. +sm.background_write(loop=out_buf) + +ITERATIONS = 100 +for n in range(ITERATIONS): + sm.background_read(once=in_buf) + deadline = time.monotonic() + 2.0 + while sm.reading and time.monotonic() < deadline: + pass + if sm.reading: + raise RuntimeError(f"background_read stuck on iteration {n} (shared DMA IRQ wedged?)") + if not audio.playing: + raise RuntimeError(f"audio stopped unexpectedly on iteration {n}") + if n % 20 == 0: + print("iteration", n, "audio.playing", audio.playing, "first read word", hex(in_buf[0])) + +sm.stop_background_write() +audio.stop() +sm.deinit() +audio.deinit() +print("PASS: audio + rp2pio background write/read shared DMA_IRQ_0 for", ITERATIONS, "iterations") diff --git a/tests/circuitpython-manual/rp2pio/audio_dma_shared_irq_write.py b/tests/circuitpython-manual/rp2pio/audio_dma_shared_irq_write.py new file mode 100644 index 0000000000000..fe00df0681527 --- /dev/null +++ b/tests/circuitpython-manual/rp2pio/audio_dma_shared_irq_write.py @@ -0,0 +1,67 @@ +"""Audio playback + rp2pio background_write sharing DMA_IRQ_0. + +This is the scenario from issue #9868: audio output and a PIO peripheral (e.g. a +status NeoPixel) both want DMA completion interrupts at the same time. Audio +uses the interrupt-driven double-buffered path (`single_buffer=False`) and +rp2pio uses background_write, so both subsystems are serviced by DMA_IRQ_0 +concurrently. + +The test passes if the PIO writes keep completing AND audio keeps playing for +the whole run. A hang, crash, or audio stopping early indicates a regression. + +Wiring: PWM audio on GP13 (speaker optional), PIO output on GP2 (no connection +required). +""" + +import array +import math +import time + +import audiocore +import audiopwmio +import board +import rp2pio + +# --- Audio: signed-16 sine wave, double-buffered so it uses the DMA IRQ path. +SAMPLE_RATE = 16000 +SAMPLE_COUNT = 4000 # even count (required for single_buffer=False), several periods +PERIODS = 32 +sine = array.array("h", [0] * SAMPLE_COUNT) +for i in range(SAMPLE_COUNT): + sine[i] = int(math.sin(2 * math.pi * PERIODS * i / SAMPLE_COUNT) * (2**14)) +sample = audiocore.RawSample(sine, sample_rate=SAMPLE_RATE, single_buffer=False) + +audio = audiopwmio.PWMAudioOut(board.GP13) +audio.play(sample, loop=True) +print("audio playing:", audio.playing) + +# --- PIO: drain program that clocks the TX FIFO out on GP2 (see +# pio_background_write.py for the encoding). +PROGRAM = array.array("H", [0x6001]) +sm = rp2pio.StateMachine( + PROGRAM, + frequency=2_000_000, + first_out_pin=board.GP2, + auto_pull=True, + pull_threshold=32, + out_shift_right=True, +) +data = array.array("I", [0x5555AAAA] * 4096) + +ITERATIONS = 100 +for n in range(ITERATIONS): + sm.background_write(once=data) + deadline = time.monotonic() + 2.0 + while sm.writing and time.monotonic() < deadline: + pass + if sm.writing: + raise RuntimeError(f"background_write stuck on iteration {n} (shared DMA IRQ wedged?)") + if not audio.playing: + raise RuntimeError(f"audio stopped unexpectedly on iteration {n}") + if n % 20 == 0: + print("iteration", n, "audio.playing", audio.playing) + +audio.stop() +sm.deinit() +audio.deinit() +print("PASS: audio + rp2pio background_write shared DMA_IRQ_0 for", ITERATIONS, "iterations") diff --git a/tests/circuitpython-manual/rp2pio/pio_background_write.py b/tests/circuitpython-manual/rp2pio/pio_background_write.py new file mode 100644 index 0000000000000..796175da7b1e3 --- /dev/null +++ b/tests/circuitpython-manual/rp2pio/pio_background_write.py @@ -0,0 +1,53 @@ +"""Baseline: rp2pio background_write DMA only (no audio). + +Exercises the PIO *write* arm of the shared DMA_IRQ_0 handler, plus the +add/remove of the shared handler as the only channel comes and goes. + +Repeatedly starts a one-shot background write and waits for it to finish by +polling `StateMachine.writing`. If `writing` never clears, the DMA completion +interrupt is not being serviced (a regression). Ends with a PASS line. + +Wiring: PIO output on GP2 (no connection required). +""" + +import array +import time + +import board +import rp2pio + +# PIO program (hand assembled, no adafruit_pioasm dependency): +# .wrap_target +# out pins, 1 ; shift one bit from OSR to the output pin +# .wrap +# With auto_pull and pull_threshold=32, the OSR is refilled from the TX FIFO +# every 32 bits, so the FIFO (and therefore the DMA) drains continuously. +# Encoding of `out pins, 1`: opcode OUT=0b011, dest PINS=0b000, count 1 -> 0x6001 +PROGRAM = array.array("H", [0x6001]) + +sm = rp2pio.StateMachine( + PROGRAM, + frequency=2_000_000, + first_out_pin=board.GP2, + auto_pull=True, + pull_threshold=32, + out_shift_right=True, +) + +# 16 KB of data per transfer, enough that the DMA genuinely runs and completes +# via interrupt rather than fitting in the FIFO. +data = array.array("I", [0x5555AAAA] * 4096) + +ITERATIONS = 50 +for n in range(ITERATIONS): + sm.background_write(once=data) + deadline = time.monotonic() + 2.0 + while sm.writing and time.monotonic() < deadline: + pass + if sm.writing: + raise RuntimeError(f"background_write stuck on iteration {n} (DMA IRQ not serviced?)") + if n % 10 == 0: + print("iteration", n, "ok") + +sm.deinit() +print("PASS: rp2pio background_write completed", ITERATIONS, "times")