Add ifndef to WS2812 timing constraints

[Woovie]

Description

Due to the way that the PrimeKB Meridian PCB was assembled (LEDs not to spec), this change is needed in order to properly use the LEDs.

Note that this change alone does not fix the LEDs. A follow-up PR with changes to the keyboard firmware will fully solve the issue.

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

• [Bug] PrimeKB/Meridian LED indicator lights won't turn off. #11755

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

[Woovie]

Added a new commit to fix the linting errors.

[sigprof]

If these defines are exposed as a public interface, as a minimum, they should be renamed to something like WS2812_T0H (or maybe even WS2812_T0H_NS to avoid any confusion about the measurement unit).

However, did you actually check the resulting timings with an oscilloscope or logical analyzer? Some time ago I measured the timings produced by the bitbang driver on STM32F401, and with the default settings (T0H=350, T1H=900, period=1250) I actually got T0H = 167…177 us, T1H = 719…730 us, period = 948…1385 us with a peak at 990 us (but whatever WS2812-like LEDs I had still worked with those timings). So the real problem is that the bitbang driver may be producing timings that are far off the expected values, and your adjustments just make its behavior closer to the spec without completely fixing the problem.

I'm not sure how to fix the bitbang driver completely, however (these things were much simpler on AVR chips, but getting cycle-level precision on various ARM chips may be much more complicated).

[holtenc]

@sigprof Thank you for your thoughts. Your explanation here and in PR#11755 make sense and I think they describe the small issue that was occurring that we were experiencing with the PCBs that utilized actual World Semi WS2812 LEDs. Basically if any of the indicator functions (caps/num/scroll/layer/etc) were spammed then the LEDs would bug out.

My assumption is that the driver, and therefore the default timings, were based on the World Semi WS2812 data sheet. And I assume most WS2812 clone LEDs are manufactured to comparable specs. However, with the second and much larger batch of PCBs, the LEDs that the PCB manufacturer substituted in lieu of actual World Semi WS2812 apparently stray a bit too far off from these specifications. They simply do not work with the default timings available in the chibios WS2812 drivers.

It's possible that the issue is caused by a combination of this deviation from 'standard' spec and what you outlined here and in the other PR. I spent quite a lot of time trying to trouble shoot these LEDs through countless builds and experimentation. Which included both bit bang and SPI drivers. Unfortunately the alternative timings is the only solution that worked.

Specifically it was the T1L and TRST variable that deviated most severely from the 'standard'. Mind you the values I used were based on the hardware datasheet for the actual LED used by the manufacturer.

I just wanted to add a bit of history and context as to how this PR came about. Happy to try any other recommendations you guys have.

quick edit

we did implement and test both of the following suggestions from the other PR

#define WS2812_SPI_USE_CIRCULAR_BUFFER

#define WS2812_SPI_SYNC

Neither produced any positive results

[sigprof]

There is also another option you could try — the PWM driver:
sigprof@bfd905e
Did not actually test this on any F072 hardware, just added options according to the datasheet and tested that it compiles (and it did not compile when I tried to use TIM15_CH2).

Could you test this on your boards? You can also try changing WS2812_PWM_TARGET_PERIOD (which is badly named — it's actually the PWM output frequency in Hz, and the default is 80000 instead of 800000 for some reason). And you could also try changing WS2812_DUTYCYCLE_0 and WS2812_DUTYCYCLE_1 in platforms/chibios/drivers/ws2812_pwm.c to adjust T0H and T1H (the last number in the calculations is the corresponding time in ns; T0L and T1L cannot be adjusted independently — WS2812_PWM_TARGET_PERIOD determines the total bit time, which is the same for 0 and 1).

And yet another workaround you can try is increasing RGBLED_NUM beyond the actual number of LEDs, as was done in #10555 — I don't understand that at all, but apparently this fixed things somehow…

[sigprof]

As for the SPI driver, the timings from it might really be not ideal, but they are also not adjustable. With the default settings on F072 that driver runs SPI at 3 MHz, and this results in T0H = 333 ns, T1H = 1000 ns, period = 1333 ns.

[holtenc]

Unfortunately the PWM driver was a no go. The SPI driver gave poor results. PWN just didn't give any results at all.

Increasing the RGBLED_NUM was was tried extensively with SPI a while back.

[tzarc]

Please change the rest of the file to match the defintiions, as well as adding appropriate documentation.

Would be good if the other ws2812 drivers got changed in the same way as well.

[tzarc]

Retargeted to develop as it's a core change. If we can get these changes sorted out, in the next day or two we'll see if we can get it into the current merge cycle.

[Woovie]

Thank you for the CR, I'll get a new commit out soon.

[Woovie]

Pulled in changes by @Gondolindrim as requested.

[Gondolindrim]

Ok guys, so a summary of the changes:

I incorporated the naming convenstions that @tzarc requested; the new core files were flashed into Meridian PCBs with KTR1010 LEDs, which use different timing parameters than the WS2812 defaults, and work fine.

To test if the changes break the SPI or the PWM driver we used:

• A Meridian using F072 with WS2812s using SPI driver; and
• A Mode SixtyFive HA PCB using F4x1 MCU and PWM driver with DMA access

Both also work splendidly.

The changes also encompass a change in the documentation of the WS2812 drivers; basically these macros can be redefined in the keyboard's config.h file.

Thanks @holtenc , @Woovie and @tzarc for the amazing help!

[holtenc]

Thank you everyone for helping to get this implemented. Cheers!