JeremyGrosser / rp2040_hal

Ada drivers for the Raspberry Pi RP2040 SoC
https://pico-doc.synack.me/
BSD 3-Clause "New" or "Revised" License
35 stars 11 forks source link

RP.PIO.WS2812: add support of ws2812 smart LEDs #21

Closed Fabien-Chouteau closed 2 years ago

Fabien-Chouteau commented 2 years ago

Hi @JeremyGrosser,

I use WS2812s in multiple projects and I wanted to factorize this code.

I think it makes sens to have "high-level" drivers based on PIO programs in the rp2040_hal, what do you think?

RFC since API doc is missing.

JeremyGrosser commented 2 years ago

I'd like to keep rp2040_hal limited to the peripherals internal to the SoC, plus the boot flash. We already have Pico.Audio_I2S providing this kind of high level API in pico_bsp, so I think we should continue putting PIO-based drivers there, or create a separate crate for them.

JeremyGrosser commented 2 years ago

My thinking is that Raspberry Pi could put PIO in their application processors and I'd like those drivers that use PIO to be agnostic of what device they're running on. That said, the use of DMA here isn't going to be very portable, so this driver is going to want to track changes to rp2040_hal anyway. I guess I'm okay with including it in this crate. Maybe we should create a separate subdir under src/ for PIO programs?

I'm bringing up the Pimoroni VGA Demo board, so I've got a few PIO things in the works as well.

JeremyGrosser commented 2 years ago

I've rebased and merged this. We'll keep the PIO programs in src/drivers for now and see how things go.

Fabien-Chouteau commented 2 years ago

My thinking is that Raspberry Pi could put PIO in their application processors and I'd like those drivers that use PIO to be agnostic of what device they're running on.

I understand that, and I hope what we will see more hardware with PIOs (RP2749 :crossed_fingers: :smile: ). But in my opinion it would be a waste of time to do that know. We will be able to improve the code re-use when/if it's necessary.