* Re: [PATCH v7 2/2] cpufreq: Add boost_freq_req QoS request
2026-03-25 16:52 ` [PATCH v7 2/2] cpufreq: Add boost_freq_req QoS request Pierre Gondois
@ 2026-03-26 5:03 ` Viresh Kumar
2026-03-26 8:10 ` Pierre Gondois
2026-03-26 8:18 ` Zhongqiu Han
` (2 subsequent siblings)
3 siblings, 1 reply; 14+ messages in thread
From: Viresh Kumar @ 2026-03-26 5:03 UTC (permalink / raw)
To: Pierre Gondois
Cc: linux-kernel, Lifeng Zheng, Huang Rui, Gautham R. Shenoy,
Mario Limonciello, Perry Yuan, Rafael J. Wysocki, linux-pm
On 25-03-26, 17:52, Pierre Gondois wrote:
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> @@ -1445,18 +1456,42 @@ static int cpufreq_policy_online(struct cpufreq_policy *policy,
> cpumask_and(policy->cpus, policy->cpus, cpu_online_mask);
>
> if (new_policy) {
> + unsigned int req_nr;
Maybe rename to `count` ?
> for_each_cpu(j, policy->related_cpus) {
> per_cpu(cpufreq_cpu_data, j) = policy;
> add_cpu_dev_symlink(policy, j, get_cpu_device(j));
> }
>
> - policy->min_freq_req = kzalloc(2 * sizeof(*policy->min_freq_req),
> + req_nr = policy->boost_supported ? 3 : 2;
> + policy->min_freq_req = kzalloc(req_nr * sizeof(*policy->min_freq_req),
> GFP_KERNEL);
> if (!policy->min_freq_req) {
> ret = -ENOMEM;
> goto out_destroy_policy;
> }
>
> + if (policy->boost_supported) {
I would rather add this last: min -> max -> boost.
And change the code in free as:
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 58e3839a2140..7f5d18da78c6 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1358,17 +1358,18 @@ 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->boost_freq_req) {
/*
- * Remove max_freq_req after sending CPUFREQ_REMOVE_POLICY
+ * Remove boost_freq_req after sending CPUFREQ_REMOVE_POLICY
* notification, since CPUFREQ_CREATE_POLICY notification was
- * sent after adding max_freq_req earlier.
+ * sent after adding boost_freq_req earlier.
*/
blocking_notifier_call_chain(&cpufreq_policy_notifier_list,
CPUFREQ_REMOVE_POLICY, policy);
- freq_qos_remove_request(policy->max_freq_req);
+ freq_qos_remove_request(policy->boost_freq_req);
}
+ freq_qos_remove_request(policy->max_freq_req);
freq_qos_remove_request(policy->min_freq_req);
kfree(policy->min_freq_req);
> + policy->boost_freq_req = policy->min_freq_req + 2;
> +
> + /*
> + * If boost is supported,
> + * init the constraint with cpuinfo.max_freq.
> + */
Don't need a comment for obvious code ?
> + ret = freq_qos_add_request(&policy->constraints,
> + policy->boost_freq_req,
> + FREQ_QOS_MAX,
> + policy->cpuinfo.max_freq);
> + if (ret < 0) {
> + /*
> + * So we don't call freq_qos_remove_request() for an
> + * uninitialized request.
> + */
Actually we are calling freq_qos_remove_request() for NULL values. This comment
and other exiting ones like this can be removed I guess. They aren't adding much
value.
--
viresh
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v7 2/2] cpufreq: Add boost_freq_req QoS request
2026-03-26 5:03 ` Viresh Kumar
@ 2026-03-26 8:10 ` Pierre Gondois
2026-03-26 8:40 ` Viresh Kumar
0 siblings, 1 reply; 14+ messages in thread
From: Pierre Gondois @ 2026-03-26 8:10 UTC (permalink / raw)
To: Viresh Kumar
Cc: linux-kernel, Lifeng Zheng, Huang Rui, Gautham R. Shenoy,
Mario Limonciello, Perry Yuan, Rafael J. Wysocki, linux-pm
On 3/26/26 06:03, Viresh Kumar wrote:
> On 25-03-26, 17:52, Pierre Gondois wrote:
>> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
>> @@ -1445,18 +1456,42 @@ static int cpufreq_policy_online(struct cpufreq_policy *policy,
>> cpumask_and(policy->cpus, policy->cpus, cpu_online_mask);
>>
>> if (new_policy) {
>> + unsigned int req_nr;
> Maybe rename to `count` ?
Ok
>
>> for_each_cpu(j, policy->related_cpus) {
>> per_cpu(cpufreq_cpu_data, j) = policy;
>> add_cpu_dev_symlink(policy, j, get_cpu_device(j));
>> }
>>
>> - policy->min_freq_req = kzalloc(2 * sizeof(*policy->min_freq_req),
>> + req_nr = policy->boost_supported ? 3 : 2;
>> + policy->min_freq_req = kzalloc(req_nr * sizeof(*policy->min_freq_req),
>> GFP_KERNEL);
>> if (!policy->min_freq_req) {
>> ret = -ENOMEM;
>> goto out_destroy_policy;
>> }
>>
>> + if (policy->boost_supported) {
> I would rather add this last: min -> max -> boost.
>
> And change the code in free as:
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 58e3839a2140..7f5d18da78c6 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -1358,17 +1358,18 @@ 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->boost_freq_req) {
> /*
> - * Remove max_freq_req after sending CPUFREQ_REMOVE_POLICY
> + * Remove boost_freq_req after sending CPUFREQ_REMOVE_POLICY
> * notification, since CPUFREQ_CREATE_POLICY notification was
> - * sent after adding max_freq_req earlier.
> + * sent after adding boost_freq_req earlier.
> */
> blocking_notifier_call_chain(&cpufreq_policy_notifier_list,
> CPUFREQ_REMOVE_POLICY, policy);
> - freq_qos_remove_request(policy->max_freq_req);
> + freq_qos_remove_request(policy->boost_freq_req);
> }
>
> + freq_qos_remove_request(policy->max_freq_req);
> freq_qos_remove_request(policy->min_freq_req);
> kfree(policy->min_freq_req);
>
>
I thought there was an issue by adding boost last:
not all policies support boost frequencies (and thus requests).
So blocking_notifier_call_chain() should be called if:
+ if ((policy->max_freq_req && !policy->boost_supported) || +
policy->boost_freq_req) {
This was in an earlier version, but Rafael and Lifeng suggested
to add boost_freq_req first to simplify the logic:
https://lore.kernel.org/all/20260225084930.1692228-3-pierre.gondois@arm.com/
>> + policy->boost_freq_req = policy->min_freq_req + 2;
>> +
>> + /*
>> + * If boost is supported,
>> + * init the constraint with cpuinfo.max_freq.
>> + */
> Don't need a comment for obvious code ?
Ok
>
>> + ret = freq_qos_add_request(&policy->constraints,
>> + policy->boost_freq_req,
>> + FREQ_QOS_MAX,
>> + policy->cpuinfo.max_freq);
>> + if (ret < 0) {
>> + /*
>> + * So we don't call freq_qos_remove_request() for an
>> + * uninitialized request.
>> + */
> Actually we are calling freq_qos_remove_request() for NULL values. This comment
> and other exiting ones like this can be removed I guess. They aren't adding much
> value.
>
Ok
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v7 2/2] cpufreq: Add boost_freq_req QoS request
2026-03-26 8:10 ` Pierre Gondois
@ 2026-03-26 8:40 ` Viresh Kumar
0 siblings, 0 replies; 14+ messages in thread
From: Viresh Kumar @ 2026-03-26 8:40 UTC (permalink / raw)
To: Pierre Gondois
Cc: linux-kernel, Lifeng Zheng, Huang Rui, Gautham R. Shenoy,
Mario Limonciello, Perry Yuan, Rafael J. Wysocki, linux-pm
On 26-03-26, 09:10, Pierre Gondois wrote:
> I thought there was an issue by adding boost last:
> not all policies support boost frequencies (and thus requests).
>
> So blocking_notifier_call_chain() should be called if:
>
> + if ((policy->max_freq_req && !policy->boost_supported) || +
> policy->boost_freq_req) {
>
> This was in an earlier version, but Rafael and Lifeng suggested
> to add boost_freq_req first to simplify the logic:
>
> https://lore.kernel.org/all/20260225084930.1692228-3-pierre.gondois@arm.com/
Makes sense.
--
viresh
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v7 2/2] cpufreq: Add boost_freq_req QoS request
2026-03-25 16:52 ` [PATCH v7 2/2] cpufreq: Add boost_freq_req QoS request Pierre Gondois
2026-03-26 5:03 ` Viresh Kumar
@ 2026-03-26 8:18 ` Zhongqiu Han
2026-03-26 8:50 ` Viresh Kumar
2026-03-26 8:48 ` Viresh Kumar
2026-03-26 13:09 ` zhenglifeng (A)
3 siblings, 1 reply; 14+ messages in thread
From: Zhongqiu Han @ 2026-03-26 8:18 UTC (permalink / raw)
To: Pierre Gondois, linux-kernel
Cc: Lifeng Zheng, Huang Rui, Gautham R. Shenoy, Mario Limonciello,
Perry Yuan, Rafael J. Wysocki, Viresh Kumar, linux-pm,
zhongqiu.han
On 3/26/2026 12:52 AM, 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.
>
> policy_set_boost() calls the cpufreq set_boost callback.
> Update the newly added boost_freq_req request from there:
> - whenever boost is toggled
> - to cover all possible paths
>
> In the existing .set_boost() callbacks:
> - Don't update policy->max as this is done through the qos notifier
> cpufreq_notifier_max() which calls cpufreq_set_policy().
> - Remove freq_qos_update_request() calls as the qos request is now
> done in policy_set_boost() and updates the new boost_freq_req
>
> $ ## Init state
> scaling_max_freq:1000000
> cpuinfo_max_freq:1000000
>
> $ echo 700000 > scaling_max_freq
> scaling_max_freq:700000
> cpuinfo_max_freq:1000000
>
> $ echo 1 > ../boost
> scaling_max_freq:1200000
> cpuinfo_max_freq:1200000
>
> $ echo 800000 > scaling_max_freq
> scaling_max_freq:800000
> cpuinfo_max_freq:1200000
>
> $ ## Final step:
> $ ## Without the patches:
> $ echo 0 > ../boost
> scaling_max_freq:1000000
> cpuinfo_max_freq:1000000
>
> $ ## With the patches:
> $ echo 0 > ../boost
> scaling_max_freq:800000
> cpuinfo_max_freq:1000000
>
> Note:
> cpufreq_frequency_table_cpuinfo() updates policy->min
> and max from:
> A.
> cpufreq_boost_set_sw()
> \-cpufreq_frequency_table_cpuinfo()
> B.
> cpufreq_policy_online()
> \-cpufreq_table_validate_and_sort()
> \-cpufreq_frequency_table_cpuinfo()
> Keep these updates as some drivers expect policy->min and
> max to be set through B.
>
> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
> ---
> drivers/cpufreq/amd-pstate.c | 2 --
> drivers/cpufreq/cppc_cpufreq.c | 10 ++-----
> drivers/cpufreq/cpufreq.c | 51 ++++++++++++++++++++++++++--------
> include/linux/cpufreq.h | 1 +
> 4 files changed, 43 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 5aa9fcd80cf51..d0675d6a19fe1 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -769,8 +769,6 @@ static int amd_pstate_cpu_boost_update(struct cpufreq_policy *policy, bool on)
> else if (policy->cpuinfo.max_freq > nominal_freq)
> policy->cpuinfo.max_freq = nominal_freq;
>
> - policy->max = policy->cpuinfo.max_freq;
> -
> if (cppc_state == AMD_PSTATE_PASSIVE) {
> ret = freq_qos_update_request(&cpudata->req[1], policy->cpuinfo.max_freq);
> if (ret < 0)
> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
> index 011f35cb47b94..f4f574fbe547b 100644
> --- a/drivers/cpufreq/cppc_cpufreq.c
> +++ b/drivers/cpufreq/cppc_cpufreq.c
> @@ -807,17 +807,11 @@ static int cppc_cpufreq_set_boost(struct cpufreq_policy *policy, int state)
> {
> struct cppc_cpudata *cpu_data = policy->driver_data;
> struct cppc_perf_caps *caps = &cpu_data->perf_caps;
> - int ret;
>
> if (state)
> - policy->max = cppc_perf_to_khz(caps, caps->highest_perf);
> + policy->cpuinfo.max_freq = cppc_perf_to_khz(caps, caps->highest_perf);
> else
> - policy->max = cppc_perf_to_khz(caps, caps->nominal_perf);
> - policy->cpuinfo.max_freq = policy->max;
> -
> - ret = freq_qos_update_request(policy->max_freq_req, policy->max);
> - if (ret < 0)
> - return ret;
> + policy->cpuinfo.max_freq = cppc_perf_to_khz(caps, caps->nominal_perf);
>
> return 0;
> }
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 5757f12633d16..947ed87cf8d76 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -609,10 +609,19 @@ static int policy_set_boost(struct cpufreq_policy *policy, bool enable)
> policy->boost_enabled = enable;
>
> ret = cpufreq_driver->set_boost(policy, enable);
> - if (ret)
> + if (ret) {
> policy->boost_enabled = !policy->boost_enabled;
> + return ret;
> + }
>
> - return ret;
> + ret = freq_qos_update_request(policy->boost_freq_req, policy->cpuinfo.max_freq);
Hello Pierre,
Would it be reasonable to add a NULL check for policy->boost_freq_req in
policy_set_boost() before calling freq_qos_update_request(), even though
callers already check policy->boost_supported? Thanks
> + if (ret < 0) {
> + policy->boost_enabled = !policy->boost_enabled;
> + cpufreq_driver->set_boost(policy, policy->boost_enabled);
> + return ret;
> + }
> +
> + return 0;
> }
>
> static ssize_t store_local_boost(struct cpufreq_policy *policy,
> @@ -1377,6 +1386,8 @@ static void cpufreq_policy_free(struct cpufreq_policy *policy)
> }
>
> freq_qos_remove_request(policy->min_freq_req);
> + if (policy->boost_freq_req)
> + freq_qos_remove_request(policy->boost_freq_req);
> kfree(policy->min_freq_req);
>
> cpufreq_policy_put_kobj(policy);
> @@ -1445,18 +1456,42 @@ static int cpufreq_policy_online(struct cpufreq_policy *policy,
> cpumask_and(policy->cpus, policy->cpus, cpu_online_mask);
>
> if (new_policy) {
> + unsigned int req_nr;
> +
> for_each_cpu(j, policy->related_cpus) {
> per_cpu(cpufreq_cpu_data, j) = policy;
> add_cpu_dev_symlink(policy, j, get_cpu_device(j));
> }
>
> - policy->min_freq_req = kzalloc(2 * sizeof(*policy->min_freq_req),
> + req_nr = policy->boost_supported ? 3 : 2;
> + policy->min_freq_req = kzalloc(req_nr * sizeof(*policy->min_freq_req),
> GFP_KERNEL);
> if (!policy->min_freq_req) {
> ret = -ENOMEM;
> goto out_destroy_policy;
> }
>
> + if (policy->boost_supported) {
> + policy->boost_freq_req = policy->min_freq_req + 2;
> +
> + /*
> + * If boost is supported,
> + * init the constraint with cpuinfo.max_freq.
> + */
> + ret = freq_qos_add_request(&policy->constraints,
> + policy->boost_freq_req,
> + FREQ_QOS_MAX,
> + policy->cpuinfo.max_freq);
> + if (ret < 0) {
> + /*
> + * So we don't call freq_qos_remove_request() for an
> + * uninitialized request.
> + */
> + policy->boost_freq_req = NULL;
> + goto out_destroy_policy;
> + }
> + }
> +
> ret = freq_qos_add_request(&policy->constraints,
> policy->min_freq_req, FREQ_QOS_MIN,
> FREQ_QOS_MIN_DEFAULT_VALUE);
> @@ -2788,16 +2823,10 @@ int cpufreq_boost_set_sw(struct cpufreq_policy *policy, int state)
> return -ENXIO;
>
> ret = cpufreq_frequency_table_cpuinfo(policy);
> - if (ret) {
> + if (ret)
> pr_err("%s: Policy frequency update failed\n", __func__);
> - return ret;
> - }
> -
> - ret = freq_qos_update_request(policy->max_freq_req, policy->max);
> - if (ret < 0)
> - return ret;
>
> - return 0;
> + return ret;
> }
> EXPORT_SYMBOL_GPL(cpufreq_boost_set_sw);
>
> diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
> index cc894fc389710..89157e367eefa 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;
--
Thx and BRs,
Zhongqiu Han
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v7 2/2] cpufreq: Add boost_freq_req QoS request
2026-03-26 8:18 ` Zhongqiu Han
@ 2026-03-26 8:50 ` Viresh Kumar
2026-03-26 9:12 ` Zhongqiu Han
0 siblings, 1 reply; 14+ messages in thread
From: Viresh Kumar @ 2026-03-26 8:50 UTC (permalink / raw)
To: Zhongqiu Han
Cc: Pierre Gondois, linux-kernel, Lifeng Zheng, Huang Rui,
Gautham R. Shenoy, Mario Limonciello, Perry Yuan,
Rafael J. Wysocki, linux-pm
On 26-03-26, 16:18, Zhongqiu Han wrote:
> Would it be reasonable to add a NULL check for policy->boost_freq_req in
> policy_set_boost() before calling freq_qos_update_request(), even though
> callers already check policy->boost_supported? Thanks
Not required. policy_set_boost() should only be called if boost is supported.
--
viresh
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v7 2/2] cpufreq: Add boost_freq_req QoS request
2026-03-26 8:50 ` Viresh Kumar
@ 2026-03-26 9:12 ` Zhongqiu Han
0 siblings, 0 replies; 14+ messages in thread
From: Zhongqiu Han @ 2026-03-26 9:12 UTC (permalink / raw)
To: Viresh Kumar
Cc: Pierre Gondois, linux-kernel, Lifeng Zheng, Huang Rui,
Gautham R. Shenoy, Mario Limonciello, Perry Yuan,
Rafael J. Wysocki, linux-pm, zhongqiu.han
On 3/26/2026 4:50 PM, Viresh Kumar wrote:
> On 26-03-26, 16:18, Zhongqiu Han wrote:
>> Would it be reasonable to add a NULL check for policy->boost_freq_req in
>> policy_set_boost() before calling freq_qos_update_request(), even though
>> callers already check policy->boost_supported? Thanks
>
> Not required. policy_set_boost() should only be called if boost is supported.
>
Got it, thanks — no NULL check needed since boost_supported is the
precondition.
--
Thx and BRs,
Zhongqiu Han
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v7 2/2] cpufreq: Add boost_freq_req QoS request
2026-03-25 16:52 ` [PATCH v7 2/2] cpufreq: Add boost_freq_req QoS request Pierre Gondois
2026-03-26 5:03 ` Viresh Kumar
2026-03-26 8:18 ` Zhongqiu Han
@ 2026-03-26 8:48 ` Viresh Kumar
2026-03-26 8:54 ` Pierre Gondois
2026-03-26 13:09 ` zhenglifeng (A)
3 siblings, 1 reply; 14+ messages in thread
From: Viresh Kumar @ 2026-03-26 8:48 UTC (permalink / raw)
To: Pierre Gondois
Cc: linux-kernel, Lifeng Zheng, Huang Rui, Gautham R. Shenoy,
Mario Limonciello, Perry Yuan, Rafael J. Wysocki, linux-pm
On 25-03-26, 17:52, Pierre Gondois wrote:
> @@ -1377,6 +1386,8 @@ static void cpufreq_policy_free(struct cpufreq_policy *policy)
> }
>
> freq_qos_remove_request(policy->min_freq_req);
Since this doesn't check min_freq_req (and depend on the routine to return
early), shouldn't we do the same for below one ?
> + if (policy->boost_freq_req)
> + freq_qos_remove_request(policy->boost_freq_req);
> kfree(policy->min_freq_req);
>
> cpufreq_policy_put_kobj(policy);
--
viresh
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v7 2/2] cpufreq: Add boost_freq_req QoS request
2026-03-26 8:48 ` Viresh Kumar
@ 2026-03-26 8:54 ` Pierre Gondois
0 siblings, 0 replies; 14+ messages in thread
From: Pierre Gondois @ 2026-03-26 8:54 UTC (permalink / raw)
To: Viresh Kumar
Cc: linux-kernel, Lifeng Zheng, Huang Rui, Gautham R. Shenoy,
Mario Limonciello, Perry Yuan, Rafael J. Wysocki, linux-pm
On 3/26/26 09:48, Viresh Kumar wrote:
> On 25-03-26, 17:52, Pierre Gondois wrote:
>> @@ -1377,6 +1386,8 @@ static void cpufreq_policy_free(struct cpufreq_policy *policy)
>> }
>>
>> freq_qos_remove_request(policy->min_freq_req);
> Since this doesn't check min_freq_req (and depend on the routine to return
> early), shouldn't we do the same for below one ?
Yes it is possible,
there were different views but without the check is ok aswell.
>
>> + if (policy->boost_freq_req)
>> + freq_qos_remove_request(policy->boost_freq_req);
>> kfree(policy->min_freq_req);
>>
>> cpufreq_policy_put_kobj(policy);
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v7 2/2] cpufreq: Add boost_freq_req QoS request
2026-03-25 16:52 ` [PATCH v7 2/2] cpufreq: Add boost_freq_req QoS request Pierre Gondois
` (2 preceding siblings ...)
2026-03-26 8:48 ` Viresh Kumar
@ 2026-03-26 13:09 ` zhenglifeng (A)
3 siblings, 0 replies; 14+ messages in thread
From: zhenglifeng (A) @ 2026-03-26 13:09 UTC (permalink / raw)
To: Pierre Gondois, linux-kernel
Cc: Huang Rui, Gautham R. Shenoy, Mario Limonciello, Perry Yuan,
Rafael J. Wysocki, Viresh Kumar, linux-pm
This patch looks good to me now.
Reviewed-by: Lifeng Zheng <zhenglifeng1@huawei.com>
On 3/26/2026 12:52 AM, 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.
>
> policy_set_boost() calls the cpufreq set_boost callback.
> Update the newly added boost_freq_req request from there:
> - whenever boost is toggled
> - to cover all possible paths
>
> In the existing .set_boost() callbacks:
> - Don't update policy->max as this is done through the qos notifier
> cpufreq_notifier_max() which calls cpufreq_set_policy().
> - Remove freq_qos_update_request() calls as the qos request is now
> done in policy_set_boost() and updates the new boost_freq_req
>
> $ ## Init state
> scaling_max_freq:1000000
> cpuinfo_max_freq:1000000
>
> $ echo 700000 > scaling_max_freq
> scaling_max_freq:700000
> cpuinfo_max_freq:1000000
>
> $ echo 1 > ../boost
> scaling_max_freq:1200000
> cpuinfo_max_freq:1200000
>
> $ echo 800000 > scaling_max_freq
> scaling_max_freq:800000
> cpuinfo_max_freq:1200000
>
> $ ## Final step:
> $ ## Without the patches:
> $ echo 0 > ../boost
> scaling_max_freq:1000000
> cpuinfo_max_freq:1000000
>
> $ ## With the patches:
> $ echo 0 > ../boost
> scaling_max_freq:800000
> cpuinfo_max_freq:1000000
>
> Note:
> cpufreq_frequency_table_cpuinfo() updates policy->min
> and max from:
> A.
> cpufreq_boost_set_sw()
> \-cpufreq_frequency_table_cpuinfo()
> B.
> cpufreq_policy_online()
> \-cpufreq_table_validate_and_sort()
> \-cpufreq_frequency_table_cpuinfo()
> Keep these updates as some drivers expect policy->min and
> max to be set through B.
>
> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
> ---
> drivers/cpufreq/amd-pstate.c | 2 --
> drivers/cpufreq/cppc_cpufreq.c | 10 ++-----
> drivers/cpufreq/cpufreq.c | 51 ++++++++++++++++++++++++++--------
> include/linux/cpufreq.h | 1 +
> 4 files changed, 43 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 5aa9fcd80cf51..d0675d6a19fe1 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -769,8 +769,6 @@ static int amd_pstate_cpu_boost_update(struct cpufreq_policy *policy, bool on)
> else if (policy->cpuinfo.max_freq > nominal_freq)
> policy->cpuinfo.max_freq = nominal_freq;
>
> - policy->max = policy->cpuinfo.max_freq;
> -
> if (cppc_state == AMD_PSTATE_PASSIVE) {
> ret = freq_qos_update_request(&cpudata->req[1], policy->cpuinfo.max_freq);
> if (ret < 0)
> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
> index 011f35cb47b94..f4f574fbe547b 100644
> --- a/drivers/cpufreq/cppc_cpufreq.c
> +++ b/drivers/cpufreq/cppc_cpufreq.c
> @@ -807,17 +807,11 @@ static int cppc_cpufreq_set_boost(struct cpufreq_policy *policy, int state)
> {
> struct cppc_cpudata *cpu_data = policy->driver_data;
> struct cppc_perf_caps *caps = &cpu_data->perf_caps;
> - int ret;
>
> if (state)
> - policy->max = cppc_perf_to_khz(caps, caps->highest_perf);
> + policy->cpuinfo.max_freq = cppc_perf_to_khz(caps, caps->highest_perf);
> else
> - policy->max = cppc_perf_to_khz(caps, caps->nominal_perf);
> - policy->cpuinfo.max_freq = policy->max;
> -
> - ret = freq_qos_update_request(policy->max_freq_req, policy->max);
> - if (ret < 0)
> - return ret;
> + policy->cpuinfo.max_freq = cppc_perf_to_khz(caps, caps->nominal_perf);
>
> return 0;
> }
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 5757f12633d16..947ed87cf8d76 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -609,10 +609,19 @@ static int policy_set_boost(struct cpufreq_policy *policy, bool enable)
> policy->boost_enabled = enable;
>
> ret = cpufreq_driver->set_boost(policy, enable);
> - if (ret)
> + if (ret) {
> policy->boost_enabled = !policy->boost_enabled;
> + return ret;
> + }
>
> - return ret;
> + ret = freq_qos_update_request(policy->boost_freq_req, policy->cpuinfo.max_freq);
> + if (ret < 0) {
> + policy->boost_enabled = !policy->boost_enabled;
> + cpufreq_driver->set_boost(policy, policy->boost_enabled);
> + return ret;
> + }
> +
> + return 0;
> }
>
> static ssize_t store_local_boost(struct cpufreq_policy *policy,
> @@ -1377,6 +1386,8 @@ static void cpufreq_policy_free(struct cpufreq_policy *policy)
> }
>
> freq_qos_remove_request(policy->min_freq_req);
> + if (policy->boost_freq_req)
> + freq_qos_remove_request(policy->boost_freq_req);
> kfree(policy->min_freq_req);
>
> cpufreq_policy_put_kobj(policy);
> @@ -1445,18 +1456,42 @@ static int cpufreq_policy_online(struct cpufreq_policy *policy,
> cpumask_and(policy->cpus, policy->cpus, cpu_online_mask);
>
> if (new_policy) {
> + unsigned int req_nr;
> +
> for_each_cpu(j, policy->related_cpus) {
> per_cpu(cpufreq_cpu_data, j) = policy;
> add_cpu_dev_symlink(policy, j, get_cpu_device(j));
> }
>
> - policy->min_freq_req = kzalloc(2 * sizeof(*policy->min_freq_req),
> + req_nr = policy->boost_supported ? 3 : 2;
> + policy->min_freq_req = kzalloc(req_nr * sizeof(*policy->min_freq_req),
> GFP_KERNEL);
> if (!policy->min_freq_req) {
> ret = -ENOMEM;
> goto out_destroy_policy;
> }
>
> + if (policy->boost_supported) {
> + policy->boost_freq_req = policy->min_freq_req + 2;
> +
> + /*
> + * If boost is supported,
> + * init the constraint with cpuinfo.max_freq.
> + */
> + ret = freq_qos_add_request(&policy->constraints,
> + policy->boost_freq_req,
> + FREQ_QOS_MAX,
> + policy->cpuinfo.max_freq);
> + if (ret < 0) {
> + /*
> + * So we don't call freq_qos_remove_request() for an
> + * uninitialized request.
> + */
> + policy->boost_freq_req = NULL;
> + goto out_destroy_policy;
> + }
> + }
> +
> ret = freq_qos_add_request(&policy->constraints,
> policy->min_freq_req, FREQ_QOS_MIN,
> FREQ_QOS_MIN_DEFAULT_VALUE);
> @@ -2788,16 +2823,10 @@ int cpufreq_boost_set_sw(struct cpufreq_policy *policy, int state)
> return -ENXIO;
>
> ret = cpufreq_frequency_table_cpuinfo(policy);
> - if (ret) {
> + if (ret)
> pr_err("%s: Policy frequency update failed\n", __func__);
> - return ret;
> - }
> -
> - ret = freq_qos_update_request(policy->max_freq_req, policy->max);
> - if (ret < 0)
> - return ret;
>
> - return 0;
> + return ret;
> }
> EXPORT_SYMBOL_GPL(cpufreq_boost_set_sw);
>
> diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
> index cc894fc389710..89157e367eefa 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;
^ permalink raw reply [flat|nested] 14+ messages in thread