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