Skip to content

Some PIO examples are not working on RP2350/Pico 2 #83

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

Open
magy00 opened this issue Jan 14, 2025 · 8 comments
Open

Some PIO examples are not working on RP2350/Pico 2 #83

magy00 opened this issue Jan 14, 2025 · 8 comments

Comments

@magy00
Copy link

magy00 commented Jan 14, 2025

MicroPython v1.25.0-preview.187.g1b4c969ce on 2025-01-14; Raspberry Pi Pico2 with RP2350

pio_1hz.py prints:
ValueError: freq out of range

A quick fix would probably be machine.freq(125_000_000) but the LED only switches on and never off (possibly needs some sleep() at the end).

pio_blink.py works if machine.freq(125_000_000) is prepended, but if the state engine id is changed from 0 to 811 then nothing happens.

pio_exec.py works even when the state engine id is changed from 0 to 811.

pio_pwm.py works as long as the state engine id is not changed to 811 (i.e. 07 are OK).

@lurch
Copy link
Contributor

lurch commented Jan 14, 2025

ping @peterharperuk

@magy00
Copy link
Author

magy00 commented Jan 16, 2025

It looks like it may be a bug in micropython about PIO2: There are only enums PROG_OFFSET_PIO0 and PROG_OFFSET_PIO1 in ports/rp2/rp2_pio.c and ports/rp2/modules/rp2.py. It's actually not clear to me why PROG_OFFSET_PIO1is needed because a unique list is apparently allocated for every program. Maybe all occurrences of PROG_OFFSET_PIO0 + pio_get_index(self->pio) could just be replaced by PROG_OFFSET_PIO0 itself (and the free PROG_OFFSET_PIO1 field could be used to implement different loading and start offsets, possibly renamed)?

@lurch
Copy link
Contributor

lurch commented Jan 16, 2025

It looks like it may be a bug in micropython about PIO2

I think you might be right. From a quick look at the rp2_pio.c that you linked to, we have the enum:

enum {
    PROG_DATA,
    PROG_OFFSET_PIO0,
    PROG_OFFSET_PIO1,
    PROG_EXECCTRL,
    PROG_SHIFTCTRL,
    PROG_OUT_PINS,
    PROG_SET_PINS,
    PROG_SIDESET_PINS,
    PROG_MAX_FIELDS,
};

so PROG_DATA will have the value 0, PROG_OFFSET_PIO0 will have the value 1, PROG_OFFSET_PIO1 will have the value 2, PROG_EXECCTRL will have the value 3, etc.
And pio_get_index(self->pio) will return 0 for PIO0, 1 for PIO1 and 2 for PIO2. So if you try to add a program to state machine 9 (i.e. the second state-machine on PIO2), then PROG_OFFSET_PIO0 + pio_get_index(self->pio) will evaluate to 3, which is the value of PROG_EXECCTRL, and presumably that's why things are going wrong.
I agree that it doesn't seem to make sense to store separate offsets for PIO0 and PIO1, when a particular program only gets loaded into one PIO.

So pinging also @dpgeorge (I wonder if this issue should get closed, and a new issue be opened at https://github.com/micropython/micropython ?)

@lurch
Copy link
Contributor

lurch commented Jan 16, 2025

pio_1hz.py prints:
ValueError: freq out of range

Ahhh, looks like this is #18 biting us again! 😂 (see additional discussion in micropython/micropython#7025 )
With a 16-bit clock divisor, and the Pico2 running by default at 150MHz, I guess the minimum PIO frequency is now 2289 Hz, so I guess the low-speed PIO examples need to be tweaked again, similarly to what happened in #26

@magy00
Copy link
Author

magy00 commented Jan 16, 2025

Yes; maybe increase to 4 kHz (but then the delay slots look more complicated to a beginner)?

diff --git pio/pio_1hz.py pio/pio_1hz.py
index f891a25..b42b09c 100644
--- pio/pio_1hz.py
+++ pio/pio_1hz.py
@@ -7,27 +7,29 @@ import rp2

 @rp2.asm_pio(set_init=rp2.PIO.OUT_LOW)
 def blink_1hz():
-    # Cycles: 1 + 1 + 6 + 32 * (30 + 1) = 1000
+    # Cycles: 1 + 1 + 14 + 32 * (31 + 31) = 2000
     irq(rel(0))
     set(pins, 1)
-    set(x, 31)                  [5]
+    set(x, 31)                  [13]
     label("delay_high")
-    nop()                       [29]
-    jmp(x_dec, "delay_high")
+    nop()                       [30]
+    jmp(x_dec, "delay_high")    [30]

-    # Cycles: 1 + 7 + 32 * (30 + 1) = 1000
+    # Cycles: 1 + 15 + 32 * (31 + 31) = 2000
     set(pins, 0)
-    set(x, 31)                  [6]
+    set(x, 31)                  [14]
     label("delay_low")
-    nop()                       [29]
-    jmp(x_dec, "delay_low")
+    nop()                       [30]
+    jmp(x_dec, "delay_low")     [30]


 # Create the StateMachine with the blink_1hz program, outputting on Pin(25).
-sm = rp2.StateMachine(0, blink_1hz, freq=2000, set_base=Pin(25))
+sm = rp2.StateMachine(0, blink_1hz, freq=4000, set_base=Pin(25))

 # Set the IRQ handler to print the millisecond timestamp.
 sm.irq(lambda p: print(time.ticks_ms()))

 # Start the StateMachine.
 sm.active(1)
+time.sleep(3)
+sm.active(0)

@lurch
Copy link
Contributor

lurch commented Jan 16, 2025

then the delay slots look more complicated to a beginner

I wonder if:

    nop()                       [30]
    nop()                       [30]
    jmp(x_dec, "delay_low")

is "easier to read" than

    nop()                       [30]
    jmp(x_dec, "delay_low")     [30]

?
(and we probably don't want the additional time.sleep(3); sm.active(0) on the end?)

@magy00
Copy link
Author

magy00 commented Jan 16, 2025

I think one would be [29] and the other [30] (or [31] and [28])? I guess it will also depend on the accompanying documentation that might need to be updated.

With my Pico 2, the LED just switches on and doesn't blink at all without the sleep…

@PaulskPt
Copy link

PaulskPt commented Mar 17, 2025

Regarding the pio_blink.py example. I discovered by cut and try method that, in line 23, the minimum value for "freq= " is 2289. I am using "MicroPython v1.24.1 on 2024-11-29; Raspberry Pi Pico2 with RP2350", board: iLabs iPico (RP2350).
Below that value a "ValueError: freq out of range" occurred. My findings are equal according to what @lurch wrote above: "Ahhh, looks like this is #18 biting us again..."

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants