Re: [PATCH v5 4/6] cpufreq: Update .set_boost() callbacks to rely on boost_freq_req

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

On 3/13/26 12:43, Rafael J. Wysocki wrote:
> On Fri, Mar 13, 2026 at 10:38 AM Viresh Kumar<viresh.kumar@linaro.org> wrote:
>> On 11-03-26, 17:19, Rafael J. Wysocki wrote:
>>> On Wed, Feb 25, 2026 at 9:50 AM Pierre Gondois<pierre.gondois@arm.com> wrote:
>>>> In the existing .set_boost() callbacks:
>>>> - Don't update policy->max as this is done through the qos notifier
>>>>    cpufreq_notifier_max() which calls cpufreq_set_policy().
>>>> - Remove freq_qos_update_request() calls as the qos request is now
>>>>    done in policy_set_boost() and updates the new boost_freq_req
>>>>
>>>> Note:
>>>> cpufreq_frequency_table_cpuinfo() is also called through:
>>>> cpufreq_policy_online()
>>>> \-cpufreq_table_validate_and_sort()
>>>>    \-cpufreq_frequency_table_cpuinfo()
>>>> which relies on cpufreq_frequency_table_cpuinfo() to set
>>>> policy->min and max initizalization at driver init. This
>>>> regression is solved in the next patch.
>>>>
>>>> Note2:
>>>> acpi-cpufreq.c seems to be the only cpufreq driver not
>>>> setting cpuinfo.max_freq. Populate it the nominal frequency at
>>>> driver init.
>>>>
>>>> Signed-off-by: Pierre Gondois<pierre.gondois@arm.com>
>>>> ---
>>>>   drivers/cpufreq/acpi-cpufreq.c |  1 +
>>>>   drivers/cpufreq/amd-pstate.c   |  2 --
>>>>   drivers/cpufreq/cppc_cpufreq.c | 10 ++--------
>>>>   drivers/cpufreq/cpufreq.c      | 16 +++++++---------
>>>>   drivers/cpufreq/freq_table.c   |  7 +++----
>>>>   5 files changed, 13 insertions(+), 23 deletions(-)
>>>>
>>>> diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
>>>> index e73a66785d69d..6a6e26e1be14a 100644
>>>> --- a/drivers/cpufreq/acpi-cpufreq.c
>>>> +++ b/drivers/cpufreq/acpi-cpufreq.c
>>>> @@ -857,6 +857,7 @@ static int acpi_cpufreq_cpu_init(struct cpufreq_policy *policy)
>>>>                   * governor from selecting inadequate CPU frequencies.
>>>>                   */
>>>>                  arch_set_max_freq_ratio(true);
>>>> +               policy->cpuinfo.max_freq = nominal_freq;
>>>>          }
>>>>
>>>>          policy->freq_table = freq_table;
>>> This looks like a fix for acpi_cpufreq that should be sent separately.
>> cpufreq_frequency_table_cpuinfo() (called before this) should always end up
>> setting cpuinfo.max_freq properly and so this never broke earlier.
> So is this change needed?

Yes right indeed, this should be removed.


>>>> 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 42de32488f422..20266fb42d18d 100644
>>>> --- a/drivers/cpufreq/cpufreq.c
>>>> +++ b/drivers/cpufreq/cpufreq.c
>>>> @@ -1500,10 +1500,14 @@ static int cpufreq_policy_online(struct cpufreq_policy *policy,
>>>>                                  goto out_destroy_policy;
>>>>                          }
>>>>
>>>> +                       /*
>>>> +                        * 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,
>>>> -                                                  FREQ_QOS_MAX_DEFAULT_VALUE);
>>>> +                                                  policy->cpuinfo.max_freq);
>>>>                          if (ret < 0) {
>>>>                                  /*
>>>>                                   * So we don't call freq_qos_remove_request() for an
>>> Why do you need to update stuff introduced in the previous patch?
>>>
>>> Is that because policy->cpuinfo.max_freq is not set consistently
>>> before the changes made here?  In which case, wouldn't it be better to
>>> make all drivers set cpuinfo.max_freq consistently before making the
>>> changes in the second patch and fold the chunk above into the latter?
>> Since cpuinfo.max_freq is set correctly earlier, I think this can be done
>> earlier only.
> I'm not sure what you mean.
>
> To clarify my point, the question really is what prevents patch [2/6]
> from passing policy->cpuinfo.max_freq to freq_qos_add_request() as the
> last argument right away.

Nothing should prevent from doing that I think.

The first patchset was only intending to add a boost QoS hook,
then additional modifications have been added at the end of the
serie. I will modify the order to make it easier to follow.