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

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

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

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@intel.com>
---
 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;
 

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@rjwysocki.net> wrote:
>
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> 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@intel.com>

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

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

[srinivas pandruvada]

On Thu, 2025-02-20 at 20:30 +0100, Rafael J. Wysocki wrote:
> On Fri, Feb 7, 2025 at 1:48 PM Rafael J. Wysocki <rjw@rjwysocki.net>
> wrote:
> > 
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > 
> > 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@intel.com>
> 
> Srinivas, I'm wondering if you have any concerns regarding this one?
None.

Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>

Thanks,
Srinivas


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