mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: K Prateek Nayak <kprateek.nayak@amd.com>
To: "Aiden Hecker" <heckeraiden@gmail.com>,
	"Huang Rui" <ray.huang@amd.com>,
	"Mario Limonciello" <mario.limonciello@amd.com>,
	"Perry Yuan" <perry.yuan@amd.com>,
	"Rafael J . Wysocki" <rafael@kernel.org>,
	"Viresh Kumar" <viresh.kumar@linaro.org>,
	"Hans de Goede" <hansg@kernel.org>,
	"Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>,
	"Vishal Badole" <Vishal.Badole@amd.com>
Cc: Ingo Molnar <mingo@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>, <linux-pm@vger.kernel.org>,
	<platform-driver-x86@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2 1/2] cpufreq: amd-pstate: Enable hybrid CPU capacity scaling on heterogeneous Zen CPUs
Date: Fri, 13 Nov 2026 12:45:40 +0530	[thread overview]
Message-ID: <bd326953-f693-4994-91e4-f8301430557a@amd.com> (raw)
In-Reply-To: <20261113022934.13397-2-heckeraiden@gmail.com>

(+ Vishal who too was working on this)

On 11/13/2026 7:59 AM, Aiden Hecker wrote:
> +struct amd_hybrid_cpu_cap {
> +	u32 highest_perf;
> +	u32 nominal_perf;
> +};
> +
> +static DEFINE_PER_CPU(struct amd_hybrid_cpu_cap, amd_hybrid_caps);
> +static DEFINE_MUTEX(amd_hybrid_cap_lock);
> +static u32 amd_hybrid_max_perf;
> +static bool amd_hybrid_cap_enabled;
> +
> +static void amd_hybrid_cap_workfn(struct work_struct *work)
> +{
> +	rebuild_sched_domains();
> +}
> +
> +static DECLARE_WORK(amd_hybrid_cap_work, amd_hybrid_cap_workfn);

This is probably not necessary - more on that later ...

> +
>  /*
>   * The numeric EPP value programmed for each named preference. First dimension
>   * is CPU type (TOPO_CPU_TYPE_ANY for non-hybrid, TOPO_CPU_TYPE_PERFORMANCE/
> @@ -969,6 +990,99 @@ static void amd_pstate_init_prefcore(struct amd_cpudata *cpudata)
>  	sched_set_itmt_core_prio((int)READ_ONCE(cpudata->prefcore_ranking), cpudata->cpu);
>  }
>  
> +static void amd_pstate_record_hybrid_cap(struct amd_cpudata *cpudata)
> +{
> +	struct amd_hybrid_cpu_cap *cap;
> +	union perf_cached perf;
> +	u32 highest_perf;
> +
> +	if (!cpu_feature_enabled(X86_FEATURE_AMD_HTR_CORES))
> +		return;
> +
> +	perf = READ_ONCE(cpudata->perf);
> +	highest_perf = READ_ONCE(cpudata->prefcore_ranking);
> +	if (!highest_perf)
> +		highest_perf = perf.highest_perf;

I think it is a terrible idea to mix the preferred core rankings
with asym capacities. You should just stick to using the
highest freq as a reference fot capacity.

> +
> +	guard(mutex)(&amd_hybrid_cap_lock);
> +	cap = per_cpu_ptr(&amd_hybrid_caps, cpudata->cpu);
> +	cap->highest_perf = highest_perf;
> +	cap->nominal_perf = perf.nominal_perf;
> +}
> +
> +static void amd_pstate_sync_hybrid_capacity(void)
> +{
> +	u32 max_perf = 0, min_perf = U32_MAX;
> +	bool updated = false;
> +	int cpu;
> +
> +	if (!cpu_feature_enabled(X86_FEATURE_AMD_HTR_CORES))
> +		return;
> +
> +	guard(mutex)(&amd_hybrid_cap_lock);
> +
> +	for_each_online_cpu(cpu) {
> +		const struct cpumask *sib_mask = topology_sibling_cpumask(cpu);
> +		u32 sib_max = 0;
> +		int sib;
> +
> +		/*
> +		 * SMT siblings share SD_SHARE_CPUCAPACITY and must expose identical
> +		 * arch_scale_cpu_capacity() values to avoid scheduler warnings.
> +		 */
> +		for_each_cpu(sib, sib_mask) {
> +			u32 p = per_cpu(amd_hybrid_caps, sib).highest_perf;
> +
> +			if (p > sib_max)
> +				sib_max = p;

This should never happen - even with preferred "core", both
siblings will have same ranking (but you should use the
amd_get_effective_highest_perf() - more on that below).

> +		}
> +		if (!sib_max)
> +			continue;
> +
> +		for_each_cpu(sib, sib_mask)
> +			per_cpu(amd_hybrid_caps, sib).highest_perf = sib_max;
> +
> +		if (sib_max > max_perf)
> +			max_perf = sib_max;
> +		if (sib_max < min_perf)
> +			min_perf = sib_max;
> +	}
> +
> +	if (!max_perf || min_perf >= max_perf)
> +		return;
> +
> +	if (!amd_hybrid_cap_enabled) {
> +		if (!arch_enable_hybrid_capacity_scale())
> +			return;

Capacity of a core derived from the fact whether it is classic or
dense, should not change at runtime and you can do this full setup at
amd_pstate_init() when the driver loads at boot.

That way you don't need to rebuild sched domains for mode switch
since it is done before the sched domains are built.

> +		amd_hybrid_cap_enabled = true;
> +		updated = true;
> +	}
> +
> +	if (max_perf != amd_hybrid_max_perf) {
> +		amd_hybrid_max_perf = max_perf;
> +		updated = true;
> +	}
> +
> +	for_each_online_cpu(cpu) {
> +		struct amd_hybrid_cpu_cap info = per_cpu(amd_hybrid_caps, cpu);
> +		unsigned long prev_cap;
> +
> +		if (!info.highest_perf || !info.nominal_perf)
> +			continue;
> +
> +		prev_cap = arch_scale_cpu_capacity(cpu);
> +		arch_set_cpu_capacity(cpu, info.highest_perf, max_perf,
> +				      info.highest_perf, info.nominal_perf);

Nope! Please don't use preferred core rankings for capacity.
Use amd_get_effective_highest_perf() to get the largest perf
value use that as a reference.

> +		topology_set_cpu_scale(cpu, arch_scale_cpu_capacity(cpu));
> +
> +		if (arch_scale_cpu_capacity(cpu) != prev_cap)
> +			updated = true;
> +	}
> +
> +	if (updated)
> +		schedule_work(&amd_hybrid_cap_work);
> +}
> +
>  static void amd_pstate_update_limits(struct cpufreq_policy *policy)
>  {
>  	struct amd_cpudata *cpudata;
> @@ -1130,6 +1244,7 @@ static int amd_pstate_cpu_init(struct cpufreq_policy *policy)
>  	if (ret)
>  		goto free_cpudata1;
>  
> +	amd_pstate_record_hybrid_cap(cpudata);
>  	amd_pstate_init_prefcore(cpudata);
>  
>  	ret = amd_pstate_init_freq(cpudata);
> @@ -1769,11 +1884,14 @@ static int amd_pstate_register_driver(int mode)
>  	if (amd_pstate_prefcore)
>  		sched_set_itmt_support();
>  
> +	amd_pstate_sync_hybrid_capacity();
> +
>  	return 0;
>  }
>  
>  static int amd_pstate_unregister_driver(int dummy)
>  {
> +	cancel_work_sync(&amd_hybrid_cap_work);
>  	cpufreq_unregister_driver(current_pstate_driver);
>  	amd_pstate_driver_cleanup();
>  	return 0;
> @@ -1985,6 +2103,7 @@ static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy)
>  	if (ret)
>  		goto free_cpudata1;
>  
> +	amd_pstate_record_hybrid_cap(cpudata);
>  	amd_pstate_init_prefcore(cpudata);
>  
>  	ret = amd_pstate_init_freq(cpudata);
> @@ -2149,6 +2268,8 @@ static int amd_pstate_cpu_online(struct cpufreq_policy *policy)
>  		return ret;
>  
>  	cached_floor_perf = freq_to_perf(perf, cpudata->nominal_freq, cpudata->floor_freq);
> +	amd_pstate_record_hybrid_cap(cpudata);
> +	amd_pstate_sync_hybrid_capacity();

Apparently boot CPU can now be a dense core which means if someone
boots with "maxcpus=1" and then decides to online rest of the CPUs
this bit must go and adjust capaity for rest of the system since
reference capacity has changed.

On MSR systems, you cannot find the core characteristics / highest
perf since it requires reading CPUID / CPPC_CAP MSR of an offline
core which isn't possible so you must account for it here.

Sched domain rebuild comes later so just adjusting capacities
should be enough.

Also, it all the classic cores go offline, in a system with
Dense and Low-power core, you'll have to start using the capacity
of the dense core as reference and adjust accordingly.

>  	return amd_pstate_set_floor_perf(policy, cached_floor_perf);
>  }
>  

-- 
Thanks and Regards,
Prateek


  reply	other threads:[~2026-11-13  7:29 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-13  2:29 [PATCH v2 0/2] cpufreq: amd-pstate, platform/x86/amd/hfi: Enable hybrid capacity scaling & SMT ITMT ordering on Zen 5 / Zen 5c Aiden Hecker
2026-11-13  2:29 ` [PATCH v2 1/2] cpufreq: amd-pstate: Enable hybrid CPU capacity scaling on heterogeneous Zen CPUs Aiden Hecker
2026-11-13  7:15   ` K Prateek Nayak [this message]
2026-11-14 16:25     ` Badole, Vishal
2026-11-13  2:29 ` [PATCH v2 2/2] platform/x86/amd/hfi: Rank SMT siblings below primary cores for ITMT packing Aiden Hecker
2026-11-13  7:18   ` K Prateek Nayak

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=bd326953-f693-4994-91e4-f8301430557a@amd.com \
    --to=kprateek.nayak@amd.com \
    --cc=Vishal.Badole@amd.com \
    --cc=hansg@kernel.org \
    --cc=heckeraiden@gmail.com \
    --cc=ilpo.jarvinen@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=mario.limonciello@amd.com \
    --cc=mingo@redhat.com \
    --cc=perry.yuan@amd.com \
    --cc=peterz@infradead.org \
    --cc=platform-driver-x86@vger.kernel.org \
    --cc=rafael@kernel.org \
    --cc=ray.huang@amd.com \
    --cc=viresh.kumar@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®