Re: [PATCH v1] cpufreq: intel_pstate: Relocate platform preference check

[Rafael J. Wysocki]

On Fri, Feb 7, 2025 at 1:48 PM Rafael J. Wysocki <rjw@xxxxxxxxxxxxx> wrote:
>
> From: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
>
> Move the invocation of intel_pstate_platform_pwr_mgmt_exists() before
> checking whether or not HWP is enabled because it does not depend on
> any code running before it except for the vendor check and if CPU
> performance scaling is going to be carried out by the platform, all of
> the code that runs before that function (again, except for the vendor
> check) is redundant.
>
> This is not expected to alter any functionality except for the ordering
> of messages printed by intel_pstate_init() when it is going to return an
> error before attempting to register the driver.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>

Srinivas, I'm wondering if you have any concerns regarding this one?

> ---
>  drivers/cpufreq/intel_pstate.c |   18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
>
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -3688,6 +3688,15 @@
>         if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL)
>                 return -ENODEV;
>
> +       /*
> +        * The Intel pstate driver will be ignored if the platform
> +        * firmware has its own power management modes.
> +        */
> +       if (intel_pstate_platform_pwr_mgmt_exists()) {
> +               pr_info("P-states controlled by the platform\n");
> +               return -ENODEV;
> +       }
> +
>         id = x86_match_cpu(hwp_support_ids);
>         if (id) {
>                 hwp_forced = intel_pstate_hwp_is_enabled();
> @@ -3743,15 +3752,6 @@
>                 default_driver = &intel_cpufreq;
>
>  hwp_cpu_matched:
> -       /*
> -        * The Intel pstate driver will be ignored if the platform
> -        * firmware has its own power management modes.
> -        */
> -       if (intel_pstate_platform_pwr_mgmt_exists()) {
> -               pr_info("P-states controlled by the platform\n");
> -               return -ENODEV;
> -       }
> -
>         if (!hwp_active && hwp_only)
>                 return -ENOTSUPP;
>
>
>
>
>