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

[Pierre Gondois <pierre.gondois@arm.com>]

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@arm.com/

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@huawei.com>
>>>> ---
>>>> 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@huawei.com/
>>>> ---
>>>>    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;
>>>>    }
>>>>