[PATCH 1/2] docs: Clarify abstract scale usage for power values in Energy Model

[PATCH 1/2] docs: Clarify abstract scale usage for power values in Energy Model

[Lukasz Luba]

The Energy Model (EM) can store power values in milli-Watts or in abstract
scale. This might cause issues in the subsystems which use the EM for
estimating the device power, such as:
- mixing of different scales in a subsystem which uses multiple
  (cooling) devices (e.g. thermal Intelligent Power Allocation (IPA))
- assuming that energy [milli-Joules] can be derived from the EM power
  values which might not be possible since the power scale doesn't have to
  be in milli-Watts

To avoid misconfiguration add the needed documentation to the EM and
related subsystems: EAS and IPA.

Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
---
 .../driver-api/thermal/power_allocator.rst          |  8 ++++++++
 Documentation/power/energy-model.rst                | 13 +++++++++++++
 Documentation/scheduler/sched-energy.rst            |  5 +++++
 3 files changed, 26 insertions(+)

diff --git a/Documentation/driver-api/thermal/power_allocator.rst b/Documentation/driver-api/thermal/power_allocator.rst
index 67b6a3297238..5e04553ded5f 100644
--- a/Documentation/driver-api/thermal/power_allocator.rst
+++ b/Documentation/driver-api/thermal/power_allocator.rst
@@ -269,3 +269,11 @@ won't be very good.  Note that this is not particular to this
 governor, step-wise will also misbehave if you call its throttle()
 faster than the normal thermal framework tick (due to interrupts for
 example) as it will overreact.
+
+Energy Model requirements
+=========================
+
+Another important thing is the consistent scale of the power values
+provided by the cooling devices. All of the cooling devices in a single
+thermal zone should have power values reported either in milli-Watts
+or scaled to the same 'abstract scale'.
diff --git a/Documentation/power/energy-model.rst b/Documentation/power/energy-model.rst
index a6fb986abe3c..ba7aa581b307 100644
--- a/Documentation/power/energy-model.rst
+++ b/Documentation/power/energy-model.rst
@@ -20,6 +20,19 @@ possible source of information on its own, the EM framework intervenes as an
 abstraction layer which standardizes the format of power cost tables in the
 kernel, hence enabling to avoid redundant work.
 
+The power values might be expressed in milli-Watts or in an 'abstract scale'.
+Multiple subsystems might use the EM and it is up to the system integrator to
+check that the requirements for the power value scale types are met. An example
+can be found in the Energy-Aware Scheduler documentation
+Documentation/scheduler/sched-energy.rst. For some subsystems like thermal or
+powercap power values expressed in an 'abstract scale' might cause issues.
+These subsystems are more interested in estimation of power used in the past,
+thus the real milli-Watts might be needed. An example of these requirements can
+be found in the Intelligent Power Allocation in
+Documentation/driver-api/thermal/power_allocator.rst.
+Important thing to keep in mind is that when the power values are expressed in
+an 'abstract scale' deriving real energy in milli-Joules would not be possible.
+
 The figure below depicts an example of drivers (Arm-specific here, but the
 approach is applicable to any architecture) providing power costs to the EM
 framework, and interested clients reading the data from it::
diff --git a/Documentation/scheduler/sched-energy.rst b/Documentation/scheduler/sched-energy.rst
index 001e09c95e1d..afe02d394402 100644
--- a/Documentation/scheduler/sched-energy.rst
+++ b/Documentation/scheduler/sched-energy.rst
@@ -350,6 +350,11 @@ independent EM framework in Documentation/power/energy-model.rst.
 Please also note that the scheduling domains need to be re-built after the
 EM has been registered in order to start EAS.
 
+EAS uses the EM to make a forecasting decision on energy usage and thus it is
+more focused on the difference when checking possible options for task
+placement. For EAS it doesn't matter whether the EM power values are expressed
+in milli-Watts or in an 'abstract scale'.
+
 
 6.3 - Energy Model complexity
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-- 
2.17.1

[PATCH 2/2] PM / EM: update the comments related to power scale

[Lukasz Luba]

The Energy Model supports power values expressed in milli-Watts or in an
'abstract scale'. Update the related comments is the code to reflect that
state.

Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
---
 include/linux/energy_model.h | 11 +++++------
 kernel/power/energy_model.c  |  2 +-
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/include/linux/energy_model.h b/include/linux/energy_model.h
index b67a51c574b9..b19146b31760 100644
--- a/include/linux/energy_model.h
+++ b/include/linux/energy_model.h
@@ -13,9 +13,8 @@
 /**
  * em_perf_state - Performance state of a performance domain
  * @frequency:	The frequency in KHz, for consistency with CPUFreq
- * @power:	The power consumed at this level, in milli-watts (by 1 CPU or
-		by a registered device). It can be a total power: static and
-		dynamic.
+ * @power:	The power consumed at this level (by 1 CPU or by a registered
+ *		device). It can be a total power: static and dynamic.
  * @cost:	The cost coefficient associated with this level, used during
  *		energy calculation. Equal to: power * max_frequency / frequency
  */
@@ -55,7 +54,7 @@ struct em_data_callback {
 	/**
 	 * active_power() - Provide power at the next performance state of
 	 *		a device
-	 * @power	: Active power at the performance state in mW
+	 * @power	: Active power at the performance state
 	 *		(modified)
 	 * @freq	: Frequency at the performance state in kHz
 	 *		(modified)
@@ -66,8 +65,8 @@ struct em_data_callback {
 	 * and frequency.
 	 *
 	 * In case of CPUs, the power is the one of a single CPU in the domain,
-	 * expressed in milli-watts. It is expected to fit in the
-	 * [0, EM_MAX_POWER] range.
+	 * expressed in milli-Watts or an abstract scale. It is expected to
+	 * fit in the [0, EM_MAX_POWER] range.
 	 *
 	 * Return 0 on success.
 	 */
diff --git a/kernel/power/energy_model.c b/kernel/power/energy_model.c
index c1ff7fa030ab..2bd2afbb83f5 100644
--- a/kernel/power/energy_model.c
+++ b/kernel/power/energy_model.c
@@ -130,7 +130,7 @@ static int em_create_perf_table(struct device *dev, struct em_perf_domain *pd,
 
 		/*
 		 * The power returned by active_state() is expected to be
-		 * positive, in milli-watts and to fit into 16 bits.
+		 * positive and to fit into 16 bits.
 		 */
 		if (!power || power > EM_MAX_POWER) {
 			dev_err(dev, "EM: invalid power: %lu\n",
-- 
2.17.1

Re: [PATCH 1/2] docs: Clarify abstract scale usage for power values in Energy Model

[Doug Anderson]

Hi,

On Tue, Sep 29, 2020 at 5:16 AM Lukasz Luba <lukasz.luba@arm.com> wrote:
>
> The Energy Model (EM) can store power values in milli-Watts or in abstract
> scale. This might cause issues in the subsystems which use the EM for
> estimating the device power, such as:
> - mixing of different scales in a subsystem which uses multiple
>   (cooling) devices (e.g. thermal Intelligent Power Allocation (IPA))
> - assuming that energy [milli-Joules] can be derived from the EM power
>   values which might not be possible since the power scale doesn't have to
>   be in milli-Watts
>
> To avoid misconfiguration add the needed documentation to the EM and
> related subsystems: EAS and IPA.
>
> Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
> ---
>  .../driver-api/thermal/power_allocator.rst          |  8 ++++++++
>  Documentation/power/energy-model.rst                | 13 +++++++++++++
>  Documentation/scheduler/sched-energy.rst            |  5 +++++
>  3 files changed, 26 insertions(+)

I haven't read through these files in massive detail, but the quick
skim makes me believe that your additions seem sane.  In general, I'm
happy with documenting reality, thus:

Reviewed-by: Douglas Anderson <dianders@chromium.org>

I will note: you haven't actually updated the device tree bindings.
Thus, presumably, anyone who is specifying these numbers in the device
tree is still supposed to specify them in a way that mW can be
recovered, right?  Said another way: nothing about your patches makes
it OK to specify numbers in device trees using an "abstract scale",
right?

-Doug

Re: [PATCH 1/2] docs: Clarify abstract scale usage for power values in Energy Model

[Lukasz Luba]

Hi Douglas,

On 9/30/20 12:53 AM, Doug Anderson wrote:
> Hi,
> 
> On Tue, Sep 29, 2020 at 5:16 AM Lukasz Luba <lukasz.luba@arm.com> wrote:
>>
>> The Energy Model (EM) can store power values in milli-Watts or in abstract
>> scale. This might cause issues in the subsystems which use the EM for
>> estimating the device power, such as:
>> - mixing of different scales in a subsystem which uses multiple
>>    (cooling) devices (e.g. thermal Intelligent Power Allocation (IPA))
>> - assuming that energy [milli-Joules] can be derived from the EM power
>>    values which might not be possible since the power scale doesn't have to
>>    be in milli-Watts
>>
>> To avoid misconfiguration add the needed documentation to the EM and
>> related subsystems: EAS and IPA.
>>
>> Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
>> ---
>>   .../driver-api/thermal/power_allocator.rst          |  8 ++++++++
>>   Documentation/power/energy-model.rst                | 13 +++++++++++++
>>   Documentation/scheduler/sched-energy.rst            |  5 +++++
>>   3 files changed, 26 insertions(+)
> 
> I haven't read through these files in massive detail, but the quick
> skim makes me believe that your additions seem sane.  In general, I'm
> happy with documenting reality, thus:
> 
> Reviewed-by: Douglas Anderson <dianders@chromium.org>

Thank you for the review.

> 
> I will note: you haven't actually updated the device tree bindings.
> Thus, presumably, anyone who is specifying these numbers in the device
> tree is still supposed to specify them in a way that mW can be
> recovered, right?  Said another way: nothing about your patches makes
> it OK to specify numbers in device trees using an "abstract scale",
> right?

For completeness, we are talking here about the binding from:
Documentation/devicetree/bindings/arm/cpus.yaml
which is 'dynamic-power-coefficient'. Yes, it stays untouched, also the
unit (uW/MHz/V^2) which then allows to have mW in the power
values in the EM.

Regards,
Lukasz

Re: [PATCH 1/2] docs: Clarify abstract scale usage for power values in Energy Model

[Rajendra Nayak]

On 9/30/2020 1:55 PM, Lukasz Luba wrote:
> Hi Douglas,
> 
> On 9/30/20 12:53 AM, Doug Anderson wrote:
>> Hi,
>>
>> On Tue, Sep 29, 2020 at 5:16 AM Lukasz Luba <lukasz.luba@arm.com> wrote:
>>>
>>> The Energy Model (EM) can store power values in milli-Watts or in abstract
>>> scale. This might cause issues in the subsystems which use the EM for
>>> estimating the device power, such as:
>>> - mixing of different scales in a subsystem which uses multiple
>>>    (cooling) devices (e.g. thermal Intelligent Power Allocation (IPA))
>>> - assuming that energy [milli-Joules] can be derived from the EM power
>>>    values which might not be possible since the power scale doesn't have to
>>>    be in milli-Watts
>>>
>>> To avoid misconfiguration add the needed documentation to the EM and
>>> related subsystems: EAS and IPA.
>>>
>>> Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
>>> ---
>>>   .../driver-api/thermal/power_allocator.rst          |  8 ++++++++
>>>   Documentation/power/energy-model.rst                | 13 +++++++++++++
>>>   Documentation/scheduler/sched-energy.rst            |  5 +++++
>>>   3 files changed, 26 insertions(+)
>>
>> I haven't read through these files in massive detail, but the quick
>> skim makes me believe that your additions seem sane.  In general, I'm
>> happy with documenting reality, thus:
>>
>> Reviewed-by: Douglas Anderson <dianders@chromium.org>
> 
> Thank you for the review.
> 
>>
>> I will note: you haven't actually updated the device tree bindings.
>> Thus, presumably, anyone who is specifying these numbers in the device
>> tree is still supposed to specify them in a way that mW can be
>> recovered, right?  Said another way: nothing about your patches makes
>> it OK to specify numbers in device trees using an "abstract scale",
>> right?
> 
> For completeness, we are talking here about the binding from:
> Documentation/devicetree/bindings/arm/cpus.yaml
> which is 'dynamic-power-coefficient'. Yes, it stays untouched, also the
> unit (uW/MHz/V^2) which then allows to have mW in the power
> values in the EM.

So for platforms where 'dynamic-power-coefficient' is specified in device tree,
its always expected to be derived from 'real' power numbers on these platforms in
'real' mW?

Atleast on Qualcomm platforms we have these numbers scaled, so in essence it
can't be used to derive 'real' mW values. That said we also do not have any of
the 'platform might face potential issue of mixing devices in one thermal zone
of two scales' problem.

So the question is, can such platforms still use 'dynamic-power-coefficient'
in device tree and create an abstract scale? The other way of doing this would
be to *not* specify this value in device tree and have these values stored in the
cpufreq driver and register a custom callback to do the math.

It just feels like jumping through hoops just to deal with the fact that the
device tree bindings say its expected to be in mW and can't be abstract.

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH 1/2] docs: Clarify abstract scale usage for power values in Energy Model

[Lukasz Luba]

On 9/30/20 11:55 AM, Rajendra Nayak wrote:
> 
> On 9/30/2020 1:55 PM, Lukasz Luba wrote:
>> Hi Douglas,
>>
>> On 9/30/20 12:53 AM, Doug Anderson wrote:
>>> Hi,
>>>
>>> On Tue, Sep 29, 2020 at 5:16 AM Lukasz Luba <lukasz.luba@arm.com> wrote:
>>>>
>>>> The Energy Model (EM) can store power values in milli-Watts or in 
>>>> abstract
>>>> scale. This might cause issues in the subsystems which use the EM for
>>>> estimating the device power, such as:
>>>> - mixing of different scales in a subsystem which uses multiple
>>>>    (cooling) devices (e.g. thermal Intelligent Power Allocation (IPA))
>>>> - assuming that energy [milli-Joules] can be derived from the EM power
>>>>    values which might not be possible since the power scale doesn't 
>>>> have to
>>>>    be in milli-Watts
>>>>
>>>> To avoid misconfiguration add the needed documentation to the EM and
>>>> related subsystems: EAS and IPA.
>>>>
>>>> Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
>>>> ---
>>>>   .../driver-api/thermal/power_allocator.rst          |  8 ++++++++
>>>>   Documentation/power/energy-model.rst                | 13 
>>>> +++++++++++++
>>>>   Documentation/scheduler/sched-energy.rst            |  5 +++++
>>>>   3 files changed, 26 insertions(+)
>>>
>>> I haven't read through these files in massive detail, but the quick
>>> skim makes me believe that your additions seem sane.  In general, I'm
>>> happy with documenting reality, thus:
>>>
>>> Reviewed-by: Douglas Anderson <dianders@chromium.org>
>>
>> Thank you for the review.
>>
>>>
>>> I will note: you haven't actually updated the device tree bindings.
>>> Thus, presumably, anyone who is specifying these numbers in the device
>>> tree is still supposed to specify them in a way that mW can be
>>> recovered, right?  Said another way: nothing about your patches makes
>>> it OK to specify numbers in device trees using an "abstract scale",
>>> right?
>>
>> For completeness, we are talking here about the binding from:
>> Documentation/devicetree/bindings/arm/cpus.yaml
>> which is 'dynamic-power-coefficient'. Yes, it stays untouched, also the
>> unit (uW/MHz/V^2) which then allows to have mW in the power
>> values in the EM.
> 
> So for platforms where 'dynamic-power-coefficient' is specified in 
> device tree,
> its always expected to be derived from 'real' power numbers on these 
> platforms in
> 'real' mW?

Yes, the purpose and the name of that binding was only for 'real'
power in mW.

> 
> Atleast on Qualcomm platforms we have these numbers scaled, so in 
> essence it
> can't be used to derive 'real' mW values. That said we also do not have 
> any of
> the 'platform might face potential issue of mixing devices in one 
> thermal zone
> of two scales' problem.

If you have these numbers scaled, then it's probably documented
somewhere in your docs for your OEMs, because they might assume it's in
uW/MHz/V^2 (according to the bindings doc). If not, they probably
realized it during the measurements and comparison (that the power in
EM is not what they see on the power meter).
This binding actually helps those developers who take the experiments
and based on measured power values, store derived coefficient.
Everyone can just measure in local setup and compare the results
easily, speaking the same language (proposing maybe a patch adjusting
the value in DT).

> 
> So the question is, can such platforms still use 
> 'dynamic-power-coefficient'
> in device tree and create an abstract scale? The other way of doing this 
> would
> be to *not* specify this value in device tree and have these values 
> stored in the
> cpufreq driver and register a custom callback to do the math.

But then we would also have to change the name of that binding.

I'd recommend you the second way that you've described. It will avoid
your OEMs confusion. In your cpufreq driver you can simply register
to EM using the em_dev_register_perf_domain(). In your local
callback you can do whatever you need (read driver array, firmware,
DT, scale or not, etc).
The helper code in dev_pm_opp_of_register_em() is probably not suited
for your use case (when you don't want to share the real power of the
SoC).

> 
> It just feels like jumping through hoops just to deal with the fact that 
> the
> device tree bindings say its expected to be in mW and can't be abstract.
> 

I don't want to add more confusion into the EM power values topic.
Overloading the meaning of that binding would create more mess.

Regards,
Lukasz

Re: [PATCH 1/2] docs: Clarify abstract scale usage for power values in Energy Model

[Rajendra Nayak]

On 9/30/2020 7:34 PM, Lukasz Luba wrote:
> 
> 
> On 9/30/20 11:55 AM, Rajendra Nayak wrote:
>>
>> On 9/30/2020 1:55 PM, Lukasz Luba wrote:
>>> Hi Douglas,
>>>
>>> On 9/30/20 12:53 AM, Doug Anderson wrote:
>>>> Hi,
>>>>
>>>> On Tue, Sep 29, 2020 at 5:16 AM Lukasz Luba <lukasz.luba@arm.com> wrote:
>>>>>
>>>>> The Energy Model (EM) can store power values in milli-Watts or in abstract
>>>>> scale. This might cause issues in the subsystems which use the EM for
>>>>> estimating the device power, such as:
>>>>> - mixing of different scales in a subsystem which uses multiple
>>>>>    (cooling) devices (e.g. thermal Intelligent Power Allocation (IPA))
>>>>> - assuming that energy [milli-Joules] can be derived from the EM power
>>>>>    values which might not be possible since the power scale doesn't have to
>>>>>    be in milli-Watts
>>>>>
>>>>> To avoid misconfiguration add the needed documentation to the EM and
>>>>> related subsystems: EAS and IPA.
>>>>>
>>>>> Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
>>>>> ---
>>>>>   .../driver-api/thermal/power_allocator.rst          |  8 ++++++++
>>>>>   Documentation/power/energy-model.rst                | 13 +++++++++++++
>>>>>   Documentation/scheduler/sched-energy.rst            |  5 +++++
>>>>>   3 files changed, 26 insertions(+)
>>>>
>>>> I haven't read through these files in massive detail, but the quick
>>>> skim makes me believe that your additions seem sane.  In general, I'm
>>>> happy with documenting reality, thus:
>>>>
>>>> Reviewed-by: Douglas Anderson <dianders@chromium.org>
>>>
>>> Thank you for the review.
>>>
>>>>
>>>> I will note: you haven't actually updated the device tree bindings.
>>>> Thus, presumably, anyone who is specifying these numbers in the device
>>>> tree is still supposed to specify them in a way that mW can be
>>>> recovered, right?  Said another way: nothing about your patches makes
>>>> it OK to specify numbers in device trees using an "abstract scale",
>>>> right?
>>>
>>> For completeness, we are talking here about the binding from:
>>> Documentation/devicetree/bindings/arm/cpus.yaml
>>> which is 'dynamic-power-coefficient'. Yes, it stays untouched, also the
>>> unit (uW/MHz/V^2) which then allows to have mW in the power
>>> values in the EM.
>>
>> So for platforms where 'dynamic-power-coefficient' is specified in device tree,
>> its always expected to be derived from 'real' power numbers on these platforms in
>> 'real' mW?
> 
> Yes, the purpose and the name of that binding was only for 'real'
> power in mW.
> 
>>
>> Atleast on Qualcomm platforms we have these numbers scaled, so in essence it
>> can't be used to derive 'real' mW values. That said we also do not have any of
>> the 'platform might face potential issue of mixing devices in one thermal zone
>> of two scales' problem.
> 
> If you have these numbers scaled, then it's probably documented
> somewhere in your docs for your OEMs, because they might assume it's in
> uW/MHz/V^2 (according to the bindings doc). If not, they probably
> realized it during the measurements and comparison (that the power in
> EM is not what they see on the power meter).
> This binding actually helps those developers who take the experiments
> and based on measured power values, store derived coefficient.
> Everyone can just measure in local setup and compare the results
> easily, speaking the same language (proposing maybe a patch adjusting
> the value in DT).
> 
>>
>> So the question is, can such platforms still use 'dynamic-power-coefficient'
>> in device tree and create an abstract scale? The other way of doing this would
>> be to *not* specify this value in device tree and have these values stored in the
>> cpufreq driver and register a custom callback to do the math.
> 
> But then we would also have to change the name of that binding.
> 
> I'd recommend you the second way that you've described. It will avoid
> your OEMs confusion. In your cpufreq driver you can simply register
> to EM using the em_dev_register_perf_domain(). In your local
> callback you can do whatever you need (read driver array, firmware,
> DT, scale or not, etc).
> The helper code in dev_pm_opp_of_register_em() is probably not suited
> for your use case (when you don't want to share the real power of the
> SoC).

Got it, thanks for the clarification. I will get the cpufreq driver updated
to use em_dev_register_perf_domain() with a custom callback and get rid of these
values from device tree.

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH 1/2] docs: Clarify abstract scale usage for power values in Energy Model

[Doug Anderson]

Hi,

On Wed, Sep 30, 2020 at 8:48 AM Rajendra Nayak <rnayak@codeaurora.org> wrote:
>
>
> On 9/30/2020 7:34 PM, Lukasz Luba wrote:
> >
> >
> > On 9/30/20 11:55 AM, Rajendra Nayak wrote:
> >>
> >> On 9/30/2020 1:55 PM, Lukasz Luba wrote:
> >>> Hi Douglas,
> >>>
> >>> On 9/30/20 12:53 AM, Doug Anderson wrote:
> >>>> Hi,
> >>>>
> >>>> On Tue, Sep 29, 2020 at 5:16 AM Lukasz Luba <lukasz.luba@arm.com> wrote:
> >>>>>
> >>>>> The Energy Model (EM) can store power values in milli-Watts or in abstract
> >>>>> scale. This might cause issues in the subsystems which use the EM for
> >>>>> estimating the device power, such as:
> >>>>> - mixing of different scales in a subsystem which uses multiple
> >>>>>    (cooling) devices (e.g. thermal Intelligent Power Allocation (IPA))
> >>>>> - assuming that energy [milli-Joules] can be derived from the EM power
> >>>>>    values which might not be possible since the power scale doesn't have to
> >>>>>    be in milli-Watts
> >>>>>
> >>>>> To avoid misconfiguration add the needed documentation to the EM and
> >>>>> related subsystems: EAS and IPA.
> >>>>>
> >>>>> Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
> >>>>> ---
> >>>>>   .../driver-api/thermal/power_allocator.rst          |  8 ++++++++
> >>>>>   Documentation/power/energy-model.rst                | 13 +++++++++++++
> >>>>>   Documentation/scheduler/sched-energy.rst            |  5 +++++
> >>>>>   3 files changed, 26 insertions(+)
> >>>>
> >>>> I haven't read through these files in massive detail, but the quick
> >>>> skim makes me believe that your additions seem sane.  In general, I'm
> >>>> happy with documenting reality, thus:
> >>>>
> >>>> Reviewed-by: Douglas Anderson <dianders@chromium.org>
> >>>
> >>> Thank you for the review.
> >>>
> >>>>
> >>>> I will note: you haven't actually updated the device tree bindings.
> >>>> Thus, presumably, anyone who is specifying these numbers in the device
> >>>> tree is still supposed to specify them in a way that mW can be
> >>>> recovered, right?  Said another way: nothing about your patches makes
> >>>> it OK to specify numbers in device trees using an "abstract scale",
> >>>> right?
> >>>
> >>> For completeness, we are talking here about the binding from:
> >>> Documentation/devicetree/bindings/arm/cpus.yaml
> >>> which is 'dynamic-power-coefficient'. Yes, it stays untouched, also the
> >>> unit (uW/MHz/V^2) which then allows to have mW in the power
> >>> values in the EM.
> >>
> >> So for platforms where 'dynamic-power-coefficient' is specified in device tree,
> >> its always expected to be derived from 'real' power numbers on these platforms in
> >> 'real' mW?
> >
> > Yes, the purpose and the name of that binding was only for 'real'
> > power in mW.
> >
> >>
> >> Atleast on Qualcomm platforms we have these numbers scaled, so in essence it
> >> can't be used to derive 'real' mW values. That said we also do not have any of
> >> the 'platform might face potential issue of mixing devices in one thermal zone
> >> of two scales' problem.
> >
> > If you have these numbers scaled, then it's probably documented
> > somewhere in your docs for your OEMs, because they might assume it's in
> > uW/MHz/V^2 (according to the bindings doc). If not, they probably
> > realized it during the measurements and comparison (that the power in
> > EM is not what they see on the power meter).
> > This binding actually helps those developers who take the experiments
> > and based on measured power values, store derived coefficient.
> > Everyone can just measure in local setup and compare the results
> > easily, speaking the same language (proposing maybe a patch adjusting
> > the value in DT).
> >
> >>
> >> So the question is, can such platforms still use 'dynamic-power-coefficient'
> >> in device tree and create an abstract scale? The other way of doing this would
> >> be to *not* specify this value in device tree and have these values stored in the
> >> cpufreq driver and register a custom callback to do the math.
> >
> > But then we would also have to change the name of that binding.
> >
> > I'd recommend you the second way that you've described. It will avoid
> > your OEMs confusion. In your cpufreq driver you can simply register
> > to EM using the em_dev_register_perf_domain(). In your local
> > callback you can do whatever you need (read driver array, firmware,
> > DT, scale or not, etc).
> > The helper code in dev_pm_opp_of_register_em() is probably not suited
> > for your use case (when you don't want to share the real power of the
> > SoC).
>
> Got it, thanks for the clarification. I will get the cpufreq driver updated
> to use em_dev_register_perf_domain() with a custom callback and get rid of these
> values from device tree.

This sounds good.  ...except...

How exactly are boards supposed to provide their "sustainable-power"
number in this model?  As far as I'm aware, there's no place to
specify this board-specific file other than in device tree, and the
bindings [1] say that this value has to be in mW.  Lukasz: how do you
envision boards can provide "sustainable-power" in cases where the
energy model is in "abstract scale"?

[1] Documentation/devicetree/bindings/thermal/thermal-zones.yaml


-Doug

Re: [PATCH 1/2] docs: Clarify abstract scale usage for power values in Energy Model

[Lukasz Luba]

Hi Douglas

On 9/30/20 6:24 PM, Doug Anderson wrote:
> Hi,
> 
> On Wed, Sep 30, 2020 at 8:48 AM Rajendra Nayak <rnayak@codeaurora.org> wrote:
>>
>>
>> On 9/30/2020 7:34 PM, Lukasz Luba wrote:
>>>
>>>
>>> On 9/30/20 11:55 AM, Rajendra Nayak wrote:
>>>>
>>>> On 9/30/2020 1:55 PM, Lukasz Luba wrote:
>>>>> Hi Douglas,
>>>>>
>>>>> On 9/30/20 12:53 AM, Doug Anderson wrote:
>>>>>> Hi,
>>>>>>
>>>>>> On Tue, Sep 29, 2020 at 5:16 AM Lukasz Luba <lukasz.luba@arm.com> wrote:
>>>>>>>
>>>>>>> The Energy Model (EM) can store power values in milli-Watts or in abstract
>>>>>>> scale. This might cause issues in the subsystems which use the EM for
>>>>>>> estimating the device power, such as:
>>>>>>> - mixing of different scales in a subsystem which uses multiple
>>>>>>>     (cooling) devices (e.g. thermal Intelligent Power Allocation (IPA))
>>>>>>> - assuming that energy [milli-Joules] can be derived from the EM power
>>>>>>>     values which might not be possible since the power scale doesn't have to
>>>>>>>     be in milli-Watts
>>>>>>>
>>>>>>> To avoid misconfiguration add the needed documentation to the EM and
>>>>>>> related subsystems: EAS and IPA.
>>>>>>>
>>>>>>> Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
>>>>>>> ---
>>>>>>>    .../driver-api/thermal/power_allocator.rst          |  8 ++++++++
>>>>>>>    Documentation/power/energy-model.rst                | 13 +++++++++++++
>>>>>>>    Documentation/scheduler/sched-energy.rst            |  5 +++++
>>>>>>>    3 files changed, 26 insertions(+)
>>>>>>
>>>>>> I haven't read through these files in massive detail, but the quick
>>>>>> skim makes me believe that your additions seem sane.  In general, I'm
>>>>>> happy with documenting reality, thus:
>>>>>>
>>>>>> Reviewed-by: Douglas Anderson <dianders@chromium.org>
>>>>>
>>>>> Thank you for the review.
>>>>>
>>>>>>
>>>>>> I will note: you haven't actually updated the device tree bindings.
>>>>>> Thus, presumably, anyone who is specifying these numbers in the device
>>>>>> tree is still supposed to specify them in a way that mW can be
>>>>>> recovered, right?  Said another way: nothing about your patches makes
>>>>>> it OK to specify numbers in device trees using an "abstract scale",
>>>>>> right?
>>>>>
>>>>> For completeness, we are talking here about the binding from:
>>>>> Documentation/devicetree/bindings/arm/cpus.yaml
>>>>> which is 'dynamic-power-coefficient'. Yes, it stays untouched, also the
>>>>> unit (uW/MHz/V^2) which then allows to have mW in the power
>>>>> values in the EM.
>>>>
>>>> So for platforms where 'dynamic-power-coefficient' is specified in device tree,
>>>> its always expected to be derived from 'real' power numbers on these platforms in
>>>> 'real' mW?
>>>
>>> Yes, the purpose and the name of that binding was only for 'real'
>>> power in mW.
>>>
>>>>
>>>> Atleast on Qualcomm platforms we have these numbers scaled, so in essence it
>>>> can't be used to derive 'real' mW values. That said we also do not have any of
>>>> the 'platform might face potential issue of mixing devices in one thermal zone
>>>> of two scales' problem.
>>>
>>> If you have these numbers scaled, then it's probably documented
>>> somewhere in your docs for your OEMs, because they might assume it's in
>>> uW/MHz/V^2 (according to the bindings doc). If not, they probably
>>> realized it during the measurements and comparison (that the power in
>>> EM is not what they see on the power meter).
>>> This binding actually helps those developers who take the experiments
>>> and based on measured power values, store derived coefficient.
>>> Everyone can just measure in local setup and compare the results
>>> easily, speaking the same language (proposing maybe a patch adjusting
>>> the value in DT).
>>>
>>>>
>>>> So the question is, can such platforms still use 'dynamic-power-coefficient'
>>>> in device tree and create an abstract scale? The other way of doing this would
>>>> be to *not* specify this value in device tree and have these values stored in the
>>>> cpufreq driver and register a custom callback to do the math.
>>>
>>> But then we would also have to change the name of that binding.
>>>
>>> I'd recommend you the second way that you've described. It will avoid
>>> your OEMs confusion. In your cpufreq driver you can simply register
>>> to EM using the em_dev_register_perf_domain(). In your local
>>> callback you can do whatever you need (read driver array, firmware,
>>> DT, scale or not, etc).
>>> The helper code in dev_pm_opp_of_register_em() is probably not suited
>>> for your use case (when you don't want to share the real power of the
>>> SoC).
>>
>> Got it, thanks for the clarification. I will get the cpufreq driver updated
>> to use em_dev_register_perf_domain() with a custom callback and get rid of these
>> values from device tree.
> 
> This sounds good.  ...except...
> 
> How exactly are boards supposed to provide their "sustainable-power"
> number in this model?  As far as I'm aware, there's no place to
> specify this board-specific file other than in device tree, and the
> bindings [1] say that this value has to be in mW.  Lukasz: how do you
> envision boards can provide "sustainable-power" in cases where the
> energy model is in "abstract scale"?
> 
> [1] Documentation/devicetree/bindings/thermal/thermal-zones.yaml
> 


I am currently investigating this issue. I will keep you in CC list
when I send some patches.

Regards,
Lukasz