Fix #3566 use an hardware timer for software PWM stability

[masterzen]

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

[drashna]

Timer 1 is used by audio, in some cases, so this may cause issues.

[masterzen]

@drashna ,

Thanks for the input!
I've modified my PR to disable the hardware timer whenever audio is enabled (do you think that's enough?).

I've also fixed the mt40 compilation. Hopefully the travis tests will pass now.

Thanks,
Brice

[drashna]

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 B5_AUDIO, B6_AUDIO or B7_AUDIO is defined, and then error out then, rather than if audio is enabled in general

[masterzen]

Hi @drashna

I did something a bit differently than what you suggested. If any of B5_AUDIO, B6_AUDIO, B7_AUDIO is enabled, we'll use Timer 3. If it's any of C6_AUDIO, C5_AUDIO, or C4_AUDIO, (or nothing) then we'll use Timer 1. If both timers are in use with audio then we won't use this feature and revert to matrix scan PWM computation.

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!
Brice

[drashna]

That sounds good!

@jackhumbert ?

[masterzen]

@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).

[masterzen]

@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,
Brice

[drashna]

Documentation looks good to me

[masterzen]

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.

[masterzen]

I've pushed the latest changes which works well for my xd60 implementing this last idea:

• reuses most of hardware PWM code
• compatible with Audio if only one pin is in use by Audio
• supports breathing in software mode
• doesn't flicker when typing like software mode
• contains documentation and several code comments to explain it
• support for multiple backlight pins (allows for instance to control the brightness of other leds to match the backlight leds, useful when Caps Lock is mapped as another key and you want the caps lock led to be part of the backlight). This feature is only available in pure software mode and this new hybrid mode, but not hardware mode.

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 :)

[masterzen]

Just fixed the conflict in the documentation. I think you can remove the in progress and needs doc label :)

[drashna]

Well, that PR is applied, so both PRs should be good to go, I think.

[drashna]

@jackhumbert @skullydazed @fredizzimo

This looks ready to go, and just needs to be reviewed.

[jbarrios]

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).
I detected the issues that in sometimes without press any key the backlight flicks, more frequently with low backlight level. And with breathing backlight this flicks are more common when starts the cycle

[masterzen]

@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).

[masterzen]

@jbarrios , I did more testing today.
Indeed this is coming from the rgblight feature. If you deactivate or use a still color or a very slow rgb underglow effect, then there is no flickering appearing.
If you instead use rgblight with a quickly color changing scheme, then flickering appears.

My explanation is that, all rgblight effects are using timer_read and timer_elapsed. Those are blocking interrupts to happen while reading the timer. If interrupts are blocked, even for a very short time, my hybrid implementation interrupts that powers on and off the backlight led is either not called or called to late, leaving the leds on for a bit too long. Indeed the frequency of my hybrid interrupt is quite high to be able to support breathing.

Unfortunately, I don't see how this can be fixed, unless rewriting the rgb effects to not depend on timer_read but instead depend on a fixed time interrupt (like this backlight code as discussed in this PR).

Meanwhile I suggest using a slow motion RGB effect or no RGB effect at all :)

[jbarrios]

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 :)

[masterzen]

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.
Both seems to be exclusive.

[mechmerlin]

Thanks!

[masterzen]

Hey @mechmerlin and @drashna, thanks for merging this PR !
I'm going to update #4324 ASAP.

[al3ph]

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.