drivers/sled1734x: Refactor the register to LED binding for reduced RAM usage - #478
drivers/sled1734x: Refactor the register to LED binding for reduced RAM usage#478DEREFERENC3D wants to merge 1 commit into
Conversation
github-actions
Bot
added
documentation
This effectively swaps the PWM buffer and config, decreasing the former's size by making the latter larger. Now the config contains an entry for each register in each IC, providing a lookup table that allows to determine the LED index and color channel that register is used for. Unused registers should be set to an index of NO_LED (255). The PWM buffer has been replaced with an LED buffer, storing a 3 byte RGB structure for each LED, thus minimizing its size from 256 to 3 * (LED count). In combination with the new config, this allows us to construct the relevant chunk of the buffer at flush time, without adding too much complexity or performance overhead (I have not noticed any impact, but I'm just eyeballing it). This means that now the buffer size is dependent on the amount of LEDs defined. On keyboards with less LEDs the savings will be greater. Previous buffer size: * 256 bytes for PWM registers * 32 bytes for control registers * a few bytes for the "dirty" state booleans => ~294 bytes per IC New buffer size: * 3 * (LED count) bytes * 32 bytes for control registers * a few bytes for the "dirty" state booleans => ~38 b per IC + 3 b per LED On the Redragon Aryaman K569, which uses two ICs and has 118 LEDs, this translates to: * previously: 294 bytes * 2 ICs = 588 bytes * now: ((32 b + 6 b) * 2 ICs) + (3 colors * 118 LEDs) = 430 bytes => difference: 588 b - 430 b = 158 bytes saved
8619379 to
2419575
Compare
|
Sorry, forgot the formatting 😅 Should be fixed now. |
|
I have my reservations. Less RAM footprint? Sure. |
|
Thanks for the comment, that's a good point.
I didn't realize this was core style (I only ever used the WS2812 driver on mainline QMK so far), I agree we should stay with it where possible.
Another issue I now thought of is that merging this would be a doubly bad deal for 24x-based boards (driver config rewrite needed for a little RAM they don't really need). But I thought it's significant enough for 26x chips to submit it - boards with fewer LEDs could probably gain 200+ bytes, that's 10% total RAM. The new binding is still 1:1, so I think it would not be too difficult to codegen it from a core-style config. What do you think about such an approach, should I give it a try? |
26x suffers from RAM starvation, so any bytes saved is good. However most 26x designs max out the controller
Codegen might be the solution here. You might have to change the struct signature, but the keyboard declared triplets should ideally stay the same. It will be delicate to actually save RAM, but worth a try. |
2 participants
Description
The SLED1734X driver stores all the of the IC's register values in memory, but many of them are unused, either as a result of the wiring method used, or simply because the keyboard does not feature enough LEDs. This change refactors the register to LED binding, so that instead of storing all the register values in RAM and the binding in PROGMEM, we now do the opposite: store all LED color values in RAM, and a register to LED index and color channel binding in the ROM. This way, we do not waste memory by storing the values of the unused registers, and the memory usage now scales with LED count, rather than being a flat value per IC.
Before:
Total: ~294 bytes per IC
After:
Total: ~38 bytes per IC + 3 bytes per LED
On the flip side, the stack usage may be slightly increased, given that I introduced a few more variables into the relevant functions.
I have tested these changes on the Redragon Aryaman K569 (branch and commit updating the config for this change), and have not noticed any adverse effects. On this keyboard, which features 2 of these ICs and 118 LEDs, this change saves 158 bytes of memory, and on boards with just one IC and / or less LEDs, the savings will be greater.
Types of Changes
Issues Fixed or Closed by This PR
Checklist