From 18e561b82cdb71bed0162b3ce4083a2d269e5336 Mon Sep 17 00:00:00 2001 From: Joshua Diamond Date: Fri, 19 Jun 2020 22:59:28 -0400 Subject: [PATCH] Fix incorrect delay when setting WS2812 (and similar) leds (#9302) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fix incorrect delay when setting WS2812 (and similar) leds * Add documentation for WS2812_DELAY_MICROSECONDS * Remove improper cast to uint8_t Co-authored-by: Sergey Vlasov * Remove unneeded cast to uint8_t and correct math Co-authored-by: Sergey Vlasov * microseconds -> µs Co-authored-by: Ryan * Make documentation better match the spec sheet. Co-authored-by: Ryan * Rename macro to match spec sheet * Further correction to the delay maths for the SPI case. Co-authored-by: Joel Challis * Move ws2812_common.h to the drivers directory * Revert "Further correction to the delay maths for the SPI case." This reverts commit e61b56a2cfc7dfec9992a7a3af92afa50e5b8ec0. * Remove ws2812_setleds_pin(); consolidate ws2812.h Co-authored-by: Sergey Vlasov Co-authored-by: Ryan Co-authored-by: Joel Challis --- docs/ws2812_driver.md | 9 +++++++++ drivers/avr/ws2812.c | 20 +++++--------------- drivers/chibios/ws2812.c | 2 +- drivers/chibios/ws2812.h | 16 ---------------- drivers/chibios/ws2812_pwm.c | 7 +++---- drivers/chibios/ws2812_spi.c | 2 +- drivers/{avr => }/ws2812.h | 18 +++++++++--------- keyboards/wilba_tech/wt_rgb_backlight.c | 2 +- 8 files changed, 29 insertions(+), 47 deletions(-) delete mode 100644 drivers/chibios/ws2812.h rename drivers/{avr => }/ws2812.h (75%) diff --git a/docs/ws2812_driver.md b/docs/ws2812_driver.md index fe5f88585a..941e1bde08 100644 --- a/docs/ws2812_driver.md +++ b/docs/ws2812_driver.md @@ -19,6 +19,15 @@ These LEDs are called "addressable" because instead of using a wire per color, e ## Driver configuration +### All drivers + +Different versions of the addressable LEDs have differing requirements for the TRST period between frames. +The default setting is 280 µs, which should work for most cases, but this can be overridden in your config.h. e.g.: + +```c +#define WS2812_TRST_US 80 +``` + ### Bitbang Default driver, the absence of configuration assumes this driver. To configure it, add this to your rules.mk: diff --git a/drivers/avr/ws2812.c b/drivers/avr/ws2812.c index 5c3d72dcb5..dd2ef89912 100644 --- a/drivers/avr/ws2812.c +++ b/drivers/avr/ws2812.c @@ -36,25 +36,15 @@ static inline void ws2812_sendarray_mask(uint8_t *data, uint16_t datlen, uint8_t masklo, uint8_t maskhi); -// Setleds for standard RGB -void inline ws2812_setleds(LED_TYPE *ledarray, uint16_t number_of_leds) { - // wrap up usage of RGB_DI_PIN - ws2812_setleds_pin(ledarray, number_of_leds, RGB_DI_PIN); -} +void ws2812_setleds(LED_TYPE *ledarray, uint16_t number_of_leds) { + DDRx_ADDRESS(RGB_DI_PIN) |= pinmask(RGB_DI_PIN); -void ws2812_setleds_pin(LED_TYPE *ledarray, uint16_t number_of_leds, uint8_t pin) { - DDRx_ADDRESS(RGB_DI_PIN) |= pinmask(pin); - - uint8_t masklo = ~(pinmask(pin)) & PORTx_ADDRESS(pin); - uint8_t maskhi = pinmask(pin) | PORTx_ADDRESS(pin); + uint8_t masklo = ~(pinmask(RGB_DI_PIN)) & PORTx_ADDRESS(RGB_DI_PIN); + uint8_t maskhi = pinmask(RGB_DI_PIN) | PORTx_ADDRESS(RGB_DI_PIN); ws2812_sendarray_mask((uint8_t *)ledarray, number_of_leds * sizeof(LED_TYPE), masklo, maskhi); -#ifdef RGBW - _delay_us(80); -#else - _delay_us(50); -#endif + _delay_us(WS2812_TRST_US); } /* diff --git a/drivers/chibios/ws2812.c b/drivers/chibios/ws2812.c index 2c2d9fb2dc..5917b7f0ec 100644 --- a/drivers/chibios/ws2812.c +++ b/drivers/chibios/ws2812.c @@ -51,7 +51,7 @@ // The reset gap can be 6000 ns, but depending on the LED strip it may have to be increased // to values like 600000 ns. If it is too small, the pixels will show nothing most of the time. -#define RES 10000 // Width of the low gap between bits to cause a frame to latch +#define RES (1000 * WS2812_TRST_US) // Width of the low gap between bits to cause a frame to latch void sendByte(uint8_t byte) { // WS2812 protocol wants most significant bits first diff --git a/drivers/chibios/ws2812.h b/drivers/chibios/ws2812.h deleted file mode 100644 index 41c22a00b8..0000000000 --- a/drivers/chibios/ws2812.h +++ /dev/null @@ -1,16 +0,0 @@ -#pragma once - -#include "quantum/color.h" - -/* User Interface - * - * Input: - * ledarray: An array of GRB data describing the LED colors - * number_of_leds: The number of LEDs to write - * - * The functions will perform the following actions: - * - Set the data-out pin as output - * - Send out the LED data - * - Wait 50us to reset the LEDs - */ -void ws2812_setleds(LED_TYPE *ledarray, uint16_t number_of_leds); diff --git a/drivers/chibios/ws2812_pwm.c b/drivers/chibios/ws2812_pwm.c index ba45d00425..7113db11e0 100644 --- a/drivers/chibios/ws2812_pwm.c +++ b/drivers/chibios/ws2812_pwm.c @@ -53,11 +53,10 @@ /** * @brief Number of bit-periods to hold the data line low at the end of a frame * - * The reset period for each frame must be at least 50 uS; so we add in 50 bit-times - * of zeroes at the end. (50 bits)*(1.25 uS/bit) = 62.5 uS, which gives us some - * slack in the timing requirements + * The reset period for each frame is defined in WS2812_TRST_US. + * Calculate the number of zeroes to add at the end assuming 1.25 uS/bit: */ -#define WS2812_RESET_BIT_N (50) +#define WS2812_RESET_BIT_N (1000 * WS2812_TRST_US / 1250) #define WS2812_COLOR_BIT_N (RGBLED_NUM * 24) /**< Number of data bits */ #define WS2812_BIT_N (WS2812_COLOR_BIT_N + WS2812_RESET_BIT_N) /**< Total number of bits in a frame */ diff --git a/drivers/chibios/ws2812_spi.c b/drivers/chibios/ws2812_spi.c index 3bbada7fef..7a1d2f05da 100644 --- a/drivers/chibios/ws2812_spi.c +++ b/drivers/chibios/ws2812_spi.c @@ -36,7 +36,7 @@ #define NB_COLORS 3 #define BYTES_FOR_LED (BYTES_FOR_LED_BYTE * NB_COLORS) #define DATA_SIZE (BYTES_FOR_LED * RGBLED_NUM) -#define RESET_SIZE 200 +#define RESET_SIZE (1000 * WS2812_TRST_US / (2 * 1250)) #define PREAMBLE_SIZE 4 static uint8_t txbuf[PREAMBLE_SIZE + DATA_SIZE + RESET_SIZE] = {0}; diff --git a/drivers/avr/ws2812.h b/drivers/ws2812.h similarity index 75% rename from drivers/avr/ws2812.h rename to drivers/ws2812.h index 88eb081894..370b14f3e8 100644 --- a/drivers/avr/ws2812.h +++ b/drivers/ws2812.h @@ -1,11 +1,4 @@ /* - * light weight WS2812 lib include - * - * Version 2.3 - Nev 29th 2015 - * Author: Tim (cpldcpu@gmail.com) - * - * Please do not change this file! All configuration is handled in "ws2812_config.h" - * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by * the Free Software Foundation, either version 2 of the License, or @@ -24,12 +17,20 @@ #include "quantum/color.h" +/* + * Older WS2812s can handle a reset time (TRST) of 50us, but recent + * component revisions require a minimum of 280us. + */ + +#if !defined(WS2812_TRST_US) +#define WS2812_TRST_US 280 +#endif + /* User Interface * * Input: * ledarray: An array of GRB data describing the LED colors * number_of_leds: The number of LEDs to write - * pin (optional): A pin_t definition for the line to drive * * The functions will perform the following actions: * - Set the data-out pin as output @@ -37,4 +38,3 @@ * - Wait 50us to reset the LEDs */ void ws2812_setleds(LED_TYPE *ledarray, uint16_t number_of_leds); -void ws2812_setleds_pin(LED_TYPE *ledarray, uint16_t number_of_leds, uint8_t pin); diff --git a/keyboards/wilba_tech/wt_rgb_backlight.c b/keyboards/wilba_tech/wt_rgb_backlight.c index e2506bf3f9..92e14165ee 100644 --- a/keyboards/wilba_tech/wt_rgb_backlight.c +++ b/keyboards/wilba_tech/wt_rgb_backlight.c @@ -55,7 +55,7 @@ #endif #if defined(RGB_BACKLIGHT_DAWN60) -#include "drivers/avr/ws2812.h" +#include "ws2812.h" LED_TYPE g_ws2812_leds[WS2812_LED_TOTAL]; #endif