Re: [PATCH v2] pwm: tegra: Optimize period calculation

[Thierry Reding <thierry.reding@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2236 bytes --]

On Mon, Apr 25, 2022 at 03:22:44PM +0200, Uwe Kleine-König wrote:
> Dividing by the result of a division looses precision because the result is
> rounded twice. E.g. with clk_rate = 48000000 and period = 32760033 the
> following numbers result:
> 
> 	rate = pc->clk_rate >> PWM_DUTY_WIDTH = 187500
> 	hz = DIV_ROUND_CLOSEST_ULL(100ULL * NSEC_PER_SEC, period_ns) = 3052
> 	rate = DIV_ROUND_CLOSEST_ULL(100ULL * rate, hz) = 6144
> 
> The exact result would be 6142.5061875 and (apart from rounding) this is
> found by using a single division. As a side effect is also a tad
> cheaper to calculate.
> 
> Also using clk_rate >> PWM_DUTY_WIDTH looses precision. Consider for
> example clk_rate = 47999999 and period = 106667:
> 
> 	mul_u64_u64_div_u64(pc->clk_rate >> PWM_DUTY_WIDTH, period_ns,
> 			    NSEC_PER_SEC) = 19
> 
> 	mul_u64_u64_div_u64(pc->clk_rate, period_ns,
> 			    NSEC_PER_SEC << PWM_DUTY_WIDTH) = 20
> 
> (The exact result is 20.000062083332033.)
> 
> With this optimizations also switch from round-closest to round-down for
> the period calculation. Given that the calculations were non-optimal for
> quite some time now with variations in both directions which nobody
> reported as a problem, this is the opportunity to align the driver's
> behavior to the requirements of new drivers. This has several upsides:
> 
>  - Implementation is easier as there are no round-nearest variants of
>    mul_u64_u64_div_u64().
>  - Requests for too small periods are now consistently refused. This was
>    kind of arbitrary before, where period_ns < min_period_ns was
>    refused, but in some cases min_period_ns isn't actually implementable
>    and then values between min_period_ns and the actual minimum were
>    rounded up to the actual minimum.
> 
> Note that the duty_cycle calculation isn't using the usual round-down
> approach yet.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> Hello,
> 
> changes since (implicit) v1: Updated changelog to explain why rate = 0
> is refused now.
> 
> Best regards
> Uwe
> 
>  drivers/pwm/pwm-tegra.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)

Applied, thanks.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]