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

Neopixel examples are problematic #752

Open
ianrrees opened this issue Aug 28, 2024 · 8 comments · Fixed by #802
Open

Neopixel examples are problematic #752

ianrrees opened this issue Aug 28, 2024 · 8 comments · Fixed by #802

Comments

@ianrrees
Copy link
Contributor

In #750 I've been working on updating PyGamer to the current HAL, and I'm finding that the WS2812 / neopixel is a tricky problem to solve generically. In the timer-based WS2812 driver, there's a fundamental issue with the time it takes to toggle a GPIO varying between platforms, and also being comparable to the protocol's timing margins. The SPI driver relies on behaviours of the provided SpiBus which aren't described by the trait (nor reliably provided by our current implementation).

So, it's not really clear to me how we should proceed here. A few ideas:

  1. With Spi implementation enhancements #751 point 2 in place, it might be feasible to get the WS2812 SPI driver to work well enough for a demo with a few asterisks like only working on release profile (the status quo). I think we'd need to modify the driver so that it shifts out 3 bytes of WS2812 data (12 SPI bytes, because each bit of WS2812 data is synthesized from 4 bits on SPI) per call to SpiBus::write() - this would minimise the variation in bit timing to any particular WS2812, and hopefully it doesn't matter as much for the data between WS2812s.
  2. When the timer+GPIO driver is updated to use embedded-hal 1.0 DelayNs (in progress), we could possibly introduce a parameter for the time that a GPIO write takes, to allow tuning for the particular board.
  3. At least on the PyGamer, another option is to use the TCC peripheral to generate the WS2812 bit timing. Going this route might provide a good example of driving a peripheral directly, which could help for people who want to use peripherals/features which aren't provided by the HAL.
@jbeaurivage
Copy link
Contributor

#800 Fixes neopixels when using SPI without DMA.

@sajattack
Copy link
Member

Should we close this then or are there other cases we need to test first?

@jbeaurivage
Copy link
Contributor

I think at this point it's just a matter of reinstating the neopixel examples in the pygamer BSP - we had temporarily removed them to avoid shipping broken examples. I'm not sure if other BSPs would also benefit from using SPI rather than bitbanging the protocol.

@jbeaurivage
Copy link
Contributor

Before this issue gets closed, I 'd like to point out that wile we seem to be gravitating towards using SPI to drive our neopixel examples, there could be other ways to drive them using hardware acceleration.

One that comes to mind is using PWM while also using DMA to set the correct timer compare value according to whether we're sending a 0 or a 1. It would certainly use more memory than (ab)using SPI, but it would allow one to avoid unnecessarily tying up a clock pin in the SPI driver.

It would definitely be a little cursed, but it could be a fun exercise.

@ianrrees
Copy link
Contributor Author

ianrrees commented Dec 8, 2024

I think this should stay open as long as we use the timer-based WS2812 driver for examples, as it's going to remain a point of friction.

@ianrrees
Copy link
Contributor Author

ianrrees commented Dec 8, 2024

there could be other ways to drive them using hardware acceleration

I think the SERCOM is well suited, just not through a generic SPI interface like the ehal traits provide. The TC/TCC seems like a viable option too, at least for the pygamer.

@jbeaurivage
Copy link
Contributor

jbeaurivage commented Dec 9, 2024

I think this should stay open as long as we use the timer-based WS2812 driver for examples, as it's going to remain a point of friction.

Do you mean in Tier 1 BSPs only?

I just checked, and the remaining T1 BSPs that have a neopixel example are feather_m4 and metro_m4. The issue though is that neither can drive a neopixel with a SERCOM peripheral (at least in SPI mode), because they both don't have their neopixel pin connected to a suitable pad that's configurable for data transmission.

@ianrrees
Copy link
Contributor Author

ianrrees commented Dec 9, 2024

Do you mean in Tier 1 BSPs only?

I guess this depends on whether we want to use the tracker for tier 2 BSPs, perhaps a wider discussion really - AFAIK there's no other tracker that would be appropriate for the tier 2 BSPs at the moment. My feeling is that the timer based neopixel driver was a good test case for early Rust firmware work, and is still handy for hobby use, but it's not something I'd want to support long term.

neither can drive a neopixel with a SERCOM peripheral (at least in SPI mode), because they both don't have their neopixel pin connected to a suitable pad that's configurable for data transmission.

Here's a table for the current T1 BSPs - if we had a TC implementation we'd be set. I like the idea of supplying an example driver that talks to a PAC directly, and maybe this is a good case to make one. This would look like a new crate eg samd-tc-neopixel that lives somewhere outside the HAL, as an example of how to make a peripheral driver to be used alongside the ones we provide in the HAL.

board pad SERCOM TC TCC
atsame54_xpro N/A N/A N/A N/A
feather_m0 PA06 SERCOM0/PAD[2] - TCC1/WO[0] or TCC3/WO[4]
feather_m4 PB03 SERCOM5/PAD[1] * TC6/WO[1] -
metro_m0 PA30 SERCOM1/PAD[2] - TCC1/WO[0] or TCC3/WO[4]
metro_m4 PB22 SERCOM1/PAD[2] or SERCOM5/PAD[2] * TC7/WO[0] -
pygamer PA15 SERCOM2/PAD[3] or SERCOM4/PAD[3] TC3/WO[1] TCC2/WO[1] or TCC1/WO[3]
samd11_bare N/A N/A N/A N/A

* SERCOM on SAMD51 can output data on pads 0 and 3 only

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

Successfully merging a pull request may close this issue.

3 participants