Re: [PATCH v2] cpufreq: intel_pstate: Fix scaling for hybrid capable system with disabled E-cores

From: Rafael J. Wysocki
Date: Fri Jun 30 2023 - 04:59:49 EST


On Thu, Jun 29, 2023 at 9:45 PM Srinivas Pandruvada
<srinivas.pandruvada@xxxxxxxxxxxxxxx> wrote:
>
> Some system BIOS configuration may provide option to disable E-cores.
> As part of this change, CPUID feature for hybrid (Leaf 7 sub leaf 0,
> EDX[15] = 0) may not be set. But HWP performance limits will still be
> using a scaling factor like any other hybrid enabled system.
>
> The current check for applying scaling factor will fail when hybrid
> CPUID feature is not set. Only way to make sure that scaling should be
> applied by checking CPPC nominal frequency and nominal performance. If
> CPPC nominal frequency and nominal performance is defined and nominal
> frequency is not in multiples of 100MHz of nominal performance, then use
> hybrid scaling factor.
>
> The above check will fail for non hybrid capable systems as they don't
> publish nominal frequency field in CPPC, so this function can be used
> for all HWP systems without additional cpu model check.
>
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@xxxxxxxxxxxxxxx>
> ---
> v2:
> Compile errors reported by kernel test robot and Rafael for the case
> when CONFIG_ACPI is not defined
>
> drivers/cpufreq/intel_pstate.c | 58 ++++++++++++++++++++++++++++------
> 1 file changed, 48 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> index 2548ec92faa2..7e18999be46a 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -302,6 +302,13 @@ static bool hwp_forced __read_mostly;
>
> static struct cpufreq_driver *intel_pstate_driver __read_mostly;
>
> +#define HYBRID_SCALING_FACTOR 78741
> +
> +static inline int core_get_scaling(void)
> +{
> + return 100000;
> +}
> +
> #ifdef CONFIG_ACPI
> static bool acpi_ppc;
> #endif
> @@ -400,6 +407,25 @@ static int intel_pstate_get_cppc_guaranteed(int cpu)
>
> return cppc_perf.nominal_perf;
> }
> +
> +static int intel_pstate_cppc_get_scaling(int cpu)
> +{
> + struct cppc_perf_caps cppc_perf;
> + int ret;
> +
> + ret = cppc_get_perf_caps(cpu, &cppc_perf);
> +
> + /*
> + * Check if nominal frequency is multiples of 100 MHz, if
> + * not return hybrid scaling factor.
> + */
> + if (!ret && cppc_perf.nominal_perf && cppc_perf.nominal_freq &&
> + (cppc_perf.nominal_perf * 100 != cppc_perf.nominal_freq))
> + return HYBRID_SCALING_FACTOR;
> +
> + return core_get_scaling();
> +}
> +
> #else /* CONFIG_ACPI_CPPC_LIB */
> static inline void intel_pstate_set_itmt_prio(int cpu)
> {
> @@ -492,6 +518,11 @@ static inline int intel_pstate_get_cppc_guaranteed(int cpu)
> {
> return -ENOTSUPP;
> }
> +
> +static int intel_pstate_cppc_get_scaling(int cpu)
> +{
> + return core_get_scaling();
> +}
> #endif /* CONFIG_ACPI_CPPC_LIB */
>
> /**
> @@ -1895,11 +1926,6 @@ static int core_get_turbo_pstate(int cpu)
> return ret;
> }
>
> -static inline int core_get_scaling(void)
> -{
> - return 100000;
> -}
> -
> static u64 core_get_val(struct cpudata *cpudata, int pstate)
> {
> u64 val;
> @@ -1936,16 +1962,29 @@ static void hybrid_get_type(void *data)
> *cpu_type = get_this_hybrid_cpu_type();
> }
>
> -static int hybrid_get_cpu_scaling(int cpu)
> +static int hwp_get_cpu_scaling(int cpu)
> {
> u8 cpu_type = 0;
>
> smp_call_function_single(cpu, hybrid_get_type, &cpu_type, 1);
> /* P-cores have a smaller perf level-to-freqency scaling factor. */
> if (cpu_type == 0x40)
> - return 78741;
> + return HYBRID_SCALING_FACTOR;
>
> - return core_get_scaling();
> + /* Use default core scaling for E-cores */
> + if (cpu_type == 0x20)
> + return core_get_scaling();
> +
> + /*
> + * If reached here, it means that, this system is either non
> + * hybrid system (like Tiger Lake) or hybrid capable system (like
> + * Alder Lake or Raptor Lake) with no E cores (CPUID for hybrid
> + * support is 0).
> + * All non hybrid systems, don't publish nominal_frequency
> + * field (means nominal frequency = 0), In that case
> + * the legacy core scaling is used.
> + */
> + return intel_pstate_cppc_get_scaling(cpu);
> }
>
> static void intel_pstate_set_pstate(struct cpudata *cpu, int pstate)
> @@ -3393,8 +3432,7 @@ static int __init intel_pstate_init(void)
> if (!default_driver)
> default_driver = &intel_pstate;
>
> - if (boot_cpu_has(X86_FEATURE_HYBRID_CPU))
> - pstate_funcs.get_cpu_scaling = hybrid_get_cpu_scaling;
> + pstate_funcs.get_cpu_scaling = hwp_get_cpu_scaling;
>
> goto hwp_cpu_matched;
> }
> --

Applied as 6.5-rc material with some adjustments. Please check the
bleeding-edge branch.

Thanks!