Re: [PATCH 1/2] cpufreq: amd-pstate: Set asymmetric CPU capacity on hybrid platforms

[Mario Limonciello <mario.limonciello@amd.com>]

On 11/14/26 09:03, Vishal Badole wrote:
> AMD heterogeneous processors combine multiple core types (e.g.
> performance and efficiency cores) whose maximum sustained performance
> and frequency differ. On such platforms the Linux scheduler relies on
> per-CPU capacity values to place and balance tasks across cores.
> 
> Today amd-pstate does not export any per-CPU capacity information. As a
> result the scheduler sees every CPU as having identical capacity and
> treats performance and efficiency cores as interchangeable. Tasks are
> therefore placed and balanced without regard to core strength, which
> hurts both throughput and energy efficiency on hybrid systems.
> 
> This happens because the scheduler defaults to a uniform
> SCHED_CAPACITY_SCALE when no arch- or driver-supplied capacity exists,
> and amd-pstate never feeds the asymmetry it knows (via CPPC highest
> frequency or the effective highest performance register) into the arch
> hybrid capacity scaling interface.
> 
> Enable capacity aware scheduling on AMD heterogeneous-core processors.
> Use CPU 0 as the capacity reference, assuming it is a highest-performance
> CPU on the supported platforms. Cache its maximum frequency in kHz and
> effective highest performance once before driver registration, treating
> these reference capabilities as fixed for the lifetime of the system.
> During per-CPU initialization, compute each CPU's capacity relative to the
> cached reference and publish it:
> 
>   - When maximum-frequency data is available for both CPUs, scale by
> 	 frequency via arch_set_cpu_capacity().
>   - Otherwise, fall back to performance-register based scaling using the
>     effective highest_perf values.
> 
> Record frequency availability before applying the per-CPU fallback. Keep
> frequency and performance references separate instead of interpreting a
> boost-ratio numerator in performance units as MHz. The fallback base
> frequency still depends on each CPU's nominal frequency and performance,
> so calculate that conversion per CPU. Disable CAS if reference setup fails
> without preventing normal CPUFreq registration.
> 
> Capacity aware scheduling is enabled by default on supported hardware and
> can be turned off with the "amd_cas=disable" boot parameter. Its effective
> state is exposed through the "cas" sysfs attribute, and also exported via
> amd_pstate_cas_active() so amd-hfi can stand down when capacity aware
> scheduling owns the heterogeneous cores.
> 
> Signed-off-by: Vishal Badole <Vishal.Badole@amd.com>
> ---
>   MAINTAINERS                  |   1 +
>   drivers/cpufreq/amd-pstate.c | 102 ++++++++++++++++++++++++++++++++++-
>   include/linux/amd-pstate.h   |  14 +++++
>   3 files changed, 115 insertions(+), 2 deletions(-)
>   create mode 100644 include/linux/amd-pstate.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 222b7fc2c3aa..26217272be95 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1324,6 +1324,7 @@ L:	linux-pm@vger.kernel.org
>   S:	Supported
>   F:	Documentation/admin-guide/pm/amd-pstate.rst
>   F:	drivers/cpufreq/amd-pstate*
> +F:	include/linux/amd-pstate.h
>   F:	tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py
>   
>   AMD PTDMA DRIVER
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 4bedfbfc18b6..59eb2c4c7867 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -29,6 +29,7 @@
>   #include <linux/smp.h>
>   #include <linux/sched.h>
>   #include <linux/cpufreq.h>
> +#include <linux/sched/smt.h>
>   #include <linux/compiler.h>
>   #include <linux/dmi.h>
>   #include <linux/slab.h>
> @@ -39,6 +40,8 @@
>   #include <linux/power_supply.h>
>   #include <linux/static_call.h>
>   #include <linux/topology.h>
> +#include <linux/cpuset.h>
> +#include <linux/amd-pstate.h>
>   
>   #include <acpi/processor.h>
>   #include <acpi/cppc_acpi.h>
> @@ -86,6 +89,10 @@ static struct cpufreq_driver amd_pstate_driver;
>   static struct cpufreq_driver amd_pstate_epp_driver;
>   static int cppc_state = AMD_PSTATE_UNDEFINED;
>   static bool amd_pstate_prefcore = true;
> +static bool amd_pstate_cas = true;

As this strictly requires SMP support, what do you think about this 
instead to pick the default.

static bool amd_pstate_cas = IS_ENABLED(CONFIG_SMP);

> +static bool amd_pstate_cas_initialized;
> +static u32 amd_pstate_ref_freq_khz;
> +static u32 amd_pstate_ref_highest_perf;
>   static struct quirk_entry *quirks;
>   
>   /*
> @@ -957,8 +964,8 @@ static void amd_pstate_init_prefcore(struct amd_cpudata *cpudata)
>   		return;
>   
>   	/* should use amd-hfi instead */
> -	if (cpu_feature_enabled(X86_FEATURE_AMD_WORKLOAD_CLASS) &&
> -	    IS_ENABLED(CONFIG_AMD_HFI)) {
> +	if ((cpu_feature_enabled(X86_FEATURE_AMD_WORKLOAD_CLASS) &&
> +	     IS_ENABLED(CONFIG_AMD_HFI)) || amd_pstate_cas) {
>   		amd_pstate_prefcore = false;
>   		return;
>   	}
> @@ -969,6 +976,64 @@ static void amd_pstate_init_prefcore(struct amd_cpudata *cpudata)
>   	sched_set_itmt_core_prio((int)READ_ONCE(cpudata->prefcore_ranking), cpudata->cpu);
>   }
>   
> +bool amd_pstate_cas_active(void)
> +{
> +	return READ_ONCE(amd_pstate_cas_initialized);
> +}
> +EXPORT_SYMBOL_GPL(amd_pstate_cas_active);
> +
> +static bool __init amd_pstate_init_cas(void)
> +{
> +	int highest_perf, max_freq;
> +
> +	highest_perf = amd_get_effective_highest_perf(0);
> +	if (highest_perf <= 0) {
> +		pr_warn("Invalid reference highest performance (%d)\n", highest_perf);
> +		return false;
> +	}
> +
> +	max_freq = amd_get_max_frequency(0);
> +	if (max_freq < 0 || (u32)max_freq > U32_MAX / 1000) {
> +		pr_warn("Invalid reference maximum frequency (%d)\n", max_freq);
> +		return false;
> +	}
> +
> +	amd_pstate_ref_highest_perf = highest_perf;
> +	amd_pstate_ref_freq_khz = max_freq * 1000U;
> +
> +	return arch_enable_hybrid_capacity_scale();
> +}
> +
> +static void amd_pstate_set_cpu_capacity(struct amd_cpudata *cpudata,
> +					bool highest_freq_available)
> +{
> +	unsigned long capacity, max_capacity, capacity_freq, base_freq;
> +
> +	if (!IS_ENABLED(CONFIG_SMP))
> +		return;

 From what I can tell; I don't think this is realistically something 
that can get called since you already check to enable CAS based on this 
same check (IS_ENABLED(CONFIG_SMP)).

So please drop this check here.  More later about IS_ENABLED(CONFIG_SMP).

> +
> +	if (highest_freq_available && amd_pstate_ref_freq_khz) {
> +		capacity = cpudata->max_freq;
> +		max_capacity = amd_pstate_ref_freq_khz;
> +		capacity_freq = capacity;
> +		base_freq = max_capacity;
> +	} else {
> +		union perf_cached perf = READ_ONCE(cpudata->perf);
> +
> +		capacity = perf.highest_perf;
> +		max_capacity = amd_pstate_ref_highest_perf;
> +		capacity_freq = perf_to_freq(perf, cpudata->nominal_freq, perf.highest_perf);
> +		base_freq = perf_to_freq(perf, cpudata->nominal_freq, max_capacity);
> +	}
> +
> +	guard(mutex)(&sched_domains_mutex);
> +	arch_set_cpu_capacity(cpudata->cpu, capacity, max_capacity,
> +			      capacity_freq, base_freq);
> +	topology_set_cpu_scale(cpudata->cpu, arch_scale_cpu_capacity(cpudata->cpu));
> +	x86_topology_update = true;
> +	WRITE_ONCE(amd_pstate_cas_initialized, true);
> +}
> +
>   static void amd_pstate_update_limits(struct cpufreq_policy *policy)
>   {
>   	struct amd_cpudata *cpudata;
> @@ -1043,6 +1108,7 @@ static int amd_pstate_init_freq(struct amd_cpudata *cpudata)
>   	u32 min_freq, max_freq, nominal_freq, lowest_nonlinear_freq;
>   	struct cppc_perf_caps cppc_perf;
>   	union perf_cached perf;
> +	bool highest_freq_available;
>   	int ret;
>   
>   	ret = cppc_get_perf_caps(cpudata->cpu, &cppc_perf);
> @@ -1069,6 +1135,7 @@ static int amd_pstate_init_freq(struct amd_cpudata *cpudata)
>   
>   	/* try to look up from BIOS/quirk first, fall back to (nominal_freq * highest_perf)/nominal_perf */
>   	max_freq = amd_get_max_frequency(cpudata->cpu) * 1000;
> +	highest_freq_available = !!max_freq;
>   	if (!max_freq)
>   		max_freq = perf_to_freq(perf, nominal_freq, perf.highest_perf);
>   	WRITE_ONCE(cpudata->max_freq, max_freq);
> @@ -1100,6 +1167,9 @@ static int amd_pstate_init_freq(struct amd_cpudata *cpudata)
>   			 bios_min_freq, cpudata->cpu);
>   	}
>   
> +	if (amd_pstate_cas)
> +		amd_pstate_set_cpu_capacity(cpudata, highest_freq_available);
> +
>   	return 0;
>   }
>   
> @@ -1923,12 +1993,20 @@ static ssize_t prefcore_show(struct device *dev,
>   	return sysfs_emit(buf, "%s\n", str_enabled_disabled(amd_pstate_prefcore));
>   }
>   
> +static ssize_t cas_show(struct device *dev,
> +			struct device_attribute *attr, char *buf)
> +{
> +	return sysfs_emit(buf, "%s\n", str_enabled_disabled(amd_pstate_cas_active()));
> +}
> +
>   static DEVICE_ATTR_RW(status);
>   static DEVICE_ATTR_RO(prefcore);
> +static DEVICE_ATTR_RO(cas);
>   
>   static struct attribute *pstate_global_attributes[] = {
>   	&dev_attr_status.attr,
>   	&dev_attr_prefcore.attr,
> +	&dev_attr_cas.attr,
>   	NULL
>   };
>   
> @@ -2438,12 +2516,23 @@ static int __init amd_pstate_init(void)
>   			return ret;
>   	}
>   
> +	amd_pstate_cas = amd_pstate_cas && IS_ENABLED(CONFIG_SMP) &&
> +			cpu_feature_enabled(X86_FEATURE_AMD_HTR_CORES);
> +
> +	/* Enable CAS for processors with heterogeneous cores. */
> +	if (amd_pstate_cas)
> +		amd_pstate_cas = amd_pstate_init_cas();
> +

This seem to be a pointless variable initialization, especially as it's 
a global variable and this runs on non-hetero systems too.  Maybe 
something like this:

0) See my previous comment about where to use IS_ENABLED(CONFIG_SMP).

1) Change amd_pstate_init_cas return type to void.

 > +static void __init amd_pstate_init_cas(void)

2) Add checks into the function like this:

static void __init amd_pstate_init_cas(void) {
	if (!amd_pstate_cas)
		return;
	if (!cpu_feature_enabled(X86_FEATURE_AMD_HTR_CORES))
		return;
	.
	.
	.
	amd_pstate_cas = arch_enable_hybrid_capacity_scale();
	
}

3) Change the call in this function to just:
amd_pstate_init_cas();

That should help avoid some extra variable writes.

>   	ret = amd_pstate_register_driver(cppc_state);
>   	if (ret) {
>   		pr_err("failed to register with return %d\n", ret);
>   		return ret;
>   	}
>   
> +	/* Per-CPU capacities are set during registration; rebuild domains once. */
> +	if (amd_pstate_cas)
> +		rebuild_sched_domains();
> +
>   	dev_root = bus_get_dev_root(&cpu_subsys);
>   	if (dev_root) {
>   		ret = sysfs_create_group(&dev_root->kobj, &amd_pstate_global_attr_group);
> @@ -2484,8 +2573,17 @@ static int __init amd_prefcore_param(char *str)
>   	return 0;
>   }
>   
> +static int __init amd_cas_param(char *str)
> +{
> +	if (!strcmp(str, "disable"))
> +		amd_pstate_cas = false;
> +
> +	return 0;
> +}
> +
>   early_param("amd_pstate", amd_pstate_param);
>   early_param("amd_prefcore", amd_prefcore_param);
> +early_param("amd_cas", amd_cas_param);
>   
>   MODULE_AUTHOR("Huang Rui <ray.huang@amd.com>");
>   MODULE_DESCRIPTION("AMD Processor P-state Frequency Driver");
> diff --git a/include/linux/amd-pstate.h b/include/linux/amd-pstate.h
> new file mode 100644
> index 000000000000..eb750abd20f7
> --- /dev/null
> +++ b/include/linux/amd-pstate.h
> @@ -0,0 +1,14 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Public interface for the AMD P-state driver.
> + */
> +#ifndef __LINUX_AMD_PSTATE_H__
> +#define __LINUX_AMD_PSTATE_H__
> +
> +#if IS_ENABLED(CONFIG_X86_AMD_PSTATE)
> +bool amd_pstate_cas_active(void);
> +#else
> +static inline bool amd_pstate_cas_active(void) { return false; }
> +#endif
> +
> +#endif /* __LINUX_AMD_PSTATE_H__ */