WS2812 ARM Driver (rgblight+rgb_matrix) #6675

kratsyn · 2019-09-05T04:38:32Z

I know there is an existing PR for RGB support for arm however there has not been much movement in those PRs and this implementation differs enough to warrant new discussion.

Description

This implementation uses a single timer and dma channel to avoid affecting an entire gpio port and reduces the chances of impacting other feature usage. One caveat to note is that we are forced to reduce the global system clock to avoid having to introduce a divisor to timer_read(). There are no system level performance impacts for this change as it will only affect the virtual timer implementation within chibi.

This PR has been tested against the following boards: planck rev6, preonic rev3, proton_c, f103 bluepill, f103 blackpill, and robodyn black pill (f303).

Types of Changes

Issues Fixed or Closed by This PR

Unknown

Checklist

My code follows the code style of this project.
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).

…eriod and dutycycle values.

…, tickless not supported on ST frquencies lower than 10khz. The alternate for this would be to add a divisor to timer_read() to account for the increased timer resolution. (lots of impacts to userland and custom builds)

brought back matrix.c and removed dipswitch specific functionality that is now covered by the quantum 'driver'.

This reverts commit 8bb1c26. Issue actually introduced by problem with merge.

drashna · 2019-09-06T03:51:42Z

While the approach may not be identical, it's very similar.

#4670, #6301, #6516 all implement WS2812 support for ARM.

This is NOT a dig at you, this is mostly for notes and reference.

kratsyn · 2019-09-06T06:57:18Z

I was not aware of #4670. It would be nice to have both options available.

…

On Thu, Sep 5, 2019 at 10:51 PM Drashna Jaelre ***@***.***> wrote: While the approach may not be identical, it's very similar. #4670 <#4670>, #6301 <#6301>, #6515 <#6515> all implement WS2812 support for ARM. This is NOT a dig at you, this is mostly for notes and reference. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#6675?email_source=notifications&email_token=AAPFL2JFC2DMJKLWL4V7MCDQIHHVRA5CNFSM4ITZHYO2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD6BULWI#issuecomment-528696793>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAPFL2MJR5QKZSO7QDB5UPLQIHHVRANCNFSM4ITZHYOQ> .

drashna · 2019-09-06T14:42:56Z

I was not aware of #4670. It would be nice to have both options available.

Absolutely! However, that PR is (in theory) waiting on an update to ChibiOS (the main ARM HAL that we use).

nooges · 2019-09-08T19:17:09Z

There's quite a bit of mixing of tabs and spaces in the PR.

kratsyn · 2019-09-08T19:41:40Z

True. I'll take a look at correcting those this week.

…

On Sun, Sep 8, 2019, 2:17 PM Danny ***@***.***> wrote: There's quite a bit of mixing of tabs and spaces in the PR. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#6675?email_source=notifications&email_token=AAPFL2IJQMVVTZS7YUPWCZDQIVFT3A5CNFSM4ITZHYO2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD6FXKJA#issuecomment-529233188>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAPFL2IAB4XC4N6OFDH5IP3QIVFT3ANCNFSM4ITZHYOQ> . On Sun, Sep 8, 2019, 2:17 PM Danny ***@***.***> wrote: There's quite a bit of mixing of tabs and spaces in the PR. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#6675?email_source=notifications&email_token=AAPFL2IJQMVVTZS7YUPWCZDQIVFT3A5CNFSM4ITZHYO2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD6FXKJA#issuecomment-529233188>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAPFL2IAB4XC4N6OFDH5IP3QIVFT3ANCNFSM4ITZHYOQ> .

…dtab;retab)

kratsyn · 2019-09-22T00:31:10Z

I would like some input or normalizing the configuration on this pr. I'm not really a fan of WS2812 specific vars but i also do not want to make any choices in a vacuum.

hsgw · 2019-09-22T16:26:39Z

@kratsyn
Hi, I tried this code on stm32f072 board.
I found wrong setting In ws2812.c L41,

#define WS2812_PWM_PERIOD                   (WS2812_PWM_FREQUENCY / 80000)

This should divide by 800000.
Could you test this?

zvecr · 2019-09-22T21:08:25Z

This PR has been tested against the following boards: planck rev6, preonic rev3, proton_c, f103 bluepill, f103 blackpill, and robodyn black pill (f303).

@kratsyn are you sure this has been tested on f103? My understanding is PAL_MODE_ALTERNATE() does not exist on that platform.

kratsyn · 2019-09-22T21:52:04Z

Crud. Good catch, seems I missed bringing that ifdef statement from my f1 test setup. I'm tied up with family until mid week so this likely will not happen until then. I'll also take a look at the period math for the f0.

…

On Sun, Sep 22, 2019, 4:08 PM Joel Challis ***@***.***> wrote: This PR has been tested against the following boards: planck rev6, preonic rev3, proton_c, f103 bluepill, f103 blackpill, and robodyn black pill (f303). @kratsyn <https://github.com/kratsyn> are you sure this has been tested on f103? My understanding is PAL_MODE_ALTERNATE() does not exist on that platform. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#6675?email_source=notifications&email_token=AAPFL2LYUI5ZX7IFBKFKWMDQK7NFDA5CNFSM4ITZHYO2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD7JPJRI#issuecomment-533918917>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAPFL2KYFUIKBO6UD3AUKOLQK7NFDANCNFSM4ITZHYOQ> .

kratsyn · 2019-10-06T22:51:37Z

I've not spaced on this, dealing with family drama and will resume when i have the time the spare.

chax · 2019-10-08T20:45:06Z

quantum/stm32/chconf.h


 /**
 * @brief   System tick frequency.
 * @details Frequency of the system timer that drives the system ticks. This
 *          setting also defines the system tick time unit.
 */
-#    define CH_CFG_ST_FREQUENCY 100000
+#    define CH_CFG_ST_FREQUENCY	1000


use space instead of tab

chax · 2019-10-08T20:45:12Z

quantum/stm32/chconf.h

@@ -58,7 +58,7 @@
 *          The value one is not valid, timeouts are rounded up to
 *          this value.
 */
-#    define CH_CFG_ST_TIMEDELTA 2
+#    define CH_CFG_ST_TIMEDELTA	0


use space instead of tab

Ideally instead this should be run through clang-format instead

chax · 2019-10-08T20:45:45Z

keyboards/preonic/rev3/config.h

+#define RGB_DI_PIN      A1
+#ifdef RGB_DI_PIN
+  #define RGBLED_NUM		9
+  #define DRIVER_LED_TOTAL	RGBLED_NUM


use space instead of tab

chax · 2019-10-08T20:46:05Z

keyboards/planck/rev6/config.h

+#define RGB_DI_PIN      A1
+#ifdef RGB_DI_PIN
+  #define RGBLED_NUM		9
+  #define DRIVER_LED_TOTAL	RGBLED_NUM


use space instead of tab

drashna · 2019-10-23T07:58:31Z

tmk_core/protocol/chibios/main.c

@@ -206,6 +209,9 @@ int main(void) {
 #endif

        keyboard_task();
+#if defined(RGBLIGHT_ANIMATIONS) & defined(RGBLIGHT_ENABLE)


Suggested change

#if defined(RGBLIGHT_ANIMATIONS) & defined(RGBLIGHT_ENABLE)

#if defined(RGBLIGHT_ANIMATIONS) && defined(RGBLIGHT_ENABLE)

zvecr · 2019-11-07T16:37:10Z

Now #7183 and #7173 have dropped, we now have a new entrypoint for a pwm based implementation. When fixing the merge conflicts, could you please target drivers/arm/ws2812_pwm.c instead 😃

stale · 2019-12-09T23:29:52Z

Thank you for your contribution!
This pull request has been automatically closed because it has not had activity in the last 30 days. Please feel free to give a status update now, ping for review, or re-open when it's ready.

kratsyn and others added 12 commits September 3, 2019 02:04

new branch.. new start from master

c4ee4b0

build fixes

4accf62

'final' version. removed noisy debug print statements and finalized p…

8438fe7

…eriod and dutycycle values.

enable the use of rgbmatrix

aab3308

clarified comment regarding default settings

620e73b

more clarifying comments for configuration

26c68f0

enable rgb_matrix on olkb boards

c9de71c

Merge branch 'master' into ws2812_arm

b2133f6

resolved build issue introduced by #6104

8bb1c26

brought back matrix.c and removed dipswitch specific functionality that is now covered by the quantum 'driver'.

Revert "resolved build issue introduced by #6104"

11aadf1

This reverts commit 8bb1c26. Issue actually introduced by problem with merge.

resolved issues with merge from master

ae7dddb

drashna requested a review from a team September 6, 2019 03:46

drashna added core duplicate labels Sep 6, 2019

add rgblight_task() to main()

91c8b1e

replaced tabs with 4 space indents. (set tabstop=4 shiftwidth=4 expan…

08daac0

…dtab;retab)

chax mentioned this pull request Sep 21, 2019

WS2812 support for ARM boards (planck rev6, preonic rev3) #6516

Closed

13 tasks

chax reviewed Oct 8, 2019

View reviewed changes

drashna reviewed Oct 23, 2019

View reviewed changes

zvecr added the awaiting changes label Nov 7, 2019

stale bot closed this Dec 9, 2019

zvecr mentioned this pull request Jan 19, 2020

PWM DMA based RGB Underglow for STM32 #7928

Merged

13 tasks

kratsyn deleted the ws2812_arm branch April 2, 2020 09:53

rileyinman mentioned this pull request Jul 16, 2020

planck/rev6: Enable WS2812 LED matrix with pwm drivers #9735

Merged

13 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WS2812 ARM Driver (rgblight+rgb_matrix) #6675

WS2812 ARM Driver (rgblight+rgb_matrix) #6675

kratsyn commented Sep 5, 2019 •

edited

Loading

drashna commented Sep 6, 2019 •

edited

Loading

kratsyn commented Sep 6, 2019 via email

drashna commented Sep 6, 2019

nooges commented Sep 8, 2019

kratsyn commented Sep 8, 2019 via email

kratsyn commented Sep 22, 2019

hsgw commented Sep 22, 2019

zvecr commented Sep 22, 2019

kratsyn commented Sep 22, 2019 via email

kratsyn commented Oct 6, 2019

chax Oct 8, 2019

chax Oct 8, 2019

zvecr Oct 8, 2019

chax Oct 8, 2019

chax Oct 8, 2019

drashna Oct 23, 2019

zvecr commented Nov 7, 2019

stale bot commented Dec 9, 2019

	#if defined(RGBLIGHT_ANIMATIONS) & defined(RGBLIGHT_ENABLE)
	#if defined(RGBLIGHT_ANIMATIONS) && defined(RGBLIGHT_ENABLE)

WS2812 ARM Driver (rgblight+rgb_matrix) #6675

WS2812 ARM Driver (rgblight+rgb_matrix) #6675

Conversation

kratsyn commented Sep 5, 2019 • edited Loading

Description

Types of Changes

Issues Fixed or Closed by This PR

Checklist

drashna commented Sep 6, 2019 • edited Loading

kratsyn commented Sep 6, 2019 via email

drashna commented Sep 6, 2019

nooges commented Sep 8, 2019

kratsyn commented Sep 8, 2019 via email

kratsyn commented Sep 22, 2019

hsgw commented Sep 22, 2019

zvecr commented Sep 22, 2019

kratsyn commented Sep 22, 2019 via email

kratsyn commented Oct 6, 2019

chax Oct 8, 2019

Choose a reason for hiding this comment

chax Oct 8, 2019

Choose a reason for hiding this comment

zvecr Oct 8, 2019

Choose a reason for hiding this comment

chax Oct 8, 2019

Choose a reason for hiding this comment

chax Oct 8, 2019

Choose a reason for hiding this comment

drashna Oct 23, 2019

Choose a reason for hiding this comment

zvecr commented Nov 7, 2019

stale bot commented Dec 9, 2019

kratsyn commented Sep 5, 2019 •

edited

Loading

drashna commented Sep 6, 2019 •

edited

Loading