Re: [PATCH v4 04/18] PM: EM: Refactor em_pd_get_efficient_state() to be more flexible

[Lukasz Luba]

On 10/23/23 18:39, Daniel Lezcano wrote:
> On 25/09/2023 10:11, Lukasz Luba wrote:
> > The Energy Model (EM) is going to support runtime modification. There
> > are going to be 2 EM tables which store information. This patch aims
> > to prepare the code to be generic and use one of the tables. The function
> > will no longer get a pointer to 'struct em_perf_domain' (the EM) but
> > instead a pointer to 'struct em_perf_state' (which is one of the EM's
> > tables).
> >
> > Prepare em_pd_get_efficient_state() for the upcoming changes and
> > make it possible to re-use. Return an index for the best performance
> > state for a given EM table. The function arguments that are introduced
> > should allow to work on different performance state arrays. The caller of
> > em_pd_get_efficient_state() should be able to use the index either
> > on the default or the modifiable EM table.
> >
> > Signed-off-by: Lukasz Luba <lukasz.luba@xxxxxxx>
> > ---
>
> [ ... ]
>
> > @@ -251,7 +253,9 @@ static inline unsigned long em_cpu_energy(struct 
> > em_perf_domain *pd,
> >        * Find the lowest performance state of the Energy Model above the
> >        * requested frequency.
> >        */
> > -    ps = em_pd_get_efficient_state(pd, freq);
> > +    i = em_pd_get_efficient_state(pd->table, pd->nr_perf_states, freq,
> > +                      pd->flags);
>
> nitpicking but s/i/state/

Here it makes sense, I'll try to use 'state', but if that could be a bit
odd in later patches code, where I have:

ps = &runtime_table->state[i];

than:

'->state[state]'

won't fly. Although, let me check, because I'm going to drop the
2 tables design so some fields might get different names.

> Other than that:
>
> Reviewed-by: Daniel Lezcano <daniel.lezcano@xxxxxxxxxx>

Thanks!

> > +    ps = &pd->table[i];
> >       /*
> >        * The capacity of a CPU in the domain at the performance state 
> > (ps)