Create a dedicated function which will be easier to maintain and re-use
in future. The upcoming changes for the modifiable EM perf_state table
will use it.
Signed-off-by: Lukasz Luba <[email protected]>
---
kernel/power/energy_model.c | 72 ++++++++++++++++++++++---------------
1 file changed, 43 insertions(+), 29 deletions(-)
diff --git a/kernel/power/energy_model.c b/kernel/power/energy_model.c
index 85a70b7da023..fd1066dcf38b 100644
--- a/kernel/power/energy_model.c
+++ b/kernel/power/energy_model.c
@@ -103,14 +103,52 @@ static void em_debug_create_pd(struct device *dev) {}
static void em_debug_remove_pd(struct device *dev) {}
#endif
+static int em_compute_costs(struct device *dev, struct em_perf_state *table,
+ struct em_data_callback *cb, int nr_states,
+ unsigned long flags)
+{
+ unsigned long prev_cost = ULONG_MAX;
+ u64 fmax;
+ int i, ret;
+
+ /* Compute the cost of each performance state. */
+ fmax = (u64) table[nr_states - 1].frequency;
+ for (i = nr_states - 1; i >= 0; i--) {
+ unsigned long power_res, cost;
+
+ if (flags & EM_PERF_DOMAIN_ARTIFICIAL) {
+ ret = cb->get_cost(dev, table[i].frequency, &cost);
+ if (ret || !cost || cost > EM_MAX_POWER) {
+ dev_err(dev, "EM: invalid cost %lu %d\n",
+ cost, ret);
+ return -EINVAL;
+ }
+ } else {
+ power_res = table[i].power;
+ cost = div64_u64(fmax * power_res, table[i].frequency);
+ }
+
+ table[i].cost = cost;
+
+ if (table[i].cost >= prev_cost) {
+ table[i].flags = EM_PERF_STATE_INEFFICIENT;
+ dev_dbg(dev, "EM: OPP:%lu is inefficient\n",
+ table[i].frequency);
+ } else {
+ prev_cost = table[i].cost;
+ }
+ }
+
+ return 0;
+}
+
static int em_create_perf_table(struct device *dev, struct em_perf_domain *pd,
int nr_states, struct em_data_callback *cb,
unsigned long flags)
{
- unsigned long power, freq, prev_freq = 0, prev_cost = ULONG_MAX;
+ unsigned long power, freq, prev_freq = 0;
struct em_perf_state *table;
int i, ret;
- u64 fmax;
table = kcalloc(nr_states, sizeof(*table), GFP_KERNEL);
if (!table)
@@ -154,33 +192,9 @@ static int em_create_perf_table(struct device *dev, struct em_perf_domain *pd,
table[i].frequency = prev_freq = freq;
}
- /* Compute the cost of each performance state. */
- fmax = (u64) table[nr_states - 1].frequency;
- for (i = nr_states - 1; i >= 0; i--) {
- unsigned long power_res, cost;
-
- if (flags & EM_PERF_DOMAIN_ARTIFICIAL) {
- ret = cb->get_cost(dev, table[i].frequency, &cost);
- if (ret || !cost || cost > EM_MAX_POWER) {
- dev_err(dev, "EM: invalid cost %lu %d\n",
- cost, ret);
- goto free_ps_table;
- }
- } else {
- power_res = table[i].power;
- cost = div64_u64(fmax * power_res, table[i].frequency);
- }
-
- table[i].cost = cost;
-
- if (table[i].cost >= prev_cost) {
- table[i].flags = EM_PERF_STATE_INEFFICIENT;
- dev_dbg(dev, "EM: OPP:%lu is inefficient\n",
- table[i].frequency);
- } else {
- prev_cost = table[i].cost;
- }
- }
+ ret = em_compute_costs(dev, table, cb, nr_states, flags);
+ if (ret)
+ goto free_ps_table;
pd->table = table;
pd->nr_perf_states = nr_states;
--
2.25.1
On 12/05/2023 11:57, Lukasz Luba wrote:
> Create a dedicated function which will be easier to maintain and re-use
I guess `refactor` would be more suitable than `create` here?
AFAICS, you factor out em_compute_costs() from em_create_perf_table() so
you can use it later for the updater of the runtime modifiable EM
em_dev_update_perf_domain() (in 10/17) as well.
[...]
On 5/30/23 10:51, Dietmar Eggemann wrote:
> On 12/05/2023 11:57, Lukasz Luba wrote:
>> Create a dedicated function which will be easier to maintain and re-use
>
> I guess `refactor` would be more suitable than `create` here?
Good point, I'll rephrase that.
>
> AFAICS, you factor out em_compute_costs() from em_create_perf_table() so
> you can use it later for the updater of the runtime modifiable EM
> em_dev_update_perf_domain() (in 10/17) as well.
>
> [...]
yes