Skip to content
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

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

Open
JeronimusII opened this issue Oct 18, 2024 · 4 comments
Open

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

JeronimusII opened this issue Oct 18, 2024 · 4 comments
Assignees
Labels
bug investigating Currently under investigation for more understanding of the problem.

Comments

@JeronimusII
Copy link

JeronimusII commented Oct 18, 2024

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):

  • OS: Windows 10
  • Build Environment: PlatformIO (VSCode; Arduino)
  • Board target: Raspberry Pi Pico W (RP2040)
  • Library version: v2.8.3

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
Copy link
Owner

Makuna commented Oct 18, 2024

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
Copy link
Author

JeronimusII commented Oct 19, 2024

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
Copy link
Owner

Makuna commented Oct 19, 2024

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
};

@Makuna Makuna added bug investigating Currently under investigation for more understanding of the problem. labels Oct 19, 2024
@Makuna Makuna self-assigned this Oct 19, 2024
@JeronimusII
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug investigating Currently under investigation for more understanding of the problem.
Projects
None yet
Development

No branches or pull requests

2 participants