From 5426ac38548778acac00e3a3690d1348c239280f Mon Sep 17 00:00:00 2001 From: Ray Morris Date: Tue, 11 Aug 2026 22:10:27 -0500 Subject: [PATCH 1/7] Shrink ledStripDMABuffer from 32-bit to 16-bit elements TIM3/TIM4's CCR is a 16-bit register; the buffer only needs to hold compare values 0-3, so 32-bit elements wasted RAM without matching any hardware requirement. Halves ledStripDMABuffer from 12,460 B to 6,230 B on affected F4 targets. --- src/main/drivers/light_ws2811strip.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/main/drivers/light_ws2811strip.c b/src/main/drivers/light_ws2811strip.c index cc052fcd872..c206b90fc3d 100644 --- a/src/main/drivers/light_ws2811strip.c +++ b/src/main/drivers/light_ws2811strip.c @@ -49,7 +49,10 @@ #define WS2811_BIT_COMPARE_1 ((WS2811_PERIOD * 2) / 3) #define WS2811_BIT_COMPARE_0 (WS2811_PERIOD / 3) -static DMA_RAM timerDMASafeType_t ledStripDMABuffer[WS2811_DMA_BUFFER_SIZE]; +// TIM3/TIM4 are 16-bit timers — CCR is a 16-bit register, so DMA must write +// it at that width or the high byte is left stale, corrupting the compare +// value. +static DMA_RAM uint16_t ledStripDMABuffer[WS2811_DMA_BUFFER_SIZE]; static IO_t ws2811IO = IO_NONE; static TCH_t * ws2811TCH = NULL; From 7a6e57ffc451eb6c6eec026d9f32ca3630f68b08 Mon Sep 17 00:00:00 2001 From: Ray Morris Date: Tue, 18 Aug 2026 00:10:19 -0500 Subject: [PATCH 2/7] Add opt-in circular-DMA refill callback to F4 timer driver Lets a circular-DMA consumer refill each half of its buffer as DMA finishes sending it, instead of pre-loading the whole transfer up front. Opt-in and null-guarded: DMA_IT_HT is only enabled, and the callback only invoked, when a consumer registers one via impl_timerPWMSetDMARefillCallback, so existing circular-DMA users (motor DShot idle-packet repeat) see no behavior change. --- src/main/drivers/timer.h | 10 ++++++ src/main/drivers/timer_impl.h | 1 + src/main/drivers/timer_impl_stdperiph.c | 44 ++++++++++++++++++++----- 3 files changed, 47 insertions(+), 8 deletions(-) diff --git a/src/main/drivers/timer.h b/src/main/drivers/timer.h index 8a81b6d5f27..697bddf00a7 100644 --- a/src/main/drivers/timer.h +++ b/src/main/drivers/timer.h @@ -156,6 +156,15 @@ typedef struct timerCallbacks_s { timerCallbackFn * callbackOvr; } timerCallbacks_t; +// Circular-DMA refill callback: invoked from the DMA IRQ while +// dmaState == TCH_DMA_CIRCULAR, once per half-cycle, so the consumer can +// refill the half that was just transmitted. transferComplete is true for +// the TC (second-half-just-sent) event, false for the HT +// (first-half-just-sent) event. Optional (NULL) for circular DMA consumers +// that don't need refilling (e.g. motor DShot idle-packet repeat during +// EEPROM writes) — those get no HT/TC IRQs at all. +typedef void timerDmaRefillFn(struct TCH_s * tch, bool transferComplete); + // Run-time TCH (Timer CHannel) context typedef struct TCH_s { struct timHardwareContext_s * timCtx; // Run-time initialized to parent timer @@ -164,6 +173,7 @@ typedef struct TCH_s { DMA_t dma; // Timer channel DMA handle volatile tchDmaState_e dmaState; void * dmaBuffer; + timerDmaRefillFn * dmaRefillCallback; // optional, see typedef above } TCH_t; // Run-time timer context (dynamically allocated), includes 4x TCH diff --git a/src/main/drivers/timer_impl.h b/src/main/drivers/timer_impl.h index 6a302f57cb4..081569bb70a 100644 --- a/src/main/drivers/timer_impl.h +++ b/src/main/drivers/timer_impl.h @@ -85,6 +85,7 @@ void impl_timerPWMPrepareDMA(TCH_t * tch, uint32_t dmaBufferElementCount); void impl_timerPWMStartDMA(TCH_t * tch); void impl_timerPWMStopDMA(TCH_t * tch); void impl_timerPWMSetDMACircular(TCH_t * tch, bool circular, uint32_t dmaBufferSize); +void impl_timerPWMSetDMARefillCallback(TCH_t * tch, timerDmaRefillFn * callback); #ifdef USE_DSHOT_DMAR bool impl_timerPWMConfigDMABurst(burstDmaTimer_t *burstDmaTimer, TCH_t * tch, void * dmaBuffer, uint8_t dmaBufferElementSize, uint32_t dmaBufferElementCount); diff --git a/src/main/drivers/timer_impl_stdperiph.c b/src/main/drivers/timer_impl_stdperiph.c index 13b38cdd2b3..1b1b4b34ab8 100644 --- a/src/main/drivers/timer_impl_stdperiph.c +++ b/src/main/drivers/timer_impl_stdperiph.c @@ -268,15 +268,31 @@ void impl_timerChCaptureCompareEnable(TCH_t * tch, bool enable) static void impl_timerDMA_IRQHandler(DMA_t descriptor) { - if (DMA_GET_FLAG_STATUS(descriptor, DMA_IT_TCIF)) { - TCH_t * tch = (TCH_t *)descriptor->userParam; + TCH_t * tch = (TCH_t *)descriptor->userParam; + + if (tch->dmaState == TCH_DMA_CIRCULAR) { + // Let DMA keep running - don't disable the stream. HT/TC are only + // enabled here when a refill callback is registered (see + // impl_timerPWMSetDMACircular); non-refilling circular consumers + // never reach this branch. + if (DMA_GET_FLAG_STATUS(descriptor, DMA_IT_HTIF)) { + DMA_CLEAR_FLAG(descriptor, DMA_IT_HTIF); + if (tch->dmaRefillCallback) { + tch->dmaRefillCallback(tch, false); + } + } - // In circular mode, let DMA keep running - don't disable the stream - if (tch->dmaState == TCH_DMA_CIRCULAR) { + if (DMA_GET_FLAG_STATUS(descriptor, DMA_IT_TCIF)) { DMA_CLEAR_FLAG(descriptor, DMA_IT_TCIF); - return; + if (tch->dmaRefillCallback) { + tch->dmaRefillCallback(tch, true); + } } + return; + } + + if (DMA_GET_FLAG_STATUS(descriptor, DMA_IT_TCIF)) { tch->dmaState = TCH_DMA_IDLE; TIM_DMACmd(tch->timHw->tim, lookupDMASourceTable[tch->timHw->channelIndex], DISABLE); @@ -286,6 +302,11 @@ static void impl_timerDMA_IRQHandler(DMA_t descriptor) } } +void impl_timerPWMSetDMARefillCallback(TCH_t * tch, timerDmaRefillFn * callback) +{ + tch->dmaRefillCallback = callback; +} + bool impl_timerPWMConfigChannelDMA(TCH_t * tch, void * dmaBuffer, uint8_t dmaBufferElementSize, uint32_t dmaBufferElementCount) { DMA_InitTypeDef DMA_InitStructure; @@ -594,12 +615,19 @@ void impl_timerPWMSetDMACircular(TCH_t * tch, bool circular, uint32_t dmaBufferS if (circular) { tch->dma->ref->CR |= DMA_SxCR_CIRC; DMA_SetCurrDataCounter(tch->dma->ref, dmaBufferSize); - // Disable TC interrupt — in circular mode, TC fires every cycle - // and the IRQ handler would otherwise disable the stream - DMA_ITConfig(tch->dma->ref, DMA_IT_TC, DISABLE); + if (tch->dmaRefillCallback) { + // Refill consumer needs an IRQ every half-cycle to keep the + // buffer fed + DMA_ITConfig(tch->dma->ref, DMA_IT_HT | DMA_IT_TC, ENABLE); + } else { + // Disable TC interrupt — in circular mode, TC fires every cycle + // and the IRQ handler would otherwise disable the stream + DMA_ITConfig(tch->dma->ref, DMA_IT_TC, DISABLE); + } tch->dmaState = TCH_DMA_CIRCULAR; } else { tch->dma->ref->CR &= ~DMA_SxCR_CIRC; + DMA_ITConfig(tch->dma->ref, DMA_IT_HT, DISABLE); DMA_ITConfig(tch->dma->ref, DMA_IT_TC, ENABLE); tch->dmaState = TCH_DMA_IDLE; } From d3281638edab62d121f2b2105315219730cdab8f Mon Sep 17 00:00:00 2001 From: Ray Morris Date: Tue, 18 Aug 2026 00:10:26 -0500 Subject: [PATCH 3/7] Cache recent HSV->RGB conversions in hsvToRgb24 hsvToRgb24 reruns its full divide/multiply conversion for every LED on every strip update (up to 100Hz), even though most updates set only a handful of distinct colors across the whole strip. A 4-entry direct-mapped cache, keyed on exact HSV input equality, skips the recompute on a repeat. --- src/main/common/colorconversion.c | 36 +++++++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) diff --git a/src/main/common/colorconversion.c b/src/main/common/colorconversion.c index a37bc821ab6..0ebfedccc68 100644 --- a/src/main/common/colorconversion.c +++ b/src/main/common/colorconversion.c @@ -16,6 +16,7 @@ */ #include "stdint.h" +#include "stdbool.h" #include "color.h" #include "colorconversion.h" @@ -24,9 +25,33 @@ * Source below found here: http://www.kasperkamperman.com/blog/arduino/arduino-programming-hsb-to-rgb/ */ +// Small direct-mapped cache of recent HSV->RGB conversions. LED strip +// patterns tend to reuse a handful of distinct colors across many LEDs and +// updates, so a few cached entries catch most repeats without the cost of a +// full RGB-native color store. Round-robin replacement is enough here: the +// working set per frame is normally <= 4 distinct colors, so eviction order +// doesn't matter much. +static struct { + hsvColor_t in; + rgbColor24bpp_t out; + bool valid; +} hsvToRgbCache[4]; +static uint8_t hsvToRgbCacheNextSlot = 0; + +static bool hsvColorEqual(const hsvColor_t *a, const hsvColor_t *b) +{ + return a->h == b->h && a->s == b->s && a->v == b->v; +} + rgbColor24bpp_t* hsvToRgb24(const hsvColor_t* c) { - static rgbColor24bpp_t r; + for (int i = 0; i < 4; i++) { + if (hsvToRgbCache[i].valid && hsvColorEqual(&hsvToRgbCache[i].in, c)) { + return &hsvToRgbCache[i].out; + } + } + + rgbColor24bpp_t r; uint16_t val = c->v; uint16_t sat = 255 - c->s; @@ -79,6 +104,13 @@ rgbColor24bpp_t* hsvToRgb24(const hsvColor_t* c) } } - return &r; + + hsvToRgbCache[hsvToRgbCacheNextSlot].in = *c; + hsvToRgbCache[hsvToRgbCacheNextSlot].out = r; + hsvToRgbCache[hsvToRgbCacheNextSlot].valid = true; + rgbColor24bpp_t *cached = &hsvToRgbCache[hsvToRgbCacheNextSlot].out; + hsvToRgbCacheNextSlot = (hsvToRgbCacheNextSlot + 1) % 4; + + return cached; } From df8903bc50cdb693c40237f70e21d27caf4b7989 Mon Sep 17 00:00:00 2001 From: Ray Morris Date: Tue, 18 Aug 2026 00:10:43 -0500 Subject: [PATCH 4/7] Replace whole-strip WS2811 DMA buffer with a bounded, chunked one ledStripDMABuffer held every WS2811_LED_STRIP_LENGTH (128) LED's worth of protocol bits for a single one-shot DMA burst, regardless of how many LEDs were actually configured (6,230 bytes, uint16_t elements). It's now a 2-half, 4-LED-per-half circular buffer (384 bytes) refilled from the DMA half/full-transfer interrupt as each group finishes transmitting, via the refill callback hook added in the timer driver. ws2811UpdateStrip() now takes the actual configured LED count and bounds the transfer to it instead of always processing the full 128 slots. WS2812's reset/latch condition is a minimum, not a maximum, low duration, so the old fixed DMA preamble and idle-tail buffer elements are gone too: starting a transfer now does a cheap timestamp check against how long the line has already been idling low (falling back to a blocking wait only if it hasn't been, e.g. right after PINIO idle-high), and stopping does a direct write to the timer's preload-buffered compare register instead of one more DMA element. --- src/main/drivers/light_ws2811strip.c | 162 +++++++++++++++++++++------ src/main/drivers/light_ws2811strip.h | 7 +- src/main/io/ledstrip.c | 4 +- 3 files changed, 133 insertions(+), 40 deletions(-) diff --git a/src/main/drivers/light_ws2811strip.c b/src/main/drivers/light_ws2811strip.c index c206b90fc3d..c64dcb8e5ea 100644 --- a/src/main/drivers/light_ws2811strip.c +++ b/src/main/drivers/light_ws2811strip.c @@ -37,10 +37,14 @@ #include "common/color.h" #include "common/colorconversion.h" +#include "common/maths.h" +#include "common/time.h" #include "drivers/dma.h" #include "drivers/io.h" +#include "drivers/time.h" #include "drivers/timer.h" +#include "drivers/timer_impl.h" #include "drivers/light_ws2811strip.h" #include "fc/runtime_config.h" @@ -49,10 +53,20 @@ #define WS2811_BIT_COMPARE_1 ((WS2811_PERIOD * 2) / 3) #define WS2811_BIT_COMPARE_0 (WS2811_PERIOD / 3) -// TIM3/TIM4 are 16-bit timers — CCR is a 16-bit register, so DMA must write -// it at that width or the high byte is left stale, corrupting the compare -// value. -static DMA_RAM uint16_t ledStripDMABuffer[WS2811_DMA_BUFFER_SIZE]; +// Circular DMA buffer: 2 halves, 1 LED group each. ws2811DMARefillCallback +// refills whichever half DMA just finished sending, so this only needs to +// hold a couple of LEDs regardless of strip length. +#define WS2811_LEDS_PER_GROUP 4 +#define WS2811_GROUP_BITS (WS2811_LEDS_PER_GROUP * WS2811_BITS_PER_LED) +#define WS2811_CHUNK_BUFFER_SIZE (2 * WS2811_GROUP_BITS) + +// WS2812 reset/latch is a *minimum* low duration with no upper bound, so +// this is a threshold to check against, not a preamble to budget for. +#define WS2811_RESET_US 60 + +// CCR is a 16-bit register on TIM3/TIM4; DMA must write it at that width or +// the high byte is left stale. +static DMA_RAM uint16_t ledStripDMABuffer[WS2811_CHUNK_BUFFER_SIZE]; static IO_t ws2811IO = IO_NONE; static TCH_t * ws2811TCH = NULL; @@ -96,6 +110,8 @@ void setStripColors(const hsvColor_t *colors) } } +static void ws2811DMARefillCallback(TCH_t * tch, bool transferComplete); + bool ledConfigureDMA(void) { /* Compute the prescaler value */ uint8_t period = WS2811_TIMER_HZ / WS2811_CARRIER_HZ; @@ -103,9 +119,23 @@ bool ledConfigureDMA(void) { timerConfigBase(ws2811TCH, period, WS2811_TIMER_HZ); timerPWMConfigChannel(ws2811TCH, 0); - return timerPWMConfigChannelDMA(ws2811TCH, ledStripDMABuffer, sizeof(ledStripDMABuffer[0]), WS2811_DMA_BUFFER_SIZE); + return timerPWMConfigChannelDMA(ws2811TCH, ledStripDMABuffer, sizeof(ledStripDMABuffer[0]), WS2811_CHUNK_BUFFER_SIZE); } +// Number of LEDs in the most recent transfer; bounds the DMA transfer (and +// ws2811SetIdleHigh's target) to what's actually configured. +static uint16_t activeLedCount = WS2811_LED_STRIP_LENGTH; + +// groupInHalf[i]: group index currently in half i, so the refill callback +// can tell when the group it just finished sending was the last one. +static uint16_t totalGroups; +static uint16_t nextGroupToAssign; +static uint16_t groupInHalf[2]; + +// Shared between normal transfer completion and ws2811SetIdleHigh (PINIO). +static bool lineIdleLow = true; +static timeUs_t lastLowAtUs = 0; + void ws2811LedStripInit(void) { const timerHardware_t * timHw = timerGetByTag(IO_TAG(WS2811_PIN), TIM_USE_ANY); @@ -123,6 +153,8 @@ void ws2811LedStripInit(void) return; } + impl_timerPWMSetDMARefillCallback(ws2811TCH, ws2811DMARefillCallback); + ws2811IO = IOGetByTag(timHw->tag); //IOGetByTag(IO_TAG(WS2811_PIN)); IOInit(ws2811IO, OWNER_LED_STRIP, RESOURCE_OUTPUT, 0); IOConfigGPIOAF(ws2811IO, IOCFG_AF_PP_FAST, timHw->alternateFunction); @@ -135,9 +167,11 @@ void ws2811LedStripInit(void) // Zero out DMA buffer — LED pin idles LOW between WS2812 bursts memset(&ledStripDMABuffer, 0, sizeof(ledStripDMABuffer)); + lineIdleLow = true; + lastLowAtUs = micros(); ws2811Initialised = true; - ws2811UpdateStrip(); + ws2811UpdateStrip(WS2811_LED_STRIP_LENGTH); } bool isWS2811LedStripReady(void) @@ -145,55 +179,119 @@ bool isWS2811LedStripReady(void) return !timerPWMDMAInProgress(ws2811TCH); } -STATIC_UNIT_TESTED uint16_t dmaBufferOffset; -static int16_t ledIndex; - -STATIC_UNIT_TESTED void fastUpdateLEDDMABuffer(rgbColor24bpp_t *color) +static void writeLedBits(uint16_t *dest, const rgbColor24bpp_t *color) { uint32_t grb = (color->rgb.g << 16) | (color->rgb.r << 8) | (color->rgb.b); for (int8_t index = 23; index >= 0; index--) { - ledStripDMABuffer[WS2811_DELAY_BUFFER_LENGTH + dmaBufferOffset++] = (grb & (1 << index)) ? WS2811_BIT_COMPARE_1 : WS2811_BIT_COMPARE_0; + *dest++ = (grb & (1 << index)) ? WS2811_BIT_COMPARE_1 : WS2811_BIT_COMPARE_0; } } -/* - * This method is non-blocking unless an existing LED update is in progress. - * it does not wait until all the LEDs have been updated, that happens in the background. - */ -void ws2811UpdateStrip(void) +// Slots at or beyond activeLedCount get a direct "off" write instead of a +// color lookup, so a group is self-contained regardless of where the +// configured strip actually ends. +static void ws2811FillGroup(uint8_t halfIndex, uint16_t groupIndex) { - static rgbColor24bpp_t *rgb24; + uint16_t *half = &ledStripDMABuffer[halfIndex * WS2811_GROUP_BITS]; + uint16_t baseLed = groupIndex * WS2811_LEDS_PER_GROUP; + + for (uint8_t slot = 0; slot < WS2811_LEDS_PER_GROUP; slot++) { + uint16_t *dest = &half[slot * WS2811_BITS_PER_LED]; + uint16_t ledIdx = baseLed + slot; + + if (ledIdx < activeLedCount) { + writeLedBits(dest, hsvToRgb24(&ledColorBuffer[ledIdx])); + } else { + for (uint8_t bit = 0; bit < WS2811_BITS_PER_LED; bit++) { + dest[bit] = WS2811_BIT_COMPARE_0; + } + } + } +} - // don't wait - risk of infinite block, just get an update next time round - if (timerPWMDMAInProgress(ws2811TCH)) { +static void ws2811RefillHalf(uint8_t halfIndex) +{ + ws2811FillGroup(halfIndex, nextGroupToAssign); + groupInHalf[halfIndex] = nextGroupToAssign; + nextGroupToAssign++; +} + +// CCR is preload/shadow-buffered, so the direct write below takes effect +// cleanly at the next period boundary without needing further DMA. +static void ws2811StopTransfer(void) +{ + timerPWMStopDMA(ws2811TCH); + *timerCCR(ws2811TCH) = 0; + lineIdleLow = true; + lastLowAtUs = micros(); +} + +// transferComplete: true = half 1 just finished (DMA wrapped to half 0), +// false = half 0 just finished (DMA moved on to half 1). +static void ws2811DMARefillCallback(TCH_t * tch, bool transferComplete) +{ + (void)tch; + + uint8_t finishedHalf = transferComplete ? 1 : 0; + + if (groupInHalf[finishedHalf] == totalGroups - 1) { + ws2811StopTransfer(); return; } - dmaBufferOffset = 0; // reset buffer memory index - ledIndex = 0; // reset led index + ws2811RefillHalf(finishedHalf); +} - // fill transmit buffer with correct compare values to achieve - // correct pulse widths according to color values - while (ledIndex < WS2811_LED_STRIP_LENGTH) - { - rgb24 = hsvToRgb24(&ledColorBuffer[ledIndex]); - fastUpdateLEDDMABuffer(rgb24); - ledIndex++; +static void ws2811EnsureResetGap(void) +{ + // A gap before real data is always safe regardless of length — only a + // mid-frame gap (prevented by true circular DMA) risks looking like a + // premature reset — so usually this is just a compare, not a wait. + if (!lineIdleLow || cmpTimeUs(micros(), lastLowAtUs) < WS2811_RESET_US) { + *timerCCR(ws2811TCH) = 0; + lineIdleLow = true; + delayMicroseconds(WS2811_RESET_US); + lastLowAtUs = micros(); } +} - // Initiate hardware transfer +// Non-blocking except when the line was left idle-high by PINIO or updates +// are requested faster than the reset window allows. LEDs are transmitted +// in the background via the DMA refill callback. +void ws2811UpdateStrip(uint16_t usedLedCount) +{ if (!ws2811Initialised || !ws2811TCH) { return; } - timerPWMPrepareDMA(ws2811TCH, WS2811_DMA_BUFFER_SIZE); - timerPWMStartDMA(ws2811TCH); + // don't wait - risk of infinite block, just get an update next time round + if (timerPWMDMAInProgress(ws2811TCH)) { + return; + } + + activeLedCount = MIN(usedLedCount, (uint16_t)WS2811_LED_STRIP_LENGTH); + if (activeLedCount == 0) { + return; + } + + ws2811EnsureResetGap(); + + totalGroups = (activeLedCount + WS2811_LEDS_PER_GROUP - 1) / WS2811_LEDS_PER_GROUP; + nextGroupToAssign = 0; + ws2811RefillHalf(0); + ws2811RefillHalf(1); + + impl_timerPWMSetDMACircular(ws2811TCH, true, WS2811_CHUNK_BUFFER_SIZE); } void ws2811SetIdleHigh(bool high) { - ledStripDMABuffer[WS2811_DMA_BUFFER_SIZE - 1] = high ? 255 : 0; + lineIdleLow = !high; + *timerCCR(ws2811TCH) = high ? 255 : 0; + if (!high) { + lastLowAtUs = micros(); + } } #endif diff --git a/src/main/drivers/light_ws2811strip.h b/src/main/drivers/light_ws2811strip.h index d0edcc276ea..062aafa5441 100644 --- a/src/main/drivers/light_ws2811strip.h +++ b/src/main/drivers/light_ws2811strip.h @@ -23,11 +23,6 @@ #define WS2811_LED_STRIP_LENGTH 128 #define WS2811_BITS_PER_LED 24 -#define WS2811_DELAY_BUFFER_LENGTH 42 // for 50us delay - -#define WS2811_DATA_BUFFER_SIZE (WS2811_BITS_PER_LED * WS2811_LED_STRIP_LENGTH) - -#define WS2811_DMA_BUFFER_SIZE (WS2811_DELAY_BUFFER_LENGTH + WS2811_DATA_BUFFER_SIZE + 1) // leading bytes (reset low 302us) + data bytes LEDS*3 + 1 byte(keep line high optionally) #define WS2811_TIMER_HZ 2400000 #define WS2811_CARRIER_HZ 800000 @@ -35,7 +30,7 @@ void ws2811LedStripInit(void); void ws2811SetIdleHigh(bool high); -void ws2811UpdateStrip(void); +void ws2811UpdateStrip(uint16_t usedLedCount); void setLedHsv(uint16_t index, const hsvColor_t *color); void getLedHsv(uint16_t index, hsvColor_t *color); diff --git a/src/main/io/ledstrip.c b/src/main/io/ledstrip.c index 8e8e5771450..7169cf6cfb9 100644 --- a/src/main/io/ledstrip.c +++ b/src/main/io/ledstrip.c @@ -989,7 +989,7 @@ void ledStripUpdate(timeUs_t currentTimeUs) bool updateNow = timActive & (1 << timId); (*layerTable[timId])(updateNow, timer); } - ws2811UpdateStrip(); + ws2811UpdateStrip(ledCounts.count); } bool parseColor(int index, const char *colorConfig) @@ -1077,6 +1077,6 @@ static void ledStripDisable(void) { setStripColor(&HSV(BLACK)); - ws2811UpdateStrip(); + ws2811UpdateStrip(ledCounts.count); } #endif From 6ab6edb601d2ae69c928dfb68d6bb61b4d060578 Mon Sep 17 00:00:00 2001 From: Ray Morris Date: Tue, 18 Aug 2026 00:36:22 -0500 Subject: [PATCH 5/7] Add DMA refill callback support to HAL and AT32 timer backends MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The refill-callback hook was only wired up in the F4 StdPeriph timer backend, but light_ws2811strip.c calls it unconditionally on every platform with USE_LED_STRIP — leaving H7/F7 (HAL) and AT32 targets with an undefined reference to impl_timerPWMSetDMARefillCallback at link time. Mirrors the same opt-in, null-guarded HT/TC dispatch added to the StdPeriph backend. Also has all three backends' impl_timerPWMStopDMA poll for the DMA stream's enable bit to actually clear before returning, matching the poll already used by impl_timerPWMSetDMACircular for the same hardware constraint (disabling a stream isn't instantaneous) - stop is now called from a circular-DMA refill callback, not just after a one-shot transfer's TC event, so it can run while a stream is still mid-flight. --- src/main/drivers/timer_impl_hal.c | 52 +++++++++++++++++--- src/main/drivers/timer_impl_stdperiph.c | 7 +++ src/main/drivers/timer_impl_stdperiph_at32.c | 52 +++++++++++++++++--- 3 files changed, 95 insertions(+), 16 deletions(-) diff --git a/src/main/drivers/timer_impl_hal.c b/src/main/drivers/timer_impl_hal.c index a24875eec08..93283368d04 100644 --- a/src/main/drivers/timer_impl_hal.c +++ b/src/main/drivers/timer_impl_hal.c @@ -316,15 +316,31 @@ static inline void LL_TIM_DisableDMAReq_CCx(TIM_TypeDef * TIMx, uint16_t dmaSour static void impl_timerDMA_IRQHandler(DMA_t descriptor) { - if (DMA_GET_FLAG_STATUS(descriptor, DMA_IT_TCIF)) { - TCH_t * tch = (TCH_t *)descriptor->userParam; + TCH_t * tch = (TCH_t *)descriptor->userParam; + + if (tch->dmaState == TCH_DMA_CIRCULAR) { + // Let DMA keep running - don't disable the stream. HT/TC are only + // enabled here when a refill callback is registered (see + // impl_timerPWMSetDMACircular); non-refilling circular consumers + // never reach this branch. + if (DMA_GET_FLAG_STATUS(descriptor, DMA_IT_HTIF)) { + DMA_CLEAR_FLAG(descriptor, DMA_IT_HTIF); + if (tch->dmaRefillCallback) { + tch->dmaRefillCallback(tch, false); + } + } - // In circular mode, let DMA keep running - don't disable the stream - if (tch->dmaState == TCH_DMA_CIRCULAR) { + if (DMA_GET_FLAG_STATUS(descriptor, DMA_IT_TCIF)) { DMA_CLEAR_FLAG(descriptor, DMA_IT_TCIF); - return; + if (tch->dmaRefillCallback) { + tch->dmaRefillCallback(tch, true); + } } + return; + } + + if (DMA_GET_FLAG_STATUS(descriptor, DMA_IT_TCIF)) { // If it was ACTIVE - switch to IDLE if (tch->dmaState == TCH_DMA_ACTIVE) { tch->dmaState = TCH_DMA_IDLE; @@ -337,6 +353,11 @@ static void impl_timerDMA_IRQHandler(DMA_t descriptor) } } +void impl_timerPWMSetDMARefillCallback(TCH_t * tch, timerDmaRefillFn * callback) +{ + tch->dmaRefillCallback = callback; +} + bool impl_timerPWMConfigChannelDMA(TCH_t * tch, void * dmaBuffer, uint8_t dmaBufferElementSize, uint32_t dmaBufferElementCount) { tch->dma = dmaGetByTag(tch->timHw->dmaTag); @@ -634,6 +655,13 @@ void impl_timerPWMStopDMA(TCH_t * tch) ATOMIC_BLOCK(NVIC_PRIO_MAX) { LL_TIM_DisableDMAReq_CCx(tch->timHw->tim, lookupDMASourceTable[tch->timHw->channelIndex]); LL_DMA_DisableStream(dmaBase, streamLL); + + // STM32H7 RM: poll EN bit until stream is actually disabled + uint32_t timeout = 10000; // ~20us at 480MHz, well above worst-case disable latency + while (LL_DMA_IsEnabledStream(dmaBase, streamLL) && timeout--) { + __NOP(); + } + DMA_CLEAR_FLAG(tch->dma, DMA_IT_TCIF); } tch->dmaState = TCH_DMA_IDLE; @@ -672,12 +700,20 @@ void impl_timerPWMSetDMACircular(TCH_t * tch, bool circular, uint32_t dmaBufferS LL_DMA_SetMode(dmaBase, streamLL, LL_DMA_MODE_CIRCULAR); // Circular mode requires non-zero NDTR (STM32H7 RM constraint) LL_DMA_SetDataLength(dmaBase, streamLL, dmaBufferSize); - // Disable TC interrupt — in circular mode, TC fires every cycle - // and the IRQ handler would otherwise disable the stream - LL_DMA_DisableIT_TC(dmaBase, streamLL); + if (tch->dmaRefillCallback) { + // Refill consumer needs an IRQ every half-cycle to keep the + // buffer fed + LL_DMA_EnableIT_HT(dmaBase, streamLL); + LL_DMA_EnableIT_TC(dmaBase, streamLL); + } else { + // Disable TC interrupt — in circular mode, TC fires every cycle + // and the IRQ handler would otherwise disable the stream + LL_DMA_DisableIT_TC(dmaBase, streamLL); + } tch->dmaState = TCH_DMA_CIRCULAR; } else { LL_DMA_SetMode(dmaBase, streamLL, LL_DMA_MODE_NORMAL); + LL_DMA_DisableIT_HT(dmaBase, streamLL); LL_DMA_EnableIT_TC(dmaBase, streamLL); tch->dmaState = TCH_DMA_IDLE; } diff --git a/src/main/drivers/timer_impl_stdperiph.c b/src/main/drivers/timer_impl_stdperiph.c index 1b1b4b34ab8..81e6c4d9fd1 100644 --- a/src/main/drivers/timer_impl_stdperiph.c +++ b/src/main/drivers/timer_impl_stdperiph.c @@ -584,6 +584,13 @@ void impl_timerPWMStopDMA(TCH_t * tch) { TIM_DMACmd(tch->timHw->tim, lookupDMASourceTable[tch->timHw->channelIndex], DISABLE); DMA_Cmd(tch->dma->ref, DISABLE); + + // STM32F4/F7 RM: poll EN bit until stream is actually disabled + uint32_t timeout = 10000; // ~60us at 168MHz, well above worst-case disable latency + while ((tch->dma->ref->CR & DMA_SxCR_EN) && timeout--) { + __NOP(); + } + tch->dmaState = TCH_DMA_IDLE; TIM_Cmd(tch->timHw->tim, ENABLE); } diff --git a/src/main/drivers/timer_impl_stdperiph_at32.c b/src/main/drivers/timer_impl_stdperiph_at32.c index 54c6d257078..0bc26b0d613 100644 --- a/src/main/drivers/timer_impl_stdperiph_at32.c +++ b/src/main/drivers/timer_impl_stdperiph_at32.c @@ -267,15 +267,31 @@ void impl_timerChCaptureCompareEnable(TCH_t * tch, bool enable) // lookupDMASourceTable static void impl_timerDMA_IRQHandler(DMA_t descriptor) { - if (DMA_GET_FLAG_STATUS(descriptor, DMA_IT_TCIF)) { - TCH_t * tch = (TCH_t *)descriptor->userParam; + TCH_t * tch = (TCH_t *)descriptor->userParam; + + if (tch->dmaState == TCH_DMA_CIRCULAR) { + // Let DMA keep running - don't disable the channel. HT/TC are only + // enabled here when a refill callback is registered (see + // impl_timerPWMSetDMACircular); non-refilling circular consumers + // never reach this branch. + if (DMA_GET_FLAG_STATUS(descriptor, DMA_IT_HTIF)) { + DMA_CLEAR_FLAG(descriptor, DMA_IT_HTIF); + if (tch->dmaRefillCallback) { + tch->dmaRefillCallback(tch, false); + } + } - // In circular mode, let DMA keep running - don't disable the channel - if (tch->dmaState == TCH_DMA_CIRCULAR) { + if (DMA_GET_FLAG_STATUS(descriptor, DMA_IT_TCIF)) { DMA_CLEAR_FLAG(descriptor, DMA_IT_TCIF); - return; + if (tch->dmaRefillCallback) { + tch->dmaRefillCallback(tch, true); + } } + return; + } + + if (DMA_GET_FLAG_STATUS(descriptor, DMA_IT_TCIF)) { tch->dmaState = TCH_DMA_IDLE; dma_channel_enable(tch->dma->ref,FALSE); tmr_dma_request_enable(tch->timHw->tim, lookupDMASourceTable[tch->timHw->channelIndex], FALSE); @@ -283,6 +299,11 @@ static void impl_timerDMA_IRQHandler(DMA_t descriptor) } } +void impl_timerPWMSetDMARefillCallback(TCH_t * tch, timerDmaRefillFn * callback) +{ + tch->dmaRefillCallback = callback; +} + bool impl_timerPWMConfigChannelDMA(TCH_t * tch, void * dmaBuffer, uint8_t dmaBufferElementSize, uint32_t dmaBufferElementCount) { dma_init_type dma_init_struct = {0}; @@ -411,6 +432,13 @@ void impl_timerPWMStopDMA(TCH_t * tch) { tmr_dma_request_enable(tch->timHw->tim, lookupDMASourceTable[tch->timHw->channelIndex], FALSE); dma_channel_enable(tch->dma->ref,FALSE); + + // AT32: poll enable bit until channel is actually disabled + uint32_t timeout = 10000; // ~40us at 288MHz, well above worst-case disable latency + while (tch->dma->ref->ctrl_bit.chen && timeout--) { + __NOP(); + } + tch->dmaState = TCH_DMA_IDLE; tmr_counter_enable(tch->timHw->tim, TRUE); } @@ -442,12 +470,20 @@ void impl_timerPWMSetDMACircular(TCH_t * tch, bool circular, uint32_t dmaBufferS if (circular) { tch->dma->ref->ctrl_bit.lm = TRUE; dma_data_number_set(tch->dma->ref, dmaBufferSize); - // Disable TC interrupt — in circular mode, TC fires every cycle - // and the IRQ handler would otherwise disable the channel - dma_interrupt_enable(tch->dma->ref, DMA_IT_TCIF, FALSE); + if (tch->dmaRefillCallback) { + // Refill consumer needs an IRQ every half-cycle to keep the + // buffer fed + dma_interrupt_enable(tch->dma->ref, DMA_IT_HTIF, TRUE); + dma_interrupt_enable(tch->dma->ref, DMA_IT_TCIF, TRUE); + } else { + // Disable TC interrupt — in circular mode, TC fires every cycle + // and the IRQ handler would otherwise disable the channel + dma_interrupt_enable(tch->dma->ref, DMA_IT_TCIF, FALSE); + } tch->dmaState = TCH_DMA_CIRCULAR; } else { tch->dma->ref->ctrl_bit.lm = FALSE; + dma_interrupt_enable(tch->dma->ref, DMA_IT_HTIF, FALSE); dma_interrupt_enable(tch->dma->ref, DMA_IT_TCIF, TRUE); tch->dmaState = TCH_DMA_IDLE; } From b824447c212d44b8a104a5f60ef4145504a367b2 Mon Sep 17 00:00:00 2001 From: Ray Morris Date: Tue, 18 Aug 2026 00:36:30 -0500 Subject: [PATCH 6/7] Mark ISR-shared LED strip state volatile; note cache's caller assumption The chunked-buffer refill state (activeLedCount, totalGroups, nextGroupToAssign, groupInHalf, lineIdleLow, lastLowAtUs) is written from both task context and the DMA refill ISR. Accesses don't actually overlap in practice, but marking them volatile documents that and matches the existing convention (TCH_t's dmaState is volatile for the same reason) instead of relying on the compiler not reordering around the assumption. --- src/main/common/colorconversion.c | 4 +++- src/main/drivers/light_ws2811strip.c | 17 +++++++++++------ 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/src/main/common/colorconversion.c b/src/main/common/colorconversion.c index 0ebfedccc68..7bc4a9f7d61 100644 --- a/src/main/common/colorconversion.c +++ b/src/main/common/colorconversion.c @@ -30,7 +30,9 @@ // updates, so a few cached entries catch most repeats without the cost of a // full RGB-native color store. Round-robin replacement is enough here: the // working set per frame is normally <= 4 distinct colors, so eviction order -// doesn't matter much. +// doesn't matter much. Not safe for concurrent callers (e.g. one from an +// ISR, one from task context) — currently fine since the sole caller +// (light_ws2811strip.c) never does that. static struct { hsvColor_t in; rgbColor24bpp_t out; diff --git a/src/main/drivers/light_ws2811strip.c b/src/main/drivers/light_ws2811strip.c index c64dcb8e5ea..91dd4b58553 100644 --- a/src/main/drivers/light_ws2811strip.c +++ b/src/main/drivers/light_ws2811strip.c @@ -122,19 +122,24 @@ bool ledConfigureDMA(void) { return timerPWMConfigChannelDMA(ws2811TCH, ledStripDMABuffer, sizeof(ledStripDMABuffer[0]), WS2811_CHUNK_BUFFER_SIZE); } +// Written from both task context and the DMA refill ISR (never truly +// concurrently — the ISR only runs while a transfer is active, and task +// context only touches these once it isn't — but volatile documents that +// and guards against the compiler assuming otherwise). + // Number of LEDs in the most recent transfer; bounds the DMA transfer (and // ws2811SetIdleHigh's target) to what's actually configured. -static uint16_t activeLedCount = WS2811_LED_STRIP_LENGTH; +static volatile uint16_t activeLedCount = WS2811_LED_STRIP_LENGTH; // groupInHalf[i]: group index currently in half i, so the refill callback // can tell when the group it just finished sending was the last one. -static uint16_t totalGroups; -static uint16_t nextGroupToAssign; -static uint16_t groupInHalf[2]; +static volatile uint16_t totalGroups; +static volatile uint16_t nextGroupToAssign; +static volatile uint16_t groupInHalf[2]; // Shared between normal transfer completion and ws2811SetIdleHigh (PINIO). -static bool lineIdleLow = true; -static timeUs_t lastLowAtUs = 0; +static volatile bool lineIdleLow = true; +static volatile timeUs_t lastLowAtUs = 0; void ws2811LedStripInit(void) { From 429312cc4c1f4cf73ef38e1eb29b781f64e7bb04 Mon Sep 17 00:00:00 2001 From: Ray Morris Date: Tue, 18 Aug 2026 02:14:14 -0500 Subject: [PATCH 7/7] Fix PINIO idle-high regressions from the DMA buffer rewrite The old idle-tail lived in the DMA buffer itself, so it was memory- safe by construction and got resent every transfer. Replacing it with a direct CCR write dropped both properties: - ws2811SetIdleHigh() dereferenced ws2811TCH unconditionally, so a PINIO call after a failed/absent LED strip init (ws2811TCH still NULL) would hard fault. Now guarded the same way ws2811UpdateStrip already is. - ws2811StopTransfer() unconditionally forced the line low, silently discarding a prior ws2811SetIdleHigh(true) once the in-flight transfer finished. Now tracks the requested idle level separately and restores it instead of always going low. --- src/main/drivers/light_ws2811strip.c | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/src/main/drivers/light_ws2811strip.c b/src/main/drivers/light_ws2811strip.c index 91dd4b58553..1bbaae02991 100644 --- a/src/main/drivers/light_ws2811strip.c +++ b/src/main/drivers/light_ws2811strip.c @@ -141,6 +141,10 @@ static volatile uint16_t groupInHalf[2]; static volatile bool lineIdleLow = true; static volatile timeUs_t lastLowAtUs = 0; +// Idle level PINIO last asked for; persists across transfers so +// ws2811StopTransfer() knows what to restore the line to. +static volatile bool idleHighRequested = false; + void ws2811LedStripInit(void) { const timerHardware_t * timHw = timerGetByTag(IO_TAG(WS2811_PIN), TIM_USE_ANY); @@ -223,13 +227,20 @@ static void ws2811RefillHalf(uint8_t halfIndex) } // CCR is preload/shadow-buffered, so the direct write below takes effect -// cleanly at the next period boundary without needing further DMA. +// cleanly at the next period boundary without needing further DMA. Restores +// whatever idle level PINIO last asked for, rather than always going low — +// a transfer finishing shouldn't silently override that. static void ws2811StopTransfer(void) { timerPWMStopDMA(ws2811TCH); - *timerCCR(ws2811TCH) = 0; - lineIdleLow = true; - lastLowAtUs = micros(); + if (idleHighRequested) { + *timerCCR(ws2811TCH) = 255; + lineIdleLow = false; + } else { + *timerCCR(ws2811TCH) = 0; + lineIdleLow = true; + lastLowAtUs = micros(); + } } // transferComplete: true = half 1 just finished (DMA wrapped to half 0), @@ -292,6 +303,12 @@ void ws2811UpdateStrip(uint16_t usedLedCount) void ws2811SetIdleHigh(bool high) { + idleHighRequested = high; // record even if not initialised yet + + if (!ws2811Initialised || !ws2811TCH) { + return; + } + lineIdleLow = !high; *timerCCR(ws2811TCH) = high ? 255 : 0; if (!high) {