[PATCH] cpufreq: intel_pstate: Fix global turbo disable check

[PATCH] cpufreq: intel_pstate: Fix global turbo disable check

[Srinivas Pandruvada]

Setting global turbo flag based on CPU 0 P-state limits is problematic
as it limits max P-state request on every CPU on the system just based
on its P-state limits.

There are two cases in which global.turbo_disabled flag is set:
- When the MSR_IA32_MISC_ENABLE_TURBO_DISABLE bit is set to 1
in the MSR MSR_IA32_MISC_ENABLE. This bit can be only changed by
the system BIOS before power up.
- When the max non turbo P-state is same as max turbo P-state for CPU 0.

The second check is not a valid to decide global turbo state based on
the CPU 0. CPU 0 max turbo P-state can be same as max non turbo P-state,
but for other CPUs this may not be true.

There is no guarantee that max P-state limits are same for every CPU. This
is possible that during fusing max P-state for a CPU is constrained. Also
with the Intel Speed Select performance profile, CPU 0 may not be present
in all profiles. In this case the max non turbo and turbo P-state can be
set to the lowest possible P-state by the hardware when switched to
such profile. Since max non turbo and turbo P-state is same,
global.turbo_disabled flag will be set.

Once global.turbo_disabled is set, any scaling max and min frequency
update for any CPU will result in its max P-state constrained to the max
non turbo P-state.

Hence remove the check of max non turbo P-state equal to max turbo P-state
of CPU 0 to set global turbo disabled flag.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 drivers/cpufreq/intel_pstate.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index dc50c9fb488d..a534a1f7f1ee 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -571,13 +571,9 @@ static void intel_pstate_hybrid_hwp_adjust(struct cpudata *cpu)
 static inline void update_turbo_state(void)
 {
 	u64 misc_en;
-	struct cpudata *cpu;
 
-	cpu = all_cpu_data[0];
 	rdmsrl(MSR_IA32_MISC_ENABLE, misc_en);
-	global.turbo_disabled =
-		(misc_en & MSR_IA32_MISC_ENABLE_TURBO_DISABLE ||
-		 cpu->pstate.max_pstate == cpu->pstate.turbo_pstate);
+	global.turbo_disabled = misc_en & MSR_IA32_MISC_ENABLE_TURBO_DISABLE;
 }
 
 static int min_perf_pct_min(void)
-- 
2.34.1

Re: [PATCH] cpufreq: intel_pstate: Fix global turbo disable check

[Rafael J. Wysocki]

On Thu, Sep 7, 2023 at 8:02 PM Srinivas Pandruvada
<srinivas.pandruvada@linux.intel.com> wrote:
>
> Setting global turbo flag based on CPU 0 P-state limits is problematic
> as it limits max P-state request on every CPU on the system just based
> on its P-state limits.
>
> There are two cases in which global.turbo_disabled flag is set:
> - When the MSR_IA32_MISC_ENABLE_TURBO_DISABLE bit is set to 1
> in the MSR MSR_IA32_MISC_ENABLE. This bit can be only changed by
> the system BIOS before power up.
> - When the max non turbo P-state is same as max turbo P-state for CPU 0.
>
> The second check is not a valid to decide global turbo state based on
> the CPU 0. CPU 0 max turbo P-state can be same as max non turbo P-state,
> but for other CPUs this may not be true.
>
> There is no guarantee that max P-state limits are same for every CPU. This
> is possible that during fusing max P-state for a CPU is constrained. Also
> with the Intel Speed Select performance profile, CPU 0 may not be present
> in all profiles. In this case the max non turbo and turbo P-state can be
> set to the lowest possible P-state by the hardware when switched to
> such profile. Since max non turbo and turbo P-state is same,
> global.turbo_disabled flag will be set.
>
> Once global.turbo_disabled is set, any scaling max and min frequency
> update for any CPU will result in its max P-state constrained to the max
> non turbo P-state.
>
> Hence remove the check of max non turbo P-state equal to max turbo P-state
> of CPU 0 to set global turbo disabled flag.
>
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> ---
>  drivers/cpufreq/intel_pstate.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> index dc50c9fb488d..a534a1f7f1ee 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -571,13 +571,9 @@ static void intel_pstate_hybrid_hwp_adjust(struct cpudata *cpu)
>  static inline void update_turbo_state(void)
>  {
>         u64 misc_en;
> -       struct cpudata *cpu;
>
> -       cpu = all_cpu_data[0];
>         rdmsrl(MSR_IA32_MISC_ENABLE, misc_en);
> -       global.turbo_disabled =
> -               (misc_en & MSR_IA32_MISC_ENABLE_TURBO_DISABLE ||
> -                cpu->pstate.max_pstate == cpu->pstate.turbo_pstate);
> +       global.turbo_disabled = misc_en & MSR_IA32_MISC_ENABLE_TURBO_DISABLE;
>  }
>
>  static int min_perf_pct_min(void)
> --

Applied as 6.7 material (under a slightly modified subject), thanks!