Re: [PATCH v6 2/4] cpufreq: Add boost_freq_req QoS request

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

On 3/18/26 03:50, zhenglifeng (A) wrote:
> On 3/17/2026 6:17 PM, 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      | 52 +++++++++++++++++++++++++++-------
>>   include/linux/cpufreq.h        |  1 +
>>   4 files changed, 44 insertions(+), 21 deletions(-)
>>
>> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
>> index c45bc98721d24..310d5938cbdf6 100644
>> --- a/drivers/cpufreq/amd-pstate.c
>> +++ b/drivers/cpufreq/amd-pstate.c
>> @@ -756,8 +756,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 9eac77c4f2944..4c46c7ea318eb 100644
>> --- a/drivers/cpufreq/cppc_cpufreq.c
>> +++ b/drivers/cpufreq/cppc_cpufreq.c
>> @@ -775,17 +775,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 db414c052658b..e4f24754df164 100644
>> --- a/drivers/cpufreq/cpufreq.c
>> +++ b/drivers/cpufreq/cpufreq.c
>> @@ -603,10 +603,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,
>> @@ -1370,6 +1379,9 @@ static void cpufreq_policy_free(struct cpufreq_policy *policy)
>>   		freq_qos_remove_request(policy->max_freq_req);
>>   	}
>>   
>> +	if (policy->boost_freq_req)
>> +		freq_qos_remove_request(policy->boost_freq_req);
>> +
>>   	freq_qos_remove_request(policy->min_freq_req);
>>   	kfree(policy->min_freq_req);
>>   
>> @@ -1439,12 +1451,15 @@ 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;
>> @@ -1479,6 +1494,27 @@ static int cpufreq_policy_online(struct cpufreq_policy *policy,
>>   			goto out_destroy_policy;
>>   		}
>>   
>> +		if (policy->boost_supported) {
>> +			policy->boost_freq_req = policy->max_freq_req + 1;
>> +
>> +			/*
>> +			 * 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;
>> +			}
>> +		}
>> +
> Something we discussed before has resurfaced here. CPUFREQ_REMOVE_POLICY
> notification will be sent without sending CPUFREQ_CREATE_POLICY
> notification before if adding boost_freq_req fails.
>
> What about adding boost_freq_req before adding min_freq_req and removing it
> after removing min_freq_req?
Yes right, this is indeed what Rafael suggested looking back at the 
messages.
I ll wait a bit before submitting a correction.


>
>>   		blocking_notifier_call_chain(&cpufreq_policy_notifier_list,
>>   				CPUFREQ_CREATE_POLICY, policy);
>>   	}
>> @@ -2782,16 +2818,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 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;