[PATCH v2 0/4] cpufreq: Set policy->min and max as real QoS constraints

[PATCH v2 0/4] cpufreq: Set policy->min and max as real QoS constraints

[Pierre Gondois]

This patch is a follow-up from the serie:
- [PATCH v6 0/4] cpufreq: Introduce boost frequency QoS
https://lore.kernel.org/lkml/20260317101753.2284763-1-pierre.gondois@arm.com/

v2:
- Split the patch in multiple steps
- Replace min/max -> min_freq/max_freq
- Add references to commit 521223d8b3ec ("cpufreq: Fix initialization
  of min and max frequency QoS requests") to explain the intent
  of the patch
- Update documentation (cpu-drivers.rst)
- Set default policy->min/max values before the call to
  blocking_notifier_call_chain()
- Create a new cpufreq_policy_init_qos() function to put all
  the QoS and policy->min/max logic inside.
- Didn't add Sumit's reviewed-by as the patches changed a bit.
v1:
https://lore.kernel.org/lkml/20260423084731.1090384-1-pierre.gondois@arm.com/#t

Pierre Gondois (4):
  cpufreq: Extract cpufreq_policy_init_qos() function
  cpufreq: Set default policy->min/max values for all drivers
  cpufreq: Remove driver default policy->min/max init
  cpufreq: Use policy->min/max init as QoS request

 Documentation/cpu-freq/cpu-drivers.rst | 10 +++-
 drivers/cpufreq/amd-pstate.c           | 14 +++---
 drivers/cpufreq/cppc_cpufreq.c         |  5 +-
 drivers/cpufreq/cpufreq-nforce2.c      |  4 +-
 drivers/cpufreq/cpufreq.c              | 68 ++++++++++++++++++--------
 drivers/cpufreq/freq_table.c           |  7 ++-
 drivers/cpufreq/gx-suspmod.c           |  2 +-
 drivers/cpufreq/intel_pstate.c         |  3 --
 drivers/cpufreq/pcc-cpufreq.c          | 10 ++--
 drivers/cpufreq/pxa3xx-cpufreq.c       |  5 +-
 drivers/cpufreq/sh-cpufreq.c           |  6 +--
 drivers/cpufreq/virtual-cpufreq.c      |  5 +-
 12 files changed, 78 insertions(+), 61 deletions(-)

--
2.43.0

[PATCH v2 1/4] cpufreq: Extract cpufreq_policy_init_qos() function

[Pierre Gondois]

Extract the QoS related logic from cpufreq_policy_online()
to make the function shorter/simpler.

The logic is placed in cpufreq_policy_init_qos() and is
now executed right after the following calls:
- cpufreq_driver->init()
- cpufreq_table_validate_and_sort()

This helps preparing following patches that will,
in cpufreq_policy_init_qos():
- treat the policy->min/max values set by drivers as QoS requests.
- set a default policy->min/max value to all policies.

No functional change.

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

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 44eb1b7e7fc1b..034603c2af325 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1397,6 +1397,32 @@ static void cpufreq_policy_free(struct cpufreq_policy *policy)
 	kfree(policy);
 }
 
+static int cpufreq_policy_init_qos(struct cpufreq_policy *policy)
+{
+	int ret;
+
+	if (policy->boost_supported) {
+		ret = freq_qos_add_request(&policy->constraints,
+						&policy->boost_freq_req,
+						FREQ_QOS_MAX,
+						policy->cpuinfo.max_freq);
+		if (ret < 0)
+			return ret;
+	}
+
+	ret = freq_qos_add_request(&policy->constraints, &policy->min_freq_req,
+				   FREQ_QOS_MIN, FREQ_QOS_MIN_DEFAULT_VALUE);
+	if (ret < 0)
+		return ret;
+
+	ret = freq_qos_add_request(&policy->constraints, &policy->max_freq_req,
+				   FREQ_QOS_MAX, FREQ_QOS_MAX_DEFAULT_VALUE);
+	if (ret < 0)
+		return ret;
+
+	return ret;
+}
+
 static int cpufreq_policy_online(struct cpufreq_policy *policy,
 				 unsigned int cpu, bool new_policy)
 {
@@ -1442,6 +1468,12 @@ static int cpufreq_policy_online(struct cpufreq_policy *policy,
 		if (ret)
 			goto out_offline_policy;
 
+		if (new_policy) {
+			ret = cpufreq_policy_init_qos(policy);
+			if (ret < 0)
+				goto out_offline_policy;
+		}
+
 		/* related_cpus should at least include policy->cpus. */
 		cpumask_copy(policy->related_cpus, policy->cpus);
 	}
@@ -1458,27 +1490,6 @@ static int cpufreq_policy_online(struct cpufreq_policy *policy,
 			add_cpu_dev_symlink(policy, j, get_cpu_device(j));
 		}
 
-		if (policy->boost_supported) {
-			ret = freq_qos_add_request(&policy->constraints,
-						   &policy->boost_freq_req,
-						   FREQ_QOS_MAX,
-						   policy->cpuinfo.max_freq);
-			if (ret < 0)
-				goto out_destroy_policy;
-		}
-
-		ret = freq_qos_add_request(&policy->constraints,
-					   &policy->min_freq_req, FREQ_QOS_MIN,
-					   FREQ_QOS_MIN_DEFAULT_VALUE);
-		if (ret < 0)
-			goto out_destroy_policy;
-
-		ret = freq_qos_add_request(&policy->constraints,
-					   &policy->max_freq_req, FREQ_QOS_MAX,
-					   FREQ_QOS_MAX_DEFAULT_VALUE);
-		if (ret < 0)
-			goto out_destroy_policy;
-
 		blocking_notifier_call_chain(&cpufreq_policy_notifier_list,
 				CPUFREQ_CREATE_POLICY, policy);
 	}
-- 
2.43.0

[PATCH v2 2/4] cpufreq: Set default policy->min/max values for all drivers

[Pierre Gondois]

Some drivers set policy->min/max in their .init() callback.
cpufreq_set_policy() will ultimately override them through:
cpufreq_policy_online()
\-cpufreq_init_policy()
  \-cpufreq_set_policy()
    \-/* Set policy->min/max */
Thus the policy min/max values provided are only temporary.

There is an exception if CPUFREQ_NEED_INITIAL_FREQ_CHECK is set and:
cpufreq_policy_online()
\-__cpufreq_driver_target()
  \-cpufreq_driver->target()

To prepare for a following patch that will remove all
policy->min/max initialization in the driver .init() callback
if the min/max value is equal to the cpuinfo.min/max_freq,
set a default policy->min/max value for all drivers.

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

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 034603c2af325..9e2d9d3fc5351 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1401,6 +1401,13 @@ static int cpufreq_policy_init_qos(struct cpufreq_policy *policy)
 {
 	int ret;
 
+	/*
+	 * If the driver didn't set policy->min/max, set them as
+	 * they are used to clamp frequency requests.
+	 */
+	policy->min = policy->min ? policy->min : policy->cpuinfo.min_freq;
+	policy->max = policy->max ? policy->max : policy->cpuinfo.max_freq;
+
 	if (policy->boost_supported) {
 		ret = freq_qos_add_request(&policy->constraints,
 						&policy->boost_freq_req,
-- 
2.43.0

[PATCH v2 3/4] cpufreq: Remove driver default policy->min/max init

[Pierre Gondois]

Prior to [1], drivers were setting policy->min/max and
the value was used as a QoS constraint. After that change,
the values were only temporarily used: cpufreq_set_policy()
ultimately overriding them through:
cpufreq_policy_online()
\-cpufreq_init_policy()
  \-cpufreq_set_policy()
    \-/* Set policy->min/max */

This patch reinstate the initial behaviour. This will allow
drivers to request min/max QoS frequencies if desired.
For instance, the cppc driver advertises a lowest non-linear
frequency, which should be used as a min QoS value.

To avoid having drivers setting policy->min/max to default
values which are considered as QoS values (i.e. the reason
why [1] was introduced), remove the initialization of
policy->min/max in .init() callbacks wherever the
policy->min/max values are identical to the
policy->cpuinfo.min/max_freq.

Indeed, the previous patch ("cpufreq: Set default
policy->min/max values for all drivers") makes this initialization
redundant.

The only drivers where these values are different are:
- gx-suspmod.c (min)
- cppc-cpufreq.c (min)
- longrun.c

[1]
commit 521223d8b3ec ("cpufreq: Fix initialization of min and
max frequency QoS requests")

Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
---
 drivers/cpufreq/amd-pstate.c      | 14 ++++++--------
 drivers/cpufreq/cppc_cpufreq.c    |  5 ++---
 drivers/cpufreq/cpufreq-nforce2.c |  4 ++--
 drivers/cpufreq/freq_table.c      |  7 +++----
 drivers/cpufreq/gx-suspmod.c      |  2 +-
 drivers/cpufreq/intel_pstate.c    |  3 ---
 drivers/cpufreq/pcc-cpufreq.c     | 10 ++++------
 drivers/cpufreq/pxa3xx-cpufreq.c  |  5 ++---
 drivers/cpufreq/sh-cpufreq.c      |  6 ++----
 drivers/cpufreq/virtual-cpufreq.c |  5 +----
 10 files changed, 23 insertions(+), 38 deletions(-)

diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 453084c67327f..ecc3779e047d5 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -1090,10 +1090,9 @@ static int amd_pstate_cpu_init(struct cpufreq_policy *policy)
 
 	perf = READ_ONCE(cpudata->perf);
 
-	policy->cpuinfo.min_freq = policy->min = perf_to_freq(perf,
-							      cpudata->nominal_freq,
-							      perf.lowest_perf);
-	policy->cpuinfo.max_freq = policy->max = cpudata->max_freq;
+	policy->cpuinfo.min_freq = perf_to_freq(perf, cpudata->nominal_freq,
+						perf.lowest_perf);
+	policy->cpuinfo.max_freq = cpudata->max_freq;
 
 	policy->driver_data = cpudata;
 	ret = amd_pstate_cppc_enable(policy);
@@ -1907,10 +1906,9 @@ static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy)
 
 	perf = READ_ONCE(cpudata->perf);
 
-	policy->cpuinfo.min_freq = policy->min = perf_to_freq(perf,
-							      cpudata->nominal_freq,
-							      perf.lowest_perf);
-	policy->cpuinfo.max_freq = policy->max = cpudata->max_freq;
+	policy->cpuinfo.min_freq = perf_to_freq(perf, cpudata->nominal_freq,
+						perf.lowest_perf);
+	policy->cpuinfo.max_freq = cpudata->max_freq;
 	policy->driver_data = cpudata;
 
 	ret = amd_pstate_cppc_enable(policy);
diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
index 7e7f9dfb7a24c..5abac50df7508 100644
--- a/drivers/cpufreq/cppc_cpufreq.c
+++ b/drivers/cpufreq/cppc_cpufreq.c
@@ -660,8 +660,6 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy)
 	 * 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
@@ -669,7 +667,8 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy)
 	 * 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;
diff --git a/drivers/cpufreq/cpufreq-nforce2.c b/drivers/cpufreq/cpufreq-nforce2.c
index fbbbe501cf2dc..831102522ad64 100644
--- a/drivers/cpufreq/cpufreq-nforce2.c
+++ b/drivers/cpufreq/cpufreq-nforce2.c
@@ -355,8 +355,8 @@ static int nforce2_cpu_init(struct cpufreq_policy *policy)
 		min_fsb = NFORCE2_MIN_FSB;
 
 	/* cpuinfo and default policy values */
-	policy->min = policy->cpuinfo.min_freq = min_fsb * fid * 100;
-	policy->max = policy->cpuinfo.max_freq = max_fsb * fid * 100;
+	policy->cpuinfo.min_freq = min_fsb * fid * 100;
+	policy->cpuinfo.max_freq = max_fsb * fid * 100;
 
 	return 0;
 }
diff --git a/drivers/cpufreq/freq_table.c b/drivers/cpufreq/freq_table.c
index 5b364d8da4f92..ea994647abc88 100644
--- a/drivers/cpufreq/freq_table.c
+++ b/drivers/cpufreq/freq_table.c
@@ -49,16 +49,15 @@ int cpufreq_frequency_table_cpuinfo(struct cpufreq_policy *policy)
 			max_freq = freq;
 	}
 
-	policy->min = policy->cpuinfo.min_freq = min_freq;
-	policy->max = max_freq;
+	policy->cpuinfo.min_freq = min_freq;
 	/*
 	 * If the driver has set its own cpuinfo.max_freq above max_freq, leave
 	 * it as is.
 	 */
 	if (policy->cpuinfo.max_freq < max_freq)
-		policy->max = policy->cpuinfo.max_freq = max_freq;
+		policy->cpuinfo.max_freq = max_freq;
 
-	if (policy->min == ~0)
+	if (min_freq == ~0)
 		return -EINVAL;
 	else
 		return 0;
diff --git a/drivers/cpufreq/gx-suspmod.c b/drivers/cpufreq/gx-suspmod.c
index d269a4f26f98e..d40c9e0bbb740 100644
--- a/drivers/cpufreq/gx-suspmod.c
+++ b/drivers/cpufreq/gx-suspmod.c
@@ -421,7 +421,7 @@ static int cpufreq_gx_cpu_init(struct cpufreq_policy *policy)
 		policy->min = maxfreq / max_duration;
 	else
 		policy->min = maxfreq / POLICY_MIN_DIV;
-	policy->max = maxfreq;
+
 	policy->cpuinfo.min_freq = maxfreq / max_duration;
 	policy->cpuinfo.max_freq = maxfreq;
 
diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index 1292da53e5fcb..68ccc6eb1ef30 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -3049,9 +3049,6 @@ static int __intel_pstate_cpu_init(struct cpufreq_policy *policy)
 	policy->cpuinfo.max_freq = READ_ONCE(global.no_turbo) ?
 			cpu->pstate.max_freq : cpu->pstate.turbo_freq;
 
-	policy->min = policy->cpuinfo.min_freq;
-	policy->max = policy->cpuinfo.max_freq;
-
 	intel_pstate_init_acpi_perf_limits(policy);
 
 	policy->fast_switch_possible = true;
diff --git a/drivers/cpufreq/pcc-cpufreq.c b/drivers/cpufreq/pcc-cpufreq.c
index ac2e90a65f0c4..0f185a13577f8 100644
--- a/drivers/cpufreq/pcc-cpufreq.c
+++ b/drivers/cpufreq/pcc-cpufreq.c
@@ -551,13 +551,11 @@ static int pcc_cpufreq_cpu_init(struct cpufreq_policy *policy)
 		goto out;
 	}
 
-	policy->max = policy->cpuinfo.max_freq =
-		ioread32(&pcch_hdr->nominal) * 1000;
-	policy->min = policy->cpuinfo.min_freq =
-		ioread32(&pcch_hdr->minimum_frequency) * 1000;
+	policy->cpuinfo.max_freq = ioread32(&pcch_hdr->nominal) * 1000;
+	policy->cpuinfo.min_freq = ioread32(&pcch_hdr->minimum_frequency) * 1000;
 
-	pr_debug("init: policy->max is %d, policy->min is %d\n",
-		policy->max, policy->min);
+	pr_debug("init: max_freq is %d, min_freq is %d\n",
+		 policy->cpuinfo.max_freq, policy->cpuinfo.min_freq);
 out:
 	return result;
 }
diff --git a/drivers/cpufreq/pxa3xx-cpufreq.c b/drivers/cpufreq/pxa3xx-cpufreq.c
index 50ff3b6a69000..06b27cbc59d6a 100644
--- a/drivers/cpufreq/pxa3xx-cpufreq.c
+++ b/drivers/cpufreq/pxa3xx-cpufreq.c
@@ -185,9 +185,8 @@ static int pxa3xx_cpufreq_init(struct cpufreq_policy *policy)
 	int ret = -EINVAL;
 
 	/* set default policy and cpuinfo */
-	policy->min = policy->cpuinfo.min_freq = 104000;
-	policy->max = policy->cpuinfo.max_freq =
-		(cpu_is_pxa320()) ? 806000 : 624000;
+	policy->cpuinfo.min_freq = 104000;
+	policy->cpuinfo.max_freq = (cpu_is_pxa320()) ? 806000 : 624000;
 	policy->cpuinfo.transition_latency = 1000; /* FIXME: 1 ms, assumed */
 
 	if (cpu_is_pxa300() || cpu_is_pxa310())
diff --git a/drivers/cpufreq/sh-cpufreq.c b/drivers/cpufreq/sh-cpufreq.c
index 642ddb9ea217e..3c99d7009cbe2 100644
--- a/drivers/cpufreq/sh-cpufreq.c
+++ b/drivers/cpufreq/sh-cpufreq.c
@@ -124,10 +124,8 @@ static int sh_cpufreq_cpu_init(struct cpufreq_policy *policy)
 		dev_notice(dev, "no frequency table found, falling back "
 			   "to rate rounding.\n");
 
-		policy->min = policy->cpuinfo.min_freq =
-			(clk_round_rate(cpuclk, 1) + 500) / 1000;
-		policy->max = policy->cpuinfo.max_freq =
-			(clk_round_rate(cpuclk, ~0UL) + 500) / 1000;
+		policy->cpuinfo.min_freq = (clk_round_rate(cpuclk, 1) + 500) / 1000;
+		policy->cpuinfo.max_freq = (clk_round_rate(cpuclk, ~0UL) + 500) / 1000;
 	}
 
 	return 0;
diff --git a/drivers/cpufreq/virtual-cpufreq.c b/drivers/cpufreq/virtual-cpufreq.c
index 4159f31349b16..dc78b74409af4 100644
--- a/drivers/cpufreq/virtual-cpufreq.c
+++ b/drivers/cpufreq/virtual-cpufreq.c
@@ -164,10 +164,7 @@ static int virt_cpufreq_get_freq_info(struct cpufreq_policy *policy)
 		policy->cpuinfo.min_freq = 1;
 		policy->cpuinfo.max_freq = virt_cpufreq_get_perftbl_entry(policy->cpu, 0);
 
-		policy->min = policy->cpuinfo.min_freq;
-		policy->max = policy->cpuinfo.max_freq;
-
-		policy->cur = policy->max;
+		policy->cur = policy->cpuinfo.max_freq;
 		return 0;
 	}
 
-- 
2.43.0

[PATCH v2 4/4] cpufreq: Use policy->min/max init as QoS request

[Pierre Gondois]

Consider policy->min/max being set in the driver .init()
callback as a QoS request. Impacted driver are:
- gx-suspmod.c (min)
- cppc-cpufreq.c (min)
- longrun.c (min/max)

Update the documentation accordingly.

Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
---
 Documentation/cpu-freq/cpu-drivers.rst | 10 ++++++++--
 drivers/cpufreq/cpufreq.c              | 12 ++++++++++--
 2 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/Documentation/cpu-freq/cpu-drivers.rst b/Documentation/cpu-freq/cpu-drivers.rst
index c5635ac3de547..ab4f3c0f3a89b 100644
--- a/Documentation/cpu-freq/cpu-drivers.rst
+++ b/Documentation/cpu-freq/cpu-drivers.rst
@@ -114,8 +114,14 @@ Then, the driver must fill in the following values:
 |policy->cur			    | The current operating frequency of   |
 |				    | this CPU (if appropriate)		   |
 +-----------------------------------+--------------------------------------+
-|policy->min,			    |					   |
-|policy->max,			    |					   |
+|policy->min			    | If set by the driver in ->init(),    |
+|				    | used as initial minimum frequency	   |
+|				    | QoS request.			   |
++-----------------------------------+--------------------------------------+
+|policy->max			    | If set by the driver in ->init(),    |
+|				    | used as initial maximum frequency	   |
+|				    | QoS request.			   |
++-----------------------------------+--------------------------------------+
 |policy->policy and, if necessary,  |					   |
 |policy->governor		    | must contain the "default policy" for|
 |				    | this CPU. A few moments later,       |
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 9e2d9d3fc5351..9a005367ed87b 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1399,8 +1399,16 @@ static void cpufreq_policy_free(struct cpufreq_policy *policy)
 
 static int cpufreq_policy_init_qos(struct cpufreq_policy *policy)
 {
+	unsigned int min_freq, max_freq;
 	int ret;
 
+	/* Use policy->min/max set by the driver as QoS requests. */
+	min_freq = max(FREQ_QOS_MIN_DEFAULT_VALUE, policy->min);
+	if (policy->max)
+		max_freq = min(FREQ_QOS_MAX_DEFAULT_VALUE, policy->max);
+	else
+		max_freq = FREQ_QOS_MAX_DEFAULT_VALUE;
+
 	/*
 	 * If the driver didn't set policy->min/max, set them as
 	 * they are used to clamp frequency requests.
@@ -1418,12 +1426,12 @@ static int cpufreq_policy_init_qos(struct cpufreq_policy *policy)
 	}
 
 	ret = freq_qos_add_request(&policy->constraints, &policy->min_freq_req,
-				   FREQ_QOS_MIN, FREQ_QOS_MIN_DEFAULT_VALUE);
+				   FREQ_QOS_MIN, min_freq);
 	if (ret < 0)
 		return ret;
 
 	ret = freq_qos_add_request(&policy->constraints, &policy->max_freq_req,
-				   FREQ_QOS_MAX, FREQ_QOS_MAX_DEFAULT_VALUE);
+				   FREQ_QOS_MAX, max_freq);
 	if (ret < 0)
 		return ret;
 
-- 
2.43.0