cpufreq: intel_pstate: Enable EAS on hybrid platforms without SMT

From: Rafael J. Wysocki <[email protected]>

In preparation for subsequent changes, move the em_compute_costs() invocation from em_create_perf_table() to em_create_pd() which is its only caller.

From: Rafael J. Wysocki <[email protected]>

Allow em_dev_register_perf_domain() to register a cost-only stub perf domain with one-element states table if the .active_power() callback is not provided.

Subsequently, this will be used by the intel_pstate driver to register stub perf domains for CPUs on hybrid systems.

From: Rafael J. Wysocki <[email protected]>

Allow em_dev_register_perf_domain() to register a cost-only stub perf domain with one-element states table if the .active_power() callback is not provided.

Subsequently, this will be used by the intel_pstate driver to register stub perf domains for CPUs on hybrid systems.

No intentional functional impact.

Signed-off-by Rafael J. Wysocki <[email protected]>

---
  kernel/power/energy_model.c |   26 +++++++++++++++++++++++---
  1 file changed, 23 insertions(+), 3 deletions(-)

Index: linux-pm/kernel/power/energy_model.c
===================================================================

kernel/power/energy_model.c +6 -3

@@ -426,9 +426,11 @@ static int em_create_pd(struct device *d
  	if (!em_table)
  		goto free_pd;
 
-	ret = em_create_perf_table(dev, pd, em_table->state, cb, flags);
-	if (ret)
-		goto free_pd_table;
+	if (cb->active_power) {
+		ret = em_create_perf_table(dev, pd, em_table->state, cb, flags);
+		if (ret)
+			goto free_pd_table;
+	}
 
  	ret = em_compute_costs(dev, em_table->state, cb, nr_states, flags);
  	if (ret)

@@ -561,11 +563,20 @@ int em_dev_register_perf_domain(struct d
  {
  	unsigned long cap, prev_cap = 0;
  	unsigned long flags = 0;
+	bool stub_pd = false;
  	int cpu, ret;
 
  	if (!dev || !nr_states || !cb)

kernel/power/energy_model.c +9 -0

  	/*
  	 * Use a mutex to serialize the registration of performance domains and
  	 * let the driver-defined callback functions sleep.

@@ -590,6 +601,15 @@ int em_dev_register_perf_domain(struct d
  				ret = -EEXIST;
  				goto unlock;
  			}
+
+			/*
+			 * The capacity need not be the same for all CPUs in a
+			 * stub perf domain, so long as the average cost of
+			 * running on each of them is approximately the same.
+			 */
+			if (stub_pd)
+				continue;
+
  			/*
  			 * All CPUs of a domain must have the same
  			 * micro-architecture since they all share the same

Quoted 45 lines · Rafael J. Wysocki

From: Rafael J. Wysocki <[email protected]>

Allow em_dev_register_perf_domain() to register a cost-only stub perf domain with one-element states table if the .active_power() callback is not provided.

Subsequently, this will be used by the intel_pstate driver to register stub perf domains for CPUs on hybrid systems.

No intentional functional impact.

Signed-off-by Rafael J. Wysocki <[email protected]>

---
  kernel/power/energy_model.c |   26 +++++++++++++++++++++++---
  1 file changed, 23 insertions(+), 3 deletions(-)

Index: linux-pm/kernel/power/energy_model.c
===================================================================

kernel/power/energy_model.c +6 -3

@@ -426,9 +426,11 @@ static int em_create_pd(struct device *d
      if (!em_table)
              goto free_pd;
 
-     ret = em_create_perf_table(dev, pd, em_table->state, cb, flags);
-     if (ret)
-             goto free_pd_table;
+     if (cb->active_power) {
+             ret = em_create_perf_table(dev, pd, em_table->state, cb, flags);
+             if (ret)
+                     goto free_pd_table;
+     }
 
      ret = em_compute_costs(dev, em_table->state, cb, nr_states, flags);
      if (ret)

@@ -561,11 +563,20 @@ int em_dev_register_perf_domain(struct d
  {
      unsigned long cap, prev_cap = 0;
      unsigned long flags = 0;
+     bool stub_pd = false;
      int cpu, ret;
 
      if (!dev || !nr_states || !cb)
              return -EINVAL;
 

kernel/power/energy_model.c int em_dev_register_perf_domain(struct d · line 579

Quoted 8 lines

kernel/power/energy_model.c +8 -0

+     if (!cb->active_power) {
+             if (!cb->get_cost || nr_states > 1 || microwatts)
+                     return -EINVAL;
+
+             /* Special case: a stub perf domain. */
+             stub_pd = true;
+     }
+

I wonder if the only purpose of stub_pd is to just skip the capacity check below, which doesn't look very nice.

I may be echoing Dietmar's comments here. What's the problem of just having 3 domains?

Or, could you just specify the same capacities so that the same-capacity check won't fail, but just to use hardware load or CPU pressure to model the slight difference in real capacities? This way you'd re-use a lot of existing infrastructure.

Quoted 22 lines

kernel/power/energy_model.c +9 -0

      /*
       * Use a mutex to serialize the registration of performance domains and
       * let the driver-defined callback functions sleep.

@@ -590,6 +601,15 @@ int em_dev_register_perf_domain(struct d
                              ret = -EEXIST;
                              goto unlock;
                      }
+
+                     /*
+                      * The capacity need not be the same for all CPUs in a
+                      * stub perf domain, so long as the average cost of
+                      * running on each of them is approximately the same.
+                      */
+                     if (stub_pd)
+                             continue;
+
                      /*
                       * All CPUs of a domain must have the same
                       * micro-architecture since they all share the same

From: Rafael J. Wysocki <[email protected]>

Some cpufreq drivers, like intel_pstate, have built-in governors that are used instead of regular cpufreq governors, schedutil in particular, but they can work with EAS just fine, so allow EAS to be used with those drivers.

Also update the debug message printed when the cpufreq governor in use is not schedutil and the related comment, to better match the code after the change.

Signed-off-by Rafael J. Wysocki <[email protected]>

---

I'm not sure how much value there is in refusing to enable EAS without schedutil in general. For instance, if there are no crossover points between the cost curves for different perf domains, EAS may as well be used with the performance and powersave governors AFAICS.

---
 kernel/sched/topology.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Index: linux-pm/kernel/sched/topology.c
===================================================================

kernel/sched/topology.c +3 -3

@@ -251,7 +251,7 @@ static bool sched_is_eas_possible(const
 		return false;
 	}
 
-	/* Do not attempt EAS if schedutil is not being used. */
+	/* Do not attempt EAS with a cpufreq governor other than schedutil. */
 	for_each_cpu(i, cpu_mask) {
 		policy = cpufreq_cpu_get(i);
 		if (!policy) {

@@ -263,9 +263,9 @@ static bool sched_is_eas_possible(const
 		}
 		gov = policy->governor;
 		cpufreq_cpu_put(policy);
-		if (gov != &schedutil_gov) {
+		if (gov && gov != &schedutil_gov) {
 			if (sched_debug()) {
-				pr_info("rd %*pbl: Checking EAS, schedutil is mandatory\n",
+				pr_info("rd %*pbl: Checking EAS, cpufreq governor is not schedutil\n",
 					cpumask_pr_args(cpu_mask));
 			}
 			return false;

Quoted 14 lines · Rafael J. Wysocki

From: Rafael J. Wysocki <[email protected]>

Some cpufreq drivers, like intel_pstate, have built-in governors that are used instead of regular cpufreq governors, schedutil in particular, but they can work with EAS just fine, so allow EAS to be used with those drivers.

Also update the debug message printed when the cpufreq governor in use is not schedutil and the related comment, to better match the code after the change.

Signed-off-by Rafael J. Wysocki <[email protected]>

---

Quoted 4 lines

I'm not sure how much value there is in refusing to enable EAS without schedutil in general. For instance, if there are no crossover points between the cost curves for different perf domains, EAS may as well be used with the performance and powersave governors AFAICS.

Agreed, but having no cross-over points or no DVFS at all should be the only instances, right?

For plain (non-intel_pstate) powersave and performance we could replace sugov_effective_cpu_perf()
that determines the OPP of the perf-domain by the OPP they will be choosing, but for the rest?

Also there is the entire uclamp thing, not sure what the best solution is there.
Will intel_pstate just always ignore it? Might be better then to depend on !intel_pstate?

Quoted 33 lines

---
 kernel/sched/topology.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Index: linux-pm/kernel/sched/topology.c
===================================================================

kernel/sched/topology.c +3 -3

@@ -251,7 +251,7 @@ static bool sched_is_eas_possible(const
              return false;
      }
 
-     /* Do not attempt EAS if schedutil is not being used. */
+     /* Do not attempt EAS with a cpufreq governor other than schedutil. */
      for_each_cpu(i, cpu_mask) {
              policy = cpufreq_cpu_get(i);
              if (!policy) {

@@ -263,9 +263,9 @@ static bool sched_is_eas_possible(const
              }
              gov = policy->governor;
              cpufreq_cpu_put(policy);
-             if (gov != &schedutil_gov) {
+             if (gov && gov != &schedutil_gov) {
                      if (sched_debug()) {
-                             pr_info("rd %*pbl: Checking EAS, schedutil is mandatory\n",
+                             pr_info("rd %*pbl: Checking EAS, cpufreq governor is not schedutil\n",
                                      cpumask_pr_args(cpu_mask));
                      }
                      return false;

Quoted 16 lines · Christian Loehle

Quoted 14 lines · Rafael J. Wysocki

From: Rafael J. Wysocki <[email protected]>

Some cpufreq drivers, like intel_pstate, have built-in governors that are used instead of regular cpufreq governors, schedutil in particular, but they can work with EAS just fine, so allow EAS to be used with those drivers.

Also update the debug message printed when the cpufreq governor in use is not schedutil and the related comment, to better match the code after the change.

Signed-off-by Rafael J. Wysocki <[email protected]>

---

Quoted 7 lines

Quoted 4 lines

I'm not sure how much value there is in refusing to enable EAS without schedutil in general. For instance, if there are no crossover points between the cost curves for different perf domains, EAS may as well be used with the performance and powersave governors AFAICS.

Agreed, but having no cross-over points or no DVFS at all should be the only instances, right?

Not really. This is the most obvious case, but there are other less obvious ones.

Say there are two cross-over points: The "performance" and "powersave" governors should still be fine with EAS in that case.

Or what if somebody has a governor in user space that generally behaves like schedutil?

Or what about ondemand? Is it alway completely broken with EAS?

Quoted 5 lines

For plain (non-intel_pstate) powersave and performance we could replace sugov_effective_cpu_perf()
that determines the OPP of the perf-domain by the OPP they will be choosing, but for the rest?

I generally think that depending on schedutil for EAS is a mistake.

I would just print a warning that results may be suboptimal or generally not as expected if the cpufreq governor is not schedutil instead of preventing EAS from running at all.

Quoted 4 lines

Also there is the entire uclamp thing, not sure what the best solution is there.
Will intel_pstate just always ignore it? Might be better then to depend on !intel_pstate?

Well, it can be made dependent on policy->policy == CPUFREQ_POLICY_POWERSAVE if gov is NULL or similar, but honestly why bother?

Quoted 35 lines

Quoted 33 lines

---
 kernel/sched/topology.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Index: linux-pm/kernel/sched/topology.c
===================================================================

kernel/sched/topology.c +3 -3

@@ -251,7 +251,7 @@ static bool sched_is_eas_possible(const
              return false;
      }
 
-     /* Do not attempt EAS if schedutil is not being used. */
+     /* Do not attempt EAS with a cpufreq governor other than schedutil. */
      for_each_cpu(i, cpu_mask) {
              policy = cpufreq_cpu_get(i);
              if (!policy) {

@@ -263,9 +263,9 @@ static bool sched_is_eas_possible(const
              }
              gov = policy->governor;
              cpufreq_cpu_put(policy);
-             if (gov != &schedutil_gov) {
+             if (gov && gov != &schedutil_gov) {
                      if (sched_debug()) {
-                             pr_info("rd %*pbl: Checking EAS, schedutil is mandatory\n",
+                             pr_info("rd %*pbl: Checking EAS, cpufreq governor is not schedutil\n",
                                      cpumask_pr_args(cpu_mask));
                      }
                      return false;

Or what about ondemand? Is it alway completely broken with EAS?

Quoted 4 lines

For plain (non-intel_pstate) powersave and performance we could replace sugov_effective_cpu_perf()
that determines the OPP of the perf-domain by the OPP they will be choosing, but for the rest?

I generally think that depending on schedutil for EAS is a mistake.

Quoted 1 line · Rafael J. Wysocki

Or what about ondemand? Is it alway completely broken with EAS?

I thought that thing was mostly considered broken anyway :-)

Quoted 6 lines

Quoted 4 lines

For plain (non-intel_pstate) powersave and performance we could replace sugov_effective_cpu_perf()
that determines the OPP of the perf-domain by the OPP they will be choosing, but for the rest?

I generally think that depending on schedutil for EAS is a mistake.

Well, the thinking was that we wanted to move to a single governor, and not proliferate things.

From: Rafael J. Wysocki <[email protected]>

Modify intel_pstate to register stub EM perf domains for CPUs on hybrid platforms via em_dev_register_perf_domain() and to use em_dev_expand_perf_domain() introduced previously for adding new CPUs to existing EM perf domains when those CPUs become online for the first time after driver initialization.

This change is targeting platforms (for example, Lunar Lake) where "small" CPUs (E-cores) are always more energy-efficient than the "big" or "performance" CPUs (P-cores) when run at the same HWP performance level, so it is sufficient to tell the EAS that E-cores are always preferred (so long as there is enough spare capacity on one of them to run the given task).

Accordingly, the perf domains are registered per CPU type (that is, all P-cores belong to one perf domain and all E-cores belong to another perf domain) and they are registered only if asymmetric CPU capacity is enabled. Each perf domain has a one-element states table and that element only contains the relative cost value (the other fields in it are not initialized, so they are all equal to zero), and the cost value for the E-core perf domain is lower.

drivers/cpufreq/intel_pstate.c +24 -0

+}
+
+static int hybrid_ecore_cost(struct device *dev, unsigned long freq,
+			     unsigned long *cost)
+{
+	*cost = 1;
+	return 0;
+}
+
+static struct hybrid_em_perf_domain perf_domains[HYBRID_NR_TYPES] = {
+	[HYBRID_PCORE] = { .cb.get_cost = hybrid_pcore_cost, },
+	[HYBRID_ECORE] = { .cb.get_cost = hybrid_ecore_cost, }
+};
+
+static bool hybrid_register_perf_domain(struct hybrid_em_perf_domain *pd)
+{
+	/*
+	 * Registering EM perf domains without asymmetric CPU capacity
+	 * support enabled is wasteful, so don't do that.
+	 */
+	if (!hybrid_max_perf_cpu)
+		return false;
+
+	pd->dev = get_cpu_device(cpumask_first(&pd->cpumask));

drivers/cpufreq/intel_pstate.c +12 -0

+static void hybrid_register_all_perf_domains(void)
+{
+	enum hybrid_cpu_type type;
+
+	for (type = HYBRID_PCORE; type < HYBRID_NR_TYPES; type++)
+		hybrid_register_perf_domain(&perf_domains[type]);
+}
+
+static void hybrid_add_to_perf_domain(int cpu, enum hybrid_cpu_type type)
+{
+	struct hybrid_em_perf_domain *pd = &perf_domains[type];
+

Quoted 8 lines · Rafael J. Wysocki

From: Rafael J. Wysocki <[email protected]>

Modify intel_pstate to register stub EM perf domains for CPUs on hybrid platforms via em_dev_register_perf_domain() and to use em_dev_expand_perf_domain() introduced previously for adding new CPUs to existing EM perf domains when those CPUs become online for the first time after driver initialization.

Quoted 6 lines

This change is targeting platforms (for example, Lunar Lake) where "small" CPUs (E-cores) are always more energy-efficient than the "big" or "performance" CPUs (P-cores) when run at the same HWP performance level, so it is sufficient to tell the EAS that E-cores are always preferred (so long as there is enough spare capacity on one of them to run the given task).

By treating all big CPUs (ignoring the different itmt prio values between them) we would have a system in which PD's are not in sync with the asym_cap_list* or the CPU capacities of individual CPUs and sched groups within the sched domain. Not sure if we want to go this way?

* used by misfit handling - 22d5607400c6 ("sched/fair: Check if a task has a fitting CPU when updating misfit")

Quoted 7 lines

Accordingly, the perf domains are registered per CPU type (that is, all P-cores belong to one perf domain and all E-cores belong to another perf domain) and they are registered only if asymmetric CPU capacity is enabled. Each perf domain has a one-element states table and that element only contains the relative cost value (the other fields in it are not initialized, so they are all equal to zero), and the cost value for the E-core perf domain is lower.

[...]

drivers/cpufreq/intel_pstate.c static struct cpudata *hybrid_max_perf_c · line 971

Quoted 10 lines

drivers/cpufreq/intel_pstate.c +10 -0

+static int hybrid_pcore_cost(struct device *dev, unsigned long freq,
+                          unsigned long *cost)
+{
+     /*
+      * The number used here needs to be higher than the analogous
+      * one in hybrid_ecore_cost() below.  The units and the actual
+      * values don't matter.
+      */
+     *cost = 2;
+     return 0;

So you're not tying this to HFI energy scores?

Quoted 24 lines

drivers/cpufreq/intel_pstate.c +24 -0

+}
+
+static int hybrid_ecore_cost(struct device *dev, unsigned long freq,
+                          unsigned long *cost)
+{
+     *cost = 1;
+     return 0;
+}
+
+static struct hybrid_em_perf_domain perf_domains[HYBRID_NR_TYPES] = {
+     [HYBRID_PCORE] = { .cb.get_cost = hybrid_pcore_cost, },
+     [HYBRID_ECORE] = { .cb.get_cost = hybrid_ecore_cost, }
+};
+
+static bool hybrid_register_perf_domain(struct hybrid_em_perf_domain *pd)
+{
+     /*
+      * Registering EM perf domains without asymmetric CPU capacity
+      * support enabled is wasteful, so don't do that.
+      */
+     if (!hybrid_max_perf_cpu)
+             return false;
+
+     pd->dev = get_cpu_device(cpumask_first(&pd->cpumask));

drivers/cpufreq/intel_pstate.c static struct cpudata *hybrid_max_perf_c · line 1005

Quoted 10 lines

drivers/cpufreq/intel_pstate.c +10 -0

+     if (!pd->dev)
+             return false;
+
+     if (em_dev_register_perf_domain(pd->dev, 1, &pd->cb, &pd->cpumask, false)) {
+             pd->dev = NULL;
+             return false;
+     }
+
+     return true;
+}

What are the issues in case you would use the existing ways (non-stub) to setup the EM?

static int intel_pstate_get_cpu_cost()

static void intel_pstate_register_em(struct cpufreq_policy *policy)

struct em_data_callback em_cb = EM_ADV_DATA_CB(NULL,
intel_pstate_get_cpu_cost)

em_dev_register_perf_domain(get_cpu_device(policy->cpu), 1,

                              &em_cb, policy->related_cpus, 1);
                                      ^^^^^^^^^^^^^^^^^^^^*

static void intel_pstate_set_register_em_fct(void)

default_driver->register_em = intel_pstate_register_em

static int __init intel_pstate_init(void)

...
intel_pstate_set_register_em_fct()
...

I guess one issue is the per-CPU policy as an argument to em_dev_register_perf_domain() (*) ?

Quoted 17 lines

drivers/cpufreq/intel_pstate.c +17 -0

+static void hybrid_register_all_perf_domains(void)
+{
+     enum hybrid_cpu_type type;
+
+     for (type = HYBRID_PCORE; type < HYBRID_NR_TYPES; type++)
+             hybrid_register_perf_domain(&perf_domains[type]);
+}
+
+static void hybrid_add_to_perf_domain(int cpu, enum hybrid_cpu_type type)
+{
+     struct hybrid_em_perf_domain *pd = &perf_domains[type];
+
+     guard(mutex)(&hybrid_capacity_lock);
+
+     if (cpumask_test_cpu(cpu, &pd->cpumask))
+             return;
+

drivers/cpufreq/intel_pstate.c static struct cpudata *hybrid_max_perf_c · line 1028

Quoted 5 lines

drivers/cpufreq/intel_pstate.c +5 -0

+     cpumask_set_cpu(cpu, &pd->cpumask);
+     if (pd->dev)
+             em_dev_expand_perf_domain(pd->dev, cpu);
+     else if (hybrid_register_perf_domain(pd))
+             em_rebuild_perf_domains();

I assume that the 'if' and the 'else if' condition here are only taken when the CPU is brought online after boot?

From: Rafael J. Wysocki <[email protected]>

Modify intel_pstate to register stub EM perf domains for CPUs on hybrid platforms via em_dev_register_perf_domain() and to use em_dev_expand_perf_domain() introduced previously for adding new CPUs to existing EM perf domains when those CPUs become online for the first time after driver initialization.

This change is targeting platforms (for example, Lunar Lake) where "small" CPUs (E-cores) are always more energy-efficient than the "big" or "performance" CPUs (P-cores) when run at the same HWP performance level, so it is sufficient to tell the EAS that E-cores are always preferred (so long as there is enough spare capacity on one of them to run the given task).

Accordingly, the perf domains are registered per CPU type (that is, all P-cores belong to one perf domain and all E-cores belong to another perf domain) and they are registered only if asymmetric CPU capacity is enabled. Each perf domain has a one-element states table and that element only contains the relative cost value (the other fields in it are not initialized, so they are all equal to zero), and the cost value for the E-core perf domain is lower.

Signed-off-by Rafael J. Wysocki <[email protected]>

---
  drivers/cpufreq/intel_pstate.c |  110 ++++++++++++++++++++++++++++++++++++++---
  1 file changed, 104 insertions(+), 6 deletions(-)

Index: linux-pm/drivers/cpufreq/intel_pstate.c
===================================================================

drivers/cpufreq/intel_pstate.c +22 -0

@@ -8,6 +8,7 @@
 
  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
+#include <linux/energy_model.h>
  #include <linux/kernel.h>
  #include <linux/kernel_stat.h>
  #include <linux/module.h>

@@ -938,6 +939,12 @@ static struct freq_attr *hwp_cpufreq_att
  	NULL,
  };
 
+enum hybrid_cpu_type {
+	HYBRID_PCORE = 0,
+	HYBRID_ECORE,
+	HYBRID_NR_TYPES
+};
+
  static struct cpudata *hybrid_max_perf_cpu __read_mostly;
  /*
   * Protects hybrid_max_perf_cpu, the capacity_perf fields in struct cpudata,

@@ -945,6 +952,86 @@ static struct cpudata *hybrid_max_perf_c
   */
  static DEFINE_MUTEX(hybrid_capacity_lock);
 
+#ifdef CONFIG_ENERGY_MODEL
+struct hybrid_em_perf_domain {
+	cpumask_t cpumask;
+	struct device *dev;
+	struct em_data_callback cb;
+};
+
+static int hybrid_pcore_cost(struct device *dev, unsigned long freq,
+			     unsigned long *cost)
+{
+	/*
+	 * The number used here needs to be higher than the analogous
+	 * one in hybrid_ecore_cost() below.  The units and the actual
+	 * values don't matter.
+	 */

Quoted 78 lines · Rafael J. Wysocki

From: Rafael J. Wysocki <[email protected]>

Modify intel_pstate to register stub EM perf domains for CPUs on hybrid platforms via em_dev_register_perf_domain() and to use em_dev_expand_perf_domain() introduced previously for adding new CPUs to existing EM perf domains when those CPUs become online for the first time after driver initialization.

This change is targeting platforms (for example, Lunar Lake) where "small" CPUs (E-cores) are always more energy-efficient than the "big" or "performance" CPUs (P-cores) when run at the same HWP performance level, so it is sufficient to tell the EAS that E-cores are always preferred (so long as there is enough spare capacity on one of them to run the given task).

Accordingly, the perf domains are registered per CPU type (that is, all P-cores belong to one perf domain and all E-cores belong to another perf domain) and they are registered only if asymmetric CPU capacity is enabled. Each perf domain has a one-element states table and that element only contains the relative cost value (the other fields in it are not initialized, so they are all equal to zero), and the cost value for the E-core perf domain is lower.

Signed-off-by Rafael J. Wysocki <[email protected]>

---
  drivers/cpufreq/intel_pstate.c |  110 ++++++++++++++++++++++++++++++++++++++---
  1 file changed, 104 insertions(+), 6 deletions(-)

Index: linux-pm/drivers/cpufreq/intel_pstate.c
===================================================================

drivers/cpufreq/intel_pstate.c +28 -0

@@ -8,6 +8,7 @@
 
  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
+#include <linux/energy_model.h>
  #include <linux/kernel.h>
  #include <linux/kernel_stat.h>
  #include <linux/module.h>

@@ -938,6 +939,12 @@ static struct freq_attr *hwp_cpufreq_att
      NULL,
  };
 
+enum hybrid_cpu_type {
+     HYBRID_PCORE = 0,
+     HYBRID_ECORE,
+     HYBRID_NR_TYPES
+};
+
  static struct cpudata *hybrid_max_perf_cpu __read_mostly;
  /*
   * Protects hybrid_max_perf_cpu, the capacity_perf fields in struct cpudata,

@@ -945,6 +952,86 @@ static struct cpudata *hybrid_max_perf_c
   */
  static DEFINE_MUTEX(hybrid_capacity_lock);
 
+#ifdef CONFIG_ENERGY_MODEL
+struct hybrid_em_perf_domain {
+     cpumask_t cpumask;
+     struct device *dev;
+     struct em_data_callback cb;
+};
+
+static int hybrid_pcore_cost(struct device *dev, unsigned long freq,
+                          unsigned long *cost)
+{
+     /*
+      * The number used here needs to be higher than the analogous
+      * one in hybrid_ecore_cost() below.  The units and the actual
+      * values don't matter.
+      */
+     *cost = 2;
+     return 0;
+}
+
+static int hybrid_ecore_cost(struct device *dev, unsigned long freq,
+                          unsigned long *cost)

drivers/cpufreq/intel_pstate.c static struct cpudata *hybrid_max_perf_c · line 979

Quoted 4 lines

drivers/cpufreq/intel_pstate.c +4 -0

+{
+     *cost = 1;
+     return 0;
+}

The artificial EM was introduced for CPPC based platforms since these platforms only provide an 'efficiency class' entry to describe the relative efficiency of CPUs. The case seems similar to yours.

'Fake' OPPs were created to have an incentive for EAS to balance the load on the CPUs in one perf. domain. Indeed, in feec(), during the energy computation of a pd, if the cost is independent from the max_util value, then one CPU in the pd could end up having a high util, and another CPU a NULL util.

For CPPC platforms, this was problematic as a lower OPP could have been selected for the same load, so energy was lost for no reason.

In your case it seems that the OPP selection is done independently on each CPU. However I assume it is still more energy efficient to have 2 CPUs loaded at 50% than one CPU loaded at 100% and an idle CPU.

Also as Dietmar suggested, maybe it would make sense to have some way to prefer an CPU with a "energy saving" HFI configuration than a similar CPU with a "performance" HFI configuration.

Also, out of curiosity, do you have energy numbers to share ?

Quoted 82 lines · Pierre Gondois

Quoted 78 lines · Rafael J. Wysocki

From: Rafael J. Wysocki <[email protected]>

Modify intel_pstate to register stub EM perf domains for CPUs on hybrid platforms via em_dev_register_perf_domain() and to use em_dev_expand_perf_domain() introduced previously for adding new CPUs to existing EM perf domains when those CPUs become online for the first time after driver initialization.

This change is targeting platforms (for example, Lunar Lake) where "small" CPUs (E-cores) are always more energy-efficient than the "big" or "performance" CPUs (P-cores) when run at the same HWP performance level, so it is sufficient to tell the EAS that E-cores are always preferred (so long as there is enough spare capacity on one of them to run the given task).

Accordingly, the perf domains are registered per CPU type (that is, all P-cores belong to one perf domain and all E-cores belong to another perf domain) and they are registered only if asymmetric CPU capacity is enabled. Each perf domain has a one-element states table and that element only contains the relative cost value (the other fields in it are not initialized, so they are all equal to zero), and the cost value for the E-core perf domain is lower.

Signed-off-by Rafael J. Wysocki <[email protected]>

---
   drivers/cpufreq/intel_pstate.c |  110 ++++++++++++++++++++++++++++++++++++++---
   1 file changed, 104 insertions(+), 6 deletions(-)

Index: linux-pm/drivers/cpufreq/intel_pstate.c
===================================================================

drivers/cpufreq/intel_pstate.c +28 -0

@@ -8,6 +8,7 @@
 
   #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
+#include <linux/energy_model.h>
   #include <linux/kernel.h>
   #include <linux/kernel_stat.h>
   #include <linux/module.h>

@@ -938,6 +939,12 @@ static struct freq_attr *hwp_cpufreq_att
       NULL,
   };
 
+enum hybrid_cpu_type {
+     HYBRID_PCORE = 0,
+     HYBRID_ECORE,
+     HYBRID_NR_TYPES
+};
+
   static struct cpudata *hybrid_max_perf_cpu __read_mostly;
   /*
    * Protects hybrid_max_perf_cpu, the capacity_perf fields in struct cpudata,

@@ -945,6 +952,86 @@ static struct cpudata *hybrid_max_perf_c
    */
   static DEFINE_MUTEX(hybrid_capacity_lock);
 
+#ifdef CONFIG_ENERGY_MODEL
+struct hybrid_em_perf_domain {
+     cpumask_t cpumask;
+     struct device *dev;
+     struct em_data_callback cb;
+};
+
+static int hybrid_pcore_cost(struct device *dev, unsigned long freq,
+                          unsigned long *cost)
+{
+     /*
+      * The number used here needs to be higher than the analogous
+      * one in hybrid_ecore_cost() below.  The units and the actual
+      * values don't matter.
+      */
+     *cost = 2;
+     return 0;
+}
+
+static int hybrid_ecore_cost(struct device *dev, unsigned long freq,
+                          unsigned long *cost)

Quoted 8 lines

drivers/cpufreq/intel_pstate.c static struct cpudata *hybrid_max_perf_c · line 979

Quoted 4 lines

drivers/cpufreq/intel_pstate.c +4 -0

+{
+     *cost = 1;
+     return 0;
+}

The artificial EM was introduced for CPPC based platforms since these platforms only provide an 'efficiency class' entry to describe the relative efficiency of CPUs. The case seems similar to yours.

It is, but I don't particularly like the CPPC driver's approach to this.

Quoted 5 lines

'Fake' OPPs were created to have an incentive for EAS to balance the load on the CPUs in one perf. domain. Indeed, in feec(), during the energy computation of a pd, if the cost is independent from the max_util value, then one CPU in the pd could end up having a high util, and another CPU a NULL util.

Isn't this a consequence of disabling load balancing by EAS?

Quoted 7 lines

For CPPC platforms, this was problematic as a lower OPP could have been selected for the same load, so energy was lost for no reason.

In your case it seems that the OPP selection is done independently on each CPU. However I assume it is still more energy efficient to have 2 CPUs loaded at 50% than one CPU loaded at 100% and an idle CPU.

Maybe.

It really depends on the cost of the idle state etc.

Quoted 3 lines

Also as Dietmar suggested, maybe it would make sense to have some way to prefer an CPU with a "energy saving" HFI configuration than a similar CPU with a "performance" HFI configuration.

As it happens, E-cores have higher energy-efficiency scores in HFI AFAICS.

Quoted 1 line

Also, out of curiosity, do you have energy numbers to share ?

Not yet, but there will be some going forward.

Thanks!

Quoted 94 lines · Rafael J. Wysocki

Quoted 82 lines · Pierre Gondois

Quoted 78 lines · Rafael J. Wysocki

From: Rafael J. Wysocki <[email protected]>

Modify intel_pstate to register stub EM perf domains for CPUs on hybrid platforms via em_dev_register_perf_domain() and to use em_dev_expand_perf_domain() introduced previously for adding new CPUs to existing EM perf domains when those CPUs become online for the first time after driver initialization.

This change is targeting platforms (for example, Lunar Lake) where "small" CPUs (E-cores) are always more energy-efficient than the "big" or "performance" CPUs (P-cores) when run at the same HWP performance level, so it is sufficient to tell the EAS that E-cores are always preferred (so long as there is enough spare capacity on one of them to run the given task).

Accordingly, the perf domains are registered per CPU type (that is, all P-cores belong to one perf domain and all E-cores belong to another perf domain) and they are registered only if asymmetric CPU capacity is enabled. Each perf domain has a one-element states table and that element only contains the relative cost value (the other fields in it are not initialized, so they are all equal to zero), and the cost value for the E-core perf domain is lower.

Signed-off-by Rafael J. Wysocki <[email protected]>

---
   drivers/cpufreq/intel_pstate.c |  110 ++++++++++++++++++++++++++++++++++++++---
   1 file changed, 104 insertions(+), 6 deletions(-)

Index: linux-pm/drivers/cpufreq/intel_pstate.c
===================================================================

drivers/cpufreq/intel_pstate.c +28 -0

@@ -8,6 +8,7 @@
 
   #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
+#include <linux/energy_model.h>
   #include <linux/kernel.h>
   #include <linux/kernel_stat.h>
   #include <linux/module.h>

@@ -938,6 +939,12 @@ static struct freq_attr *hwp_cpufreq_att
       NULL,
   };
 
+enum hybrid_cpu_type {
+     HYBRID_PCORE = 0,
+     HYBRID_ECORE,
+     HYBRID_NR_TYPES
+};
+
   static struct cpudata *hybrid_max_perf_cpu __read_mostly;
   /*
    * Protects hybrid_max_perf_cpu, the capacity_perf fields in struct cpudata,

@@ -945,6 +952,86 @@ static struct cpudata *hybrid_max_perf_c
    */
   static DEFINE_MUTEX(hybrid_capacity_lock);
 
+#ifdef CONFIG_ENERGY_MODEL
+struct hybrid_em_perf_domain {
+     cpumask_t cpumask;
+     struct device *dev;
+     struct em_data_callback cb;
+};
+
+static int hybrid_pcore_cost(struct device *dev, unsigned long freq,
+                          unsigned long *cost)
+{
+     /*
+      * The number used here needs to be higher than the analogous
+      * one in hybrid_ecore_cost() below.  The units and the actual
+      * values don't matter.
+      */
+     *cost = 2;
+     return 0;
+}
+
+static int hybrid_ecore_cost(struct device *dev, unsigned long freq,
+                          unsigned long *cost)

Quoted 8 lines

drivers/cpufreq/intel_pstate.c static struct cpudata *hybrid_max_perf_c · line 979

Quoted 4 lines

drivers/cpufreq/intel_pstate.c +4 -0

+{
+     *cost = 1;
+     return 0;
+}

The artificial EM was introduced for CPPC based platforms since these platforms only provide an 'efficiency class' entry to describe the relative efficiency of CPUs. The case seems similar to yours.

It is, but I don't particularly like the CPPC driver's approach to this.

Quoted 7 lines

Quoted 5 lines

'Fake' OPPs were created to have an incentive for EAS to balance the load on the CPUs in one perf. domain. Indeed, in feec(), during the energy computation of a pd, if the cost is independent from the max_util value, then one CPU in the pd could end up having a high util, and another CPU a NULL util.

Isn't this a consequence of disabling load balancing by EAS?

Yes. Going in that direction, this patch from Vincent should help balancing the load in your case I think. The patch evaluates other factors when the energy cost of multiple CPU-candidates is the same.

Meaning, if all CPUs of the same type have only one OPP, the number of tasks and the the load of the CPUs is then compared. This is not the case currently. Doing so will help to avoid having one CPU close to being overutilized while others are idle.

However I think it would still be better to have multiple OPPs in the energy model. Indeed, it would be closer to reality as I assume that for Intel aswell, there might be frequency domains and the frequency of the domain is lead by the most utilized CPU.

This would also avoid hitting corner cases. As if there is one big task and many small tasks, balancing on the number of tasks per CPU is not the best idea.

https://lore.kernel.org/all/[email protected]/