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

[Rafael J. Wysocki]

On Wed, Feb 25, 2026 at 9:50 AM Pierre Gondois <pierre.gondois@xxxxxxx> 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@xxxxxxx>
> ---
>  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.

> 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?

> @@ -2818,16 +2822,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);

This also looks like it belongs to the second patch in the series.

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