cpufreq: cppc: Add support for frequency invariance

This patch attempts to make it generic enough so other parts of the kernel can also provide their own implementation of scale_freq_tick() callback, which is called by the scheduler periodically to update the per-cpu freq_scale variable.

static void amu_fie_setup(const struct cpumask *cpus) {

arch/arm64/kernel/topology.c +1 -1

@@ -227,7 +263,7 @@ static void amu_fie_setup(const struct cpumask *cpus)
 	if (!invariant && !cpumask_equal(amu_fie_cpus, cpu_present_mask))
 		return;
 
-	static_branch_enable(&amu_fie_key);
+	topology_set_scale_freq_source(&amu_sfd, amu_fie_cpus);
 
 	pr_debug("CPUs[%*pbl]: counters will be used for FIE.",
 		 cpumask_pr_args(cpus));

@@ -283,53 +319,6 @@ static int __init init_amu_fie(void)
 }
 core_initcall(init_amu_fie);

drivers/base/arch_topology.c +7 -0

+void topology_set_scale_freq_source(struct scale_freq_data *data,
+				    const struct cpumask *cpus)
+{
+	struct scale_freq_data *sfd;
+	int cpu;
+
+	for_each_cpu(cpu, cpus) {

Therefore, I think system level invariance management (checks and call to rebuild_sched_domains_energy()) also needs to move from arm64 code to arch_topology code.

Quoted 3 lines · Ionela Voinescu

Therefore, I think system level invariance management (checks and call to rebuild_sched_domains_energy()) also needs to move from arm64 code to arch_topology code.

Here is the 3rd patch of this series then :)

From: Viresh Kumar <[email protected]>
Date: Fri, 5 Feb 2021 13:31:53 +0530
Subject: [PATCH] drivers: arch_topology: rebuild sched domains on invariance
change

We already do this for the arm64, move it to arch_topology.c as we manage all sched_freq_tick sources here now.

Reported-by Ionela Voinescu <[email protected]>

Signed-off-by Viresh Kumar <[email protected]>

---
 arch/arm64/kernel/topology.c | 16 ----------------
 drivers/base/arch_topology.c | 22 ++++++++++++++++++++++
 2 files changed, 22 insertions(+), 16 deletions(-)

arch/arm64/kernel/topology.c +0 -1

index 1e47dfd465f8..47fca7376c93 100644
@@ -240,7 +240,6 @@ static struct scale_freq_data amu_sfd = {
 
 static void amu_fie_setup(const struct cpumask *cpus)
 {
-	bool invariant;
 	int cpu;
 
 	/* We are already set since the last insmod of cpufreq driver */

arch/arm64/kernel/topology.c +0 -9

 	topology_set_scale_freq_source(&amu_sfd, amu_fie_cpus);
 
 	pr_debug("CPUs[%*pbl]: counters will be used for FIE.",
 		 cpumask_pr_args(cpus));
-
-	/*
-	 * Task scheduler behavior depends on frequency invariance support,
-	 * either cpufreq or counter driven. If the support status changes as
-	 * a result of counter initialisation and use, retrigger the build of
-	 * scheduling domains to ensure the information is propagated properly.
-	 */
-	if (!invariant)
-		rebuild_sched_domains_energy();
 }
 
 static int init_amu_fie_callback(struct notifier_block *nb, unsigned long val,

drivers/base/arch_topology.c +18 -0

index 20b511949cd8..3631877f4440 100644
@@ -23,6 +23,7 @@
 
 static DEFINE_PER_CPU(struct scale_freq_data *, sft_data);
 static struct cpumask scale_freq_counters_mask;
+static bool scale_freq_invariant;
 
 static bool supports_scale_freq_counters(const struct cpumask *cpus)
 {

@@ -35,6 +36,23 @@ bool topology_scale_freq_invariant(void)
 	       supports_scale_freq_counters(cpu_online_mask);
 }
 
+static void update_scale_freq_invariant(bool status)
+{
+	if (scale_freq_invariant == status)
+		return;
+
+	/*
+	 * Task scheduler behavior depends on frequency invariance support,
+	 * either cpufreq or counter driven. If the support status changes as
+	 * a result of counter initialisation and use, retrigger the build of
+	 * scheduling domains to ensure the information is propagated properly.
+	 */
+	if (topology_scale_freq_invariant() == status) {
+		scale_freq_invariant = status;
+		rebuild_sched_domains_energy();
+	}
+}
+
 void topology_set_scale_freq_source(struct scale_freq_data *data,
 				    const struct cpumask *cpus)
 {

drivers/base/arch_topology.c +4 -0

+
+	update_scale_freq_invariant(true);
 }
 EXPORT_SYMBOL_GPL(topology_set_scale_freq_source);
 

@@ -67,6 +87,8 @@ void topology_clear_scale_freq_source(enum scale_freq_source source,
 			cpumask_clear_cpu(cpu, &scale_freq_counters_mask);
 		}
 	}
+
+	update_scale_freq_invariant(false);
 }
 EXPORT_SYMBOL_GPL(topology_clear_scale_freq_source);
 

Signature · 4 lines

2.25.0.rc1.19.g042ed3e048af

-- 
viresh

I think it could be merged in patch 1/2 as it's part of enabling the use of multiple sources of information for FIE. Up to you!

Quoted 10 lines

static void amu_fie_setup(const struct cpumask *cpus) {

arch/arm64/kernel/topology.c +0 -1

-	bool invariant;
 	int cpu;
 
 	/* We are already set since the last insmod of cpufreq driver */

@@ -257,25 +256,10 @@ static void amu_fie_setup(const struct cpumask *cpus)
 
 	cpumask_or(amu_fie_cpus, amu_fie_cpus, cpus);
 

arch/arm64/kernel/topology.c static void amu_fie_setup(const struct cpumask *cpus) · line 265

Quoted 6 lines

arch/arm64/kernel/topology.c +0 -6

-	invariant = topology_scale_freq_invariant();
-
-	/* We aren't fully invariant yet */
-	if (!invariant && !cpumask_equal(amu_fie_cpus, cpu_present_mask))
-		return;
-

You still need these checks, otherwise you could end up with only part of the CPUs setting a scale factor, when only part of the CPUs support AMUs and there is no cpufreq support for FIE.

Small(ish) optimisation at the beginning of this function:

    if (cpumask_empty(&scale_freq_counters_mask))
        scale_freq_invariant = topology_scale_freq_invariant();

This will save you a call to rebuild_sched_domains_energy(), which is quite expensive, when cpufreq supports FIE and we also have counters.

After comments addressed,

Reviewed-by Ionela Voinescu <[email protected]>

Tested-by Ionela Voinescu <[email protected]>

Just out of curiosity, what exactly did you test and what was the setup ? :)

Quoted 2 lines · Ionela Voinescu

I think it could be merged in patch 1/2 as it's part of enabling the use of multiple sources of information for FIE. Up to you!

Sure.

Quoted 10 lines

Quoted 10 lines

static void amu_fie_setup(const struct cpumask *cpus) {

arch/arm64/kernel/topology.c +0 -1

-	bool invariant;
 	int cpu;
 
 	/* We are already set since the last insmod of cpufreq driver */

@@ -257,25 +256,10 @@ static void amu_fie_setup(const struct cpumask *cpus)
 
 	cpumask_or(amu_fie_cpus, amu_fie_cpus, cpus);
 

Quoted 10 lines

arch/arm64/kernel/topology.c static void amu_fie_setup(const struct cpumask *cpus) · line 265

Quoted 6 lines

arch/arm64/kernel/topology.c +0 -6

-	invariant = topology_scale_freq_invariant();
-
-	/* We aren't fully invariant yet */
-	if (!invariant && !cpumask_equal(amu_fie_cpus, cpu_present_mask))
-		return;
-

You still need these checks, otherwise you could end up with only part of the CPUs setting a scale factor, when only part of the CPUs support AMUs and there is no cpufreq support for FIE.

Both supports_scale_freq_counters() and topology_scale_freq_invariant() take care of this now and they will keep reporting the system as invariant until the time all the CPUs have counters (in absence of cpufreq).

The topology_set_scale_freq_source() API is supposed to be called multiple times, probably once for each policy and so I don't see a need of these checks anymore.

Quoted 7 lines

Small(ish) optimisation at the beginning of this function:

    if (cpumask_empty(&scale_freq_counters_mask))
        scale_freq_invariant = topology_scale_freq_invariant();

This will save you a call to rebuild_sched_domains_energy(), which is quite expensive, when cpufreq supports FIE and we also have counters.

Good Point.

Quoted 3 lines

After comments addressed,

Reviewed-by Ionela Voinescu <[email protected]>

Thanks.

Quoted 1 line

Tested-by Ionela Voinescu <[email protected]>

Signature · 1 line

viresh

The problem is not topology_scale_freq_invariant() but whether a scale factor is set for some CPUs.

Scenario (test system above):
- "AMUs" are only supported for [1-2],
- cpufreq_supports_freq_invariance() -> false

What should happen:
- topology_scale_freq_invariant() -> false (passed)
- all CPUs should have their freq_scale unmodified (1024) - (failed) because only 2 out of 6 CPUs have a method of setting a scale factor

What does happen:
- arch_set_freq_tick() -> topology_set_freq_tick() will set a scale factor for [1-2] based on AMUs. This should not happen. We will end up with invariant signals for bigs and signals that are not freq invariant for littles.

Quoted 12 lines · Ionela Voinescu

The problem is not topology_scale_freq_invariant() but whether a scale factor is set for some CPUs.

Scenario (test system above):
- "AMUs" are only supported for [1-2],
- cpufreq_supports_freq_invariance() -> false

What should happen:
- topology_scale_freq_invariant() -> false (passed)
- all CPUs should have their freq_scale unmodified (1024) - (failed) because only 2 out of 6 CPUs have a method of setting a scale factor

Quoted 5 lines

What does happen:
- arch_set_freq_tick() -> topology_set_freq_tick() will set a scale factor for [1-2] based on AMUs. This should not happen. We will end up with invariant signals for bigs and signals that are not freq invariant for littles.

Another case. cpufreq is included as a module and AMU is implemented partially.

- first time cpufreq driver is inserted, we set up everything and freq_scale gets updated on ticks.

- remove cpufreq driver, we are back in same situation.

We can't control it that way.. Or we add another call layer in middle before the tick-handler gets called for AMU, which will check if we are fully invariant or not ?

Signature · 1 line

viresh

See a very useful comment someone added recently :) :

"""

(quoted diff) +9 -0

+	/*
+	 * We don't need to handle CPUFREQ_REMOVE_POLICY event as the AMU
+	 * counters don't have any dependency on cpufreq driver once we have
+	 * initialized AMU support and enabled invariance. The AMU counters will
+	 * keep on working just fine in the absence of the cpufreq driver, and
+	 * for the CPUs for which there are no counters available, the last set
+	 * value of freq_scale will remain valid as that is the frequency those
+	 * CPUs are running at.
+	 */

"""

Quoted 16 lines

arch/arm64/kernel/topology.c +0 -1

index 1e47dfd465f8..47fca7376c93 100644
@@ -240,7 +240,6 @@ static struct scale_freq_data amu_sfd = {
 
 static void amu_fie_setup(const struct cpumask *cpus)
 {
-	bool invariant;
 	int cpu;
 
 	/* We are already set since the last insmod of cpufreq driver */

@@ -257,25 +256,10 @@ static void amu_fie_setup(const struct cpumask *cpus)
 
 	cpumask_or(amu_fie_cpus, amu_fie_cpus, cpus);
 

arch/arm64/kernel/topology.c static void amu_fie_setup(const struct cpumask *cpus) · line 265

Quoted 6 lines

arch/arm64/kernel/topology.c +0 -6

-	invariant = topology_scale_freq_invariant();
-
-	/* We aren't fully invariant yet */
-	if (!invariant && !cpumask_equal(amu_fie_cpus, cpu_present_mask))
-		return;
-

You still need these checks, otherwise you could end up with only part of the CPUs setting a scale factor, when only part of the CPUs support AMUs and there is no cpufreq support for FIE.

Quoted 16 lines · Ionela Voinescu

Quoted 16 lines

arch/arm64/kernel/topology.c +0 -1

index 1e47dfd465f8..47fca7376c93 100644
@@ -240,7 +240,6 @@ static struct scale_freq_data amu_sfd = {
 
 static void amu_fie_setup(const struct cpumask *cpus)
 {
-	bool invariant;
 	int cpu;
 
 	/* We are already set since the last insmod of cpufreq driver */

@@ -257,25 +256,10 @@ static void amu_fie_setup(const struct cpumask *cpus)
 
 	cpumask_or(amu_fie_cpus, amu_fie_cpus, cpus);
 

Quoted 10 lines

arch/arm64/kernel/topology.c static void amu_fie_setup(const struct cpumask *cpus) · line 265

Quoted 6 lines

arch/arm64/kernel/topology.c +0 -6

-	invariant = topology_scale_freq_invariant();
-
-	/* We aren't fully invariant yet */
-	if (!invariant && !cpumask_equal(amu_fie_cpus, cpu_present_mask))
-		return;
-

You still need these checks, otherwise you could end up with only part of the CPUs setting a scale factor, when only part of the CPUs support AMUs and there is no cpufreq support for FIE.

Another look at it and here goes another reason (hope I don't have another in-code comment somewhere else to kill this one) :)

We don't need to care for the reason you gave (which is a valid reason otherwise), as we are talking specifically about amu_fie_setup() here and it gets called from cpufreq policy-notifier. i.e. we won't support AMUs without cpufreq being there in the first place and the same goes for cppc-driver.

Does that sound reasonable ?

Signature · 1 line

viresh

Yes, we don't care if there is no cpufreq driver, as the use of AMUs won't get initialised either. But we do care if there is a cpufreq driver that does not support frequency invariance, which is the example above.

The intention with the patches that made cpufreq based invariance generic a while back was for it to be present, seamlessly, for as many drivers as possible, as a less than accurate invariance default method is still better than nothing.

So only a few drivers today don't support cpufreq based FI

but it's not a guarantee that it will stay this way.

Quoted 8 lines · Ionela Voinescu

Yes, we don't care if there is no cpufreq driver, as the use of AMUs won't get initialised either. But we do care if there is a cpufreq driver that does not support frequency invariance, which is the example above.

The intention with the patches that made cpufreq based invariance generic a while back was for it to be present, seamlessly, for as many drivers as possible, as a less than accurate invariance default method is still better than nothing.

Right.

Quoted 1 line

So only a few drivers today don't support cpufreq based FI

Only two AFAICT, both x86, and the AMU stuff doesn't conflict with them.

drivers/cpufreq/intel_pstate.c
drivers/cpufreq/longrun.c

Quoted 1 line

but it's not a guarantee that it will stay this way.

What do you mean by "no guarantee" here ?

The very core routines (cpufreq_freq_transition_end() and cpufreq_driver_fast_switch()) of the cpufreq core call arch_set_freq_scale() today and this isn't going to change anytime soon. If something gets changed there someone will need to see other parts of the kernel which may get broken with that.

I don't see any need of complicating other parts of the kernel like, amu or cppc code for that. They should be kept simple and they should assume cpufreq invariance will be supported as it is today.

Signature · 1 line

viresh

Quoted 6 lines · Viresh Kumar

The very core routines (cpufreq_freq_transition_end() and cpufreq_driver_fast_switch()) of the cpufreq core call arch_set_freq_scale() today and this isn't going to change anytime soon. If something gets changed there someone will need to see other parts of the kernel which may get broken with that.

Yes, but it won't really be straightforward to notice this breakage if that happens, so in my opinion it was worth to keep that condition.

The Frequency Invariance Engine (FIE) is providing a frequency scaling correction factor that helps achieve more accurate load-tracking.

Normally, this scaling factor can be obtained directly with the help of the cpufreq drivers as they know the exact frequency the hardware is running at. But that isn't the case for CPPC cpufreq driver.

Another way of obtaining that is using the arch specific counter support, which is already present in kernel, but that hardware is optional for platforms.

This patch thus obtains this scaling factor using the existing logic present in the cppc driver. 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.

This also exports sched_setattr_nocheck() as the CPPC driver can be built as a module.

Signed-off-by Viresh Kumar <[email protected]>

---
 drivers/cpufreq/cppc_cpufreq.c | 179 ++++++++++++++++++++++++++++++---
 include/linux/arch_topology.h  |   1 +
 kernel/sched/core.c            |   1 +
 3 files changed, 169 insertions(+), 12 deletions(-)

drivers/cpufreq/cppc_cpufreq.c +4 -0

index 8a482c434ea6..53815f6d2797 100644
@@ -10,14 +10,18 @@
 
 #define pr_fmt(fmt)	"CPPC Cpufreq:"	fmt
 
+#include <linux/arch_topology.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/delay.h>
 #include <linux/cpu.h>
 #include <linux/cpufreq.h>
 #include <linux/dmi.h>
+#include <linux/irq_work.h>
+#include <linux/kthread.h>
 #include <linux/time.h>
 #include <linux/vmalloc.h>
+#include <uapi/linux/sched/types.h>
 
 #include <asm/unaligned.h>
 

drivers/cpufreq/cppc_cpufreq.c +10 -5

 struct cppc_workaround_oem_info {
 	char oem_id[ACPI_OEM_ID_SIZE + 1];
 	char oem_table_id[ACPI_OEM_TABLE_ID_SIZE + 1];

@@ -292,7 +307,7 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy)
 	unsigned int cpu = policy->cpu;
 	struct cppc_cpudata *cpu_data;
 	struct cppc_perf_caps *caps;
-	int ret;
+	int ret, i;
 
 	cpu_data = cppc_cpufreq_get_cpu_data(cpu);
 	if (!cpu_data) {

@@ -343,6 +358,11 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy)
 		return -EFAULT;
 	}
 
+	for_each_cpu(i, policy->cpus) {
+		per_cpu(cppc_f_i, i).max_freq = policy->cpuinfo.max_freq;
+		per_cpu(cppc_f_i, i).cpu_data = cpu_data;
+	}
+
 	/*
 	 * If 'highest_perf' is greater than 'nominal_perf', we assume CPU Boost
 	 * is supported.

@@ -370,12 +390,12 @@ static inline u64 get_delta(u64 t1, u64 t0)
 	return (u32)t1 - (u32)t0;
 }
 
-static int cppc_get_rate_from_fbctrs(struct cppc_cpudata *cpu_data,
-				     struct cppc_perf_fb_ctrs fb_ctrs_t0,
-				     struct cppc_perf_fb_ctrs fb_ctrs_t1)
+static int cppc_perf_from_fbctrs(struct cppc_cpudata *cpu_data,
+				 struct cppc_perf_fb_ctrs fb_ctrs_t0,
+				 struct cppc_perf_fb_ctrs fb_ctrs_t1)
 {
 	u64 delta_reference, delta_delivered;
-	u64 reference_perf, delivered_perf;
+	u64 reference_perf;
 
 	reference_perf = fb_ctrs_t0.reference_perf;
 

drivers/cpufreq/cppc_cpufreq.c +27 -1

+	return (reference_perf * delta_delivered) / delta_reference;
+}
+
+static int cppc_get_rate_from_fbctrs(struct cppc_cpudata *cpu_data,
+				     struct cppc_perf_fb_ctrs fb_ctrs_t0,
+				     struct cppc_perf_fb_ctrs fb_ctrs_t1)
+{
+	u64 delivered_perf;
+
+	delivered_perf = cppc_perf_from_fbctrs(cpu_data, fb_ctrs_t0,
+					       fb_ctrs_t1);
 
 	return cppc_cpufreq_perf_to_khz(cpu_data, delivered_perf);
 }

@@ -420,7 +449,7 @@ static int cppc_cpufreq_set_boost(struct cpufreq_policy *policy, int state)
 {
 	struct cppc_cpudata *cpu_data = policy->driver_data;
 	struct cppc_perf_caps *caps = &cpu_data->perf_caps;
-	int ret;
+	int ret, i;
 
 	if (!boost_supported) {
 		pr_err("BOOST not supported by CPU or firmware\n");

@@ -435,6 +464,9 @@ static int cppc_cpufreq_set_boost(struct cpufreq_policy *policy, int state)
 						       caps->nominal_perf);
 	policy->cpuinfo.max_freq = policy->max;
 
+	for_each_cpu(i, policy->related_cpus)
+		per_cpu(cppc_f_i, i).max_freq = policy->cpuinfo.max_freq;
+
 	ret = freq_qos_update_request(policy->max_freq_req, policy->max);
 	if (ret < 0)
 		return ret;

@@ -512,8 +544,126 @@ static void cppc_check_hisi_workaround(void)
 	acpi_put_table(tbl);
 }
 
+static void cppc_scale_freq_workfn(struct kthread_work *work)
+{
+	struct cppc_freq_invariance *cppc_fi;
+	struct cppc_perf_fb_ctrs fb_ctrs = {0};
+	int cpu = raw_smp_processor_id();
+	struct cppc_cpudata *cpu_data;
+	u64 perf;
+
+	cppc_fi = container_of(work, struct cppc_freq_invariance, work);
+	cpu_data = cppc_fi->cpu_data;
+
+	if (cppc_get_perf_ctrs(cpu, &fb_ctrs)) {

drivers/cpufreq/cppc_cpufreq.c +4 -0

+}
+
+static void cppc_irq_work(struct irq_work *irq_work)
+{

drivers/cpufreq/cppc_cpufreq.c +51 -0

+	/*
+	 * cppc_get_perf_ctrs() can potentially sleep, call that from the right
+	 * context.
+	 */
+	irq_work_queue(&cppc_fi->irq_work);
+}
+
+static struct scale_freq_data cppc_sftd = {
+	.source = SCALE_FREQ_SOURCE_CPPC,
+	.set_freq_scale = cppc_scale_freq_tick,
+};
+
+static void cppc_freq_invariance_exit(void)
+{
+	struct cppc_freq_invariance *cppc_fi;
+	int i;
+
+	if (cppc_cpufreq_driver.get == hisi_cppc_cpufreq_get_rate)
+		return;
+
+	topology_clear_scale_freq_source(SCALE_FREQ_SOURCE_CPPC, cpu_present_mask);
+
+	for_each_possible_cpu(i) {
+		cppc_fi = &per_cpu(cppc_f_i, i);
+		if (cppc_fi->worker) {
+			irq_work_sync(&cppc_fi->irq_work);
+			kthread_destroy_worker(cppc_fi->worker);
+			cppc_fi->worker = NULL;
+		}
+	}
+}
+
+static void __init cppc_freq_invariance_init(void)
+{
+	struct cppc_perf_fb_ctrs fb_ctrs = {0};
+	struct cppc_freq_invariance *cppc_fi;
+	struct sched_attr attr = {
+		.size		= sizeof(struct sched_attr),
+		.sched_policy	= SCHED_DEADLINE,
+		.sched_nice	= 0,
+		.sched_priority	= 0,
+		/*
+		 * Fake (unused) bandwidth; workaround to "fix"
+		 * priority inheritance.
+		 */
+		.sched_runtime	= 1000000,
+		.sched_deadline = 10000000,
+		.sched_period	= 10000000,
+	};
+	struct kthread_worker *worker;
+	int i, ret;

drivers/cpufreq/cppc_cpufreq.c +8 -0

+	}
+
+	/* Register for freq-invariance */
+	topology_set_scale_freq_source(&cppc_sftd, cpu_present_mask);
+}
+
 static int __init cppc_cpufreq_init(void)
 {
+	int ret;
+
 	if ((acpi_disabled) || !acpi_cpc_valid())
 		return -ENODEV;
 

drivers/cpufreq/cppc_cpufreq.c +3 -0

+
+	return ret;
 }
 
 static inline void free_cpu_data(void)

@@ -538,6 +692,7 @@ static inline void free_cpu_data(void)
 
 static void __exit cppc_cpufreq_exit(void)
 {
+	cppc_freq_invariance_exit();
 	cpufreq_unregister_driver(&cppc_cpufreq_driver);
 
 	free_cpu_data();

include/linux/arch_topology.h +1 -0

index 3bcfba5c21a7..47ac4b41c28d 100644
@@ -37,6 +37,7 @@ bool topology_scale_freq_invariant(void);
 enum scale_freq_source {
 	SCALE_FREQ_SOURCE_CPUFREQ = 0,
 	SCALE_FREQ_SOURCE_ARCH,
+	SCALE_FREQ_SOURCE_CPPC,
 };
 
 struct scale_freq_data {

kernel/sched/core.c +1 -0

index 8c5481077c9c..85d1d23951ba 100644
@@ -6190,6 +6190,7 @@ int sched_setattr_nocheck(struct task_struct *p, const struct sched_attr *attr)
 {
 	return __sched_setscheduler(p, attr, false, true);
 }
+EXPORT_SYMBOL_GPL(sched_setattr_nocheck);
 
 /**
  * sched_setscheduler_nocheck - change the scheduling policy and/or RT priority of a thread from kernelspace.

Signature · 1 line

2.25.0.rc1.19.g042ed3e048af

Quoted 2 lines

drivers/cpufreq/cppc_cpufreq.c +2 -0

+static void __init cppc_freq_invariance_init(void)
+{

drivers/cpufreq/cppc_cpufreq.c static void cppc_check_hisi_workaround(void) · line 656

Quoted 4 lines

drivers/cpufreq/cppc_cpufreq.c +4 -0

+
+		ret = cppc_get_perf_ctrs(i, &fb_ctrs);
+		if (!ret)
+			per_cpu(cppc_fi->prev_perf_fb_ctrs, i) = fb_ctrs;

Hey,

Some test results:

[..]

Quoted 2 lines

Quoted 2 lines

drivers/cpufreq/cppc_cpufreq.c +2 -0

+static void __init cppc_freq_invariance_init(void)
+{

[..]

Quoted 5 lines

drivers/cpufreq/cppc_cpufreq.c static void cppc_check_hisi_workaround(void) · line 656

Quoted 4 lines

drivers/cpufreq/cppc_cpufreq.c +4 -0

+
+		ret = cppc_get_perf_ctrs(i, &fb_ctrs);
+		if (!ret)
+			per_cpu(cppc_fi->prev_perf_fb_ctrs, i) = fb_ctrs;

After fixing this one:

			cppc_fi->prev_perf_fb_ctrs = fb_ctrs;

I got the following:

Platform:

- Juno R2 (CPUs [0-3] are littles, CPUs [4-5] are bigs)
+ PMU counters, used by CPPC through FFH
+ userspace/schedutil

- Verifying that with userspace governor we see a correct change in

    scale factor:

	root@buildroot:~# dmesg | grep FIE
	[    6.436770] AMU: CPUs[0-3]: AMU counters WON'T be used for FIE.
	[    6.436962] AMU: CPUs[4-5]: AMU counters WON'T be used for FIE.
	[    6.451510] CPPC:CPUs[0-5]: CPPC counters will be used for FIE.

	root@buildroot:~# echo 600000 > policy4/scaling_setspeed
	[  353.939495] CPU4: Invariance(cppc) scale: 512.
	[  353.939497] CPU5: Invariance(cppc) scale: 512.

	root@buildroot:~# echo 1200000 > policy4/scaling_setspeed
	[  372.683511] CPU5: Invariance(cppc) scale: 1024.
	[  372.683518] CPU4: Invariance(cppc) scale: 1024.

	root@buildroot:~# echo 450000 > policy0/scaling_setspeed
	[  641.495513] CPU2: Invariance(cppc) scale: 485.
	[  641.495514] CPU1: Invariance(cppc) scale: 485.

	[  641.495517] CPU0: Invariance(cppc) scale: 485.
	[  641.495542] CPU3: Invariance(cppc) scale: 485.

	root@buildroot:~# echo 950000 > policy0/scaling_setspeed
	[  852.015514] CPU2: Invariance(cppc) scale: 1024.
	[  852.015514] CPU1: Invariance(cppc) scale: 1024.
	[  852.015517] CPU0: Invariance(cppc) scale: 1024.
	[  852.015541] CPU3: Invariance(cppc) scale: 1024.

- I ran some benchmarks as well (perf, hackbench, dhrystone) on the same platform, using the userspace governor at fixed frequency, to evaluate the impact of the work we do or don't do on the tick.

./perf bench sched pipe
(10 iterations, higher is better, ops/s, comparisons with cpufreq-based FIE)

   cpufreq-based FIE    AMU-based FIE    CPPC-based FIE
   ----------------------------------------------------
   39498.8		40984.7		 38893.4
   std: 3.766%		std: 4.461%	 std: 0.575%
   			diff: 3.625%	 diff: -1.556%

   ./hackbench -l 1000
   (10 iterations, lower is better, seconds, comparison with
   cpufreq-based FIE)

   cpufreq-based FIE    AMU-based FIE    CPPC-based FIE
   ----------------------------------------------------
   6.4207		6.3386		 6.7841
   std: 7.298%		std: 2.252%	 std: 2.460%
   			diff: -1.295%	 diff: 5.356%

   This shows a small regression for the CPPC-based FIE, but within the

   standard deviation.

   I ran some dhrystone benchmarks (./dhrystone -t 2/34/5/6/ -l 5000) as
   well with schedutil governor to understand if an increase in accuracy
   with the AMU/CPPC counters makes a difference. Given the
   characteristics of the platform it's no surprise that the results
   were very similar between the three cases, so I won't bore you with
   the numbers.