2023-06-28 23:11:44

by srinivas pandruvada

[permalink] [raw]
Subject: [PATCH] cpufreq: intel_pstate: Fix scaling for hybrid capable system with disabled E-cores

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 <[email protected]>
---
drivers/cpufreq/intel_pstate.c | 59 ++++++++++++++++++++++++++++------
1 file changed, 49 insertions(+), 10 deletions(-)

diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index 2548ec92faa2..b562ed7c4f37 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -330,6 +330,13 @@ static bool intel_pstate_get_ppc_enable_status(void)
return acpi_ppc;
}

+#define HYBRID_SCALING_FACTOR 78741
+
+static inline int core_get_scaling(void)
+{
+ return 100000;
+}
+
#ifdef CONFIG_ACPI_CPPC_LIB

/* The work item is needed to avoid CPU hotplug locking issues */
@@ -400,10 +407,35 @@ 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)
{
}
+
+static int intel_pstate_cppc_get_scaling(int cpu)
+{
+ return core_get_scaling();
+}
+
#endif /* CONFIG_ACPI_CPPC_LIB */

static void intel_pstate_init_acpi_perf_limits(struct cpufreq_policy *policy)
@@ -1895,11 +1927,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 +1963,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 +3433,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;
}
--
2.31.1



2023-06-29 02:09:15

by kernel test robot

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

Hi Srinivas,

kernel test robot noticed the following build errors:

[auto build test ERROR on rafael-pm/linux-next]
[also build test ERROR on linus/master v6.4 next-20230628]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Srinivas-Pandruvada/cpufreq-intel_pstate-Fix-scaling-for-hybrid-capable-system-with-disabled-E-cores/20230629-065506
base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
patch link: https://lore.kernel.org/r/20230628225341.3718351-1-srinivas.pandruvada%40linux.intel.com
patch subject: [PATCH] cpufreq: intel_pstate: Fix scaling for hybrid capable system with disabled E-cores
config: i386-buildonly-randconfig-r004-20230628 (https://download.01.org/0day-ci/archive/20230629/[email protected]/config)
compiler: clang version 15.0.7 (https://github.com/llvm/llvm-project.git 8dfdcc7b7bf66834a761bd8de445840ef68e4d1a)
reproduce: (https://download.01.org/0day-ci/archive/20230629/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>):

>> drivers/cpufreq/intel_pstate.c:1975:10: error: use of undeclared identifier 'HYBRID_SCALING_FACTOR'
return HYBRID_SCALING_FACTOR;
^
>> drivers/cpufreq/intel_pstate.c:1979:10: error: call to undeclared function 'core_get_scaling'; ISO C99 and later do not support implicit function declarations [-Werror,-Wimplicit-function-declaration]
return core_get_scaling();
^
drivers/cpufreq/intel_pstate.c:1979:10: note: did you mean 'core_get_val'?
drivers/cpufreq/intel_pstate.c:1932:12: note: 'core_get_val' declared here
static u64 core_get_val(struct cpudata *cpudata, int pstate)
^
>> drivers/cpufreq/intel_pstate.c:1990:9: error: call to undeclared function 'intel_pstate_cppc_get_scaling'; ISO C99 and later do not support implicit function declarations [-Werror,-Wimplicit-function-declaration]
return intel_pstate_cppc_get_scaling(cpu);
^
>> drivers/cpufreq/intel_pstate.c:2350:17: error: use of undeclared identifier 'core_get_scaling'; did you mean 'core_get_val'?
.get_scaling = core_get_scaling,
^~~~~~~~~~~~~~~~
core_get_val
drivers/cpufreq/intel_pstate.c:1932:12: note: 'core_get_val' declared here
static u64 core_get_val(struct cpudata *cpudata, int pstate)
^
>> drivers/cpufreq/intel_pstate.c:2350:17: error: incompatible function pointer types initializing 'int (*)(void)' with an expression of type 'u64 (struct cpudata *, int)' (aka 'unsigned long long (struct cpudata *, int)') [-Werror,-Wincompatible-function-pointer-types]
.get_scaling = core_get_scaling,
^~~~~~~~~~~~~~~~
drivers/cpufreq/intel_pstate.c:2380:17: error: use of undeclared identifier 'core_get_scaling'; did you mean 'core_get_val'?
.get_scaling = core_get_scaling,
^~~~~~~~~~~~~~~~
core_get_val
drivers/cpufreq/intel_pstate.c:1932:12: note: 'core_get_val' declared here
static u64 core_get_val(struct cpudata *cpudata, int pstate)
^
drivers/cpufreq/intel_pstate.c:2380:17: error: incompatible function pointer types initializing 'int (*)(void)' with an expression of type 'u64 (struct cpudata *, int)' (aka 'unsigned long long (struct cpudata *, int)') [-Werror,-Wincompatible-function-pointer-types]
.get_scaling = core_get_scaling,
^~~~~~~~~~~~~~~~
7 errors generated.


vim +/HYBRID_SCALING_FACTOR +1975 drivers/cpufreq/intel_pstate.c

1967
1968 static int hwp_get_cpu_scaling(int cpu)
1969 {
1970 u8 cpu_type = 0;
1971
1972 smp_call_function_single(cpu, hybrid_get_type, &cpu_type, 1);
1973 /* P-cores have a smaller perf level-to-freqency scaling factor. */
1974 if (cpu_type == 0x40)
> 1975 return HYBRID_SCALING_FACTOR;
1976
1977 /* Use default core scaling for E-cores */
1978 if (cpu_type == 0x20)
> 1979 return core_get_scaling();
1980
1981 /*
1982 * If reached here, it means that, this system is either non
1983 * hybrid system (like Tiger Lake) or hybrid capable system (like
1984 * Alder Lake or Raptor Lake) with no E cores (CPUID for hybrid
1985 * support is 0).
1986 * All non hybrid systems, don't publish nominal_frequency
1987 * field (means nominal frequency = 0), In that case
1988 * the legacy core scaling is used.
1989 */
> 1990 return intel_pstate_cppc_get_scaling(cpu);
1991 }
1992

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

2023-06-29 09:50:45

by Rafael J. Wysocki

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

On Thu, Jun 29, 2023 at 12:54 AM Srinivas Pandruvada
<[email protected]> 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 <[email protected]>
> ---
> drivers/cpufreq/intel_pstate.c | 59 ++++++++++++++++++++++++++++------
> 1 file changed, 49 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> index 2548ec92faa2..b562ed7c4f37 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -330,6 +330,13 @@ static bool intel_pstate_get_ppc_enable_status(void)
> return acpi_ppc;
> }
>
> +#define HYBRID_SCALING_FACTOR 78741
> +
> +static inline int core_get_scaling(void)
> +{
> + return 100000;
> +}

The above should be defined outside #ifdef CONFIG_ACPI.

> +
> #ifdef CONFIG_ACPI_CPPC_LIB
>
> /* The work item is needed to avoid CPU hotplug locking issues */
> @@ -400,10 +407,35 @@ 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)
> {
> }
> +
> +static int intel_pstate_cppc_get_scaling(int cpu)
> +{
> + return core_get_scaling();
> +}
> +
> #endif /* CONFIG_ACPI_CPPC_LIB */
>
> static void intel_pstate_init_acpi_perf_limits(struct cpufreq_policy *policy)
> @@ -1895,11 +1927,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 +1963,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 +3433,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;
> }
> --
> 2.31.1
>