pioasm & sdk support of `wait gpio` on rp2350b

jepler commented

2024-12-18 15:42:31 -06:00

(Migrated from github.com)

Hi! Over in CircuitPython I'm working on proper support for the high IO bank on the RP2350B.

As part of that, I think I've found a problem in pioasm and maybe add_program_at_offset.

I have the following (useless) pio program:

.pio_version 1
.program baker
    wait 0 gpio 18
    wait 0 gpio 34
    .word 0x2022

pioasm from 2.1.0 (& tip of develop, 2.1.0-6-g969f589) compiles it to the following:

static const uint16_t baker_program_instructions[] = {
            //     .wrap_target
    0x2012, //  0: wait   0 gpio, 18                 
    0x2022, //  1: wait   0 pin, 34                  
    0x2022, //  2: wait   0 pin, 2                   
            //     .wrap
};

Notice how the instruction encoding 0x2022 is used for both the wait gpio and wait pin, and the instruction written as wait 0 gpio 34 has decoded to the impossible (?) wait 0 pin, 34.

I'm not sure what needs to happen here. I am pretty sure that at runtime you need a PIO peripheral with its gpio_offset set to 16, and that the instruction encodings loaded into instr_mem should actually be 0x2002 (gpio 16+2 = 18) and 0x2012 (gpio 16+18=34). but I don't know whether this should be the encoding in the program_instructions[] array, or if add_program_at_offset in the sdk should take care of altering the wait gpio instruction encodings similar to how it alters jmp encodings depending on the load address.

This bears directly on how we implement this in CircuitPython, since we want to use pio_add_program and friends to load the instructions into the PIO peripheral, but also use our own python implementation of pioasm.

Hi! Over in CircuitPython I'm working on proper support for the high IO bank on the RP2350B. As part of that, I think I've found a problem in pioasm and maybe `add_program_at_offset`. I have the following (useless) pio program: ``` .pio_version 1 .program baker wait 0 gpio 18 wait 0 gpio 34 .word 0x2022 ``` pioasm from 2.1.0 (& tip of develop, 2.1.0-6-g969f589) compiles it to the following: ``` static const uint16_t baker_program_instructions[] = { // .wrap_target 0x2012, // 0: wait 0 gpio, 18 0x2022, // 1: wait 0 pin, 34 0x2022, // 2: wait 0 pin, 2 // .wrap }; ``` Notice how the instruction encoding 0x2022 is used for both the `wait gpio` and `wait pin`, and the instruction written as `wait 0 gpio 34` has decoded to the impossible (?) `wait 0 pin, 34`. I'm not sure what needs to happen here. I am pretty sure that at runtime you need a PIO peripheral with its gpio_offset set to 16, and that the instruction encodings loaded into `instr_mem` should actually be `0x2002` (gpio 16+2 = 18) and `0x2012` (gpio 16+18=34). but I don't know whether this should be the encoding in the `program_instructions[]` array, or if `add_program_at_offset` in the sdk should take care of altering the `wait gpio` instruction encodings similar to how it alters `jmp` encodings depending on the load address. This bears directly on how we implement this in CircuitPython, since we want to use `pio_add_program` and friends to load the instructions into the PIO peripheral, but also use our own python implementation of pioasm.

lurch commented

2024-12-18 19:44:10 -06:00

(Migrated from github.com)

Looking at section 11.4.3. of https://datasheets.raspberrypi.com/rp2350/rp2350-datasheet.pdf shows that Index is only encoded with 5 bits (and thus has a maximum value of 32), which I guess means that the mapping of gpio34 -> gpio18 has to be done before encoding things into the program_instructions[] array? 🤔 And presumably at the moment trying to set an Index field of 34 is overflowing into the Source field? Which would explain why wait 0 gpio 34 is encoding to the same thing as wait 0 pin, 2 ( because 34 = (1 << 5) | (2 & 0x1f) ) ?

Looking at https://github.com/raspberrypi/pico-sdk/blob/develop/tools/pioasm/pio_assembler.cpp#L394 it seems that it does keep track of which GPIOs are being used, so I guess the part of pioasm that does the final output needs to be modified to take program.used_gpio_ranges into account when calculation the WAIT gpio Index?
And I believe that this would need to be done at the "output stage" rather than "parse stage" because if you have wait 0 gpio 20 followed by wait 0 gpio 1 then the GPIO20 would get encoded to Index 20 (because the later GPIO1 forces the use of the "lower" GPIO range); but if you had wait 0 gpio 20 followed by wait 0 gpio 34 then the GPIO20 would get encoded to Index 4 (because the later GPIO34 forces the use of the "higher" GPIO range).

(This is all just my educated guess - I could be entirely wrong! 😂 )

Looking at section 11.4.3. of https://datasheets.raspberrypi.com/rp2350/rp2350-datasheet.pdf shows that `Index` is only encoded with 5 bits (and thus has a maximum value of 32), which I guess means that the mapping of gpio34 -> gpio18 **has** to be done before encoding things into the `program_instructions[]` array? :thinking: And presumably at the moment trying to set an `Index` field of 34 is overflowing into the `Source` field? Which would explain why `wait 0 gpio 34` is encoding to the same thing as `wait 0 pin, 2` ( because `34 = (1 << 5) | (2 & 0x1f)` ) ? Looking at https://github.com/raspberrypi/pico-sdk/blob/develop/tools/pioasm/pio_assembler.cpp#L394 it seems that it _does_ keep track of which GPIOs are being used, so I guess the part of `pioasm` that does the final output needs to be modified to take `program.used_gpio_ranges` into account when calculation the WAIT gpio `Index`? And I believe that this would need to be done at the "output stage" rather than "parse stage" because if you have `wait 0 gpio 20` followed by `wait 0 gpio 1` then the GPIO20 would get encoded to `Index` 20 (because the later GPIO1 forces the use of the "lower" GPIO range); but if you had `wait 0 gpio 20` followed by `wait 0 gpio 34` then the GPIO20 would get encoded to `Index` 4 (because the later GPIO34 forces the use of the "higher" GPIO range). (This is all just my educated guess - I could be entirely wrong! :joy: )

jepler commented

2024-12-18 20:20:48 -06:00

(Migrated from github.com)

without presuming how the implementation should look, I thought of two possibilities: First, that the encoding in program_instructions just takes the lower 5 bits (so wait 0 gpio 34 would encode to 0x2002), and then ``add_program_at_offsetperformsinstr ^ 0x10onwait gpio` instructions when loading them, if the high gpio range is marked as used.

The second one is what I understand from your reply: when pioasm gets to the output stage, and the high gpio range is marked as used, it outputs 0x2002 for wait 0 gpio 18 and 0x2012 for wait 0 gpio 34. In that case, no change is needed to add_program_at_offset. Not requiring a change to the runtime sdk code does seem like it could be preferable.

without presuming how the implementation should look, I thought of two possibilities: First, that the encoding in `program_instructions` just takes the lower 5 bits (so wait 0 gpio 34 would encode to 0x2002), and then ``add_program_at_offset` performs `instr ^ 0x10` on `wait gpio` instructions when loading them, if the high gpio range is marked as used. The second one is what I understand from your reply: when pioasm gets to the output stage, and the high gpio range is marked as used, it outputs `0x2002` for `wait 0 gpio 18` and `0x2012` for `wait 0 gpio 34`. In that case, no change is needed to `add_program_at_offset`. Not requiring a change to the runtime sdk code does seem like it could be preferable.

jepler commented

2024-12-18 20:22:30 -06:00

(Migrated from github.com)

I see there's a file .../pioasm/test/amethyst.pio that has

.program wibble2
.pio_version 1
wait gpio 23
wait gpio 40

but I don't see if this is part of an automated test suite that can be run, or whether there's an expected output. However, it looks like someone did at least check that pioasm accepts these high pin numbers in wait gpio.

I see there's a file `.../pioasm/test/amethyst.pio` that has ``` .program wibble2 .pio_version 1 wait gpio 23 wait gpio 40 ``` but I don't see if this is part of an automated test suite that can be run, or whether there's an expected output. However, it looks like someone did at least check that pioasm accepts these high pin numbers in `wait gpio`.

jepler commented

2024-12-29 09:23:18 -06:00

(Migrated from github.com)

On further reflection, there has to be a component of this at load time: A program which uses wait gpio 18 could be loaded in a pio with gpio_offset=0 (so the required encoding is 0x2012) or in one with gpio_offset=16 (so the required encoding is 0x2002).

On further reflection, there has to be a component of this at load time: A program which uses `wait gpio 18` could be loaded in a pio with gpio_offset=0 (so the required encoding is 0x2012) or in one with gpio_offset=16 (so the required encoding is 0x2002).

peterharperuk commented

2025-01-06 04:52:39 -06:00

(Migrated from github.com)

pio_claim_free_sm_and_add_program_for_gpio_range should handle this? It can load a program into the appropriate pio with the base set correctly. However I can't immediately see it handling the invalid (?) request for a gpio count >= 32 for rp2350b

kilograham commented

2025-01-06 11:33:11 -06:00

(Migrated from github.com)

Yes, from a brief look, i think the majority of comments above are correct

This is an oversight; The 16-bit program does store (pin & 31) which matches what happens with the pio_sm_config register values. We also need to add a 32-bit value to the pio_program which holds bit 5 of arg2

I'll have to have a bit of a think about where the runtime component belongs such that we can make sure we do it after the PIO base is set.

Yes, from a brief look, i think the majority of comments above are correct This is an oversight; The 16-bit program does store `(pin & 31)` which matches what happens with the `pio_sm_config` register values. We also need to add a 32-bit value to the pio_program which holds bit 5 of `arg2` I'll have to have a bit of a think about where the runtime component belongs such that we can make sure we do it after the PIO base is set.

kilograham commented

2025-02-14 13:14:32 -06:00

(Migrated from github.com)

merged into develop

jepler commented

2025-02-14 16:55:11 -06:00

(Migrated from github.com)

Thanks!

shilga commented

2025-02-20 09:20:45 -06:00

(Migrated from github.com)

@jepler Just a short question in regards to this issue: Have you been able to test this on hardware? I came to the conclusion a few months back that WAIT GPIO is just broken for pins > 32 and it is a hardware issue. I tried to get attention for this in the forums, but I don't think there are that many RP2350B out there at the moment: https://forums.raspberrypi.com/viewtopic.php?p=2258703&hilit=wait+gpio+rp2350b

In my tests I actually did the calculations of pin shift manually, setting the GPIO_BASE register manually. The same code got different results depending on if the hardware came out of a POR or reset via debugger depending on what software I flashed beforehand. Really strange behavior. I settled on skipping using WAIT GPIO and use WAIT PIN instead as that works flawlessly although there are even more pins shifts envolved.

@jepler Just a short question in regards to this issue: Have you been able to test this on hardware? I came to the conclusion a few months back that WAIT GPIO is just broken for pins > 32 and it is a hardware issue. I tried to get attention for this in the forums, but I don't think there are that many RP2350B out there at the moment: https://forums.raspberrypi.com/viewtopic.php?p=2258703&hilit=wait+gpio+rp2350b In my tests I actually did the calculations of pin shift manually, setting the GPIO_BASE register manually. The same code got different results depending on if the hardware came out of a POR or reset via debugger depending on what software I flashed beforehand. Really strange behavior. I settled on skipping using WAIT GPIO and use WAIT PIN instead as that works flawlessly although there are even more pins shifts envolved.

lurch commented

2025-02-20 09:43:18 -06:00

(Migrated from github.com)

@shilga If you're still seeing problems with this even after upgrading to pico-sdk 2.1.1 then I suggest you create a new issue here explaining your problems in detail, including example code. Thanks!

Rows
Columns

pioasm & sdk support of wait gpio on rp2350b #2141

pioasm & sdk support of `wait gpio` on rp2350b #2141