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;
-- 
2.43.0

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;

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

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);

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

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

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

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

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

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