[PATCH v6 0/4] cpufreq: Introduce boost frequency QoS

[PATCH v6 0/4] cpufreq: Introduce boost 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

This patch-set additionally does:
- if a driver .init() callback sets policy->min and policy->max,
  these values are used are min/max_freq_req QoS constraints
- Remove policy->min and policy->max initialization in .init()
  callback if the value is identical to cpuinfo.min/max_freq
- RFC: allow decreasing cpuinfo.max_freq when using freq_table.

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

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.

---

Note:
It seems that there is a confusion in the cpufreq framework between:
- the min/max frequency requested by the user
- the min/max frequency constraint applied when selecting a frequency.

E.g:
A.
$ echo XXX > scaling_max_freq
updates the max_freq_req QoS request.

B.
$ cat scaling_max_freq
shows the content of policy->max, which is the not representing
the value of the max_freq_req QoS request.

C.
Whenever policy->max is accessed in the cpufreq framework,
the aggregation of all the requests on the maximum frequency should
be used instead.

cpufreq_set_policy() aggregates min/max constraints and
writes the resulting value in policy->min/max. These values
are then used in the cpufreq drivers.

Creating a clear distinction would be doable but quite invasive.
This patchset focuses on handling the boost frequency QoS request
first and should not change the current behaviour of policy->min
and max.

---

v1: https://lore.kernel.org/all/20251204101344.192678-1-pierre.gondois@arm.com/#t
v2: https://lore.kernel.org/all/20251208105933.1369125-1-pierre.gondois@arm.com/#t
Changes:
- Fixed error path
- Integrated [PATCH 1/4] Revert "cpufreq: Fix re-boost issue after hotplugging a CPU"
  to another patch
v3:
Changes:
- Fixed error path
- Extracted the revert of:
  "cpufreq: Fix re-boost issue after hotplugging a CPU"
  for clarity purpose
- Set cpuinfo.max_freq as a max_freq_req QoS constraint by default
New patches:
- "cpufreq: Allow decreasing cpuinfo.max_freq"
- "cpufreq: Set policy->min and max as QoS constraints"
v4:
- Correct reported issues
v5:
- Corrections
v6:
- Folded patches:
  - cpufreq: Centralize boost freq QoS requests
  - cpufreq: Update .set_boost() callbacks to rely on boost_freq_req
  inside:
  - cpufreq: Add boost_freq_req QoS request
- Simplified allocation handling of boost_freq_req
- Removed unnecessary bits

Pierre Gondois (4):
  cpufreq: Remove per-CPU QoS constraint
  cpufreq: Add boost_freq_req QoS request
  cpufreq: Set policy->min and max as real QoS constraints
  cpufreq/freq_table: Allow decreasing cpuinfo.max_freq

 drivers/cpufreq/amd-pstate.c      | 26 ++++++-----
 drivers/cpufreq/cppc_cpufreq.c    | 21 ++++-----
 drivers/cpufreq/cpufreq-nforce2.c |  4 +-
 drivers/cpufreq/cpufreq.c         | 71 +++++++++++++++++++++++--------
 drivers/cpufreq/freq_table.c      | 14 ++----
 drivers/cpufreq/gx-suspmod.c      |  9 ++--
 drivers/cpufreq/intel_pstate.c    |  3 --
 drivers/cpufreq/pcc-cpufreq.c     |  8 ++--
 drivers/cpufreq/pxa3xx-cpufreq.c  |  4 +-
 drivers/cpufreq/sh-cpufreq.c      |  7 +--
 drivers/cpufreq/virtual-cpufreq.c |  5 +--
 include/linux/cpufreq.h           |  1 +
 12 files changed, 97 insertions(+), 76 deletions(-)

--
2.43.0

[PATCH v6 1/4] cpufreq: Remove per-CPU QoS constraint

[Pierre Gondois]

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 patch reverts of:
commit 1608f0230510 ("cpufreq: Fix re-boost issue after hotplugging
a CPU")

Note:
I was unable to reproduce the issue initially reported.
So far it seems that was fixed in:
commit 121baab7b88e ("cpufreq: Force sync policy boost with
global boost on sysfs update")
Another point is that the initial patch states that the error
comes a max_freq_req request is repeated, but calls to
freq_qos_update_request() would return 0 and all calls are
checked against negative values.

Reviewed-by: Lifeng Zheng <zhenglifeng1@huawei.com>
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 4472bb1ec83c7..db414c052658b 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1481,10 +1481,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

Re: [PATCH v6 1/4] cpufreq: Remove per-CPU QoS constraint

[Viresh Kumar]

On 17-03-26, 11:17, 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.

I don't think the above is required anymore. This patch is removing stale code
now which isn't useful anymore. It has nothing to do with a boost specific QOS
constraint.

And it would be better to know for sure why this isn't required anymore and
which patch exactly fixed this issue.

-- 
viresh

Re: [PATCH v6 1/4] cpufreq: Remove per-CPU QoS constraint

[Pierre Gondois]

On 3/18/26 12:13, Viresh Kumar wrote:
> On 17-03-26, 11:17, 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.
> I don't think the above is required anymore. This patch is removing stale code
> now which isn't useful anymore. It has nothing to do with a boost specific QOS
> constraint.
Yes ok
> And it would be better to know for sure why this isn't required anymore and
> which patch exactly fixed this issue.
>
On a kernel based on 1608f0230510~, and replicating the
process described in the commit message of

commit 1608f0230510 ("cpufreq: Fix re-boost issue after hotplugging
a CPU")

I could not see any issue regarding the values of:

- policy1/cpuinfo_max_freq
- policy1/scaling_max_freq

The following sequence however had an issue:

1. echo 0 > /sys/devices/system/cpu/cpu1/online
2. echo 1 > /sys/devices/system/cpu/cpufreq/boost
3. echo 1 > /sys/devices/system/cpu/cpu1/online

as after 1.:

cpufreq_boost_trigger_state()
\-for_each_active_policy()

doesn't enable boost for inactive policies. This leads to
CPU1 having the non-boosted frequency as its max freq.

The above sequence is fixed by:

commit a153c6049ab8 ("cpufreq: Introduce a more
generic way to set default per-policy boost flag")

---

@Lifeng, should I check something else than the value of:

- policy1/cpuinfo_max_freq
- policy1/scaling_max_freq

in order to reproduce the issue fixed by:

commit 1608f0230510 ("cpufreq: Fix re-boost issue after hotplugging
a CPU")

?

Re: [PATCH v6 1/4] cpufreq: Remove per-CPU QoS constraint

[Viresh Kumar]

On 19-03-26, 10:30, Pierre Gondois wrote:
> 
> On 3/18/26 12:13, Viresh Kumar wrote:
> > On 17-03-26, 11:17, 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.
> > I don't think the above is required anymore. This patch is removing stale code
> > now which isn't useful anymore. It has nothing to do with a boost specific QOS
> > constraint.
> Yes ok
> > And it would be better to know for sure why this isn't required anymore and
> > which patch exactly fixed this issue.
> > 
> On a kernel based on 1608f0230510~, and replicating the
> process described in the commit message of
> 
> commit 1608f0230510 ("cpufreq: Fix re-boost issue after hotplugging
> a CPU")
> 
> I could not see any issue regarding the values of:
> 
> - policy1/cpuinfo_max_freq
> - policy1/scaling_max_freq

The commit message (of 1608f0230510) is confusing. The issue was discussed
properly in the following thread.

https://lore.kernel.org/all/20250120082723.am7rxujmdvzz4eky@vireshk-i7/

The problem is that policy->max and policy->cpuinfo_max_freq are incorrect after
the sequence mentioned in the commit, while max_freq_req is correct.

I think another commit has fixed that (incorrectly and unintentionally):
commit 6db0f533d320 ("cpufreq: preserve freq_table_sorted across suspend/hibernate")

@@ -1421,9 +1421,12 @@ static int cpufreq_policy_online(struct cpufreq_policy *policy,
                 * If there is a problem with its frequency table, take it
                 * offline and drop it.
                 */
-               ret = cpufreq_table_validate_and_sort(policy);
-               if (ret)
-                       goto out_offline_policy;
+               if (policy->freq_table_sorted != CPUFREQ_TABLE_SORTED_ASCENDING &&
+                   policy->freq_table_sorted != CPUFREQ_TABLE_SORTED_DESCENDING) {
+                       ret = cpufreq_table_validate_and_sort(policy);
+                       if (ret)
+                               goto out_offline_policy;
+               }

This skipped calling cpufreq_table_validate_and_sort() completely on online and
so max/cpuinfo_max_freq, max_freq_req are all in sync.

That change should be fixed with:

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 277884d91913..1f794524a1d9 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1427,12 +1427,9 @@ static int cpufreq_policy_online(struct cpufreq_policy *policy,
                 * If there is a problem with its frequency table, take it
                 * offline and drop it.
                 */
-               if (policy->freq_table_sorted != CPUFREQ_TABLE_SORTED_ASCENDING &&
-                   policy->freq_table_sorted != CPUFREQ_TABLE_SORTED_DESCENDING) {
-                       ret = cpufreq_table_validate_and_sort(policy);
-                       if (ret)
-                               goto out_offline_policy;
-               }
+               ret = cpufreq_table_validate_and_sort(policy);
+               if (ret)
+                       goto out_offline_policy;

                /* related_cpus should at least include policy->cpus. */
                cpumask_copy(policy->related_cpus, policy->cpus);
diff --git a/drivers/cpufreq/freq_table.c b/drivers/cpufreq/freq_table.c
index 7f251daf03ce..5b364d8da4f9 100644
--- a/drivers/cpufreq/freq_table.c
+++ b/drivers/cpufreq/freq_table.c
@@ -360,6 +360,10 @@ int cpufreq_table_validate_and_sort(struct cpufreq_policy *policy)
        if (policy_has_boost_freq(policy))
                policy->boost_supported = true;

+       if (policy->freq_table_sorted == CPUFREQ_TABLE_SORTED_ASCENDING ||
+           policy->freq_table_sorted == CPUFREQ_TABLE_SORTED_DESCENDING)
+               return 0;
+
        return set_freq_table_sorted(policy);
 }

-- 
viresh

Re: [PATCH v6 1/4] cpufreq: Remove per-CPU QoS constraint

[Viresh Kumar]

On 20-03-26, 14:48, Viresh Kumar wrote:
> That change should be fixed with:
> 
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 277884d91913..1f794524a1d9 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -1427,12 +1427,9 @@ static int cpufreq_policy_online(struct cpufreq_policy *policy,
>                  * If there is a problem with its frequency table, take it
>                  * offline and drop it.
>                  */
> -               if (policy->freq_table_sorted != CPUFREQ_TABLE_SORTED_ASCENDING &&
> -                   policy->freq_table_sorted != CPUFREQ_TABLE_SORTED_DESCENDING) {
> -                       ret = cpufreq_table_validate_and_sort(policy);
> -                       if (ret)
> -                               goto out_offline_policy;
> -               }
> +               ret = cpufreq_table_validate_and_sort(policy);
> +               if (ret)
> +                       goto out_offline_policy;
> 
>                 /* related_cpus should at least include policy->cpus. */
>                 cpumask_copy(policy->related_cpus, policy->cpus);
> diff --git a/drivers/cpufreq/freq_table.c b/drivers/cpufreq/freq_table.c
> index 7f251daf03ce..5b364d8da4f9 100644
> --- a/drivers/cpufreq/freq_table.c
> +++ b/drivers/cpufreq/freq_table.c
> @@ -360,6 +360,10 @@ int cpufreq_table_validate_and_sort(struct cpufreq_policy *policy)
>         if (policy_has_boost_freq(policy))
>                 policy->boost_supported = true;
> 
> +       if (policy->freq_table_sorted == CPUFREQ_TABLE_SORTED_ASCENDING ||
> +           policy->freq_table_sorted == CPUFREQ_TABLE_SORTED_DESCENDING)
> +               return 0;
> +
>         return set_freq_table_sorted(policy);
>  }

Posted as:

https://lore.kernel.org/all/65ba5c45749267c82e8a87af3dc788b37a0b3f48.1773998611.git.viresh.kumar@linaro.org/

-- 
viresh

Re: [PATCH v6 1/4] cpufreq: Remove per-CPU QoS constraint

[Pierre Gondois]

Hello Viresh,

On 3/20/26 10:18, Viresh Kumar wrote:
> On 19-03-26, 10:30, Pierre Gondois wrote:
>> On 3/18/26 12:13, Viresh Kumar wrote:
>>> On 17-03-26, 11:17, 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.
>>> I don't think the above is required anymore. This patch is removing stale code
>>> now which isn't useful anymore. It has nothing to do with a boost specific QOS
>>> constraint.
>> Yes ok
>>> And it would be better to know for sure why this isn't required anymore and
>>> which patch exactly fixed this issue.
>>>
>> On a kernel based on 1608f0230510~, and replicating the
>> process described in the commit message of
>>
>> commit 1608f0230510 ("cpufreq: Fix re-boost issue after hotplugging
>> a CPU")
>>
>> I could not see any issue regarding the values of:
>>
>> - policy1/cpuinfo_max_freq
>> - policy1/scaling_max_freq
> The commit message (of 1608f0230510) is confusing. The issue was discussed
> properly in the following thread.
>
> https://lore.kernel.org/all/20250120082723.am7rxujmdvzz4eky@vireshk-i7/
>
> The problem is that policy->max and policy->cpuinfo_max_freq are incorrect after
> the sequence mentioned in the commit, while max_freq_req is correct.

I experimented a bit more and it seems the following happens:

1. boost all CPUs: echo 1 > /sys/devices/system/cpu/cpufreq/boost
2. offline one CPU: echo 0 > /sys/devices/system/cpu/cpuX/online
3. deboost all CPUs: echo 0 > /sys/devices/system/cpu/cpufreq/boost

cpufreq_boost_trigger_state()
\-for_each_active_policy()
   \-cpufreq_driver->set_boost()
doesn't act on the policy where there are no more online CPUs,
so the max/cpuinfo.max/max_freq_req is left to the actual
boost freq.

4. online CPUX: echo 1 > /sys/devices/system/cpu/cpuX/online

cpufreq_online()
\-cpufreq_driver->init()
   \-cppc_cpufreq_cpu_init()
There:
- policy->max
- policy->cpuinfo.max_freq
are set to the maximal non-boost freq., which is the correct value.

However, max_freq_req is left to the boosted frequency, so this
is effectively an incorrect state.
Also in cpufreq_set_policy(), policy->max is set to:
   min(max_freq_req, cpuinfo.max_freq)
(cf. verify() cb), so the incorrect state of max_freq_req is not
visible.

5. boost all CPUs again: echo 1 > /sys/devices/system/cpu/cpufreq/boost

As the max_freq_req value and the new boost value are equal,
cpufreq_notifier_max() won't be called, which means that if the
CPU needed to raise its freq., it won't be notified until another
event trigger a re-evaluation of the freq. selection.

To observe that, I had to:
- use the performance governor to be sure to select the max. available
   freq.
- observe scaling_cur_freq to see the last requested freq. for the CPU

So IMO the issue was actually fixed by:
dd016f379ebc ("cpufreq: Introduce a more generic way to set default
per-policy boost flag")
which sets the correct max_freq_req value when putting back an
inactive policy.


>
> I think another commit has fixed that (incorrectly and unintentionally):
> commit 6db0f533d320 ("cpufreq: preserve freq_table_sorted across suspend/hibernate")
>
> @@ -1421,9 +1421,12 @@ static int cpufreq_policy_online(struct cpufreq_policy *policy,
>                   * If there is a problem with its frequency table, take it
>                   * offline and drop it.
>                   */
> -               ret = cpufreq_table_validate_and_sort(policy);
> -               if (ret)
> -                       goto out_offline_policy;
> +               if (policy->freq_table_sorted != CPUFREQ_TABLE_SORTED_ASCENDING &&
> +                   policy->freq_table_sorted != CPUFREQ_TABLE_SORTED_DESCENDING) {
> +                       ret = cpufreq_table_validate_and_sort(policy);
> +                       if (ret)
> +                               goto out_offline_policy;
> +               }
>
> This skipped calling cpufreq_table_validate_and_sort() completely on online and
> so max/cpuinfo_max_freq, max_freq_req are all in sync.
>
> That change should be fixed with:
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 277884d91913..1f794524a1d9 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -1427,12 +1427,9 @@ static int cpufreq_policy_online(struct cpufreq_policy *policy,
>                   * If there is a problem with its frequency table, take it
>                   * offline and drop it.
>                   */
> -               if (policy->freq_table_sorted != CPUFREQ_TABLE_SORTED_ASCENDING &&
> -                   policy->freq_table_sorted != CPUFREQ_TABLE_SORTED_DESCENDING) {
> -                       ret = cpufreq_table_validate_and_sort(policy);
> -                       if (ret)
> -                               goto out_offline_policy;
> -               }
> +               ret = cpufreq_table_validate_and_sort(policy);
> +               if (ret)
> +                       goto out_offline_policy;
>
>                  /* related_cpus should at least include policy->cpus. */
>                  cpumask_copy(policy->related_cpus, policy->cpus);
> diff --git a/drivers/cpufreq/freq_table.c b/drivers/cpufreq/freq_table.c
> index 7f251daf03ce..5b364d8da4f9 100644
> --- a/drivers/cpufreq/freq_table.c
> +++ b/drivers/cpufreq/freq_table.c
> @@ -360,6 +360,10 @@ int cpufreq_table_validate_and_sort(struct cpufreq_policy *policy)
>          if (policy_has_boost_freq(policy))
>                  policy->boost_supported = true;
>
> +       if (policy->freq_table_sorted == CPUFREQ_TABLE_SORTED_ASCENDING ||
> +           policy->freq_table_sorted == CPUFREQ_TABLE_SORTED_DESCENDING)
> +               return 0;
> +
>          return set_freq_table_sorted(policy);
>   }
>

Re: [PATCH v6 1/4] cpufreq: Remove per-CPU QoS constraint

[Viresh Kumar]

On 24-03-26, 12:39, Pierre Gondois wrote:
> I experimented a bit more and it seems the following happens:
> 
> 1. boost all CPUs: echo 1 > /sys/devices/system/cpu/cpufreq/boost
> 2. offline one CPU: echo 0 > /sys/devices/system/cpu/cpuX/online
> 3. deboost all CPUs: echo 0 > /sys/devices/system/cpu/cpufreq/boost
> 
> cpufreq_boost_trigger_state()
> \-for_each_active_policy()
>   \-cpufreq_driver->set_boost()
> doesn't act on the policy where there are no more online CPUs,
> so the max/cpuinfo.max/max_freq_req is left to the actual
> boost freq.

Right.

> 4. online CPUX: echo 1 > /sys/devices/system/cpu/cpuX/online
> 
> cpufreq_online()
> \-cpufreq_driver->init()
>   \-cppc_cpufreq_cpu_init()
> There:
> - policy->max
> - policy->cpuinfo.max_freq
> are set to the maximal non-boost freq., which is the correct value.

Right, but that is true only for CPPC. Not all drivers would set max freq from
there and depend on cpufreq_table_validate_and_sort() to fix this. Which was
broken earlier, but with my patch it should be fixed now.

So yes, we will have max/cpuinfo.max_freq set at this point (with my patch
applied) for all drivers.

> However, max_freq_req is left to the boosted frequency, so this
> is effectively an incorrect state.

Right.

> Also in cpufreq_set_policy(), policy->max is set to:
>   min(max_freq_req, cpuinfo.max_freq)
> (cf. verify() cb), so the incorrect state of max_freq_req is not
> visible.
> 
> 5. boost all CPUs again: echo 1 > /sys/devices/system/cpu/cpufreq/boost
> 
> As the max_freq_req value and the new boost value are equal,
> cpufreq_notifier_max() won't be called, which means that if the
> CPU needed to raise its freq., it won't be notified until another
> event trigger a re-evaluation of the freq. selection.

Right and max/cpuinfo.max_freq will be incorrect here. This was the issue the
commit 1608f0230510 was trying to fix.

> To observe that, I had to:
> - use the performance governor to be sure to select the max. available
>   freq.
> - observe scaling_cur_freq to see the last requested freq. for the CPU
> 
> So IMO the issue was actually fixed by:
> dd016f379ebc ("cpufreq: Introduce a more generic way to set default
> per-policy boost flag")
> which sets the correct max_freq_req value when putting back an
> inactive policy.

Right. This patch fixes the issue I think. Your patch is fine I guess, just
mention this reason and we are good to go.

-- 
viresh

Re: [PATCH v6 1/4] cpufreq: Remove per-CPU QoS constraint

[zhenglifeng (A)]

On 3/19/2026 5:30 PM, Pierre Gondois wrote:
> 
> On 3/18/26 12:13, Viresh Kumar wrote:
>> On 17-03-26, 11:17, 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.
>> I don't think the above is required anymore. This patch is removing stale code
>> now which isn't useful anymore. It has nothing to do with a boost specific QOS
>> constraint.
> Yes ok
>> And it would be better to know for sure why this isn't required anymore and
>> which patch exactly fixed this issue.
>>
> On a kernel based on 1608f0230510~, and replicating the
> process described in the commit message of
> 
> commit 1608f0230510 ("cpufreq: Fix re-boost issue after hotplugging
> a CPU")
> 
> I could not see any issue regarding the values of:
> 
> - policy1/cpuinfo_max_freq
> - policy1/scaling_max_freq
> 
> The following sequence however had an issue:
> 
> 1. echo 0 > /sys/devices/system/cpu/cpu1/online
> 2. echo 1 > /sys/devices/system/cpu/cpufreq/boost
> 3. echo 1 > /sys/devices/system/cpu/cpu1/online
> 
> as after 1.:
> 
> cpufreq_boost_trigger_state()
> \-for_each_active_policy()
> 
> doesn't enable boost for inactive policies. This leads to
> CPU1 having the non-boosted frequency as its max freq.
> 
> The above sequence is fixed by:
> 
> commit a153c6049ab8 ("cpufreq: Introduce a more
> generic way to set default per-policy boost flag")

Yes, I think this commit fixed the issue but I didn't realize it before so
I sent them both. 😂

> 
> ---
> 
> @Lifeng, should I check something else than the value of:
> 
> - policy1/cpuinfo_max_freq
> - policy1/scaling_max_freq
> 
> in order to reproduce the issue fixed by:
> 
> commit 1608f0230510 ("cpufreq: Fix re-boost issue after hotplugging
> a CPU")
> 
> ?
> 
> 
> 

[PATCH v6 2/4] 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.

policy_set_boost() calls the cpufreq set_boost callback.
Update the newly added boost_freq_req request from there:
- whenever boost is toggled
- to cover all possible paths

In the existing .set_boost() callbacks:
- Don't update policy->max as this is done through the qos notifier
  cpufreq_notifier_max() which calls cpufreq_set_policy().
- Remove freq_qos_update_request() calls as the qos request is now
  done in policy_set_boost() and updates the new boost_freq_req

$ ## Init state
scaling_max_freq:1000000
cpuinfo_max_freq:1000000

$ echo 700000 > scaling_max_freq
scaling_max_freq:700000
cpuinfo_max_freq:1000000

$ echo 1 > ../boost
scaling_max_freq:1200000
cpuinfo_max_freq:1200000

$ echo 800000 > scaling_max_freq
scaling_max_freq:800000
cpuinfo_max_freq:1200000

$ ## Final step:
$ ## Without the patches:
$ echo 0 > ../boost
scaling_max_freq:1000000
cpuinfo_max_freq:1000000

$ ## With the patches:
$ echo 0 > ../boost
scaling_max_freq:800000
cpuinfo_max_freq:1000000

Note:
cpufreq_frequency_table_cpuinfo() updates policy->min
and max from:
A.
cpufreq_boost_set_sw()
\-cpufreq_frequency_table_cpuinfo()
B.
cpufreq_policy_online()
\-cpufreq_table_validate_and_sort()
  \-cpufreq_frequency_table_cpuinfo()
Keep these updates as some drivers expect policy->min and
max to be set through B.

Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
---
 drivers/cpufreq/amd-pstate.c   |  2 --
 drivers/cpufreq/cppc_cpufreq.c | 10 ++-----
 drivers/cpufreq/cpufreq.c      | 52 +++++++++++++++++++++++++++-------
 include/linux/cpufreq.h        |  1 +
 4 files changed, 44 insertions(+), 21 deletions(-)

diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index c45bc98721d24..310d5938cbdf6 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -756,8 +756,6 @@ static int amd_pstate_cpu_boost_update(struct cpufreq_policy *policy, bool on)
 	else if (policy->cpuinfo.max_freq > nominal_freq)
 		policy->cpuinfo.max_freq = nominal_freq;
 
-	policy->max = policy->cpuinfo.max_freq;
-
 	if (cppc_state == AMD_PSTATE_PASSIVE) {
 		ret = freq_qos_update_request(&cpudata->req[1], policy->cpuinfo.max_freq);
 		if (ret < 0)
diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
index 9eac77c4f2944..4c46c7ea318eb 100644
--- a/drivers/cpufreq/cppc_cpufreq.c
+++ b/drivers/cpufreq/cppc_cpufreq.c
@@ -775,17 +775,11 @@ static int cppc_cpufreq_set_boost(struct cpufreq_policy *policy, int state)
 {
 	struct cppc_cpudata *cpu_data = policy->driver_data;
 	struct cppc_perf_caps *caps = &cpu_data->perf_caps;
-	int ret;
 
 	if (state)
-		policy->max = cppc_perf_to_khz(caps, caps->highest_perf);
+		policy->cpuinfo.max_freq = cppc_perf_to_khz(caps, caps->highest_perf);
 	else
-		policy->max = cppc_perf_to_khz(caps, caps->nominal_perf);
-	policy->cpuinfo.max_freq = policy->max;
-
-	ret = freq_qos_update_request(policy->max_freq_req, policy->max);
-	if (ret < 0)
-		return ret;
+		policy->cpuinfo.max_freq = cppc_perf_to_khz(caps, caps->nominal_perf);
 
 	return 0;
 }
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index db414c052658b..e4f24754df164 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -603,10 +603,19 @@ static int policy_set_boost(struct cpufreq_policy *policy, bool enable)
 	policy->boost_enabled = enable;
 
 	ret = cpufreq_driver->set_boost(policy, enable);
-	if (ret)
+	if (ret) {
 		policy->boost_enabled = !policy->boost_enabled;
+		return ret;
+	}
 
-	return ret;
+	ret = freq_qos_update_request(policy->boost_freq_req, policy->cpuinfo.max_freq);
+	if (ret < 0) {
+		policy->boost_enabled = !policy->boost_enabled;
+		cpufreq_driver->set_boost(policy, policy->boost_enabled);
+		return ret;
+	}
+
+	return 0;
 }
 
 static ssize_t store_local_boost(struct cpufreq_policy *policy,
@@ -1370,6 +1379,9 @@ static void cpufreq_policy_free(struct cpufreq_policy *policy)
 		freq_qos_remove_request(policy->max_freq_req);
 	}
 
+	if (policy->boost_freq_req)
+		freq_qos_remove_request(policy->boost_freq_req);
+
 	freq_qos_remove_request(policy->min_freq_req);
 	kfree(policy->min_freq_req);
 
@@ -1439,12 +1451,15 @@ static int cpufreq_policy_online(struct cpufreq_policy *policy,
 	cpumask_and(policy->cpus, policy->cpus, cpu_online_mask);
 
 	if (new_policy) {
+		unsigned int req_nr;
+
 		for_each_cpu(j, policy->related_cpus) {
 			per_cpu(cpufreq_cpu_data, j) = policy;
 			add_cpu_dev_symlink(policy, j, get_cpu_device(j));
 		}
 
-		policy->min_freq_req = kzalloc(2 * sizeof(*policy->min_freq_req),
+		req_nr = policy->boost_supported ? 3 : 2;
+		policy->min_freq_req = kzalloc(req_nr * sizeof(*policy->min_freq_req),
 					       GFP_KERNEL);
 		if (!policy->min_freq_req) {
 			ret = -ENOMEM;
@@ -1479,6 +1494,27 @@ static int cpufreq_policy_online(struct cpufreq_policy *policy,
 			goto out_destroy_policy;
 		}
 
+		if (policy->boost_supported) {
+			policy->boost_freq_req = policy->max_freq_req + 1;
+
+			/*
+			 * If boost is supported,
+			 * init the constraint with cpuinfo.max_freq.
+			 */
+			ret = freq_qos_add_request(&policy->constraints,
+						   policy->boost_freq_req,
+						   FREQ_QOS_MAX,
+						   policy->cpuinfo.max_freq);
+			if (ret < 0) {
+				/*
+				 * So we don't call freq_qos_remove_request() for an
+				 * uninitialized request.
+				 */
+				policy->boost_freq_req = NULL;
+				goto out_destroy_policy;
+			}
+		}
+
 		blocking_notifier_call_chain(&cpufreq_policy_notifier_list,
 				CPUFREQ_CREATE_POLICY, policy);
 	}
@@ -2782,16 +2818,10 @@ int cpufreq_boost_set_sw(struct cpufreq_policy *policy, int state)
 		return -ENXIO;
 
 	ret = cpufreq_frequency_table_cpuinfo(policy);
-	if (ret) {
+	if (ret)
 		pr_err("%s: Policy frequency update failed\n", __func__);
-		return ret;
-	}
-
-	ret = freq_qos_update_request(policy->max_freq_req, policy->max);
-	if (ret < 0)
-		return ret;
 
-	return 0;
+	return ret;
 }
 EXPORT_SYMBOL_GPL(cpufreq_boost_set_sw);
 
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index 0465d1e6f72ac..c292a6a19e4f5 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -81,6 +81,7 @@ struct cpufreq_policy {
 	struct freq_constraints	constraints;
 	struct freq_qos_request	*min_freq_req;
 	struct freq_qos_request	*max_freq_req;
+	struct freq_qos_request *boost_freq_req;
 
 	struct cpufreq_frequency_table	*freq_table;
 	enum cpufreq_table_sorting freq_table_sorted;
-- 
2.43.0

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

[zhenglifeng (A)]

On 3/17/2026 6:17 PM, Pierre Gondois wrote:
> The Power Management Quality of Service (PM QoS) allows to
> aggregate constraints from multiple entities. It is currently
> used to manage the min/max frequency of a given policy.
> 
> Frequency constraints can come for instance from:
> - Thermal framework: acpi_thermal_cpufreq_init()
> - Firmware: _PPC objects: acpi_processor_ppc_init()
> - User: by setting policyX/scaling_[min|max]_freq
> The minimum of the max frequency constraints is used to compute
> the resulting maximum allowed frequency.
> 
> When enabling boost frequencies, the same frequency request object
> (policy->max_freq_req) as to handle requests from users is used.
> As a result, when setting:
> - scaling_max_freq
> - boost
> The last sysfs file used overwrites the request from the other
> sysfs file.
> 
> To avoid this, create a per-policy boost_freq_req to save the boost
> constraints instead of overwriting the last scaling_max_freq
> constraint.
> 
> policy_set_boost() calls the cpufreq set_boost callback.
> Update the newly added boost_freq_req request from there:
> - whenever boost is toggled
> - to cover all possible paths
> 
> In the existing .set_boost() callbacks:
> - Don't update policy->max as this is done through the qos notifier
>   cpufreq_notifier_max() which calls cpufreq_set_policy().
> - Remove freq_qos_update_request() calls as the qos request is now
>   done in policy_set_boost() and updates the new boost_freq_req
> 
> $ ## Init state
> scaling_max_freq:1000000
> cpuinfo_max_freq:1000000
> 
> $ echo 700000 > scaling_max_freq
> scaling_max_freq:700000
> cpuinfo_max_freq:1000000
> 
> $ echo 1 > ../boost
> scaling_max_freq:1200000
> cpuinfo_max_freq:1200000
> 
> $ echo 800000 > scaling_max_freq
> scaling_max_freq:800000
> cpuinfo_max_freq:1200000
> 
> $ ## Final step:
> $ ## Without the patches:
> $ echo 0 > ../boost
> scaling_max_freq:1000000
> cpuinfo_max_freq:1000000
> 
> $ ## With the patches:
> $ echo 0 > ../boost
> scaling_max_freq:800000
> cpuinfo_max_freq:1000000
> 
> Note:
> cpufreq_frequency_table_cpuinfo() updates policy->min
> and max from:
> A.
> cpufreq_boost_set_sw()
> \-cpufreq_frequency_table_cpuinfo()
> B.
> cpufreq_policy_online()
> \-cpufreq_table_validate_and_sort()
>   \-cpufreq_frequency_table_cpuinfo()
> Keep these updates as some drivers expect policy->min and
> max to be set through B.
> 
> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
> ---
>  drivers/cpufreq/amd-pstate.c   |  2 --
>  drivers/cpufreq/cppc_cpufreq.c | 10 ++-----
>  drivers/cpufreq/cpufreq.c      | 52 +++++++++++++++++++++++++++-------
>  include/linux/cpufreq.h        |  1 +
>  4 files changed, 44 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index c45bc98721d24..310d5938cbdf6 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -756,8 +756,6 @@ static int amd_pstate_cpu_boost_update(struct cpufreq_policy *policy, bool on)
>  	else if (policy->cpuinfo.max_freq > nominal_freq)
>  		policy->cpuinfo.max_freq = nominal_freq;
>  
> -	policy->max = policy->cpuinfo.max_freq;
> -
>  	if (cppc_state == AMD_PSTATE_PASSIVE) {
>  		ret = freq_qos_update_request(&cpudata->req[1], policy->cpuinfo.max_freq);
>  		if (ret < 0)
> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
> index 9eac77c4f2944..4c46c7ea318eb 100644
> --- a/drivers/cpufreq/cppc_cpufreq.c
> +++ b/drivers/cpufreq/cppc_cpufreq.c
> @@ -775,17 +775,11 @@ static int cppc_cpufreq_set_boost(struct cpufreq_policy *policy, int state)
>  {
>  	struct cppc_cpudata *cpu_data = policy->driver_data;
>  	struct cppc_perf_caps *caps = &cpu_data->perf_caps;
> -	int ret;
>  
>  	if (state)
> -		policy->max = cppc_perf_to_khz(caps, caps->highest_perf);
> +		policy->cpuinfo.max_freq = cppc_perf_to_khz(caps, caps->highest_perf);
>  	else
> -		policy->max = cppc_perf_to_khz(caps, caps->nominal_perf);
> -	policy->cpuinfo.max_freq = policy->max;
> -
> -	ret = freq_qos_update_request(policy->max_freq_req, policy->max);
> -	if (ret < 0)
> -		return ret;
> +		policy->cpuinfo.max_freq = cppc_perf_to_khz(caps, caps->nominal_perf);
>  
>  	return 0;
>  }
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index db414c052658b..e4f24754df164 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -603,10 +603,19 @@ static int policy_set_boost(struct cpufreq_policy *policy, bool enable)
>  	policy->boost_enabled = enable;
>  
>  	ret = cpufreq_driver->set_boost(policy, enable);
> -	if (ret)
> +	if (ret) {
>  		policy->boost_enabled = !policy->boost_enabled;
> +		return ret;
> +	}
>  
> -	return ret;
> +	ret = freq_qos_update_request(policy->boost_freq_req, policy->cpuinfo.max_freq);
> +	if (ret < 0) {
> +		policy->boost_enabled = !policy->boost_enabled;
> +		cpufreq_driver->set_boost(policy, policy->boost_enabled);
> +		return ret;
> +	}
> +
> +	return 0;
>  }
>  
>  static ssize_t store_local_boost(struct cpufreq_policy *policy,
> @@ -1370,6 +1379,9 @@ static void cpufreq_policy_free(struct cpufreq_policy *policy)
>  		freq_qos_remove_request(policy->max_freq_req);
>  	}
>  
> +	if (policy->boost_freq_req)
> +		freq_qos_remove_request(policy->boost_freq_req);
> +
>  	freq_qos_remove_request(policy->min_freq_req);
>  	kfree(policy->min_freq_req);
>  
> @@ -1439,12 +1451,15 @@ static int cpufreq_policy_online(struct cpufreq_policy *policy,
>  	cpumask_and(policy->cpus, policy->cpus, cpu_online_mask);
>  
>  	if (new_policy) {
> +		unsigned int req_nr;
> +
>  		for_each_cpu(j, policy->related_cpus) {
>  			per_cpu(cpufreq_cpu_data, j) = policy;
>  			add_cpu_dev_symlink(policy, j, get_cpu_device(j));
>  		}
>  
> -		policy->min_freq_req = kzalloc(2 * sizeof(*policy->min_freq_req),
> +		req_nr = policy->boost_supported ? 3 : 2;
> +		policy->min_freq_req = kzalloc(req_nr * sizeof(*policy->min_freq_req),
>  					       GFP_KERNEL);
>  		if (!policy->min_freq_req) {
>  			ret = -ENOMEM;
> @@ -1479,6 +1494,27 @@ static int cpufreq_policy_online(struct cpufreq_policy *policy,
>  			goto out_destroy_policy;
>  		}
>  
> +		if (policy->boost_supported) {
> +			policy->boost_freq_req = policy->max_freq_req + 1;
> +
> +			/*
> +			 * If boost is supported,
> +			 * init the constraint with cpuinfo.max_freq.
> +			 */
> +			ret = freq_qos_add_request(&policy->constraints,
> +						   policy->boost_freq_req,
> +						   FREQ_QOS_MAX,
> +						   policy->cpuinfo.max_freq);
> +			if (ret < 0) {
> +				/*
> +				 * So we don't call freq_qos_remove_request() for an
> +				 * uninitialized request.
> +				 */
> +				policy->boost_freq_req = NULL;
> +				goto out_destroy_policy;
> +			}
> +		}
> +

Something we discussed before has resurfaced here. CPUFREQ_REMOVE_POLICY
notification will be sent without sending CPUFREQ_CREATE_POLICY
notification before if adding boost_freq_req fails.

What about adding boost_freq_req before adding min_freq_req and removing it
after removing min_freq_req?

>  		blocking_notifier_call_chain(&cpufreq_policy_notifier_list,
>  				CPUFREQ_CREATE_POLICY, policy);
>  	}
> @@ -2782,16 +2818,10 @@ int cpufreq_boost_set_sw(struct cpufreq_policy *policy, int state)
>  		return -ENXIO;
>  
>  	ret = cpufreq_frequency_table_cpuinfo(policy);
> -	if (ret) {
> +	if (ret)
>  		pr_err("%s: Policy frequency update failed\n", __func__);
> -		return ret;
> -	}
> -
> -	ret = freq_qos_update_request(policy->max_freq_req, policy->max);
> -	if (ret < 0)
> -		return ret;
>  
> -	return 0;
> +	return ret;
>  }
>  EXPORT_SYMBOL_GPL(cpufreq_boost_set_sw);
>  
> diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
> index 0465d1e6f72ac..c292a6a19e4f5 100644
> --- a/include/linux/cpufreq.h
> +++ b/include/linux/cpufreq.h
> @@ -81,6 +81,7 @@ struct cpufreq_policy {
>  	struct freq_constraints	constraints;
>  	struct freq_qos_request	*min_freq_req;
>  	struct freq_qos_request	*max_freq_req;
> +	struct freq_qos_request *boost_freq_req;
>  
>  	struct cpufreq_frequency_table	*freq_table;
>  	enum cpufreq_table_sorting freq_table_sorted;

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

[Pierre Gondois]

On 3/18/26 03:50, zhenglifeng (A) wrote:
> On 3/17/2026 6:17 PM, Pierre Gondois wrote:
>> The Power Management Quality of Service (PM QoS) allows to
>> aggregate constraints from multiple entities. It is currently
>> used to manage the min/max frequency of a given policy.
>>
>> Frequency constraints can come for instance from:
>> - Thermal framework: acpi_thermal_cpufreq_init()
>> - Firmware: _PPC objects: acpi_processor_ppc_init()
>> - User: by setting policyX/scaling_[min|max]_freq
>> The minimum of the max frequency constraints is used to compute
>> the resulting maximum allowed frequency.
>>
>> When enabling boost frequencies, the same frequency request object
>> (policy->max_freq_req) as to handle requests from users is used.
>> As a result, when setting:
>> - scaling_max_freq
>> - boost
>> The last sysfs file used overwrites the request from the other
>> sysfs file.
>>
>> To avoid this, create a per-policy boost_freq_req to save the boost
>> constraints instead of overwriting the last scaling_max_freq
>> constraint.
>>
>> policy_set_boost() calls the cpufreq set_boost callback.
>> Update the newly added boost_freq_req request from there:
>> - whenever boost is toggled
>> - to cover all possible paths
>>
>> In the existing .set_boost() callbacks:
>> - Don't update policy->max as this is done through the qos notifier
>>    cpufreq_notifier_max() which calls cpufreq_set_policy().
>> - Remove freq_qos_update_request() calls as the qos request is now
>>    done in policy_set_boost() and updates the new boost_freq_req
>>
>> $ ## Init state
>> scaling_max_freq:1000000
>> cpuinfo_max_freq:1000000
>>
>> $ echo 700000 > scaling_max_freq
>> scaling_max_freq:700000
>> cpuinfo_max_freq:1000000
>>
>> $ echo 1 > ../boost
>> scaling_max_freq:1200000
>> cpuinfo_max_freq:1200000
>>
>> $ echo 800000 > scaling_max_freq
>> scaling_max_freq:800000
>> cpuinfo_max_freq:1200000
>>
>> $ ## Final step:
>> $ ## Without the patches:
>> $ echo 0 > ../boost
>> scaling_max_freq:1000000
>> cpuinfo_max_freq:1000000
>>
>> $ ## With the patches:
>> $ echo 0 > ../boost
>> scaling_max_freq:800000
>> cpuinfo_max_freq:1000000
>>
>> Note:
>> cpufreq_frequency_table_cpuinfo() updates policy->min
>> and max from:
>> A.
>> cpufreq_boost_set_sw()
>> \-cpufreq_frequency_table_cpuinfo()
>> B.
>> cpufreq_policy_online()
>> \-cpufreq_table_validate_and_sort()
>>    \-cpufreq_frequency_table_cpuinfo()
>> Keep these updates as some drivers expect policy->min and
>> max to be set through B.
>>
>> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
>> ---
>>   drivers/cpufreq/amd-pstate.c   |  2 --
>>   drivers/cpufreq/cppc_cpufreq.c | 10 ++-----
>>   drivers/cpufreq/cpufreq.c      | 52 +++++++++++++++++++++++++++-------
>>   include/linux/cpufreq.h        |  1 +
>>   4 files changed, 44 insertions(+), 21 deletions(-)
>>
>> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
>> index c45bc98721d24..310d5938cbdf6 100644
>> --- a/drivers/cpufreq/amd-pstate.c
>> +++ b/drivers/cpufreq/amd-pstate.c
>> @@ -756,8 +756,6 @@ static int amd_pstate_cpu_boost_update(struct cpufreq_policy *policy, bool on)
>>   	else if (policy->cpuinfo.max_freq > nominal_freq)
>>   		policy->cpuinfo.max_freq = nominal_freq;
>>   
>> -	policy->max = policy->cpuinfo.max_freq;
>> -
>>   	if (cppc_state == AMD_PSTATE_PASSIVE) {
>>   		ret = freq_qos_update_request(&cpudata->req[1], policy->cpuinfo.max_freq);
>>   		if (ret < 0)
>> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
>> index 9eac77c4f2944..4c46c7ea318eb 100644
>> --- a/drivers/cpufreq/cppc_cpufreq.c
>> +++ b/drivers/cpufreq/cppc_cpufreq.c
>> @@ -775,17 +775,11 @@ static int cppc_cpufreq_set_boost(struct cpufreq_policy *policy, int state)
>>   {
>>   	struct cppc_cpudata *cpu_data = policy->driver_data;
>>   	struct cppc_perf_caps *caps = &cpu_data->perf_caps;
>> -	int ret;
>>   
>>   	if (state)
>> -		policy->max = cppc_perf_to_khz(caps, caps->highest_perf);
>> +		policy->cpuinfo.max_freq = cppc_perf_to_khz(caps, caps->highest_perf);
>>   	else
>> -		policy->max = cppc_perf_to_khz(caps, caps->nominal_perf);
>> -	policy->cpuinfo.max_freq = policy->max;
>> -
>> -	ret = freq_qos_update_request(policy->max_freq_req, policy->max);
>> -	if (ret < 0)
>> -		return ret;
>> +		policy->cpuinfo.max_freq = cppc_perf_to_khz(caps, caps->nominal_perf);
>>   
>>   	return 0;
>>   }
>> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
>> index db414c052658b..e4f24754df164 100644
>> --- a/drivers/cpufreq/cpufreq.c
>> +++ b/drivers/cpufreq/cpufreq.c
>> @@ -603,10 +603,19 @@ static int policy_set_boost(struct cpufreq_policy *policy, bool enable)
>>   	policy->boost_enabled = enable;
>>   
>>   	ret = cpufreq_driver->set_boost(policy, enable);
>> -	if (ret)
>> +	if (ret) {
>>   		policy->boost_enabled = !policy->boost_enabled;
>> +		return ret;
>> +	}
>>   
>> -	return ret;
>> +	ret = freq_qos_update_request(policy->boost_freq_req, policy->cpuinfo.max_freq);
>> +	if (ret < 0) {
>> +		policy->boost_enabled = !policy->boost_enabled;
>> +		cpufreq_driver->set_boost(policy, policy->boost_enabled);
>> +		return ret;
>> +	}
>> +
>> +	return 0;
>>   }
>>   
>>   static ssize_t store_local_boost(struct cpufreq_policy *policy,
>> @@ -1370,6 +1379,9 @@ static void cpufreq_policy_free(struct cpufreq_policy *policy)
>>   		freq_qos_remove_request(policy->max_freq_req);
>>   	}
>>   
>> +	if (policy->boost_freq_req)
>> +		freq_qos_remove_request(policy->boost_freq_req);
>> +
>>   	freq_qos_remove_request(policy->min_freq_req);
>>   	kfree(policy->min_freq_req);
>>   
>> @@ -1439,12 +1451,15 @@ static int cpufreq_policy_online(struct cpufreq_policy *policy,
>>   	cpumask_and(policy->cpus, policy->cpus, cpu_online_mask);
>>   
>>   	if (new_policy) {
>> +		unsigned int req_nr;
>> +
>>   		for_each_cpu(j, policy->related_cpus) {
>>   			per_cpu(cpufreq_cpu_data, j) = policy;
>>   			add_cpu_dev_symlink(policy, j, get_cpu_device(j));
>>   		}
>>   
>> -		policy->min_freq_req = kzalloc(2 * sizeof(*policy->min_freq_req),
>> +		req_nr = policy->boost_supported ? 3 : 2;
>> +		policy->min_freq_req = kzalloc(req_nr * sizeof(*policy->min_freq_req),
>>   					       GFP_KERNEL);
>>   		if (!policy->min_freq_req) {
>>   			ret = -ENOMEM;
>> @@ -1479,6 +1494,27 @@ static int cpufreq_policy_online(struct cpufreq_policy *policy,
>>   			goto out_destroy_policy;
>>   		}
>>   
>> +		if (policy->boost_supported) {
>> +			policy->boost_freq_req = policy->max_freq_req + 1;
>> +
>> +			/*
>> +			 * If boost is supported,
>> +			 * init the constraint with cpuinfo.max_freq.
>> +			 */
>> +			ret = freq_qos_add_request(&policy->constraints,
>> +						   policy->boost_freq_req,
>> +						   FREQ_QOS_MAX,
>> +						   policy->cpuinfo.max_freq);
>> +			if (ret < 0) {
>> +				/*
>> +				 * So we don't call freq_qos_remove_request() for an
>> +				 * uninitialized request.
>> +				 */
>> +				policy->boost_freq_req = NULL;
>> +				goto out_destroy_policy;
>> +			}
>> +		}
>> +
> Something we discussed before has resurfaced here. CPUFREQ_REMOVE_POLICY
> notification will be sent without sending CPUFREQ_CREATE_POLICY
> notification before if adding boost_freq_req fails.
>
> What about adding boost_freq_req before adding min_freq_req and removing it
> after removing min_freq_req?
Yes right, this is indeed what Rafael suggested looking back at the 
messages.
I ll wait a bit before submitting a correction.


>
>>   		blocking_notifier_call_chain(&cpufreq_policy_notifier_list,
>>   				CPUFREQ_CREATE_POLICY, policy);
>>   	}
>> @@ -2782,16 +2818,10 @@ int cpufreq_boost_set_sw(struct cpufreq_policy *policy, int state)
>>   		return -ENXIO;
>>   
>>   	ret = cpufreq_frequency_table_cpuinfo(policy);
>> -	if (ret) {
>> +	if (ret)
>>   		pr_err("%s: Policy frequency update failed\n", __func__);
>> -		return ret;
>> -	}
>> -
>> -	ret = freq_qos_update_request(policy->max_freq_req, policy->max);
>> -	if (ret < 0)
>> -		return ret;
>>   
>> -	return 0;
>> +	return ret;
>>   }
>>   EXPORT_SYMBOL_GPL(cpufreq_boost_set_sw);
>>   
>> diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
>> index 0465d1e6f72ac..c292a6a19e4f5 100644
>> --- a/include/linux/cpufreq.h
>> +++ b/include/linux/cpufreq.h
>> @@ -81,6 +81,7 @@ struct cpufreq_policy {
>>   	struct freq_constraints	constraints;
>>   	struct freq_qos_request	*min_freq_req;
>>   	struct freq_qos_request	*max_freq_req;
>> +	struct freq_qos_request *boost_freq_req;
>>   
>>   	struct cpufreq_frequency_table	*freq_table;
>>   	enum cpufreq_table_sorting freq_table_sorted;

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

[Pierre Gondois]

cpufreq_set_policy() will ultimately override the policy min/max
values written in the .init() callback 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_init_policy()
  \-__cpufreq_driver_target()
    \-cpufreq_driver->target()
is called. In this case, some drivers use the policy min/max
values in their .target() callback before they are overridden.
This should only concern the sh-cpufreq driver, so policy->min
and max values are replaced with their cpuinfo equivalent.

In this patch:
- Setting policy->min or max value in driver .init() cb is
  interpreted as setting a QoS constraint.
- All policy->min and max initialization is removed as the value
  is not used.
- For the cppc-cpufreq driver, the lowest non-linear freq. is
  used as a min QoS constraint as suggested at:
  https://lore.kernel.org/lkml/20260213100633.15413-1-zhangpengjie2@huawei.com/

Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
---
 drivers/cpufreq/amd-pstate.c      | 24 ++++++++++++------------
 drivers/cpufreq/cppc_cpufreq.c    | 11 +++++++----
 drivers/cpufreq/cpufreq-nforce2.c |  4 ++--
 drivers/cpufreq/cpufreq.c         | 15 +++++++++++++--
 drivers/cpufreq/freq_table.c      |  7 +++----
 drivers/cpufreq/gx-suspmod.c      |  9 ++++-----
 drivers/cpufreq/intel_pstate.c    |  3 ---
 drivers/cpufreq/pcc-cpufreq.c     |  8 ++++----
 drivers/cpufreq/pxa3xx-cpufreq.c  |  4 ++--
 drivers/cpufreq/sh-cpufreq.c      |  7 ++++---
 drivers/cpufreq/virtual-cpufreq.c |  5 +----
 11 files changed, 52 insertions(+), 45 deletions(-)

diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 310d5938cbdf6..aaafbe9b26cae 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -1003,12 +1003,12 @@ 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 = perf_to_freq(perf,
-							      cpudata->nominal_freq,
-							      perf.highest_perf);
+	policy->cpuinfo.min_freq = perf_to_freq(perf,
+						cpudata->nominal_freq,
+						perf.lowest_perf);
+	policy->cpuinfo.max_freq = perf_to_freq(perf,
+						cpudata->nominal_freq,
+						perf.highest_perf);
 
 	ret = amd_pstate_cppc_enable(policy);
 	if (ret)
@@ -1485,12 +1485,12 @@ 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 = perf_to_freq(perf,
-							      cpudata->nominal_freq,
-							      perf.highest_perf);
+	policy->cpuinfo.min_freq = perf_to_freq(perf,
+						cpudata->nominal_freq,
+						perf.lowest_perf);
+	policy->cpuinfo.max_freq = perf_to_freq(perf,
+						cpudata->nominal_freq,
+						perf.highest_perf);
 	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 4c46c7ea318eb..2bbf36516a0f0 100644
--- a/drivers/cpufreq/cppc_cpufreq.c
+++ b/drivers/cpufreq/cppc_cpufreq.c
@@ -586,6 +586,7 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy)
 	unsigned int cpu = policy->cpu;
 	struct cppc_cpudata *cpu_data;
 	struct cppc_perf_caps *caps;
+	unsigned int min, max;
 	int ret;
 
 	cpu_data = cppc_cpufreq_get_cpu_data(cpu);
@@ -596,13 +597,15 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy)
 	caps = &cpu_data->perf_caps;
 	policy->driver_data = cpu_data;
 
+	min = cppc_perf_to_khz(caps, caps->lowest_nonlinear_perf);
+	max = cppc_perf_to_khz(caps, policy->boost_enabled ?
+			       caps->highest_perf : caps->nominal_perf);
+
 	/*
 	 * 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);
+	policy->min = min;
 
 	/*
 	 * Set cpuinfo.min_freq to Lowest to make the full range of performance
@@ -610,7 +613,7 @@ 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 = max;
 
 	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/cpufreq.c b/drivers/cpufreq/cpufreq.c
index e4f24754df164..3059200766b0d 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1451,8 +1451,19 @@ static int cpufreq_policy_online(struct cpufreq_policy *policy,
 	cpumask_and(policy->cpus, policy->cpus, cpu_online_mask);
 
 	if (new_policy) {
+		unsigned int min, max;
 		unsigned int req_nr;
 
+		/*
+		 * If the driver has set policy->min or max,
+		 * use the value as a QoS request.
+		 */
+		min = max(FREQ_QOS_MIN_DEFAULT_VALUE, policy->min);
+		if (policy->max)
+			max = min(FREQ_QOS_MAX_DEFAULT_VALUE, policy->max);
+		else
+			max = FREQ_QOS_MAX_DEFAULT_VALUE;
+
 		for_each_cpu(j, policy->related_cpus) {
 			per_cpu(cpufreq_cpu_data, j) = policy;
 			add_cpu_dev_symlink(policy, j, get_cpu_device(j));
@@ -1468,7 +1479,7 @@ static int cpufreq_policy_online(struct cpufreq_policy *policy,
 
 		ret = freq_qos_add_request(&policy->constraints,
 					   policy->min_freq_req, FREQ_QOS_MIN,
-					   FREQ_QOS_MIN_DEFAULT_VALUE);
+					   min);
 		if (ret < 0) {
 			/*
 			 * So we don't call freq_qos_remove_request() for an
@@ -1488,7 +1499,7 @@ static int cpufreq_policy_online(struct cpufreq_policy *policy,
 
 		ret = freq_qos_add_request(&policy->constraints,
 					   policy->max_freq_req, FREQ_QOS_MAX,
-					   FREQ_QOS_MAX_DEFAULT_VALUE);
+					   max);
 		if (ret < 0) {
 			policy->max_freq_req = NULL;
 			goto out_destroy_policy;
diff --git a/drivers/cpufreq/freq_table.c b/drivers/cpufreq/freq_table.c
index 7f251daf03ce3..9b37f37c36389 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 75b3ef7ec6796..57999b8d51fa2 100644
--- a/drivers/cpufreq/gx-suspmod.c
+++ b/drivers/cpufreq/gx-suspmod.c
@@ -397,7 +397,7 @@ static int cpufreq_gx_target(struct cpufreq_policy *policy,
 
 static int cpufreq_gx_cpu_init(struct cpufreq_policy *policy)
 {
-	unsigned int maxfreq;
+	unsigned int minfreq, maxfreq;
 
 	if (!policy || policy->cpu != 0)
 		return -ENODEV;
@@ -418,11 +418,10 @@ static int cpufreq_gx_cpu_init(struct cpufreq_policy *policy)
 	policy->cpu = 0;
 
 	if (max_duration < POLICY_MIN_DIV)
-		policy->min = maxfreq / max_duration;
+		minfreq = maxfreq / max_duration;
 	else
-		policy->min = maxfreq / POLICY_MIN_DIV;
-	policy->max = maxfreq;
-	policy->cpuinfo.min_freq = maxfreq / max_duration;
+		minfreq = maxfreq / POLICY_MIN_DIV;
+	policy->cpuinfo.min_freq = minfreq;
 	policy->cpuinfo.max_freq = maxfreq;
 
 	return 0;
diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index ec4abe3745736..bf2f7524d04a9 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -3047,9 +3047,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..231edfe8cabaa 100644
--- a/drivers/cpufreq/pcc-cpufreq.c
+++ b/drivers/cpufreq/pcc-cpufreq.c
@@ -551,13 +551,13 @@ static int pcc_cpufreq_cpu_init(struct cpufreq_policy *policy)
 		goto out;
 	}
 
-	policy->max = policy->cpuinfo.max_freq =
+	policy->cpuinfo.max_freq =
 		ioread32(&pcch_hdr->nominal) * 1000;
-	policy->min = policy->cpuinfo.min_freq =
+	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 4afa48d172dbe..f53b9d7edc76a 100644
--- a/drivers/cpufreq/pxa3xx-cpufreq.c
+++ b/drivers/cpufreq/pxa3xx-cpufreq.c
@@ -185,8 +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 =
+	policy->cpuinfo.min_freq = 104000;
+	policy->cpuinfo.max_freq =
 		(cpu_is_pxa320()) ? 806000 : 624000;
 	policy->cpuinfo.transition_latency = 1000; /* FIXME: 1 ms, assumed */
 
diff --git a/drivers/cpufreq/sh-cpufreq.c b/drivers/cpufreq/sh-cpufreq.c
index 642ddb9ea217e..5cf0f482924d5 100644
--- a/drivers/cpufreq/sh-cpufreq.c
+++ b/drivers/cpufreq/sh-cpufreq.c
@@ -57,7 +57,8 @@ static long __sh_cpufreq_target(void *arg)
 	/* Convert target_freq from kHz to Hz */
 	freq = clk_round_rate(cpuclk, target->freq * 1000);
 
-	if (freq < (policy->min * 1000) || freq > (policy->max * 1000))
+	if (freq < (policy->cpuinfo.min_freq * 1000) ||
+	    freq > (policy->cpuinfo.max_freq * 1000))
 		return -EINVAL;
 
 	dev_dbg(dev, "requested frequency %u Hz\n", target->freq * 1000);
@@ -124,9 +125,9 @@ 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 =
+		policy->cpuinfo.min_freq =
 			(clk_round_rate(cpuclk, 1) + 500) / 1000;
-		policy->max = policy->cpuinfo.max_freq =
+		policy->cpuinfo.max_freq =
 			(clk_round_rate(cpuclk, ~0UL) + 500) / 1000;
 	}
 
diff --git a/drivers/cpufreq/virtual-cpufreq.c b/drivers/cpufreq/virtual-cpufreq.c
index 6ffa16d239b2b..60707bf6ce1e3 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

Re: [PATCH v6 3/4] cpufreq: Set policy->min and max as real QoS constraints

[Viresh Kumar]

On 17-03-26, 11:17, Pierre Gondois wrote:
> cpufreq_set_policy() will ultimately override the policy min/max
> values written in the .init() callback through:
> cpufreq_policy_online()
> \-cpufreq_init_policy()
>   \-cpufreq_set_policy()
>     \-/* Set policy->min/max */
> Thus the policy min/max values provided are only temporary.

I am not comfortable with this patch to be honest. policy->min/max are used at
so many places that it is really difficult to make sure if this patch will break
something or not.

For example:

cpufreq_set_policy()
 cpufreq_driver->verify()
  cpufreq_frequency_table_verify()

This uses min/max before it is set by the path you mentioned.

I would suggest dropping this change, or most of it and doing only what is
really required for this series.

-- 
viresh

Re: [PATCH v6 3/4] cpufreq: Set policy->min and max as real QoS constraints

[Pierre Gondois]

On 3/20/26 11:14, Viresh Kumar wrote:
> On 17-03-26, 11:17, Pierre Gondois wrote:
>> cpufreq_set_policy() will ultimately override the policy min/max
>> values written in the .init() callback through:
>> cpufreq_policy_online()
>> \-cpufreq_init_policy()
>>    \-cpufreq_set_policy()
>>      \-/* Set policy->min/max */
>> Thus the policy min/max values provided are only temporary.
> I am not comfortable with this patch to be honest. policy->min/max are used at
> so many places that it is really difficult to make sure if this patch will break
> something or not.
>
> For example:
>
> cpufreq_set_policy()
>   cpufreq_driver->verify()
>    cpufreq_frequency_table_verify()
>
> This uses min/max before it is set by the path you mentioned.
>
> I would suggest dropping this change, or most of it and doing only what is
> really required for this series.
>
Being able to set the min/max_freq_req from the driver might be
something that is needed, cf:
https://lore.kernel.org/lkml/20260213100633.15413-1-zhangpengjie2@huawei.com/

It would also allow to have a common way to set policy->min/max values
as they are set to the the cpuinfo.min/max_freq.

On the other hand I agree that I didn't test all the possible paths
for this change, so this is a bit audacious.


What about adding the following to have the values set for all drivers:

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c

index 70814c567243b..3a1e5f58a301f 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1530,6 +1530,9 @@ static int cpufreq_policy_online(struct 
cpufreq_policy *policy,
                                 CPUFREQ_CREATE_POLICY, policy);
         }

+       policy->max = policy->cpuinfo.max_freq;
+       policy->min = policy->cpuinfo.min_freq;
+
         if (cpufreq_driver->get && has_target()) {
                 policy->cur = cpufreq_driver->get(policy->cpu);
                 if (!policy->cur) {

Re: [PATCH v6 3/4] cpufreq: Set policy->min and max as real QoS constraints

[Viresh Kumar]

On 24-03-26, 12:40, Pierre Gondois wrote:
> Being able to set the min/max_freq_req from the driver might be
> something that is needed, cf:
> https://lore.kernel.org/lkml/20260213100633.15413-1-zhangpengjie2@huawei.com/
> 
> It would also allow to have a common way to set policy->min/max values
> as they are set to the the cpuinfo.min/max_freq.
> 
> On the other hand I agree that I didn't test all the possible paths
> for this change, so this is a bit audacious.
> 
> 
> What about adding the following to have the values set for all drivers:
> 
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> 
> index 70814c567243b..3a1e5f58a301f 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -1530,6 +1530,9 @@ static int cpufreq_policy_online(struct cpufreq_policy
> *policy,
>                                 CPUFREQ_CREATE_POLICY, policy);
>         }
> 
> +       policy->max = policy->cpuinfo.max_freq;
> +       policy->min = policy->cpuinfo.min_freq;
> +
>         if (cpufreq_driver->get && has_target()) {
>                 policy->cur = cpufreq_driver->get(policy->cpu);
>                 if (!policy->cur) {

I am okay with a separate series doing all these cleanups. We can set some sort
of rule on what is expected from the driver here and what is left from the core.
For example driver only sets cpuinfo.max/min and core takes care of rest. This
series would cleanup all the drivers, etc.

-- 
viresh

Re: [PATCH v6 3/4] cpufreq: Set policy->min and max as real QoS constraints

[Pierre Gondois]

On 3/25/26 07:49, Viresh Kumar wrote:
> On 24-03-26, 12:40, Pierre Gondois wrote:
>> Being able to set the min/max_freq_req from the driver might be
>> something that is needed, cf:
>> https://lore.kernel.org/lkml/20260213100633.15413-1-zhangpengjie2@huawei.com/
>>
>> It would also allow to have a common way to set policy->min/max values
>> as they are set to the the cpuinfo.min/max_freq.
>>
>> On the other hand I agree that I didn't test all the possible paths
>> for this change, so this is a bit audacious.
>>
>>
>> What about adding the following to have the values set for all drivers:
>>
>> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
>>
>> index 70814c567243b..3a1e5f58a301f 100644
>> --- a/drivers/cpufreq/cpufreq.c
>> +++ b/drivers/cpufreq/cpufreq.c
>> @@ -1530,6 +1530,9 @@ static int cpufreq_policy_online(struct cpufreq_policy
>> *policy,
>>                                  CPUFREQ_CREATE_POLICY, policy);
>>          }
>>
>> +       policy->max = policy->cpuinfo.max_freq;
>> +       policy->min = policy->cpuinfo.min_freq;
>> +
>>          if (cpufreq_driver->get && has_target()) {
>>                  policy->cur = cpufreq_driver->get(policy->cpu);
>>                  if (!policy->cur) {
> I am okay with a separate series doing all these cleanups. We can set some sort
> of rule on what is expected from the driver here and what is left from the core.
> For example driver only sets cpuinfo.max/min and core takes care of rest. This
> series would cleanup all the drivers, etc.
>
Ok,

Is the current state of this patch + the above modification acceptable ?

Re: [PATCH v6 3/4] cpufreq: Set policy->min and max as real QoS constraints

[Viresh Kumar]

On 25-03-26, 17:54, Pierre Gondois wrote:
> Is the current state of this patch + the above modification acceptable ?

I will review it closely again once you send it.. There were too many small
changes and I got confused earlier :)

-- 
viresh

[RFC PATCH v6 4/4] cpufreq/freq_table: Allow decreasing cpuinfo.max_freq

[Pierre Gondois]

Drivers not using freq. tables update cpuinfo.max_freq in their
.set_boost() callback. E.g. amd-pstate, cppc_cpufreq.
Drivers relying on freq. tables and supporting boost frequencies
rely on cpufreq_frequency_table_cpuinfo(). cpuinfo.max_freq is
only updated if the new maximal value is higher than the previous
one.

Using the scmi-cpufreq driver which relies on freq. tables, enabling
boost will permanently increases the cpuinfo.max_freq value.
This patch allows to lower cpuinfo.max_freq.

Note:
commit 538b0188da46 ("cpufreq: ACPI: Set cpuinfo.max_freq directly
if max boost is known")
favored having cpuinfo.max_freq reporting the maximal boosted
frequency of a CPU instead of the maximal reachable frequency
due to regressions in the frequency reported by cpuinfo.max
and scaling_cur_freq.
As stated above, this is not what most of the other cpufreq driver
do. I assume that the following patch:
commit 3c55e94c0ade ("cpufreq: ACPI: Extend frequency tables to
cover boost frequencies")
was correct, but might not have tagged the boosted frequency with
the CPUFREQ_BOOST_FREQ flag in the freq. table.

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

diff --git a/drivers/cpufreq/freq_table.c b/drivers/cpufreq/freq_table.c
index 9b37f37c36389..bd08cbe9e9ba3 100644
--- a/drivers/cpufreq/freq_table.c
+++ b/drivers/cpufreq/freq_table.c
@@ -50,12 +50,7 @@ int cpufreq_frequency_table_cpuinfo(struct cpufreq_policy *policy)
 	}

 	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->cpuinfo.max_freq = max_freq;
+	policy->cpuinfo.max_freq = max_freq;

 	if (min_freq == ~0)
 		return -EINVAL;
--
2.43.0

Re: [RFC PATCH v6 4/4] cpufreq/freq_table: Allow decreasing cpuinfo.max_freq

[Viresh Kumar]

On 17-03-26, 11:17, Pierre Gondois wrote:
> Drivers not using freq. tables update cpuinfo.max_freq in their
> .set_boost() callback. E.g. amd-pstate, cppc_cpufreq.
> Drivers relying on freq. tables and supporting boost frequencies
> rely on cpufreq_frequency_table_cpuinfo(). cpuinfo.max_freq is
> only updated if the new maximal value is higher than the previous
> one.
> 
> Using the scmi-cpufreq driver which relies on freq. tables, enabling
> boost will permanently increases the cpuinfo.max_freq value.
> This patch allows to lower cpuinfo.max_freq.
> 
> Note:
> commit 538b0188da46 ("cpufreq: ACPI: Set cpuinfo.max_freq directly
> if max boost is known")

Rafael needs to decide this part, since it may end up breaking acpi-cpufreq.

I am okay otherwise.

-- 
viresh