Allow waveforms to be specified in clock cycles

[earlephilhower]

Allow the PWM to specify sub-microsecond waveform edges, as have been
proposed by @dok-net and me. No other changes intended.

This will increase the linearity at 30 and 40 kHZ PWM rates, but leave
most other things unaffected.

[devyte]

Does it make sense to bring Servo and Tone to the startWaveformCycles api?

[earlephilhower]

I'll throw them in, but it's not going to affect things much. Servos are 20ms periods, so you have 20,000 steps to work with now. Tone is up to 20khz (50us) so it might have some effect.

-edit- Servo's api only allows microseconds or degrees, both as integer. So there's nothing to do there.

[dok-net]

@earlephilhower @devyte @Tech-TX Let us first consider the physics of CPU cycle resolution vs. µs resolution on the ESP8266 more deeply before possibly wasting time on this.

The TIMER1 can fire at CPU cycle precision, but the ISR has a latency of multi-µs, and a precision of equal or worse than 1µs, even when generating just a single waveform.

BTW, the use of "cycle" is more than mildly confusing here, there's cycle as synonym for a period, there's duty cycle, idle / off cycle, and there's (CPU) clock cycle (a full clock oscillator period) :-) Could we please name any new startWaveform at CPU cycle precision startWaveformClockCycles (unless we agree to my sticking point and there will not be such a function after all)?

My observations are that for long period durations, the CPU cycle precision is achieved due to the TIMER1 resolution, but for duty or off cycles below or near 1µs, it cannot.

On the other hand, attempting to put waveform edges out at CPU cycle resolution, invariably does one performance hurting thing, it increases the number of iterations in the ISR's loop, which contributes to overall system latency and works against the originally intended precision increase.

It would probably be better to align all edges at µs steps (CPU cycle % system_get_cpu_freq() == 0).

[earlephilhower]

The change is a precision vs. accuracy thing. Yes, it's not precise, but it can be relatively accurate with a small (1) number of waveforms. Sending in 12,500ns may not get you exactly 12,500ns to the edge, but it will get you somewhere between what you'd get at 12us and 13us. So you're not artificially quantizing things. For low frequencies, it's a no-op. For 40khz PWM, it can help smooth the jaggies (only 25us per cycle there, so you would only be able to specify 24 different levels from 1-24us otherwise).

Idea is to get the simple, very limited changes in for 2.7 since there is still no consensus on the big changes for 3.0.

[dok-net]

@earlephilhower To the contrary:
Due to the internal loop latency of the ISR, there is always a quantum, so 12µs, 12500ns, 13µs, in CPU clock cycles, may or may not give the same measurable wall clock time, below or above 12µs, 13µs, in steps of but on average 1µs. The ostensible linearity you see in @Tech-TX pictures is faux and in fact just an artifact of random jitter which masks the real low number of steps.

That said, adding an startWaveformClockCycles() API doesn't hurt, but for now does nothing for the claim that this PR only makes minimal changes to achieve a given goal, because it's not minimal and doesn't achieve the goal.

I've been working hard on final improvements and tunings for the PR #7022 so I don't have hard numbers at hand for this #7211, but if you give it a little time, both @Tech-TX and myself should look into it. Also, I'd hope @s-hadinger will report from LED stripe field tests :-)

[devyte]

This fixes a bug related to the calculation of the time when edges are supposed to be serviced. The need for it was found experimentally from a linearity graph, and looking at the code it is obvious. The current code imposes an artificial granularity of 1us due to the waveform() arguments. At low frequencies that's ok, but at higher frequencies the calculation error becomes dominant.
This has nothing to do with the inherent overhead between timer IRQ and ISR.
The result of the enhancement has been determined experimentally to be good by looking at the linearity graph of pwm. With one pwm ch @40KHz, the current code shows a staircased graph, while with this fix it is a smooth triangular waveform.
This change is a strict and limited enhancement over current master, and it has been split off from the other PRs due to concerns about other changes in those PRs. That means we know this change is needed, but we're not happy with the experimental results from the other PRs (e.g.: handling of phase). So this goes in now, and investigation into the other concerns continues in the meantime.

[dok-net]

Admitted, this PR got mistaken for #7192 - so yes, there's no breaking changes in here. I have also done timings tests and the popular linearity triangle wave test and found the results as good as current PR#7022, actually also a tiny bit better at in-ISR resolution. Which means in numbers, for #7211, the quantum is 0.9us, whereas #7022 performs at a quantum of (after latest commit better than) 1.28us. Though, the first and the last step at zero or 100% duty are worse for #7211.Now, before you merge #7211, could you please adopt the pertinent changes to core_esp8266_waveform.h from #7022 to everyone's benefit? Some improvements to the comments, the new CPU cycle resolution, and let me implore you once more to opt into the much more expressive namingstartWaveformClockCycles ?It also aligns so much better with the ESP class's naming conventions.Also it avoids API breakage with #7022.

[dok-net]

To explain a misunderstanding, the resolution issue I am referring to is not about IRQ / timer INT to ISR startup latency! The internal loop in the waveform generator ISR has a physical limitation at which rate it can toggle GPIOs, this is currently the aforementioned 0.9 (corner case: 1.3) us switching time. PR#7022 has 1.68/1.28 (with approx. 10% improvement in latest commit) microsecs switching time. Which is not bad given the advanced features in 7022 :-) BTW, if patched in core_esp8266_wiring_pwm.cpp, both PR can deal with a single waveform at 60kHz just nicely...

[dok-net]

Linearity graphs, analogWrite PWM, 40kHz, 1023 steps default:
This is PR7022, commit 166d2a35c24dbe56828229315b0bacee22066bbf:

PR 7211, current branch head:

[earlephilhower]

Thanks for the update, @dok-net! I think we're OK here and can continue in the other PRs to find the best way forward. I believe you've also moved to cycle-level API so I'll merge this and we can then continue on finding the best way forward for 3.0!

[devyte]

Per @Tech-TX :
master + #7211 at 80MHz:

and same at 160MHz:

I think this agrees with @dok-net 's findings above.
So: with this change merged we now have a decent master linearity reference, and we can move on to evaluate other individual pwm enhancements.