Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unsynced PWM causes LED flickering #7054

Closed
6 tasks done
s-hadinger opened this issue Feb 1, 2020 · 11 comments
Closed
6 tasks done

Unsynced PWM causes LED flickering #7054

s-hadinger opened this issue Feb 1, 2020 · 11 comments
Assignees

Comments

@s-hadinger
Copy link
Contributor

Basic Infos

  • This issue complies with the issue POLICY doc.
  • I have read the documentation at readthedocs and the issue is not addressed there.
  • I have tested that the issue is present in current master branch (aka latest git).
  • I have searched the issue tracker for a similar issue.
  • If there is a stack dump, I have decoded it.
  • I have filled out all fields below.

Platform

  • Hardware: All ESP8266/8265 based
  • Core Version: 2.6.1
  • Development Env: Platformio
  • Operating System: MacOS

Settings in IDE

  • Module: Generic ESP8266 Module
  • Flash Mode: DOUT
  • Flash Size: 1MB
  • lwip Variant: v2 Higher Bandwidth
  • Reset Method: ?
  • Flash Frequency: ?
  • CPU Frequency: 80Mhz
  • Upload Using: SERIAL
  • Upload Speed: 115200

Problem Description

For several months, some Tasmota users reported LED flickering when set at low brightness. The flickering is subtle but noticeable. The PWM values are set with analogWrite() and left unchanged.

The problem occurs with core 2.5.x and 2.6.x but did not occur with 2.4.x.

I finally took a scope and found the cause of the issue.

Setup:

  • Wemos D1 mini, with 3 GPIOs (4,5,12) set as PWM.
  • PWM frequency: 880Hz
  • PWM range: 0..1023
  • PWM1=5, PWM2=10, PWM3=3

Below are traces for PWM1 and PWM2, taken at different moments:
scope0
scope1
scope2

PWM pulses are most of the time happening almost at the same time, but regularly they get desynchronized (3rd screenshot) and get back to sync after a fraction of seconds.

Although the timing of the pulses are correct, the power supply of LED drivers are not perfect and draining current at different moments cannot be compensated by the capacitor.

For led drivers with high current, could we have PWM ensure that pulses are synced again? I will take a look at the waveform generator in between.

@devyte
Copy link
Collaborator

devyte commented Feb 1, 2020

@s-hadinger please make sure to test with 2.6.3 or latest git.

@earlephilhower
Copy link
Collaborator

earlephilhower commented Feb 1, 2020

There was a fundamental change in the analogWrite behind this. The latest GIT may have some difference, but nothing that would change what @s-hadinger is talking about, IIRC.

The 2.4/pre version guaranteed phase alignment since it took over the timer and everyone analogWrite pin had a sync'd start point (i.e. at t=0 all PWM pins went high, and then at the appropriate time individual pins were set to 0). Other stuff that needed a timer was SOL, though. The 2.5, as a shared setup, does not use the same logic and works to guarantee duty cycle, not phase relationships.

Wouldn't the low frequency cause weird optical effects at low values, no matter what? Is the 800hz immutable? Can you go to 4khz?

Seems like a LED driver board issue, honestly. I would have thought that having different high current sources coming on at different times, vs. sync'd, would be better for most boards. The high current power supply instantaneous delta-I will be worse with more pins changing at the same time, no? And the integral over time still has the same percent on for all channels. With all PWM phase aligned don't you also increase the harmonics noise you're injecting, too?

@joba-1
Copy link

joba-1 commented Feb 2, 2020

you are right with delta-I being more gentle to the power source and less harmonic noise with unsynced pwm vs all start at the same time.
But it can (and apparently does) cause flicker, because sometimes, when desynced, it is brighter.
Optimal solution would probably be a predictive desync (always the same).

@s-hadinger
Copy link
Contributor Author

I agree that having the pulses with phase differences is good for the power supply. The flickering is caused by the fact that the phase difference between channels is changing over time.

I tried a simple patch that seem to keep phases locked with each others:

In core_esp8266_waveform.cpp, timer1Interrupt():

Before


        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);
          }

After

        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 + 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 + cyclesToGo;
            nextEventCycles = min_u32(nextEventCycles, min_u32(wave->nextTimeLowCycles + cyclesToGo, 1));
          }

Basically if the NMI arrives late and cyclesToGo is significantly negative, compensate the calculation of the next edge. This means that if the NMI is late, only one pulse will be shortened but the relative phases will remain the same.

I tried with the same LED drivers, and the flickering seems gone.

@Tech-TX
Copy link
Contributor

Tech-TX commented Feb 2, 2020

I'm with you on this one, Earle. Having 3 PWM outputs 120 degrees apart would be easier on a cheap switching PSU than having them all in sync. At 880Hz frequency your eyes can't perceive a phase shift, so it shouldn't be causing a visual flicker, and if it is then boost the PWM frequency up over 1KHz. 1ms (1KHz) is about the limit that you can discern under low-light conditions as 'flicker'; at higher frequencies or intensities the eye won't see it, and 880Hz is just below that borderline.

Reading that other thread he linked, they don't have any idea what's causing the flicker, and this is the only thing they've noticed so far. They're not measuring LED current to see what's going on, nor are they measuring lux, so there's no way to correlate this to 'flicker'.

What he failed to mention was what someone else noted:

The PWM is controlled by the SDK. Unlikely that this causes the flicker. As far as I understand you see the flicker only during dimming up and down. In this case the tasmota way how to change the pwm could be an issue. There a recent changes to the code to make this much smoother with a 100Hz instead of 20Hz change process. Anyhow is this really a flickering or is the dim down process not as smooth as you expect?

The duty cycle step size at low intensities is important; you can see small shifts in intensity at low levels that's masked at higher intensity by the eye. Ideally you want something with a log or semi-log duty cycle change at low duty cycle (smaller step increments for low levels). That's beyond the scope at the moment unless someone wants to write one. PRs are always welcomed.

I can't tell from that thread what LED type they're having issues with, and it matters. LEDs are not identical in response. I'm purely guessing it's 5050 strips. Also no mention anywhere in that thread if it does the same flicker with a 12V battery, eliminating the PSU from the equation. They haven't done anything more than identify a problem. Picking the core PWM as the culprit is wildly premature.

There's no MCVE here so I can't reproduce the problem. No way am I going to attempt loading that Tasmota stack, as it could be causing the 'flicker' as well as the core PWM could. I have a lab-grade Hamamatsu photodiode (somewhere) that should be able to detect flicker, but I'd have to build a preamp for it; it's been ~15 years since I used it last. It's stored protected, so it likely still works.
Hamamatsu S2281

s-hadinger added a commit to s-hadinger/Arduino that referenced this issue Feb 2, 2020
See esp8266#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.
@s-hadinger
Copy link
Contributor Author

s-hadinger commented Feb 2, 2020

Just to make sure we are not confusing issues. There has been a stuttering issue when fading on Tasmota, this has been fixed. The current flickering issue is when LED are in steady state and is not linked to PWM frequency (880Hz is not visible). The flickering is due to phase change between 3 PWM channels, and power supplies deliver different currents to LEDs whether the pulses are at the same time or spread in time.

To be specific, the flickering is due to continuous change in phases - no calls made to analogWrite(). You only can see the result with a scope.

The explanation, is that when cyclesToGo < 0 detects that the next edge time is passed, and the next edge duration is taken from now. But if the interrupt arrives late, the next pulse is progressively dephased by -cyclesToGo time. My proposed fix is just to resync the next pulse by compensating the actual delay cyclesToGo.

Hence wave->nextServiceCycle = now + wave->nextTimeHighCycles + cyclesToGo;

I proposed PR #7057 to solve the phase issue. I also added a safeguard making sure that if the delay goes over the length of a full PWM cycle, we ignore the lost PWM cycles.

cyclesToGo = -((-cyclesToGo) % (wave->nextTimeHighCycles + wave->nextTimeLowCycles));

(The double minus is to avoid negative modulus issues)

This patch has been tested by 4 people from Tasmota contributors, and they all confirm the flickering is gone.

I hope it is clear and can be integrated in Arduino Core.

@dok-net
Copy link
Contributor

dok-net commented Feb 5, 2020

Dear @s-hadinger, I've a PR pending that appears to pre-date your issue report and fix, plus is much more comprehensive, I would like to urge you to test the effects of my changes: #7022
I am aware that the much larger change may seem discouraging and bears more risk, but our PRs open merging conflicts, which I would of course like to avoid.
Also, naturally I don't want to break your use case, taking the opportunity to have you check via oscilloscope is also quite attractive, I personally had to resort to my ears and check the Tone output results :-)

@TD-er
Copy link
Contributor

TD-er commented Feb 5, 2020

I personally had to resort to my ears and check the Tone output results :-)

That's also quite effective to detect phase shifts.
Just place some piezo elements in anti-phase next to eachother and you should hear nothing if their phases line up. If the phase experiences jitter, you should hear it.

@lassivv
Copy link

lassivv commented Mar 2, 2020

Have just same problems than @s-hadinger . Over year ago other rgb strip project (with tasmota and nodemcu) and now have same problems yesterday when i try to do new RGBW strip light.

Here little video to give problem more information. Problem is that when you have static color rgbw strip is flickering thats very easily to recognize. Dark room most more easier see that problem, not know is that seems ok video. Video have manual explosure etc settings and up corner see lab power supply and see how much current is changing static color setting. Try couple different power supplies and battery powered problem is same all of them.

Here video:
https://youtu.be/Hqb7E9mPSAE

@dok-net
Copy link
Contributor

dok-net commented Mar 8, 2020

Here's an MCVE that I have built based on @Tech-TX's input.

#if !defined(D6)
#define D6 (12)
#endif         

void setup() {
    analogWrite(LED_BUILTIN, 1023);
    analogWrite(D6, 512);
}

uint32_t pass = 0;

void loop()
{
    if (0 == pass++ % 100000)
    {
        analogWrite(LED_BUILTIN, 1021);
        delay(500);
        analogWrite(LED_BUILTIN, 1023);
    }
}

This sketch turns off the internal LED (analogWrite 1023), but at every 100000th iteration of loop, it dims it at 1021 for 500ms.
In master, one can clearly see, if at irregular intervals, that the brightness of the internal LED during the 500ms delay is very unsteady.
In PR #7057, there is no discernible flicker.
In latest PR #7022, there is also no visible flicker.

@devyte
Copy link
Collaborator

devyte commented Nov 22, 2020

Both #7022 and #7231 have been merged. Closing.

@devyte devyte closed this as completed Nov 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants