Makuna / NeoPixelBus

An Arduino NeoPixel support library supporting a large variety of individually addressable LEDs. Please refer to the Wiki for more details. Please use the GitHub Discussions to ask questions as the GitHub Issues feature is used for bug tracking.
GNU Lesser General Public License v3.0
1.18k stars 264 forks source link

RP2040 Can't use both PIO channels at the same time #854

Open JeronimusII opened 1 month ago

JeronimusII commented 1 month ago

Describe the bug When creating NeoPixelBus instances for both PIO 0 and PIO 1 (for example to have up to 8 instances instead of 4) and the first Begin() call is made on an instance using PIO 0, all instances using PIO 0 will work fine but the other ones (using PIO 1) will never be able to send data to the LED and get stuck blocking forever on the second Show() call.

To Reproduce

  1. Create instances of NeoPixelBus for PIO 0 and PIO 1.
  2. Call Begin() on the instances using PIO 0 first.
  3. Change colors using for example ClearTo(RgbwColor(random(256)).
  4. Call Show() methods on all instances.
  5. Repeat once from step 3 and it will get stuck at step 4 when making a call to Show() of any instance using PIO 1.

Expected behavior Show() calls to all instances (using both PIO 0 and 1) should update the LEDs and not get stuck blocking forever.

Development environment (please complete the following information):

Minimal Sketch that reproduced the problem:

#include <Arduino.h>
#include <NeoPixelBus.h>

using LEDStrip = NeoPixelBus<NeoGrbwFeature, Rp2040x4NSk6812Method>;

LEDStrip strip0(100, 0, NeoBusChannel_0);
LEDStrip strip1(100, 1, NeoBusChannel_1);

void setup()
{
    strip0.Begin();
    strip1.Begin();
}

void loop()
{
    strip0.ClearTo(RgbwColor(random(256)));
    strip0.Show();
    strip1.ClearTo(RgbwColor(random(256)));
    strip1.Show();  // Gets stuck second time here.
    delay(500);
}

Root cause In src/internal/methods/Rp2040/NeoRp2040PioMonoProgram.h class NeoRp2040PioMonoProgram it loads the program into the instruction memory of the first PIO (PIO 0) when making the first call to add() and stores the offset. But when you want to use the other PIO (PIO 1) it won't load the program into the memory of PIO 1 since the s_loadedOffset it already set. Instead it will try using the offset of the program stored in PIO 0 instruction memory which will obviously cause the state machine in PIO 1 to not run how it should and the data to never finish sending.

(very inconvenient) workaround It is still possible to get it to work like this if you try running it with PIO 0 initializing first then flashing it (dont turn off power; using a debug probe) with a new program that initializes PIO 1 first. Since the program stays on the memory of PIO 0 everything will work... until you try turning the power off and on again.

Makuna commented 1 month ago

It should have worked. But things do change in the board and library that may affect it. I will try to take a look soon. until then, try these two things to see if it effects it...

NeoPixelBus<NeoGrbwFeature, Rp2040x4NSk6812Method> strip0(100, 0, NeoBusChannel_0);
NeoPixelBus<NeoGrbwFeature, Rp2040x4NWs2812xMethod> strip1(100, 1, NeoBusChannel_1);

and

NeoPixelBus<NeoGrbwFeature, Rp2040x4Pio0Sk6812Method> strip0(100, 0);
NeoPixelBus<NeoGrbwFeature, Rp2040x4Pio1Sk6812Method> strip1(100, 1);

The s_loadedOffset should be unique as the templates create different classes but the way you are defining it the compiler may or may not do so. The two things I list above will force the matter. The first makes unique due to different speeds. The second due to different PioInstance classes.

JeronimusII commented 1 month ago

The second this is where I started trying to figure out the issue. It also didn't work there.

I think maybe it doesn't work because NeoRp2040PioSpeedSk6812 is a subclass of NeoRp2040PioMonoProgram<NeoRp2040PioCadenceMono3Step>. So a single class is generated off that template and then inherited from NeoRp2040PioSpeedSk6812 which then gets used in generating the two methods (not sure).

I haven't tried the first one yet but since Rp2040x4NWs2812xMethod also uses NeoRp2040PioMonoProgram<NeoRp2040PioCadenceMono3Step> it might also have that issue.

I managed to fix it for myself locally by patching the lib to have two s_loadedOffset static variables for each PIO instance.

Makuna commented 1 month ago

Did you do something like this?

template<typename T_CADENCE>
class NeoRp2040PioMonoProgram
{
public:
    static inline uint add(PIO pio_instance)
    {
        size_t index = (pio_instance == pio0) ? 0 : 1;
        if (s_loadedOffset[index] == c_ProgramNotLoaded)
        {
            assert(pio_can_add_program(pio_instance, &T_CADENCE::program));
            s_loadedOffset[index] = pio_add_program(pio_instance, &T_CADENCE::program);
        }
        return s_loadedOffset[index];
    }

    static inline void init(PIO pio_instance, 
            uint sm, 
            uint offset, 
            uint pin, 
            float bitrate, 
            uint shiftBits)
    {
        float div = clock_get_hz(clk_sys) / (bitrate * T_CADENCE::bit_cycles);
        pio_sm_config c = T_CADENCE::get_default_config(offset);

        sm_config_set_sideset_pins(&c, pin);

        sm_config_set_out_shift(&c, false, true, shiftBits);
        sm_config_set_fifo_join(&c, PIO_FIFO_JOIN_TX);
        sm_config_set_clkdiv(&c, div);

        // Set this pin's GPIO function (connect PIO to the pad)
        pio_gpio_init(pio_instance, pin);

        // Set the pin direction to output at the PIO
        pio_sm_set_consecutive_pindirs(pio_instance, sm, pin, 1, true);

        // Load our configuration, and jump to the start of the program
        pio_sm_init(pio_instance, sm, offset, &c);

        // Set the state machine running
        pio_sm_set_enabled(pio_instance, sm, true);
    }

private:
    static uint s_loadedOffset[2]; // singlet instance of loaded program, one for each PIO hardware unit
};
JeronimusII commented 1 month ago

Yes, that's almost exactly how I did it.