timing tweaks with testing

1 files changed, 16 insertions, 9 deletions

diff --git a/ports/stm32f4/common-hal/neopixel_write/__init__.c b/ports/stm32f4/common-hal/neopixel_write/__init__.c
index c5d810ae9..fd46825d0 100644
--- a/ports/stm32f4/common-hal/neopixel_write/__init__.c
+++ b/ports/stm32f4/common-hal/neopixel_write/__init__.c
@@ -35,6 +35,11 @@
 uint64_t next_start_tick_ms = 0;
 uint32_t next_start_tick_us = 1000;
 
+//sysclock divisors
+#define MAGIC_800_INT  900000  // ~1.11 us  -> 1.2  field
+#define MAGIC_800_T0H  2800000  // ~0.36 us -> 0.44 field
+#define MAGIC_800_T1H  1350000  // ~0.74 us -> 0.84 field
+
 void common_hal_neopixel_write (const digitalio_digitalinout_obj_t* digitalinout, uint8_t *pixels, 
                                 uint32_t numBytes) {
     uint8_t *p = pixels, *end = p + numBytes, pix = *p++, mask = 0x80;
@@ -42,32 +47,34 @@ void common_hal_neopixel_write (const digitalio_digitalinout_obj_t* digitalinout
     uint32_t cyc = 0;
 
     //assumes 800_000Hz frequency
+    //Theoretical values here are 800_000 -> 1.25us, 2500000->0.4us, 1250000->0.8us
+    //But they don't work, possibly due to bad optimization? Use tested magic values instead
     uint32_t sys_freq = HAL_RCC_GetSysClockFreq();
-    uint32_t interval = sys_freq/800000;             // cycles per interval (1.25 us). 210@168MHz
-    uint32_t t0 = interval - (sys_freq/2500000);     // 0.4 us
-    uint32_t t1 = interval - (sys_freq/1250000);     // 0.8 us
+    uint32_t interval = sys_freq/MAGIC_800_INT;
+    uint32_t t0 = (sys_freq/MAGIC_800_T0H);
+    uint32_t t1 = (sys_freq/MAGIC_800_T1H);
 
     // This must be called while interrupts are on in case we're waiting for a
     // future ms tick.
     wait_until(next_start_tick_ms, next_start_tick_us);
 
+    GPIO_TypeDef * p_port = pin_port(digitalinout->pin->port);
+    uint32_t p_mask = pin_mask(digitalinout->pin->number);
+
     __disable_irq();
     // Enable DWT in debug core. Useable when interrupts disabled, as opposed to Systick->VAL
-    //ITM->LAR = 0xC5ACCE55; //is this required?
+    //ITM->LAR = 0xC5ACCE55; //this should be required but isn't
     CoreDebug->DEMCR |= CoreDebug_DEMCR_TRCENA_Msk;
     DWT->CTRL |= DWT_CTRL_CYCCNTENA_Msk;
     DWT->CYCCNT = 0;
 
-    GPIO_TypeDef * p_port = pin_port(digitalinout->pin->port);
-    uint32_t p_mask = pin_mask(digitalinout->pin->number);
-
     for(;;) {
+        start = DWT->CYCCNT;
         LL_GPIO_SetOutputPin(p_port, p_mask);
         cyc = (pix & mask) ? t1 : t0;
-        start = DWT->CYCCNT;
         while(DWT->CYCCNT - start < cyc);
         LL_GPIO_ResetOutputPin(p_port, p_mask);
-        if(!(mask >>= 1)) { //max has shifted all the way
+        if(!(mask >>= 1)) {
             if(p >= end) break;
             pix       = *p++;
             mask = 0x80;