| Message ID | 20201002114426.31277-1-lukasz.luba@arm.com |
|---|---|
| Headers | show |
| Series | Clarify abstract scale usage for power values in Energy Model, EAS and IPA | expand |
Hi Lukasz, On 09/10/2020 11:16, Lukasz Luba wrote: > Hi Rafael, > > On 10/2/20 12:44 PM, 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. There is also a need to update the DT binding for >> the >> 'sustainable-power' and allow it to have abstract scale as well. >> >> Changes: >> v2: >> - updated sustainable power section in IPA documentation >> - updated DT binding for the 'sustainable-power' >> >> The v1 of the patch set and related discussion can be found in [1]. >> >> Regards, >> Lukasz Luba >> >> [1] >> https://lore.kernel.org/linux-doc/20200929121610.16060-1-lukasz.luba@arm.com/ >> >> >> Lukasz Luba (3): >> docs: Clarify abstract scale usage for power values in Energy Model >> PM / EM: update the comments related to power scale >> dt-bindings: thermal: update sustainable-power with abstract scale >> >> .../devicetree/bindings/thermal/thermal-zones.yaml | 13 +++++++++---- >> .../driver-api/thermal/power_allocator.rst | 13 ++++++++++++- >> Documentation/power/energy-model.rst | 13 +++++++++++++ >> Documentation/scheduler/sched-energy.rst | 5 +++++ >> include/linux/energy_model.h | 11 +++++------ >> kernel/power/energy_model.c | 2 +- >> 6 files changed, 45 insertions(+), 12 deletions(-) >> > > Could you take patch 1/3 and patch 2/3 via your PM tree, > please? I will be very grateful. > > These patches just update the documentation and comments regarding > an issue that we can have: bogoWatts in the Energy Model (and we > already have). One of the drawbacks is that we cannot derive real energy > from these numbers. Will see how this would evolve. The purpose of the energy model is to provide these power numbers. If the SoC vendors do not want to share those numbers, then better to not use the energy model at all. If they want to use the EAS and the IPA at all costs without sharing the power numbers, then it is up to them to take responsibility of providing consistent numbers, not the community to document how to hack the energy model. And that is even more true as mentioned by Doug: the power numbers are not impossible to measure. Documenting the scale values give the opportunity to the SoC vendor to never share the power numbers, and even worst, that implies all the existing and future frameworks based on the energy model (and its evolution) *must* comply with these dummy values. That is the promise of a real pain. IMO, we must keep a strong constraint on the power values for the energy model. However, nothing prevents to write a recipe on a website explaining how to use the energy model without the power numbers with a big warning that could not work in the future if the energy model evolves or it could be incompatible with the IPA. I suggest to solve the energy model main issue: the SoC vendor do not want to share the power numbers. Why not give the opportunity to load a firmware where the power numbers will be ? The firmware could be in a vendor partition for example.
Hi Daniel, On 10/14/20 9:22 AM, Daniel Lezcano wrote: > > Hi Lukasz, > > On 09/10/2020 11:16, Lukasz Luba wrote: >> Hi Rafael, >> >> On 10/2/20 12:44 PM, 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. There is also a need to update the DT binding for >>> the >>> 'sustainable-power' and allow it to have abstract scale as well. >>> >>> Changes: >>> v2: >>> - updated sustainable power section in IPA documentation >>> - updated DT binding for the 'sustainable-power' >>> >>> The v1 of the patch set and related discussion can be found in [1]. >>> >>> Regards, >>> Lukasz Luba >>> >>> [1] >>> https://lore.kernel.org/linux-doc/20200929121610.16060-1-lukasz.luba@arm.com/ >>> >>> >>> Lukasz Luba (3): >>> docs: Clarify abstract scale usage for power values in Energy Model >>> PM / EM: update the comments related to power scale >>> dt-bindings: thermal: update sustainable-power with abstract scale >>> >>> .../devicetree/bindings/thermal/thermal-zones.yaml | 13 +++++++++---- >>> .../driver-api/thermal/power_allocator.rst | 13 ++++++++++++- >>> Documentation/power/energy-model.rst | 13 +++++++++++++ >>> Documentation/scheduler/sched-energy.rst | 5 +++++ >>> include/linux/energy_model.h | 11 +++++------ >>> kernel/power/energy_model.c | 2 +- >>> 6 files changed, 45 insertions(+), 12 deletions(-) >>> >> >> Could you take patch 1/3 and patch 2/3 via your PM tree, >> please? I will be very grateful. >> >> These patches just update the documentation and comments regarding >> an issue that we can have: bogoWatts in the Energy Model (and we >> already have). One of the drawbacks is that we cannot derive real energy >> from these numbers. Will see how this would evolve. > > The purpose of the energy model is to provide these power numbers. > > If the SoC vendors do not want to share those numbers, then better to > not use the energy model at all. > > If they want to use the EAS and the IPA at all costs without sharing the > power numbers, then it is up to them to take responsibility of providing > consistent numbers, not the community to document how to hack the energy > model. > > And that is even more true as mentioned by Doug: the power numbers are > not impossible to measure. > > Documenting the scale values give the opportunity to the SoC vendor to > never share the power numbers, and even worst, that implies all the > existing and future frameworks based on the energy model (and its > evolution) *must* comply with these dummy values. That is the promise of > a real pain. > > IMO, we must keep a strong constraint on the power values for the energy > model. > > However, nothing prevents to write a recipe on a website explaining how > to use the energy model without the power numbers with a big warning > that could not work in the future if the energy model evolves or it > could be incompatible with the IPA. > > I suggest to solve the energy model main issue: the SoC vendor do not > want to share the power numbers. Why not give the opportunity to load a > firmware where the power numbers will be ? The firmware could be in a > vendor partition for example. > > I understand your concerns. Unfortunately, the reality is that the bogoWatts are there. I had discussion about it a few days ago with Rajendra and Doug [1], where I was also opposed to allow bogoValue coming from DT 'dynamic-power-coefficient'. But I have discussed it internally and we allow, because developers would do it anyway. Regarding your question with firmware where the power numbers can be stored. Unfortunately, it is quite opposite, FW might want to hide it. We even allow bogoWatts to come from firmware, the SCMI spec: (4.5.1 Performance domain management protocol background) 'The power can be expressed in mW or in an abstract scale. Vendors are not obliged to reveal power costs if it is undesirable, but a linear scale is required.' The callback which does this is not able to check if the value is a bogoWatt [2]. EAS can handle EM with bogoWatts, as I described in the patch. IPA has some issues: 'sustainable-power' in DT (which shouldn't be used when EM devices use abstract scale) but sysfs interface can be used. This patch set just align the SCMI spec with EM, EAS, IPA documentation and already present platforms which use it. I hope that the real milliWatts would come to EM via the DT 'dynamic-power-coefficient' and function dev_pm_opp_of_register_em(). But no guaranties as you can see in [1]. Regards, Lukasz [1] https://lore.kernel.org/linux-pm/62540312-65a2-b6d9-86ce-b4deaaa913c1@codeaurora.org/ [2] https://elixir.bootlin.com/linux/v5.9/source/drivers/cpufreq/scmi-cpufreq.c#L118
On 14/10/2020 11:08, Lukasz Luba wrote: > Hi Daniel, > > On 10/14/20 9:22 AM, Daniel Lezcano wrote: >> >> Hi Lukasz, >> >> On 09/10/2020 11:16, Lukasz Luba wrote: >>> Hi Rafael, >>> >>> On 10/2/20 12:44 PM, 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. There is also a need to update the DT binding for >>>> the >>>> 'sustainable-power' and allow it to have abstract scale as well. >>>> >>>> Changes: >>>> v2: >>>> - updated sustainable power section in IPA documentation >>>> - updated DT binding for the 'sustainable-power' >>>> >>>> The v1 of the patch set and related discussion can be found in [1]. >>>> >>>> Regards, >>>> Lukasz Luba >>>> >>>> [1] >>>> https://lore.kernel.org/linux-doc/20200929121610.16060-1-lukasz.luba@arm.com/ >>>> >>>> >>>> >>>> Lukasz Luba (3): >>>> docs: Clarify abstract scale usage for power values in Energy Model >>>> PM / EM: update the comments related to power scale >>>> dt-bindings: thermal: update sustainable-power with abstract scale >>>> >>>> .../devicetree/bindings/thermal/thermal-zones.yaml | 13 >>>> +++++++++---- >>>> .../driver-api/thermal/power_allocator.rst | 13 >>>> ++++++++++++- >>>> Documentation/power/energy-model.rst | 13 >>>> +++++++++++++ >>>> Documentation/scheduler/sched-energy.rst | 5 +++++ >>>> include/linux/energy_model.h | 11 +++++------ >>>> kernel/power/energy_model.c | 2 +- >>>> 6 files changed, 45 insertions(+), 12 deletions(-) >>>> >>> >>> Could you take patch 1/3 and patch 2/3 via your PM tree, >>> please? I will be very grateful. >>> >>> These patches just update the documentation and comments regarding >>> an issue that we can have: bogoWatts in the Energy Model (and we >>> already have). One of the drawbacks is that we cannot derive real energy >>> from these numbers. Will see how this would evolve. >> >> The purpose of the energy model is to provide these power numbers. >> >> If the SoC vendors do not want to share those numbers, then better to >> not use the energy model at all. >> >> If they want to use the EAS and the IPA at all costs without sharing the >> power numbers, then it is up to them to take responsibility of providing >> consistent numbers, not the community to document how to hack the energy >> model. >> >> And that is even more true as mentioned by Doug: the power numbers are >> not impossible to measure. >> >> Documenting the scale values give the opportunity to the SoC vendor to >> never share the power numbers, and even worst, that implies all the >> existing and future frameworks based on the energy model (and its >> evolution) *must* comply with these dummy values. That is the promise of >> a real pain. >> >> IMO, we must keep a strong constraint on the power values for the energy >> model. >> >> However, nothing prevents to write a recipe on a website explaining how >> to use the energy model without the power numbers with a big warning >> that could not work in the future if the energy model evolves or it >> could be incompatible with the IPA. >> >> I suggest to solve the energy model main issue: the SoC vendor do not >> want to share the power numbers. Why not give the opportunity to load a >> firmware where the power numbers will be ? The firmware could be in a >> vendor partition for example. >> >> > > I understand your concerns. Unfortunately, the reality is that the > bogoWatts are there. I had discussion about it a few days ago with > Rajendra and Doug [1], where I was also opposed to allow bogoValue > coming from DT 'dynamic-power-coefficient'. But I have discussed it > internally and we allow, because developers would do it anyway. With all respects, 'internal discussions' is like out of tree kernels: for the community, they don't exist :) The development process in opensource is, by essence, public. That said, if the developers want to use abstract values, up to them to make sure it is consistent with the existing framework. Why do you need to document that and involve the community responsibility by adding these information in the Documentation even if you know different frameworks could be incompatible ? > Regarding your question with firmware where the power numbers can be > stored. Unfortunately, it is quite opposite, FW might want to hide it. No, I meant a firmware file, called by request_firmware(). So instead of having the power numbers in the DT, so published with the kernel, they are in the SoC vendor's partition in the firmware file, like 'energy_model.bin'. Then when the energy model is initialized, it will try to request an energy model firmware file. That gives the opportunity to the SoC vendor to put the power numbers in the file and distribute it with the product. > We even allow bogoWatts to come from firmware, the SCMI spec: > (4.5.1 Performance domain management protocol background) > 'The power can be expressed in mW or in an abstract scale. Vendors are > not obliged to reveal power costs if it is undesirable, but a linear > scale is required.' > The callback which does this is not able to check if the value is a > bogoWatt [2]. So the definition is clear: '... linear scale is required'. So that implies *all* power numbers for all devices defined in the SCMI. It is up to the SoC vendor to provide the right numbers. The EM / IPA / EAS do not have to care about the values. > EAS can handle EM with bogoWatts, as I described in the patch. > IPA has some issues: 'sustainable-power' in DT (which shouldn't be used > when EM devices use abstract scale) but sysfs interface can be used. Here the platform is mixing the numbers from different firmwares with different units. Why not make things consistent ? If the power numbers are coming from the SCMI, then ignore the ones coming from the DT, no? That should be simpler now that we have the energy model used for devfreq and cpufreq. May be add a flag in the energy model giving the origin of the data? > This patch set just align the SCMI spec with EM, EAS, IPA > documentation and already present platforms which use it. Actually, it is the opposite, the patch aligns EAS and EM to the SCMI spec, but we end up with IPA based on the EM/SCMI & DT and EAS based on EM/SCMI, right ? That is the root cause of the incompatibility. > I hope that the real milliWatts would come to EM via the DT > 'dynamic-power-coefficient' and function dev_pm_opp_of_register_em(). > But no guaranties as you can see in [1]. It is not a kernel problem if inconsistent values are specified in the DT. May be make developer life easier by submitting a script which will take a device tree, check all power numbers, and consistently abstract them. The developer will write the real values in the DT, test everything is working fine, then run the script which will make the 'linear scale' of all the power numbers and convert them to bogoWatt (with different properties name, so watt and bogowatt mix can be detected). In any case, if the DT is specifying real numbers, and SCMI abstract numbers or the opposite, obviously there is a conflict if we are using both. I suggest to fix the conflict first and provide the features to make the numbers more easy to share (like the script described above and/or the firmware file). Then with the right tools, everything can be documented.
On 10/14/20 12:23 PM, Daniel Lezcano wrote: > On 14/10/2020 11:08, Lukasz Luba wrote: >> Hi Daniel, >> >> On 10/14/20 9:22 AM, Daniel Lezcano wrote: >>> >>> Hi Lukasz, >>> >>> On 09/10/2020 11:16, Lukasz Luba wrote: >>>> Hi Rafael, >>>> >>>> On 10/2/20 12:44 PM, 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. There is also a need to update the DT binding for >>>>> the >>>>> 'sustainable-power' and allow it to have abstract scale as well. >>>>> >>>>> Changes: >>>>> v2: >>>>> - updated sustainable power section in IPA documentation >>>>> - updated DT binding for the 'sustainable-power' >>>>> >>>>> The v1 of the patch set and related discussion can be found in [1]. >>>>> >>>>> Regards, >>>>> Lukasz Luba >>>>> >>>>> [1] >>>>> https://lore.kernel.org/linux-doc/20200929121610.16060-1-lukasz.luba@arm.com/ >>>>> >>>>> >>>>> >>>>> Lukasz Luba (3): >>>>> docs: Clarify abstract scale usage for power values in Energy Model >>>>> PM / EM: update the comments related to power scale >>>>> dt-bindings: thermal: update sustainable-power with abstract scale >>>>> >>>>> .../devicetree/bindings/thermal/thermal-zones.yaml | 13 >>>>> +++++++++---- >>>>> .../driver-api/thermal/power_allocator.rst | 13 >>>>> ++++++++++++- >>>>> Documentation/power/energy-model.rst | 13 >>>>> +++++++++++++ >>>>> Documentation/scheduler/sched-energy.rst | 5 +++++ >>>>> include/linux/energy_model.h | 11 +++++------ >>>>> kernel/power/energy_model.c | 2 +- >>>>> 6 files changed, 45 insertions(+), 12 deletions(-) >>>>> >>>> >>>> Could you take patch 1/3 and patch 2/3 via your PM tree, >>>> please? I will be very grateful. >>>> >>>> These patches just update the documentation and comments regarding >>>> an issue that we can have: bogoWatts in the Energy Model (and we >>>> already have). One of the drawbacks is that we cannot derive real energy >>>> from these numbers. Will see how this would evolve. >>> >>> The purpose of the energy model is to provide these power numbers. >>> >>> If the SoC vendors do not want to share those numbers, then better to >>> not use the energy model at all. >>> >>> If they want to use the EAS and the IPA at all costs without sharing the >>> power numbers, then it is up to them to take responsibility of providing >>> consistent numbers, not the community to document how to hack the energy >>> model. >>> >>> And that is even more true as mentioned by Doug: the power numbers are >>> not impossible to measure. >>> >>> Documenting the scale values give the opportunity to the SoC vendor to >>> never share the power numbers, and even worst, that implies all the >>> existing and future frameworks based on the energy model (and its >>> evolution) *must* comply with these dummy values. That is the promise of >>> a real pain. >>> >>> IMO, we must keep a strong constraint on the power values for the energy >>> model. >>> >>> However, nothing prevents to write a recipe on a website explaining how >>> to use the energy model without the power numbers with a big warning >>> that could not work in the future if the energy model evolves or it >>> could be incompatible with the IPA. >>> >>> I suggest to solve the energy model main issue: the SoC vendor do not >>> want to share the power numbers. Why not give the opportunity to load a >>> firmware where the power numbers will be ? The firmware could be in a >>> vendor partition for example. >>> >>> >> >> I understand your concerns. Unfortunately, the reality is that the >> bogoWatts are there. I had discussion about it a few days ago with >> Rajendra and Doug [1], where I was also opposed to allow bogoValue >> coming from DT 'dynamic-power-coefficient'. But I have discussed it >> internally and we allow, because developers would do it anyway. > > With all respects, 'internal discussions' is like out of tree kernels: > for the community, they don't exist :) > > The development process in opensource is, by essence, public. Please don't get me wrong. I just wanted to say that this issue has been discussed in Arm Power team and presented here in public (and it's not my private view - but the team) responding to community questions. > > That said, if the developers want to use abstract values, up to them to > make sure it is consistent with the existing framework. > > Why do you need to document that and involve the community > responsibility by adding these information in the Documentation even if > you know different frameworks could be incompatible ? The community asked for better documentation [1]. 'I believe relative values work perfectly fine for scheduling decisions' Rajendra (who was right, EAS works fine, but it is not documented) It started from the discussion there, then discussion on v1 of this patch set. So it's the Community who wanted to know more information about 'real' vs 'abstract' scale for power and the EM registration process. They made a point and I had to consult it with my team. > > >> Regarding your question with firmware where the power numbers can be >> stored. Unfortunately, it is quite opposite, FW might want to hide it. > > No, I meant a firmware file, called by request_firmware(). So instead of > having the power numbers in the DT, so published with the kernel, they > are in the SoC vendor's partition in the firmware file, like > 'energy_model.bin'. > > Then when the energy model is initialized, it will try to request an > energy model firmware file. > > That gives the opportunity to the SoC vendor to put the power numbers in > the file and distribute it with the product. Yes, but it will not protect EM from vendors writing bogoWatts in there. > >> We even allow bogoWatts to come from firmware, the SCMI spec: >> (4.5.1 Performance domain management protocol background) >> 'The power can be expressed in mW or in an abstract scale. Vendors are >> not obliged to reveal power costs if it is undesirable, but a linear >> scale is required.' >> The callback which does this is not able to check if the value is a >> bogoWatt [2]. > > So the definition is clear: '... linear scale is required'. So that > implies *all* power numbers for all devices defined in the SCMI. It is > up to the SoC vendor to provide the right numbers. Yes, we wanted to add this to the documentation to make it clear. > > The EM / IPA / EAS do not have to care about the values. Community developers are asking about it: if IPA or EAS can handle it. Thus this documentation update. > >> EAS can handle EM with bogoWatts, as I described in the patch. >> IPA has some issues: 'sustainable-power' in DT (which shouldn't be used >> when EM devices use abstract scale) but sysfs interface can be used. > > Here the platform is mixing the numbers from different firmwares with > different units. > > Why not make things consistent ? If the power numbers are coming from > the SCMI, then ignore the ones coming from the DT, no? That should be > simpler now that we have the energy model used for devfreq and cpufreq. The numbers coming from SCMI might be also real mW, the spec allows to be either milliWatts or bogoWatts. So we cannot block SCMI in your example. > > May be add a flag in the energy model giving the origin of the data? I think the flag could cause wrong assumptions. > >> This patch set just align the SCMI spec with EM, EAS, IPA >> documentation and already present platforms which use it. > > Actually, it is the opposite, the patch aligns EAS and EM to the SCMI > spec, but we end up with IPA based on the EM/SCMI & DT and EAS based on > EM/SCMI, right ? Yes, correct, it aligns EAS, EM with SCMI spec and also community driver. It makes it clear that EM, EAS allows abstract scale for all drivers not only SCMI (like Qcom with values not exposing sensitive information (milliWatts)). > > That is the root cause of the incompatibility. The patch 3/3 from this series is dropped. Just the IPA documentation is going to be updated (patch 1/3). It should be, because in EM abstract scale is allowed and we use EM in IPA. DT binding for 'sustainable-power' stays as is. > >> I hope that the real milliWatts would come to EM via the DT >> 'dynamic-power-coefficient' and function dev_pm_opp_of_register_em(). >> But no guaranties as you can see in [1]. > > It is not a kernel problem if inconsistent values are specified in the DT. > > May be make developer life easier by submitting a script which will take > a device tree, check all power numbers, and consistently abstract them. > The developer will write the real values in the DT, test everything is > working fine, then run the script which will make the 'linear scale' of > all the power numbers and convert them to bogoWatt (with different > properties name, so watt and bogowatt mix can be detected). Do you mean a new property in DT? Which would handle the bogoWatts? IMHO DT describes only the HW, using it as a helper for SW abstractions abuses it. It already has these 'sust. power', 'dynamic-power-coeff.' and 'capacity-dmips-mhz'. I think Rob will also be against another stuff fixing our 'generic' software. We have to update the EM doc about allowed abstract scale, which implies EAS, IPA doc update with some information to the community that these components can handle it. The script will just make developers life easier, but the current documentation does not say anything about abstract scale. > > In any case, if the DT is specifying real numbers, and SCMI abstract > numbers or the opposite, obviously there is a conflict if we are using both. True, DT only allows real numbers (I have Rob's opinion regarding patch 3/3). It's not that there is only SCMI which might use abstract scale. Qcom already has it and other vendors will follow (not exposing real numbers). They would register bogoWatts to EM because they know that EAS can deal with both. > > I suggest to fix the conflict first and provide the features to make the > numbers more easy to share (like the script described above and/or the > firmware file). > > Then with the right tools, everything can be documented. > We cannot block one way of registration to EM when the other was used. They might have correct and consistent numbers. It's up to the platform developers to choose the path: - go with bogoWatts - if they are not allowed to expose sensitive information, use em_dev_register_perf_domain() in drivers, not DT; make sure everything that is needed works; check the doc, which sub-systems can handle it or needs some tuning (patches 1/3 and 2/3 try to help here); - use milliWatts - easier; DT is allowed; help from the community in reviews, possible results comparisons; both EM registration ways might be used; We cannot force vendors/OEM engineers to store milliWatts in the Energy Model if these values are protected by some NDA. Your proposed way of providing data into EM from user-space firmware.bin IMHO also falls into the same bucket. That information would be accessible in EM debugfs and they would avoid it. Regards, Lukasz [1] https://lore.kernel.org/linux-devicetree/248bb01e-1746-c84c-78c4-3cf7d2541a70@codeaurora.org/
On 14/10/2020 17:24, Lukasz Luba wrote: [ ... ] > We have to update the EM doc about allowed abstract scale, which > implies EAS, IPA doc update with some information to the community that > these components can handle it. > > The script will just make developers life easier, but the current > documentation does not say anything about abstract scale. ... yes, because there is no consistency across the source of power numbers and no tools to ensure DT power numbers consistency, yet. >> In any case, if the DT is specifying real numbers, and SCMI abstract >> numbers or the opposite, obviously there is a conflict if we are using >> both. > > True, DT only allows real numbers (I have Rob's opinion regarding > patch 3/3). > > It's not that there is only SCMI which might use abstract scale. Qcom > already has it and other vendors will follow (not exposing real > numbers). They would register bogoWatts to EM because they know that EAS > can deal with both. So vendors are using bogoWatts, despite the documentation. By updating the documentation saying it supports the abstract values, that means every new framework, device with power values, will have to comply with that. How is it possible to add a device with power numbers if the existing ones are obfuscated ? With two subsystems using the energy model, evolving independently we can see there are conflicts. With more subsystems, that may become a source of confusion, especially with different contributors. I think the energy model should stick to milliwatts and keep the documentation unchanged regarding this. And vendors should take the responsibility of not sticking to the documentation. >> I suggest to fix the conflict first and provide the features to make the >> numbers more easy to share (like the script described above and/or the >> firmware file). >> >> Then with the right tools, everything can be documented. >> > > We cannot block one way of registration to EM when the other was used. > They might have correct and consistent numbers. What is the rational of using two firmware power information ? > It's up to the platform developers to choose the path: > - go with bogoWatts - if they are not allowed to expose sensitive > information, use em_dev_register_perf_domain() in drivers, not DT; > make sure everything that is needed works; check the doc, which > sub-systems can handle it or needs some tuning (patches 1/3 and 2/3 > try to help here); > - use milliWatts - easier; DT is allowed; help from the community in > reviews, possible results comparisons; both EM registration ways > might be used; > > We cannot force vendors/OEM engineers to store milliWatts in the > Energy Model if these values are protected by some NDA. If I am able to measure one real power value, (and I'm pretty sure it is quite possible), whatever which one, it is possible to deduce all the numbers with the linear scale. IMO that is a false debate. Anyway ... > Your proposed > way of providing data into EM from user-space firmware.bin IMHO also > falls into the same bucket. That information would be accessible in EM > debugfs and they would avoid it. I think you misunderstood my point. There is the SCMI and the DT. Because there are two sources where it is impossible to know if they are using the same units, we are stuck to ensure a consistency for the kernel. The platform should use: - the SCMI only (scaled or real) - the DT only (real) [ - the firmware file only (scaled or real) ] As it is not possible to know if they are scaled or real, there is no choice except making them mutually exclusive.
On 10/14/20 6:10 PM, Daniel Lezcano wrote: > On 14/10/2020 17:24, Lukasz Luba wrote: > > [ ... ] > >> We have to update the EM doc about allowed abstract scale, which >> implies EAS, IPA doc update with some information to the community that >> these components can handle it. >> >> The script will just make developers life easier, but the current >> documentation does not say anything about abstract scale. > > ... yes, because there is no consistency across the source of power > numbers and no tools to ensure DT power numbers consistency, yet. > >>> In any case, if the DT is specifying real numbers, and SCMI abstract >>> numbers or the opposite, obviously there is a conflict if we are using >>> both. >> >> True, DT only allows real numbers (I have Rob's opinion regarding >> patch 3/3). >> >> It's not that there is only SCMI which might use abstract scale. Qcom >> already has it and other vendors will follow (not exposing real >> numbers). They would register bogoWatts to EM because they know that EAS >> can deal with both. > > So vendors are using bogoWatts, despite the documentation. > > By updating the documentation saying it supports the abstract values, > that means every new framework, device with power values, will have to > comply with that. How is it possible to add a device with power numbers > if the existing ones are obfuscated ? Good question. I've been thinking about this Qcom platform. If someone after a while would like to add new device to EM e.g DSP of that SoC, then it has to measure the real power and decode the existing numbers for the CPUs. Then encode the new device power numbers in the same way and register to EM. > > With two subsystems using the energy model, evolving independently we > can see there are conflicts. With more subsystems, that may become a > source of confusion, especially with different contributors. > > I think the energy model should stick to milliwatts and keep the > documentation unchanged regarding this. And vendors should take the > responsibility of not sticking to the documentation. > >>> I suggest to fix the conflict first and provide the features to make the >>> numbers more easy to share (like the script described above and/or the >>> firmware file). >>> >>> Then with the right tools, everything can be documented. >>> >> >> We cannot block one way of registration to EM when the other was used. >> They might have correct and consistent numbers. > > What is the rational of using two firmware power information ? I mean, there is two ways to register into EM: 1) em_dev_register_perf_domain() - can bring milliWatts or bogoWatts 2) dev_pm_opp_of_register_em() - should bring only milliWatts > >> It's up to the platform developers to choose the path: >> - go with bogoWatts - if they are not allowed to expose sensitive >> information, use em_dev_register_perf_domain() in drivers, not DT; >> make sure everything that is needed works; check the doc, which >> sub-systems can handle it or needs some tuning (patches 1/3 and 2/3 >> try to help here); >> - use milliWatts - easier; DT is allowed; help from the community in >> reviews, possible results comparisons; both EM registration ways >> might be used; >> >> We cannot force vendors/OEM engineers to store milliWatts in the >> Energy Model if these values are protected by some NDA. > > If I am able to measure one real power value, (and I'm pretty sure it is > quite possible), whatever which one, it is possible to deduce all the > numbers with the linear scale. IMO that is a false debate. Anyway ... > >> Your proposed >> way of providing data into EM from user-space firmware.bin IMHO also >> falls into the same bucket. That information would be accessible in EM >> debugfs and they would avoid it. > > I think you misunderstood my point. > > There is the SCMI and the DT. Because there are two sources where it is > impossible to know if they are using the same units, we are stuck to > ensure a consistency for the kernel. > > The platform should use: > - the SCMI only (scaled or real) > - the DT only (real) > [ - the firmware file only (scaled or real) ] > Do you mean by SCMI - registration using em_dev_register_perf_domain() ? > > As it is not possible to know if they are scaled or real, there is no > choice except making them mutually exclusive. So you propose a bit more restriction in registration EM, to not get lost in the future. I also have these doubts. Let's consider it and maybe agree. I've recommended Qcom to use em_dev_register_perf_domain() when they have this obfuscated power values. Then any developer in the future who wants to add EM for a new device on that platform, should use the em_dev_register_perf_domain(). In this case the flag in EM that you have proposed makes sense. We probably need an argument 'bool abstract_scale' in the em_dev_register_perf_domain(..., bool abstract_scale) as a source of information. We would allow to co-exist em_dev_register_perf_domain(..., false) with dev_pm_opp_of_register_em() EM devices. Is it make sense? > > From my POV, it is not adequate to let SCMI power information co-exists > with the DT power information if we know they can be with different units. > > I've just expressed my opinions: > > - vendors take responsibility of putting different units for the EM > > - Power numbers should come from the same source > > > Up to Rafael to decide what to do with this documentation update. > > Thanks > -- Daniel > > Thank you for your valuable opinion. Regards, Lukasz
On 15/10/2020 11:00, Lukasz Luba wrote: [ ... ] >> There is the SCMI and the DT. Because there are two sources where it is >> impossible to know if they are using the same units, we are stuck to >> ensure a consistency for the kernel. >> >> The platform should use: >> - the SCMI only (scaled or real) >> - the DT only (real) >> [ - the firmware file only (scaled or real) ] >> > > Do you mean by SCMI - registration using em_dev_register_perf_domain() ? It was high level description, but yes, I guess it is the case. >> As it is not possible to know if they are scaled or real, there is no >> choice except making them mutually exclusive. > > So you propose a bit more restriction in registration EM, to not get > lost in the future. I also have these doubts. Let's consider it and > maybe agree. > > I've recommended Qcom to use em_dev_register_perf_domain() when they > have this obfuscated power values. Then any developer in the future > who wants to add EM for a new device on that platform, should use the > em_dev_register_perf_domain(). > > In this case the flag in EM that you have proposed makes sense. > We probably need an argument 'bool abstract_scale' in the > em_dev_register_perf_domain(..., bool abstract_scale) > as a source of information. I was suggesting to add a flag to the em_perf_domain structure giving the source of the power numbers. So if the IPA is having the 'sustainable-power' set in DT but the em_perf_domain is flagged with power number coming from SCMI, then they will be incompatible, the thermal zone will fail to register. > We would allow to co-exist em_dev_register_perf_domain(..., false) > with dev_pm_opp_of_register_em() EM devices. > > Is it make sense? Well, it does not change my opinion. We should assume the energy model is always milliwatts. If the SoC vendors find a way to get around with bogoWatts, then good to them and up to them to deal with in the future.
On Wed, Oct 14, 2020 at 7:10 PM Daniel Lezcano <daniel.lezcano@linaro.org> wrote: > > On 14/10/2020 17:24, Lukasz Luba wrote: > > [ ... ] > > > We have to update the EM doc about allowed abstract scale, which > > implies EAS, IPA doc update with some information to the community that > > these components can handle it. > > > > The script will just make developers life easier, but the current > > documentation does not say anything about abstract scale. > > ... yes, because there is no consistency across the source of power > numbers and no tools to ensure DT power numbers consistency, yet. > > >> In any case, if the DT is specifying real numbers, and SCMI abstract > >> numbers or the opposite, obviously there is a conflict if we are using > >> both. > > > > True, DT only allows real numbers (I have Rob's opinion regarding > > patch 3/3). > > > > It's not that there is only SCMI which might use abstract scale. Qcom > > already has it and other vendors will follow (not exposing real > > numbers). They would register bogoWatts to EM because they know that EAS > > can deal with both. > > So vendors are using bogoWatts, despite the documentation. > > By updating the documentation saying it supports the abstract values, > that means every new framework, device with power values, will have to > comply with that. How is it possible to add a device with power numbers > if the existing ones are obfuscated ? > > With two subsystems using the energy model, evolving independently we > can see there are conflicts. With more subsystems, that may become a > source of confusion, especially with different contributors. > > I think the energy model should stick to milliwatts and keep the > documentation unchanged regarding this. And vendors should take the > responsibility of not sticking to the documentation. > > >> I suggest to fix the conflict first and provide the features to make the > >> numbers more easy to share (like the script described above and/or the > >> firmware file). > >> > >> Then with the right tools, everything can be documented. > >> > > > > We cannot block one way of registration to EM when the other was used. > > They might have correct and consistent numbers. > > What is the rational of using two firmware power information ? > > > It's up to the platform developers to choose the path: > > - go with bogoWatts - if they are not allowed to expose sensitive > > information, use em_dev_register_perf_domain() in drivers, not DT; > > make sure everything that is needed works; check the doc, which > > sub-systems can handle it or needs some tuning (patches 1/3 and 2/3 > > try to help here); > > - use milliWatts - easier; DT is allowed; help from the community in > > reviews, possible results comparisons; both EM registration ways > > might be used; > > > > We cannot force vendors/OEM engineers to store milliWatts in the > > Energy Model if these values are protected by some NDA. > > If I am able to measure one real power value, (and I'm pretty sure it is > quite possible), whatever which one, it is possible to deduce all the > numbers with the linear scale. IMO that is a false debate. Anyway ... > > > Your proposed > > way of providing data into EM from user-space firmware.bin IMHO also > > falls into the same bucket. That information would be accessible in EM > > debugfs and they would avoid it. > > I think you misunderstood my point. > > There is the SCMI and the DT. Because there are two sources where it is > impossible to know if they are using the same units, we are stuck to > ensure a consistency for the kernel. > > The platform should use: > - the SCMI only (scaled or real) > - the DT only (real) > [ - the firmware file only (scaled or real) ] > > > As it is not possible to know if they are scaled or real, there is no > choice except making them mutually exclusive. > > From my POV, it is not adequate to let SCMI power information co-exists > with the DT power information if we know they can be with different units. > > I've just expressed my opinions: > > - vendors take responsibility of putting different units for the EM > > - Power numbers should come from the same source > > > Up to Rafael to decide what to do with this documentation update. Well, I was hoping that you both would reach some kind of agreement. I don't feel like the decision is mine here to be honest.
On 15/10/2020 15:33, Rafael J. Wysocki wrote: [ ... ] >> Up to Rafael to decide what to do with this documentation update. > > Well, I was hoping that you both would reach some kind of agreement. > > I don't feel like the decision is mine here to be honest. No problem, probably we have to think about that a bit more before reaching the agreement.
On Thu, Oct 15, 2020 at 12:22 PM Daniel Lezcano <daniel.lezcano@linaro.org> wrote: > > On 15/10/2020 11:00, Lukasz Luba wrote: > > [ ... ] > > >> There is the SCMI and the DT. Because there are two sources where it is > >> impossible to know if they are using the same units, we are stuck to > >> ensure a consistency for the kernel. > >> > >> The platform should use: > >> - the SCMI only (scaled or real) > >> - the DT only (real) > >> [ - the firmware file only (scaled or real) ] > >> > > > > Do you mean by SCMI - registration using em_dev_register_perf_domain() ? > > It was high level description, but yes, I guess it is the case. > > >> As it is not possible to know if they are scaled or real, there is no > >> choice except making them mutually exclusive. > > > > So you propose a bit more restriction in registration EM, to not get > > lost in the future. I also have these doubts. Let's consider it and > > maybe agree. > > > > I've recommended Qcom to use em_dev_register_perf_domain() when they > > have this obfuscated power values. Then any developer in the future > > who wants to add EM for a new device on that platform, should use the > > em_dev_register_perf_domain(). > > > > In this case the flag in EM that you have proposed makes sense. > > We probably need an argument 'bool abstract_scale' in the > > em_dev_register_perf_domain(..., bool abstract_scale) > > as a source of information. > > I was suggesting to add a flag to the em_perf_domain structure giving > the source of the power numbers. > > So if the IPA is having the 'sustainable-power' set in DT but the > em_perf_domain is flagged with power number coming from SCMI, then they > will be incompatible, the thermal zone will fail to register. > > > > We would allow to co-exist em_dev_register_perf_domain(..., false) > > with dev_pm_opp_of_register_em() EM devices. > > > > Is it make sense? > > Well, it does not change my opinion. We should assume the energy model > is always milliwatts. If the SoC vendors find a way to get around with > bogoWatts, then good to them and up to them to deal with in the future. That sounds fair enough, but it also means that any kernel patches using power units different from milliwatts for the EM should be rejected in the future, doesn't it? And the existing code using different power units for the EM (if any) should be updated/fixed accordingly, shouldn't it? Otherwise I don't see now this can be regarded as a hard rule.
On Thursday 15 Oct 2020 at 15:40:16 (+0200), Rafael J. Wysocki wrote: > On Thu, Oct 15, 2020 at 12:22 PM Daniel Lezcano > <daniel.lezcano@linaro.org> wrote: > > > > On 15/10/2020 11:00, Lukasz Luba wrote: > > > > [ ... ] > > > > >> There is the SCMI and the DT. Because there are two sources where it is > > >> impossible to know if they are using the same units, we are stuck to > > >> ensure a consistency for the kernel. > > >> > > >> The platform should use: > > >> - the SCMI only (scaled or real) > > >> - the DT only (real) > > >> [ - the firmware file only (scaled or real) ] > > >> > > > > > > Do you mean by SCMI - registration using em_dev_register_perf_domain() ? > > > > It was high level description, but yes, I guess it is the case. > > > > >> As it is not possible to know if they are scaled or real, there is no > > >> choice except making them mutually exclusive. > > > > > > So you propose a bit more restriction in registration EM, to not get > > > lost in the future. I also have these doubts. Let's consider it and > > > maybe agree. > > > > > > I've recommended Qcom to use em_dev_register_perf_domain() when they > > > have this obfuscated power values. Then any developer in the future > > > who wants to add EM for a new device on that platform, should use the > > > em_dev_register_perf_domain(). > > > > > > In this case the flag in EM that you have proposed makes sense. > > > We probably need an argument 'bool abstract_scale' in the > > > em_dev_register_perf_domain(..., bool abstract_scale) > > > as a source of information. > > > > I was suggesting to add a flag to the em_perf_domain structure giving > > the source of the power numbers. > > > > So if the IPA is having the 'sustainable-power' set in DT but the > > em_perf_domain is flagged with power number coming from SCMI, then they > > will be incompatible, the thermal zone will fail to register. > > > > > > > We would allow to co-exist em_dev_register_perf_domain(..., false) > > > with dev_pm_opp_of_register_em() EM devices. > > > > > > Is it make sense? > > > > Well, it does not change my opinion. We should assume the energy model > > is always milliwatts. If the SoC vendors find a way to get around with > > bogoWatts, then good to them and up to them to deal with in the future. > > That sounds fair enough, but it also means that any kernel patches > using power units different from milliwatts for the EM should be > rejected in the future, doesn't it? > > And the existing code using different power units for the EM (if any) > should be updated/fixed accordingly, shouldn't it? > > Otherwise I don't see now this can be regarded as a hard rule. Sorry, jumping late in the discussion :) To add a bit of background to this, it's been the plan from the very beginning to make PM_EM use an abstract scale. The only reason it was not merged like that is because the first version only worked for CPUs, and IPA was using a totally different source for other devices. So we had no choice but to specify PM_EM in mW to keep things compatible and allow to transition IPA. But that is no longer true, so I'm in favor of evolving PM_EM where it was supposed to be to begin with. IMO, the only thing the kernel cares about is consistency across power numbers, but not about the exact unit. And I agree with Rafael, we have code paths in the kernel that feed data in PM_EM but _cannot_ guarantee mW, SCMI being a prime example, so I don't think it is reasonable to mandate that. Having that properly documented + an 'abstract_scale' parameter in dev_pm_opp_of_register_em() (or even a unit, which could be bogo-watts) should work IMO. What is the concern with this approach? Thanks, Quentin
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? Can we not let the providers specify the unit? And then it's up to the clients to decide what they want to do. The scheduler wouldn't care, and IPA would have to check things are comparable, but all in all that should work out fine without a strong requirement on the actual unit. Also, I thought SCMI had a notion of sustained performance too, why can't we use that for IPA? Lukasz? Thanks. Quentin
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. Thanks, Quentin
On Friday 16 Oct 2020 at 15:42:57 (+0100), Lukasz Luba wrote: > 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? Yes, something like that. > 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. Right, I agree this is a 'nice-to-have'. > 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? I had something more complicated in mind, where units are arbitrary ('milliwats', 'scmi-bogowatts', ...) as that would help if units can be specified in the DT too, but if we don't care about that then yes I suppose a boolean flag should do. Thanks! Quentin
On 10/16/20 5:02 PM, Quentin Perret wrote: > On Friday 16 Oct 2020 at 15:42:57 (+0100), Lukasz Luba wrote: >> 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? > > Yes, something like that. > >> 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. > > Right, I agree this is a 'nice-to-have'. > >> 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? > > I had something more complicated in mind, where units are arbitrary > ('milliwats', 'scmi-bogowatts', ...) as that would help if units can be > specified in the DT too, but if we don't care about that then yes I > suppose a boolean flag should do. Thank you Quentin for help in sorting this out. I'll send the v3. Regards, Lukasz > > Thanks! > Quentin >