-
-
Notifications
You must be signed in to change notification settings - Fork 40.3k
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
Fix #3566 use an hardware timer for software PWM stability #3615
Fix #3566 use an hardware timer for software PWM stability #3615
Conversation
Timer 1 is used by audio, in some cases, so this may cause issues. |
1946519
to
c988317
Compare
@drashna , Thanks for the input! I've also fixed the mt40 compilation. Hopefully the travis tests will pass now. Thanks, |
I think that a warning should be generated, actually. That way, there isn't any confusion about why it's having issues if you have audio enabled. Though, to clarify, it's the B pins (B5, B6, B7) that use Timer 1. So you could detect if |
c988317
to
02803ce
Compare
Hi @drashna I did something a bit differently than what you suggested. If any of I think this is the best solution, WDYT? I've tested both timers on my xd60, but since I have no keyboard with audio it's possible I've missed something. Thanks! |
02803ce
to
205889b
Compare
That sounds good! |
205889b
to
77b20ff
Compare
@drashna , I've added a small documentation change, but since I'm not an English native speaker, you might want to proof-read it (or propose some rewriting). |
77b20ff
to
7162e37
Compare
@drashna were you able to proof-read the latest version of the documentation I pushed yesterday Let me know if there's any modifications I should add. Thanks, |
Documentation looks good to me |
While thinking about this, I have a better idea that can reuse most of the hardware PWM code and will allow breathing to work in software PWM mode. I'll work on this during the week-end and push there when ready. |
7162e37
to
2ad22d6
Compare
I've pushed the latest changes which works well for my xd60 implementing this last idea:
I haven't unfortunately been able to test if hardware PWM mode is still working (I don't see why it wouldn't), because I don't have any keyboards with this mode. I'd appreciate if someone could test that for me. Thanks for taking a look, I'm ready for comments and suggestions :) |
414b4a1
to
e8df7e6
Compare
Just fixed the conflict in the documentation. I think you can remove the |
Well, that PR is applied, so both PRs should be good to go, I think. |
@jackhumbert @skullydazed @fredizzimo This looks ready to go, and just needs to be reviewed. |
I have a xd60 rev3 and I fetched and checkout the PR, the backlights works better than the origin code but it's not perfect (I'll use it without doubt). |
@jbarrios do you have the rgb underglow activated? I've also noticed that it sometimes flickers when it is activated. I haven't yet found the cause, since it doesn't happen that frequently (it's less than one time per minute). I need to rebuild my firmware with more debug and let it run to "see" why it sometimes jumps... Those flickers happens because PWM is very sensible to timing, so if we "miss" an interrupt for one cycle the led will stay on and it looks like it flickers. At low level, it is more problematic because the "turn off" interrupt can happen approximately at the same time as the "turn on" one (I haven't yet understood exactly how it is possible since normally there can be only one interrupt executing at the same time). |
@jbarrios , I did more testing today. My explanation is that, all rgblight effects are using Unfortunately, I don't see how this can be fixed, unless rewriting the rgb effects to not depend on Meanwhile I suggest using a slow motion RGB effect or no RGB effect at all :) |
Hello, @masterzen, I've been writing without rgblight around the last 3 days and you are right, the flickering has disapear. Actually i'm using a close case and the underglow have no sense at all, I'll use the backlight with this PR. Thank you for the explanation and the implementation, maybe someday we will enjoy all full features :) |
Thanks @jbarrios for confirming. On my side I've found the real cause of flicker during rgb underglow changes. Unfortunately to control the rgb lights, the MCU globally disable the interrupts (see https://github.com/qmk/qmk_firmware/blob/master/drivers/avr/ws2812.c#L275) while sending the data to the rgblights. So if you have a rapidly changing lighting pattern, the longer the MCU will run with interrupts disabled, and the longer the interrupts are disabled, the higher is the chance to have the backlights leds to stay on after the PWM duty cycle, leading to flickering. I don't think this is solvable, because we need the backlight interrupts to arrive on time, but you need a precise timing to control the rgb led array. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Hey @mechmerlin and @drashna, thanks for merging this PR ! |
* 'master' of https://github.com/qmk/qmk_firmware: (99 commits) [Keyboard] Add a new keyboard ADKB96 (qmk#5685) test commit add RGBLIGHT_SPLIT_SET_CHANGE_MODEHSVS; to rgblight_update_dword() add RGBLIGHT_SPLIT_SET_CHANGE_MODEHSVS; to eeconfig_update_rgblight_default() Refactor cospad to current standards and enable support for backlight keycodes (qmk#5582) [Keymap] update (mouse emulation, rev 6 compatibility) (qmk#5696) [Keyboard] fix project zen rev1 bootloader declaration (qmk#5695) [FIX] Misspelled RGB_YELLOW (qmk#5692) [Keymap] Fix broken Shift-Insert binding (qmk#5689) [Keyboard] forgot to omit k05 from the electrical matrix in hhkb layout (qmk#5684) [Keyboard] Fix red an green leds location (qmk#5698) Translate docs into Chinese (qmk#5693) [Keymap] Fix my userspace RGB bug (qmk#5686) Boston meetup 2019 (qmk#5611) [Keymap] Update to Drashna Keymaps (qmk#5594) fix LIB_SRC and QUANTUM_LIB_SRC for ARM (qmk#5623) RGB Matrix Animations: Three/six new reactive effects (wide, cross, nexus) (qmk#5602) Fix qmk#3566 use an hardware timer for software PWM stability (qmk#3615) added info.json for ymd96 (qmk#4982) Define RGB colors (qmk#5300) ...
With my XD60, I noticed that when typing the backlight was flickering. The XD60 doesn't have the backlight wired to a hardware PWM pin. I assumed it was a timing issue in the matrix scan that made the PWM lit the LED a bit too longer. I verified it because the more keys that were pressed, the more lighting I observed. This patch makes the software PWM be called during CPU interruptions. It works almost like the hardware PWM, except instead of using the CPU waveform generation, the CPU will fire interruption when the LEDs need be turned on or off. Using the same timer system as for hardware PWM, when the counter will reach OCRxx (the current backlight level), an Output Compare match interrupt will be fired and we'll turn the LEDs off. When the counter reaches its maximum value, an overflow interrupt will be triggered in which we turn the LEDs on. This way we replicate the hardware backlight PWM duty cycle. This gives a better time stability of the PWM computation than pure software PWM, leading to a flicker free backlight. Since this is reusing the hardware PWM code, software PWM also supports backlight breathing. Note that if timer1 is used for audio, backlight will use timer3, and if timer3 is used for audio backlight will use timer1. If both timers are used for audio, then this feature is disabled and we revert to the matrix scan based PWM computation. Signed-off-by: Brice Figureau <brice@daysofwonder.com>
With my XD60, I noticed that when typing the backlight was flickering. The XD60 doesn't have the backlight wired to a hardware PWM pin. I assumed it was a timing issue in the matrix scan that made the PWM lit the LED a bit too longer. I verified it because the more keys that were pressed, the more lighting I observed. This patch makes the software PWM be called during CPU interruptions. It works almost like the hardware PWM, except instead of using the CPU waveform generation, the CPU will fire interruption when the LEDs need be turned on or off. Using the same timer system as for hardware PWM, when the counter will reach OCRxx (the current backlight level), an Output Compare match interrupt will be fired and we'll turn the LEDs off. When the counter reaches its maximum value, an overflow interrupt will be triggered in which we turn the LEDs on. This way we replicate the hardware backlight PWM duty cycle. This gives a better time stability of the PWM computation than pure software PWM, leading to a flicker free backlight. Since this is reusing the hardware PWM code, software PWM also supports backlight breathing. Note that if timer1 is used for audio, backlight will use timer3, and if timer3 is used for audio backlight will use timer1. If both timers are used for audio, then this feature is disabled and we revert to the matrix scan based PWM computation. Signed-off-by: Brice Figureau <brice@daysofwonder.com>
Will this work for the XD87 (pin D0) ? As that has flickering backlight problems, though it's random, and not related to key presses (as far as I can tell), I assume the firmware is servicing USB events and that causes the soft PWM servicing to get briefly delayed causing the flicker. |
With my XD60, I noticed that when typing the backlight was flickering. The XD60 doesn't have the backlight wired to a hardware PWM pin. I assumed it was a timing issue in the matrix scan that made the PWM lit the LED a bit too longer. I verified it because the more keys that were pressed, the more lighting I observed. This patch makes the software PWM be called during CPU interruptions. It works almost like the hardware PWM, except instead of using the CPU waveform generation, the CPU will fire interruption when the LEDs need be turned on or off. Using the same timer system as for hardware PWM, when the counter will reach OCRxx (the current backlight level), an Output Compare match interrupt will be fired and we'll turn the LEDs off. When the counter reaches its maximum value, an overflow interrupt will be triggered in which we turn the LEDs on. This way we replicate the hardware backlight PWM duty cycle. This gives a better time stability of the PWM computation than pure software PWM, leading to a flicker free backlight. Since this is reusing the hardware PWM code, software PWM also supports backlight breathing. Note that if timer1 is used for audio, backlight will use timer3, and if timer3 is used for audio backlight will use timer1. If both timers are used for audio, then this feature is disabled and we revert to the matrix scan based PWM computation. Signed-off-by: Brice Figureau <brice@daysofwonder.com>
With my XD60, I noticed that when typing the backlight was flickering (see issue #3566).
The XD60 doesn't have the backlight wired to a hardware PWM pin (too bad), unlike the dz60.
I assumed it was a timing issue in the matrix scan that made the PWM lit the LED a bit too longer. I verified it because the more keys that were pressed, the more lighting flicker I observed.
This patch removes the matrix scan based PWM computation, and instead adds a timer interrupt that ticks the backlight PWMs, leading to a better time stability, and removing all flickering on my keyboard :)
I think this patch might not work with some other software PWM based keyboards as I'm using Timer1B (and have only this xd60 to test with).
I'd be more than happy to amend this patch with a more generic fix or a better solution if needed :)
Thanks,
Brice