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

[Lukasz Luba]

On 1/17/24 12:45, Hongyan Xia wrote:
> On 17/01/2024 09:56, 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 be re-used. 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.
> >
> > Reviewed-by: Daniel Lezcano <daniel.lezcano@xxxxxxxxxx>
> > Signed-off-by: Lukasz Luba <lukasz.luba@xxxxxxx>
> > ---
> >   include/linux/energy_model.h | 30 +++++++++++++++++-------------
> >   1 file changed, 17 insertions(+), 13 deletions(-)
> >
> > diff --git a/include/linux/energy_model.h b/include/linux/energy_model.h
> > index c19e7effe764..b01277b17946 100644
> > --- a/include/linux/energy_model.h
> > +++ b/include/linux/energy_model.h
> > @@ -175,33 +175,35 @@ void em_dev_unregister_perf_domain(struct device 
> > *dev);
> >   /**
> >    * em_pd_get_efficient_state() - Get an efficient performance state 
> > from the EM
> > - * @pd   : Performance domain for which we want an efficient frequency
> > - * @freq : Frequency to map with the EM
> > + * @table:        List of performance states, in ascending order
> > + * @nr_perf_states:    Number of performance states
> > + * @freq:        Frequency to map with the EM
> > + * @pd_flags:        Performance Domain flags
> >    *
> >    * It is called from the scheduler code quite frequently and as a 
> > consequence
> >    * doesn't implement any check.
> >    *
> > - * Return: An efficient performance state, high enough to meet @freq
> > + * Return: An efficient performance state id, high enough to meet @freq
> >    * requirement.
> >    */
> > -static inline
> > -struct em_perf_state *em_pd_get_efficient_state(struct em_perf_domain 
> > *pd,
> > -                        unsigned long freq)
> > +static inline int
> > +em_pd_get_efficient_state(struct em_perf_state *table, int 
> > nr_perf_states,
> > +              unsigned long freq, unsigned long pd_flags)
> >   {
> >       struct em_perf_state *ps;
> >       int i;
> > -    for (i = 0; i < pd->nr_perf_states; i++) {
> > -        ps = &pd->table[i];
> > +    for (i = 0; i < nr_perf_states; i++) {
> > +        ps = &table[i];
> >           if (ps->frequency >= freq) {
> > -            if (pd->flags & EM_PERF_DOMAIN_SKIP_INEFFICIENCIES &&
> > +            if (pd_flags & EM_PERF_DOMAIN_SKIP_INEFFICIENCIES &&
> >                   ps->flags & EM_PERF_STATE_INEFFICIENT)
> >                   continue;
> > -            break;
> > +            return i;
> >           }
> >       }
> > -    return ps;
> > +    return nr_perf_states - 1;
> >   }
> >   /**
> > @@ -226,7 +228,7 @@ static inline unsigned long em_cpu_energy(struct 
> > em_perf_domain *pd,
> >   {
> >       unsigned long freq, ref_freq, scale_cpu;
> >       struct em_perf_state *ps;
> > -    int cpu;
> > +    int cpu, i;
> >       if (!sum_util)
> >           return 0;
> > @@ -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);
> > +    ps = &pd->table[i];
> >       /*
> >        * The capacity of a CPU in the domain at the performance state 
> > (ps)
>
> Reviewed-by: Hongyan Xia <hongyan.xia@xxxxxxx>

Thank you Hongyan for the reviews!
I might address your NIT comment for the patch 2/24 when
I do the re-basing and sending the v8 (if there is a need).

Regards,
Lukasz