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

[Lukasz Luba <lukasz.luba@arm.com>]

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@arm.com>

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.