[PATCH v2 0/3] cpufreq: Introduce boot frequency QoS

[PATCH v2 0/3] cpufreq: Introduce boot frequency QoS

[Pierre Gondois]

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 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:
1. Create a per-policy boost_freq_req to save the boost
constraints instead of overwriting the last scaling_max_freq
constraint.

2. 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

3. 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

------------

E.g.:
On a Juno with available frequencies: 600.000, 1.000.000
Boost frequencies: 1.200.000
Using the cppc-cpufreq driver.

---
Without the patches:
# ## 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

# echo 0 > ../boost
scaling_max_freq:1000000
cpuinfo_max_freq:1000000

---
With the patches:
# ## Init
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:700000
cpuinfo_max_freq:1200000

# echo 800000 > scaling_max_freq
scaling_max_freq:800000
cpuinfo_max_freq:1200000

# echo 0 > ../boost
scaling_max_freq:800000
cpuinfo_max_freq:1000000

---

With the patches, the maximum scaling frequency requested is
conserved even though boosting is enabled/disabled.

The patches might be eligible for a stable tag.

------------

v1 -> v2:
- Don't call blocking_notifier_call_chain() when freeing boost_freq_req
- Free boost_freq_req before max_freq_req
- Removed unused 'ret' variable
- Merged patch:
Revert "cpufreq: Fix re-boost issue after hotplugging a CPU"
into:
cpufreq: Update set_boost callbacks to rely on boost_freq_req

Pierre Gondois (3):
  cpufreq: Add boost_freq_req QoS request
  cpufreq: Centralize boost freq QoS requests
  cpufreq: Update set_boost callbacks to rely on boost_freq_req

 drivers/cpufreq/amd-pstate.c   |  2 --
 drivers/cpufreq/cppc_cpufreq.c | 21 +++----------
 drivers/cpufreq/cpufreq.c      | 54 +++++++++++++++++++++++++---------
 include/linux/cpufreq.h        |  1 +
 4 files changed, 45 insertions(+), 33 deletions(-)

--
2.43.0

[PATCH v2 1/3] cpufreq: Add boost_freq_req QoS request

[Pierre Gondois]

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@arm.com>
---
 drivers/cpufreq/cpufreq.c | 28 ++++++++++++++++++++++++++++
 include/linux/cpufreq.h   |  1 +
 2 files changed, 29 insertions(+)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 852e024facc3c..942416f2741b0 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1359,6 +1359,11 @@ 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->boost_freq_req) {
+		freq_qos_remove_request(policy->boost_freq_req);
+		kfree(policy->boost_freq_req);
+	}
+
 	if (policy->max_freq_req) {
 		/*
 		 * Remove max_freq_req after sending CPUFREQ_REMOVE_POLICY
@@ -1476,6 +1481,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);
 	} else {
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;
-- 
2.43.0

Re: [PATCH v2 1/3] cpufreq: Add boost_freq_req QoS request

[zhenglifeng (A)]

On 2025/12/8 18:59, 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@arm.com>
> ---
>  drivers/cpufreq/cpufreq.c | 28 ++++++++++++++++++++++++++++
>  include/linux/cpufreq.h   |  1 +
>  2 files changed, 29 insertions(+)
> 
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 852e024facc3c..942416f2741b0 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -1359,6 +1359,11 @@ 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->boost_freq_req) {
> +		freq_qos_remove_request(policy->boost_freq_req);
> +		kfree(policy->boost_freq_req);
> +	}
> +
>  	if (policy->max_freq_req) {
>  		/*
>  		 * Remove max_freq_req after sending CPUFREQ_REMOVE_POLICY

If adding boost_freq_req fails, CPUFREQ_CREATE_POLICY notification will
never be sent but CPUFREQ_REMOVE_POLICY notification will be sent here. So
maybe something like this is better:

@@ -1365,17 +1365,28 @@ 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);
+               kfree(policy->boost_freq_req);
        }
 
+       if (policy->max_freq_req && !policy->boost_supported) {
+               /*
+                * Send CPUFREQ_REMOVE_POLICY notification here if
+                * boost_freq_req is not present.
+                */
+               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->min_freq_req);
        kfree(policy->min_freq_req);

---
It's a bit verbose, but I can't think of a better way.

> @@ -1476,6 +1481,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);
>  	} else {
> 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;

Re: [PATCH v2 1/3] cpufreq: Add boost_freq_req QoS request

[Pierre Gondois]

On 12/10/25 04:01, zhenglifeng (A) wrote:
> On 2025/12/8 18:59, 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@arm.com>
>> ---
>>   drivers/cpufreq/cpufreq.c | 28 ++++++++++++++++++++++++++++
>>   include/linux/cpufreq.h   |  1 +
>>   2 files changed, 29 insertions(+)
>>
>> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
>> index 852e024facc3c..942416f2741b0 100644
>> --- a/drivers/cpufreq/cpufreq.c
>> +++ b/drivers/cpufreq/cpufreq.c
>> @@ -1359,6 +1359,11 @@ 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->boost_freq_req) {
>> +		freq_qos_remove_request(policy->boost_freq_req);
>> +		kfree(policy->boost_freq_req);
>> +	}
>> +
>>   	if (policy->max_freq_req) {
>>   		/*
>>   		 * Remove max_freq_req after sending CPUFREQ_REMOVE_POLICY
> If adding boost_freq_req fails, CPUFREQ_CREATE_POLICY notification will
> never be sent but CPUFREQ_REMOVE_POLICY notification will be sent here. So
> maybe something like this is better:

Yes right indeed.
However cf. what you suggested in patch 3/3 I believe, it might be necessary
to always set boost_freq_req, even for drivers that don't actually 
support boost
frequencies.

This might simplify this patch.

>
> @@ -1365,17 +1365,28 @@ 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);
> +               kfree(policy->boost_freq_req);
>          }
>   
> +       if (policy->max_freq_req && !policy->boost_supported) {
> +               /*
> +                * Send CPUFREQ_REMOVE_POLICY notification here if
> +                * boost_freq_req is not present.
> +                */
> +               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->min_freq_req);
>          kfree(policy->min_freq_req);
>
> ---
> It's a bit verbose, but I can't think of a better way.
>
>> @@ -1476,6 +1481,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);
>>   	} else {
>> 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;

[PATCH v2 2/3] cpufreq: Centralize boost freq QoS requests

[Pierre Gondois]

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

Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
---
 drivers/cpufreq/cpufreq.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 942416f2741b0..65ef0fa70c388 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -603,10 +603,18 @@ 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;
+		return ret;
+	}
+
+	return 0;
 }
 
 static ssize_t store_local_boost(struct cpufreq_policy *policy,
-- 
2.43.0

Re: [PATCH v2 2/3] cpufreq: Centralize boost freq QoS requests

[Viresh Kumar]

On 08-12-25, 11:59, Pierre Gondois wrote:
> 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
> 
> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
> ---
>  drivers/cpufreq/cpufreq.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 942416f2741b0..65ef0fa70c388 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -603,10 +603,18 @@ 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;

what about calling set_boost() as well on failure to reverse prev change ?

> +		return ret;
> +	}
> +
> +	return 0;
>  }
>  
>  static ssize_t store_local_boost(struct cpufreq_policy *policy,
> -- 
> 2.43.0

-- 
viresh

Re: [PATCH v2 2/3] cpufreq: Centralize boost freq QoS requests

[Pierre Gondois]

On 1/8/26 06:10, Viresh Kumar wrote:
> On 08-12-25, 11:59, Pierre Gondois wrote:
>> 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
>>
>> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
>> ---
>>   drivers/cpufreq/cpufreq.c | 12 ++++++++++--
>>   1 file changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
>> index 942416f2741b0..65ef0fa70c388 100644
>> --- a/drivers/cpufreq/cpufreq.c
>> +++ b/drivers/cpufreq/cpufreq.c
>> @@ -603,10 +603,18 @@ 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;
> what about calling set_boost() as well on failure to reverse prev change ?
Yes right indeed
>
>> +		return ret;
>> +	}
>> +
>> +	return 0;
>>   }
>>   
>>   static ssize_t store_local_boost(struct cpufreq_policy *policy,
>> -- 
>> 2.43.0

[PATCH v2 3/3] cpufreq: Update set_boost callbacks to rely on boost_freq_req

[Pierre Gondois]

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

Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
---
 drivers/cpufreq/amd-pstate.c   |  2 --
 drivers/cpufreq/cppc_cpufreq.c | 21 ++++-----------------
 drivers/cpufreq/cpufreq.c      | 14 ++------------
 3 files changed, 6 insertions(+), 31 deletions(-)

diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index b44f0f7a5ba1c..50416358a96ac 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -754,8 +754,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 e23d9abea1359..3baf7baaec371 100644
--- a/drivers/cpufreq/cppc_cpufreq.c
+++ b/drivers/cpufreq/cppc_cpufreq.c
@@ -597,21 +597,14 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy)
 	caps = &cpu_data->perf_caps;
 	policy->driver_data = cpu_data;
 
-	/*
-	 * Set min to lowest nonlinear perf to avoid any efficiency penalty (see
-	 * Section 8.4.7.1.1.5 of ACPI 6.1 spec)
-	 */
-	policy->min = cppc_perf_to_khz(caps, caps->lowest_nonlinear_perf);
-	policy->max = cppc_perf_to_khz(caps, policy->boost_enabled ?
-						caps->highest_perf : caps->nominal_perf);
-
 	/*
 	 * Set cpuinfo.min_freq to Lowest to make the full range of performance
 	 * available if userspace wants to use any perf between lowest & lowest
 	 * nonlinear perf
 	 */
 	policy->cpuinfo.min_freq = cppc_perf_to_khz(caps, caps->lowest_perf);
-	policy->cpuinfo.max_freq = policy->max;
+	policy->cpuinfo.max_freq = cppc_perf_to_khz(caps, policy->boost_enabled ?
+						caps->highest_perf : caps->nominal_perf);
 
 	policy->transition_delay_us = cppc_cpufreq_get_transition_delay_us(cpu);
 	policy->shared_type = cpu_data->shared_type;
@@ -776,17 +769,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 65ef0fa70c388..ab2def9e4d188 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1514,10 +1514,6 @@ static int cpufreq_policy_online(struct cpufreq_policy *policy,
 
 		blocking_notifier_call_chain(&cpufreq_policy_notifier_list,
 				CPUFREQ_CREATE_POLICY, policy);
-	} else {
-		ret = freq_qos_update_request(policy->max_freq_req, policy->max);
-		if (ret < 0)
-			goto out_destroy_policy;
 	}
 
 	if (cpufreq_driver->get && has_target()) {
@@ -2819,16 +2815,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);
 
-- 
2.43.0

Re: [PATCH v2 3/3] cpufreq: Update set_boost callbacks to rely on boost_freq_req

[zhenglifeng (A)]

On 2025/12/8 18:59, Pierre Gondois wrote:
> 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
> 
> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
> ---
>  drivers/cpufreq/amd-pstate.c   |  2 --
>  drivers/cpufreq/cppc_cpufreq.c | 21 ++++-----------------
>  drivers/cpufreq/cpufreq.c      | 14 ++------------
>  3 files changed, 6 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index b44f0f7a5ba1c..50416358a96ac 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -754,8 +754,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 e23d9abea1359..3baf7baaec371 100644
> --- a/drivers/cpufreq/cppc_cpufreq.c
> +++ b/drivers/cpufreq/cppc_cpufreq.c
> @@ -597,21 +597,14 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy)
>  	caps = &cpu_data->perf_caps;
>  	policy->driver_data = cpu_data;
>  
> -	/*
> -	 * Set min to lowest nonlinear perf to avoid any efficiency penalty (see
> -	 * Section 8.4.7.1.1.5 of ACPI 6.1 spec)
> -	 */
> -	policy->min = cppc_perf_to_khz(caps, caps->lowest_nonlinear_perf);
> -	policy->max = cppc_perf_to_khz(caps, policy->boost_enabled ?
> -						caps->highest_perf : caps->nominal_perf);

Why remove this?

> -
>  	/*
>  	 * Set cpuinfo.min_freq to Lowest to make the full range of performance
>  	 * available if userspace wants to use any perf between lowest & lowest
>  	 * nonlinear perf
>  	 */
>  	policy->cpuinfo.min_freq = cppc_perf_to_khz(caps, caps->lowest_perf);
> -	policy->cpuinfo.max_freq = policy->max;
> +	policy->cpuinfo.max_freq = cppc_perf_to_khz(caps, policy->boost_enabled ?
> +						caps->highest_perf : caps->nominal_perf);
>  
>  	policy->transition_delay_us = cppc_cpufreq_get_transition_delay_us(cpu);
>  	policy->shared_type = cpu_data->shared_type;
> @@ -776,17 +769,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 65ef0fa70c388..ab2def9e4d188 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -1514,10 +1514,6 @@ static int cpufreq_policy_online(struct cpufreq_policy *policy,
>  
>  		blocking_notifier_call_chain(&cpufreq_policy_notifier_list,
>  				CPUFREQ_CREATE_POLICY, policy);
> -	} else {
> -		ret = freq_qos_update_request(policy->max_freq_req, policy->max);
> -		if (ret < 0)
> -			goto out_destroy_policy;

I think boost_freq_req should be updated here, to solve the problem that
this code originally intended to solve.

>  	}
>  
>  	if (cpufreq_driver->get && has_target()) {
> @@ -2819,16 +2815,10 @@ int cpufreq_boost_set_sw(struct cpufreq_policy *policy, int state)
>  		return -ENXIO;
>  
>  	ret = cpufreq_frequency_table_cpuinfo(policy);

cpufreq_frequency_table_cpuinfo() may change policy->max. I believe this
isn't what you want.

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

Re: [PATCH v2 3/3] cpufreq: Update set_boost callbacks to rely on boost_freq_req

[Pierre Gondois]

Hello Lifeng,

Thanks for the review.
I wrote a bit of text, but IIUC you already agree with what I describe.
This might be more to be sure of what I want to do.

If you disagree with something, please let me know.

On 12/10/25 10:26, zhenglifeng (A) wrote:
> On 2025/12/8 18:59, Pierre Gondois wrote:
>> 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
>>
>> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
>> ---
>>   drivers/cpufreq/amd-pstate.c   |  2 --
>>   drivers/cpufreq/cppc_cpufreq.c | 21 ++++-----------------
>>   drivers/cpufreq/cpufreq.c      | 14 ++------------
>>   3 files changed, 6 insertions(+), 31 deletions(-)
>>
>> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
>> index b44f0f7a5ba1c..50416358a96ac 100644
>> --- a/drivers/cpufreq/amd-pstate.c
>> +++ b/drivers/cpufreq/amd-pstate.c
>> @@ -754,8 +754,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 e23d9abea1359..3baf7baaec371 100644
>> --- a/drivers/cpufreq/cppc_cpufreq.c
>> +++ b/drivers/cpufreq/cppc_cpufreq.c
>> @@ -597,21 +597,14 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy)
>>   	caps = &cpu_data->perf_caps;
>>   	policy->driver_data = cpu_data;
>>   
>> -	/*
>> -	 * Set min to lowest nonlinear perf to avoid any efficiency penalty (see
>> -	 * Section 8.4.7.1.1.5 of ACPI 6.1 spec)
>> -	 */
>> -	policy->min = cppc_perf_to_khz(caps, caps->lowest_nonlinear_perf);
>> -	policy->max = cppc_perf_to_khz(caps, policy->boost_enabled ?
>> -						caps->highest_perf : caps->nominal_perf);
> Why remove this?

This is partly a mistake.
As you suggested below (I think), policy->max should not be set directly.
It might be better to set the boost_freq_req for all cpufreq drivers, 
which should
result in setting policy->max.

>
>> -
>>   	/*
>>   	 * Set cpuinfo.min_freq to Lowest to make the full range of performance
>>   	 * available if userspace wants to use any perf between lowest & lowest
>>   	 * nonlinear perf
>>   	 */
>>   	policy->cpuinfo.min_freq = cppc_perf_to_khz(caps, caps->lowest_perf);
>> -	policy->cpuinfo.max_freq = policy->max;
>> +	policy->cpuinfo.max_freq = cppc_perf_to_khz(caps, policy->boost_enabled ?
>> +						caps->highest_perf : caps->nominal_perf);
>>   
>>   	policy->transition_delay_us = cppc_cpufreq_get_transition_delay_us(cpu);
>>   	policy->shared_type = cpu_data->shared_type;
>> @@ -776,17 +769,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 65ef0fa70c388..ab2def9e4d188 100644
>> --- a/drivers/cpufreq/cpufreq.c
>> +++ b/drivers/cpufreq/cpufreq.c
>> @@ -1514,10 +1514,6 @@ static int cpufreq_policy_online(struct cpufreq_policy *policy,
>>   
>>   		blocking_notifier_call_chain(&cpufreq_policy_notifier_list,
>>   				CPUFREQ_CREATE_POLICY, policy);
>> -	} else {
>> -		ret = freq_qos_update_request(policy->max_freq_req, policy->max);
>> -		if (ret < 0)
>> -			goto out_destroy_policy;
> I think boost_freq_req should be updated here, to solve the problem that
> this code originally intended to solve.

Yes right indeed.

>>   	}
>>   
>>   	if (cpufreq_driver->get && has_target()) {
>> @@ -2819,16 +2815,10 @@ int cpufreq_boost_set_sw(struct cpufreq_policy *policy, int state)
>>   		return -ENXIO;
>>   
>>   	ret = cpufreq_frequency_table_cpuinfo(policy);
> cpufreq_frequency_table_cpuinfo() may change policy->max. I believe this
> isn't what you want.


cpufreq_frequency_table_cpuinfo() can effectively update
policy->cpuinfo.max_freq, but directly setting policy->max should be wrong
as it bypasses the other QoS constraints on the maximal frequency.

Updates to policy->max should go through the following call chain
to be sure all constraints/notifiers are respected/called.
freq_qos_update_request()
\-freq_qos_apply()
   \-pm_qos_update_target()
     \-blocking_notifier_call_chain()
       \-cpufreq_notifier_max()
         \-handle_update()
           \-refresh_frequency_limits()
             \-cpufreq_set_policy()

FYIU, we should have:
- max_freq_req: the maximal frequency constraint as set by the user.
   It is updated whenever the user write to scaling_max_freq.
- boost_freq_req: the maximal frequency constraint as set by the
   driver. It is updated whenever boost is enabled/disabled.
- policy->cpuinfo.max_freq: the maximal frequency reachable by the driver.
   This value is used in cpufreq at various places to check frequencies
   are within valid boundaries.
- policy->max: the maximal frequency cpufreq can use. It is a resultant
   of all the QoS constraints received (from the user, boost, thermal).
   It should be updated whenever one of the QoS constraint is updated.
   It should never be set directly to avoid bypassing the QoS constraints.

Whenever a cpufreq driver is initialized, policy->max is set, but the
value is overridden whenever the user writes to scaling_max_freq.
Thus we might think it should be replaced with a max_freq_req constraint.

However if boost is enabled, the maximal frequency will be limited by
max_freq_req. So at init, cpufreq drivers should set boost_freq_req
instead (to policy->cpuinfo.max_freql).
That way, if boost is enabled, the maximal frequency available is the
boost frequency.

------

Summary:
-
policy->max should never be set directly. It should only be set through
cpufreq_set_policy(). cpufreq_set_policy() might be called indirectly
after updating a QoS constraint using freq_qos_update_request().

-
boost_freq_req should be set for all cpufreq drivers, with a default value
of policy->cpuinfo.max_freq. This represents the maximal frequency available
with/without boost.
Note: the name "boost_freq_req" might not be well chosen.

-
Any update to policy->cpuinfo.max_freq should be followed by a call to
freq_qos_update_request(policy->boost_freq_req).
This will allow to update "policy->max" with the new boost frequency.


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

Re: [PATCH v2 3/3] cpufreq: Update set_boost callbacks to rely on boost_freq_req

[zhenglifeng (A)]

On 2025/12/18 0:22, Pierre Gondois wrote:
> Hello Lifeng,
> 
> Thanks for the review.
> I wrote a bit of text, but IIUC you already agree with what I describe.
> This might be more to be sure of what I want to do.
> 
> If you disagree with something, please let me know.
> 
> On 12/10/25 10:26, zhenglifeng (A) wrote:
>> On 2025/12/8 18:59, Pierre Gondois wrote:
>>> 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
>>>
>>> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
>>> ---
>>>   drivers/cpufreq/amd-pstate.c   |  2 --
>>>   drivers/cpufreq/cppc_cpufreq.c | 21 ++++-----------------
>>>   drivers/cpufreq/cpufreq.c      | 14 ++------------
>>>   3 files changed, 6 insertions(+), 31 deletions(-)
>>>
>>> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
>>> index b44f0f7a5ba1c..50416358a96ac 100644
>>> --- a/drivers/cpufreq/amd-pstate.c
>>> +++ b/drivers/cpufreq/amd-pstate.c
>>> @@ -754,8 +754,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 e23d9abea1359..3baf7baaec371 100644
>>> --- a/drivers/cpufreq/cppc_cpufreq.c
>>> +++ b/drivers/cpufreq/cppc_cpufreq.c
>>> @@ -597,21 +597,14 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy)
>>>       caps = &cpu_data->perf_caps;
>>>       policy->driver_data = cpu_data;
>>>   -    /*
>>> -     * Set min to lowest nonlinear perf to avoid any efficiency penalty (see
>>> -     * Section 8.4.7.1.1.5 of ACPI 6.1 spec)
>>> -     */
>>> -    policy->min = cppc_perf_to_khz(caps, caps->lowest_nonlinear_perf);
>>> -    policy->max = cppc_perf_to_khz(caps, policy->boost_enabled ?
>>> -                        caps->highest_perf : caps->nominal_perf);
>> Why remove this?
> 
> This is partly a mistake.
> As you suggested below (I think), policy->max should not be set directly.
> It might be better to set the boost_freq_req for all cpufreq drivers, which should
> result in setting policy->max.
> 
>>
>>> -
>>>       /*
>>>        * Set cpuinfo.min_freq to Lowest to make the full range of performance
>>>        * available if userspace wants to use any perf between lowest & lowest
>>>        * nonlinear perf
>>>        */
>>>       policy->cpuinfo.min_freq = cppc_perf_to_khz(caps, caps->lowest_perf);
>>> -    policy->cpuinfo.max_freq = policy->max;
>>> +    policy->cpuinfo.max_freq = cppc_perf_to_khz(caps, policy->boost_enabled ?
>>> +                        caps->highest_perf : caps->nominal_perf);
>>>         policy->transition_delay_us = cppc_cpufreq_get_transition_delay_us(cpu);
>>>       policy->shared_type = cpu_data->shared_type;
>>> @@ -776,17 +769,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 65ef0fa70c388..ab2def9e4d188 100644
>>> --- a/drivers/cpufreq/cpufreq.c
>>> +++ b/drivers/cpufreq/cpufreq.c
>>> @@ -1514,10 +1514,6 @@ static int cpufreq_policy_online(struct cpufreq_policy *policy,
>>>             blocking_notifier_call_chain(&cpufreq_policy_notifier_list,
>>>                   CPUFREQ_CREATE_POLICY, policy);
>>> -    } else {
>>> -        ret = freq_qos_update_request(policy->max_freq_req, policy->max);
>>> -        if (ret < 0)
>>> -            goto out_destroy_policy;
>> I think boost_freq_req should be updated here, to solve the problem that
>> this code originally intended to solve.
> 
> Yes right indeed.
> 
>>>       }
>>>         if (cpufreq_driver->get && has_target()) {
>>> @@ -2819,16 +2815,10 @@ int cpufreq_boost_set_sw(struct cpufreq_policy *policy, int state)
>>>           return -ENXIO;
>>>         ret = cpufreq_frequency_table_cpuinfo(policy);
>> cpufreq_frequency_table_cpuinfo() may change policy->max. I believe this
>> isn't what you want.
> 
> 
> cpufreq_frequency_table_cpuinfo() can effectively update
> policy->cpuinfo.max_freq, but directly setting policy->max should be wrong
> as it bypasses the other QoS constraints on the maximal frequency.
> 
> Updates to policy->max should go through the following call chain
> to be sure all constraints/notifiers are respected/called.
> freq_qos_update_request()
> \-freq_qos_apply()
>   \-pm_qos_update_target()
>     \-blocking_notifier_call_chain()
>       \-cpufreq_notifier_max()
>         \-handle_update()
>           \-refresh_frequency_limits()
>             \-cpufreq_set_policy()
> 
> FYIU, we should have:
> - max_freq_req: the maximal frequency constraint as set by the user.
>   It is updated whenever the user write to scaling_max_freq.
> - boost_freq_req: the maximal frequency constraint as set by the
>   driver. It is updated whenever boost is enabled/disabled.
> - policy->cpuinfo.max_freq: the maximal frequency reachable by the driver.
>   This value is used in cpufreq at various places to check frequencies
>   are within valid boundaries.
> - policy->max: the maximal frequency cpufreq can use. It is a resultant
>   of all the QoS constraints received (from the user, boost, thermal).
>   It should be updated whenever one of the QoS constraint is updated.
>   It should never be set directly to avoid bypassing the QoS constraints.
> 
> Whenever a cpufreq driver is initialized, policy->max is set, but the
> value is overridden whenever the user writes to scaling_max_freq.
> Thus we might think it should be replaced with a max_freq_req constraint.
> 
> However if boost is enabled, the maximal frequency will be limited by
> max_freq_req. So at init, cpufreq drivers should set boost_freq_req
> instead (to policy->cpuinfo.max_freql).
> That way, if boost is enabled, the maximal frequency available is the
> boost frequency.
> 
> ------
> 
> Summary:
> -
> policy->max should never be set directly. It should only be set through
> cpufreq_set_policy(). cpufreq_set_policy() might be called indirectly
> after updating a QoS constraint using freq_qos_update_request().
> 
> -
> boost_freq_req should be set for all cpufreq drivers, with a default value
> of policy->cpuinfo.max_freq. This represents the maximal frequency available
> with/without boost.
> Note: the name "boost_freq_req" might not be well chosen.
> 
> -
> Any update to policy->cpuinfo.max_freq should be followed by a call to
> freq_qos_update_request(policy->boost_freq_req).
> This will allow to update "policy->max" with the new boost frequency.

Yes. I agree. So the source of the problem is that max_freq_req includes
both user-defined limits and driver-defined limits. And now you try to
separate them. That's nice. Looking forward to the next version!

At the same time, I'm curious whether a similar problem would occur with
min_freq_req if a driver existed that could change cpuinfo.min_freq in
runtime (not quite certain whether such a driver exists).

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

Re: [PATCH v2 3/3] cpufreq: Update set_boost callbacks to rely on boost_freq_req

[zhenglifeng (A)]

On 2025/12/18 0:22, Pierre Gondois wrote:
> cpufreq_frequency_table_cpuinfo() can effectively update
> policy->cpuinfo.max_freq, but directly setting policy->max should be wrong
> as it bypasses the other QoS constraints on the maximal frequency.
> 
> Updates to policy->max should go through the following call chain
> to be sure all constraints/notifiers are respected/called.
> freq_qos_update_request()
> \-freq_qos_apply()
>   \-pm_qos_update_target()
>     \-blocking_notifier_call_chain()
>       \-cpufreq_notifier_max()
>         \-handle_update()
>           \-refresh_frequency_limits()
>             \-cpufreq_set_policy()
> 
> FYIU, we should have:
> - max_freq_req: the maximal frequency constraint as set by the user.
>   It is updated whenever the user write to scaling_max_freq.
> - boost_freq_req: the maximal frequency constraint as set by the
>   driver. It is updated whenever boost is enabled/disabled.
> - policy->cpuinfo.max_freq: the maximal frequency reachable by the driver.
>   This value is used in cpufreq at various places to check frequencies
>   are within valid boundaries.
> - policy->max: the maximal frequency cpufreq can use. It is a resultant
>   of all the QoS constraints received (from the user, boost, thermal).
>   It should be updated whenever one of the QoS constraint is updated.
>   It should never be set directly to avoid bypassing the QoS constraints.
> 
> Whenever a cpufreq driver is initialized, policy->max is set, but the
> value is overridden whenever the user writes to scaling_max_freq.
> Thus we might think it should be replaced with a max_freq_req constraint.
> 
> However if boost is enabled, the maximal frequency will be limited by
> max_freq_req. So at init, cpufreq drivers should set boost_freq_req
> instead (to policy->cpuinfo.max_freql).
> That way, if boost is enabled, the maximal frequency available is the
> boost frequency.
> 
> ------
> 
> Summary:
> -
> policy->max should never be set directly. It should only be set through
> cpufreq_set_policy(). cpufreq_set_policy() might be called indirectly
> after updating a QoS constraint using freq_qos_update_request().
> 
> -
> boost_freq_req should be set for all cpufreq drivers, with a default value
> of policy->cpuinfo.max_freq. This represents the maximal frequency available
> with/without boost.
> Note: the name "boost_freq_req" might not be well chosen.
> 
> -
> Any update to policy->cpuinfo.max_freq should be followed by a call to
> freq_qos_update_request(policy->boost_freq_req).
> This will allow to update "policy->max" with the new boost frequency.
> 

Hi Pierre,

I now think we might not need to add a new QoS constraints. Calling
refresh_frequency_limits() instead of freq_qos_update_request() when
setting boost might solve your problem, since cpuinfo.max_freq is already
used to limit policy->max in cpufreq_set_policy().

What do you think?

Re: [PATCH v2 3/3] cpufreq: Update set_boost callbacks to rely on boost_freq_req

[Pierre Gondois]

Hello Lifeng,

On 12/23/25 09:15, zhenglifeng (A) wrote:
> On 2025/12/18 0:22, Pierre Gondois wrote:
>> cpufreq_frequency_table_cpuinfo() can effectively update
>> policy->cpuinfo.max_freq, but directly setting policy->max should be wrong
>> as it bypasses the other QoS constraints on the maximal frequency.
>>
>> Updates to policy->max should go through the following call chain
>> to be sure all constraints/notifiers are respected/called.
>> freq_qos_update_request()
>> \-freq_qos_apply()
>>    \-pm_qos_update_target()
>>      \-blocking_notifier_call_chain()
>>        \-cpufreq_notifier_max()
>>          \-handle_update()
>>            \-refresh_frequency_limits()
>>              \-cpufreq_set_policy()
>>
>> FYIU, we should have:
>> - max_freq_req: the maximal frequency constraint as set by the user.
>>    It is updated whenever the user write to scaling_max_freq.
>> - boost_freq_req: the maximal frequency constraint as set by the
>>    driver. It is updated whenever boost is enabled/disabled.
>> - policy->cpuinfo.max_freq: the maximal frequency reachable by the driver.
>>    This value is used in cpufreq at various places to check frequencies
>>    are within valid boundaries.
>> - policy->max: the maximal frequency cpufreq can use. It is a resultant
>>    of all the QoS constraints received (from the user, boost, thermal).
>>    It should be updated whenever one of the QoS constraint is updated.
>>    It should never be set directly to avoid bypassing the QoS constraints.
>>
>> Whenever a cpufreq driver is initialized, policy->max is set, but the
>> value is overridden whenever the user writes to scaling_max_freq.
>> Thus we might think it should be replaced with a max_freq_req constraint.
>>
>> However if boost is enabled, the maximal frequency will be limited by
>> max_freq_req. So at init, cpufreq drivers should set boost_freq_req
>> instead (to policy->cpuinfo.max_freql).
>> That way, if boost is enabled, the maximal frequency available is the
>> boost frequency.
>>
>> ------
>>
>> Summary:
>> -
>> policy->max should never be set directly. It should only be set through
>> cpufreq_set_policy(). cpufreq_set_policy() might be called indirectly
>> after updating a QoS constraint using freq_qos_update_request().
>>
>> -
>> boost_freq_req should be set for all cpufreq drivers, with a default value
>> of policy->cpuinfo.max_freq. This represents the maximal frequency available
>> with/without boost.
>> Note: the name "boost_freq_req" might not be well chosen.
>>
>> -
>> Any update to policy->cpuinfo.max_freq should be followed by a call to
>> freq_qos_update_request(policy->boost_freq_req).
>> This will allow to update "policy->max" with the new boost frequency.
>>
> Hi Pierre,
>
> I now think we might not need to add a new QoS constraints. Calling
> refresh_frequency_limits() instead of freq_qos_update_request() when
> setting boost might solve your problem, since cpuinfo.max_freq is already
> used to limit policy->max in cpufreq_set_policy().
>
> What do you think?

In:
cpufreq_set_policy()
\-cpufreq_driver->verify(&new_data)
   \-cpufreq_verify_within_cpu_limits()

the requested min/max values are clamped wrt the cpuinfo.[min|max]_freq.
However this clamping happens after the QoS constraints have been
aggregated. This means that if a CPU has:
- min = 100.000 kHz
- max = 1.000.000 kHz
- boost = 1.200.000 kHz

With boost enabled, the user requests:
- scaling_min: 1.100.000
- scaling_max: 1.200.000

If boost is disabled, we will have:
policy->min == policy->max == 1.000.000
without notifying anybody.

Ideally I assume it would be better to prevent the user from disabling
boost without first asking to update the scaling_[min|max] frequencies,
or at least detecting this case and have a warning message.

It would be possible to detect this case in cpufreq_set_policy(),
but I m not sure it would be easy to act on it easily.

Please let me know if you prefer not adding the new qos constraint,
I ll try harder not to have it if yes.

Re: [PATCH v2 3/3] cpufreq: Update set_boost callbacks to rely on boost_freq_req

[Viresh Kumar]

On 12-01-26, 16:02, Pierre Gondois wrote:
> In:
> cpufreq_set_policy()
> \-cpufreq_driver->verify(&new_data)
>   \-cpufreq_verify_within_cpu_limits()
> 
> the requested min/max values are clamped wrt the cpuinfo.[min|max]_freq.
> However this clamping happens after the QoS constraints have been
> aggregated. This means that if a CPU has:
> - min = 100.000 kHz
> - max = 1.000.000 kHz
> - boost = 1.200.000 kHz
> 
> With boost enabled, the user requests:
> - scaling_min: 1.100.000
> - scaling_max: 1.200.000
> 
> If boost is disabled, we will have:
> policy->min == policy->max == 1.000.000
> without notifying anybody.
> 
> Ideally I assume it would be better to prevent the user from disabling
> boost without first asking to update the scaling_[min|max] frequencies,
> or at least detecting this case and have a warning message.

I don't think this is a problem and doesn't really need special care.
It is the user who is disabling the boost feature, its okay to force
set to clamped values.

> Please let me know if you prefer not adding the new qos constraint,
> I ll try harder not to have it if yes.

But even with that (the issue pointed earlier not being a problem), I
think a new constraint for boost does make the code cleaner and easy
to follow.

Rafael ?

-- 
viresh

Re: [PATCH v2 3/3] cpufreq: Update set_boost callbacks to rely on boost_freq_req

[Rafael J. Wysocki]

On Tue, Jan 13, 2026 at 2:30 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> On 12-01-26, 16:02, Pierre Gondois wrote:
> > In:
> > cpufreq_set_policy()
> > \-cpufreq_driver->verify(&new_data)
> >   \-cpufreq_verify_within_cpu_limits()
> >
> > the requested min/max values are clamped wrt the cpuinfo.[min|max]_freq.
> > However this clamping happens after the QoS constraints have been
> > aggregated. This means that if a CPU has:
> > - min = 100.000 kHz
> > - max = 1.000.000 kHz
> > - boost = 1.200.000 kHz
> >
> > With boost enabled, the user requests:
> > - scaling_min: 1.100.000
> > - scaling_max: 1.200.000
> >
> > If boost is disabled, we will have:
> > policy->min == policy->max == 1.000.000
> > without notifying anybody.
> >
> > Ideally I assume it would be better to prevent the user from disabling
> > boost without first asking to update the scaling_[min|max] frequencies,
> > or at least detecting this case and have a warning message.
>
> I don't think this is a problem and doesn't really need special care.
> It is the user who is disabling the boost feature, its okay to force
> set to clamped values.
>
> > Please let me know if you prefer not adding the new qos constraint,
> > I ll try harder not to have it if yes.
>
> But even with that (the issue pointed earlier not being a problem), I
> think a new constraint for boost does make the code cleaner and easy
> to follow.
>
> Rafael ?

I agree.

Re: [PATCH v2 3/3] cpufreq: Update set_boost callbacks to rely on boost_freq_req

[zhenglifeng (A)]

Hi Pierre,

On 2026/1/13 20:20, Rafael J. Wysocki wrote:
> On Tue, Jan 13, 2026 at 2:30 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>>
>> On 12-01-26, 16:02, Pierre Gondois wrote:
>>> In:
>>> cpufreq_set_policy()
>>> \-cpufreq_driver->verify(&new_data)
>>>   \-cpufreq_verify_within_cpu_limits()
>>>
>>> the requested min/max values are clamped wrt the cpuinfo.[min|max]_freq.
>>> However this clamping happens after the QoS constraints have been
>>> aggregated. This means that if a CPU has:
>>> - min = 100.000 kHz
>>> - max = 1.000.000 kHz
>>> - boost = 1.200.000 kHz
>>>
>>> With boost enabled, the user requests:
>>> - scaling_min: 1.100.000
>>> - scaling_max: 1.200.000
>>>
>>> If boost is disabled, we will have:
>>> policy->min == policy->max == 1.000.000
>>> without notifying anybody.
>>>
>>> Ideally I assume it would be better to prevent the user from disabling
>>> boost without first asking to update the scaling_[min|max] frequencies,
>>> or at least detecting this case and have a warning message.
>>
>> I don't think this is a problem and doesn't really need special care.
>> It is the user who is disabling the boost feature, its okay to force
>> set to clamped values.
>>
>>> Please let me know if you prefer not adding the new qos constraint,
>>> I ll try harder not to have it if yes.
>>
>> But even with that (the issue pointed earlier not being a problem), I
>> think a new constraint for boost does make the code cleaner and easy
>> to follow.
>>
>> Rafael ?
> 
> I agree.
> 

An explicitly defined QoS helps make the code cleaner and easy to follow.
I agree too. Let's do it that way.

Re: [PATCH v2 0/3] cpufreq: Introduce boot frequency QoS

[Viresh Kumar]

Sorry for the delay Pierre.

s/boot/boost/ in $Subject.

On 08-12-25, 11:59, Pierre Gondois wrote:
> E.g.:
> On a Juno with available frequencies: 600.000, 1.000.000
> Boost frequencies: 1.200.000
> Using the cppc-cpufreq driver.
> 
> ---
> Without the patches:
> # ## 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
> 
> # echo 0 > ../boost
> scaling_max_freq:1000000
> cpuinfo_max_freq:1000000
> 
> ---
> With the patches:
> # ## Init
> 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:700000
> cpuinfo_max_freq:1200000
> 
> # echo 800000 > scaling_max_freq
> scaling_max_freq:800000
> cpuinfo_max_freq:1200000
> 
> # echo 0 > ../boost
> scaling_max_freq:800000
> cpuinfo_max_freq:1000000

This is rather important, please add this to 1/3's commit log.

-- 
viresh