PWM glitching with analogWrite()

[dwillmore]

When running this code:

#define ledPin 30
#define delayTime 127
#define fadeMax 31

uint16_t fadeSquared;
uint16_t fadeError=0;

void setup() {
  pinMode(ledPin, OUTPUT);
  pinMode(34, OUTPUT);
  digitalWrite(34,LOW);
}

void loop() {
  for (uint16_t fadeValue = 0; fadeValue < fadeMax; fadeValue++) {
    for(uint16_t waitTime = 0; waitTime < delayTime; waitTime++) {
      fadeSquared = (fadeValue*fadeValue)+fadeError;
      fadeError = fadeSquared & 0xff;
      fadeSquared = fadeSquared >>8;
      analogWrite(ledPin, fadeSquared);
    delay(1);
    }
  }

  for (uint16_t fadeValue = fadeMax ; fadeValue >= 1; fadeValue--) {
    for(uint16_t waitTime = 0; waitTime < delayTime; waitTime++) {
      fadeSquared = (fadeValue*fadeValue)+fadeError;
      fadeError = fadeSquared & 0xff;
      fadeSquared = fadeSquared >>8;
      analogWrite(ledPin, fadeSquared);
    delay(1);
    }
  }
}

I note that the LED flickers to full brightness from time to time. The values given to analogWrite() are below the level that the flickering would indicate the PWM is set to. It seems that something in the analogWrite() function is glitching and writing a larger value to the PWM control registers. Is there some race condition in there?

Thank you!

[DeqingSun]

This is a glitch caused by hardware limitation, there may be caused by a not well designed buffer in CH552. There was FIFO mentioned in datasheet so there should be an buffer.

Here is a screenshot of the flicker you see.

And I've made another test to show this kind of issue more clearly.

  P3_3 = 1;
  PWM_DATA1 = 2;
  delay(1);
  P3_3 = 0;
  PWM_DATA1 = 1;
  delay(1);

The symptom seems to be, if you write a lower value to PWM in the PWM clock cycle it is about to fall, you get a glitch for a full period PWM.

I'm not pretty sure what is the logic behind the PWM output because I do not have a good explanation in detail why the 1ms high level got generated. But you may do 2 things:

1. change PWM_CK_SE to make PWM faster so the glitch will be less obvious.

2. doing some sync with bPWM_IE_END

[dwillmore]

Looking randomly around the internet, I see a bunch of ways to deal with it. A common one (which is pretty heavy handed) is to have a global buffer and an IRQ which coppies the global buffer into the chip when a PWM cycle is done. I'm not sure that still won't glitch for small values as well. It seems it's setting up a race between the PWM counter and the IRQ latency (possilby variable as other higher priority IRQs may be processing).

Another method is similar and they turn the PWM off when they update the match register. I'm not sure if that just stops the counting, but it only seems to matter if you're using more than one byte of the counter (we're in the 8 bit mode, I assume), so that won't help us.

I'll keep looking.

[dwillmore]

I tried disabling the counter before the manipulation of PWM_DATA# and enabling it after and it made no difference.

It thought it might work because the Atmel 8051/2 reference manual says that writes to these registers are undefined if the timer is enabled. Still sparkles.

[DeqingSun]

I guess the PWM is WCH's invention and not connected to a regular timer. I've posted a question about this glitch in their support forum.

[dwillmore]

Thank you. I asked on their discord. I don't expect to hear back until they've dug themselves out from under all the work that piled up over the recent holiday.

[dwillmore]

Wait! The PWM isn't linked to Timer2! I had assumed that from the way it's organized in the manual. That's my mistake, sorry.

It calls bit 7 of the control register "PWM cycle end or MFM buffer interrupt enabled". MFM? What? Does this come from a floppy or old HD controller? Maybe that's a mistranslation? And I see on bit 1 where it talks about a FIFO, but only in terms of resetting it and the DATA registers. I wonder if that waits for a cycle to complete? That might be an ugly workaround: set that bit and wait for it to clear and then write new DATA.

The pwm_ie_end is confusing. Does it set whenever the PWM cycle ends or only if there's an IRQ enabled? Clearing that flat can be done by writing a '1' to it or writing to DATA1 (what about DATA2?)

[dwillmore]

Turning off the output while changing the register doesn't work, FWIW.

[dwillmore]

Busy waiting on bPWM_IF_END set or clear doesn't help, either. I don't have interrupts enabled, so I wouldn't expect that to ever get set.

[dwillmore]

I need to take my board down to the scope, but I think you not ony get one glitched cycle, I think you get glitched until you write a new value. Because I decreased the clock divisor and the flashes seemed to be the same length--I'm wiring a new value roughly every 1ms. I need to verify this on the scope, though. The human eye is notoriously bad at measuring pulse length around 1ms. :)

[usbman01]

writing to PWM_DATA1 should be done only when bPWM_IF_END =1.

This should be done by irq if you do not want to waste cycles by polling bPWM_IF_END.

[dwillmore]

while(~(PWM_CTRL&bPWM_IF_END));

never exits. This is with bPWM_IE_END set or cleared.

[usbman01]

while((PWM_CTRL&bPWM_IF_END)==0); //wait unitil its 1

I think this may be fixed only in analogWrite() by a seperate interrupt function

[dwillmore]

I tried that in a previous test. It makes no difference.

[dwillmore]

@DeqingSun How do you attach functions to the PWM interrupt. The normal attach function only seems to handle the external pin interrupts.

[DeqingSun]

@dwillmore for testing purpose, edit https://github.com/DeqingSun/ch55xduino/blob/ch55xduino/ch55xduino/ch55x/cores/ch55xduino/main.c and declare your interrupt handler there. This is required by SDCC

code such as void Timer2Interrupt(void) __interrupt (INT_NO_TMR2); serves that purpose.

Then in you code you can do something like

void Timer2Interrupt(void)  __interrupt {

}

[dwillmore]

Okay, I have it working. Thank you, @DeqingSun and @usbman01.
Here's the test:

#define ledPin 34

uint8_t ledState = LOW;
volatile uint8_t intCount = 0;
uint8_t intCountOld = 0, intTemp = 0;

void PWMInterrupt(void)  __interrupt {
  intCount++;
}

void setup() {
  IE_PWMX=1;
  PWM_CTRL |= bPWM_IE_END;
  pinMode(ledPin,OUTPUT);
  pinMode(30, OUTPUT);
  digitalWrite(ledPin, ledState);
  analogWrite(30,1);
}

void loop() {
  intTemp = intCount;
  if (intTemp != intCountOld) {
    intCountOld = intTemp;
    if(ledState == LOW){
      ledState = HIGH;
    }else{
      ledState = LOW;
    }
    digitalWrite(ledPin,ledState);
  }
  delay(100);
  PWM_DATA1 = 1;
}

It works with or without the delay(), but I put it in there to make the blinking of the test signal very obvious.

So, the next step is to incorporate this into the analogWrite() function. It looks like we try to set the PWM frequency to 1KHz. Are we okay with the overhead this will introduce? We'll need two byte globals (more for the bigger chips) and a couple of byte coppies 1000 times a second. Of course, this only needs to run if the user chooses to use analogWrite(). Maybe we need a flag or some clever method to disable it if they don't really use it. There are already value==0 and value==256 exceptions in the code. We could use those to disable the PWM interrupt. Unfortunately, we can't store 256 in a byte or we could just check every time an analogWrite() of an out of bound value is set, but there's no good way to see if the other channel is also invalid.

[DeqingSun]

Thanks for the test. I'll wait for the tech support and see if this is the best way to do it.

[dwillmore]

Any news?

[DeqingSun]

It seems there is a bug even the official engineer is not willing to confirm. But they can not deny either. So I think it is a bug.

https://www.wch.cn/bbs/thread-103851-1.html

The bug I found:

If you write a lower value to PWM in the PWM clock cycle it is about to fall, you get a glitch for a full period PWM.

I might expose Timer2Interrupt in the main framework for the user to walk around the bug.

[dwillmore]

We don't want to make the analogWrite function write to a global variable and have an IRQ routine pick it up whenever a cycle completes? Either solution would be fine. I am just curious as to you you chose one over the other. I guess there's no reason we can't do both for more 'analog' pins.

On Sun, Jun 4, 2023 at 5:26 PM Deqing Sun @.***> wrote:

It seems there is a bug even the official engineer is not willing to confirm. But they can not deny either. So I think it is a bug.

https://www.wch.cn/bbs/thread-103851-1.html

The bug I found:

If you write a lower value to PWM in the PWM clock cycle it is about to fall, you get a glitch for a full period PWM.

I might expose Timer2Interrupt in the main framework for the user to walk around the bug.

— Reply to this email directly, view it on GitHub https://github.com/DeqingSun/ch55xduino/issues/135#issuecomment-1575736145, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACPEX7CTE35YB5ZK7IVFS7DXJT4SHANCNFSM6AAAAAAXU62RVE . You are receiving this because you were mentioned.Message ID: @.***>

[DeqingSun]

Because what you proposed does not solve the problem when you write 0 when the pwm is 1, that is a special case.

[dwillmore]

Because the pin needs to be set to gpio and set low?

On Sun, Jun 4, 2023, 6:18 PM Deqing Sun @.***> wrote:

Because what you proposed does not solve the problem when you write 0 when the pwm is 1, that is a special case.

— Reply to this email directly, view it on GitHub https://github.com/DeqingSun/ch55xduino/issues/135#issuecomment-1575754604, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACPEX7DM3R7RUUR4Q5CQMYDXJUCU5ANCNFSM6AAAAAAXU62RVE . You are receiving this because you were mentioned.Message ID: @.***>

[DeqingSun]

Yes, that's right. I will see what the best way to do it.

[dwillmore]

Please let me know if I can be of any further assistance. Thank you.

On Sun, Jun 4, 2023, 7:54 PM Deqing Sun @.***> wrote:

Yes, that's right. I will see what the best way to do it.

— Reply to this email directly, view it on GitHub https://github.com/DeqingSun/ch55xduino/issues/135#issuecomment-1575815982, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACPEX7AKPBUCU5BXLU7BF33XJUN3HANCNFSM6AAAAAAXU62RVE . You are receiving this because you were mentioned.Message ID: @.***>

[dwillmore]

Have you decided on a solution to this?

[DeqingSun]

I did some experiment with USB sound card recently and I got some idea.

The PWM_CK_SE was set to 93. And the PWM is actually running in a low speed. So we can clear and poll bPWM_IF_END to catch the start of the PWM cycle, and update it there. writing 0 should not be a problem as it was treated as digitalWrite.

I'll test on a read hardware these days.

[dwillmore]

I stand ready to test should you need me.

On Fri, Mar 29, 2024 at 11:39 AM Deqing Sun @.***> wrote:

I did some experiment with USB sound card recently and I got some idea.

The PWM_CK_SE was set to 93. And the PWM is actually running in a low speed. So we can clear and poll bPWM_IF_END to catch the start of the PWM cycle, and update it there. writing 0 should not be a problem as it was treated as digitalWrite.

I'll test on a read hardware these days.

— Reply to this email directly, view it on GitHub https://github.com/DeqingSun/ch55xduino/issues/135#issuecomment-2027397761, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACPEX7AQ5WSPUDOQVW7GPPTY2WDJTAVCNFSM6AAAAAAXU62RVGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMRXGM4TONZWGE . You are receiving this because you were mentioned.Message ID: @.***>

[DeqingSun]

https://github.com/DeqingSun/ch55xduino/commit/a962b9bb848081f1310fae11713ea93064624352

This seems fix the problem on myside.