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

Qais Yousef
2025-09-15T10:02:11Z

On 09/15/25 15:29, Shawn Guo wrote:
> On Sun, Sep 14, 2025 at 06:43:26PM +0100, Qais Yousef wrote:
> > > > Why do you want to address the issue in the cpufreq core instead of
> > > > doing that in the cpufreq-dt driver?
> > >
> > > My intuition was to fix the regression at where the regression was
> > > introduced by recovering the code behavior.
> >
> > Isn't the right fix here is at the driver level still? We can only give drivers
> > what they ask for. If they ask for something wrong and result in something
> > wrong, it is still their fault, no?
>
> I'm not sure. The cpufreq-dt driver is following suggestion to use
> CPUFREQ_ETERNAL, which has the implication that core will figure out
> a reasonable default value for platforms where the latency is unknown.
> And that was exactly the situation before the regression. How does it
> become the fault of cpufreq-dt driver?

Rafael and Viresh would know better, but amd-pstate chooses to fallback to
specific values if cppc returned CPUFREQ_ETERNAL.

Have you tried to look why dev_pm_opp_get_max_transition_latency() returns
0 for your platform? I think this is the problem that was being masked before.

>
> > Alternatively maybe we can add special handling for CPUFREQ_ETERNAL value,
> > though I'd suggest to return 1ms (similar to the case of value being 0). Maybe
> > we can redefine CPUFREQ_ETERNAL to be 0, but not sure if this can have side
> > effects.
>
> Changing CPUFREQ_ETERNAL to 0 looks so risky to me. What about adding
> an explicit check for CPUFREQ_ETERNAL?

Yeah this is what I had in mind. I think treating CPUFREQ_ETERNAL like 0 where
we don't know the right value and end up with a sensible default makes sense to
me.

I think printing info/warn message that the driver is not specifying the actual
hardware transition delay would be helpful for admins. A driver/DT file is
likely needs to be updated.

Better hear from Rafael first to make sure it makes sense to him too.

>
> ---8<---
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index fc7eace8b65b..053f3a0288bc 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -549,11 +549,15 @@ unsigned int cpufreq_policy_transition_delay_us(struct cpufreq_policy *policy)
> if (policy->transition_delay_us)
> return policy->transition_delay_us;
>
> + if (policy->cpuinfo.transition_latency == CPUFREQ_ETERNAL)
> + goto default_delay;
> +
> latency = policy->cpuinfo.transition_latency / NSEC_PER_USEC;
> if (latency)
> /* Give a 50% breathing room between updates */
> return latency + (latency >> 1);
>
> +default_delay:
> return USEC_PER_MSEC;
> }
> EXPORT_SYMBOL_GPL(cpufreq_policy_transition_delay_us);
>
> --->8---
>
> Shawn
>