From b89932c64d858dea58229f22bf6ca4b61dcf9cdd Mon Sep 17 00:00:00 2001
From: "Dirk O. Kaar" <dok@dok-net.net>
Date: Thu, 29 Apr 2021 22:30:39 +0200
Subject: [PATCH 01/12] Pin has uint8_t, not int type.
---
cores/esp8266/core_esp8266_waveform.h | 7 +++----
cores/esp8266/core_esp8266_waveform_phase.cpp | 5 +++--
cores/esp8266/core_esp8266_waveform_pwm.cpp | 17 ++++++++---------
3 files changed, 14 insertions(+), 15 deletions(-)
diff --git a/cores/esp8266/core_esp8266_waveform.h b/cores/esp8266/core_esp8266_waveform.h
index d3d303f99f..040e737bf8 100644
--- a/cores/esp8266/core_esp8266_waveform.h
+++ b/cores/esp8266/core_esp8266_waveform.h
@@ -110,11 +110,10 @@ int stopWaveform(uint8_t pin);
// Make sure the CB function has the IRAM_ATTR decorator.
void setTimer1Callback(uint32_t (*fn)());
-
// Internal-only calls, not for applications
-extern void _setPWMFreq(uint32_t freq);
-extern bool _stopPWM(uint8_t pin);
-extern bool _setPWM(int pin, uint32_t val, uint32_t range);
+void _setPWMFreq(uint32_t freq);
+bool _stopPWM(uint8_t pin);
+bool _setPWM(uint8_t pin, uint32_t val, uint32_t range);
#ifdef __cplusplus
}
diff --git a/cores/esp8266/core_esp8266_waveform_phase.cpp b/cores/esp8266/core_esp8266_waveform_phase.cpp
index 4240ccb3c1..966f4bb546 100644
--- a/cores/esp8266/core_esp8266_waveform_phase.cpp
+++ b/cores/esp8266/core_esp8266_waveform_phase.cpp
@@ -55,8 +55,8 @@ extern "C" void enablePhaseLockedWaveform (void)
// No-op calls to override the PWM implementation
extern "C" void _setPWMFreq_weak(uint32_t freq) { (void) freq; }
-extern "C" IRAM_ATTR bool _stopPWM_weak(int pin) { (void) pin; return false; }
-extern "C" bool _setPWM_weak(int pin, uint32_t val, uint32_t range) { (void) pin; (void) val; (void) range; return false; }
+extern "C" IRAM_ATTR bool _stopPWM_weak(uint8_t pin) { (void) pin; return false; }
+extern "C" bool _setPWM_weak(uint8_t pin, uint32_t val, uint32_t range) { (void) pin; (void) val; (void) range; return false; }
// Timer is 80MHz fixed. 160MHz CPU frequency need scaling.
@@ -116,6 +116,7 @@ static IRAM_ATTR void timer1Interrupt();
// Non-speed critical bits
#pragma GCC optimize ("Os")
+static void initTimer() __attribute__((noinline));
static void initTimer() {
timer1_disable();
ETS_FRC_TIMER1_INTR_ATTACH(NULL, NULL);
diff --git a/cores/esp8266/core_esp8266_waveform_pwm.cpp b/cores/esp8266/core_esp8266_waveform_pwm.cpp
index f85ccb76aa..f232302b49 100644
--- a/cores/esp8266/core_esp8266_waveform_pwm.cpp
+++ b/cores/esp8266/core_esp8266_waveform_pwm.cpp
@@ -97,7 +97,7 @@ static WVFState wvfState;
static IRAM_ATTR void timer1Interrupt();
static bool timerRunning = false;
-static __attribute__((noinline)) void initTimer() {
+static void initTimer() {
if (!timerRunning) {
timer1_disable();
ETS_FRC_TIMER1_INTR_ATTACH(NULL, NULL);
@@ -169,8 +169,7 @@ static IRAM_ATTR void _notifyPWM(PWMState *p, bool idle) {
}
}
-static void _addPWMtoList(PWMState &p, int pin, uint32_t val, uint32_t range);
-
+static void _addPWMtoList(PWMState &p, uint8_t pin, uint32_t val, uint32_t range);
// Called when analogWriteFreq() changed to update the PWM total period
extern void _setPWMFreq_weak(uint32_t freq) __attribute__((weak));
@@ -216,7 +215,7 @@ void _setPWMFreq(uint32_t freq) {
// Helper routine to remove an entry from the state machine
// and clean up any marked-off entries
-static void _cleanAndRemovePWM(PWMState *p, int pin) {
+static void _cleanAndRemovePWM(PWMState *p, uint8_t pin) {
uint32_t leftover = 0;
uint32_t in, out;
for (in = 0, out = 0; in < p->cnt; in++) {
@@ -265,7 +264,7 @@ IRAM_ATTR bool _stopPWM(uint8_t pin) {
return _stopPWM_bound(pin);
}
-static void _addPWMtoList(PWMState &p, int pin, uint32_t val, uint32_t range) {
+static void _addPWMtoList(PWMState &p, uint8_t pin, uint32_t val, uint32_t range) {
// Stash the val and range so we can re-evaluate the fraction
// should the user change PWM frequency. This allows us to
// give as great a precision as possible. We know by construction
@@ -311,8 +310,8 @@ static void _addPWMtoList(PWMState &p, int pin, uint32_t val, uint32_t range) {
}
// Called by analogWrite(1...99%) to set the PWM duty in clock cycles
-extern bool _setPWM_weak(int pin, uint32_t val, uint32_t range) __attribute__((weak));
-bool _setPWM_weak(int pin, uint32_t val, uint32_t range) {
+extern bool _setPWM_weak(uint8_t pin, uint32_t val, uint32_t range) __attribute__((weak));
+bool _setPWM_weak(uint8_t pin, uint32_t val, uint32_t range) {
stopWaveform(pin);
PWMState p; // Working copy
p = pwmState;
@@ -342,8 +341,8 @@ bool _setPWM_weak(int pin, uint32_t val, uint32_t range) {
return true;
}
-static bool _setPWM_bound(int pin, uint32_t val, uint32_t range) __attribute__((weakref("_setPWM_weak")));
-bool _setPWM(int pin, uint32_t val, uint32_t range) {
+static bool _setPWM_bound(uint8_t pin, uint32_t val, uint32_t range) __attribute__((weakref("_setPWM_weak")));
+bool _setPWM(uint8_t pin, uint32_t val, uint32_t range) {
return _setPWM_bound(pin, val, range);
}
From 2776d16836f9356fb0847518219d04c29e47ffc8 Mon Sep 17 00:00:00 2001
From: "Dirk O. Kaar" <dok@dok-net.net>
Date: Thu, 29 Apr 2021 22:51:08 +0200
Subject: [PATCH 02/12] Can't image stopWaveform is called but PWM shall not
stop.
startWaveform already implied stopping PWM before.
---
cores/esp8266/Tone.cpp | 5 -----
cores/esp8266/core_esp8266_waveform_pwm.cpp | 3 +++
cores/esp8266/core_esp8266_wiring_digital.cpp | 1 -
3 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/cores/esp8266/Tone.cpp b/cores/esp8266/Tone.cpp
index b4dc93c642..601b1df51d 100644
--- a/cores/esp8266/Tone.cpp
+++ b/cores/esp8266/Tone.cpp
@@ -30,11 +30,6 @@ static void _startTone(uint8_t _pin, uint32_t high, uint32_t low, uint32_t durat
return;
}
- // Stop any analogWrites (PWM) because they are a different generator
- _stopPWM(_pin);
- // If there's another Tone or startWaveform on this pin
- // it will be changed on-the-fly (no need to stop it)
-
pinMode(_pin, OUTPUT);
high = std::max(high, (uint32_t)microsecondsToClockCycles(25)); // new 20KHz maximum tone frequency,
diff --git a/cores/esp8266/core_esp8266_waveform_pwm.cpp b/cores/esp8266/core_esp8266_waveform_pwm.cpp
index f232302b49..a2664f98bf 100644
--- a/cores/esp8266/core_esp8266_waveform_pwm.cpp
+++ b/cores/esp8266/core_esp8266_waveform_pwm.cpp
@@ -435,6 +435,9 @@ IRAM_ATTR int stopWaveform_weak(uint8_t pin) {
if (!timerRunning) {
return false;
}
+
+ _stopPWM(pin); // Make sure there's no PWM live here
+
// 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<<pin;
diff --git a/cores/esp8266/core_esp8266_wiring_digital.cpp b/cores/esp8266/core_esp8266_wiring_digital.cpp
index 6b42c3ee36..015c21bdbf 100644
--- a/cores/esp8266/core_esp8266_wiring_digital.cpp
+++ b/cores/esp8266/core_esp8266_wiring_digital.cpp
@@ -83,7 +83,6 @@ extern void __pinMode(uint8_t pin, uint8_t mode) {
extern void IRAM_ATTR __digitalWrite(uint8_t pin, uint8_t val) {
stopWaveform(pin); // Disable any Tone or startWaveform on this pin
- _stopPWM(pin); // and any analogWrites (PWM)
if(pin < 16){
if(val) GPOS = (1 << pin);
else GPOC = (1 << pin);
From 98537bd150194d88c2140fb2dd51086810d4a11b Mon Sep 17 00:00:00 2001
From: "Dirk O. Kaar" <dok@dok-net.net>
Date: Sun, 2 May 2021 11:50:10 +0200
Subject: [PATCH 03/12] let analogWrite coexist with open-drain mode
---
cores/esp8266/core_esp8266_wiring_digital.cpp | 2 +-
cores/esp8266/core_esp8266_wiring_pwm.cpp | 26 ++++++++++++-------
libraries/Servo/src/Servo.cpp | 3 +--
3 files changed, 18 insertions(+), 13 deletions(-)
diff --git a/cores/esp8266/core_esp8266_wiring_digital.cpp b/cores/esp8266/core_esp8266_wiring_digital.cpp
index 015c21bdbf..b04310dc74 100644
--- a/cores/esp8266/core_esp8266_wiring_digital.cpp
+++ b/cores/esp8266/core_esp8266_wiring_digital.cpp
@@ -82,7 +82,7 @@ extern void __pinMode(uint8_t pin, uint8_t mode) {
}
extern void IRAM_ATTR __digitalWrite(uint8_t pin, uint8_t val) {
- stopWaveform(pin); // Disable any Tone or startWaveform on this pin
+ stopWaveform(pin); // Disable any Tone, startWaveform, or analogWrite on this pin
if(pin < 16){
if(val) GPOS = (1 << pin);
else GPOC = (1 << pin);
diff --git a/cores/esp8266/core_esp8266_wiring_pwm.cpp b/cores/esp8266/core_esp8266_wiring_pwm.cpp
index 5bd5416d70..8ab0f296a1 100644
--- a/cores/esp8266/core_esp8266_wiring_pwm.cpp
+++ b/cores/esp8266/core_esp8266_wiring_pwm.cpp
@@ -44,7 +44,14 @@ extern void __analogWriteFreq(uint32_t freq) {
}
extern void __analogWrite(uint8_t pin, int val) {
- analogWriteMode(pin, val, false);
+ if (pin > 16) {
+ return;
+ }
+ bool openDrain = false;
+ if (analogMap & 1UL << pin) {
+ openDrain = GPC(pin) & (1 << GPCD);
+ }
+ analogWriteMode(pin, val, openDrain);
}
extern void __analogWriteMode(uint8_t pin, int val, bool openDrain) {
@@ -59,23 +66,22 @@ extern void __analogWriteMode(uint8_t pin, int val, bool openDrain) {
}
if (analogMap & 1UL << pin) {
- // Per the Arduino docs at https://www.arduino.cc/reference/en/language/functions/analog-io/analogwrite/
- // val: the duty cycle: between 0 (always off) and 255 (always on).
- // So if val = 0 we have digitalWrite(LOW), if we have val==range we have digitalWrite(HIGH)
-
analogMap &= ~(1 << pin);
+ const bool isOpenDrain = GPC(pin) & (1 << GPCD);
+ if (isOpenDrain != openDrain) {
+ GPC(pin) ^= (1 << GPCD);
+ }
}
else {
- if(openDrain) {
- pinMode(pin, OUTPUT_OPEN_DRAIN);
- } else {
- pinMode(pin, OUTPUT);
- }
+ pinMode(pin, openDrain ? OUTPUT_OPEN_DRAIN : OUTPUT);
}
uint32_t high = (analogPeriod * val) / analogScale;
uint32_t low = analogPeriod - high;
// 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)
int phaseReference = __builtin_ffs(analogMap) - 1;
+ // Per the Arduino docs at https://www.arduino.cc/reference/en/language/functions/analog-io/analogwrite/
+ // val: the duty cycle: between 0 (always off) and 255 (always on).
+ // So if val = 0 we have digitalWrite(LOW), if we have val==range we have digitalWrite(HIGH)
if (_setPWM(pin, val, analogScale)) {
analogMap |= (1 << pin);
} else if (startWaveformClockCycles(pin, high, low, 0, phaseReference, 0, true)) {
diff --git a/libraries/Servo/src/Servo.cpp b/libraries/Servo/src/Servo.cpp
index 1d19f62683..f5c410083b 100644
--- a/libraries/Servo/src/Servo.cpp
+++ b/libraries/Servo/src/Servo.cpp
@@ -116,8 +116,7 @@ void Servo::writeMicroseconds(int value)
_servoMap &= ~(1 << _pin);
// 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)
int phaseReference = __builtin_ffs(_servoMap) - 1;
- if (startWaveform(_pin, _valueUs, REFRESH_INTERVAL - _valueUs, 0, phaseReference))
- {
+ if (startWaveform(_pin, _valueUs, REFRESH_INTERVAL - _valueUs, 0, phaseReference)) {
_servoMap |= (1 << _pin);
}
}
From 36b2bd3dd248cb2a011b83ace0784297603d41a1 Mon Sep 17 00:00:00 2001
From: "Dirk O. Kaar" <dok@dok-net.net>
Date: Sun, 2 May 2021 12:40:33 +0200
Subject: [PATCH 04/12] Don't force timer on PWM update as this is pulled by
ISR, but only implemented
on next PWM period begin anyway.
---
cores/esp8266/core_esp8266_waveform_pwm.cpp | 45 +++++++++++----------
1 file changed, 23 insertions(+), 22 deletions(-)
diff --git a/cores/esp8266/core_esp8266_waveform_pwm.cpp b/cores/esp8266/core_esp8266_waveform_pwm.cpp
index a2664f98bf..3402159ee9 100644
--- a/cores/esp8266/core_esp8266_waveform_pwm.cpp
+++ b/cores/esp8266/core_esp8266_waveform_pwm.cpp
@@ -128,7 +128,7 @@ static IRAM_ATTR void forceTimerInterrupt() {
constexpr int maxPWMs = 8;
// PWM machine state
-typedef struct PWMState {
+struct PWMState {
uint32_t mask; // Bitmask of active pins
uint32_t cnt; // How many entries
uint32_t idx; // Where the state machine is along the list
@@ -136,13 +136,12 @@ typedef struct PWMState {
uint32_t delta[maxPWMs + 1];
uint32_t nextServiceCycle; // Clock cycle for next step
struct PWMState *pwmUpdate; // Set by main code, cleared by ISR
-} PWMState;
+};
static PWMState pwmState;
static uint32_t _pwmFreq = 1000;
static uint32_t _pwmPeriod = microsecondsToClockCycles(1000000UL) / _pwmFreq;
-
// If there are no more scheduled activities, shut down Timer 1.
// Otherwise, do nothing.
static IRAM_ATTR void disableIdleTimer() {
@@ -158,6 +157,7 @@ static IRAM_ATTR void disableIdleTimer() {
// Wait for mailbox to be emptied (either busy or delay() as needed)
static IRAM_ATTR void _notifyPWM(PWMState *p, bool idle) {
p->pwmUpdate = nullptr;
+ MEMBARRIER();
pwmState.pwmUpdate = p;
MEMBARRIER();
forceTimerInterrupt();
@@ -242,8 +242,7 @@ IRAM_ATTR bool _stopPWM_weak(uint8_t pin) {
return false; // Pin not actually active
}
- PWMState p; // The working copy since we can't edit the one in use
- p = pwmState;
+ PWMState p = pwmState; // The working copy since we can't edit the one in use
// In _stopPWM we just clear the mask but keep everything else
// untouched to save IRAM. The main startPWM will handle cleanup.
@@ -264,7 +263,7 @@ IRAM_ATTR bool _stopPWM(uint8_t pin) {
return _stopPWM_bound(pin);
}
-static void _addPWMtoList(PWMState &p, uint8_t pin, uint32_t val, uint32_t range) {
+static void _addPWMtoList(PWMState& p, uint8_t pin, uint32_t val, uint32_t range) {
// Stash the val and range so we can re-evaluate the fraction
// should the user change PWM frequency. This allows us to
// give as great a precision as possible. We know by construction
@@ -278,7 +277,8 @@ static void _addPWMtoList(PWMState &p, uint8_t pin, uint32_t val, uint32_t range
// Clip to sane values in the case we go from OK to not-OK when adjusting frequencies
if (cc == 0) {
cc = 1;
- } else if (cc >= _pwmPeriod) {
+ }
+ else if (cc >= _pwmPeriod) {
cc = _pwmPeriod - 1;
}
@@ -286,9 +286,10 @@ static void _addPWMtoList(PWMState &p, uint8_t pin, uint32_t val, uint32_t range
// Starting up from scratch, special case 1st element and PWM period
p.pin[0] = pin;
p.delta[0] = cc;
- // Final pin is never used: p.pin[1] = 0xff;
+ // Final pin is never used: p.pin[1] = 0xff;
p.delta[1] = _pwmPeriod - cc;
- } else {
+ }
+ else {
uint32_t ttl = 0;
uint32_t i;
// Skip along until we're at the spot to insert
@@ -532,39 +533,39 @@ static IRAM_ATTR void timer1Interrupt() {
// PWM state machine implementation
if (pwmState.cnt) {
- int32_t cyclesToGo;
+ int32_t pwmCyclesToGo;
do {
- cyclesToGo = pwmState.nextServiceCycle - GetCycleCountIRQ();
- if (cyclesToGo < 0) {
+ pwmCyclesToGo = pwmState.nextServiceCycle - GetCycleCountIRQ();
+ if (pwmCyclesToGo <= 0) {
if (pwmState.idx == pwmState.cnt) { // Start of pulses, possibly copy new
if (pwmState.pwmUpdate) {
// Do the memory copy from temp to global and clear mailbox
- pwmState = *(PWMState*)pwmState.pwmUpdate;
+ pwmState = *pwmState.pwmUpdate;
}
GPOS = pwmState.mask; // Set all active pins high
if (pwmState.mask & (1<<16)) {
GP16O = 1;
}
pwmState.idx = 0;
- } else {
+ }
+ else {
do {
// Drop the pin at this edge
- if (pwmState.mask & (1<<pwmState.pin[pwmState.idx])) {
- GPOC = 1<<pwmState.pin[pwmState.idx];
- if (pwmState.pin[pwmState.idx] == 16) {
- GP16O = 0;
- }
+ GPOC = 1<<pwmState.pin[pwmState.idx];
+ if (pwmState.pin[pwmState.idx] == 16) {
+ GP16O = 0;
}
pwmState.idx++;
// Any other pins at this same PWM value will have delta==0, drop them too.
} while (pwmState.delta[pwmState.idx] == 0);
}
// Preserve duty cycle over PWM period by using now+xxx instead of += delta
- cyclesToGo = adjust(pwmState.delta[pwmState.idx]);
- pwmState.nextServiceCycle = GetCycleCountIRQ() + cyclesToGo;
+ pwmCyclesToGo = adjust(pwmState.delta[pwmState.idx]);
+ pwmState.nextServiceCycle = GetCycleCountIRQ() + pwmCyclesToGo;
}
nextEventCycle = earliest(nextEventCycle, pwmState.nextServiceCycle);
- } while (pwmState.cnt && (cyclesToGo < 100));
+ // PWM can starve the generic waveform generator if pwmCyclesToGo remains below 100
+ } while (pwmState.cnt && pwmCyclesToGo < 100);
}
for (auto i = wvfState.startPin; i <= wvfState.endPin; i++) {
From ff0782c405fc950588200f93d1a8f50cae5043cf Mon Sep 17 00:00:00 2001
From: "Dirk O. Kaar" <dok@dok-net.net>
Date: Sun, 2 May 2021 15:27:35 +0200
Subject: [PATCH 05/12] Correct shutdown sequence for phase-sync waveform
generator in analogWrite.
---
cores/esp8266/core_esp8266_wiring_pwm.cpp | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/cores/esp8266/core_esp8266_wiring_pwm.cpp b/cores/esp8266/core_esp8266_wiring_pwm.cpp
index 8ab0f296a1..5fc6e026e8 100644
--- a/cores/esp8266/core_esp8266_wiring_pwm.cpp
+++ b/cores/esp8266/core_esp8266_wiring_pwm.cpp
@@ -83,9 +83,18 @@ extern void __analogWriteMode(uint8_t pin, int val, bool openDrain) {
// val: the duty cycle: between 0 (always off) and 255 (always on).
// So if val = 0 we have digitalWrite(LOW), if we have val==range we have digitalWrite(HIGH)
if (_setPWM(pin, val, analogScale)) {
- analogMap |= (1 << pin);
- } else if (startWaveformClockCycles(pin, high, low, 0, phaseReference, 0, true)) {
- analogMap |= (1 << pin);
+ if (val > 0 && val < analogScale) analogMap |= (1 << pin);
+ } else {
+ const bool detach = (val == 0 || val == analogScale);
+ // To go steady LOW or HIGH, let the waveform run into next duty cycle, if any. Then stop.
+ if (startWaveformClockCycles(pin, high, low, static_cast<uint32_t>(detach), phaseReference, 0, true)) {
+ if (detach) {
+ delay((1000 + analogFreq) / analogFreq);
+ stopWaveform(pin);
+ } else {
+ analogMap |= (1 << pin);
+ }
+ }
}
}
From 293f767b4ca6b0b3bbbe08defdd7ddf07282e127 Mon Sep 17 00:00:00 2001
From: "Dirk O. Kaar" <dok@dok-net.net>
Date: Mon, 3 May 2021 10:49:59 +0200
Subject: [PATCH 06/12] Correction to IRQ latency and ISR delta logic.
---
cores/esp8266/core_esp8266_waveform_phase.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/cores/esp8266/core_esp8266_waveform_phase.cpp b/cores/esp8266/core_esp8266_waveform_phase.cpp
index 966f4bb546..1dd4a98643 100644
--- a/cores/esp8266/core_esp8266_waveform_phase.cpp
+++ b/cores/esp8266/core_esp8266_waveform_phase.cpp
@@ -309,7 +309,7 @@ static IRAM_ATTR void timer1Interrupt() {
uint32_t now = ESP.getCycleCount();
uint32_t isrNextEventCcy = now;
while (busyPins) {
- if (static_cast<int32_t>(isrNextEventCcy - now) > IRQLATENCYCCYS) {
+ if (static_cast<int32_t>(isrNextEventCcy - now) > IRQLATENCYCCYS + DELTAIRQCCYS) {
waveform.nextEventCcy = isrNextEventCcy;
break;
}
From 50ded373f982119b7190c8b81ace55ba0e65ecdb Mon Sep 17 00:00:00 2001
From: "Dirk O. Kaar" <dok@dok-net.net>
Date: Mon, 3 May 2021 11:20:29 +0200
Subject: [PATCH 07/12] Early firing the ISR causes interruption to other
waveforms, let it pick up new starts at next regular interval.
---
cores/esp8266/core_esp8266_waveform_phase.cpp | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/cores/esp8266/core_esp8266_waveform_phase.cpp b/cores/esp8266/core_esp8266_waveform_phase.cpp
index 1dd4a98643..3e3dae89f0 100644
--- a/cores/esp8266/core_esp8266_waveform_phase.cpp
+++ b/cores/esp8266/core_esp8266_waveform_phase.cpp
@@ -195,10 +195,7 @@ int startWaveformClockCycles_weak(uint8_t pin, uint32_t highCcys, uint32_t lowCc
if (!waveform.timer1Running) {
initTimer();
}
- else if (T1V > IRQLATENCYCCYS) {
- // Must not interfere if Timer is due shortly
- timer1_write(IRQLATENCYCCYS);
- }
+ // The ISR pulls updates on next PWM interval
}
else {
wave.mode = WaveformMode::INFINITE; // turn off possible expiry to make update atomic from NMI
From f17ba7fb39ce364c400bca0d4e6f62eac99cf8a3 Mon Sep 17 00:00:00 2001
From: "Dirk O. Kaar" <dok@dok-net.net>
Date: Mon, 3 May 2021 15:20:20 +0200
Subject: [PATCH 08/12] Stopping of the same pin always wins over starting.
---
cores/esp8266/core_esp8266_waveform_phase.cpp | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/cores/esp8266/core_esp8266_waveform_phase.cpp b/cores/esp8266/core_esp8266_waveform_phase.cpp
index 3e3dae89f0..c56bacabdf 100644
--- a/cores/esp8266/core_esp8266_waveform_phase.cpp
+++ b/cores/esp8266/core_esp8266_waveform_phase.cpp
@@ -78,7 +78,7 @@ constexpr int32_t DELTAIRQCCYS = ISCPUFREQ160MHZ ?
enum class WaveformMode : uint8_t {INFINITE = 0, EXPIRES = 1, UPDATEEXPIRY = 2, INIT = 3};
// Waveform generator can create tones, PWM, and servos
-typedef struct {
+struct Waveform {
uint32_t nextPeriodCcy; // ESP clock cycle when a period begins. If WaveformMode::INIT, temporarily holds positive phase offset ccy count
uint32_t endDutyCcy; // ESP clock cycle when going from duty to off
int32_t dutyCcys; // Set next off cycle at low->high to maintain phase
@@ -88,7 +88,7 @@ typedef struct {
WaveformMode mode;
int8_t alignPhase; // < 0 no phase alignment, otherwise starts waveform in relative phase offset to given pin
bool autoPwm; // perform PWM duty to idle cycle ratio correction under high load at the expense of precise timings
-} Waveform;
+};
namespace {
@@ -116,7 +116,6 @@ static IRAM_ATTR void timer1Interrupt();
// Non-speed critical bits
#pragma GCC optimize ("Os")
-static void initTimer() __attribute__((noinline));
static void initTimer() {
timer1_disable();
ETS_FRC_TIMER1_INTR_ATTACH(NULL, NULL);
@@ -126,7 +125,7 @@ static void initTimer() {
timer1_write(IRQLATENCYCCYS); // Cause an interrupt post-haste
}
-static void IRAM_ATTR deinitTimer() {
+static IRAM_ATTR void deinitTimer() {
ETS_FRC_TIMER1_NMI_INTR_ATTACH(NULL);
timer1_disable();
timer1_isr_init();
@@ -195,7 +194,7 @@ int startWaveformClockCycles_weak(uint8_t pin, uint32_t highCcys, uint32_t lowCc
if (!waveform.timer1Running) {
initTimer();
}
- // The ISR pulls updates on next PWM interval
+ // The ISR pulls updates on the next waveform interval
}
else {
wave.mode = WaveformMode::INFINITE; // turn off possible expiry to make update atomic from NMI
@@ -265,7 +264,7 @@ static IRAM_ATTR void timer1Interrupt() {
const bool isCPU2X = CPU2X & 1;
if ((waveform.toSetBits && !(waveform.enabled & waveform.toSetBits)) || waveform.toDisableBits) {
// Handle enable/disable requests from main app.
- waveform.enabled = (waveform.enabled & ~waveform.toDisableBits) | waveform.toSetBits; // Set the requested waveforms on/off
+ waveform.enabled = (waveform.enabled | waveform.toSetBits) & ~waveform.toDisableBits; // Set the requested waveforms on/off
// 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)
waveform.toDisableBits = 0;
}
From 5de25bde5e3a947bc6f0f275ba6725451f5f8ef8 Mon Sep 17 00:00:00 2001
From: "Dirk O. Kaar" <dok@dok-net.net>
Date: Wed, 19 May 2021 10:17:54 +0200
Subject: [PATCH 09/12] Clean code.
---
cores/esp8266/core_esp8266_wiring_pwm.cpp | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/cores/esp8266/core_esp8266_wiring_pwm.cpp b/cores/esp8266/core_esp8266_wiring_pwm.cpp
index 5fc6e026e8..07eac4e907 100644
--- a/cores/esp8266/core_esp8266_wiring_pwm.cpp
+++ b/cores/esp8266/core_esp8266_wiring_pwm.cpp
@@ -83,7 +83,9 @@ extern void __analogWriteMode(uint8_t pin, int val, bool openDrain) {
// val: the duty cycle: between 0 (always off) and 255 (always on).
// So if val = 0 we have digitalWrite(LOW), if we have val==range we have digitalWrite(HIGH)
if (_setPWM(pin, val, analogScale)) {
- if (val > 0 && val < analogScale) analogMap |= (1 << pin);
+ if (val > 0 && val < analogScale) {
+ analogMap |= (1 << pin);
+ }
} else {
const bool detach = (val == 0 || val == analogScale);
// To go steady LOW or HIGH, let the waveform run into next duty cycle, if any. Then stop.
From 6dbd371a0324b160b81353a5d3af9f2db793d854 Mon Sep 17 00:00:00 2001
From: "Dirk O. Kaar" <dok@dok-net.net>
Date: Mon, 31 May 2021 17:57:42 +0200
Subject: [PATCH 10/12] Prevent initial bad clock drift calculation on timer
init.
---
cores/esp8266/core_esp8266_waveform_phase.cpp | 2 ++
1 file changed, 2 insertions(+)
diff --git a/cores/esp8266/core_esp8266_waveform_phase.cpp b/cores/esp8266/core_esp8266_waveform_phase.cpp
index c56bacabdf..9d200fa56c 100644
--- a/cores/esp8266/core_esp8266_waveform_phase.cpp
+++ b/cores/esp8266/core_esp8266_waveform_phase.cpp
@@ -122,6 +122,7 @@ static void initTimer() {
ETS_FRC_TIMER1_NMI_INTR_ATTACH(timer1Interrupt);
timer1_enable(TIM_DIV1, TIM_EDGE, TIM_SINGLE);
waveform.timer1Running = true;
+ waveform.nextEventCcy = ESP.getCycleCount() + IRQLATENCYCCYS;
timer1_write(IRQLATENCYCCYS); // Cause an interrupt post-haste
}
@@ -229,6 +230,7 @@ IRAM_ATTR int stopWaveform_weak(uint8_t pin) {
std::atomic_thread_fence(std::memory_order_release);
// Must not interfere if Timer is due shortly
if (T1V > IRQLATENCYCCYS) {
+ waveform.nextEventCcy = ESP.getCycleCount() + IRQLATENCYCCYS;
timer1_write(IRQLATENCYCCYS);
}
while (waveform.toDisableBits) {
From d8893b09c886c089e5bc7c6b8fb366c66c7fd2a2 Mon Sep 17 00:00:00 2001
From: "Dirk O. Kaar" <dok@dok-net.net>
Date: Sun, 6 Jun 2021 11:05:35 +0200
Subject: [PATCH 11/12] stopWaveform in _setPWM caused unconditional _stopPWM.
---
cores/esp8266/core_esp8266_waveform_pwm.cpp | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/cores/esp8266/core_esp8266_waveform_pwm.cpp b/cores/esp8266/core_esp8266_waveform_pwm.cpp
index 3402159ee9..5fd3925703 100644
--- a/cores/esp8266/core_esp8266_waveform_pwm.cpp
+++ b/cores/esp8266/core_esp8266_waveform_pwm.cpp
@@ -160,7 +160,9 @@ static IRAM_ATTR void _notifyPWM(PWMState *p, bool idle) {
MEMBARRIER();
pwmState.pwmUpdate = p;
MEMBARRIER();
- forceTimerInterrupt();
+ if (idle) {
+ forceTimerInterrupt();
+ }
while (pwmState.pwmUpdate) {
if (idle) {
esp_yield();
@@ -313,7 +315,10 @@ static void _addPWMtoList(PWMState& p, uint8_t pin, uint32_t val, uint32_t range
// Called by analogWrite(1...99%) to set the PWM duty in clock cycles
extern bool _setPWM_weak(uint8_t pin, uint32_t val, uint32_t range) __attribute__((weak));
bool _setPWM_weak(uint8_t pin, uint32_t val, uint32_t range) {
- stopWaveform(pin);
+ const uint32_t mask = 1<<pin;
+ if (wvfState.waveformEnabled & mask) {
+ stopWaveform(pin);
+ }
PWMState p; // Working copy
p = pwmState;
// Get rid of any entries for this pin
@@ -368,7 +373,7 @@ int startWaveformClockCycles_weak(uint8_t pin, uint32_t timeHighCycles, uint32_t
_stopPWM(pin); // Make sure there's no PWM live here
- uint32_t mask = 1<<pin;
+ const uint32_t mask = 1<<pin;
MEMBARRIER();
if (wvfState.waveformEnabled & mask) {
// Make sure no waveform changes are waiting to be applied
@@ -441,7 +446,7 @@ IRAM_ATTR int stopWaveform_weak(uint8_t pin) {
// 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<<pin;
+ const uint32_t mask = 1<<pin;
if (wvfState.waveformEnabled & mask) {
wvfState.waveformToDisable = mask;
// Cancel any pending updates for this waveform, too.
@@ -569,7 +574,7 @@ static IRAM_ATTR void timer1Interrupt() {
}
for (auto i = wvfState.startPin; i <= wvfState.endPin; i++) {
- uint32_t mask = 1<<i;
+ const uint32_t mask = 1<<i;
// If it's not on, ignore!
if (!(wvfState.waveformEnabled & mask)) {
From 9c612a9c86fa59ddca4f19e70dcefdc0893153ff Mon Sep 17 00:00:00 2001
From: "Dirk O. Kaar" <dok@dok-net.net>
Date: Sat, 10 Dec 2022 23:25:54 +0100
Subject: [PATCH 12/12] Prevent inlining of `initTimer()`
---
cores/esp8266/core_esp8266_waveform_pwm.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/cores/esp8266/core_esp8266_waveform_pwm.cpp b/cores/esp8266/core_esp8266_waveform_pwm.cpp
index 5fd3925703..6c591366f5 100644
--- a/cores/esp8266/core_esp8266_waveform_pwm.cpp
+++ b/cores/esp8266/core_esp8266_waveform_pwm.cpp
@@ -97,7 +97,7 @@ static WVFState wvfState;
static IRAM_ATTR void timer1Interrupt();
static bool timerRunning = false;
-static void initTimer() {
+static __attribute__((noinline)) void initTimer() {
if (!timerRunning) {
timer1_disable();
ETS_FRC_TIMER1_INTR_ATTACH(NULL, NULL);