From 7f2c4e956e2f810d1f32984aa9b0f4a1c7ae4916 Mon Sep 17 00:00:00 2001 From: s-hadinger <49731213+s-hadinger@users.noreply.github.com> Date: Sun, 2 Feb 2020 18:24:37 +0100 Subject: [PATCH 1/5] Make sure PWM phases remain constant See #7054, PWM channels phases can change over time causing visible flickering on LED drivers (Tasmota). This fix makes sure the PWM pulses are kept in sync, phases constant whatever the delay of the NMI. --- cores/esp8266/core_esp8266_waveform.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/cores/esp8266/core_esp8266_waveform.cpp b/cores/esp8266/core_esp8266_waveform.cpp index 59fce8695e..06a3733917 100644 --- a/cores/esp8266/core_esp8266_waveform.cpp +++ b/cores/esp8266/core_esp8266_waveform.cpp @@ -256,6 +256,7 @@ static ICACHE_RAM_ATTR void timer1Interrupt() { // Check for toggles int32_t cyclesToGo = wave->nextServiceCycle - now; if (cyclesToGo < 0) { + cyclesToGo = -((-cyclesToGo) % (wave->nextTimeHighCycles + wave->nextTimeLowCycles)); waveformState ^= mask; if (waveformState & mask) { if (i == 16) { @@ -263,16 +264,16 @@ static ICACHE_RAM_ATTR void timer1Interrupt() { } else { SetGPIO(mask); } - wave->nextServiceCycle = now + wave->nextTimeHighCycles; - nextEventCycles = min_u32(nextEventCycles, wave->nextTimeHighCycles); + wave->nextServiceCycle = now + wave->nextTimeHighCycles + cyclesToGo; + nextEventCycles = min_u32(nextEventCycles, min_u32(wave->nextTimeHighCycles + cyclesToGo, 1)); } 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); + wave->nextServiceCycle = now + wave->nextTimeLowCycles + cyclesToGo; + nextEventCycles = min_u32(nextEventCycles, min_u32(wave->nextTimeLowCycles + cyclesToGo, 1)); } } else { uint32_t deltaCycles = wave->nextServiceCycle - now; From 64c6f047aa9afc2c9bbcb11edcb59b6862fef645 Mon Sep 17 00:00:00 2001 From: Hadinger Date: Wed, 26 Feb 2020 21:36:27 +0100 Subject: [PATCH 2/5] Remove the entire waveform overshoot compensation --- cores/esp8266/core_esp8266_waveform.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/cores/esp8266/core_esp8266_waveform.cpp b/cores/esp8266/core_esp8266_waveform.cpp index 06a3733917..81a9c7dfed 100644 --- a/cores/esp8266/core_esp8266_waveform.cpp +++ b/cores/esp8266/core_esp8266_waveform.cpp @@ -256,7 +256,13 @@ static ICACHE_RAM_ATTR void timer1Interrupt() { // Check for toggles int32_t cyclesToGo = wave->nextServiceCycle - now; if (cyclesToGo < 0) { - cyclesToGo = -((-cyclesToGo) % (wave->nextTimeHighCycles + wave->nextTimeLowCycles)); + // See #7057 + // The following is a no-op unless we have overshot by an entire waveform cycle. + // As modulus is an expensive operation, this code is removed for now: + // cyclesToGo = -((-cyclesToGo) % (wave->nextTimeHighCycles + wave->nextTimeLowCycles)); + // + // Alternative version with lower CPU impact: + // while (-cyclesToGo > wave->nextTimeHighCycles + wave->nextTimeLowCycles) { cyclesToGo += wave->nextTimeHighCycles + wave->nextTimeLowCycles)}; waveformState ^= mask; if (waveformState & mask) { if (i == 16) { From 7f821cea6f66f59ff788231233073983f0fe997a Mon Sep 17 00:00:00 2001 From: s-hadinger <49731213+s-hadinger@users.noreply.github.com> Date: Tue, 3 Mar 2020 23:28:36 +0100 Subject: [PATCH 3/5] Avoid thrashing Code changes suggested by @dok-net --- cores/esp8266/core_esp8266_waveform.cpp | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/cores/esp8266/core_esp8266_waveform.cpp b/cores/esp8266/core_esp8266_waveform.cpp index 81a9c7dfed..8c6f1a17cc 100644 --- a/cores/esp8266/core_esp8266_waveform.cpp +++ b/cores/esp8266/core_esp8266_waveform.cpp @@ -165,6 +165,13 @@ static inline ICACHE_RAM_ATTR uint32_t min_u32(uint32_t a, uint32_t b) { return b; } +static inline ICACHE_RAM_ATTR int32_t max_32(int32_t a, int32_t b) { + if (a < b) { + return b; + } + return a; +} + // 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 @@ -270,16 +277,16 @@ static ICACHE_RAM_ATTR void timer1Interrupt() { } else { SetGPIO(mask); } - wave->nextServiceCycle = now + wave->nextTimeHighCycles + cyclesToGo; - nextEventCycles = min_u32(nextEventCycles, min_u32(wave->nextTimeHighCycles + cyclesToGo, 1)); + wave->nextServiceCycle += wave->nextTimeHighCycles; + nextEventCycles = min_u32(nextEventCycles, max_32(wave->nextTimeHighCycles + cyclesToGo, microsecondsToClockCycles(1))); } else { if (i == 16) { GP16O &= ~1; // GPIO16 write slow as it's RMW } else { ClearGPIO(mask); } - wave->nextServiceCycle = now + wave->nextTimeLowCycles + cyclesToGo; - nextEventCycles = min_u32(nextEventCycles, min_u32(wave->nextTimeLowCycles + cyclesToGo, 1)); + wave->nextServiceCycle += wave->nextTimeLowCycles; + nextEventCycles = min_u32(nextEventCycles, max_32(wave->nextTimeLowCycles + cyclesToGo, microsecondsToClockCycles(1))); } } else { uint32_t deltaCycles = wave->nextServiceCycle - now; From 5db5df684e6e5eb750ca0d3b4cc3cd960b4766cf Mon Sep 17 00:00:00 2001 From: Stephan Hadinger Date: Wed, 8 Apr 2020 12:02:50 +0200 Subject: [PATCH 4/5] Better handling of interrupt storm during wifi connection --- cores/esp8266/core_esp8266_waveform.cpp | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/cores/esp8266/core_esp8266_waveform.cpp b/cores/esp8266/core_esp8266_waveform.cpp index 8c6f1a17cc..f4c540c583 100644 --- a/cores/esp8266/core_esp8266_waveform.cpp +++ b/cores/esp8266/core_esp8266_waveform.cpp @@ -269,7 +269,11 @@ static ICACHE_RAM_ATTR void timer1Interrupt() { // cyclesToGo = -((-cyclesToGo) % (wave->nextTimeHighCycles + wave->nextTimeLowCycles)); // // Alternative version with lower CPU impact: - // while (-cyclesToGo > wave->nextTimeHighCycles + wave->nextTimeLowCycles) { cyclesToGo += wave->nextTimeHighCycles + wave->nextTimeLowCycles)}; + // while (-cyclesToGo > wave->nextTimeHighCycles + wave->nextTimeLowCycles) { cyclesToGo += wave->nextTimeHighCycles + wave->nextTimeLowCycles); } + // + // detect interrupt storm, for example during wifi connection. + // if we overshoot the cycle by more than 25%, we forget phase and keep PWM duration + int32_t overshoot = (-cyclesToGo) > ((wave->nextTimeHighCycles + wave->nextTimeLowCycles) >> 2); waveformState ^= mask; if (waveformState & mask) { if (i == 16) { @@ -277,16 +281,26 @@ static ICACHE_RAM_ATTR void timer1Interrupt() { } else { SetGPIO(mask); } - wave->nextServiceCycle += wave->nextTimeHighCycles; - nextEventCycles = min_u32(nextEventCycles, max_32(wave->nextTimeHighCycles + cyclesToGo, microsecondsToClockCycles(1))); + if (overshoot) { + wave->nextServiceCycle = now + wave->nextTimeHighCycles; + nextEventCycles = min_u32(nextEventCycles, wave->nextTimeHighCycles); + } else { + wave->nextServiceCycle += wave->nextTimeHighCycles; + nextEventCycles = min_u32(nextEventCycles, max_32(wave->nextTimeHighCycles + cyclesToGo, microsecondsToClockCycles(1))); + } } else { if (i == 16) { GP16O &= ~1; // GPIO16 write slow as it's RMW } else { ClearGPIO(mask); } - wave->nextServiceCycle += wave->nextTimeLowCycles; - nextEventCycles = min_u32(nextEventCycles, max_32(wave->nextTimeLowCycles + cyclesToGo, microsecondsToClockCycles(1))); + if (overshoot) { + wave->nextServiceCycle = now + wave->nextTimeLowCycles; + nextEventCycles = min_u32(nextEventCycles, wave->nextTimeLowCycles); + } else { + wave->nextServiceCycle += wave->nextTimeLowCycles; + nextEventCycles = min_u32(nextEventCycles, max_32(wave->nextTimeLowCycles + cyclesToGo, microsecondsToClockCycles(1))); + } } } else { uint32_t deltaCycles = wave->nextServiceCycle - now; From 4f4b5041a17265e03f7e839bc55fdb1136924400 Mon Sep 17 00:00:00 2001 From: Stephan Hadinger Date: Wed, 8 Apr 2020 12:08:24 +0200 Subject: [PATCH 5/5] Better handling of interrupt storm during wifi connection --- cores/esp8266/core_esp8266_waveform.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cores/esp8266/core_esp8266_waveform.cpp b/cores/esp8266/core_esp8266_waveform.cpp index f4c540c583..1a6106ab19 100644 --- a/cores/esp8266/core_esp8266_waveform.cpp +++ b/cores/esp8266/core_esp8266_waveform.cpp @@ -273,7 +273,7 @@ static ICACHE_RAM_ATTR void timer1Interrupt() { // // detect interrupt storm, for example during wifi connection. // if we overshoot the cycle by more than 25%, we forget phase and keep PWM duration - int32_t overshoot = (-cyclesToGo) > ((wave->nextTimeHighCycles + wave->nextTimeLowCycles) >> 2); + int32_t overshoot = ((uint32_t)-cyclesToGo) > ((wave->nextTimeHighCycles + wave->nextTimeLowCycles) >> 2); waveformState ^= mask; if (waveformState & mask) { if (i == 16) {