From 13db65566d590fea872121d51eeacfc5d2d612a2 Mon Sep 17 00:00:00 2001 From: Scott Shawcroft Date: Wed, 19 Jan 2022 16:19:13 -0800 Subject: [PATCH] ESP NeoPixel fixes This tweaks the RMT timing to better match the 1/3 and 2/3 of 800khz guideline for timing. It also ensures a delay of 300 microseconds with the line low before reset. Pin reset is now changed to the IDF default which pulls the pin up rather than CircuitPython's old behavior of floating the pin. Fixes #5679 --- .../common-hal/neopixel_write/__init__.c | 2 +- .../common-hal/microcontroller/Pin.c | 18 ++--------- .../common-hal/neopixel_write/__init__.c | 31 +++++++++++++------ .../common-hal/neopixel_write/__init__.c | 4 +-- supervisor/shared/status_leds.c | 7 ++++- 5 files changed, 32 insertions(+), 30 deletions(-) diff --git a/ports/atmel-samd/common-hal/neopixel_write/__init__.c b/ports/atmel-samd/common-hal/neopixel_write/__init__.c index 754bc97fdf..182b8eee14 100644 --- a/ports/atmel-samd/common-hal/neopixel_write/__init__.c +++ b/ports/atmel-samd/common-hal/neopixel_write/__init__.c @@ -100,7 +100,7 @@ static void neopixel_send_buffer_core(volatile uint32_t *clraddr, uint32_t pinMa ""); } -uint64_t next_start_raw_ticks = 0; +STATIC uint64_t next_start_raw_ticks = 0; void common_hal_neopixel_write(const digitalio_digitalinout_obj_t *digitalinout, uint8_t *pixels, uint32_t numBytes) { // This is adapted directly from the Adafruit NeoPixel library SAMD21G18A code: diff --git a/ports/espressif/common-hal/microcontroller/Pin.c b/ports/espressif/common-hal/microcontroller/Pin.c index 4648fec64d..37a235d0b3 100644 --- a/ports/espressif/common-hal/microcontroller/Pin.c +++ b/ports/espressif/common-hal/microcontroller/Pin.c @@ -36,20 +36,6 @@ STATIC uint32_t never_reset_pins[2]; STATIC uint32_t in_use[2]; -STATIC void floating_gpio_reset(gpio_num_t pin_number) { - // This is the same as gpio_reset_pin(), but without the pullup. - // Note that gpio_config resets the iomatrix to GPIO_FUNC as well. - gpio_config_t cfg = { - .pin_bit_mask = BIT64(pin_number), - .mode = GPIO_MODE_DISABLE, - .pull_up_en = false, - .pull_down_en = false, - .intr_type = GPIO_INTR_DISABLE, - }; - gpio_config(&cfg); - PIN_FUNC_SELECT(GPIO_PIN_MUX_REG[pin_number], 0); -} - void never_reset_pin_number(gpio_num_t pin_number) { if (pin_number == NO_PIN) { return; @@ -72,7 +58,7 @@ void reset_pin_number(gpio_num_t pin_number) { never_reset_pins[pin_number / 32] &= ~(1 << pin_number % 32); in_use[pin_number / 32] &= ~(1 << pin_number % 32); - floating_gpio_reset(pin_number); + gpio_reset_pin(pin_number); } void common_hal_mcu_pin_reset_number(uint8_t i) { @@ -93,7 +79,7 @@ void reset_all_pins(void) { (never_reset_pins[i / 32] & (1 << i % 32)) != 0) { continue; } - floating_gpio_reset(i); + gpio_reset_pin(i); } in_use[0] = never_reset_pins[0]; in_use[1] = never_reset_pins[1]; diff --git a/ports/espressif/common-hal/neopixel_write/__init__.c b/ports/espressif/common-hal/neopixel_write/__init__.c index b63390f155..a5040725e7 100644 --- a/ports/espressif/common-hal/neopixel_write/__init__.c +++ b/ports/espressif/common-hal/neopixel_write/__init__.c @@ -43,20 +43,23 @@ #include "py/mphal.h" #include "py/runtime.h" #include "shared-bindings/neopixel_write/__init__.h" +#include "supervisor/port.h" #include "components/driver/include/driver/rmt.h" #include "peripherals/rmt.h" -#define WS2812_T0H_NS (350) -#define WS2812_T0L_NS (1000) -#define WS2812_T1H_NS (1000) -#define WS2812_T1L_NS (350) -#define WS2812_RESET_US (280) +// 416 ns is 1/3 of the 1250ns period of a 800khz signal. +#define WS2812_T0H_NS (416) +#define WS2812_T0L_NS (416 * 2) +#define WS2812_T1H_NS (416 * 2) +#define WS2812_T1L_NS (416) static uint32_t ws2812_t0h_ticks = 0; static uint32_t ws2812_t1h_ticks = 0; static uint32_t ws2812_t0l_ticks = 0; static uint32_t ws2812_t1l_ticks = 0; +static uint64_t next_start_raw_ticks = 0; + static void IRAM_ATTR ws2812_rmt_adapter(const void *src, rmt_item32_t *dest, size_t src_size, size_t wanted_num, size_t *translated_size, size_t *item_num) { if (src == NULL || dest == NULL) { @@ -107,21 +110,29 @@ void common_hal_neopixel_write(const digitalio_digitalinout_obj_t *digitalinout, if (rmt_get_counter_clock(config.channel, &counter_clk_hz) != ESP_OK) { mp_raise_RuntimeError(translate("Could not retrieve clock")); } - float ratio = (float)counter_clk_hz / 1e9; - ws2812_t0h_ticks = (uint32_t)(ratio * WS2812_T0H_NS); - ws2812_t0l_ticks = (uint32_t)(ratio * WS2812_T0L_NS); - ws2812_t1h_ticks = (uint32_t)(ratio * WS2812_T1H_NS); - ws2812_t1l_ticks = (uint32_t)(ratio * WS2812_T1L_NS); + size_t ns_per_tick = 1e9 / counter_clk_hz; + ws2812_t0h_ticks = WS2812_T0H_NS / ns_per_tick; + ws2812_t0l_ticks = WS2812_T0L_NS / ns_per_tick; + ws2812_t1h_ticks = WS2812_T1H_NS / ns_per_tick; + ws2812_t1l_ticks = WS2812_T1L_NS / ns_per_tick; // Initialize automatic timing translator rmt_translator_init(config.channel, ws2812_rmt_adapter); + // Wait to make sure we don't append onto the last transmission. This should only be a tick or + // two. + while (port_get_raw_ticks(NULL) < next_start_raw_ticks) { + } + // Write and wait to finish if (rmt_write_sample(config.channel, pixels, (size_t)numBytes, true) != ESP_OK) { mp_raise_RuntimeError(translate("Input/output error")); } rmt_wait_tx_done(config.channel, pdMS_TO_TICKS(100)); + // Update the next start to +2 ticks. It ensures that we've gone 300+ us. + next_start_raw_ticks = port_get_raw_ticks(NULL) + 2; + // Free channel again peripherals_free_rmt(config.channel); // Swap pin back to GPIO mode diff --git a/ports/raspberrypi/common-hal/neopixel_write/__init__.c b/ports/raspberrypi/common-hal/neopixel_write/__init__.c index da7bf7c19f..09388af481 100644 --- a/ports/raspberrypi/common-hal/neopixel_write/__init__.c +++ b/ports/raspberrypi/common-hal/neopixel_write/__init__.c @@ -98,6 +98,6 @@ void common_hal_neopixel_write(const digitalio_digitalinout_obj_t *digitalinout, gpio_init(digitalinout->pin->number); common_hal_digitalio_digitalinout_switch_to_output((digitalio_digitalinout_obj_t *)digitalinout, false, DRIVE_MODE_PUSH_PULL); - // Update the next start. - next_start_raw_ticks = port_get_raw_ticks(NULL) + 1; + // Update the next start to +2 ticks. This ensures we give it at least 300us. + next_start_raw_ticks = port_get_raw_ticks(NULL) + 2; } diff --git a/supervisor/shared/status_leds.c b/supervisor/shared/status_leds.c index 0c433271fc..6c7cde28cc 100644 --- a/supervisor/shared/status_leds.c +++ b/supervisor/shared/status_leds.c @@ -48,6 +48,7 @@ uint8_t rgb_status_brightness = 63; #define MICROPY_HW_NEOPIXEL_COUNT (1) #endif +static uint64_t next_start_raw_ticks; static uint8_t status_neopixel_color[3 * MICROPY_HW_NEOPIXEL_COUNT]; static digitalio_digitalinout_obj_t status_neopixel; @@ -218,6 +219,10 @@ void status_led_init() { void status_led_deinit() { #ifdef MICROPY_HW_NEOPIXEL + // Make sure the pin stays low for the reset period. The pin reset may pull + // it up and stop the reset period. + while (port_get_raw_ticks(NULL) < next_start_raw_ticks) { + } common_hal_reset_pin(MICROPY_HW_NEOPIXEL); #elif defined(MICROPY_HW_APA102_MOSI) && defined(MICROPY_HW_APA102_SCK) @@ -265,7 +270,7 @@ void new_status_color(uint32_t rgb) { status_neopixel_color[3 * i + 2] = rgb_adjusted & 0xff; } common_hal_neopixel_write(&status_neopixel, status_neopixel_color, 3 * MICROPY_HW_NEOPIXEL_COUNT); - + next_start_raw_ticks = port_get_raw_ticks(NULL) + 2; #elif defined(MICROPY_HW_APA102_MOSI) && defined(MICROPY_HW_APA102_SCK) for (size_t i = 0; i < MICROPY_HW_APA102_COUNT; i++) { // Skip 4 + offset to skip the header bytes too.