public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1 0/4] cpufreq: Introduce boot frequency QoS
@ 2025-12-04 10:13 Pierre Gondois
  2025-12-04 10:13 ` [PATCH v1 1/4] Revert "cpufreq: Fix re-boost issue after hotplugging a CPU" Pierre Gondois
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Pierre Gondois @ 2025-12-04 10:13 UTC (permalink / raw)
  To: linux-kernel
  Cc: Christian Loehle, Ionela Voinescu, zhenglifeng1, Jie Zhan,
	Pierre Gondois, Huang Rui, Gautham R. Shenoy, Mario Limonciello,
	Perry Yuan, Rafael J. Wysocki, Viresh Kumar, linux-pm

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.

Pierre Gondois (4):
  Revert "cpufreq: Fix re-boost issue after hotplugging a CPU"
  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 | 20 +++---------
 drivers/cpufreq/cpufreq.c      | 59 ++++++++++++++++++++++++++--------
 include/linux/cpufreq.h        |  1 +
 4 files changed, 50 insertions(+), 32 deletions(-)

--
2.43.0

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v1 1/4] Revert "cpufreq: Fix re-boost issue after hotplugging a CPU"
  2025-12-04 10:13 [PATCH v1 0/4] cpufreq: Introduce boot frequency QoS Pierre Gondois
@ 2025-12-04 10:13 ` Pierre Gondois
  2025-12-04 12:09   ` zhenglifeng (A)
  2025-12-04 10:13 ` [PATCH v1 2/4] cpufreq: Add boost_freq_req QoS request Pierre Gondois
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 12+ messages in thread
From: Pierre Gondois @ 2025-12-04 10:13 UTC (permalink / raw)
  To: linux-kernel
  Cc: Christian Loehle, Ionela Voinescu, zhenglifeng1, Jie Zhan,
	Pierre Gondois, Huang Rui, Gautham R. Shenoy, Mario Limonciello,
	Perry Yuan, Rafael J. Wysocki, Viresh Kumar, linux-pm

policy->max_freq_req represents the maximum allowed frequency as
requested by the policyX/scaling_max_freq sysfs file. This request
applies to all CPUs of the policy. It is not possible to request
a per-CPU maximum frequency.

Thus, the interaction between the policy boost and scaling_max_freq
settings should be handled by adding a boost specific QoS constraint.
This will be handled in the following patches.

This reverts commit 1608f0230510489d74a2e24e47054233b7e4678a.

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

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 852e024facc3c..11b29c7dbea9e 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1478,10 +1478,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()) {
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v1 2/4] cpufreq: Add boost_freq_req QoS request
  2025-12-04 10:13 [PATCH v1 0/4] cpufreq: Introduce boot frequency QoS Pierre Gondois
  2025-12-04 10:13 ` [PATCH v1 1/4] Revert "cpufreq: Fix re-boost issue after hotplugging a CPU" Pierre Gondois
@ 2025-12-04 10:13 ` Pierre Gondois
  2025-12-04 11:50   ` zhenglifeng (A)
  2025-12-04 10:13 ` [PATCH v1 3/4] cpufreq: Centralize boost freq QoS requests Pierre Gondois
  2025-12-04 10:13 ` [PATCH v1 4/4] cpufreq: Update set_boost callbacks to rely on boost_freq_req Pierre Gondois
  3 siblings, 1 reply; 12+ messages in thread
From: Pierre Gondois @ 2025-12-04 10:13 UTC (permalink / raw)
  To: linux-kernel
  Cc: Christian Loehle, Ionela Voinescu, zhenglifeng1, Jie Zhan,
	Pierre Gondois, Huang Rui, Gautham R. Shenoy, Mario Limonciello,
	Perry Yuan, Rafael J. Wysocki, Viresh Kumar, linux-pm

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 | 35 +++++++++++++++++++++++++++++++++++
 include/linux/cpufreq.h   |  1 +
 2 files changed, 36 insertions(+)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 11b29c7dbea9e..23f64346b80f8 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1370,6 +1370,18 @@ static void cpufreq_policy_free(struct cpufreq_policy *policy)
 		freq_qos_remove_request(policy->max_freq_req);
 	}
 
+	if (policy->boost_freq_req) {
+		/*
+		 * Remove boost_freq_req after sending CPUFREQ_REMOVE_POLICY
+		 * notification, since CPUFREQ_CREATE_POLICY notification was
+		 * sent after adding boost_freq_req earlier.
+		 */
+		blocking_notifier_call_chain(&cpufreq_policy_notifier_list,
+					     CPUFREQ_REMOVE_POLICY, policy);
+		freq_qos_remove_request(policy->boost_freq_req);
+		kfree(policy->boost_freq_req);
+	}
+
 	freq_qos_remove_request(policy->min_freq_req);
 	kfree(policy->min_freq_req);
 
@@ -1476,6 +1488,29 @@ static int cpufreq_policy_online(struct cpufreq_policy *policy,
 			goto out_destroy_policy;
 		}
 
+		if (policy->boost_supported) {
+			policy->boost_freq_req = kzalloc(sizeof(*policy->boost_freq_req),
+				GFP_KERNEL);
+			if (!policy->boost_freq_req) {
+				ret = -ENOMEM;
+				goto out_destroy_policy;
+			}
+
+			ret = freq_qos_add_request(&policy->constraints,
+						   policy->boost_freq_req,
+						   FREQ_QOS_MAX,
+						   FREQ_QOS_MAX_DEFAULT_VALUE);
+			if (ret < 0) {
+				/*
+				 * So we don't call freq_qos_remove_request() for an
+				 * uninitialized request.
+				 */
+				kfree(policy->boost_freq_req);
+				policy->boost_freq_req = NULL;
+				goto out_destroy_policy;
+			}
+		}
+
 		blocking_notifier_call_chain(&cpufreq_policy_notifier_list,
 				CPUFREQ_CREATE_POLICY, policy);
 	}
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index 0465d1e6f72ac..c292a6a19e4f5 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -81,6 +81,7 @@ struct cpufreq_policy {
 	struct freq_constraints	constraints;
 	struct freq_qos_request	*min_freq_req;
 	struct freq_qos_request	*max_freq_req;
+	struct freq_qos_request *boost_freq_req;
 
 	struct cpufreq_frequency_table	*freq_table;
 	enum cpufreq_table_sorting freq_table_sorted;
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v1 3/4] cpufreq: Centralize boost freq QoS requests
  2025-12-04 10:13 [PATCH v1 0/4] cpufreq: Introduce boot frequency QoS Pierre Gondois
  2025-12-04 10:13 ` [PATCH v1 1/4] Revert "cpufreq: Fix re-boost issue after hotplugging a CPU" Pierre Gondois
  2025-12-04 10:13 ` [PATCH v1 2/4] cpufreq: Add boost_freq_req QoS request Pierre Gondois
@ 2025-12-04 10:13 ` Pierre Gondois
  2025-12-04 12:02   ` zhenglifeng (A)
  2025-12-04 10:13 ` [PATCH v1 4/4] cpufreq: Update set_boost callbacks to rely on boost_freq_req Pierre Gondois
  3 siblings, 1 reply; 12+ messages in thread
From: Pierre Gondois @ 2025-12-04 10:13 UTC (permalink / raw)
  To: linux-kernel
  Cc: Christian Loehle, Ionela Voinescu, zhenglifeng1, Jie Zhan,
	Pierre Gondois, Huang Rui, Gautham R. Shenoy, Mario Limonciello,
	Perry Yuan, Rafael J. Wysocki, Viresh Kumar, linux-pm

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 | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 23f64346b80f8..9d98b98e7981c 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -603,10 +603,16 @@ 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)
+		return ret;
+
+	return 0;
 }
 
 static ssize_t store_local_boost(struct cpufreq_policy *policy,
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v1 4/4] cpufreq: Update set_boost callbacks to rely on boost_freq_req
  2025-12-04 10:13 [PATCH v1 0/4] cpufreq: Introduce boot frequency QoS Pierre Gondois
                   ` (2 preceding siblings ...)
  2025-12-04 10:13 ` [PATCH v1 3/4] cpufreq: Centralize boost freq QoS requests Pierre Gondois
@ 2025-12-04 10:13 ` Pierre Gondois
  2025-12-05 15:16   ` kernel test robot
  3 siblings, 1 reply; 12+ messages in thread
From: Pierre Gondois @ 2025-12-04 10:13 UTC (permalink / raw)
  To: linux-kernel
  Cc: Christian Loehle, Ionela Voinescu, zhenglifeng1, Jie Zhan,
	Pierre Gondois, Huang Rui, Gautham R. Shenoy, Mario Limonciello,
	Perry Yuan, Rafael J. Wysocki, Viresh Kumar, linux-pm

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 | 20 ++++----------------
 drivers/cpufreq/cpufreq.c      | 10 ++--------
 3 files changed, 6 insertions(+), 26 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..a2f64099d39a4 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;
@@ -779,14 +772,9 @@ static int cppc_cpufreq_set_boost(struct cpufreq_policy *policy, int state)
 	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 9d98b98e7981c..c360af436f1ea 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -2820,16 +2820,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


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v1 2/4] cpufreq: Add boost_freq_req QoS request
  2025-12-04 10:13 ` [PATCH v1 2/4] cpufreq: Add boost_freq_req QoS request Pierre Gondois
@ 2025-12-04 11:50   ` zhenglifeng (A)
  2025-12-08 10:25     ` Pierre Gondois
  0 siblings, 1 reply; 12+ messages in thread
From: zhenglifeng (A) @ 2025-12-04 11:50 UTC (permalink / raw)
  To: Pierre Gondois
  Cc: linux-kernel, Christian Loehle, Ionela Voinescu, Jie Zhan,
	Huang Rui, Gautham R. Shenoy, Mario Limonciello, Perry Yuan,
	Rafael J. Wysocki, Viresh Kumar, linux-pm

On 2025/12/4 18:13, 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 | 35 +++++++++++++++++++++++++++++++++++
>  include/linux/cpufreq.h   |  1 +
>  2 files changed, 36 insertions(+)
> 
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 11b29c7dbea9e..23f64346b80f8 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -1370,6 +1370,18 @@ static void cpufreq_policy_free(struct cpufreq_policy *policy)
>  		freq_qos_remove_request(policy->max_freq_req);
>  	}
>  
> +	if (policy->boost_freq_req) {
> +		/*
> +		 * Remove boost_freq_req after sending CPUFREQ_REMOVE_POLICY
> +		 * notification, since CPUFREQ_CREATE_POLICY notification was
> +		 * sent after adding boost_freq_req earlier.
> +		 */
> +		blocking_notifier_call_chain(&cpufreq_policy_notifier_list,
> +					     CPUFREQ_REMOVE_POLICY, policy);

The CPUFREQ_REMOVE_POLICY notification is sent before removing
max_freq_req, I don't think it should be sent again here.

> +		freq_qos_remove_request(policy->boost_freq_req);
> +		kfree(policy->boost_freq_req);
> +	}
> +

I think boost_freq_req should be removed before removing max_freq_req,
since it was added after adding max_freq_req as shown below.

>  	freq_qos_remove_request(policy->min_freq_req);
>  	kfree(policy->min_freq_req);
>  
> @@ -1476,6 +1488,29 @@ static int cpufreq_policy_online(struct cpufreq_policy *policy,
>  			goto out_destroy_policy;
>  		}
>  
> +		if (policy->boost_supported) {
> +			policy->boost_freq_req = kzalloc(sizeof(*policy->boost_freq_req),
> +				GFP_KERNEL);
> +			if (!policy->boost_freq_req) {
> +				ret = -ENOMEM;
> +				goto out_destroy_policy;
> +			}
> +
> +			ret = freq_qos_add_request(&policy->constraints,
> +						   policy->boost_freq_req,
> +						   FREQ_QOS_MAX,
> +						   FREQ_QOS_MAX_DEFAULT_VALUE);
> +			if (ret < 0) {
> +				/*
> +				 * So we don't call freq_qos_remove_request() for an
> +				 * uninitialized request.
> +				 */
> +				kfree(policy->boost_freq_req);
> +				policy->boost_freq_req = NULL;
> +				goto out_destroy_policy;
> +			}
> +		}
> +
>  		blocking_notifier_call_chain(&cpufreq_policy_notifier_list,
>  				CPUFREQ_CREATE_POLICY, policy);
>  	}
> diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
> index 0465d1e6f72ac..c292a6a19e4f5 100644
> --- a/include/linux/cpufreq.h
> +++ b/include/linux/cpufreq.h
> @@ -81,6 +81,7 @@ struct cpufreq_policy {
>  	struct freq_constraints	constraints;
>  	struct freq_qos_request	*min_freq_req;
>  	struct freq_qos_request	*max_freq_req;
> +	struct freq_qos_request *boost_freq_req;
>  
>  	struct cpufreq_frequency_table	*freq_table;
>  	enum cpufreq_table_sorting freq_table_sorted;


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1 3/4] cpufreq: Centralize boost freq QoS requests
  2025-12-04 10:13 ` [PATCH v1 3/4] cpufreq: Centralize boost freq QoS requests Pierre Gondois
@ 2025-12-04 12:02   ` zhenglifeng (A)
  2025-12-08 10:24     ` Pierre Gondois
  0 siblings, 1 reply; 12+ messages in thread
From: zhenglifeng (A) @ 2025-12-04 12:02 UTC (permalink / raw)
  To: Pierre Gondois
  Cc: linux-kernel, Christian Loehle, Ionela Voinescu, Jie Zhan,
	Huang Rui, Gautham R. Shenoy, Mario Limonciello, Perry Yuan,
	Rafael J. Wysocki, Viresh Kumar, linux-pm

On 2025/12/4 18:13, 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 | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 23f64346b80f8..9d98b98e7981c 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -603,10 +603,16 @@ 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)

I think policy->boost_enabled should be set to !policy->boost_enabled here,
too. It'll be confusing if users got an error but set the enabled flag
successfully.

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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1 1/4] Revert "cpufreq: Fix re-boost issue after hotplugging a CPU"
  2025-12-04 10:13 ` [PATCH v1 1/4] Revert "cpufreq: Fix re-boost issue after hotplugging a CPU" Pierre Gondois
@ 2025-12-04 12:09   ` zhenglifeng (A)
  2025-12-08 10:24     ` Pierre Gondois
  0 siblings, 1 reply; 12+ messages in thread
From: zhenglifeng (A) @ 2025-12-04 12:09 UTC (permalink / raw)
  To: Pierre Gondois
  Cc: linux-kernel, Christian Loehle, Ionela Voinescu, Jie Zhan,
	Huang Rui, Gautham R. Shenoy, Mario Limonciello, Perry Yuan,
	Rafael J. Wysocki, Viresh Kumar, linux-pm

On 2025/12/4 18:13, Pierre Gondois wrote:
> policy->max_freq_req represents the maximum allowed frequency as
> requested by the policyX/scaling_max_freq sysfs file. This request
> applies to all CPUs of the policy. It is not possible to request
> a per-CPU maximum frequency.
> 
> Thus, the interaction between the policy boost and scaling_max_freq
> settings should be handled by adding a boost specific QoS constraint.
> This will be handled in the following patches.
> 
> This reverts commit 1608f0230510489d74a2e24e47054233b7e4678a.
> 
> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
> ---
>  drivers/cpufreq/cpufreq.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 852e024facc3c..11b29c7dbea9e 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -1478,10 +1478,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()) {

I don't think this commit should be reverted individually. These changes
can be included in patch 4, as they are doing the same thing if I
understand it correctly.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1 4/4] cpufreq: Update set_boost callbacks to rely on boost_freq_req
  2025-12-04 10:13 ` [PATCH v1 4/4] cpufreq: Update set_boost callbacks to rely on boost_freq_req Pierre Gondois
@ 2025-12-05 15:16   ` kernel test robot
  0 siblings, 0 replies; 12+ messages in thread
From: kernel test robot @ 2025-12-05 15:16 UTC (permalink / raw)
  To: Pierre Gondois, linux-kernel
  Cc: llvm, oe-kbuild-all, Christian Loehle, Ionela Voinescu,
	zhenglifeng1, Jie Zhan, Pierre Gondois, Huang Rui,
	Gautham R. Shenoy, Mario Limonciello, Perry Yuan,
	Rafael J. Wysocki, Viresh Kumar, linux-pm

Hi Pierre,

kernel test robot noticed the following build warnings:

[auto build test WARNING on rafael-pm/linux-next]
[also build test WARNING on rafael-pm/bleeding-edge linus/master v6.18 next-20251205]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Pierre-Gondois/Revert-cpufreq-Fix-re-boost-issue-after-hotplugging-a-CPU/20251204-182201
base:   https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
patch link:    https://lore.kernel.org/r/20251204101344.192678-5-pierre.gondois%40arm.com
patch subject: [PATCH v1 4/4] cpufreq: Update set_boost callbacks to rely on boost_freq_req
config: riscv-defconfig (https://download.01.org/0day-ci/archive/20251205/202512052257.WliHSJi6-lkp@intel.com/config)
compiler: clang version 22.0.0git (https://github.com/llvm/llvm-project 14bf95b06a18b9b59c89601cbc0e5a6f2176b118)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251205/202512052257.WliHSJi6-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202512052257.WliHSJi6-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/cpufreq/cppc_cpufreq.c:771:6: warning: unused variable 'ret' [-Wunused-variable]
     771 |         int ret;
         |             ^~~
   1 warning generated.


vim +/ret +771 drivers/cpufreq/cppc_cpufreq.c

33477d84c26bbf George Cherian  2018-07-11  766  
54e74df5d76dea Xiongfeng Wang  2020-05-30  767  static int cppc_cpufreq_set_boost(struct cpufreq_policy *policy, int state)
54e74df5d76dea Xiongfeng Wang  2020-05-30  768  {
a28b2bfc099c6b Ionela Voinescu 2020-12-14  769  	struct cppc_cpudata *cpu_data = policy->driver_data;
bb025fb6c276ac Ionela Voinescu 2020-11-05  770  	struct cppc_perf_caps *caps = &cpu_data->perf_caps;
54e74df5d76dea Xiongfeng Wang  2020-05-30 @771  	int ret;
54e74df5d76dea Xiongfeng Wang  2020-05-30  772  
54e74df5d76dea Xiongfeng Wang  2020-05-30  773  	if (state)
75f25e3bda7140 Pierre Gondois  2025-12-04  774  		policy->cpuinfo.max_freq = cppc_perf_to_khz(caps, caps->highest_perf);
54e74df5d76dea Xiongfeng Wang  2020-05-30  775  	else
75f25e3bda7140 Pierre Gondois  2025-12-04  776  		policy->cpuinfo.max_freq = cppc_perf_to_khz(caps, caps->nominal_perf);
54e74df5d76dea Xiongfeng Wang  2020-05-30  777  
54e74df5d76dea Xiongfeng Wang  2020-05-30  778  	return 0;
54e74df5d76dea Xiongfeng Wang  2020-05-30  779  }
54e74df5d76dea Xiongfeng Wang  2020-05-30  780  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1 1/4] Revert "cpufreq: Fix re-boost issue after hotplugging a CPU"
  2025-12-04 12:09   ` zhenglifeng (A)
@ 2025-12-08 10:24     ` Pierre Gondois
  0 siblings, 0 replies; 12+ messages in thread
From: Pierre Gondois @ 2025-12-08 10:24 UTC (permalink / raw)
  To: zhenglifeng (A)
  Cc: linux-kernel, Christian Loehle, Ionela Voinescu, Jie Zhan,
	Huang Rui, Gautham R. Shenoy, Mario Limonciello, Perry Yuan,
	Rafael J. Wysocki, Viresh Kumar, linux-pm

Hello Lifeng,

On 12/4/25 13:09, zhenglifeng (A) wrote:
> On 2025/12/4 18:13, Pierre Gondois wrote:
>> policy->max_freq_req represents the maximum allowed frequency as
>> requested by the policyX/scaling_max_freq sysfs file. This request
>> applies to all CPUs of the policy. It is not possible to request
>> a per-CPU maximum frequency.
>>
>> Thus, the interaction between the policy boost and scaling_max_freq
>> settings should be handled by adding a boost specific QoS constraint.
>> This will be handled in the following patches.
>>
>> This reverts commit 1608f0230510489d74a2e24e47054233b7e4678a.
>>
>> Signed-off-by: Pierre Gondois<pierre.gondois@arm.com>
>> ---
>>   drivers/cpufreq/cpufreq.c | 4 ----
>>   1 file changed, 4 deletions(-)
>>
>> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
>> index 852e024facc3c..11b29c7dbea9e 100644
>> --- a/drivers/cpufreq/cpufreq.c
>> +++ b/drivers/cpufreq/cpufreq.c
>> @@ -1478,10 +1478,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()) {
> I don't think this commit should be reverted individually. These changes
> can be included in patch 4, as they are doing the same thing if I
> understand it correctly.

Ok I can do that, unless some else prefers it that way,

Thanks for the review,
Pierre


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1 3/4] cpufreq: Centralize boost freq QoS requests
  2025-12-04 12:02   ` zhenglifeng (A)
@ 2025-12-08 10:24     ` Pierre Gondois
  0 siblings, 0 replies; 12+ messages in thread
From: Pierre Gondois @ 2025-12-08 10:24 UTC (permalink / raw)
  To: zhenglifeng (A)
  Cc: linux-kernel, Christian Loehle, Ionela Voinescu, Jie Zhan,
	Huang Rui, Gautham R. Shenoy, Mario Limonciello, Perry Yuan,
	Rafael J. Wysocki, Viresh Kumar, linux-pm


On 12/4/25 13:02, zhenglifeng (A) wrote:
> On 2025/12/4 18:13, 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 | 10 ++++++++--
>>   1 file changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
>> index 23f64346b80f8..9d98b98e7981c 100644
>> --- a/drivers/cpufreq/cpufreq.c
>> +++ b/drivers/cpufreq/cpufreq.c
>> @@ -603,10 +603,16 @@ 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)
> I think policy->boost_enabled should be set to !policy->boost_enabled here,
> too. It'll be confusing if users got an error but set the enabled flag
> successfully.
Yes right indeed
>> +		return ret;
>> +
>> +	return 0;
>>   }
>>   
>>   static ssize_t store_local_boost(struct cpufreq_policy *policy,

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1 2/4] cpufreq: Add boost_freq_req QoS request
  2025-12-04 11:50   ` zhenglifeng (A)
@ 2025-12-08 10:25     ` Pierre Gondois
  0 siblings, 0 replies; 12+ messages in thread
From: Pierre Gondois @ 2025-12-08 10:25 UTC (permalink / raw)
  To: zhenglifeng (A)
  Cc: linux-kernel, Christian Loehle, Ionela Voinescu, Jie Zhan,
	Huang Rui, Gautham R. Shenoy, Mario Limonciello, Perry Yuan,
	Rafael J. Wysocki, Viresh Kumar, linux-pm


On 12/4/25 12:50, zhenglifeng (A) wrote:
> On 2025/12/4 18:13, 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 | 35 +++++++++++++++++++++++++++++++++++
>>   include/linux/cpufreq.h   |  1 +
>>   2 files changed, 36 insertions(+)
>>
>> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
>> index 11b29c7dbea9e..23f64346b80f8 100644
>> --- a/drivers/cpufreq/cpufreq.c
>> +++ b/drivers/cpufreq/cpufreq.c
>> @@ -1370,6 +1370,18 @@ static void cpufreq_policy_free(struct cpufreq_policy *policy)
>>   		freq_qos_remove_request(policy->max_freq_req);
>>   	}
>>   
>> +	if (policy->boost_freq_req) {
>> +		/*
>> +		 * Remove boost_freq_req after sending CPUFREQ_REMOVE_POLICY
>> +		 * notification, since CPUFREQ_CREATE_POLICY notification was
>> +		 * sent after adding boost_freq_req earlier.
>> +		 */
>> +		blocking_notifier_call_chain(&cpufreq_policy_notifier_list,
>> +					     CPUFREQ_REMOVE_POLICY, policy);
> The CPUFREQ_REMOVE_POLICY notification is sent before removing
> max_freq_req, I don't think it should be sent again here.
Yes indeed
>> +		freq_qos_remove_request(policy->boost_freq_req);
>> +		kfree(policy->boost_freq_req);
>> +	}
>> +
> I think boost_freq_req should be removed before removing max_freq_req,
> since it was added after adding max_freq_req as shown below.

Ok right

>>   	freq_qos_remove_request(policy->min_freq_req);
>>   	kfree(policy->min_freq_req);
>>   
>> @@ -1476,6 +1488,29 @@ static int cpufreq_policy_online(struct cpufreq_policy *policy,
>>   			goto out_destroy_policy;
>>   		}
>>   
>> +		if (policy->boost_supported) {
>> +			policy->boost_freq_req = kzalloc(sizeof(*policy->boost_freq_req),
>> +				GFP_KERNEL);
>> +			if (!policy->boost_freq_req) {
>> +				ret = -ENOMEM;
>> +				goto out_destroy_policy;
>> +			}
>> +
>> +			ret = freq_qos_add_request(&policy->constraints,
>> +						   policy->boost_freq_req,
>> +						   FREQ_QOS_MAX,
>> +						   FREQ_QOS_MAX_DEFAULT_VALUE);
>> +			if (ret < 0) {
>> +				/*
>> +				 * So we don't call freq_qos_remove_request() for an
>> +				 * uninitialized request.
>> +				 */
>> +				kfree(policy->boost_freq_req);
>> +				policy->boost_freq_req = NULL;
>> +				goto out_destroy_policy;
>> +			}
>> +		}
>> +
>>   		blocking_notifier_call_chain(&cpufreq_policy_notifier_list,
>>   				CPUFREQ_CREATE_POLICY, policy);
>>   	}
>> diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
>> index 0465d1e6f72ac..c292a6a19e4f5 100644
>> --- a/include/linux/cpufreq.h
>> +++ b/include/linux/cpufreq.h
>> @@ -81,6 +81,7 @@ struct cpufreq_policy {
>>   	struct freq_constraints	constraints;
>>   	struct freq_qos_request	*min_freq_req;
>>   	struct freq_qos_request	*max_freq_req;
>> +	struct freq_qos_request *boost_freq_req;
>>   
>>   	struct cpufreq_frequency_table	*freq_table;
>>   	enum cpufreq_table_sorting freq_table_sorted;

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2025-12-08 10:26 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-04 10:13 [PATCH v1 0/4] cpufreq: Introduce boot frequency QoS Pierre Gondois
2025-12-04 10:13 ` [PATCH v1 1/4] Revert "cpufreq: Fix re-boost issue after hotplugging a CPU" Pierre Gondois
2025-12-04 12:09   ` zhenglifeng (A)
2025-12-08 10:24     ` Pierre Gondois
2025-12-04 10:13 ` [PATCH v1 2/4] cpufreq: Add boost_freq_req QoS request Pierre Gondois
2025-12-04 11:50   ` zhenglifeng (A)
2025-12-08 10:25     ` Pierre Gondois
2025-12-04 10:13 ` [PATCH v1 3/4] cpufreq: Centralize boost freq QoS requests Pierre Gondois
2025-12-04 12:02   ` zhenglifeng (A)
2025-12-08 10:24     ` Pierre Gondois
2025-12-04 10:13 ` [PATCH v1 4/4] cpufreq: Update set_boost callbacks to rely on boost_freq_req Pierre Gondois
2025-12-05 15:16   ` kernel test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox