[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1ff87fbe-6457-4ea0-918b-dbf109663dc5@arm.com>
Date: Thu, 5 Feb 2026 14:58:56 +0100
From: Pierre Gondois <pierre.gondois@....com>
To: "zhenglifeng (A)" <zhenglifeng1@...wei.com>
Cc: Jie Zhan <zhanjie9@...ilicon.com>,
Ionela Voinescu <ionela.voinescu@....com>,
Christian Loehle <christian.loehle@....com>, sumitg@...dia.com,
"Rafael J. Wysocki" <rafael@...nel.org>,
Viresh Kumar <viresh.kumar@...aro.org>, Huang Rui <ray.huang@....com>,
"Gautham R. Shenoy" <gautham.shenoy@....com>,
Mario Limonciello <mario.limonciello@....com>,
Perry Yuan <perry.yuan@....com>,
Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>,
Len Brown <lenb@...nel.org>, Saravana Kannan <saravanak@...nel.org>,
linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/6] cpufreq: Add boost_freq_req QoS request
Hello Lifeng,
Thanks for the review,
On 1/31/26 05:00, zhenglifeng (A) wrote:
> On 2026/1/26 18:18, Pierre Gondois wrote:
>> The Power Management Quality of Service (PM QoS) allows to
>> aggregate constraints from multiple entities. It is currently
>> used to manage the min/max frequency of a given policy.
>>
>> Frequency constraints can come for instance from:
>> - Thermal framework: acpi_thermal_cpufreq_init()
>> - Firmware: _PPC objects: acpi_processor_ppc_init()
>> - User: by setting policyX/scaling_[min|max]_freq
>> The minimum of the max frequency constraints is used to compute
>> the resulting maximum allowed frequency.
>>
>> When enabling boost frequencies, the same frequency request object
>> (policy->max_freq_req) as to handle requests from users is used.
>> As a result, when setting:
>> - scaling_max_freq
>> - boost
>> The last sysfs file used overwrites the request from the other
>> sysfs file.
>>
>> To avoid this, create a per-policy boost_freq_req to save the boost
>> constraints instead of overwriting the last scaling_max_freq
>> constraint.
>>
>> Signed-off-by: Pierre Gondois <pierre.gondois@....com>
>> ---
>> drivers/cpufreq/cpufreq.c | 40 ++++++++++++++++++++++++++++++++++-----
>> include/linux/cpufreq.h | 1 +
>> 2 files changed, 36 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
>> index db414c052658b..c8fb4c6656e94 100644
>> --- a/drivers/cpufreq/cpufreq.c
>> +++ b/drivers/cpufreq/cpufreq.c
>> @@ -1359,17 +1359,24 @@ static void cpufreq_policy_free(struct cpufreq_policy *policy)
>> /* Cancel any pending policy->update work before freeing the policy. */
>> cancel_work_sync(&policy->update);
>>
>> - if (policy->max_freq_req) {
>> + if (policy->max_freq_req || policy->boost_freq_req) {
>> /*
>> - * Remove max_freq_req after sending CPUFREQ_REMOVE_POLICY
>> - * notification, since CPUFREQ_CREATE_POLICY notification was
>> - * sent after adding max_freq_req earlier.
>> + * Remove max/boost _freq_req after sending CPUFREQ_REMOVE_POLICY
>> + * notification, since CPUFREQ_CREATE_POLICY notification was sent
>> + * after adding max/boost _freq_req earlier.
>> */
>> blocking_notifier_call_chain(&cpufreq_policy_notifier_list,
>> CPUFREQ_REMOVE_POLICY, policy);
> As we discussed in [1], CPUFREQ_REMOVE_POLICY notification will be sent
> here without sending CPUFREQ_CREATE_POLICY notification before if adding
> boost_freq_req fails.
>
> [1] https://lore.kernel.org/all/a615ab13-bd54-4051-ae61-2bfe8b59427e@arm.com/
Yes right indeed.
The following condition should be more correct:
if ((policy->max_freq_req && !policy->boost_supported) ||
policy->boost_freq_req) {
...
}
>> - freq_qos_remove_request(policy->max_freq_req);
>> }
>>
>> + if (policy->boost_freq_req) {
>> + freq_qos_remove_request(policy->boost_freq_req);
>> + kfree(policy->boost_freq_req);
>> + }
>> +
>> + if (policy->max_freq_req)
>> + freq_qos_remove_request(policy->max_freq_req);
>> +
> Thses two 'if's are unnecessary. It's OK to call freq_qos_remove_request
> and kfree when the QoS request is NULL.
Ok
>> freq_qos_remove_request(policy->min_freq_req);
>> kfree(policy->min_freq_req);
>>
>> @@ -1479,6 +1486,29 @@ static int cpufreq_policy_online(struct cpufreq_policy *policy,
>> goto out_destroy_policy;
>> }
>>
>> + if (policy->boost_supported) {
>> + policy->boost_freq_req = kzalloc(sizeof(*policy->boost_freq_req),
>> + GFP_KERNEL);
>> + if (!policy->boost_freq_req) {
>> + ret = -ENOMEM;
>> + goto out_destroy_policy;
>> + }
>> +
>> + ret = freq_qos_add_request(&policy->constraints,
>> + policy->boost_freq_req,
>> + FREQ_QOS_MAX,
>> + FREQ_QOS_MAX_DEFAULT_VALUE);
>> + if (ret < 0) {
>> + /*
>> + * So we don't call freq_qos_remove_request() for an
>> + * uninitialized request.
>> + */
>> + kfree(policy->boost_freq_req);
>> + policy->boost_freq_req = NULL;
>> + goto out_destroy_policy;
>> + }
>> + }
>> +
>> blocking_notifier_call_chain(&cpufreq_policy_notifier_list,
>> CPUFREQ_CREATE_POLICY, policy);
>> }
>> diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
>> index 0465d1e6f72ac..c292a6a19e4f5 100644
>> --- a/include/linux/cpufreq.h
>> +++ b/include/linux/cpufreq.h
>> @@ -81,6 +81,7 @@ struct cpufreq_policy {
>> struct freq_constraints constraints;
>> struct freq_qos_request *min_freq_req;
>> struct freq_qos_request *max_freq_req;
>> + struct freq_qos_request *boost_freq_req;
>>
>> struct cpufreq_frequency_table *freq_table;
>> enum cpufreq_table_sorting freq_table_sorted;
Powered by blists - more mailing lists