| Message ID | 20241023174357.34338-6-mario.limonciello@amd.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere, archived |
| Headers | show |
| Series | x86 Heterogeneous design identification | expand |
On Wed, Oct 23, 2024 at 12:43:57PM -0500, Mario Limonciello wrote: > int amd_get_boost_ratio_numerator(unsigned int cpu, u64 *numerator) > { > + enum x86_topology_cpu_type core_type = get_topology_generic_cpu_type(&cpu_data(cpu)); > bool prefcore; > int ret; > + u32 tmp; > > ret = amd_detect_prefcore(&prefcore); > if (ret) > @@ -261,6 +263,27 @@ int amd_get_boost_ratio_numerator(unsigned int cpu, u64 *numerator) > break; > } > } > + What's the difference between this case: > + /* detect if running on heterogeneous design */ > + switch (core_type) { > + case TOPO_CPU_TYPE_UNKNOWN: ^^^^^^^^^^^^^^^^^^^^^^^ > + break; > + case TOPO_CPU_TYPE_PERFORMANCE: > + /* use the max scale for performance cores */ > + *numerator = CPPC_HIGHEST_PERF_PERFORMANCE; > + return 0; > + case TOPO_CPU_TYPE_EFFICIENCY: > + /* use the highest perf value for efficiency cores */ > + ret = amd_get_highest_perf(cpu, &tmp); > + if (ret) > + return ret; > + *numerator = tmp; > + return 0; > + default: ... and this case and why aren't you warning if TOPO_CPU_TYPE_UNKNOWN? I think for that you need to check X86_FEATURE_AMD_HETEROGENEOUS_CORES and warn if set but still CPU type unknown or? > + pr_warn("WARNING: Undefined core type %d found\n", core_type); > + break; > + }
On 10/25/2024 08:51, Borislav Petkov wrote: > On Wed, Oct 23, 2024 at 12:43:57PM -0500, Mario Limonciello wrote: >> int amd_get_boost_ratio_numerator(unsigned int cpu, u64 *numerator) >> { >> + enum x86_topology_cpu_type core_type = get_topology_generic_cpu_type(&cpu_data(cpu)); >> bool prefcore; >> int ret; >> + u32 tmp; >> >> ret = amd_detect_prefcore(&prefcore); >> if (ret) >> @@ -261,6 +263,27 @@ int amd_get_boost_ratio_numerator(unsigned int cpu, u64 *numerator) >> break; >> } >> } >> + > > What's the difference between this case: > >> + /* detect if running on heterogeneous design */ >> + switch (core_type) { >> + case TOPO_CPU_TYPE_UNKNOWN: > ^^^^^^^^^^^^^^^^^^^^^^^ > >> + break; >> + case TOPO_CPU_TYPE_PERFORMANCE: >> + /* use the max scale for performance cores */ >> + *numerator = CPPC_HIGHEST_PERF_PERFORMANCE; >> + return 0; >> + case TOPO_CPU_TYPE_EFFICIENCY: >> + /* use the highest perf value for efficiency cores */ >> + ret = amd_get_highest_perf(cpu, &tmp); >> + if (ret) >> + return ret; >> + *numerator = tmp; >> + return 0; >> + default: > > ... and this case and why aren't you warning if TOPO_CPU_TYPE_UNKNOWN? > > I think for that you need to check X86_FEATURE_AMD_HETEROGENEOUS_CORES and > warn if set but still CPU type unknown or? Yeah; you're right. An earlier version of this behaved differently and I missed updating this switch/case when using Pawan's updated patch. After we get Intel feedback on the previous patch I'll drop the 'default' case in the next version and switch it to this: case TOPO_CPU_TYPE_UNKNOWN: if (cpu_feature_enabled(X86_FEATURE_AMD_HETEROGENEOUS_CORES)) pr_warn("Undefined core type found for cpu %d\n", cpu); break; > >> + pr_warn("WARNING: Undefined core type %d found\n", core_type); >> + break; >> + } >
diff --git a/arch/x86/kernel/acpi/cppc.c b/arch/x86/kernel/acpi/cppc.c index 956984054bf30..9983d4537968f 100644 --- a/arch/x86/kernel/acpi/cppc.c +++ b/arch/x86/kernel/acpi/cppc.c @@ -234,8 +234,10 @@ EXPORT_SYMBOL_GPL(amd_detect_prefcore); */ int amd_get_boost_ratio_numerator(unsigned int cpu, u64 *numerator) { + enum x86_topology_cpu_type core_type = get_topology_generic_cpu_type(&cpu_data(cpu)); bool prefcore; int ret; + u32 tmp; ret = amd_detect_prefcore(&prefcore); if (ret) @@ -261,6 +263,27 @@ int amd_get_boost_ratio_numerator(unsigned int cpu, u64 *numerator) break; } } + + /* detect if running on heterogeneous design */ + switch (core_type) { + case TOPO_CPU_TYPE_UNKNOWN: + break; + case TOPO_CPU_TYPE_PERFORMANCE: + /* use the max scale for performance cores */ + *numerator = CPPC_HIGHEST_PERF_PERFORMANCE; + return 0; + case TOPO_CPU_TYPE_EFFICIENCY: + /* use the highest perf value for efficiency cores */ + ret = amd_get_highest_perf(cpu, &tmp); + if (ret) + return ret; + *numerator = tmp; + return 0; + default: + pr_warn("WARNING: Undefined core type %d found\n", core_type); + break; + } + *numerator = CPPC_HIGHEST_PERF_PREFCORE; return 0;
AMD heterogeneous designs include two types of cores: * Performance * Efficiency Each core type has different highest performance values configured by the platform. Drivers such as `amd_pstate` need to identify the type of core to correctly set an appropriate boost numerator to calculate the maximum frequency. X86_FEATURE_AMD_HETEROGENEOUS_CORES is used to identify whether the SoC supports heterogeneous core type by reading CPUID leaf Fn_0x80000026. On performance cores the scaling factor of 196 is used. On efficiency cores the scaling factor is the value reported as the highest perf. Efficiency cores have the same preferred core rankings. Suggested-by: Perry Yuan <perry.yuan@amd.com> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> --- arch/x86/kernel/acpi/cppc.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+)