STM32H5/N6: drive WS2811 LED strip correctly on GPDMA - #15438
Conversation
|
Do you want to test this code? You can flash it directly from the Betaflight App:
WARNING: It may be unstable. Use only for testing! |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughUpdates STM32H5/N6 WS2811 DMA initialization, timer UPDATE request handling, transfer sizing, and completion cleanup while preserving existing behavior on other STM32 platforms. ChangesSTM32 WS2811 GPDMA handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
The WS2811 LED-strip DMA needed three GPDMA-specific corrections on the shared H5/N6 path; without them the strip shows nothing even though plain PWM on the same pin works. 1. Request source: drive the channel from the timer UPDATE request (UDE via timerHardware->dmaTimUPChannel) instead of the per-channel compare request (CCxDE). Empirically a CCxDE-driven GPDMA channel re-fires continuously and races through the whole bit buffer in microseconds; RM0481 documents no semantic difference between the CC and UP requests, but UP gives exactly one CCRx write per timer period and matches the proven DShot GPDMA path (pwm_output_dshot_hal.c). 2. Transfer length: HAL_DMA_Start_IT() programs CBR1.BNDT, which on GPDMA is the block size in BYTES (RM0481 section 16.8.12), not transfer items. Each item is a 32-bit word (one CCRx write per bit), so pass WS2811_DMA_BUFFER_SIZE * 4; the word count alone clocks out only a quarter of the frame and cuts the reset latch. Classic DMA (F4/F7/H7/G4) counts items and is unchanged. 3. Completion: TIM_DMACmd() was the only place TimHandle.State returned to HAL_TIM_STATE_READY, and the UDE path no longer calls it — reset the handle state in the DMA IRQ handler, otherwise DMA_SetCurrDataCounter() returns HAL_BUSY from the second frame on and the strip freezes on the first frame. Also split the transfer across GPDMA master ports (source buffer in SRAM on PORT1, timer CCRx on PORT0). On H5 both ports reach everything and the split just picks each port's zero-latency fast path (RM0481 fig. 1); on N6 PORT1 (AXI) is required to reach AXISRAM.
dd6e4c5 to
b984661
Compare
Split out of #15419 (which now carries only the hardware-verified ADC fix) so the LED strip change can be retested independently.
The WS2811 LED-strip DMA needed three GPDMA-specific corrections on the shared H5/N6 path; without them the strip shows nothing even though plain PWM on the same pin works.
Request source — drive the channel from the timer UPDATE request (UDE via
timerHardware->dmaTimUPChannel) instead of the per-channel compare request (CCxDE). Empirically a CCxDE-driven GPDMA channel re-fires continuously and races through the whole bit buffer in microseconds; RM0481 documents no semantic difference between the CC and UP requests, but UP gives exactly one CCRx write per timer period and matches the proven DShot GPDMA path (pwm_output_dshot_hal.c).Transfer length —
HAL_DMA_Start_IT()programs CBR1.BNDT, which on GPDMA is the block size in bytes (RM0481 §16.8.12), not transfer items. Each item is a 32-bit word (one CCRx write per bit), so passWS2811_DMA_BUFFER_SIZE * 4; the word count alone clocks out only a quarter of the frame and cuts the reset latch. Worst case (USE_LED_STRIP_64) is 8360 bytes, well within the 16-bit BNDT field. Classic DMA (F4/F7/H7/G4) counts items and is unchanged.Completion —
TIM_DMACmd()was the only placeTimHandle.Statereturned toHAL_TIM_STATE_READY, and the UDE path no longer calls it, so the handle state is now reset in the DMA IRQ handler. Without this,DMA_SetCurrDataCounter()returnsHAL_BUSYfrom the second frame on and the strip freezes on the first frame — a static colour looks fine, but nothing ever updates.The transfer is also split across GPDMA master ports (source buffer in SRAM on PORT1, timer CCRx on PORT0). On H5 both ports reach everything and the split just picks each port's zero-latency fast bus multiplexer path (RM0481 §2.1.5/§2.1.7, fig. 1); on N6 PORT1 (AXI) is genuinely required to reach AXISRAM.
The functional behaviour was checked against RM0481: with OCx preload enabled the DMA-written CCR values latch at the next update event, so the waveform shifts one bit period, which the 42-word trailing zero padding absorbs; the line idles low after the frame, keeping the >50 µs reset latch intact.
make STM32H563builds clean with zero warnings.Testing: needs an on-hardware retest with an animated pattern (e.g. larson scanner) — a static colour cannot distinguish a working strip from one frozen on frame 1, which is exactly what the pre-fix code did.
Summary by CodeRabbit