diff mbox series

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

Message ID 20241023174357.34338-6-mario.limonciello@amd.com
State New
Headers show
Series x86 Heterogeneous design identification | expand

Commit Message

Mario Limonciello Oct. 23, 2024, 5:43 p.m. UTC
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(+)

Comments

Borislav Petkov Oct. 25, 2024, 1:51 p.m. UTC | #1
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;
> +	}
Mario Limonciello Oct. 25, 2024, 1:57 p.m. UTC | #2
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 mbox series

Patch

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;