Skip to content

fix pio_encode_mov in release mode - #3072

Open
kilograham wants to merge 3 commits into
developfrom
pio-encoding-fix
Open

fix pio_encode_mov in release mode#3072
kilograham wants to merge 3 commits into
developfrom
pio-encoding-fix

Conversation

@kilograham

Copy link
Copy Markdown
Contributor

fixes #3068

@kilograham
kilograham requested a review from will-v-pi July 14, 2026 23:41
@kilograham kilograham added this to the 2.3.1 milestone Jul 14, 2026
@cadwal

cadwal commented Jul 28, 2026

Copy link
Copy Markdown

I can confirm this fixes the issue.

On a Pimoroni Presto (RP2350B, ST7701 panel driven by PIO + DMA) the display comes up lit and permanently black on 2.3.0, and renders normally on 2.1.1.

The driver clears the OSR before restarting a frame:

pio_sm_set_enabled(pio, sm, false);
pio_sm_clear_fifos(pio, sm);
pio_sm_exec_wait_blocking(pio, sm, pio_encode_mov(pio_osr, pio_null));
pio_sm_exec_wait_blocking(pio, sm, pio_encode_out(pio_null, 32));

In Release on 2.3.0 the first instruction encodes as MOV PINDIRS, NULL, so the OSR is never cleared. With autopull enabled the following OUT NULL, 32 stalls on a TX FIFO that was just cleared with its DMA aborted, and pio_sm_exec_wait_blocking never returns.

One small thing in the new test — the pio_pindirs_mov check is guarded by #if PICO_PIO_VERSION > 1, but pio_pindirs_mov is defined under #if PICO_PIO_VERSION > 0. > On RP2350 (PIO version 1) that assertion compiles out, so the case closest to the original #2946 fix is not actually covered. Looks like it should be > 0?

Comment thread test/pio_encoding_test/pio_encoding_test.c Outdated
Co-authored-by: Andrew Scheller <andrew.scheller@raspberrypi.com>

@will-v-pi will-v-pi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants