mbox series

[v4,0/4] Clarify abstract scale usage for power values in Energy Model, EAS and IPA

Message ID 20201103090600.29053-1-lukasz.luba@arm.com
Headers show
Series Clarify abstract scale usage for power values in Energy Model, EAS and IPA | expand

Message

Lukasz Luba Nov. 3, 2020, 9:05 a.m. UTC
Hi all,

The Energy Model supports power values expressed in an abstract scale.
This has an impact on Intelligent Power Allocation (IPA) and should be
documented properly. Kernel sub-systems like EAS, IPA and DTPM
(new comming PowerCap framework) would use the new flag to capture
potential miss-configuration where the devices have registered different
power scales, thus cannot operate together.

There was a discussion below v2 of this patch series, which might help
you to get context of these changes [2].

The agreed approach is to have the DT as a source of power values expressed
always in milli-Watts and the only way to submit with abstract scale values
is via the em_dev_register_perf_domain() API.

Changes:
v4:
- change bool to int type for 'miliwatts' in struct em_perf_domain
  (suggested by Quentin)
- removed one sentence from patch 2/4 in IPA doc power_allocator.rst
  (suggested by Quentin)
- added reviewed-by from Quentin to 1/4, 3/4, 4/4 patches
v3 [3]:
- added boolean flag to struct em_perf_domain and registration function
  indicating if EM holds real power values in milli-Watts (suggested by
  Daniel and aggreed with Quentin)
- updated documentation regarding this new flag
- dropped DT binding change for 'sustainable-power'
- added more maintainers on CC (due to patch 1/3 touching different things)
v2 [2]:
- updated sustainable power section in IPA documentation
- updated DT binding for the 'sustainable-power'
v1 [1]:
- simple documenation update with new 'abstract scale' in EAS, EM, IPA

Regards,
Lukasz Luba

[1] https://lore.kernel.org/linux-doc/20200929121610.16060-1-lukasz.luba@arm.com/
[2] https://lore.kernel.org/lkml/20201002114426.31277-1-lukasz.luba@arm.com/
[3] https://lore.kernel.org/lkml/20201019140601.3047-1-lukasz.luba@arm.com/

Lukasz Luba (4):
  PM / EM: Add a flag indicating units of power values in Energy Model
  docs: Clarify abstract scale usage for power values in Energy Model
  PM / EM: update the comments related to power scale
  docs: power: Update Energy Model with new flag indicating power scale

 .../driver-api/thermal/power_allocator.rst    | 12 +++++++-
 Documentation/power/energy-model.rst          | 30 +++++++++++++++----
 Documentation/scheduler/sched-energy.rst      |  5 ++++
 drivers/cpufreq/scmi-cpufreq.c                |  3 +-
 drivers/opp/of.c                              |  2 +-
 include/linux/energy_model.h                  | 20 ++++++++-----
 kernel/power/energy_model.c                   | 26 ++++++++++++++--
 7 files changed, 80 insertions(+), 18 deletions(-)

Comments

Morten Rasmussen Nov. 5, 2020, 9:18 a.m. UTC | #1
On Tue, Nov 03, 2020 at 09:05:57AM +0000, Lukasz Luba wrote:
> @@ -79,7 +82,8 @@ struct em_data_callback {
>  struct em_perf_domain *em_cpu_get(int cpu);
>  struct em_perf_domain *em_pd_get(struct device *dev);
>  int em_dev_register_perf_domain(struct device *dev, unsigned int nr_states,
> -				struct em_data_callback *cb, cpumask_t *span);
> +				struct em_data_callback *cb, cpumask_t *spani,

"spani" looks like a typo?
Morten Rasmussen Nov. 5, 2020, 10:56 a.m. UTC | #2
On Thu, Nov 05, 2020 at 10:09:05AM +0000, Lukasz Luba wrote:
> 
> 
> On 11/5/20 9:18 AM, Morten Rasmussen wrote:
> > On Tue, Nov 03, 2020 at 09:05:57AM +0000, Lukasz Luba wrote:
> > > @@ -79,7 +82,8 @@ struct em_data_callback {
> > >   struct em_perf_domain *em_cpu_get(int cpu);
> > >   struct em_perf_domain *em_pd_get(struct device *dev);
> > >   int em_dev_register_perf_domain(struct device *dev, unsigned int nr_states,
> > > -				struct em_data_callback *cb, cpumask_t *span);
> > > +				struct em_data_callback *cb, cpumask_t *spani,
> > 
> > "spani" looks like a typo?
> > 
> 
> Good catch, yes, the vim 'i'.
> 
> Thank you Morten. I will resend this patch when you don't
> find other issues in the rest of patches.

The rest of the series looks okay to me.

Morten
Lukasz Luba Nov. 5, 2020, 12:55 p.m. UTC | #3
On 11/5/20 10:56 AM, Morten Rasmussen wrote:
> On Thu, Nov 05, 2020 at 10:09:05AM +0000, Lukasz Luba wrote:
>>
>>
>> On 11/5/20 9:18 AM, Morten Rasmussen wrote:
>>> On Tue, Nov 03, 2020 at 09:05:57AM +0000, Lukasz Luba wrote:
>>>> @@ -79,7 +82,8 @@ struct em_data_callback {
>>>>    struct em_perf_domain *em_cpu_get(int cpu);
>>>>    struct em_perf_domain *em_pd_get(struct device *dev);
>>>>    int em_dev_register_perf_domain(struct device *dev, unsigned int nr_states,
>>>> -				struct em_data_callback *cb, cpumask_t *span);
>>>> +				struct em_data_callback *cb, cpumask_t *spani,
>>>
>>> "spani" looks like a typo?
>>>
>>
>> Good catch, yes, the vim 'i'.
>>
>> Thank you Morten. I will resend this patch when you don't
>> find other issues in the rest of patches.
> 
> The rest of the series looks okay to me.

Thank you for checking the whole series. I have re-sent this patch only.

Lukasz

> 
> Morten
>
Rafael J. Wysocki Nov. 10, 2020, 7:32 p.m. UTC | #4
On Wed, Nov 4, 2020 at 11:58 AM Lukasz Luba <lukasz.luba@arm.com> wrote:
>

> Hi Rafael,

>

> On 11/3/20 9:05 AM, Lukasz Luba wrote:

> > Hi all,

> >

> > The Energy Model supports power values expressed in an abstract scale.

> > This has an impact on Intelligent Power Allocation (IPA) and should be

> > documented properly. Kernel sub-systems like EAS, IPA and DTPM

> > (new comming PowerCap framework) would use the new flag to capture

> > potential miss-configuration where the devices have registered different

> > power scales, thus cannot operate together.

> >

> > There was a discussion below v2 of this patch series, which might help

> > you to get context of these changes [2].

> >

> > The agreed approach is to have the DT as a source of power values expressed

> > always in milli-Watts and the only way to submit with abstract scale values

> > is via the em_dev_register_perf_domain() API.

> >

> > Changes:

> > v4:

> > - change bool to int type for 'miliwatts' in struct em_perf_domain

> >    (suggested by Quentin)

> > - removed one sentence from patch 2/4 in IPA doc power_allocator.rst

> >    (suggested by Quentin)

> > - added reviewed-by from Quentin to 1/4, 3/4, 4/4 patches

>

> There was no major objections in the v3 and this v4 just addressed

> minor comments. The important discussions mostly happen in v2.

>

> Could you take the patches via your tree, please?


Applied as 5.11 material, thanks!
Lukasz Luba Nov. 17, 2020, 9:24 a.m. UTC | #5
On 11/10/20 7:32 PM, Rafael J. Wysocki wrote:
> On Wed, Nov 4, 2020 at 11:58 AM Lukasz Luba <lukasz.luba@arm.com> wrote:
>>
>> Hi Rafael,
>>
>> On 11/3/20 9:05 AM, Lukasz Luba wrote:
>>> Hi all,
>>>
>>> The Energy Model supports power values expressed in an abstract scale.
>>> This has an impact on Intelligent Power Allocation (IPA) and should be
>>> documented properly. Kernel sub-systems like EAS, IPA and DTPM
>>> (new comming PowerCap framework) would use the new flag to capture
>>> potential miss-configuration where the devices have registered different
>>> power scales, thus cannot operate together.
>>>
>>> There was a discussion below v2 of this patch series, which might help
>>> you to get context of these changes [2].
>>>
>>> The agreed approach is to have the DT as a source of power values expressed
>>> always in milli-Watts and the only way to submit with abstract scale values
>>> is via the em_dev_register_perf_domain() API.
>>>
>>> Changes:
>>> v4:
>>> - change bool to int type for 'miliwatts' in struct em_perf_domain
>>>     (suggested by Quentin)
>>> - removed one sentence from patch 2/4 in IPA doc power_allocator.rst
>>>     (suggested by Quentin)
>>> - added reviewed-by from Quentin to 1/4, 3/4, 4/4 patches
>>
>> There was no major objections in the v3 and this v4 just addressed
>> minor comments. The important discussions mostly happen in v2.
>>
>> Could you take the patches via your tree, please?
> 
> Applied as 5.11 material, thanks!
> 

Thank you Rafael!

Regards,
Lukasz