drivers/ws281x: add SysTick + GPIO LL backend #20646

maribu · 2024-05-02T21:03:00Z

Contribution description

This implements a new ws281x backend that is largely inspired by @chrysn's periph_timer_poll + GPIO LL backend, but uses SysTick instead of periph_timer_poll. The advantage is that the SysTick timer is provided by all Cortex M MCUs and therefore (hopefully) should work on all Cortex M MCUs.

Testing procedure

Run the tests/drivers/ws281x app on a Cortex M MCU; it should now be used as default backend on Cortex M MCUs.

I tested this successfully with

Issues/PRs references

None

This allow checking if a SysTick timer (provided by all Cortex M MCUs) is present.

This backend is largely inspired by the `periph_timer_poll` + GPIO LL driver, but uses the SysTick timer instead of the `periph_timer`. The main advantage is that this (hopefully) works across Cortex M MCUs with no configuration other than the GPIO pin needed.

riot-ci · 2024-05-03T09:25:09Z

Murdock results

✔️ PASSED

1bbce1f fixup! drivers/ws281x: add SysTick + GPIO LL backend

Success	Failures	Total	Runtime
10088	0	10088	13m:26s

Artifacts

Documentation preview

drivers/ws281x/systick_gpio_ll.c

kfessel · 2024-05-03T17:02:29Z

drivers/ws281x/systick_gpio_ll.c

+            _systick_start(ticks_low);
+            _systick_wait();


ahh i see where something precise is need (high time) there are only: set; wait and clr. and for the low time the inprecise start; wait; and the next start is placed

instead of reconfiguring the systick each time; i would start the systick once configured to some interval that cover high and low and wait for the count flag to raise to raise n times.

eg.:

set the systick to 100ns for each bit b in byte do set gpio if b == 1 3 times syst_wait() else 6 times syst_wait() endif clear gpio if b == 1 6 times syst_wait() else 3 times syst_wait() endif done

this probably makes the whole thing less jittery

That adds a branch in the timing sensitive phase and waiting for 300 ns on a STM32F072 is already pretty challenging with one time starting the systick timer. I doubt that you can start the systick timer three times within that time budget.

not starting you just have it running and wait for n ticks

this is much faster then what you are currently doing ( and also kindof dumber (a little more the stupid of KISS)

maybe the c version in the next comment clears this up

the 3 and the 6 where guess numbers looking at the manual it is

300ns

bit high low

1 2 waits 2 waits

0 1 wait 3 waits

or 400 ns

(ws2512b version 2 but it seems that that should also work with most valid v1 timings)

bit high low

1 2 waits 1 wait

0 1 wait 2 waits

the manuals that i found that had tolerances with the highs an lows would also be fine with the 300 ns and the 1: 2/3 1/3 and 0: 2/3 1/3 ( going close to the tolerances)

What was the argument to split a single wait into three?

In any case: Each of the three waits would have to spin until a flag is set, clear the flag, and spin again. On the nucleo-f072rb the total wait is 13 cycles. Splitting that into three waits means 4 CPU cycles per wait. That is not possible to implement.

https://godbolt.org/z/4nn3a3e7n
see function t2 the wait loop has 2 instructions

Yeah, but a conditional branch will take more than one CPU cycle.

And again: Why divide a single wait? It adds lines or code, bytes to .text. And why into three waits? Wouldn't 42 or 1337 be much better numbers?

because the has timing that are 1/4:3/4 or 1/3:2/3

you can put the branch outside of the time critical path

when i wrote the first suggestion i wasn't aware that we run the nucleo 72 @ 12MHz (I am still not sure but 13 cycles in about a microseconds would fit 12MHz)

but the reconfiguration of the systick is at least 7 instructions (if the register adresses are loaded) and a wait of unknown length

it about not reconfiguring the systimer

this spreads the waits a little better so that they each have least amount of empty loops

_systick_start( 4 ); for (uint8_t cnt = 8; cnt > 0; cnt--) { int bit = !!(data & 0x80); if(!bit) { _systick_wait(); gpio_ll_set(); _systick_wait(); gpio_ll_clear(); _systick_wait(); }else{ _systick_wait(); gpio_ll_set(); _systick_wait(); _systick_wait(); gpio_ll_clear(); } data <<= 1; _systick_wait(); }

kfessel · 2024-05-03T17:23:23Z

drivers/ws281x/systick_gpio_ll.c

+        for (uint8_t cnt = 8; cnt > 0; cnt--) {
+            uint32_t ticks_high = (data & 0x80) ? ticks_one : ticks_zero;
+            uint32_t ticks_low = ticks_bit - ticks_high;
+            _systick_start(ticks_high);
+            gpio_ll_set(port, mask);
+            _systick_wait();
+            gpio_ll_clear(port, mask);
+            _systick_start(ticks_low);
+            _systick_wait();
+            data <<= 1;
+        }


lets try to rewrite my pseudo code in pseudo c

Suggested change

for (uint8_t cnt = 8; cnt > 0; cnt--) {

uint32_t ticks_high = (data & 0x80) ? ticks_one : ticks_zero;

uint32_t ticks_low = ticks_bit - ticks_high;

_systick_start(ticks_high);

gpio_ll_set(port, mask);

_systick_wait();

gpio_ll_clear(port, mask);

_systick_start(ticks_low);

_systick_wait();

data <<= 1;

}

_systick_start( 300ns );

for (uint8_t cnt = 8; cnt > 0; cnt--) {

int bit = !!(data & 0x80);

gpio_ll_set(port, mask);

if(!bit){

_systick_wait();

}else{

_systick_wait();_systick_wait();_systick_wait();

}

gpio_ll_clear(port, mask);

if(!bit){

_systick_wait();_systick_wait();

}else{

_systick_wait();_systick_wait();

}

data <<= 1;

}

and then there is the potential use of the systick irq

global byte global count systick_irq(){ if(count % 4 = 0) gpio_set(); else if (count % 4 =1 && !(byte & 0x80 >> (count / 4))) gpio_clr; else if (count % 4 = 2 ) gpio_clr; count++; if count >= 4 * 8 load next byte and reset count )

The overhead of entering the IRQ, backing up the state etc. takes way longer than the total time budget

ok the irq thing might not be the right choice if you got 4 cycles

maribu · 2024-05-03T20:32:02Z

drivers/ws281x/systick_gpio_ll.c

+     * rather be on the high side by adding a few CPU cycles. */
+    const uint32_t ticks_one = DIV_ROUND_UP((uint64_t)WS281X_T_DATA_ONE_NS * (uint64_t)coreclk(), NS_PER_SEC) + 16;
+    /* the low time should rather be on the short side, so rounding down */
+    const uint32_t ticks_zero = (uint64_t)(WS281X_T_DATA_ZERO_NS - 50) * (uint64_t)coreclk() / NS_PER_SEC;


For 48 MHz this is:

(325 - 50) * 48 * 10^6 / 10^9 = 13.2

so 13 cyles for one wait ( which has (a load, a compare and a jump back if bit 16 isn't set) i think this is plenty of time to do this multiple times.

the short wait (300ns) has exact the same number of wait as with your code but it does not do a setup in the low period but does just three short waits instead.

i thought you where talking about 13cycle for the 1 µsecond

maribu requested review from kaspar030 and bergzand as code owners May 2, 2024 21:03

github-actions bot added Platform: ARM Platform: This PR/issue effects ARM-based platforms Area: build system Area: Build system Area: drivers Area: Device drivers Area: cpu Area: CPU/MCU ports labels May 2, 2024

This was referenced May 2, 2024

drivers/ws281x: Bit banging STM32 support #20302

Open

light_ws2812 #17618

Open

maribu added 2 commits May 3, 2024 09:01

cpu/cortexm_common: provide periph_systick feature

6f6d93b

This allow checking if a SysTick timer (provided by all Cortex M MCUs) is present.

maribu force-pushed the drivers/ws281x branch from 2e45188 to 51d2b85 Compare May 3, 2024 07:01

maribu added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label May 3, 2024

kfessel reviewed May 3, 2024

View reviewed changes

fixup! drivers/ws281x: add SysTick + GPIO LL backend

1bbce1f

maribu commented May 3, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

drivers/ws281x: add SysTick + GPIO LL backend #20646

drivers/ws281x: add SysTick + GPIO LL backend #20646

maribu commented May 2, 2024

riot-ci commented May 3, 2024 •

edited

kfessel May 3, 2024 •

edited

maribu May 3, 2024

kfessel May 3, 2024 •

edited

kfessel May 3, 2024

maribu May 3, 2024

kfessel May 3, 2024

maribu May 3, 2024

kfessel May 3, 2024

kfessel May 3, 2024

kfessel May 3, 2024 •

edited

kfessel May 3, 2024 •

edited

maribu May 3, 2024

kfessel May 3, 2024

maribu May 3, 2024

kfessel May 3, 2024 •

edited

drivers/ws281x: add SysTick + GPIO LL backend #20646

Are you sure you want to change the base?

drivers/ws281x: add SysTick + GPIO LL backend #20646

Conversation

maribu commented May 2, 2024

Contribution description

Testing procedure

Issues/PRs references

riot-ci commented May 3, 2024 • edited

Murdock results

Artifacts

kfessel May 3, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kfessel May 3, 2024 • edited

Choose a reason for hiding this comment

300ns

or 400 ns

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kfessel May 3, 2024 • edited

Choose a reason for hiding this comment

kfessel May 3, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kfessel May 3, 2024 • edited

Choose a reason for hiding this comment

riot-ci commented May 3, 2024 •

edited

kfessel May 3, 2024 •

edited

kfessel May 3, 2024 •

edited

kfessel May 3, 2024 •

edited

kfessel May 3, 2024 •

edited

kfessel May 3, 2024 •

edited