Re: [PATCH v5 1/1] pwm: imx27: workaround of the pwm output bug when decrease the duty cycle

[Marek Vasut]

On 9/11/24 11:55 PM, Frank Li wrote:

Hi,

> > > This only resolves the issue when the PWM period is longer than 2us
> > > (or <500KHz) because write register is not quick enough when PWM period is
> > > very short.
> >
> > You did mention the IPS bus is slow. Do I understand it correctly that the
> > IPS bus write takes about 1us ? Because of the PWM consumes a sample every
> > 2us and we need to write 2 samples to avoid FIFO underrun, then to safely
> > write those 2 samples, we need to be able to write 1 sample per 1 us into
> > the FIFO ?
>
> The above time is just estimated, which variance at difference platform and
> impact by other IPs. If there are pending write/read from GPIO, PWM write
> have to wait for GPIO's write finish. It actually depend on IPS bus's
> loading.
>
> <500Khz is very less possiblity that write slower than PWM's consumes.
>
> > Also, would writing more samples help with such "fast" use cases ?
> > Something like this:
> >
> > if (clkrate > 500000) {
> >    // This usleep() could use some further improvement, e.g. calculate
> >    // precise delay for the FIFO to get empty based on PWM clkrate
> >    usleep(2 * 5); // wait 2us for each of the 4 samples in FIFO and a bit
> >    // Now the FIFO is surely empty, write all four FIFO slots
> >    writel_relaxed(imx->duty_cycle, imx->mmio_base + MX3_PWMSAR);
> >    writel_relaxed(imx->duty_cycle, imx->mmio_base + MX3_PWMSAR);
> >    writel_relaxed(imx->duty_cycle, imx->mmio_base + MX3_PWMSAR);
> >    writel_relaxed(imx->duty_cycle, imx->mmio_base + MX3_PWMSAR);
>
> It can help at some possiblity, but still have problem if > 1Mhz, write
> will always less than consume.
>
> If errata happen, only 1 cycle is full high. I think it is quite less
> impact at such high frequency.
>
> We found this problem by observe a screen backlight flick when change
> ducty_cycle. I think we try fix it after a real user visible impact happen.

Indeed, I observed similar problem.

> Put code here can reduce some possiblity at certain freq range, but may
> miss-leading user the problem fixed when > 500k.

You already have good code comments, maybe expanding one would help 
clarify this issue cannot really be fully fixed with current hardware.

I think the multi-write for > 500 kHz could further improve the patch, 
but let's wait for input from others, let's see what they think.