[PATCH v4 0/3] cpufreq: CPPC: Update FIE arch_freq_scale in ticks for non-PCC regs

[PATCH v4 0/3] cpufreq: CPPC: Update FIE arch_freq_scale in ticks for non-PCC regs

[Jie Zhan]

Currently, the CPPC Frequency Invariance Engine (FIE) is invoked from the
scheduler tick but defers the update of arch_freq_scale to a separate
thread because cppc_get_perf_ctrs() would sleep if the CPC regs are in PCC.

However, this deferred update mechanism is unnecessary and introduces extra
overhead for non-PCC register spaces (e.g. System Memory or FFH), where
accessing the regs won't sleep and can be safely performed from the tick
context.

Furthermore, with the CPPC FIE registered, it throws repeated warnings of
"cppc_scale_freq_workfn: failed to read perf counters" on our platform with
the CPC regs in System Memory and a power-down idle state enabled.  That's
because the remote CPU can be in a power-down idle state, and reading its
perf counters returns 0.  Moving the FIE handling back to the scheduler
tick process makes the CPU handle its own perf counters, so it won't be
idle and the issue would be inherently solved.

To address the above issues, update arch_freq_scale directly in ticks for
non-PCC regs and keep the deferred update mechanism for PCC regs.

We have tested this on Kunpeng SoCs with the CPC regs both in System Memory
and FFH.  More tests on other platforms are welcome (typically with the
regs in PCC).

Changelog:
v4:
- Allow either non-PCC or PCC scale_freq_tick callbacks to be registered
  for each cpufreq policy.
- Factor out cppc_perf_ctrs_in_pcc_cpu() and cppc_fie_kworker_init() for
  the above change.

v3:
https://lore.kernel.org/linux-pm/20251104065039.1675549-1-zhanjie9@hisilicon.com/
- Stash the state of 'cppc_perf_ctrs_in_pcc' so it won't have to check the
  CPC regs of all CPUs everywhere (Thanks to the suggestion from Beata
  Michalska).
- Update the commit log, explaining more on the warning issue caused by
  accessing perf counters on remote CPUs.
- Drop Patch 1 that has been accepted, and rebase Patch 2 on that.

v2:
https://lore.kernel.org/linux-pm/20250828110212.2108653-1-zhanjie9@hisilicon.com/
- Update the cover letter and the commit log based on v1 discussion
- Update FIE arch_freq_scale in ticks for non-PCC regs

v1:
https://lore.kernel.org/linux-pm/20250730032312.167062-1-yubowen8@huawei.com/

Jie Zhan (3):
  ACPI: CPPC: Factor out and export per-cpu cppc_perf_ctrs_in_pcc_cpu()
  cpufreq: CPPC: Factor out cppc_fie_kworker_init()
  cpufreq: CPPC: Update FIE arch_freq_scale in ticks for non-PCC regs

 drivers/acpi/cppc_acpi.c       | 45 ++++++++--------
 drivers/cpufreq/cppc_cpufreq.c | 96 ++++++++++++++++++++++------------
 include/acpi/cppc_acpi.h       |  5 ++
 3 files changed, 93 insertions(+), 53 deletions(-)

-- 
2.33.0

[PATCH v4 1/3] ACPI: CPPC: Factor out and export per-cpu cppc_perf_ctrs_in_pcc_cpu()

[Jie Zhan]

Factor out cppc_perf_ctrs_in_pcc_cpu() for checking whether per-cpu CPC
regs are defined in PCC channels, and export it out for further use.

Signed-off-by: Jie Zhan <zhanjie9@hisilicon.com>
---
 drivers/acpi/cppc_acpi.c | 45 +++++++++++++++++++++-------------------
 include/acpi/cppc_acpi.h |  5 +++++
 2 files changed, 29 insertions(+), 21 deletions(-)

diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
index 3bdeeee3414e..aa80dbcf42c0 100644
--- a/drivers/acpi/cppc_acpi.c
+++ b/drivers/acpi/cppc_acpi.c
@@ -1422,6 +1422,29 @@ int cppc_get_perf_caps(int cpunum, struct cppc_perf_caps *perf_caps)
 }
 EXPORT_SYMBOL_GPL(cppc_get_perf_caps);
 
+bool cppc_perf_ctrs_in_pcc_cpu(unsigned int cpu)
+{
+	struct cpc_desc *cpc_desc = per_cpu(cpc_desc_ptr, cpu);
+	struct cpc_register_resource *ref_perf_reg;
+
+	/*
+	 * If reference perf register is not supported then we should use the
+	 * nominal perf value
+	 */
+	ref_perf_reg = &cpc_desc->cpc_regs[REFERENCE_PERF];
+	if (!CPC_SUPPORTED(ref_perf_reg))
+		ref_perf_reg = &cpc_desc->cpc_regs[NOMINAL_PERF];
+
+	if (CPC_IN_PCC(&cpc_desc->cpc_regs[DELIVERED_CTR]) ||
+	    CPC_IN_PCC(&cpc_desc->cpc_regs[REFERENCE_CTR]) ||
+	    CPC_IN_PCC(&cpc_desc->cpc_regs[CTR_WRAP_TIME]) ||
+	    CPC_IN_PCC(ref_perf_reg))
+		return true;
+
+	return false;
+}
+EXPORT_SYMBOL_GPL(cppc_perf_ctrs_in_pcc_cpu);
+
 /**
  * cppc_perf_ctrs_in_pcc - Check if any perf counters are in a PCC region.
  *
@@ -1436,27 +1459,7 @@ bool cppc_perf_ctrs_in_pcc(void)
 	int cpu;
 
 	for_each_online_cpu(cpu) {
-		struct cpc_register_resource *ref_perf_reg;
-		struct cpc_desc *cpc_desc;
-
-		cpc_desc = per_cpu(cpc_desc_ptr, cpu);
-
-		if (CPC_IN_PCC(&cpc_desc->cpc_regs[DELIVERED_CTR]) ||
-		    CPC_IN_PCC(&cpc_desc->cpc_regs[REFERENCE_CTR]) ||
-		    CPC_IN_PCC(&cpc_desc->cpc_regs[CTR_WRAP_TIME]))
-			return true;
-
-
-		ref_perf_reg = &cpc_desc->cpc_regs[REFERENCE_PERF];
-
-		/*
-		 * If reference perf register is not supported then we should
-		 * use the nominal perf value
-		 */
-		if (!CPC_SUPPORTED(ref_perf_reg))
-			ref_perf_reg = &cpc_desc->cpc_regs[NOMINAL_PERF];
-
-		if (CPC_IN_PCC(ref_perf_reg))
+		if (cppc_perf_ctrs_in_pcc_cpu(cpu))
 			return true;
 	}
 
diff --git a/include/acpi/cppc_acpi.h b/include/acpi/cppc_acpi.h
index 13fa81504844..4bcdcaf8bf2c 100644
--- a/include/acpi/cppc_acpi.h
+++ b/include/acpi/cppc_acpi.h
@@ -154,6 +154,7 @@ extern int cppc_get_perf_ctrs(int cpu, struct cppc_perf_fb_ctrs *perf_fb_ctrs);
 extern int cppc_set_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls);
 extern int cppc_set_enable(int cpu, bool enable);
 extern int cppc_get_perf_caps(int cpu, struct cppc_perf_caps *caps);
+extern bool cppc_perf_ctrs_in_pcc_cpu(unsigned int cpu);
 extern bool cppc_perf_ctrs_in_pcc(void);
 extern unsigned int cppc_perf_to_khz(struct cppc_perf_caps *caps, unsigned int perf);
 extern unsigned int cppc_khz_to_perf(struct cppc_perf_caps *caps, unsigned int freq);
@@ -204,6 +205,10 @@ static inline int cppc_get_perf_caps(int cpu, struct cppc_perf_caps *caps)
 {
 	return -EOPNOTSUPP;
 }
+static inline bool cppc_perf_ctrs_in_pcc_cpu(unsigned int cpu)
+{
+	return false;
+}
 static inline bool cppc_perf_ctrs_in_pcc(void)
 {
 	return false;
-- 
2.33.0

[PATCH v4 2/3] cpufreq: CPPC: Factor out cppc_fie_kworker_init()

[Jie Zhan]

Factor out the CPPC FIE kworker init in cppc_freq_invariance_init() because
it's a standalone procedure for use when the CPC regs are in PCC channels.

Signed-off-by: Jie Zhan <zhanjie9@hisilicon.com>
---
 drivers/cpufreq/cppc_cpufreq.c | 29 +++++++++++++++++------------
 1 file changed, 17 insertions(+), 12 deletions(-)

diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
index 9eac77c4f294..947b4e2e1d4e 100644
--- a/drivers/cpufreq/cppc_cpufreq.c
+++ b/drivers/cpufreq/cppc_cpufreq.c
@@ -184,7 +184,7 @@ static void cppc_cpufreq_cpu_fie_exit(struct cpufreq_policy *policy)
 	}
 }
 
-static void __init cppc_freq_invariance_init(void)
+static void cppc_fie_kworker_init(void)
 {
 	struct sched_attr attr = {
 		.size		= sizeof(struct sched_attr),
@@ -201,17 +201,6 @@ static void __init cppc_freq_invariance_init(void)
 	};
 	int ret;
 
-	if (fie_disabled != FIE_ENABLED && fie_disabled != FIE_DISABLED) {
-		fie_disabled = FIE_ENABLED;
-		if (cppc_perf_ctrs_in_pcc()) {
-			pr_info("FIE not enabled on systems with registers in PCC\n");
-			fie_disabled = FIE_DISABLED;
-		}
-	}
-
-	if (fie_disabled)
-		return;
-
 	kworker_fie = kthread_run_worker(0, "cppc_fie");
 	if (IS_ERR(kworker_fie)) {
 		pr_warn("%s: failed to create kworker_fie: %ld\n", __func__,
@@ -229,6 +218,22 @@ static void __init cppc_freq_invariance_init(void)
 	}
 }
 
+static void __init cppc_freq_invariance_init(void)
+{
+	if (fie_disabled != FIE_ENABLED && fie_disabled != FIE_DISABLED) {
+		fie_disabled = FIE_ENABLED;
+		if (cppc_perf_ctrs_in_pcc()) {
+			pr_info("FIE not enabled on systems with registers in PCC\n");
+			fie_disabled = FIE_DISABLED;
+		}
+	}
+
+	if (fie_disabled)
+		return;
+
+	cppc_fie_kworker_init();
+}
+
 static void cppc_freq_invariance_exit(void)
 {
 	if (fie_disabled)
-- 
2.33.0

[PATCH v4 3/3] cpufreq: CPPC: Update FIE arch_freq_scale in ticks for non-PCC regs

[Jie Zhan]

Currently, the CPPC Frequency Invariance Engine (FIE) is invoked from the
scheduler tick but defers the update of arch_freq_scale to a separate
thread because cppc_get_perf_ctrs() would sleep if the CPC regs are in PCC.

However, this deferred update mechanism is unnecessary and introduces extra
overhead for non-PCC register spaces (e.g. System Memory or FFH), where
accessing the regs won't sleep and can be safely performed from the tick
context.

Furthermore, with the CPPC FIE registered, it throws repeated warnings of
"cppc_scale_freq_workfn: failed to read perf counters" on our platform with
the CPC regs in System Memory and a power-down idle state enabled.  That's
because the remote CPU can be in a power-down idle state, and reading its
perf counters returns 0.  Moving the FIE handling back to the scheduler
tick process makes the CPU handle its own perf counters, so it won't be
idle and the issue would be inherently solved.

To address the above issues, update arch_freq_scale directly in ticks for
non-PCC regs and keep the deferred update mechanism for PCC regs.

Signed-off-by: Jie Zhan <zhanjie9@hisilicon.com>
---
 drivers/cpufreq/cppc_cpufreq.c | 77 +++++++++++++++++++++++-----------
 1 file changed, 52 insertions(+), 25 deletions(-)

diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
index 947b4e2e1d4e..36e8a75a37f1 100644
--- a/drivers/cpufreq/cppc_cpufreq.c
+++ b/drivers/cpufreq/cppc_cpufreq.c
@@ -54,31 +54,24 @@ static int cppc_perf_from_fbctrs(struct cppc_perf_fb_ctrs *fb_ctrs_t0,
 				 struct cppc_perf_fb_ctrs *fb_ctrs_t1);
 
 /**
- * cppc_scale_freq_workfn - CPPC arch_freq_scale updater for frequency invariance
- * @work: The work item.
+ * __cppc_scale_freq_tick - CPPC arch_freq_scale updater for frequency invariance
+ * @cppc_fi: per-cpu CPPC FIE data.
  *
- * The CPPC driver register itself with the topology core to provide its own
+ * The CPPC driver registers itself with the topology core to provide its own
  * implementation (cppc_scale_freq_tick()) of topology_scale_freq_tick() which
  * gets called by the scheduler on every tick.
  *
  * Note that the arch specific counters have higher priority than CPPC counters,
  * if available, though the CPPC driver doesn't need to have any special
  * handling for that.
- *
- * On an invocation of cppc_scale_freq_tick(), we schedule an irq work (since we
- * reach here from hard-irq context), which then schedules a normal work item
- * and cppc_scale_freq_workfn() updates the per_cpu arch_freq_scale variable
- * based on the counter updates since the last tick.
  */
-static void cppc_scale_freq_workfn(struct kthread_work *work)
+static void __cppc_scale_freq_tick(struct cppc_freq_invariance *cppc_fi)
 {
-	struct cppc_freq_invariance *cppc_fi;
 	struct cppc_perf_fb_ctrs fb_ctrs = {0};
 	struct cppc_cpudata *cpu_data;
 	unsigned long local_freq_scale;
 	u64 perf;
 
-	cppc_fi = container_of(work, struct cppc_freq_invariance, work);
 	cpu_data = cppc_fi->cpu_data;
 
 	if (cppc_get_perf_ctrs(cppc_fi->cpu, &fb_ctrs)) {
@@ -102,6 +95,24 @@ static void cppc_scale_freq_workfn(struct kthread_work *work)
 	per_cpu(arch_freq_scale, cppc_fi->cpu) = local_freq_scale;
 }
 
+static void cppc_scale_freq_tick(void)
+{
+	__cppc_scale_freq_tick(&per_cpu(cppc_freq_inv, smp_processor_id()));
+}
+
+static struct scale_freq_data cppc_sftd = {
+	.source = SCALE_FREQ_SOURCE_CPPC,
+	.set_freq_scale = cppc_scale_freq_tick,
+};
+
+static void cppc_scale_freq_workfn(struct kthread_work *work)
+{
+	struct cppc_freq_invariance *cppc_fi;
+
+	cppc_fi = container_of(work, struct cppc_freq_invariance, work);
+	__cppc_scale_freq_tick(cppc_fi);
+}
+
 static void cppc_irq_work(struct irq_work *irq_work)
 {
 	struct cppc_freq_invariance *cppc_fi;
@@ -110,7 +121,14 @@ static void cppc_irq_work(struct irq_work *irq_work)
 	kthread_queue_work(kworker_fie, &cppc_fi->work);
 }
 
-static void cppc_scale_freq_tick(void)
+/*
+ * Reading perf counters may sleep if the CPC regs are in PCC.  Thus, we
+ * schedule an irq work in scale_freq_tick (since we reach here from hard-irq
+ * context), which then schedules a normal work item cppc_scale_freq_workfn()
+ * that updates the per_cpu arch_freq_scale variable based on the counter
+ * updates since the last tick.
+ */
+static void cppc_scale_freq_tick_pcc(void)
 {
 	struct cppc_freq_invariance *cppc_fi = &per_cpu(cppc_freq_inv, smp_processor_id());
 
@@ -121,13 +139,14 @@ static void cppc_scale_freq_tick(void)
 	irq_work_queue(&cppc_fi->irq_work);
 }
 
-static struct scale_freq_data cppc_sftd = {
+static struct scale_freq_data cppc_sftd_pcc = {
 	.source = SCALE_FREQ_SOURCE_CPPC,
-	.set_freq_scale = cppc_scale_freq_tick,
+	.set_freq_scale = cppc_scale_freq_tick_pcc,
 };
 
 static void cppc_cpufreq_cpu_fie_init(struct cpufreq_policy *policy)
 {
+	struct scale_freq_data *sftd = &cppc_sftd;
 	struct cppc_freq_invariance *cppc_fi;
 	int cpu, ret;
 
@@ -138,8 +157,11 @@ static void cppc_cpufreq_cpu_fie_init(struct cpufreq_policy *policy)
 		cppc_fi = &per_cpu(cppc_freq_inv, cpu);
 		cppc_fi->cpu = cpu;
 		cppc_fi->cpu_data = policy->driver_data;
-		kthread_init_work(&cppc_fi->work, cppc_scale_freq_workfn);
-		init_irq_work(&cppc_fi->irq_work, cppc_irq_work);
+		if (cppc_perf_ctrs_in_pcc_cpu(cpu)) {
+			kthread_init_work(&cppc_fi->work, cppc_scale_freq_workfn);
+			init_irq_work(&cppc_fi->irq_work, cppc_irq_work);
+			sftd = &cppc_sftd_pcc;
+		}
 
 		ret = cppc_get_perf_ctrs(cpu, &cppc_fi->prev_perf_fb_ctrs);
 
@@ -155,7 +177,7 @@ static void cppc_cpufreq_cpu_fie_init(struct cpufreq_policy *policy)
 	}
 
 	/* Register for freq-invariance */
-	topology_set_scale_freq_source(&cppc_sftd, policy->cpus);
+	topology_set_scale_freq_source(sftd, policy->cpus);
 }
 
 /*
@@ -178,6 +200,8 @@ static void cppc_cpufreq_cpu_fie_exit(struct cpufreq_policy *policy)
 	topology_clear_scale_freq_source(SCALE_FREQ_SOURCE_CPPC, policy->related_cpus);
 
 	for_each_cpu(cpu, policy->related_cpus) {
+		if (!cppc_perf_ctrs_in_pcc_cpu(cpu))
+			continue;
 		cppc_fi = &per_cpu(cppc_freq_inv, cpu);
 		irq_work_sync(&cppc_fi->irq_work);
 		kthread_cancel_work_sync(&cppc_fi->work);
@@ -206,6 +230,7 @@ static void cppc_fie_kworker_init(void)
 		pr_warn("%s: failed to create kworker_fie: %ld\n", __func__,
 			PTR_ERR(kworker_fie));
 		fie_disabled = FIE_DISABLED;
+		kworker_fie = NULL;
 		return;
 	}
 
@@ -215,20 +240,24 @@ static void cppc_fie_kworker_init(void)
 			ret);
 		kthread_destroy_worker(kworker_fie);
 		fie_disabled = FIE_DISABLED;
+		kworker_fie = NULL;
 	}
 }
 
 static void __init cppc_freq_invariance_init(void)
 {
-	if (fie_disabled != FIE_ENABLED && fie_disabled != FIE_DISABLED) {
-		fie_disabled = FIE_ENABLED;
-		if (cppc_perf_ctrs_in_pcc()) {
+	bool perf_ctrs_in_pcc = cppc_perf_ctrs_in_pcc();
+
+	if (fie_disabled == FIE_UNSET) {
+		if (perf_ctrs_in_pcc) {
 			pr_info("FIE not enabled on systems with registers in PCC\n");
 			fie_disabled = FIE_DISABLED;
+		} else {
+			fie_disabled = FIE_ENABLED;
 		}
 	}
 
-	if (fie_disabled)
+	if (fie_disabled || !perf_ctrs_in_pcc)
 		return;
 
 	cppc_fie_kworker_init();
@@ -236,10 +265,8 @@ static void __init cppc_freq_invariance_init(void)
 
 static void cppc_freq_invariance_exit(void)
 {
-	if (fie_disabled)
-		return;
-
-	kthread_destroy_worker(kworker_fie);
+	if (kworker_fie)
+		kthread_destroy_worker(kworker_fie);
 }
 
 #else
-- 
2.33.0

Re: [PATCH v4 0/3] cpufreq: CPPC: Update FIE arch_freq_scale in ticks for non-PCC regs

[zhenglifeng (A)]

Reviewed-by: Lifeng Zheng <zhenglifeng1@huawei.com>

On 2025/12/3 11:24, Jie Zhan wrote:
> Currently, the CPPC Frequency Invariance Engine (FIE) is invoked from the
> scheduler tick but defers the update of arch_freq_scale to a separate
> thread because cppc_get_perf_ctrs() would sleep if the CPC regs are in PCC.
> 
> However, this deferred update mechanism is unnecessary and introduces extra
> overhead for non-PCC register spaces (e.g. System Memory or FFH), where
> accessing the regs won't sleep and can be safely performed from the tick
> context.
> 
> Furthermore, with the CPPC FIE registered, it throws repeated warnings of
> "cppc_scale_freq_workfn: failed to read perf counters" on our platform with
> the CPC regs in System Memory and a power-down idle state enabled.  That's
> because the remote CPU can be in a power-down idle state, and reading its
> perf counters returns 0.  Moving the FIE handling back to the scheduler
> tick process makes the CPU handle its own perf counters, so it won't be
> idle and the issue would be inherently solved.
> 
> To address the above issues, update arch_freq_scale directly in ticks for
> non-PCC regs and keep the deferred update mechanism for PCC regs.
> 
> We have tested this on Kunpeng SoCs with the CPC regs both in System Memory
> and FFH.  More tests on other platforms are welcome (typically with the
> regs in PCC).
> 
> Changelog:
> v4:
> - Allow either non-PCC or PCC scale_freq_tick callbacks to be registered
>   for each cpufreq policy.
> - Factor out cppc_perf_ctrs_in_pcc_cpu() and cppc_fie_kworker_init() for
>   the above change.
> 
> v3:
> https://lore.kernel.org/linux-pm/20251104065039.1675549-1-zhanjie9@hisilicon.com/
> - Stash the state of 'cppc_perf_ctrs_in_pcc' so it won't have to check the
>   CPC regs of all CPUs everywhere (Thanks to the suggestion from Beata
>   Michalska).
> - Update the commit log, explaining more on the warning issue caused by
>   accessing perf counters on remote CPUs.
> - Drop Patch 1 that has been accepted, and rebase Patch 2 on that.
> 
> v2:
> https://lore.kernel.org/linux-pm/20250828110212.2108653-1-zhanjie9@hisilicon.com/
> - Update the cover letter and the commit log based on v1 discussion
> - Update FIE arch_freq_scale in ticks for non-PCC regs
> 
> v1:
> https://lore.kernel.org/linux-pm/20250730032312.167062-1-yubowen8@huawei.com/
> 
> Jie Zhan (3):
>   ACPI: CPPC: Factor out and export per-cpu cppc_perf_ctrs_in_pcc_cpu()
>   cpufreq: CPPC: Factor out cppc_fie_kworker_init()
>   cpufreq: CPPC: Update FIE arch_freq_scale in ticks for non-PCC regs
> 
>  drivers/acpi/cppc_acpi.c       | 45 ++++++++--------
>  drivers/cpufreq/cppc_cpufreq.c | 96 ++++++++++++++++++++++------------
>  include/acpi/cppc_acpi.h       |  5 ++
>  3 files changed, 93 insertions(+), 53 deletions(-)
> 

Re: [PATCH v4 1/3] ACPI: CPPC: Factor out and export per-cpu cppc_perf_ctrs_in_pcc_cpu()

[Rafael J. Wysocki]

On Wed, Dec 3, 2025 at 4:25 AM Jie Zhan <zhanjie9@hisilicon.com> wrote:
>
> Factor out cppc_perf_ctrs_in_pcc_cpu() for checking whether per-cpu CPC
> regs are defined in PCC channels, and export it out for further use.
>
> Signed-off-by: Jie Zhan <zhanjie9@hisilicon.com>
> ---
>  drivers/acpi/cppc_acpi.c | 45 +++++++++++++++++++++-------------------
>  include/acpi/cppc_acpi.h |  5 +++++
>  2 files changed, 29 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
> index 3bdeeee3414e..aa80dbcf42c0 100644
> --- a/drivers/acpi/cppc_acpi.c
> +++ b/drivers/acpi/cppc_acpi.c
> @@ -1422,6 +1422,29 @@ int cppc_get_perf_caps(int cpunum, struct cppc_perf_caps *perf_caps)
>  }
>  EXPORT_SYMBOL_GPL(cppc_get_perf_caps);
>
> +bool cppc_perf_ctrs_in_pcc_cpu(unsigned int cpu)
> +{
> +       struct cpc_desc *cpc_desc = per_cpu(cpc_desc_ptr, cpu);
> +       struct cpc_register_resource *ref_perf_reg;
> +
> +       /*
> +        * If reference perf register is not supported then we should use the
> +        * nominal perf value
> +        */
> +       ref_perf_reg = &cpc_desc->cpc_regs[REFERENCE_PERF];
> +       if (!CPC_SUPPORTED(ref_perf_reg))
> +               ref_perf_reg = &cpc_desc->cpc_regs[NOMINAL_PERF];
> +
> +       if (CPC_IN_PCC(&cpc_desc->cpc_regs[DELIVERED_CTR]) ||
> +           CPC_IN_PCC(&cpc_desc->cpc_regs[REFERENCE_CTR]) ||
> +           CPC_IN_PCC(&cpc_desc->cpc_regs[CTR_WRAP_TIME]) ||
> +           CPC_IN_PCC(ref_perf_reg))
> +               return true;
> +
> +       return false;

Why not

return CPC_IN_PCC(&cpc_desc->cpc_regs[DELIVERED_CTR]) ||
          CPC_IN_PCC(&cpc_desc->cpc_regs[REFERENCE_CTR]) ||
          CPC_IN_PCC(&cpc_desc->cpc_regs[CTR_WRAP_TIME]) ||
          CPC_IN_PCC(ref_perf_reg);

> +}
> +EXPORT_SYMBOL_GPL(cppc_perf_ctrs_in_pcc_cpu);
> +
>  /**
>   * cppc_perf_ctrs_in_pcc - Check if any perf counters are in a PCC region.
>   *
> @@ -1436,27 +1459,7 @@ bool cppc_perf_ctrs_in_pcc(void)
>         int cpu;
>
>         for_each_online_cpu(cpu) {
> -               struct cpc_register_resource *ref_perf_reg;
> -               struct cpc_desc *cpc_desc;
> -
> -               cpc_desc = per_cpu(cpc_desc_ptr, cpu);
> -
> -               if (CPC_IN_PCC(&cpc_desc->cpc_regs[DELIVERED_CTR]) ||
> -                   CPC_IN_PCC(&cpc_desc->cpc_regs[REFERENCE_CTR]) ||
> -                   CPC_IN_PCC(&cpc_desc->cpc_regs[CTR_WRAP_TIME]))
> -                       return true;
> -
> -
> -               ref_perf_reg = &cpc_desc->cpc_regs[REFERENCE_PERF];
> -
> -               /*
> -                * If reference perf register is not supported then we should
> -                * use the nominal perf value
> -                */
> -               if (!CPC_SUPPORTED(ref_perf_reg))
> -                       ref_perf_reg = &cpc_desc->cpc_regs[NOMINAL_PERF];
> -
> -               if (CPC_IN_PCC(ref_perf_reg))
> +               if (cppc_perf_ctrs_in_pcc_cpu(cpu))
>                         return true;
>         }
>
> diff --git a/include/acpi/cppc_acpi.h b/include/acpi/cppc_acpi.h
> index 13fa81504844..4bcdcaf8bf2c 100644
> --- a/include/acpi/cppc_acpi.h
> +++ b/include/acpi/cppc_acpi.h
> @@ -154,6 +154,7 @@ extern int cppc_get_perf_ctrs(int cpu, struct cppc_perf_fb_ctrs *perf_fb_ctrs);
>  extern int cppc_set_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls);
>  extern int cppc_set_enable(int cpu, bool enable);
>  extern int cppc_get_perf_caps(int cpu, struct cppc_perf_caps *caps);
> +extern bool cppc_perf_ctrs_in_pcc_cpu(unsigned int cpu);
>  extern bool cppc_perf_ctrs_in_pcc(void);
>  extern unsigned int cppc_perf_to_khz(struct cppc_perf_caps *caps, unsigned int perf);
>  extern unsigned int cppc_khz_to_perf(struct cppc_perf_caps *caps, unsigned int freq);
> @@ -204,6 +205,10 @@ static inline int cppc_get_perf_caps(int cpu, struct cppc_perf_caps *caps)
>  {
>         return -EOPNOTSUPP;
>  }
> +static inline bool cppc_perf_ctrs_in_pcc_cpu(unsigned int cpu)
> +{
> +       return false;
> +}
>  static inline bool cppc_perf_ctrs_in_pcc(void)
>  {
>         return false;
> --
> 2.33.0
>
>

Re: [PATCH v4 1/3] ACPI: CPPC: Factor out and export per-cpu cppc_perf_ctrs_in_pcc_cpu()

[Jie Zhan]

On 12/5/2025 11:13 PM, Rafael J. Wysocki wrote:
> On Wed, Dec 3, 2025 at 4:25 AM Jie Zhan <zhanjie9@hisilicon.com> wrote:
>>
>> Factor out cppc_perf_ctrs_in_pcc_cpu() for checking whether per-cpu CPC
>> regs are defined in PCC channels, and export it out for further use.
>>
>> Signed-off-by: Jie Zhan <zhanjie9@hisilicon.com>
>> ---
>>  drivers/acpi/cppc_acpi.c | 45 +++++++++++++++++++++-------------------
>>  include/acpi/cppc_acpi.h |  5 +++++
>>  2 files changed, 29 insertions(+), 21 deletions(-)
>>
>> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
>> index 3bdeeee3414e..aa80dbcf42c0 100644
>> --- a/drivers/acpi/cppc_acpi.c
>> +++ b/drivers/acpi/cppc_acpi.c
>> @@ -1422,6 +1422,29 @@ int cppc_get_perf_caps(int cpunum, struct cppc_perf_caps *perf_caps)
>>  }
>>  EXPORT_SYMBOL_GPL(cppc_get_perf_caps);
>>
>> +bool cppc_perf_ctrs_in_pcc_cpu(unsigned int cpu)
>> +{
>> +       struct cpc_desc *cpc_desc = per_cpu(cpc_desc_ptr, cpu);
>> +       struct cpc_register_resource *ref_perf_reg;
>> +
>> +       /*
>> +        * If reference perf register is not supported then we should use the
>> +        * nominal perf value
>> +        */
>> +       ref_perf_reg = &cpc_desc->cpc_regs[REFERENCE_PERF];
>> +       if (!CPC_SUPPORTED(ref_perf_reg))
>> +               ref_perf_reg = &cpc_desc->cpc_regs[NOMINAL_PERF];
>> +
>> +       if (CPC_IN_PCC(&cpc_desc->cpc_regs[DELIVERED_CTR]) ||
>> +           CPC_IN_PCC(&cpc_desc->cpc_regs[REFERENCE_CTR]) ||
>> +           CPC_IN_PCC(&cpc_desc->cpc_regs[CTR_WRAP_TIME]) ||
>> +           CPC_IN_PCC(ref_perf_reg))
>> +               return true;
>> +
>> +       return false;
> 
> Why not
> 
> return CPC_IN_PCC(&cpc_desc->cpc_regs[DELIVERED_CTR]) ||
>           CPC_IN_PCC(&cpc_desc->cpc_regs[REFERENCE_CTR]) ||
>           CPC_IN_PCC(&cpc_desc->cpc_regs[CTR_WRAP_TIME]) ||
>           CPC_IN_PCC(ref_perf_reg);
> 
Yes, that would save a few more lines.
Thanks!

Re: [PATCH v4 3/3] cpufreq: CPPC: Update FIE arch_freq_scale in ticks for non-PCC regs

[Pierre Gondois]

Hello Jie Zhan,


On 12/3/25 04:24, Jie Zhan wrote:
> Currently, the CPPC Frequency Invariance Engine (FIE) is invoked from the
> scheduler tick but defers the update of arch_freq_scale to a separate
> thread because cppc_get_perf_ctrs() would sleep if the CPC regs are in PCC.
>
> However, this deferred update mechanism is unnecessary and introduces extra
> overhead for non-PCC register spaces (e.g. System Memory or FFH), where
> accessing the regs won't sleep and can be safely performed from the tick
> context.
>
> Furthermore, with the CPPC FIE registered, it throws repeated warnings of
> "cppc_scale_freq_workfn: failed to read perf counters" on our platform with
> the CPC regs in System Memory and a power-down idle state enabled.  That's
> because the remote CPU can be in a power-down idle state, and reading its
> perf counters returns 0.  Moving the FIE handling back to the scheduler
> tick process makes the CPU handle its own perf counters, so it won't be
> idle and the issue would be inherently solved.
>
> To address the above issues, update arch_freq_scale directly in ticks for
> non-PCC regs and keep the deferred update mechanism for PCC regs.
>
> Signed-off-by: Jie Zhan<zhanjie9@hisilicon.com>
> ---
>   drivers/cpufreq/cppc_cpufreq.c | 77 +++++++++++++++++++++++-----------
>   1 file changed, 52 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
> index 947b4e2e1d4e..36e8a75a37f1 100644
> --- a/drivers/cpufreq/cppc_cpufreq.c
> +++ b/drivers/cpufreq/cppc_cpufreq.c
[...]
> @@ -236,10 +265,8 @@ static void __init cppc_freq_invariance_init(void)
>   
>   static void cppc_freq_invariance_exit(void)
>   {
> -	if (fie_disabled)
> -		return;
> -
> -	kthread_destroy_worker(kworker_fie);
> +	if (kworker_fie)
> +		kthread_destroy_worker(kworker_fie);

Shouldn't we have:
   kworker_fie = NULL;
here aswell ?

>   }
>   
>   #else


Otherwise, for the 3 patches:

Reviewed-by: Pierre Gondois <pierre.gondois@arm.com>

Re: [PATCH v4 1/3] ACPI: CPPC: Factor out and export per-cpu cppc_perf_ctrs_in_pcc_cpu()

[Beata Michalska]

On Wed, Dec 03, 2025 at 11:24:20AM +0800, Jie Zhan wrote:
> Factor out cppc_perf_ctrs_in_pcc_cpu() for checking whether per-cpu CPC
> regs are defined in PCC channels, and export it out for further use.
> 
> Signed-off-by: Jie Zhan <zhanjie9@hisilicon.com>
> ---
>  drivers/acpi/cppc_acpi.c | 45 +++++++++++++++++++++-------------------
>  include/acpi/cppc_acpi.h |  5 +++++
>  2 files changed, 29 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
> index 3bdeeee3414e..aa80dbcf42c0 100644
> --- a/drivers/acpi/cppc_acpi.c
> +++ b/drivers/acpi/cppc_acpi.c
> @@ -1422,6 +1422,29 @@ int cppc_get_perf_caps(int cpunum, struct cppc_perf_caps *perf_caps)
>  }
>  EXPORT_SYMBOL_GPL(cppc_get_perf_caps);
>  
> +bool cppc_perf_ctrs_in_pcc_cpu(unsigned int cpu)
> +{
> +	struct cpc_desc *cpc_desc = per_cpu(cpc_desc_ptr, cpu);
> +	struct cpc_register_resource *ref_perf_reg;
> +
> +	/*
> +	 * If reference perf register is not supported then we should use the
> +	 * nominal perf value
> +	 */
> +	ref_perf_reg = &cpc_desc->cpc_regs[REFERENCE_PERF];
> +	if (!CPC_SUPPORTED(ref_perf_reg))
> +		ref_perf_reg = &cpc_desc->cpc_regs[NOMINAL_PERF];
> +
> +	if (CPC_IN_PCC(&cpc_desc->cpc_regs[DELIVERED_CTR]) ||
> +	    CPC_IN_PCC(&cpc_desc->cpc_regs[REFERENCE_CTR]) ||
> +	    CPC_IN_PCC(&cpc_desc->cpc_regs[CTR_WRAP_TIME]) ||
> +	    CPC_IN_PCC(ref_perf_reg))
> +		return true;
> +
> +	return false;
> +}
> +EXPORT_SYMBOL_GPL(cppc_perf_ctrs_in_pcc_cpu);
It is minor, but I would prefer the earlier version when we grab the reference
performance reg only when none of the other regs is in the PCC.

---
BR
Beata
> +
>  /**
>   * cppc_perf_ctrs_in_pcc - Check if any perf counters are in a PCC region.
>   *
> @@ -1436,27 +1459,7 @@ bool cppc_perf_ctrs_in_pcc(void)
>  	int cpu;
>  
>  	for_each_online_cpu(cpu) {
> -		struct cpc_register_resource *ref_perf_reg;
> -		struct cpc_desc *cpc_desc;
> -
> -		cpc_desc = per_cpu(cpc_desc_ptr, cpu);
> -
> -		if (CPC_IN_PCC(&cpc_desc->cpc_regs[DELIVERED_CTR]) ||
> -		    CPC_IN_PCC(&cpc_desc->cpc_regs[REFERENCE_CTR]) ||
> -		    CPC_IN_PCC(&cpc_desc->cpc_regs[CTR_WRAP_TIME]))
> -			return true;
> -
> -
> -		ref_perf_reg = &cpc_desc->cpc_regs[REFERENCE_PERF];
> -
> -		/*
> -		 * If reference perf register is not supported then we should
> -		 * use the nominal perf value
> -		 */
> -		if (!CPC_SUPPORTED(ref_perf_reg))
> -			ref_perf_reg = &cpc_desc->cpc_regs[NOMINAL_PERF];
> -
> -		if (CPC_IN_PCC(ref_perf_reg))
> +		if (cppc_perf_ctrs_in_pcc_cpu(cpu))
>  			return true;
>  	}
>  
> diff --git a/include/acpi/cppc_acpi.h b/include/acpi/cppc_acpi.h
> index 13fa81504844..4bcdcaf8bf2c 100644
> --- a/include/acpi/cppc_acpi.h
> +++ b/include/acpi/cppc_acpi.h
> @@ -154,6 +154,7 @@ extern int cppc_get_perf_ctrs(int cpu, struct cppc_perf_fb_ctrs *perf_fb_ctrs);
>  extern int cppc_set_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls);
>  extern int cppc_set_enable(int cpu, bool enable);
>  extern int cppc_get_perf_caps(int cpu, struct cppc_perf_caps *caps);
> +extern bool cppc_perf_ctrs_in_pcc_cpu(unsigned int cpu);
>  extern bool cppc_perf_ctrs_in_pcc(void);
>  extern unsigned int cppc_perf_to_khz(struct cppc_perf_caps *caps, unsigned int perf);
>  extern unsigned int cppc_khz_to_perf(struct cppc_perf_caps *caps, unsigned int freq);
> @@ -204,6 +205,10 @@ static inline int cppc_get_perf_caps(int cpu, struct cppc_perf_caps *caps)
>  {
>  	return -EOPNOTSUPP;
>  }
> +static inline bool cppc_perf_ctrs_in_pcc_cpu(unsigned int cpu)
> +{
> +	return false;
> +}
>  static inline bool cppc_perf_ctrs_in_pcc(void)
>  {
>  	return false;
> -- 
> 2.33.0
> 

Re: [PATCH v4 3/3] cpufreq: CPPC: Update FIE arch_freq_scale in ticks for non-PCC regs

[Jie Zhan]

Hi Pierre,

On 12/8/2025 6:08 PM, Pierre Gondois wrote:
> Hello Jie Zhan,
> 
> 
> On 12/3/25 04:24, Jie Zhan wrote:
...
>> @@ -236,10 +265,8 @@ static void __init cppc_freq_invariance_init(void)
>>     static void cppc_freq_invariance_exit(void)
>>   {
>> -    if (fie_disabled)
>> -        return;
>> -
>> -    kthread_destroy_worker(kworker_fie);
>> +    if (kworker_fie)
>> +        kthread_destroy_worker(kworker_fie);
> 
> Shouldn't we have:
>   kworker_fie = NULL;
> here aswell ?
> 
I don't think it's needed.
This is where we unload the cppc_cpufreq module.  'kworker_fie' will no
longer be used after this, and it will be initialized to NULL again when
reloading the module.

I can't think of any other buggy scenarios.
>>   }
>>     #else
> 
> 
> Otherwise, for the 3 patches:
> 
> Reviewed-by: Pierre Gondois <pierre.gondois@arm.com>
> 
Thanks!
Jie

Re: [PATCH v4 1/3] ACPI: CPPC: Factor out and export per-cpu cppc_perf_ctrs_in_pcc_cpu()

[Jie Zhan]

On 12/9/2025 12:17 AM, Beata Michalska wrote:
> On Wed, Dec 03, 2025 at 11:24:20AM +0800, Jie Zhan wrote:
>> Factor out cppc_perf_ctrs_in_pcc_cpu() for checking whether per-cpu CPC
>> regs are defined in PCC channels, and export it out for further use.
>>
>> Signed-off-by: Jie Zhan <zhanjie9@hisilicon.com>
>> ---
>>  drivers/acpi/cppc_acpi.c | 45 +++++++++++++++++++++-------------------
>>  include/acpi/cppc_acpi.h |  5 +++++
>>  2 files changed, 29 insertions(+), 21 deletions(-)
>>
>> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
>> index 3bdeeee3414e..aa80dbcf42c0 100644
>> --- a/drivers/acpi/cppc_acpi.c
>> +++ b/drivers/acpi/cppc_acpi.c
>> @@ -1422,6 +1422,29 @@ int cppc_get_perf_caps(int cpunum, struct cppc_perf_caps *perf_caps)
>>  }
>>  EXPORT_SYMBOL_GPL(cppc_get_perf_caps);
>>  
>> +bool cppc_perf_ctrs_in_pcc_cpu(unsigned int cpu)
>> +{
>> +	struct cpc_desc *cpc_desc = per_cpu(cpc_desc_ptr, cpu);
>> +	struct cpc_register_resource *ref_perf_reg;
>> +
>> +	/*
>> +	 * If reference perf register is not supported then we should use the
>> +	 * nominal perf value
>> +	 */
>> +	ref_perf_reg = &cpc_desc->cpc_regs[REFERENCE_PERF];
>> +	if (!CPC_SUPPORTED(ref_perf_reg))
>> +		ref_perf_reg = &cpc_desc->cpc_regs[NOMINAL_PERF];
>> +
>> +	if (CPC_IN_PCC(&cpc_desc->cpc_regs[DELIVERED_CTR]) ||
>> +	    CPC_IN_PCC(&cpc_desc->cpc_regs[REFERENCE_CTR]) ||
>> +	    CPC_IN_PCC(&cpc_desc->cpc_regs[CTR_WRAP_TIME]) ||
>> +	    CPC_IN_PCC(ref_perf_reg))
>> +		return true;
>> +
>> +	return false;
>> +}
>> +EXPORT_SYMBOL_GPL(cppc_perf_ctrs_in_pcc_cpu);
> It is minor, but I would prefer the earlier version when we grab the reference
> performance reg only when none of the other regs is in the PCC.
Why?

It could return a little bit earlier for the PCC reg case, but this is only
called in initialization so efficiency is not that important I guess.
> 
> ---
> BR
> Beata

...