[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <aMEkjGN9HlwURISR@dragon>
Date: Wed, 10 Sep 2025 15:11:08 +0800
From: Shawn Guo <shawnguo2@...h.net>
To: "Rafael J . Wysocki" <rafael@...nel.org>,
Viresh Kumar <viresh.kumar@...aro.org>
Cc: Qais Yousef <qyousef@...alina.io>, linux-pm@...r.kernel.org,
linux-kernel@...r.kernel.org, Shawn Guo <shawnguo@...nel.org>,
stable@...r.kernel.org
Subject: Re: [PATCH] cpufreq: cap the default transition delay at 10 ms
On Wed, Sep 10, 2025 at 02:53:12PM +0800, Shawn Guo wrote:
> From: Shawn Guo <shawnguo@...nel.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@...r.kernel.org
> Fixes: 37c6dccd6837 ("cpufreq: Remove LATENCY_MULTIPLIER")
> Signed-off-by: Shawn Guo <shawnguo@...nel.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
>
Powered by blists - more mailing lists