8000 drivers/ws281x: improve timing for ESP32x by gschorcht · Pull Request #19422 · RIOT-OS/RIOT · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

drivers/ws281x: improve timing for ESP32x #19422

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Mar 25, 2023
Merged

drivers/ws281x: improve timing for ESP32x #19422

merged 1 commit into from
Mar 25, 2023

Conversation

gschorcht
Copy link
Contributor
@gschorcht gschorcht commented Mar 25, 2023 •
edited
Loading

Contribution description

This PR provides a small change which improves the timing for ESP32x SoCs.

If overhead like the loop control or the calculation of the waiting times for the next bit are performed while waiting for the end of the LOW phase, the time required for such operations is included in the LOW phase. This makes both the LOW phase and the period more precise.

According to the definitions

#define WS281X_T_DATA_NS                (1250U)
#define WS281X_T_DATA_ONE_NS            (650U)
#define WS281X_T_DATA_ZERO_NS           (325U)

the following timing is used:

Parameter Value Master this PR
1-Bit Period 1250 1820 1400
1-Bit HIGH 650 690 710
1-Bit LOW 600 1130 690
0-Bit Period 1250 1880 1400
0-Bit HIGH 350 380 400
0-Bit LOW 900 1500 1000

Timing with current master:
ws281x_esp32_current

Timing with this PR:
ws281x_esp32_improved

Testing procedure

tests/driver_ws281x should still work.

Issues/PRs references

@gschorcht
drivers/ws281x: improve timing for ESP32x
c40e015 
If overhead like the loop control or the calculation of the waiting times for the next bit are performed while waiting for the end of the LOW phase, the time required for such operations is included in the LOW phase. This makes both the LOW phase and the period more precise.
@gschorcht gschorcht requested a review from maribu as a code owner March 25, 2023 17:19
@github-actions github-actions bot added the Area: drivers Area: Device drivers label Mar 25, 2023
@gschorcht gschorcht added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Mar 25, 2023
@riot-ci
Copy link
riot-ci commented Mar 25, 2023

Murdock results

✔️ PASSED

c40e015 drivers/ws281x: improve timing for ESP32x

Success Failures Total Runtime
6882 0 6882 10m:32s

Artifacts

maribu
maribu approved these changes Mar 25, 2023
View reviewed changes
Copy link
Member
@maribu maribu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bors merge

Nice one, thx :)

@bors
Copy link
Contributor
bors bot commented Mar 25, 2023

Build succeeded:

@bors bors bot merged commit d50fd0b into RIOT-OS:master Mar 25, 2023
@gschorcht
Copy link
Contributor Author
gschorcht commented Mar 26, 2023 •
edited
Loading

Thanks for reviewing and merging.

BTW, I already have a prototypic driver version that uses the RMT controller of the ESP32x SoC. With the RMT it is possible to generate arbitrary digital waveforms like NEC remote control signals or even WS2812 signals.

The advantages are that the CPU is not busy when generating the WS218x signal and the phase times are accurate to nanoseconds. The disadvantage is that the configuration of the RMT is so complex due to its flexibility that I simply used the ESP-IDF API for RMT. However, this requires about 6 kBytes more ROM, about 3 kBytes more IRAM, and 88 bytes more RAM.

I also decided to use the ESP-IDF API to keep the flexibility of the RMT for other applications. The WS218x driver just needs only one channel of 4 or 8 available channels. We could try to hard-code the configuration of the RMT for WS218x driver, but this would prevent other applications using RMT if the WS281x driver is used.

@gschorcht
Copy link
Contributor Author

However, this requires about 6 kBytes more ROM, about 3 kBytes more IRAM, and 88 bytes more RAM.

We could make the use of the hardware driver optional.

@maribu
Copy link
Member
maribu commented Mar 26, 2023

Sounds like a plan. Since the ws281x backend can be flexible be chosen, one could even opt for the bit-banging driver if ROM and RAM is scarce. But given that all ESP32 MCUs are in the high end hardware leage, most use cases likely can affort to spend RAM and ROM for it. So I'd even say the RMT should be the default backend on ESP32, and people with special use cases that are heavy on memory can opt for the bit-banging driver instead.

@gschorcht gschorcht deleted the drivers/ws281x_esp32_mprove_timing branch March 27, 2023 00:12
@MrKevinWeiss MrKevinWeiss added this to the Release 2023.04 milestone Apr 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maribu maribu maribu approved these changes

Assignees
No one assigned
Labels
Area: drivers Area: Device drivers CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Milestone
Release 2023.04
Development

Successfully merging this pull request may close these issues.
4 participants
@gschorcht @riot-ci @maribu @MrKevinWeiss
0