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

Marek Vasut
2024-09-12T00:48:01Z

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.