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

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

Closed
wants to merge 39 commits into from

Conversation

chax
Copy link
Contributor

@chax chax commented Aug 9, 2019

Description

This is basically @jackhumbert's arm_rgb branch, merged with latest master branch, resolved all the conflicts and updated to use new format of rgb matrix (led_config_t).

I've been using this branch for a long time with no problem on my planck. I've been regularly merging master and testing. In the end i created new clean branch and merged things in clean way.
I've been waiting for this feature (along with a lot of others who I've seen have been asking for it) to become officially supported by qmk firmware and merged in master branch. Since last update on original branch was several months ago, i decided to merge in latest master and open PR.

If this is already planned to be implemented in another way or @jackhumbert plans to update his branch on his own, then you can close this PR.

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.
  • 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).
    • Tested on planck rev6 with soldered WS2812 leds.
    • Tested building preonic rev3 (unfortunatelly can't test because my preonic doesn't have soldered leds)
    • Tested building few other boards to check if everything still builds without errors.

@zvecr
Copy link
Member

zvecr commented Aug 10, 2019

Unfortunately, this is a duplicate of #6301. As that one already has some traction, my personal preference would be to go with that one.

There are a few things in your branch that could be PRed separately though, like use of RGB matrix (however use of RGB matrix on underglow boards might not be the desired direction and maybe worth a chat on something like Discord).

@chax
Copy link
Contributor Author

chax commented Aug 10, 2019

I wanted to preserve original commits by @jackhumbert, but if #6301 is going to be merged that's also fine.

keyboards/clueboard/66/rev4/chconf.h Outdated Show resolved Hide resolved
@@ -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
Copy link
Member

Choose a reason for hiding this comment

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

This may (does not) need to be changed if STM32_PWM_USE_TIM2 is set to false;

Or if STM32_ST_USE_TIMER was changed to something other than 2.

@@ -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
Copy link
Member

Choose a reason for hiding this comment

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

This may (does not) need to be changed if STM32_PWM_USE_TIM2 is set to false;

Or if STM32_ST_USE_TIMER was changed to something other than 2.

quantum/stm32/chconf.h Outdated Show resolved Hide resolved
@chax
Copy link
Contributor Author

chax commented Aug 10, 2019

@drashna I reverted frequency and then i started playing a bit with timers and tick-less mode. I can't find working combination of timers so that RGB together with rest of the board's features work normally, this is basically the only working combination. Do you have any suggestion what to try?

@chax chax mentioned this pull request Aug 10, 2019
13 tasks
@kratsyn
Copy link

kratsyn commented Aug 12, 2019

I'm waiting on some led's before i start testing with my boards. It may be possible to shift the timer/pwmd usage around to allow for this functionality without impacting ST. Another thing i would like to test is moving the slave timer from tim3 to TIM15|16|17. This challenge there is our version of PAL does not support anything above TIM8 so timer init and pwm control will need to be done at the register level.

@kratsyn
Copy link

kratsyn commented Aug 12, 2019

@chax you can try dumping the ST timer resolution down to 16 (CH_CFG_ST_RESOLUTION) in chconf.h and then changing ST to something other than 2 (i would suggest 4).

Note that this still impacts performance but it should not be nearly as intrusive to function.

@zvecr
Copy link
Member

zvecr commented Aug 12, 2019

@kratsyn TIM4 is still used by some IO pins, see #6473 as an example use case on f303.

@drashna
Copy link
Member

drashna commented Aug 12, 2019

That is my main concern, actually.

@kratsyn
Copy link

kratsyn commented Aug 12, 2019

That's actually the one that pulled me into messing with the led on the proton. If we're speaking of the olk boards, and current mainline capability, then TIM3 would be the non-intrusive choice.

It should be relatively easy to move the slave timer in this implementation over to TIM1 with some changes to the dma assignments.

@kratsyn
Copy link

kratsyn commented Aug 12, 2019

I'm not sure where to continue this discussion so i'll just reply back here.

I've mocked up an example using my working branch kratsyn:arm_ws2812-krats.

The only build failure is on hadron/ver3 and it appears that is because of how i have wrapped the configuration.

@chax
Copy link
Contributor Author

chax commented Aug 12, 2019

@kratsyn i tried your branch and leds don't work, also some letters are typed out double.

TThiss is an examplee oof typing with yoour versioon of fiirimware

@kratsyn
Copy link

kratsyn commented Aug 12, 2019

i'll have to dig further but i would guess it is the pwm writing those pins high when leddriverinit() is called. I figured that bit was problematic but this confirms it...

@e11i0t23
Copy link
Contributor

however use of RGB matrix on underglow boards might not be the desired direction

Ws2812b are currently supported as both rgb matrix as well as for underglow as some people do use them for perkeyrgb

@zvecr
Copy link
Member

zvecr commented Aug 12, 2019

@e11i0t23 I know ws2812 can be used for both RGB matrix and RGB light, however my comment was directed to the boards which have been converted within this PR. These are typical underglow boards, to which RGB matrix might be considered overkill, and/or just different and unexpected compared to the rest of the repo.

@kratsyn
Copy link

kratsyn commented Aug 13, 2019

Both configurations should be supportable once i wrap my head around the hsv<->rgb logic, I've synthesized an alternate solution that has less resource impact but it still needs to be fleshed out to work with the color conversion array. I'm also still waiting on my led's so any further testing will have to wait until they arrive.

@kratsyn
Copy link

kratsyn commented Aug 13, 2019

I should clarify that i do own a rev6 planck and a rev3 preonic so solving for the PR should be achievable.

@kratsyn
Copy link

kratsyn commented Aug 17, 2019

I have an alternative implementation working that only uses one dma channel and one timer. It will push an 8x8 grid and should default to olkb board settings. It is still a bit rough around the edges...

you can find it on my repo in the arm_ws2812_alt branch.

@drashna
Copy link
Member

drashna commented Sep 21, 2019

Looks like there are a number of merge conflicts here, now.

Could you update the PR to resolve these?

@kratsyn
Copy link

kratsyn commented Sep 21, 2019

Feel free to review PR#6675 for an alternative implementation that does not consume 2 TIM devices and 3 DMA channels.

@chax
Copy link
Contributor Author

chax commented Sep 21, 2019

I would close this in favour of #6675 from @kratsyn

@noroadsleft
Copy link
Member

Closing as requested.

@chax, thank you for your efforts and contributions to this, all the same.

@zvecr zvecr mentioned this pull request Jan 19, 2020
13 tasks
@chax chax deleted the arm_rgb branch March 16, 2020 14:05
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.

7 participants