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

[Qais Yousef <qyousef@layalina.io>]

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
>