pioasm: assembly of wait 0 gpio 34 seems dubious

[jepler]

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]

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! 😂 )

[jepler]

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 ^ 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]

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.