Re: [PATCH] leds: rgb: leds-qcom-lpg: Fix pwm resolution for Hi-Res PWMs

From: Konrad Dybcio
Date: Fri Feb 21 2025 - 14:34:23 EST


On 20.02.2025 11:31 AM, Abel Vesa wrote:
> Currently, for the high resolution PWMs, the resolution, clock,
> pre-divider and exponent are being selected based on period. Basically,
> the implementation loops over each one of these and tries to find the
> closest (higher) period based on the following formula:
>
> period * refclk
> prediv_exp = log2 -------------------------------------
> NSEC_PER_SEC * pre_div * resolution
>
> Since the resolution is power of 2, the actual period resulting is
> usually higher than what the resolution allows. That's why the duty
> cycle requested needs to be capped to the maximum value allowed by the
> resolution (known as PWM size).
>
> Here is an example of how this can happen:
>
> For a requested period of 5000000, the best clock is 19.2MHz, the best
> prediv is 5, the best exponent is 6 and the best resolution is 256.
>
> Then, the pwm value is determined based on requested period and duty
> cycle, best prediv, best exponent and best clock, using the following
> formula:
>
> duty * refclk
> pwm_value = ----------------------------------------------
> NSEC_PER_SEC * prediv * (1 << prediv_exp)
>
> So in this specific scenario:
>
> (5000000 * 19200000) / (1000000000 * 5 * (1 << 64)) = 300
>
> With a resolution of 8 bits, this pwm value obviously goes over.
>
> Therefore, the max pwm value allowed needs to be 255.
>
> If not, the PMIC internal logic will only value that is under the set PWM
> size, resulting in a wrapped around PWM value.
>
> This has been observed on Lenovo Thinkpad T14s Gen6 (LCD panel version)
> which uses one of the PMK8550 to control the LCD backlight.
>
> Fix the value of the PWM by capping to a max based on the chosen
> resolution (PWM size).
>
> Cc: stable@xxxxxxxxxxxxxxx # 6.4
> Fixes: b00d2ed37617 ("leds: rgb: leds-qcom-lpg: Add support for high resolution PWM")
> Signed-off-by: Abel Vesa <abel.vesa@xxxxxxxxxx>
> ---

Maybe Anjelique would know better, but the computer tells me PMK8550 has
a 1*4*(not 15)-bit PWM controller.. I don't know if it's related, but
something to keep in mind

Konrad