From c43b321d9ca76a27cdbe80a08d0000dfbc0d2c7b Mon Sep 17 00:00:00 2001 From: "Earle F. Philhower, III" Date: Wed, 2 Jan 2019 20:11:46 -0800 Subject: [PATCH 1/7] Make waveform generator a NMI to run always Make the waveform generator an NMI using the same code as in 2.4.0. Making it NMI will ensure it runs even when interrupts are disabled. Fixes #5568 --- cores/esp8266/core_esp8266_waveform.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/cores/esp8266/core_esp8266_waveform.c b/cores/esp8266/core_esp8266_waveform.c index 8df6020206..3b708472aa 100644 --- a/cores/esp8266/core_esp8266_waveform.c +++ b/cores/esp8266/core_esp8266_waveform.c @@ -38,6 +38,7 @@ */ #include +#include "ets_sys.h" #include "core_esp8266_waveform.h" // Need speed, not size, here @@ -130,15 +131,15 @@ static uint32_t lastCycleCount = 0; // Last ESP cycle counter on running the int static void initTimer() { timer1_disable(); - timer1_isr_init(); - timer1_attachInterrupt(timer1Interrupt); + ETS_FRC_TIMER1_INTR_ATTACH(NULL, NULL); + ETS_FRC_TIMER1_NMI_INTR_ATTACH(timer1Interrupt); lastCycleCount = GetCycleCount(); timer1_enable(TIM_DIV1, TIM_EDGE, TIM_SINGLE); timerRunning = true; } static void ICACHE_RAM_ATTR deinitTimer() { - timer1_attachInterrupt(NULL); + ETS_FRC_TIMER1_NMI_INTR_ATTACH(NULL); timer1_disable(); timer1_isr_init(); timerRunning = false; From 99af25c8e01b84ec2a1cd89baf8bf9f9da7a0cfd Mon Sep 17 00:00:00 2001 From: "Earle F. Philhower, III" Date: Fri, 4 Jan 2019 11:28:48 -0800 Subject: [PATCH 2/7] Move to a lockless waveform generator Make the waveform generator lockless by doing all dangerous structure updates in the interrupt handler. start/stopWaveform set a flag and cause an interrupt. They wait for the interrupt to complete and clear those flags before returning. Also rework the Waveform[] array to be lockless, results in ~48b additonal heap. About 40b of IRAM additional code generated vs. non-NMI. Tweak parameters to make more accurate timing at 80 and 160 MHz. --- cores/esp8266/core_esp8266_waveform.c | 259 ++++++++++++-------------- 1 file changed, 114 insertions(+), 145 deletions(-) diff --git a/cores/esp8266/core_esp8266_waveform.c b/cores/esp8266/core_esp8266_waveform.c index 3b708472aa..c54615f1bd 100644 --- a/cores/esp8266/core_esp8266_waveform.c +++ b/cores/esp8266/core_esp8266_waveform.c @@ -41,82 +41,41 @@ #include "ets_sys.h" #include "core_esp8266_waveform.h" -// Need speed, not size, here -#pragma GCC optimize ("O2") - // Maximum delay between IRQs #define MAXIRQUS (10000) // If the cycles from now to an event are below this value, perform it anyway since IRQs take longer than this #define CYCLES_FLUFF (100) -// Macro to get count of predefined array elements -#define countof(a) ((size_t)(sizeof(a)/sizeof(a[0]))) - -// Set/clear *any* GPIO -#define SetGPIOPin(a) do { if (a < 16) { GPOS |= (1<high to keep smooth waveform - unsigned enabled : 1; // Is this GPIO generating a waveform? - unsigned nextTimeLowCycles : 31; // Copy over high->low to keep smooth waveform + uint32_t nextServiceCycle; // ESP cycle timer when a transition required + uint32_t timeLeftCycles; // For time-limited waveform, how many ESP cycles left + uint32_t nextTimeHighCycles; // Copy over low->high to keep smooth waveform + uint32_t nextTimeLowCycles; // Copy over high->low to keep smooth waveform } Waveform; // These can be accessed in interrupts, so ensure to bracket access with SEI/CLI -static Waveform waveform[] = { - {0, 0, 1<<0, 0, 0, 0, 0, 0}, // GPIO0 - {0, 0, 1<<1, 0, 0, 0, 0, 0}, // GPIO1 - {0, 0, 1<<2, 0, 0, 0, 0, 0}, - {0, 0, 1<<3, 0, 0, 0, 0, 0}, - {0, 0, 1<<4, 0, 0, 0, 0, 0}, - {0, 0, 1<<5, 0, 0, 0, 0, 0}, - // GPIOS 6-8 not allowed, used for flash - // GPIO 9 and 10 only allowed in 2-bit flash mode -#if !isFlashInterfacePin(9) - {0, 0, 1<<9, 0, 0, 0, 0, 0}, - {0, 0, 1<<10, 0, 0, 0, 0, 0}, -#endif - // GPIO 11 not allowed, used for flash - {0, 0, 1<<12, 0, 0, 0, 0, 0}, - {0, 0, 1<<13, 0, 0, 0, 0, 0}, - {0, 0, 1<<14, 0, 0, 0, 0, 0}, - {0, 0, 1<<15, 0, 0, 0, 0, 0}, - {0, 0, 0, 1, 0, 0, 0, 0} // GPIO16 -}; +static Waveform waveform[17]; // State of all possible pins +static volatile uint32_t waveformState = 0; // Is the pin high or low +static volatile uint32_t waveformEnabled = 0; // Is it actively running -static uint32_t (*timer1CB)() = NULL;; +// Enable lock-free by only allowing updates to waveformState and waveformEnabled from IRQ service routine +static volatile uint32_t waveformToEnable = 0; // Message from startWaveform to IRQ to actuall begin a wvf +static volatile uint32_t waveformToDisable = 0; // Message from startWaveform to IRQ to actuall begin a wvf +static uint32_t (*timer1CB)() = NULL; -// Helper functions -static inline ICACHE_RAM_ATTR uint32_t MicrosecondsToCycles(uint32_t microseconds) { - return clockCyclesPerMicrosecond() * microseconds; -} -static inline ICACHE_RAM_ATTR uint32_t min_u32(uint32_t a, uint32_t b) { - if (a < b) { - return a; - } - return b; -} - -static inline ICACHE_RAM_ATTR void ReloadTimer(uint32_t a) { - // Below a threshold you actually miss the edge IRQ, so ensure enough time - if (a > 32) { - timer1_write(a); - } else { - timer1_write(32); - } -} +// Non-speed critical bits +#pragma GCC optimize ("Os") static inline ICACHE_RAM_ATTR uint32_t GetCycleCount() { uint32_t ccount; @@ -126,7 +85,7 @@ static inline ICACHE_RAM_ATTR uint32_t GetCycleCount() { // Interrupt on/off control static ICACHE_RAM_ATTR void timer1Interrupt(); -static uint8_t timerRunning = false; +static bool timerRunning = false; static uint32_t lastCycleCount = 0; // Last ESP cycle counter on running the interrupt routine static void initTimer() { @@ -150,122 +109,129 @@ void setTimer1Callback(uint32_t (*fn)()) { timer1CB = fn; if (!timerRunning && fn) { initTimer(); - } else if (timerRunning && !fn) { - int cnt = 0; - for (size_t i = 0; i < countof(waveform); i++) { - cnt += waveform[i].enabled ? 1 : 0; - } - if (!cnt) { - deinitTimer(); - } + timer1_write(MicrosecondsToCycles(1)); // Cause an interrupt post-haste + } else if (timerRunning && !fn && !waveformEnabled) { + deinitTimer(); } - ReloadTimer(MicrosecondsToCycles(1)); // Cause an interrupt post-haste } // Start up a waveform on a pin, or change the current one. Will change to the new // waveform smoothly on next low->high transition. For immediate change, stopWaveform() // first, then it will immediately begin. int startWaveform(uint8_t pin, uint32_t timeHighUS, uint32_t timeLowUS, uint32_t runTimeUS) { - Waveform *wave = NULL; - for (size_t i = 0; i < countof(waveform); i++) { - if (((pin == 16) && waveform[i].gpio16Mask==1) || ((pin != 16) && (waveform[i].gpioMask == 1< 16) || isFlashInterfacePin(pin)) { return false; } - - // To safely update the packed bitfields we need to stop interrupts while setting them as we could - // get an IRQ in the middle of a multi-instruction mask-and-set required to change them which would - // then cause an IRQ update of these values (.enabled only, for now) to be lost. - ets_intr_lock(); - - wave->nextTimeHighCycles = MicrosecondsToCycles(timeHighUS) - 70; // Take out some time for IRQ codepath - wave->nextTimeLowCycles = MicrosecondsToCycles(timeLowUS) - 70; // Take out some time for IRQ codepath + Waveform *wave = &waveform[pin]; +#if F_CPU == 160000000 + wave->nextTimeHighCycles = MicrosecondsToCycles(timeHighUS) - 140; // Take out some time for IRQ codepath + wave->nextTimeLowCycles = MicrosecondsToCycles(timeLowUS) - 140; // Take out some time for IRQ codepath +#else + wave->nextTimeHighCycles = MicrosecondsToCycles(timeHighUS) > 280 ? MicrosecondsToCycles(timeHighUS) - 280 : MicrosecondsToCycles(timeHighUS); // Take out some time for IRQ codepath + wave->nextTimeLowCycles = MicrosecondsToCycles(timeLowUS) > 280 ? MicrosecondsToCycles(timeLowUS)- 280 : MicrosecondsToCycles(timeLowUS); // Take out some time for IRQ codepath +#endif wave->timeLeftCycles = MicrosecondsToCycles(runTimeUS); - if (!wave->enabled) { - wave->state = 0; + uint32_t mask = 1<nextServiceCycle = GetCycleCount() + MicrosecondsToCycles(1); - wave->enabled = 1; + waveformToEnable |= mask; if (!timerRunning) { initTimer(); } - ReloadTimer(MicrosecondsToCycles(1)); // Cause an interrupt post-haste + timer1_write(MicrosecondsToCycles(1)); // Cause an interrupt post-haste + while (waveformToEnable) { + delay(1); // Wait for waveform to update + } } - // Re-enable interrupts here since we're done with the update - ets_intr_unlock(); - return true; } +// Speed critical bits +#pragma GCC optimize ("O2") +// Normally would not want two copies like this, but due to different +// optimization levels the inline attribute gets lost if we try the +// other version. + +static inline ICACHE_RAM_ATTR uint32_t GetCycleCountIRQ() { + uint32_t ccount; + __asm__ __volatile__("esync; rsr %0,ccount":"=a"(ccount)); + return ccount; +} + +static inline ICACHE_RAM_ATTR uint32_t min_u32(uint32_t a, uint32_t b) { + if (a < b) { + return a; + } + return b; +} + // Stops a waveform on a pin int ICACHE_RAM_ATTR stopWaveform(uint8_t pin) { // Can't possibly need to stop anything if there is no timer active if (!timerRunning) { return false; } - - for (size_t i = 0; i < countof(waveform); i++) { - if (!waveform[i].enabled) { - continue; // Skip fast to next one, can't need to stop this one since it's not running - } - if (((pin == 16) && waveform[i].gpio16Mask) || ((pin != 16) && (waveform[i].gpioMask == 1<0 - // We're also doing that, so even if an IRQ occurred it would still stay as 0. - waveform[i].enabled = 0; - int cnt = timer1CB ? 1 : 0; - for (size_t i = 0; (cnt == 0) && (i < countof(waveform)); i++) { - cnt += waveform[i].enabled ? 1 : 0; - } - if (!cnt) { - deinitTimer(); - } - return true; - } + // If user sends in a pin >16 but <32, this will always point to a 0 bit + // If they send >=32, then the shift will result in 0 and it will also return false + uint32_t mask = 1<enabled) { + if (!(waveformEnabled & mask)) { continue; } + Waveform *wave = &waveform[i]; + uint32_t now; + // Check for toggles - now = GetCycleCount(); + now = GetCycleCountIRQ(); int32_t cyclesToGo = wave->nextServiceCycle - now; if (cyclesToGo < 0) { - wave->state = !wave->state; - if (wave->state) { - SetGPIO(wave->gpioMask); - if (wave->gpio16Mask) { - GP16O |= wave->gpio16Mask; // GPIO16 write slow as it's RMW - } + waveformState ^= mask; + if (waveformState & mask) { + if (i==16) GP16O |= 1; // GPIO16 write slow as it's RMW + else SetGPIO(mask); + wave->nextServiceCycle = now + wave->nextTimeHighCycles; nextEventCycles = min_u32(nextEventCycles, wave->nextTimeHighCycles); } else { - ClearGPIO(wave->gpioMask); - if (wave->gpio16Mask) { - GP16O &= ~wave->gpio16Mask; - } + if (i==16) GP16O &= ~1; // GPIO16 write slow as it's RMW + else ClearGPIO(mask); + wave->nextServiceCycle = now + wave->nextTimeLowCycles; nextEventCycles = min_u32(nextEventCycles, wave->nextTimeLowCycles); } @@ -276,20 +242,21 @@ static ICACHE_RAM_ATTR void timer1Interrupt() { } } while (--cnt && (nextEventCycles < MicrosecondsToCycles(4))); - uint32_t curCycleCount = GetCycleCount(); + uint32_t curCycleCount = GetCycleCountIRQ(); uint32_t deltaCycles = curCycleCount - lastCycleCount; lastCycleCount = curCycleCount; // Check for timed-out waveforms out of the high-frequency toggle loop - for (size_t i = 0; i < countof(waveform); i++) { + for (size_t i = 0; i <= 16; i++) { Waveform *wave = &waveform[i]; - if (wave->enabled && wave->timeLeftCycles) { + uint32_t mask = 1<timeLeftCycles) { // Check for unsigned underflow with new > old if (deltaCycles >= wave->timeLeftCycles) { // Done, remove! - wave->enabled = false; - ClearGPIO(wave->gpioMask); - GP16O &= ~wave->gpio16Mask; + waveformEnabled &= ~mask; + if (i==16) GP16O &= ~1; + else ClearGPIO(mask); } else { uint32_t newTimeLeftCycles = wave->timeLeftCycles - deltaCycles; wave->timeLeftCycles = newTimeLeftCycles; @@ -301,20 +268,22 @@ static ICACHE_RAM_ATTR void timer1Interrupt() { nextEventCycles = min_u32(nextEventCycles, timer1CB()); } - #if F_CPU == 160000000 - if (nextEventCycles <= 5 * MicrosecondsToCycles(1)) { - nextEventCycles = MicrosecondsToCycles(1) / 2; +#if F_CPU == 160000000 + if (nextEventCycles < MicrosecondsToCycles(5)) { + nextEventCycles = MicrosecondsToCycles(1); } else { - nextEventCycles -= 5 * MicrosecondsToCycles(1); + nextEventCycles -= MicrosecondsToCycles(4) + MicrosecondsToCycles(1)/2; } nextEventCycles = nextEventCycles >> 1; - #else - if (nextEventCycles <= 6 * MicrosecondsToCycles(1)) { - nextEventCycles = MicrosecondsToCycles(1) / 2; +#else + if (nextEventCycles < MicrosecondsToCycles(8)) { + nextEventCycles = MicrosecondsToCycles(2); } else { - nextEventCycles -= 6 * MicrosecondsToCycles(1); + nextEventCycles -= MicrosecondsToCycles(6); } - #endif +#endif - ReloadTimer(nextEventCycles); + // Do it here instead of global function to save time and because we know it's edge-IRQ + T1L = nextEventCycles; // Already know we're in range by MAXIRQUS + TEIE |= TEIE1; //edge int enable } From 78fba4cd798b4783b1d4d7708aa381a9bf618e8c Mon Sep 17 00:00:00 2001 From: "Earle F. Philhower, III" Date: Sat, 5 Jan 2019 10:34:39 -0800 Subject: [PATCH 3/7] Optimize IRAM and CPU usage in IRQ Try and minimize the IRAM needed to run the IRQ while keeping performance at or better than before. --- cores/esp8266/core_esp8266_waveform.c | 53 +++++++++++---------------- 1 file changed, 22 insertions(+), 31 deletions(-) diff --git a/cores/esp8266/core_esp8266_waveform.c b/cores/esp8266/core_esp8266_waveform.c index c54615f1bd..9ccc567100 100644 --- a/cores/esp8266/core_esp8266_waveform.c +++ b/cores/esp8266/core_esp8266_waveform.c @@ -57,7 +57,7 @@ // Waveform generator can create tones, PWM, and servos typedef struct { uint32_t nextServiceCycle; // ESP cycle timer when a transition required - uint32_t timeLeftCycles; // For time-limited waveform, how many ESP cycles left + uint32_t expiryCycle; // For time-limited waveform, the cycle when this waveform must stop uint32_t nextTimeHighCycles; // Copy over low->high to keep smooth waveform uint32_t nextTimeLowCycles; // Copy over high->low to keep smooth waveform } Waveform; @@ -86,13 +86,11 @@ static inline ICACHE_RAM_ATTR uint32_t GetCycleCount() { // Interrupt on/off control static ICACHE_RAM_ATTR void timer1Interrupt(); static bool timerRunning = false; -static uint32_t lastCycleCount = 0; // Last ESP cycle counter on running the interrupt routine static void initTimer() { timer1_disable(); ETS_FRC_TIMER1_INTR_ATTACH(NULL, NULL); ETS_FRC_TIMER1_NMI_INTR_ATTACH(timer1Interrupt); - lastCycleCount = GetCycleCount(); timer1_enable(TIM_DIV1, TIM_EDGE, TIM_SINGLE); timerRunning = true; } @@ -130,7 +128,11 @@ int startWaveform(uint8_t pin, uint32_t timeHighUS, uint32_t timeLowUS, uint32_t wave->nextTimeHighCycles = MicrosecondsToCycles(timeHighUS) > 280 ? MicrosecondsToCycles(timeHighUS) - 280 : MicrosecondsToCycles(timeHighUS); // Take out some time for IRQ codepath wave->nextTimeLowCycles = MicrosecondsToCycles(timeLowUS) > 280 ? MicrosecondsToCycles(timeLowUS)- 280 : MicrosecondsToCycles(timeLowUS); // Take out some time for IRQ codepath #endif - wave->timeLeftCycles = MicrosecondsToCycles(runTimeUS); + wave->expiryCycle = runTimeUS ? GetCycleCount() + MicrosecondsToCycles(runTimeUS) : 0; + if (runTimeUS && !wave->expiryCycle) { + wave->expiryCycle = 1; // expiryCycle==0 means no timeout, so avoid setting it + } + uint32_t mask = 1<expiryCycle) { + int32_t expiryToGo = wave->expiryCycle - now; + if (expiryToGo < 0) { + // Done, remove! + waveformEnabled &= ~mask; + if (i==16) GP16O &= ~1; + else ClearGPIO(mask); + continue; + } + } // Check for toggles - now = GetCycleCountIRQ(); int32_t cyclesToGo = wave->nextServiceCycle - now; if (cyclesToGo < 0) { waveformState ^= mask; @@ -242,28 +255,6 @@ static ICACHE_RAM_ATTR void timer1Interrupt() { } } while (--cnt && (nextEventCycles < MicrosecondsToCycles(4))); - uint32_t curCycleCount = GetCycleCountIRQ(); - uint32_t deltaCycles = curCycleCount - lastCycleCount; - lastCycleCount = curCycleCount; - - // Check for timed-out waveforms out of the high-frequency toggle loop - for (size_t i = 0; i <= 16; i++) { - Waveform *wave = &waveform[i]; - uint32_t mask = 1<timeLeftCycles) { - // Check for unsigned underflow with new > old - if (deltaCycles >= wave->timeLeftCycles) { - // Done, remove! - waveformEnabled &= ~mask; - if (i==16) GP16O &= ~1; - else ClearGPIO(mask); - } else { - uint32_t newTimeLeftCycles = wave->timeLeftCycles - deltaCycles; - wave->timeLeftCycles = newTimeLeftCycles; - } - } - } - if (timer1CB) { nextEventCycles = min_u32(nextEventCycles, timer1CB()); } @@ -284,6 +275,6 @@ static ICACHE_RAM_ATTR void timer1Interrupt() { #endif // Do it here instead of global function to save time and because we know it's edge-IRQ - T1L = nextEventCycles; // Already know we're in range by MAXIRQUS TEIE |= TEIE1; //edge int enable + T1L = nextEventCycles; // Already know we're in range by MAXIRQUS } From 923f2f41033ebecfdfc64a7d4caa4a1f6d4b24d4 Mon Sep 17 00:00:00 2001 From: "Earle F. Philhower, III" Date: Sat, 5 Jan 2019 11:18:09 -0800 Subject: [PATCH 4/7] Fix boolean/bitmask typo in starting waveform --- cores/esp8266/core_esp8266_waveform.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cores/esp8266/core_esp8266_waveform.c b/cores/esp8266/core_esp8266_waveform.c index 9ccc567100..ae09a8b1a0 100644 --- a/cores/esp8266/core_esp8266_waveform.c +++ b/cores/esp8266/core_esp8266_waveform.c @@ -134,7 +134,7 @@ int startWaveform(uint8_t pin, uint32_t timeHighUS, uint32_t timeLowUS, uint32_t } uint32_t mask = 1<nextServiceCycle = GetCycleCount() + MicrosecondsToCycles(1); waveformToEnable |= mask; @@ -143,7 +143,7 @@ int startWaveform(uint8_t pin, uint32_t timeHighUS, uint32_t timeLowUS, uint32_t } timer1_write(MicrosecondsToCycles(1)); // Cause an interrupt post-haste while (waveformToEnable) { - delay(1); // Wait for waveform to update + delay(0); // Wait for waveform to update } } From 409fc9121cca1d9afd6933e530f969826620afe8 Mon Sep 17 00:00:00 2001 From: "Earle F. Philhower, III" Date: Sun, 6 Jan 2019 12:06:46 -0800 Subject: [PATCH 5/7] Add'l testing and tweaking at 160 and 80mhz --- cores/esp8266/core_esp8266_waveform.c | 57 +++++++++++++-------------- 1 file changed, 27 insertions(+), 30 deletions(-) diff --git a/cores/esp8266/core_esp8266_waveform.c b/cores/esp8266/core_esp8266_waveform.c index ae09a8b1a0..87efaf097a 100644 --- a/cores/esp8266/core_esp8266_waveform.c +++ b/cores/esp8266/core_esp8266_waveform.c @@ -44,9 +44,6 @@ // Maximum delay between IRQs #define MAXIRQUS (10000) -// If the cycles from now to an event are below this value, perform it anyway since IRQs take longer than this -#define CYCLES_FLUFF (100) - // Set/clear GPIO 0-15 by bitmask #define SetGPIO(a) do { GPOS = a; } while (0) #define ClearGPIO(a) do { GPOC = a; } while (0) @@ -121,13 +118,17 @@ int startWaveform(uint8_t pin, uint32_t timeHighUS, uint32_t timeLowUS, uint32_t return false; } Waveform *wave = &waveform[pin]; -#if F_CPU == 160000000 - wave->nextTimeHighCycles = MicrosecondsToCycles(timeHighUS) - 140; // Take out some time for IRQ codepath - wave->nextTimeLowCycles = MicrosecondsToCycles(timeLowUS) - 140; // Take out some time for IRQ codepath -#else - wave->nextTimeHighCycles = MicrosecondsToCycles(timeHighUS) > 280 ? MicrosecondsToCycles(timeHighUS) - 280 : MicrosecondsToCycles(timeHighUS); // Take out some time for IRQ codepath - wave->nextTimeLowCycles = MicrosecondsToCycles(timeLowUS) > 280 ? MicrosecondsToCycles(timeLowUS)- 280 : MicrosecondsToCycles(timeLowUS); // Take out some time for IRQ codepath + // Adjust to shave off some of the IRQ time, approximately + uint32_t delta = 0; +#if F_CPU == 80000000 + if (timeHighUS > 2) { + delta = MicrosecondsToCycles(1) + MicrosecondsToCycles(2)/3; + } +#else // 160000000 + delta = MicrosecondsToCycles(1) >> 1; // 0.5 us off each edge #endif + wave->nextTimeHighCycles = MicrosecondsToCycles(timeHighUS) - delta; + wave->nextTimeLowCycles = MicrosecondsToCycles(timeLowUS) - delta; wave->expiryCycle = runTimeUS ? GetCycleCount() + MicrosecondsToCycles(runTimeUS) : 0; if (runTimeUS && !wave->expiryCycle) { wave->expiryCycle = 1; // expiryCycle==0 means no timeout, so avoid setting it @@ -192,14 +193,17 @@ int ICACHE_RAM_ATTR stopWaveform(uint8_t pin) { return true; } -static ICACHE_RAM_ATTR void timer1Interrupt() { - uint32_t nextEventCycles; -#if F_CPU == 160000000 - int cnt = 20; +#if F_CPU == 80000000 + #define MINCYCLE MicrosecondsToCycles(6) + #define DELTAIRQ MicrosecondsToCycles(5) #else - int cnt = 10; + #define MINCYCLE MicrosecondsToCycles(4) + #define DELTAIRQ (MicrosecondsToCycles(2) + MicrosecondsToCycles(3)/4) #endif +static ICACHE_RAM_ATTR void timer1Interrupt() { + uint32_t nextEventCycles; + // Handle enable/disable requests from main app. waveformEnabled = (waveformEnabled & ~waveformToDisable) | waveformToEnable; // Set the requested waveforms on/off waveformState &= ~waveformToEnable; // And clear the state of any just started @@ -253,28 +257,21 @@ static ICACHE_RAM_ATTR void timer1Interrupt() { nextEventCycles = min_u32(nextEventCycles, deltaCycles); } } - } while (--cnt && (nextEventCycles < MicrosecondsToCycles(4))); + } while (nextEventCycles < MINCYCLE); if (timer1CB) { nextEventCycles = min_u32(nextEventCycles, timer1CB()); + if (nextEventCycles < MINCYCLE) { + nextEventCycles = MINCYCLE; + } } - -#if F_CPU == 160000000 - if (nextEventCycles < MicrosecondsToCycles(5)) { - nextEventCycles = MicrosecondsToCycles(1); - } else { - nextEventCycles -= MicrosecondsToCycles(4) + MicrosecondsToCycles(1)/2; - } - nextEventCycles = nextEventCycles >> 1; -#else - if (nextEventCycles < MicrosecondsToCycles(8)) { - nextEventCycles = MicrosecondsToCycles(2); - } else { - nextEventCycles -= MicrosecondsToCycles(6); - } -#endif + nextEventCycles -= DELTAIRQ; // Do it here instead of global function to save time and because we know it's edge-IRQ TEIE |= TEIE1; //edge int enable +#if F_CPU == 160000000 + T1L = nextEventCycles >> 1; // Already know we're in range by MAXIRQUS +#else T1L = nextEventCycles; // Already know we're in range by MAXIRQUS +#endif } From 1d2c939417faac4f9c2a2adb32c43e4597da7170 Mon Sep 17 00:00:00 2001 From: "Earle F. Philhower, III" Date: Sun, 6 Jan 2019 16:46:21 -0800 Subject: [PATCH 6/7] Avoid WDT errors, optimize pin scans Calculate first and last pins to scan for PWM, significantly increasing accuracy for pulses under 10us at 80MHz. Now if you are using a single PWM channel at 80MHz you can generate a 1.125us pulse (down from ~4us). Rework the IRQ logic to avoid potential WDT errors. When at 80MHz it appears that interrupts occuring faster than 10us apart on the timer cause WDT errors. Avoid it by increasing the minimum delay between IRQs on the timer accordingly. Torture test below passed @ 80 and 160Mhz: void loop() { int pin = rand() % 8; int val = rand() % 1000; switch(pin) { case 0: pin = D1; break; case 1: pin = D2; break; case 2: pin = D3; break; case 3: pin = D4; break; case 4: pin = D5; break; case 5: pin = D6; break; case 6: pin = D7; break; case 7: pin = D8; break; } analogWrite(pin, val); delay(30); } --- cores/esp8266/core_esp8266_waveform.c | 70 +++++++++++++++------------ 1 file changed, 39 insertions(+), 31 deletions(-) diff --git a/cores/esp8266/core_esp8266_waveform.c b/cores/esp8266/core_esp8266_waveform.c index 87efaf097a..3b7f663197 100644 --- a/cores/esp8266/core_esp8266_waveform.c +++ b/cores/esp8266/core_esp8266_waveform.c @@ -119,16 +119,8 @@ int startWaveform(uint8_t pin, uint32_t timeHighUS, uint32_t timeLowUS, uint32_t } Waveform *wave = &waveform[pin]; // Adjust to shave off some of the IRQ time, approximately - uint32_t delta = 0; -#if F_CPU == 80000000 - if (timeHighUS > 2) { - delta = MicrosecondsToCycles(1) + MicrosecondsToCycles(2)/3; - } -#else // 160000000 - delta = MicrosecondsToCycles(1) >> 1; // 0.5 us off each edge -#endif - wave->nextTimeHighCycles = MicrosecondsToCycles(timeHighUS) - delta; - wave->nextTimeLowCycles = MicrosecondsToCycles(timeLowUS) - delta; + wave->nextTimeHighCycles = MicrosecondsToCycles(timeHighUS); + wave->nextTimeLowCycles = MicrosecondsToCycles(timeLowUS); wave->expiryCycle = runTimeUS ? GetCycleCount() + MicrosecondsToCycles(runTimeUS) : 0; if (runTimeUS && !wave->expiryCycle) { wave->expiryCycle = 1; // expiryCycle==0 means no timeout, so avoid setting it @@ -142,7 +134,7 @@ int startWaveform(uint8_t pin, uint32_t timeHighUS, uint32_t timeLowUS, uint32_t if (!timerRunning) { initTimer(); } - timer1_write(MicrosecondsToCycles(1)); // Cause an interrupt post-haste + timer1_write(MicrosecondsToCycles(10)); // Cause an interrupt post-haste while (waveformToEnable) { delay(0); // Wait for waveform to update } @@ -159,7 +151,7 @@ int startWaveform(uint8_t pin, uint32_t timeHighUS, uint32_t timeLowUS, uint32_t static inline ICACHE_RAM_ATTR uint32_t GetCycleCountIRQ() { uint32_t ccount; - __asm__ __volatile__("esync; rsr %0,ccount":"=a"(ccount)); + __asm__ __volatile__("rsr %0,ccount":"=a"(ccount)); return ccount; } @@ -183,7 +175,10 @@ int ICACHE_RAM_ATTR stopWaveform(uint8_t pin) { return false; //It's not running, nothing to do here } waveformToDisable |= mask; - timer1_write(MicrosecondsToCycles(1)); + // Ensure tiomely service.... + if (T1L > MicrosecondsToCycles(10)) { + timer1_write(MicrosecondsToCycles(10)); + } while (waveformToDisable) { delay(1); // Wait for IRQ to update } @@ -194,25 +189,32 @@ int ICACHE_RAM_ATTR stopWaveform(uint8_t pin) { } #if F_CPU == 80000000 - #define MINCYCLE MicrosecondsToCycles(6) - #define DELTAIRQ MicrosecondsToCycles(5) + #define DELTAIRQ (MicrosecondsToCycles(3)) #else - #define MINCYCLE MicrosecondsToCycles(4) - #define DELTAIRQ (MicrosecondsToCycles(2) + MicrosecondsToCycles(3)/4) + #define DELTAIRQ (MicrosecondsToCycles(2)) #endif -static ICACHE_RAM_ATTR void timer1Interrupt() { - uint32_t nextEventCycles; +static int startPin = 0; +static int endPin = 0; - // Handle enable/disable requests from main app. - waveformEnabled = (waveformEnabled & ~waveformToDisable) | waveformToEnable; // Set the requested waveforms on/off - waveformState &= ~waveformToEnable; // And clear the state of any just started - waveformToEnable = 0; - waveformToDisable = 0; +static ICACHE_RAM_ATTR void timer1Interrupt() { + uint32_t nextEventCycles = MicrosecondsToCycles(MAXIRQUS); + uint32_t timeoutCycle = GetCycleCountIRQ() + MicrosecondsToCycles(14); + + if (waveformToEnable || waveformToDisable) { + // Handle enable/disable requests from main app. + waveformEnabled = (waveformEnabled & ~waveformToDisable) | waveformToEnable; // Set the requested waveforms on/off + waveformState &= ~waveformToEnable; // And clear the state of any just started + waveformToEnable = 0; + waveformToDisable = 0; + startPin = __builtin_ffs(waveformEnabled) - 1; + endPin = 32 - __builtin_clz(waveformEnabled); + } - do { + bool done = false; + if (waveformEnabled) do { nextEventCycles = MicrosecondsToCycles(MAXIRQUS); - for (size_t i = 0; i <= 16; i++) { + for (int i = startPin; i <= endPin; i++) { uint32_t mask = 1<> 1; // Already know we're in range by MAXIRQUS #else T1L = nextEventCycles; // Already know we're in range by MAXIRQUS #endif + TEIE |= TEIE1; //edge int enable } From 91bec0bd7598dcc94dc0e33fc18848423677f003 Mon Sep 17 00:00:00 2001 From: "Earle F. Philhower, III" Date: Mon, 7 Jan 2019 20:42:07 -0800 Subject: [PATCH 7/7] Clean up format/comment, remove delay() in stop stopWaveform may be called from an interrupt (it's called by digitalWrite) so we can't call delay(). Make it a busy wait for the IRQ to clear the waveform. Only set a new timeout of 10us when starting a new waveform when there is no other event coming sooner than that. Update formatting and comments per @devyte's requests. Replace MicrosecondsToCycles() with standard Arduino call. --- cores/esp8266/core_esp8266_waveform.c | 172 ++++++++++++++------------ 1 file changed, 96 insertions(+), 76 deletions(-) diff --git a/cores/esp8266/core_esp8266_waveform.c b/cores/esp8266/core_esp8266_waveform.c index 3b7f663197..e443102db7 100644 --- a/cores/esp8266/core_esp8266_waveform.c +++ b/cores/esp8266/core_esp8266_waveform.c @@ -48,9 +48,6 @@ #define SetGPIO(a) do { GPOS = a; } while (0) #define ClearGPIO(a) do { GPOC = a; } while (0) -// Convert from us to ESP8266 clocks -#define MicrosecondsToCycles(m) (clockCyclesPerMicrosecond() * (m)) - // Waveform generator can create tones, PWM, and servos typedef struct { uint32_t nextServiceCycle; // ESP cycle timer when a transition required @@ -59,14 +56,13 @@ typedef struct { uint32_t nextTimeLowCycles; // Copy over high->low to keep smooth waveform } Waveform; -// These can be accessed in interrupts, so ensure to bracket access with SEI/CLI static Waveform waveform[17]; // State of all possible pins -static volatile uint32_t waveformState = 0; // Is the pin high or low -static volatile uint32_t waveformEnabled = 0; // Is it actively running +static volatile uint32_t waveformState = 0; // Is the pin high or low, updated in NMI so no access outside the NMI code +static volatile uint32_t waveformEnabled = 0; // Is it actively running, updated in NMI so no access outside the NMI code // Enable lock-free by only allowing updates to waveformState and waveformEnabled from IRQ service routine -static volatile uint32_t waveformToEnable = 0; // Message from startWaveform to IRQ to actuall begin a wvf -static volatile uint32_t waveformToDisable = 0; // Message from startWaveform to IRQ to actuall begin a wvf +static volatile uint32_t waveformToEnable = 0; // Message to the NMI handler to start a waveform on a inactive pin +static volatile uint32_t waveformToDisable = 0; // Message to the NMI handler to disable a pin from waveform generation static uint32_t (*timer1CB)() = NULL; @@ -104,7 +100,7 @@ void setTimer1Callback(uint32_t (*fn)()) { timer1CB = fn; if (!timerRunning && fn) { initTimer(); - timer1_write(MicrosecondsToCycles(1)); // Cause an interrupt post-haste + timer1_write(microsecondsToClockCycles(1)); // Cause an interrupt post-haste } else if (timerRunning && !fn && !waveformEnabled) { deinitTimer(); } @@ -119,9 +115,9 @@ int startWaveform(uint8_t pin, uint32_t timeHighUS, uint32_t timeLowUS, uint32_t } Waveform *wave = &waveform[pin]; // Adjust to shave off some of the IRQ time, approximately - wave->nextTimeHighCycles = MicrosecondsToCycles(timeHighUS); - wave->nextTimeLowCycles = MicrosecondsToCycles(timeLowUS); - wave->expiryCycle = runTimeUS ? GetCycleCount() + MicrosecondsToCycles(runTimeUS) : 0; + wave->nextTimeHighCycles = microsecondsToClockCycles(timeHighUS); + wave->nextTimeLowCycles = microsecondsToClockCycles(timeLowUS); + wave->expiryCycle = runTimeUS ? GetCycleCount() + microsecondsToClockCycles(runTimeUS) : 0; if (runTimeUS && !wave->expiryCycle) { wave->expiryCycle = 1; // expiryCycle==0 means no timeout, so avoid setting it } @@ -129,12 +125,17 @@ int startWaveform(uint8_t pin, uint32_t timeHighUS, uint32_t timeLowUS, uint32_t uint32_t mask = 1<nextServiceCycle = GetCycleCount() + MicrosecondsToCycles(1); + wave->nextServiceCycle = GetCycleCount() + microsecondsToClockCycles(1); waveformToEnable |= mask; if (!timerRunning) { initTimer(); + timer1_write(microsecondsToClockCycles(10)); + } else { + // Ensure timely service.... + if (T1L > microsecondsToClockCycles(10)) { + timer1_write(microsecondsToClockCycles(10)); + } } - timer1_write(MicrosecondsToCycles(10)); // Cause an interrupt post-haste while (waveformToEnable) { delay(0); // Wait for waveform to update } @@ -172,15 +173,15 @@ int ICACHE_RAM_ATTR stopWaveform(uint8_t pin) { // If they send >=32, then the shift will result in 0 and it will also return false uint32_t mask = 1< MicrosecondsToCycles(10)) { - timer1_write(MicrosecondsToCycles(10)); + // Ensure timely service.... + if (T1L > microsecondsToClockCycles(10)) { + timer1_write(microsecondsToClockCycles(10)); } while (waveformToDisable) { - delay(1); // Wait for IRQ to update + /* no-op */ // Can't delay() since stopWaveform may be called from an IRQ } if (!waveformEnabled && !timer1CB) { deinitTimer(); @@ -188,18 +189,25 @@ int ICACHE_RAM_ATTR stopWaveform(uint8_t pin) { return true; } +// The SDK and hardware take some time to actually get to our NMI code, so +// decrement the next IRQ's timer value by a bit so we can actually catch the +// real CPU cycle counter we want for the waveforms. #if F_CPU == 80000000 - #define DELTAIRQ (MicrosecondsToCycles(3)) + #define DELTAIRQ (microsecondsToClockCycles(3)) #else - #define DELTAIRQ (MicrosecondsToCycles(2)) + #define DELTAIRQ (microsecondsToClockCycles(2)) #endif -static int startPin = 0; -static int endPin = 0; static ICACHE_RAM_ATTR void timer1Interrupt() { - uint32_t nextEventCycles = MicrosecondsToCycles(MAXIRQUS); - uint32_t timeoutCycle = GetCycleCountIRQ() + MicrosecondsToCycles(14); + // Optimize the NMI inner loop by keeping track of the min and max GPIO that we + // are generating. In the common case (1 PWM) these may be the same pin and + // we can avoid looking at the other pins. + static int startPin = 0; + static int endPin = 0; + + uint32_t nextEventCycles = microsecondsToClockCycles(MAXIRQUS); + uint32_t timeoutCycle = GetCycleCountIRQ() + microsecondsToClockCycles(14); if (waveformToEnable || waveformToDisable) { // Handle enable/disable requests from main app. @@ -207,71 +215,83 @@ static ICACHE_RAM_ATTR void timer1Interrupt() { waveformState &= ~waveformToEnable; // And clear the state of any just started waveformToEnable = 0; waveformToDisable = 0; + // Find the first GPIO being generated by checking GCC's find-first-set (returns 1 + the bit of the first 1 in an int32_t) startPin = __builtin_ffs(waveformEnabled) - 1; + // Find the last bit by subtracting off GCC's count-leading-zeros (no offset in this one) endPin = 32 - __builtin_clz(waveformEnabled); } bool done = false; - if (waveformEnabled) do { - nextEventCycles = MicrosecondsToCycles(MAXIRQUS); - for (int i = startPin; i <= endPin; i++) { - uint32_t mask = 1<expiryCycle) { + int32_t expiryToGo = wave->expiryCycle - now; + if (expiryToGo < 0) { + // Done, remove! + waveformEnabled &= ~mask; + if (i == 16) { + GP16O &= ~1; + } else { + ClearGPIO(mask); + } + continue; + } + } - // Disable any waveforms that are done - if (wave->expiryCycle) { - int32_t expiryToGo = wave->expiryCycle - now; - if (expiryToGo < 0) { - // Done, remove! - waveformEnabled &= ~mask; - if (i==16) GP16O &= ~1; - else ClearGPIO(mask); - continue; + // Check for toggles + int32_t cyclesToGo = wave->nextServiceCycle - now; + if (cyclesToGo < 0) { + waveformState ^= mask; + if (waveformState & mask) { + if (i == 16) { + GP16O |= 1; // GPIO16 write slow as it's RMW + } else { + SetGPIO(mask); + } + wave->nextServiceCycle = now + wave->nextTimeHighCycles; + nextEventCycles = min_u32(nextEventCycles, wave->nextTimeHighCycles); + } else { + if (i == 16) { + GP16O &= ~1; // GPIO16 write slow as it's RMW + } else { + ClearGPIO(mask); + } + wave->nextServiceCycle = now + wave->nextTimeLowCycles; + nextEventCycles = min_u32(nextEventCycles, wave->nextTimeLowCycles); } - } - - // Check for toggles - int32_t cyclesToGo = wave->nextServiceCycle - now; - if (cyclesToGo < 0) { - waveformState ^= mask; - if (waveformState & mask) { - if (i==16) GP16O |= 1; // GPIO16 write slow as it's RMW - else SetGPIO(mask); - - wave->nextServiceCycle = now + wave->nextTimeHighCycles; - nextEventCycles = min_u32(nextEventCycles, wave->nextTimeHighCycles); } else { - if (i==16) GP16O &= ~1; // GPIO16 write slow as it's RMW - else ClearGPIO(mask); - - wave->nextServiceCycle = now + wave->nextTimeLowCycles; - nextEventCycles = min_u32(nextEventCycles, wave->nextTimeLowCycles); + uint32_t deltaCycles = wave->nextServiceCycle - now; + nextEventCycles = min_u32(nextEventCycles, deltaCycles); } - } else { - uint32_t deltaCycles = wave->nextServiceCycle - now; - nextEventCycles = min_u32(nextEventCycles, deltaCycles); } - } - uint32_t now = GetCycleCountIRQ(); - done = false; - int32_t cycleDeltaNextEvent = timeoutCycle - (now + nextEventCycles); - if (cycleDeltaNextEvent < 0) done = true; - int32_t cyclesLeftTimeout = timeoutCycle - now; - if (cyclesLeftTimeout < 0) done = true; - } while (!done); + + // Exit the loop if we've hit the fixed runtime limit or the next event is known to be after that timeout would occur + uint32_t now = GetCycleCountIRQ(); + int32_t cycleDeltaNextEvent = timeoutCycle - (now + nextEventCycles); + int32_t cyclesLeftTimeout = timeoutCycle - now; + done = (cycleDeltaNextEvent < 0) || (cyclesLeftTimeout < 0); + } while (!done); + } // if (waveformEnabled) if (timer1CB) { nextEventCycles = min_u32(nextEventCycles, timer1CB()); } - if (nextEventCycles < MicrosecondsToCycles(10)) { - nextEventCycles = MicrosecondsToCycles(10); + + if (nextEventCycles < microsecondsToClockCycles(10)) { + nextEventCycles = microsecondsToClockCycles(10); } nextEventCycles -= DELTAIRQ; @@ -281,5 +301,5 @@ static ICACHE_RAM_ATTR void timer1Interrupt() { #else T1L = nextEventCycles; // Already know we're in range by MAXIRQUS #endif - TEIE |= TEIE1; //edge int enable + TEIE |= TEIE1; // Edge int enable }