Re: [PATCH v2] cpufreq: cppc: Clamp default minimum limit to lowest_nonlinear_perf

[Pierre Gondois]

On 3/10/26 10:12, Jie Zhan wrote:
> On 3/9/2026 9:16 PM, zhangpengjie (A) wrote:
> > Hi Jie,
> >
> > On 3/5/2026 9:49 PM, Jie Zhan wrote:
> > > On 2/13/2026 6:06 PM, Pengjie Zhang wrote:
> > > > The ACPI spec defines 'lowest_nonlinear_perf' as the threshold for
> > > > linear performance scaling. Performance levels below this threshold
> > > > are typically inefficient and should not be used by default.
> > > >
> > > > Currently, the QoS minimum request is initialized to 0. This defaults
> > > I'm more curious on the original commit that overrides the policy->min set
> > > by driver, which is:
> > > 521223d8b3ec ("cpufreq: Fix initialization of min and max frequency QoS requests")
> > >
> > > The changelog says:
> > > "The min and max frequency QoS requests in the cpufreq core are initialized
> > > to whatever the current min and max frequency values are at the init time,
> > > but if any of these values change later (for example, cpuinfo.max_freq is
> > > updated by the driver), these initial request values will be limiting the
> > > CPU frequency unnecessarily unless they are changed by user space via
> > > sysfs."
> > >
> > > So, instead of doing what the patch did, what about calling
> > > freq_qos_update_request(policy->max_freq_req, xxx) when cpuinfo.max_freq is
> > > updated?
> > >
> > > Jie
> > Thanks for pointing this out.
> >
> > I think commit 521223d8b3ec intentionally changed the semantics of the
> > core min/max QoS requests so that they no longer reflect the initial
> > policy limits set by the driver. Instead, they stay at the default
> > "no constraint" values until user space (or another QoS client) updates
> > them. That makes sense for the problem described in that changelog.
> >
> > What my patch is trying to address is a slightly different issue in
> > cppc_cpufreq: the driver currently treats `lowest_nonlinear_perf` as
> > the default minimum policy floor, but with the current core semantics
> > that floor is not represented explicitly and gets lost during policy
> > initialization.
> Yeah, the current status:
> Setting 'policy->min' in driver->init() is pointless at the moment, while
> some drivers are still doing it.  The effective place to set 'policy->min'
> became driver->verify(), but a minor side effect by doing so is
> 'policy->min' can't be preserved on switching governors.
> > So I agree that calling freq_qos_update_request() when
> > cpuinfo.min_freq/max_freq changes is the right approach for stale
> > request updates, but I am not sure it addresses the question
> > for `lowest_nonlinear_perf` itself:
> I was talking about calling freq_qos_update_request() in commit
> 521223d8b3ec.
> > Should it be modeled as a hard driver constraint, or only as the
> > default minimum policy value that user space may still lower explicitly?
> >
> > My patch assumes the latter, although I agree the current implementation
> > is not the cleanest way to express it.
> >
> > Thanks,
> >          Pengjie
> Does [1] also solve this issue by chance? especially Patch 5.
> If so, I think we may follow that up and get the framework to a sensible
> shape.
> [1] https://lore.kernel.org/linux-pm/20260225084930.1692228-1-pierre.gondois@xxxxxxx/

Yes right, this should help:

+ /* + * If the driver has set policy->min or max, + * use the value as 
a QoS request. + */ + min = max(FREQ_QOS_MIN_DEFAULT_VALUE, 
policy->min); + if (policy->max) + max = min(FREQ_QOS_MAX_DEFAULT_VALUE, 
policy->max); + else + max = FREQ_QOS_MAX_DEFAULT_VALUE;

However in the patchset, policy->min is not set to the lowest non linear 
freq.,
so this would have to be done aswell.

------

It also seem preferable to set the min QOS value once at init rather that
re-computing it on each ->verify() call.


> > > > the performance floor to the absolute "Lowest Performance" state
> > > > instead of "lowest_nonlinear_perf", allowing the CPU to operate in
> > > > an inefficient range unnecessarily.
> > > >
> > > > Signed-off-by: Pengjie Zhang <zhangpengjie2@xxxxxxxxxx>
> > > > ---
> > > > Changes in v2:
> > > >     - Renamed the patch subject to better reflect the logic change.
> > > >     - Updated the commit log to clarify ACPI spec details.
> > > > Link to v1:https://lore.kernel.org/all/20260116094555.2978887-1-zhangpengjie2@xxxxxxxxxx/
> > > > ---
> > > >    drivers/cpufreq/cppc_cpufreq.c | 18 ++++++++++++++++--
> > > >    1 file changed, 16 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
> > > > index 7e8042efedd1..4a3031d9fcf4 100644
> > > > --- a/drivers/cpufreq/cppc_cpufreq.c
> > > > +++ b/drivers/cpufreq/cppc_cpufreq.c
> > > > @@ -333,9 +333,23 @@ static unsigned int cppc_cpufreq_fast_switch(struct cpufreq_policy *policy,
> > > >        return target_freq;
> > > >    }
> > > >    -static int cppc_verify_policy(struct cpufreq_policy_data *policy)
> > > > +static int cppc_verify_policy(struct cpufreq_policy_data *policy_data)
> > > >    {
> > > > -    cpufreq_verify_within_cpu_limits(policy);
> > > > +    if (policy_data->min == FREQ_QOS_MIN_DEFAULT_VALUE) {
> > > > +        struct cpufreq_policy *policy __free(put_cpufreq_policy) =
> > > > +                          cpufreq_cpu_get(policy_data->cpu);
> > > > +        struct cppc_cpudata *cpu_data;
> > > > +
> > > > +        if (!policy)
> > > > +            return -EINVAL;
> > > > +
> > > > +        cpu_data = policy->driver_data;
> > > > +        policy_data->min = cppc_perf_to_khz(&cpu_data->perf_caps,
> > > > +            cpu_data->perf_caps.lowest_nonlinear_perf);
> > > > +    }
> > > > +
> > > > +    cpufreq_verify_within_cpu_limits(policy_data);
> > > > +
> > > >        return 0;
> > > >    }
> > > >