* [RFC V7 0/2] OPP: Allow OPP table to be used for power-domains @ 2017-10-31 12:47 Viresh Kumar 2017-10-31 12:47 ` [RFC V7 1/2] " Viresh Kumar ` (2 more replies) 0 siblings, 3 replies; 25+ messages in thread From: Viresh Kumar @ 2017-10-31 12:47 UTC (permalink / raw) To: ulf.hansson, Kevin Hilman Cc: Viresh Kumar, Rafael Wysocki, linux-pm, Stephen Boyd, Nishanth Menon, Vincent Guittot, robh+dt, rnayak, sudeep.holla, devicetree, linux-kernel, Viresh Kumar Hi, Now that the performance state of PM domains are supported by the kernel (merged in linux-next), I am trying once again to define the bindings which we dropped until the code is merged first. This is targeted for 4.16 kernel and I am sending it now to get early reviews. The bindings in this series have changed a bit since the time it was last sent for review [1]. I have taken care of most of the feedback given last time. Summary: Power-domains can also have their active states and this patchset enhances the OPP binding to define those. The power domains can use the OPP bindings mostly as is. Though there are some changes required to support special cases: - Allow "operating-points-v2" to contain multiple phandles for power domain providers providing multiple domains. - A new property "power-domain-opp" is added for devices to specify the minimum required OPP of the master domain for the functioning of the device. We can add this property directly to device's node if the device has a fixed minimum OPP requirement from the master power domain. Or we can add this property to each OPP node of the device, if different OPP nodes have different minimum OPP requirement from the master power domain. - Allow some of the OPP properties to accept magic values (firmware dependent) as the OS doesn't know the real freq/voltage values. -- viresh [1] https://marc.info/?l=linux-kernel&m=149320428621874&w=2 Viresh Kumar (2): OPP: Allow OPP table to be used for power-domains OPP: Allow "opp-hz" and "opp-microvolt" to contain magic values Documentation/devicetree/bindings/opp/opp.txt | 18 +++++++ .../devicetree/bindings/power/power_domain.txt | 62 ++++++++++++++++++++++ 2 files changed, 80 insertions(+) -- 2.15.0.rc1.236.g92ea95045093 ^ permalink raw reply [flat|nested] 25+ messages in thread
* [RFC V7 1/2] OPP: Allow OPP table to be used for power-domains 2017-10-31 12:47 [RFC V7 0/2] OPP: Allow OPP table to be used for power-domains Viresh Kumar @ 2017-10-31 12:47 ` Viresh Kumar 2017-11-28 15:50 ` Ulf Hansson 2017-11-29 16:46 ` Rob Herring 2017-10-31 12:47 ` [RFC V7 2/2] OPP: Allow "opp-hz" and "opp-microvolt" to contain magic values Viresh Kumar 2017-11-29 4:14 ` [RFC V7 0/2] OPP: Allow OPP table to be used for power-domains Viresh Kumar 2 siblings, 2 replies; 25+ messages in thread From: Viresh Kumar @ 2017-10-31 12:47 UTC (permalink / raw) To: ulf.hansson, Kevin Hilman, Viresh Kumar, Nishanth Menon, Stephen Boyd, Rafael J. Wysocki Cc: Viresh Kumar, linux-pm, Vincent Guittot, robh+dt, rnayak, sudeep.holla, devicetree, linux-kernel Power-domains can also have their active states and this patch enhances the OPP binding to define those. The power domains can use the OPP bindings mostly as is. Though there are some changes required to support special cases: - Allow "operating-points-v2" to contain multiple phandles for power domain providers providing multiple domains. - A new property "power-domain-opp" is added for devices to specify the minimum required OPP of the master domain for the functioning of the device. We can add this property directly to device's node if the device has a fixed minimum OPP requirement from the master power domain. Or we can add this property to each OPP node of the device, if different OPP nodes have different minimum OPP requirement from the master power domain. Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> --- Documentation/devicetree/bindings/opp/opp.txt | 12 +++++ .../devicetree/bindings/power/power_domain.txt | 62 ++++++++++++++++++++++ 2 files changed, 74 insertions(+) diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt index 9d733af26be7..203e09fe7698 100644 --- a/Documentation/devicetree/bindings/opp/opp.txt +++ b/Documentation/devicetree/bindings/opp/opp.txt @@ -45,6 +45,11 @@ Devices supporting OPPs must set their "operating-points-v2" property with phandle to a OPP table in their DT node. The OPP core will use this phandle to find the operating points for the device. +This can contain more than one phandle for power domain providers that provide +multiple power domains. That is, one phandle for each power domain. If only one +phandle is available, then the same OPP table will be used for all power domains +provided by the power domain provider. + If required, this can be extended for SoC vendor specific bindings. Such bindings should be documented as Documentation/devicetree/bindings/power/<vendor>-opp.txt and should have a compatible description like: "operating-points-v2-<vendor>". @@ -154,6 +159,13 @@ properties. - status: Marks the node enabled/disabled. +- power-domain-opp: This contains phandle to one of the OPP nodes of the master + power domain. This specifies the minimum required OPP of the master domain for + the functioning of the device in this OPP (where this property is present). + This property can only be set for a device if the device node contains the + "power-domains" property. Also, either all or none of the OPP nodes in an OPP + table should have it set. + Example 1: Single cluster Dual-core ARM cortex A9, switch DVFS states together. / { diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt index 14bd9e945ff6..0d8608f2d133 100644 --- a/Documentation/devicetree/bindings/power/power_domain.txt +++ b/Documentation/devicetree/bindings/power/power_domain.txt @@ -40,6 +40,12 @@ phandle arguments (so called PM domain specifiers) of length specified by the domain's idle states. In the absence of this property, the domain would be considered as capable of being powered-on or powered-off. +- operating-points-v2 : Phandles to the OPP tables of power domains provided by + a power domain provider. If the provider provides a single power domain only + or all the power domains provided by the provider have identical OPP tables, + then this shall contain a single phandle. Refer to ../opp/opp.txt for more + information. + Example: power: power-controller@12340000 { @@ -120,4 +126,60 @@ The node above defines a typical PM domain consumer device, which is located inside a PM domain with index 0 of a power controller represented by a node with the label "power". +Optional properties: +- power-domain-opp: This contains phandle to one of the OPP nodes of the master + power domain. This specifies the minimum required OPP of the master domain for + the functioning of the device. This property can only be set for a device, if + the device node contains the "power-domains" property. + +Example: +- OPP table for domain provider that provides two domains. + + domain0_opp_table: opp_table0 { + compatible = "operating-points-v2"; + + domain0_opp_0: opp-1000000000 { + opp-hz = /bits/ 64 <1000000000>; + opp-microvolt = <975000 970000 985000>; + }; + domain0_opp_1: opp-1100000000 { + opp-hz = /bits/ 64 <1100000000>; + opp-microvolt = <1000000 980000 1010000>; + }; + }; + + domain1_opp_table: opp_table1 { + compatible = "operating-points-v2"; + + domain1_opp_0: opp-1200000000 { + opp-hz = /bits/ 64 <1200000000>; + opp-microvolt = <975000 970000 985000>; + }; + domain1_opp_1: opp-1300000000 { + opp-hz = /bits/ 64 <1300000000>; + opp-microvolt = <1000000 980000 1010000>; + }; + }; + + parent: power-controller@12340000 { + compatible = "foo,power-controller"; + reg = <0x12340000 0x1000>; + #power-domain-cells = <1>; + operating-points-v2 = <&domain0_opp_table>, <&domain1_opp_table>; + }; + + leaky-device0@12350000 { + compatible = "foo,i-leak-current"; + reg = <0x12350000 0x1000>; + power-domains = <&parent 0>; + power-domain-opp = <&domain0_opp_0>; + }; + + leaky-device1@12350000 { + compatible = "foo,i-leak-current"; + reg = <0x12350000 0x1000>; + power-domains = <&parent 1>; + power-domain-opp = <&domain1_opp_1>; + }; + [1]. Documentation/devicetree/bindings/power/domain-idle-state.txt -- 2.15.0.rc1.236.g92ea95045093 ^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [RFC V7 1/2] OPP: Allow OPP table to be used for power-domains 2017-10-31 12:47 ` [RFC V7 1/2] " Viresh Kumar @ 2017-11-28 15:50 ` Ulf Hansson 2017-11-29 16:46 ` Rob Herring 1 sibling, 0 replies; 25+ messages in thread From: Ulf Hansson @ 2017-11-28 15:50 UTC (permalink / raw) To: Viresh Kumar Cc: Kevin Hilman, Viresh Kumar, Nishanth Menon, Stephen Boyd, Rafael J. Wysocki, linux-pm, Vincent Guittot, Rob Herring, Rajendra Nayak, Sudeep Holla, devicetree, linux-kernel On 31 October 2017 at 13:47, Viresh Kumar <viresh.kumar@linaro.org> wrote: > Power-domains can also have their active states and this patch enhances > the OPP binding to define those. > > The power domains can use the OPP bindings mostly as is. Though there > are some changes required to support special cases: > > - Allow "operating-points-v2" to contain multiple phandles for power > domain providers providing multiple domains. > > - A new property "power-domain-opp" is added for devices to specify the > minimum required OPP of the master domain for the functioning of the > device. We can add this property directly to device's node if the > device has a fixed minimum OPP requirement from the master power Please avoid the terminology "master power domain", it's confusing. Instead use only "power domain". This applies to a couple of more places of $subject patch, please fix those as well. > domain. Or we can add this property to each OPP node of the device, if > different OPP nodes have different minimum OPP requirement from the > master power domain. > > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > --- > Documentation/devicetree/bindings/opp/opp.txt | 12 +++++ > .../devicetree/bindings/power/power_domain.txt | 62 ++++++++++++++++++++++ > 2 files changed, 74 insertions(+) > > diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt > index 9d733af26be7..203e09fe7698 100644 > --- a/Documentation/devicetree/bindings/opp/opp.txt > +++ b/Documentation/devicetree/bindings/opp/opp.txt > @@ -45,6 +45,11 @@ Devices supporting OPPs must set their "operating-points-v2" property with > phandle to a OPP table in their DT node. The OPP core will use this phandle to > find the operating points for the device. > > +This can contain more than one phandle for power domain providers that provide > +multiple power domains. That is, one phandle for each power domain. If only one > +phandle is available, then the same OPP table will be used for all power domains > +provided by the power domain provider. > + > If required, this can be extended for SoC vendor specific bindings. Such bindings > should be documented as Documentation/devicetree/bindings/power/<vendor>-opp.txt > and should have a compatible description like: "operating-points-v2-<vendor>". > @@ -154,6 +159,13 @@ properties. > > - status: Marks the node enabled/disabled. > > +- power-domain-opp: This contains phandle to one of the OPP nodes of the master > + power domain. This specifies the minimum required OPP of the master domain for > + the functioning of the device in this OPP (where this property is present). > + This property can only be set for a device if the device node contains the > + "power-domains" property. Also, either all or none of the OPP nodes in an OPP > + table should have it set. > + > Example 1: Single cluster Dual-core ARM cortex A9, switch DVFS states together. > > / { > diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt > index 14bd9e945ff6..0d8608f2d133 100644 > --- a/Documentation/devicetree/bindings/power/power_domain.txt > +++ b/Documentation/devicetree/bindings/power/power_domain.txt > @@ -40,6 +40,12 @@ phandle arguments (so called PM domain specifiers) of length specified by the > domain's idle states. In the absence of this property, the domain would be > considered as capable of being powered-on or powered-off. > > +- operating-points-v2 : Phandles to the OPP tables of power domains provided by > + a power domain provider. If the provider provides a single power domain only > + or all the power domains provided by the provider have identical OPP tables, > + then this shall contain a single phandle. Refer to ../opp/opp.txt for more > + information. > + > Example: > > power: power-controller@12340000 { > @@ -120,4 +126,60 @@ The node above defines a typical PM domain consumer device, which is located > inside a PM domain with index 0 of a power controller represented by a node > with the label "power". > > +Optional properties: > +- power-domain-opp: This contains phandle to one of the OPP nodes of the master > + power domain. This specifies the minimum required OPP of the master domain for > + the functioning of the device. This property can only be set for a device, if > + the device node contains the "power-domains" property. > + > +Example: > +- OPP table for domain provider that provides two domains. > + > + domain0_opp_table: opp_table0 { > + compatible = "operating-points-v2"; > + > + domain0_opp_0: opp-1000000000 { > + opp-hz = /bits/ 64 <1000000000>; > + opp-microvolt = <975000 970000 985000>; > + }; > + domain0_opp_1: opp-1100000000 { > + opp-hz = /bits/ 64 <1100000000>; > + opp-microvolt = <1000000 980000 1010000>; > + }; > + }; > + > + domain1_opp_table: opp_table1 { > + compatible = "operating-points-v2"; > + > + domain1_opp_0: opp-1200000000 { > + opp-hz = /bits/ 64 <1200000000>; > + opp-microvolt = <975000 970000 985000>; > + }; > + domain1_opp_1: opp-1300000000 { > + opp-hz = /bits/ 64 <1300000000>; > + opp-microvolt = <1000000 980000 1010000>; > + }; > + }; > + > + parent: power-controller@12340000 { > + compatible = "foo,power-controller"; > + reg = <0x12340000 0x1000>; > + #power-domain-cells = <1>; > + operating-points-v2 = <&domain0_opp_table>, <&domain1_opp_table>; > + }; > + > + leaky-device0@12350000 { > + compatible = "foo,i-leak-current"; > + reg = <0x12350000 0x1000>; > + power-domains = <&parent 0>; > + power-domain-opp = <&domain0_opp_0>; > + }; > + > + leaky-device1@12350000 { > + compatible = "foo,i-leak-current"; > + reg = <0x12350000 0x1000>; > + power-domains = <&parent 1>; > + power-domain-opp = <&domain1_opp_1>; > + }; > + > [1]. Documentation/devicetree/bindings/power/domain-idle-state.txt > -- Besides the minor nitpick(s), this looks good to me. Feel free to add: Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org> Kind regards Uffe ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC V7 1/2] OPP: Allow OPP table to be used for power-domains 2017-10-31 12:47 ` [RFC V7 1/2] " Viresh Kumar 2017-11-28 15:50 ` Ulf Hansson @ 2017-11-29 16:46 ` Rob Herring 2017-11-30 4:48 ` Viresh Kumar 1 sibling, 1 reply; 25+ messages in thread From: Rob Herring @ 2017-11-29 16:46 UTC (permalink / raw) To: Viresh Kumar Cc: Ulf Hansson, Kevin Hilman, Viresh Kumar, Nishanth Menon, Stephen Boyd, Rafael J. Wysocki, linux-pm, Vincent Guittot, Rajendra Nayak, Sudeep Holla, devicetree, linux-kernel On Tue, Oct 31, 2017 at 7:47 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote: > Power-domains can also have their active states and this patch enhances > the OPP binding to define those. > > The power domains can use the OPP bindings mostly as is. Though there > are some changes required to support special cases: > > - Allow "operating-points-v2" to contain multiple phandles for power > domain providers providing multiple domains. > > - A new property "power-domain-opp" is added for devices to specify the > minimum required OPP of the master domain for the functioning of the > device. We can add this property directly to device's node if the > device has a fixed minimum OPP requirement from the master power > domain. Or we can add this property to each OPP node of the device, if > different OPP nodes have different minimum OPP requirement from the > master power domain. > > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > --- > Documentation/devicetree/bindings/opp/opp.txt | 12 +++++ > .../devicetree/bindings/power/power_domain.txt | 62 ++++++++++++++++++++++ > 2 files changed, 74 insertions(+) > > diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt > index 9d733af26be7..203e09fe7698 100644 > --- a/Documentation/devicetree/bindings/opp/opp.txt > +++ b/Documentation/devicetree/bindings/opp/opp.txt > @@ -45,6 +45,11 @@ Devices supporting OPPs must set their "operating-points-v2" property with > phandle to a OPP table in their DT node. The OPP core will use this phandle to > find the operating points for the device. > > +This can contain more than one phandle for power domain providers that provide > +multiple power domains. That is, one phandle for each power domain. If only one > +phandle is available, then the same OPP table will be used for all power domains > +provided by the power domain provider. > + > If required, this can be extended for SoC vendor specific bindings. Such bindings > should be documented as Documentation/devicetree/bindings/power/<vendor>-opp.txt > and should have a compatible description like: "operating-points-v2-<vendor>". > @@ -154,6 +159,13 @@ properties. > > - status: Marks the node enabled/disabled. > > +- power-domain-opp: This contains phandle to one of the OPP nodes of the master > + power domain. This specifies the minimum required OPP of the master domain for > + the functioning of the device in this OPP (where this property is present). > + This property can only be set for a device if the device node contains the > + "power-domains" property. Also, either all or none of the OPP nodes in an OPP > + table should have it set. This is a "this device requires OPP n" property. Couldn't we want this for cases other than a powerdomain OPP? What if a device has requirements 2 different OPPs? On the flipside, I don't think we want devices picking things like CPU OPPs and putting policy here. But I'd rather things be extendable than reviewing yet another OPP property next month. Rob ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC V7 1/2] OPP: Allow OPP table to be used for power-domains 2017-11-29 16:46 ` Rob Herring @ 2017-11-30 4:48 ` Viresh Kumar 0 siblings, 0 replies; 25+ messages in thread From: Viresh Kumar @ 2017-11-30 4:48 UTC (permalink / raw) To: Rob Herring Cc: Ulf Hansson, Kevin Hilman, Viresh Kumar, Nishanth Menon, Stephen Boyd, Rafael J. Wysocki, linux-pm, Vincent Guittot, Rajendra Nayak, Sudeep Holla, devicetree, linux-kernel On 29-11-17, 10:46, Rob Herring wrote: > On Tue, Oct 31, 2017 at 7:47 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote: > > +- power-domain-opp: This contains phandle to one of the OPP nodes of the master > > + power domain. This specifies the minimum required OPP of the master domain for > > + the functioning of the device in this OPP (where this property is present). > > + This property can only be set for a device if the device node contains the > > + "power-domains" property. Also, either all or none of the OPP nodes in an OPP > > + table should have it set. > > This is a "this device requires OPP n" property. Couldn't we want this > for cases other than a powerdomain OPP? What if a device has > requirements 2 different OPPs? Hmm, I agree. We can/should make it more generic. > On the flipside, I don't think we want devices picking things like CPU > OPPs and putting policy here. But I'd rather things be extendable than > reviewing yet another OPP property next month. Sure, I would rename this property and make necessary changes to it. -- viresh ^ permalink raw reply [flat|nested] 25+ messages in thread
* [RFC V7 2/2] OPP: Allow "opp-hz" and "opp-microvolt" to contain magic values 2017-10-31 12:47 [RFC V7 0/2] OPP: Allow OPP table to be used for power-domains Viresh Kumar 2017-10-31 12:47 ` [RFC V7 1/2] " Viresh Kumar @ 2017-10-31 12:47 ` Viresh Kumar 2017-10-31 16:02 ` Rob Herring 2017-11-28 16:14 ` Ulf Hansson 2017-11-29 4:14 ` [RFC V7 0/2] OPP: Allow OPP table to be used for power-domains Viresh Kumar 2 siblings, 2 replies; 25+ messages in thread From: Viresh Kumar @ 2017-10-31 12:47 UTC (permalink / raw) To: ulf.hansson, Kevin Hilman, Viresh Kumar, Nishanth Menon, Stephen Boyd Cc: Viresh Kumar, Rafael Wysocki, linux-pm, Vincent Guittot, robh+dt, rnayak, sudeep.holla, devicetree, linux-kernel On some platforms the exact frequency or voltage may be hidden from the OS by the firmware. Allow such configurations to pass magic values in the "opp-hz" or the "opp-microvolt" properties, which should be interpreted in a platform dependent way. Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> --- Documentation/devicetree/bindings/opp/opp.txt | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt index 203e09fe7698..9c5056fb120f 100644 --- a/Documentation/devicetree/bindings/opp/opp.txt +++ b/Documentation/devicetree/bindings/opp/opp.txt @@ -166,6 +166,12 @@ properties. "power-domains" property. Also, either all or none of the OPP nodes in an OPP table should have it set. + +On some platforms the exact frequency or voltage may be hidden from the OS by +the firmware and the "opp-hz" or the "opp-microvolt" properties may contain +magic values that represent the frequency or voltage in a firmware dependent +way, for example an index of an array in the firmware. + Example 1: Single cluster Dual-core ARM cortex A9, switch DVFS states together. / { -- 2.15.0.rc1.236.g92ea95045093 ^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [RFC V7 2/2] OPP: Allow "opp-hz" and "opp-microvolt" to contain magic values 2017-10-31 12:47 ` [RFC V7 2/2] OPP: Allow "opp-hz" and "opp-microvolt" to contain magic values Viresh Kumar @ 2017-10-31 16:02 ` Rob Herring 2017-11-01 2:17 ` Viresh Kumar 2017-11-28 16:14 ` Ulf Hansson 1 sibling, 1 reply; 25+ messages in thread From: Rob Herring @ 2017-10-31 16:02 UTC (permalink / raw) To: Viresh Kumar Cc: Ulf Hansson, Kevin Hilman, Viresh Kumar, Nishanth Menon, Stephen Boyd, Rafael Wysocki, linux-pm, Vincent Guittot, Rajendra Nayak, Sudeep Holla, devicetree, linux-kernel On Tue, Oct 31, 2017 at 7:47 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote: > On some platforms the exact frequency or voltage may be hidden from the > OS by the firmware. Allow such configurations to pass magic values in > the "opp-hz" or the "opp-microvolt" properties, which should be > interpreted in a platform dependent way. Why not a new property for magic values? opp-magic? Don't we want to know when we have magic values? Wouldn't magic values in opp-hz get propagated to user space? I can see the complaints now. "My 4GHz processor is running at 6Hz!" Just like people complain when BogoMIPS is not high enough. Rob ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC V7 2/2] OPP: Allow "opp-hz" and "opp-microvolt" to contain magic values 2017-10-31 16:02 ` Rob Herring @ 2017-11-01 2:17 ` Viresh Kumar 2017-11-01 20:39 ` Rob Herring 0 siblings, 1 reply; 25+ messages in thread From: Viresh Kumar @ 2017-11-01 2:17 UTC (permalink / raw) To: Rob Herring Cc: Ulf Hansson, Kevin Hilman, Viresh Kumar, Nishanth Menon, Stephen Boyd, Rafael Wysocki, linux-pm, Vincent Guittot, Rajendra Nayak, Sudeep Holla, devicetree, linux-kernel On 31 October 2017 at 16:02, Rob Herring <robh+dt@kernel.org> wrote: > Why not a new property for magic values? opp-magic? Don't we want to > know when we have magic values? I have kept a separate property since beginning (domain-performance-state) and moved to using these magic values in the existing field because of the suggestion Kevin gave earlier. https://marc.info/?l=linux-kernel&m=149306082218001&w=2 I am not sure what to do now :) > Wouldn't magic values in opp-hz get propagated to user space? The OPP core puts them in debugfs just to know how the OPPs are set. Otherwise, I am not sure that the power domain core/drivers would be exposing that to user space. > I can > see the complaints now. "My 4GHz processor is running at 6Hz!" Just > like people complain when BogoMIPS is not high enough. Hmm, would adding the right user type in bindings would be good enough for that? I mean, we can clearly add that only power-domains that are managed by firmware are allowed to use magic values here. But not sure if we should put such stuff in bindings. -- viresh ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC V7 2/2] OPP: Allow "opp-hz" and "opp-microvolt" to contain magic values 2017-11-01 2:17 ` Viresh Kumar @ 2017-11-01 20:39 ` Rob Herring 2017-11-01 21:43 ` Stephen Boyd 2017-11-02 4:49 ` Viresh Kumar 0 siblings, 2 replies; 25+ messages in thread From: Rob Herring @ 2017-11-01 20:39 UTC (permalink / raw) To: Viresh Kumar Cc: Ulf Hansson, Kevin Hilman, Viresh Kumar, Nishanth Menon, Stephen Boyd, Rafael Wysocki, linux-pm, Vincent Guittot, Rajendra Nayak, Sudeep Holla, devicetree, linux-kernel On Tue, Oct 31, 2017 at 9:17 PM, Viresh Kumar <viresh.kumar@linaro.org> wrote: > On 31 October 2017 at 16:02, Rob Herring <robh+dt@kernel.org> wrote: >> Why not a new property for magic values? opp-magic? Don't we want to >> know when we have magic values? > > I have kept a separate property since beginning (domain-performance-state) > and moved to using these magic values in the existing field because of the > suggestion Kevin gave earlier. > > https://marc.info/?l=linux-kernel&m=149306082218001&w=2 > > I am not sure what to do now :) Okay, I guess reusing the properties is fine. >> Wouldn't magic values in opp-hz get propagated to user space? > > The OPP core puts them in debugfs just to know how the OPPs are > set. Otherwise, I am not sure that the power domain core/drivers would > be exposing that to user space. I was thinking thru the cpufreq interface, but I guess this is not for cpus. Rob ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC V7 2/2] OPP: Allow "opp-hz" and "opp-microvolt" to contain magic values 2017-11-01 20:39 ` Rob Herring @ 2017-11-01 21:43 ` Stephen Boyd 2017-11-02 4:51 ` Viresh Kumar 2017-11-02 4:49 ` Viresh Kumar 1 sibling, 1 reply; 25+ messages in thread From: Stephen Boyd @ 2017-11-01 21:43 UTC (permalink / raw) To: Rob Herring Cc: Viresh Kumar, Ulf Hansson, Kevin Hilman, Viresh Kumar, Nishanth Menon, Rafael Wysocki, linux-pm, Vincent Guittot, Rajendra Nayak, Sudeep Holla, devicetree, linux-kernel On 11/01, Rob Herring wrote: > On Tue, Oct 31, 2017 at 9:17 PM, Viresh Kumar <viresh.kumar@linaro.org> wrote: > > On 31 October 2017 at 16:02, Rob Herring <robh+dt@kernel.org> wrote: > >> Why not a new property for magic values? opp-magic? Don't we want to > >> know when we have magic values? > > > > I have kept a separate property since beginning (domain-performance-state) > > and moved to using these magic values in the existing field because of the > > suggestion Kevin gave earlier. > > > > https://marc.info/?l=linux-kernel&m=149306082218001&w=2 > > > > I am not sure what to do now :) > > Okay, I guess reusing the properties is fine. > We call them corners on qcom platforms. Any reason we can't keep using that name? I'd rather not have to keep telling people that these fake values in some misnamed property is actually a corner. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC V7 2/2] OPP: Allow "opp-hz" and "opp-microvolt" to contain magic values 2017-11-01 21:43 ` Stephen Boyd @ 2017-11-02 4:51 ` Viresh Kumar 2017-11-02 7:15 ` Stephen Boyd 0 siblings, 1 reply; 25+ messages in thread From: Viresh Kumar @ 2017-11-02 4:51 UTC (permalink / raw) To: Stephen Boyd Cc: Rob Herring, Ulf Hansson, Kevin Hilman, Viresh Kumar, Nishanth Menon, Rafael Wysocki, linux-pm, Vincent Guittot, Rajendra Nayak, Sudeep Holla, devicetree, linux-kernel On 01-11-17, 14:43, Stephen Boyd wrote: > On 11/01, Rob Herring wrote: > > On Tue, Oct 31, 2017 at 9:17 PM, Viresh Kumar <viresh.kumar@linaro.org> wrote: > > > On 31 October 2017 at 16:02, Rob Herring <robh+dt@kernel.org> wrote: > > >> Why not a new property for magic values? opp-magic? Don't we want to > > >> know when we have magic values? > > > > > > I have kept a separate property since beginning (domain-performance-state) > > > and moved to using these magic values in the existing field because of the > > > suggestion Kevin gave earlier. > > > > > > https://marc.info/?l=linux-kernel&m=149306082218001&w=2 > > > > > > I am not sure what to do now :) > > > > Okay, I guess reusing the properties is fine. > > > > We call them corners on qcom platforms. Any reason we can't keep > using that name? I'd rather not have to keep telling people that > these fake values in some misnamed property is actually a corner. Surely not "corners", as these are platform and OS independent bindings we are talking about here. Even the kernel code shouldn't generally do that. Though your platform specific genpd driver can :) -- viresh ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC V7 2/2] OPP: Allow "opp-hz" and "opp-microvolt" to contain magic values 2017-11-02 4:51 ` Viresh Kumar @ 2017-11-02 7:15 ` Stephen Boyd 2017-11-02 9:00 ` Viresh Kumar 0 siblings, 1 reply; 25+ messages in thread From: Stephen Boyd @ 2017-11-02 7:15 UTC (permalink / raw) To: Viresh Kumar Cc: Rob Herring, Ulf Hansson, Kevin Hilman, Viresh Kumar, Nishanth Menon, Rafael Wysocki, linux-pm, Vincent Guittot, Rajendra Nayak, Sudeep Holla, devicetree, linux-kernel On 11/02, Viresh Kumar wrote: > On 01-11-17, 14:43, Stephen Boyd wrote: > > On 11/01, Rob Herring wrote: > > > On Tue, Oct 31, 2017 at 9:17 PM, Viresh Kumar <viresh.kumar@linaro.org> wrote: > > > > On 31 October 2017 at 16:02, Rob Herring <robh+dt@kernel.org> wrote: > > > >> Why not a new property for magic values? opp-magic? Don't we want to > > > >> know when we have magic values? > > > > > > > > I have kept a separate property since beginning (domain-performance-state) > > > > and moved to using these magic values in the existing field because of the > > > > suggestion Kevin gave earlier. > > > > > > > > https://marc.info/?l=linux-kernel&m=149306082218001&w=2 > > > > > > > > I am not sure what to do now :) > > > > > > Okay, I guess reusing the properties is fine. > > > > > > > We call them corners on qcom platforms. Any reason we can't keep > > using that name? I'd rather not have to keep telling people that > > these fake values in some misnamed property is actually a corner. > > Surely not "corners", as these are platform and OS independent > bindings we are talking about here. Even the kernel code shouldn't > generally do that. Though your platform specific genpd driver can :) > Sorry I'm not following. We're going to need to have platform specific code that understands platform specific bindings that aren't shoved into the generic OPP bindings. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC V7 2/2] OPP: Allow "opp-hz" and "opp-microvolt" to contain magic values 2017-11-02 7:15 ` Stephen Boyd @ 2017-11-02 9:00 ` Viresh Kumar 2017-11-28 16:38 ` Ulf Hansson 0 siblings, 1 reply; 25+ messages in thread From: Viresh Kumar @ 2017-11-02 9:00 UTC (permalink / raw) To: Stephen Boyd Cc: Rob Herring, Ulf Hansson, Kevin Hilman, Viresh Kumar, Nishanth Menon, Rafael Wysocki, linux-pm, Vincent Guittot, Rajendra Nayak, Sudeep Holla, devicetree, linux-kernel On 02-11-17, 00:15, Stephen Boyd wrote: > Sorry I'm not following. We're going to need to have platform > specific code that understands platform specific bindings that > aren't shoved into the generic OPP bindings. At least I am not targeting any platform specific binding right now. The way I see this to work is: - We will reuse earlier bindings and allow opp-hz and opp-microvolt to contain special values (this patch). - Platform specific DT entries will put corner numbers in opp-hz (or opp-microvolt) fields. - Some platform specific driver (in OPP or genpd) will be used to convert OPP into a performance state (corner) value. Now that can simply read opp-hz (or opp-microvolt) and return its value. - OPP core will request for a performance state (code is already merged for that). And so there is no platform specific binding here. Do you want to do this differently ? -- viresh ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC V7 2/2] OPP: Allow "opp-hz" and "opp-microvolt" to contain magic values 2017-11-02 9:00 ` Viresh Kumar @ 2017-11-28 16:38 ` Ulf Hansson 2017-11-30 0:50 ` Stephen Boyd 0 siblings, 1 reply; 25+ messages in thread From: Ulf Hansson @ 2017-11-28 16:38 UTC (permalink / raw) To: Viresh Kumar Cc: Stephen Boyd, Rob Herring, Kevin Hilman, Viresh Kumar, Nishanth Menon, Rafael Wysocki, linux-pm, Vincent Guittot, Rajendra Nayak, Sudeep Holla, devicetree, linux-kernel On 2 November 2017 at 10:00, Viresh Kumar <viresh.kumar@linaro.org> wrote: > On 02-11-17, 00:15, Stephen Boyd wrote: >> Sorry I'm not following. We're going to need to have platform >> specific code that understands platform specific bindings that >> aren't shoved into the generic OPP bindings. > > At least I am not targeting any platform specific binding right now. > The way I see this to work is: > > - We will reuse earlier bindings and allow opp-hz and opp-microvolt to > contain special values (this patch). > - Platform specific DT entries will put corner numbers in opp-hz (or > opp-microvolt) fields. > - Some platform specific driver (in OPP or genpd) will be used to > convert OPP into a performance state (corner) value. Now that can > simply read opp-hz (or opp-microvolt) and return its value. Since the "operating-points-v2" phandle(s) belongs in the power-domain controller device node, which anyway is being parsed by the genpd SoC specific driver, I assume it makes sense to start the initialization from there. Unless there is something that prevents that, of course. Then whatever library/helper functions we need for parse and create the OPP tables, can be provided to the OPP framework and the OPP OF library. > - OPP core will request for a performance state (code is already > merged for that). > > And so there is no platform specific binding here. Do you want to do > this differently ? This makes sense to me! Also, the SoC (QCOM) specific genpd driver is free to use the terminology "corner values", when it translates opp-hz|microvolt into such values. Kind regards Uffe ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC V7 2/2] OPP: Allow "opp-hz" and "opp-microvolt" to contain magic values 2017-11-28 16:38 ` Ulf Hansson @ 2017-11-30 0:50 ` Stephen Boyd 2017-11-30 6:59 ` Viresh Kumar 0 siblings, 1 reply; 25+ messages in thread From: Stephen Boyd @ 2017-11-30 0:50 UTC (permalink / raw) To: Ulf Hansson Cc: Viresh Kumar, Rob Herring, Kevin Hilman, Viresh Kumar, Nishanth Menon, Rafael Wysocki, linux-pm, Vincent Guittot, Rajendra Nayak, Sudeep Holla, devicetree, linux-kernel On 11/28, Ulf Hansson wrote: > On 2 November 2017 at 10:00, Viresh Kumar <viresh.kumar@linaro.org> wrote: > > On 02-11-17, 00:15, Stephen Boyd wrote: > >> Sorry I'm not following. We're going to need to have platform > >> specific code that understands platform specific bindings that > >> aren't shoved into the generic OPP bindings. > > > > At least I am not targeting any platform specific binding right now. > > The way I see this to work is: > > > > - We will reuse earlier bindings and allow opp-hz and opp-microvolt to > > contain special values (this patch). > > - Platform specific DT entries will put corner numbers in opp-hz (or > > opp-microvolt) fields. > > - Some platform specific driver (in OPP or genpd) will be used to > > convert OPP into a performance state (corner) value. Now that can > > simply read opp-hz (or opp-microvolt) and return its value. > > Since the "operating-points-v2" phandle(s) belongs in the power-domain > controller device node, which anyway is being parsed by the genpd SoC > specific driver, I assume it makes sense to start the initialization > from there. Unless there is something that prevents that, of course. > > Then whatever library/helper functions we need for parse and create > the OPP tables, can be provided to the OPP framework and the OPP OF > library. > > > - OPP core will request for a performance state (code is already > > merged for that). > > > > And so there is no platform specific binding here. Do you want to do > > this differently ? > > This makes sense to me! > > Also, the SoC (QCOM) specific genpd driver is free to use the > terminology "corner values", when it translates opp-hz|microvolt into > such values. > Sorry it still makes zero sense to me. It seems that we're trying to make the OPP table parsing generic just for the sake of code brevity. Is this the goal? From a DT writer perspective it seems confusing to say that opp-microvolt is sometimes a microvolt and sometimes not a microvolt. Why can't the SoC specific genpd driver parse something like "qcom,corner" instead out of the node? BTW, I don't believe I have a use-case where I want to express power domain OPP tables. I have many devices that all have different frequencies that are all tied into the same power domain. This binding makes it look like we can only have one frequency per domain which won't work. I want to express that a device with a range of frequencies (or really multiple ranges of frequencies) is inside certain physical power domains and the frequency of the clks dictates the minimum voltage requirement of those power domains. For the most complicated case, imagine something like our eMMC controller that has two clks (clk1,clk2) that it changes the rate of independently and those two clks rely on two different regulators (vreg1, vreg2) that supply voltage domains in the SoC which the eMMC controller happens to be part of (pd1, pd2). And the device is also part of another power domain that we use to turn everything off (pd3). +-------+ +-------+ | vreg1 | | vreg2 | +---+---+ +----+--+ | | | +------------+-------------+ | | | +-------+ | +--------+ | | +-----> | clk1 | | | clk2 | <---------+ | +---+---+ | +----+---+ | | | | | | +----------v--------------v-----------+ | | | | | | | | | | | | pd1 | pd2 | | | | | | | | +------------+-------------+ | | | | pd3 eMMC | +-------------------------------------+ >From a DT perspective, I see this as one emmc node: pd: power-domain-controller { #power-domain-cells = <1>; }; cc: clock-controller { #clock-cells = <1>; }; emmc { power-domains = <&pd 1> <&pd 2>, <&pd 3>; clocks = <&cc 1> <&cc 2>; } And then we really don't want to have to express every single possible frequency that clk1 and clk2 can be just to express the voltage/corner constraints. It could be that we have some table like so: clk1 Hz | pd1 Corner ------------+----------- 40000 | 0 960000 | 0 19200000 | 1 25000000 | 1 50000000 | 2 74000000 | 3 clk2 Hz | pd2 Corner ------------+----------- 19200000 | 0 150000000 | 1 340000000 | 2 BUT we also have another device that uses pd1 and has it's own clk3 with different frequency constraints: clk3 Hz | pd1 Corner -------------+----------- 250000000 | 0 360000000 | 1 730000000 | 2 So when clk3 is at 730000000, we don't care what frequency clk1 is running at, pd1 needs to be at least at corner 2. If it's at corner 3 because clk1 is at 74000000 then that's fine. I imagine DT would look like our "fmax tables" that are currently out of tree. Something like: clk1_fmax_table { fmax0 { reg = /bits/ 64 <960000>; qcom,corner = <0>; }; fmax1 { reg = /bits/ 64 <25000000>; qcom,corner = <1>; }; fmax2 { reg = /bits/ 64 <50000000>; qcom,corner = <2>; }; fmax3 { reg = /bits/ 64 <740000000>; qcom,corner = <3>; }; }; Which is similar to OPP table, but we only list the maximum frequency that requires a particular corner. We're back to the same problem we have with OPPs of figuring out how to relate a table to a certain clk and power domain though. At least for qcom, we could do that with some sort of complicated list property: emmc { performance-states = <&fmax_table &cc 1 &pd 1> }; or something like that which would parse a table, a clock, and a number of power domains. Reminds me, we can have one clk frequency map to multiple power domains too. We have this case for our CPUs and PLLs where we have individual power control on two domains for one frequency. So when the PLL frequency changes, we need to turn on and set the voltage or corner of two regulators. So I don't really know how this is all going to work. I'd really appreciate to see the full picture though. Reviewing this a bit at a time makes me lose sight of the bigger picture. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC V7 2/2] OPP: Allow "opp-hz" and "opp-microvolt" to contain magic values 2017-11-30 0:50 ` Stephen Boyd @ 2017-11-30 6:59 ` Viresh Kumar 2017-12-14 7:30 ` Viresh Kumar 2017-12-26 20:23 ` Rob Herring 0 siblings, 2 replies; 25+ messages in thread From: Viresh Kumar @ 2017-11-30 6:59 UTC (permalink / raw) To: Stephen Boyd Cc: Ulf Hansson, Rob Herring, Kevin Hilman, Viresh Kumar, Nishanth Menon, Rafael Wysocki, linux-pm, Vincent Guittot, Rajendra Nayak, Sudeep Holla, devicetree, linux-kernel On 29-11-17, 16:50, Stephen Boyd wrote: > Sorry it still makes zero sense to me. It seems that we're trying > to make the OPP table parsing generic just for the sake of code > brevity. Not just the code but bindings as well to make sure we don't add a new property (similar to earlier ones) for every platform that wants to use performance states. > Is this the goal? From a DT writer perspective it seems > confusing to say that opp-microvolt is sometimes a microvolt and > sometimes not a microvolt. Well it would still represent the voltage but not in microvolt units as the platform guys decided to hide those values from kernel and handle them directly in firmware. > Why can't the SoC specific genpd > driver parse something like "qcom,corner" instead out of the > node? Sure we can, but that means that a new property will be required for the next platform. I did it this way as Kevin (and Rob) suggested NOT to add another property but use the earlier ones as we aren't passing anything new here, just that the units of the property are different. For another SoC, we may want to hide both freq and voltage values from kernel and pass firmware dependent values. Should we add two new properties for that SoC then ? > BTW, I don't believe I have a use-case where I want to express > power domain OPP tables. I do remember that you once said [1] that you may want to pass the real voltage values as well via DT. And so I thought that you can pass performance-state (corner) in opp-hz and real voltage values in opp-microvolt. > I have many devices that all have > different frequencies that are all tied into the same power > domain. This binding makes it look like we can only have one > frequency per domain which won't work. No, that isn't the case. Looks like we have some confusion here. Let me try with a simple example: foo: foo-power-domain@09000000 { compatible = "foo,genpd"; #power-domain-cells = <0>; operating-points-v2 = <&domain_opp_table>; }; cpu0: cpu@0 { compatible = "arm,cortex-a53", "arm,armv8"; ... operating-points-v2 = <&cpu_opp_table>; power-domains = <&foo>; }; domain_opp_table: domain_opp_table { compatible = "operating-points-v2"; domain_opp_1: opp00 { opp-hz = /bits/ 64 <1>; /* These are corners AKA perf states */ }; domain_opp_2: opp01 { opp-hz = /bits/ 64 <2>; }; domain_opp_3: opp02 { opp-hz = /bits/ 64 <3>; }; }; cpu_opp_table: cpu_opp_table { compatible = "operating-points-v2"; opp-shared; opp00 { opp-hz = /bits/ 64 <208000000>; clock-latency-ns = <500000>; power-domain-opp = <&domain_opp_1>; }; opp01 { opp-hz = /bits/ 64 <432000000>; clock-latency-ns = <500000>; power-domain-opp = <&domain_opp_2>; }; opp02 { opp-hz = /bits/ 64 <729000000>; clock-latency-ns = <500000>; power-domain-opp = <&domain_opp_2>; }; opp03 { opp-hz = /bits/ 64 <960000000>; clock-latency-ns = <500000>; power-domain-opp = <&domain_opp_3>; }; }; The device frequencies are still managed by device's OPP table, just that device's OPP has OPP requirement from another device which is power domain in this case. > I want to express that a device with a range of frequencies (or > really multiple ranges of frequencies) is inside certain physical > power domains and the frequency of the clks dictates the minimum > voltage requirement of those power domains. > > For the most complicated case, imagine something like our eMMC > controller that has two clks (clk1,clk2) that it changes the rate > of independently and those two clks rely on two different > regulators (vreg1, vreg2) that supply voltage domains in the SoC > which the eMMC controller happens to be part of (pd1, pd2). And > the device is also part of another power domain that we use to > turn everything off (pd3). > > +-------+ +-------+ > | vreg1 | | vreg2 | > +---+---+ +----+--+ > | | > | +------------+-------------+ | > | | +-------+ | +--------+ | | > +-----> | clk1 | | | clk2 | <---------+ > | +---+---+ | +----+---+ | > | | | | | > +----------v--------------v-----------+ > | | | | | > | | | | | > | | pd1 | pd2 | | > | | | | | > | +------------+-------------+ | > | | > | pd3 eMMC | > +-------------------------------------+ > > > >From a DT perspective, I see this as one emmc node: > > pd: power-domain-controller { > #power-domain-cells = <1>; > }; > > cc: clock-controller { > #clock-cells = <1>; > }; > > emmc { > power-domains = <&pd 1> <&pd 2>, <&pd 3>; > clocks = <&cc 1> <&cc 2>; > } > > And then we really don't want to have to express every single > possible frequency that clk1 and clk2 can be just to express the > voltage/corner constraints. It could be that we have some table > like so: > > clk1 Hz | pd1 Corner > ------------+----------- > 40000 | 0 > 960000 | 0 > 19200000 | 1 > 25000000 | 1 > 50000000 | 2 > 74000000 | 3 > > clk2 Hz | pd2 Corner > ------------+----------- > 19200000 | 0 > 150000000 | 1 > 340000000 | 2 > > > BUT we also have another device that uses pd1 and has it's own > clk3 with different frequency constraints: > > clk3 Hz | pd1 Corner > -------------+----------- > 250000000 | 0 > 360000000 | 1 > 730000000 | 2 > > So when clk3 is at 730000000, we don't care what frequency clk1 > is running at, pd1 needs to be at least at corner 2. If it's at > corner 3 because clk1 is at 74000000 then that's fine. > > I imagine DT would look like our "fmax tables" that are currently > out of tree. Something like: > > clk1_fmax_table { > fmax0 { > reg = /bits/ 64 <960000>; > qcom,corner = <0>; > }; > fmax1 { > reg = /bits/ 64 <25000000>; > qcom,corner = <1>; > }; > fmax2 { > reg = /bits/ 64 <50000000>; > qcom,corner = <2>; > }; > fmax3 { > reg = /bits/ 64 <740000000>; > qcom,corner = <3>; > }; > }; > > Which is similar to OPP table, but we only list the maximum > frequency that requires a particular corner. We're back to the > same problem we have with OPPs of figuring out how to relate a > table to a certain clk and power domain though. At least for > qcom, we could do that with some sort of complicated list > property: > > emmc { > performance-states = <&fmax_table &cc 1 &pd 1> > }; > > or something like that which would parse a table, a clock, and a > number of power domains. Here is how this can be represented with the current proposal. pd: power-domain-controller { #power-domain-cells = <1>; operating-points-v2 = <&pd1_opp_table>, <&pd2_opp_table>, <&pd3_opp_table>; }; /* * The below OPP nodes can contain other properties like * microvolt and microamps, etc. * * Also if the below 3 tables are exactly same, then the same * table can be used for all the three power domains provided * by the above controller. */ pd1_opp_table: pd1_opp_table { compatible = "operating-points-v2"; pd1_opp_1: opp00 { opp-hz = /bits/ 64 <1>; /* These are corners AKA perf states */ }; pd1_opp_2: opp01 { opp-hz = /bits/ 64 <2>; }; pd1_opp_3: opp02 { opp-hz = /bits/ 64 <3>; }; }; pd2_opp_table: pd2_opp_table { compatible = "operating-points-v2"; pd2_opp_1: opp00 { opp-hz = /bits/ 64 <1>; /* These are corners AKA perf states */ }; pd2_opp_2: opp01 { opp-hz = /bits/ 64 <2>; }; pd2_opp_3: opp02 { opp-hz = /bits/ 64 <3>; }; }; pd3_opp_table: pd3_opp_table { compatible = "operating-points-v2"; pd3_opp_1: opp00 { opp-hz = /bits/ 64 <1>; /* These are corners AKA perf states */ }; pd3_opp_2: opp01 { opp-hz = /bits/ 64 <2>; }; pd3_opp_3: opp02 { opp-hz = /bits/ 64 <3>; }; }; cc: clock-controller { #clock-cells = <1>; }; emmc { power-domains = <&pd 1> <&pd 2>, <&pd 3>; clocks = <&cc 1> <&cc 2>; /* * We don't allow multiple OPP tables for devices * currently, but I think we need to use it for multiple * clk case, like emmc in your example. */ operating-points-v2 = <&cc1_opp_table>, <&cc2_opp_table>; } cc1_opp_table: cc1_opp_table { compatible = "operating-points-v2"; opp-shared; opp00 { opp-hz = /bits/ 64 <40000>; power-domain-opp = <&pd1_opp_1>; }; opp01 { opp-hz = /bits/ 64 <960000>; power-domain-opp = <&pd1_opp_1>; }; opp02 { opp-hz = /bits/ 64 <19200000>; power-domain-opp = <&pd1_opp_2>; }; opp03 { opp-hz = /bits/ 64 <25000000>; power-domain-opp = <&pd1_opp_2>; }; opp04 { opp-hz = /bits/ 64 <50000000>; power-domain-opp = <&pd1_opp_3>; }; opp05 { opp-hz = /bits/ 64 <74000000>; power-domain-opp = <&pd1_opp_3>; }; }; cc2_opp_table: cc2_opp_table { compatible = "operating-points-v2"; opp-shared; opp00 { opp-hz = /bits/ 64 <19200000>; power-domain-opp = <&pd2_opp_1>; }; opp01 { opp-hz = /bits/ 64 <150000000>; power-domain-opp = <&pd2_opp_2>; }; opp02 { opp-hz = /bits/ 64 <340000000>; power-domain-opp = <&pd2_opp_3>; }; }; Other devices can too have their OPP tables containing phandles to the pd1/2/3 OPPs. Wouldn't this work well ? > Reminds me, we can have one clk frequency map to multiple power > domains too. We have this case for our CPUs and PLLs where we > have individual power control on two domains for one frequency. > So when the PLL frequency changes, we need to turn on and set the > voltage or corner of two regulators. Sure, in that case the above "power-domain-opp" can contain a list. This is the multiple-power-domain case we have discussed multiple times earlier. > So I don't really know how this is all going to work. I am still hopeful :) > I'd really > appreciate to see the full picture though. Reviewing this a bit > at a time makes me lose sight of the bigger picture. I was about to publish code based on these bindings today, but then I received last minute comments from you and Rob and that work is going to wait a bit more :) -- viresh [1] https://marc.info/?l=linux-kernel&m=147995301320486&w=2 ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC V7 2/2] OPP: Allow "opp-hz" and "opp-microvolt" to contain magic values 2017-11-30 6:59 ` Viresh Kumar @ 2017-12-14 7:30 ` Viresh Kumar 2017-12-26 20:23 ` Rob Herring 1 sibling, 0 replies; 25+ messages in thread From: Viresh Kumar @ 2017-12-14 7:30 UTC (permalink / raw) To: Stephen Boyd Cc: Ulf Hansson, Rob Herring, Kevin Hilman, Viresh Kumar, Nishanth Menon, Rafael Wysocki, linux-pm, Vincent Guittot, Rajendra Nayak, Sudeep Holla, devicetree, linux-kernel On 30-11-17, 12:29, Viresh Kumar wrote: > On 29-11-17, 16:50, Stephen Boyd wrote: > > Sorry it still makes zero sense to me. It seems that we're trying > > to make the OPP table parsing generic just for the sake of code > > brevity. > > Not just the code but bindings as well to make sure we don't add a new > property (similar to earlier ones) for every platform that wants to > use performance states. @Stephen: Do you have any feedback on my previous email? If no, then I would like to refresh this version with the feedback received from Rob. -- viresh ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC V7 2/2] OPP: Allow "opp-hz" and "opp-microvolt" to contain magic values 2017-11-30 6:59 ` Viresh Kumar 2017-12-14 7:30 ` Viresh Kumar @ 2017-12-26 20:23 ` Rob Herring 2017-12-27 4:45 ` Viresh Kumar 1 sibling, 1 reply; 25+ messages in thread From: Rob Herring @ 2017-12-26 20:23 UTC (permalink / raw) To: Viresh Kumar Cc: Stephen Boyd, Ulf Hansson, Kevin Hilman, Viresh Kumar, Nishanth Menon, Rafael Wysocki, linux-pm, Vincent Guittot, Rajendra Nayak, Sudeep Holla, devicetree, linux-kernel On Thu, Nov 30, 2017 at 12:59 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote: > On 29-11-17, 16:50, Stephen Boyd wrote: >> Sorry it still makes zero sense to me. It seems that we're trying >> to make the OPP table parsing generic just for the sake of code >> brevity. > > Not just the code but bindings as well to make sure we don't add a new > property (similar to earlier ones) for every platform that wants to > use performance states. > >> Is this the goal? From a DT writer perspective it seems >> confusing to say that opp-microvolt is sometimes a microvolt and >> sometimes not a microvolt. > > Well it would still represent the voltage but not in microvolt units > as the platform guys decided to hide those values from kernel and > handle them directly in firmware. > >> Why can't the SoC specific genpd >> driver parse something like "qcom,corner" instead out of the >> node? > > Sure we can, but that means that a new property will be required for > the next platform. > > I did it this way as Kevin (and Rob) suggested NOT to add another > property but use the earlier ones as we aren't passing anything new > here, just that the units of the property are different. For another > SoC, we may want to hide both freq and voltage values from kernel and > pass firmware dependent values. Should we add two new properties for > that SoC then ? > >> BTW, I don't believe I have a use-case where I want to express >> power domain OPP tables. > > I do remember that you once said [1] that you may want to pass the > real voltage values as well via DT. And so I thought that you can pass > performance-state (corner) in opp-hz and real voltage values in > opp-microvolt. > >> I have many devices that all have >> different frequencies that are all tied into the same power >> domain. This binding makes it look like we can only have one >> frequency per domain which won't work. > > No, that isn't the case. Looks like we have some confusion here. Let > me try with a simple example: > > foo: foo-power-domain@09000000 { > compatible = "foo,genpd"; > #power-domain-cells = <0>; > operating-points-v2 = <&domain_opp_table>; > }; > > cpu0: cpu@0 { > compatible = "arm,cortex-a53", "arm,armv8"; > ... > operating-points-v2 = <&cpu_opp_table>; > power-domains = <&foo>; > }; > > > domain_opp_table: domain_opp_table { > compatible = "operating-points-v2"; > > domain_opp_1: opp00 { > opp-hz = /bits/ 64 <1>; /* These are corners AKA perf states */ > }; > domain_opp_2: opp01 { > opp-hz = /bits/ 64 <2>; > }; > domain_opp_3: opp02 { > opp-hz = /bits/ 64 <3>; > }; > }; > > cpu_opp_table: cpu_opp_table { > compatible = "operating-points-v2"; > opp-shared; > > opp00 { > opp-hz = /bits/ 64 <208000000>; > clock-latency-ns = <500000>; > power-domain-opp = <&domain_opp_1>; What is this? opp00 here is not a device. One OPP should not point to another. "power-domain-opp" is only supposed to appear in devices alongside power-domains properties. Rob ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC V7 2/2] OPP: Allow "opp-hz" and "opp-microvolt" to contain magic values 2017-12-26 20:23 ` Rob Herring @ 2017-12-27 4:45 ` Viresh Kumar 2017-12-27 21:36 ` Rob Herring 0 siblings, 1 reply; 25+ messages in thread From: Viresh Kumar @ 2017-12-27 4:45 UTC (permalink / raw) To: Rob Herring Cc: Stephen Boyd, Ulf Hansson, Kevin Hilman, Viresh Kumar, Nishanth Menon, Rafael Wysocki, linux-pm, Vincent Guittot, Rajendra Nayak, Sudeep Holla, devicetree, linux-kernel On 26-12-17, 14:23, Rob Herring wrote: > > cpu_opp_table: cpu_opp_table { > > compatible = "operating-points-v2"; > > opp-shared; > > > > opp00 { > > opp-hz = /bits/ 64 <208000000>; > > clock-latency-ns = <500000>; > > power-domain-opp = <&domain_opp_1>; > > What is this? opp00 here is not a device. One OPP should not point to > another. "power-domain-opp" is only supposed to appear in devices > alongside power-domains properties. There are two type of devices: A.) With fixed performance state requirements and they will have the new "required-opp" property in the device node itself as you said. B.) Devices which can do DVFS (CPU, MMC, LCD, etc) and those may need a different performance state of the domain for their individual OPPs and so we can't have this property in the device all the time. Does this make sense ? -- viresh ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC V7 2/2] OPP: Allow "opp-hz" and "opp-microvolt" to contain magic values 2017-12-27 4:45 ` Viresh Kumar @ 2017-12-27 21:36 ` Rob Herring 2017-12-28 4:32 ` Viresh Kumar 0 siblings, 1 reply; 25+ messages in thread From: Rob Herring @ 2017-12-27 21:36 UTC (permalink / raw) To: Viresh Kumar Cc: Stephen Boyd, Ulf Hansson, Kevin Hilman, Viresh Kumar, Nishanth Menon, Rafael Wysocki, linux-pm, Vincent Guittot, Rajendra Nayak, Sudeep Holla, devicetree, linux-kernel On Tue, Dec 26, 2017 at 10:45 PM, Viresh Kumar <viresh.kumar@linaro.org> wrote: > On 26-12-17, 14:23, Rob Herring wrote: >> > cpu_opp_table: cpu_opp_table { >> > compatible = "operating-points-v2"; >> > opp-shared; >> > >> > opp00 { >> > opp-hz = /bits/ 64 <208000000>; >> > clock-latency-ns = <500000>; >> > power-domain-opp = <&domain_opp_1>; >> >> What is this? opp00 here is not a device. One OPP should not point to >> another. "power-domain-opp" is only supposed to appear in devices >> alongside power-domains properties. > > There are two type of devices: > > A.) With fixed performance state requirements and they will have the > new "required-opp" property in the device node itself as you said. > > B.) Devices which can do DVFS (CPU, MMC, LCD, etc) and those may need > a different performance state of the domain for their individual OPPs > and so we can't have this property in the device all the time. > > Does this make sense ? No. From the definition for power-domain-opp "+- power-domain-opp: This contains phandle to one of the OPP nodes of the master + power domain. This specifies the minimum required OPP of the master domain for + the functioning of the device in this OPP (where this property is present). + This property can only be set for a device if the device node contains the + "power-domains" property. Also, either all or none of the OPP nodes in an OPP + table should have it set." In the above example, you are violating the next to last sentence. Though, I'm now confused by what the last sentence means. Rob ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC V7 2/2] OPP: Allow "opp-hz" and "opp-microvolt" to contain magic values 2017-12-27 21:36 ` Rob Herring @ 2017-12-28 4:32 ` Viresh Kumar 0 siblings, 0 replies; 25+ messages in thread From: Viresh Kumar @ 2017-12-28 4:32 UTC (permalink / raw) To: Rob Herring Cc: Stephen Boyd, Ulf Hansson, Kevin Hilman, Viresh Kumar, Nishanth Menon, Rafael Wysocki, linux-pm, Vincent Guittot, Rajendra Nayak, Sudeep Holla, devicetree, linux-kernel On 27-12-17, 15:36, Rob Herring wrote: > On Tue, Dec 26, 2017 at 10:45 PM, Viresh Kumar <viresh.kumar@linaro.org> wrote: > > On 26-12-17, 14:23, Rob Herring wrote: > >> > cpu_opp_table: cpu_opp_table { > >> > compatible = "operating-points-v2"; > >> > opp-shared; > >> > > >> > opp00 { > >> > opp-hz = /bits/ 64 <208000000>; > >> > clock-latency-ns = <500000>; > >> > power-domain-opp = <&domain_opp_1>; > >> > >> What is this? opp00 here is not a device. One OPP should not point to > >> another. "power-domain-opp" is only supposed to appear in devices > >> alongside power-domains properties. > > > > There are two type of devices: > > > > A.) With fixed performance state requirements and they will have the > > new "required-opp" property in the device node itself as you said. > > > > B.) Devices which can do DVFS (CPU, MMC, LCD, etc) and those may need > > a different performance state of the domain for their individual OPPs > > and so we can't have this property in the device all the time. > > > > Does this make sense ? > > No. From the definition for power-domain-opp > > "+- power-domain-opp: This contains phandle to one of the OPP nodes of > the master > + power domain. This specifies the minimum required OPP of the master > domain for > + the functioning of the device in this OPP (where this property is present). The per-opp thing was mentioned here. > + This property can only be set for a device if the device node contains the > + "power-domains" property. This was trying to say something else, though it wasn't clear and so your concerns. I wanted to say that the device node or its OPP nodes can have the "power-domain-opp" property only if the device node has a "power-domains" property. i.e. you need to have power domain first and then only the power-domain-opp property. > Also, either all or none of the OPP nodes in an OPP > + table should have it set." > > In the above example, you are violating the next to last sentence. > > Though, I'm now confused by what the last sentence means. Yeah, lets leave it as is as the V8 has changed this significantly and you already Acked it :) -- viresh ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC V7 2/2] OPP: Allow "opp-hz" and "opp-microvolt" to contain magic values 2017-11-01 20:39 ` Rob Herring 2017-11-01 21:43 ` Stephen Boyd @ 2017-11-02 4:49 ` Viresh Kumar 1 sibling, 0 replies; 25+ messages in thread From: Viresh Kumar @ 2017-11-02 4:49 UTC (permalink / raw) To: Rob Herring Cc: Ulf Hansson, Kevin Hilman, Viresh Kumar, Nishanth Menon, Stephen Boyd, Rafael Wysocki, linux-pm, Vincent Guittot, Rajendra Nayak, Sudeep Holla, devicetree, linux-kernel On 01-11-17, 15:39, Rob Herring wrote: > On Tue, Oct 31, 2017 at 9:17 PM, Viresh Kumar <viresh.kumar@linaro.org> wrote: > > On 31 October 2017 at 16:02, Rob Herring <robh+dt@kernel.org> wrote: > >> Why not a new property for magic values? opp-magic? Don't we want to > >> know when we have magic values? > > > > I have kept a separate property since beginning (domain-performance-state) > > and moved to using these magic values in the existing field because of the > > suggestion Kevin gave earlier. > > > > https://marc.info/?l=linux-kernel&m=149306082218001&w=2 > > > > I am not sure what to do now :) > > Okay, I guess reusing the properties is fine. Okay, great. > >> Wouldn't magic values in opp-hz get propagated to user space? > > > > The OPP core puts them in debugfs just to know how the OPPs are > > set. Otherwise, I am not sure that the power domain core/drivers would > > be exposing that to user space. > > I was thinking thru the cpufreq interface, but I guess this is not for cpus. Oh no. That's not the target here for sure. -- viresh ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC V7 2/2] OPP: Allow "opp-hz" and "opp-microvolt" to contain magic values 2017-10-31 12:47 ` [RFC V7 2/2] OPP: Allow "opp-hz" and "opp-microvolt" to contain magic values Viresh Kumar 2017-10-31 16:02 ` Rob Herring @ 2017-11-28 16:14 ` Ulf Hansson 1 sibling, 0 replies; 25+ messages in thread From: Ulf Hansson @ 2017-11-28 16:14 UTC (permalink / raw) To: Viresh Kumar Cc: Kevin Hilman, Viresh Kumar, Nishanth Menon, Stephen Boyd, Rafael Wysocki, linux-pm, Vincent Guittot, Rob Herring, Rajendra Nayak, Sudeep Holla, devicetree, linux-kernel On 31 October 2017 at 13:47, Viresh Kumar <viresh.kumar@linaro.org> wrote: > On some platforms the exact frequency or voltage may be hidden from the > OS by the firmware. Allow such configurations to pass magic values in > the "opp-hz" or the "opp-microvolt" properties, which should be > interpreted in a platform dependent way. > > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> This seems like a reasonable extension to me! Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org> Kind regards Uffe > --- > Documentation/devicetree/bindings/opp/opp.txt | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt > index 203e09fe7698..9c5056fb120f 100644 > --- a/Documentation/devicetree/bindings/opp/opp.txt > +++ b/Documentation/devicetree/bindings/opp/opp.txt > @@ -166,6 +166,12 @@ properties. > "power-domains" property. Also, either all or none of the OPP nodes in an OPP > table should have it set. > > + > +On some platforms the exact frequency or voltage may be hidden from the OS by > +the firmware and the "opp-hz" or the "opp-microvolt" properties may contain > +magic values that represent the frequency or voltage in a firmware dependent > +way, for example an index of an array in the firmware. > + > Example 1: Single cluster Dual-core ARM cortex A9, switch DVFS states together. > > / { > -- > 2.15.0.rc1.236.g92ea95045093 > ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC V7 0/2] OPP: Allow OPP table to be used for power-domains 2017-10-31 12:47 [RFC V7 0/2] OPP: Allow OPP table to be used for power-domains Viresh Kumar 2017-10-31 12:47 ` [RFC V7 1/2] " Viresh Kumar 2017-10-31 12:47 ` [RFC V7 2/2] OPP: Allow "opp-hz" and "opp-microvolt" to contain magic values Viresh Kumar @ 2017-11-29 4:14 ` Viresh Kumar 2017-11-29 16:37 ` Rob Herring 2 siblings, 1 reply; 25+ messages in thread From: Viresh Kumar @ 2017-11-29 4:14 UTC (permalink / raw) To: Stephen Boyd, robh+dt Cc: Rafael Wysocki, linux-pm, Nishanth Menon, Vincent Guittot, rnayak, sudeep.holla, devicetree, linux-kernel, Viresh Kumar, ulf.hansson, Kevin Hilman On 31-10-17, 18:17, Viresh Kumar wrote: > Hi, > > Now that the performance state of PM domains are supported by the kernel > (merged in linux-next), I am trying once again to define the bindings > which we dropped until the code is merged first. @Rob/Stephen: Can I get Ack from you guys on this series, before I apply them? Ulf has already reviewed these. -- viresh ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC V7 0/2] OPP: Allow OPP table to be used for power-domains 2017-11-29 4:14 ` [RFC V7 0/2] OPP: Allow OPP table to be used for power-domains Viresh Kumar @ 2017-11-29 16:37 ` Rob Herring 0 siblings, 0 replies; 25+ messages in thread From: Rob Herring @ 2017-11-29 16:37 UTC (permalink / raw) To: Viresh Kumar Cc: Stephen Boyd, Rafael Wysocki, linux-pm, Nishanth Menon, Vincent Guittot, Rajendra Nayak, Sudeep Holla, devicetree, linux-kernel, Viresh Kumar, Ulf Hansson, Kevin Hilman On Tue, Nov 28, 2017 at 10:14 PM, Viresh Kumar <viresh.kumar@linaro.org> wrote: > On 31-10-17, 18:17, Viresh Kumar wrote: >> Hi, >> >> Now that the performance state of PM domains are supported by the kernel >> (merged in linux-next), I am trying once again to define the bindings >> which we dropped until the code is merged first. > > @Rob/Stephen: Can I get Ack from you guys on this series, before I > apply them? Ulf has already reviewed these. I don't ack RFCs. In any case, I'm waiting to see what Stephen says. Rob ^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2017-12-28 4:32 UTC | newest] Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-10-31 12:47 [RFC V7 0/2] OPP: Allow OPP table to be used for power-domains Viresh Kumar 2017-10-31 12:47 ` [RFC V7 1/2] " Viresh Kumar 2017-11-28 15:50 ` Ulf Hansson 2017-11-29 16:46 ` Rob Herring 2017-11-30 4:48 ` Viresh Kumar 2017-10-31 12:47 ` [RFC V7 2/2] OPP: Allow "opp-hz" and "opp-microvolt" to contain magic values Viresh Kumar 2017-10-31 16:02 ` Rob Herring 2017-11-01 2:17 ` Viresh Kumar 2017-11-01 20:39 ` Rob Herring 2017-11-01 21:43 ` Stephen Boyd 2017-11-02 4:51 ` Viresh Kumar 2017-11-02 7:15 ` Stephen Boyd 2017-11-02 9:00 ` Viresh Kumar 2017-11-28 16:38 ` Ulf Hansson 2017-11-30 0:50 ` Stephen Boyd 2017-11-30 6:59 ` Viresh Kumar 2017-12-14 7:30 ` Viresh Kumar 2017-12-26 20:23 ` Rob Herring 2017-12-27 4:45 ` Viresh Kumar 2017-12-27 21:36 ` Rob Herring 2017-12-28 4:32 ` Viresh Kumar 2017-11-02 4:49 ` Viresh Kumar 2017-11-28 16:14 ` Ulf Hansson 2017-11-29 4:14 ` [RFC V7 0/2] OPP: Allow OPP table to be used for power-domains Viresh Kumar 2017-11-29 16:37 ` Rob Herring
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).