Re: [PATCH v2 1/1] PM: EM: Increase energy calculation precision

[Lukasz Luba]

On 8/3/21 10:44 AM, Dietmar Eggemann wrote:
> On 02/08/2021 08:21, Lukasz Luba wrote:
> > Hi Peter, Vincent,
> >
> > Gentle ping.
> >
> >
> > On 7/20/21 10:41 AM, Lukasz Luba wrote:
>
> [...]
>
> > > There are corner cases when the EAS energy calculation for two
> > > Performance
> > > Domains (PDs) return the same value. The EAS compares these values to
> > > choose smaller one. It might happen that this values are equal due to
> > > rounding error. In such scenario, we need better resolution, e.g. 1000
> > > times better. To provide this possibility increase the resolution in the
> > > em_perf_state::cost for 64-bit architectures. The costs for increasing
> > > resolution in 32-bit architectures are pretty high (64-bit division) and
> > > the returns do not justify the increased costs.
>
> s/The costs ... increased costs./The cost of increasing resolution on
> 32-bit is pretty high (64-bit division) and is not justified since there
> are no new 32bit big.LITTLE EAS systems expected which would benefit
> from this higher resolution./ ?

Sounds better indeed.

> > > This patch allows to avoid the rounding to milli-Watt errors, which might
> > > occur in EAS energy estimation for each Performance Domains (PD). The
>
> s/Performance Domains (PD)/PD.

OK

> [...]
>
> > > Scenario:
> > > Low utilized system e.g. ~200 sum_util for PD0 and ~220 for PD1. There
> > > are quite a few small tasks ~10-15 util. These tasks would suffer for
> > > the rounding error. Such system utilization has been seen while playing
> > > some simple games. In such condition our partner reported 5..10mA less
> > > battery drain.
>
> Hard to digest: Maybe s/Such system ... battery drain./These utilization
> values are typical when running games on Android. One of our partners
> has reported 5..10mA less battery drain when running with increased
> resolution./ ?

Sounds good.

> > > Some details:
> > > We have two Perf Domains (PDs): PD0 (big) and PD1 (little)
>
> s/Perf Domains (PDs)/PDs

OK

> [...]
>
> > > 2. Difference in the the last find_energy_efficient_cpu(): margin filter.
>
> s/in the the last find_energy_efficient_cpu(): margin filter/in the 6%
> energy margin filter at the end of find_energy_efficient_cpu()/ ?

OK

[snip]

> Otherwise, LGTM.
>
> Reviewed-by: Dietmar Eggemann <dietmar.eggemann@xxxxxxx>

Thank you Dietmar for the review. I'm going to send the v3.

> This is now an EM only patch (Task scheduler (i.e. CFS/EAS) is only
> effected via compute_energy() -> em_cpu_energy().

True it's EM only now, so I will ask Rafael to pick it.