Re: [PATCH 2/5] energy model: Add a get previous state function

[Lukasz Luba]

Hi Vincent,

On 8/30/24 14:03, Vincent Guittot wrote:
> Instead of parsing all EM table everytime, add a function to get the
> previous state.
>
> Will be used in the scheduler feec() function.
>
> Signed-off-by: Vincent Guittot <vincent.guittot@xxxxxxxxxx>
> ---
>   include/linux/energy_model.h | 18 ++++++++++++++++++
>   1 file changed, 18 insertions(+)
>
> diff --git a/include/linux/energy_model.h b/include/linux/energy_model.h
> index 1ff52020cf75..ea8ea7e031c0 100644
> --- a/include/linux/energy_model.h
> +++ b/include/linux/energy_model.h
> @@ -207,6 +207,24 @@ em_pd_get_efficient_state(struct em_perf_state *table, int nr_perf_states,
>   	return nr_perf_states - 1;
>   }
>   
> +static inline int
> +em_pd_get_previous_state(struct em_perf_state *table, int nr_perf_states,
> +			  int idx, unsigned long pd_flags)
> +{
> +	struct em_perf_state *ps;
> +	int i;
> +
> +	for (i = idx - 1; i >= 0; i--) {
> +		ps = &table[i];
> +		if (pd_flags & EM_PERF_DOMAIN_SKIP_INEFFICIENCIES &&
> +		    ps->flags & EM_PERF_STATE_INEFFICIENT)
> +			continue;
> +		return i;
> +	}

Would you mind to add a comment on top of that for loop?
Or maybe a bit more detail in the patch header what would you like to
find (e.g. 1st efficient OPP which is lower).

It's looking for a first OPP (don't forget it's ascending 'table') which
is lower or equal to the 'idx' state.

If uclamp_max is set and that OPP is inefficient, don't we choose
a higher OPP which is efficient?

I'm not against this function.

BTW, I wonder if this design is still valid with the uclamp_max.

Regards,
Lukasz