Skip to content
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.

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

[Core] Allow custom timings for WS2812 PIO driver #18006

Merged
merged 1 commit into from
Nov 9, 2022

Conversation

KarlK90
Copy link
Member

@KarlK90 KarlK90 commented Aug 12, 2022

Description

Allows setting of custom WS2812 timings using the defines found in ws2812.h, it also uses these timings as it's default now. Furthermore the reset timing is now applied correctly before starting a new transfer. Due to implementation details the waiting times are rounded to 50ns intervals.

WS2812 default timings

Period Time
WS2812_T1H 900ns
WS2812_T1L 350ns
WS2812_T0H 350ns
WS2812_T0L 900ns
RESET 280us

T1H + T1L

image

T0H + T0L

image

Reset

image

SK6812 timings

Period Time
WS2812_T1H 500ns
WS2812_T1L 450ns
WS2812_T0H 400ns
WS2812_T0L 750ns
RESET 80us

T1H + T1L

image

T0H + T0L

image

Reset

Deviation from 80us due to the driver choosing the longer T0H + T0L or T1H + T1L period which is irregular in this case.

image

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • Keyboard (addition or update)
  • Keymap/layout/userspace (addition or update)
  • Documentation

Issues Fixed or Closed by This PR

Checklist

  • My code follows the code style of this project: C, Python
  • I have read the PR Checklist document and have made the appropriate changes.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • I have tested the changes and verified that they work and don't break anything (as well as I can manage).

@github-actions github-actions bot added the core label Aug 12, 2022
@KarlK90 KarlK90 force-pushed the fix/rp2040-ws2812-pio-timings branch 2 times, most recently from 7205957 to c80004e Compare August 12, 2022 12:16
@KarlK90 KarlK90 requested review from a team and sigprof August 12, 2022 12:18
@KarlK90
Copy link
Member Author

KarlK90 commented Aug 12, 2022

CC: @xyzz @jepler

@KarlK90 KarlK90 force-pushed the fix/rp2040-ws2812-pio-timings branch from c80004e to 488310c Compare August 12, 2022 12:51
Copy link
Contributor

@jepler jepler left a comment

Choose a reason for hiding this comment

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

I didn't test with this, as I don't have any RP2040 boards on hand while travelling, but it makes perfectly good sense and looks very compile-time flexible. One other "huh I'm surprised qmk_firmware doesn't...." note below, but it doesn't affect adopting this change.

@KarlK90 KarlK90 force-pushed the fix/rp2040-ws2812-pio-timings branch 2 times, most recently from cc86b14 to 4c00b6a Compare August 14, 2022 18:18
@jepler
Copy link
Contributor

jepler commented Aug 14, 2022

Yay ARRAY_SIZE

@KarlK90
Copy link
Member Author

KarlK90 commented Sep 11, 2022

@sigprof If you happen to have some spare time I would like your opinion on this implementation 🙂

@KarlK90 KarlK90 force-pushed the fix/rp2040-ws2812-pio-timings branch from e526c9b to d2d17d3 Compare September 28, 2022 13:29
@KarlK90 KarlK90 force-pushed the fix/rp2040-ws2812-pio-timings branch from d2d17d3 to bb4836c Compare October 27, 2022 17:28
@KarlK90 KarlK90 requested review from a team and removed request for a team October 27, 2022 17:29
@KarlK90
Copy link
Member Author

KarlK90 commented Nov 8, 2022

Tested and running stable since ~2 months on my RP2040 keyboard

@tzarc tzarc merged commit 27dec8d into qmk:develop Nov 9, 2022
ramonimbao pushed a commit to ramonimbao/qmk_firmware that referenced this pull request Nov 28, 2022
elpekenin pushed a commit to elpekenin/qmk_firmware that referenced this pull request Dec 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants