| Message ID | 2776745.mvXUDI8C0e@rjwysocki.net |
|---|---|
| State | New |
| Headers | show |
| Series | [v1] cpufreq: intel_pstate: Relocate platform preference check | expand |
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; > > > > >
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; > > > > > > > > > >
--- 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;