Re: [PATCH v2 0/3] Clarify abstract scale usage for power values in Energy Model, EAS and IPA

From: Lukasz Luba
Date: Fri Oct 16 2020 - 10:43:25 EST




On 10/16/20 2:09 PM, Quentin Perret wrote:
On Friday 16 Oct 2020 at 14:50:29 (+0200), Daniel Lezcano wrote:
On 16/10/2020 14:18, Quentin Perret wrote:
On Friday 16 Oct 2020 at 13:48:33 (+0200), Daniel Lezcano wrote:
If the SCMI is returning abstract numbers, the thermal IPA governor will
use these numbers as a reference to mitigate the temperature at the
specified sustainable power which is expressed in mW in the DT. So it
does not work and we can not detect such conflict.

That is why I'm advocating to keep mW for the energy model and make the
SCMI and DT power numbers incompatible.

I think it's fair to say SCMI-provided number should only be compared to
other SCMI-provided numbers, so +1 on that. But what I don't understand
is why specifying the EM in mW helps with that?

It is already specified in mW. I'm just saying to not add the
'scale'/'abstract'/'bogoWatt' in the documentation.

Can we not let the providers specify the unit?

Yes, it is possible but the provider must give the 'unit' and the energy
model must store this information along with the "power" numbers, so we
can compare apple with apple.

Today, the energy model is using the mW unit only and the providers are
not telling the 'unit', so both are missing.

Because both are missing, it does not make sense to talk about
'abstract' values in the energy model documentation until the above is
fixed.

Right, so that sounds like a reasonable way forward with this series.

Lukasz would you be able to re-spin this with a first patch that allows
the EM provider to specify a unit? And perhaps we could use Doug's idea
for the sustained power DT binding and allow specifying a unit
explicitly there too, so we're sure to compare apples with apples.

Do you mean a new entry in DT which will be always below
'dynamic-power-coefficient' and/or 'sustainable-power' saying the unit
of above value?

There was discussion with Rob (and Doug) about this. I got the
impression he was against any new DT stuff [1].
We don't have to, I think we all agree that DT will only support mW.

I have agreed to this idea having a 'flag' inside EM [2], which
indicates the mW or bogoWatts. It could be set via API:
em_dev_register_perf_domain() and this new last argument.

I can write that patch. There is only two usage (3rd is on LKML) of
that function. The DT way, which is via:
dev_pm_opp_of_register_em() will always set 'true';
Driver direct calls of em_dev_register_perf_domain(), will have to
set appropriate value ('true' or 'false'). The EM struct em_perf_domain
will have the new bool field set based on that.
Is it make sense?

Regards,
Lukasz

[1] https://lore.kernel.org/lkml/CAL_JsqJ=brfbLiTm9D+p2N0Az-gcStbYj=RS2EaG50dHo0-5WA@xxxxxxxxxxxxxx/
[2] https://lore.kernel.org/lkml/3e3dd42c-48ac-7267-45c5-ca88205611bd@xxxxxxx/


Thanks,
Quentin