[PATCH v3 5/5] x86/amd: Use heterogeneous core topology for identifying boost numerator

Mario Limonciello posted 5 patches 1 month ago
There is a newer version of this series
[PATCH v3 5/5] x86/amd: Use heterogeneous core topology for identifying boost numerator
Posted by Mario Limonciello 1 month ago
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(+)

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;
-- 
2.43.0
Re: [PATCH v3 5/5] x86/amd: Use heterogeneous core topology for identifying boost numerator
Posted by Borislav Petkov 1 month ago
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;
> +	}

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v3 5/5] x86/amd: Use heterogeneous core topology for identifying boost numerator
Posted by Mario Limonciello 1 month ago
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;
>> +	}
>