Re: [PATCH] cpufreq: cap the default transition delay at 10 ms

[Shawn Guo <shawnguo2@yeah.net>]

On Wed, Sep 10, 2025 at 02:53:12PM +0800, Shawn Guo wrote:
> From: Shawn Guo <shawnguo@kernel.org>
> 
> A regression is seen with 6.6 -> 6.12 kernel upgrade on platforms where
> cpufreq-dt driver sets cpuinfo.transition_latency as CPUFREQ_ETERNAL (-1),
> due to that platform's DT doesn't provide the optional property
> 'clock-latency-ns'.  The dbs sampling_rate was 10000 us on 6.6 and
> suddently becomes 6442450 us (4294967295 / 1000 * 1.5) on 6.12 for these
> platforms, because that the 10 ms cap for transition_delay_us was
> accidentally dropped by the commits below.
> 
>   commit 37c6dccd6837 ("cpufreq: Remove LATENCY_MULTIPLIER")
>   commit a755d0e2d41b ("cpufreq: Honour transition_latency over transition_delay_us")
>   commit e13aa799c2a6 ("cpufreq: Change default transition delay to 2ms")
> 
> It slows down dbs governor's reacting to CPU loading change
> dramatically.  Also, as transition_delay_us is used by schedutil governor
> as rate_limit_us, it shows a negative impact on device idle power
> consumption, because the device gets slightly less time in the lowest OPP.
> 
> Fix the regressions by adding the 10 ms cap on transition delay back.
> 
> Cc: stable@vger.kernel.org
> Fixes: 37c6dccd6837 ("cpufreq: Remove LATENCY_MULTIPLIER")
> Signed-off-by: Shawn Guo <shawnguo@kernel.org>
> ---
>  drivers/cpufreq/cpufreq.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index fc7eace8b65b..36e0c85cb4e0 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -551,8 +551,13 @@ unsigned int cpufreq_policy_transition_delay_us(struct cpufreq_policy *policy)
>  
>  	latency = policy->cpuinfo.transition_latency / NSEC_PER_USEC;
>  	if (latency)
> -		/* Give a 50% breathing room between updates */
> -		return latency + (latency >> 1);
> +		/*
> +		 * Give a 50% breathing room between updates.
> +		 * And cap the transition delay to 10 ms for platforms
> +		 * where the latency is too high to be reasonable for
> +		 * reevaluating frequency.
> +		 */
> +		return min(latency + (latency >> 1), 10 * MSEC_PER_SEC);

I guess it's more correct to use USEC_PER_MSEC instead, even if both
have the value 1000.  Will fix in v2.

Shawn

>  
>  	return USEC_PER_MSEC;
>  }
> -- 
> 2.43.0
>