[V4,1/9] PM / OPP: Allow OPP table to be used for power-domains

Power-domains need to express their active states in DT and what's
better than OPP table for that.

This patch allows power-domains to reuse OPP tables to express their
active states. The "opp-hz" property isn't a required property anymore
as power-domains may not always use them.

Add a new property "domain-performance-state", which will contain
positive integer values to represent performance levels of the
power-domains as described in this patch.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 Documentation/devicetree/bindings/opp/opp.txt | 73 ++++++++++++++++++++++++++-
 1 file changed, 71 insertions(+), 2 deletions(-)

On Mon, Mar 20, 2017 at 03:02:13PM +0530, Viresh Kumar wrote:
> Power-domains need to express their active states in DT and what's
> better than OPP table for that.
> 
> This patch allows power-domains to reuse OPP tables to express their
> active states. The "opp-hz" property isn't a required property anymore
> as power-domains may not always use them.

Then maybe you shouldn't be trying to make OPP table work here. At that 
point you just need a table of voltage(s) per performance state?

> Add a new property "domain-performance-state", which will contain
> positive integer values to represent performance levels of the
> power-domains as described in this patch.

Why not reference the OPP entries from the domain:

performance-states = <&opp1>, <&opp2>;

Just thinking out loud, not saying that is what you should do. The 
continual evolution of power (management) domain, idle state, and OPP 
bindings is getting tiring.

> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  Documentation/devicetree/bindings/opp/opp.txt | 73 ++++++++++++++++++++++++++-
>  1 file changed, 71 insertions(+), 2 deletions(-)
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On 24-03-17, 10:44, Rob Herring wrote:
> On Mon, Mar 20, 2017 at 03:02:13PM +0530, Viresh Kumar wrote:
> > Power-domains need to express their active states in DT and what's
> > better than OPP table for that.
> > 
> > This patch allows power-domains to reuse OPP tables to express their
> > active states. The "opp-hz" property isn't a required property anymore
> > as power-domains may not always use them.
> 
> Then maybe you shouldn't be trying to make OPP table work here. At that 
> point you just need a table of voltage(s) per performance state?

Because that's what Kevin strongly recommended in the previous
versions.

@Kevin: Would you like to reply here ?

> > Add a new property "domain-performance-state", which will contain
> > positive integer values to represent performance levels of the
> > power-domains as described in this patch.
> 
> Why not reference the OPP entries from the domain:
> 
> performance-states = <&opp1>, <&opp2>;

Because that would require additional code in the OPP core to parse
these then. Right now it is quite straight forward with the bindings I
presented.

> Just thinking out loud, not saying that is what you should do. The 
> continual evolution of power (management) domain, idle state, and OPP 
> bindings is getting tiring.

I agree :)

Fixing Kevin's email id :(

On 10-04-17, 14:55, Viresh Kumar wrote:
> On 24-03-17, 10:44, Rob Herring wrote:
> > On Mon, Mar 20, 2017 at 03:02:13PM +0530, Viresh Kumar wrote:
> > > Power-domains need to express their active states in DT and what's
> > > better than OPP table for that.
> > > 
> > > This patch allows power-domains to reuse OPP tables to express their
> > > active states. The "opp-hz" property isn't a required property anymore
> > > as power-domains may not always use them.
> > 
> > Then maybe you shouldn't be trying to make OPP table work here. At that 
> > point you just need a table of voltage(s) per performance state?
> 
> Because that's what Kevin strongly recommended in the previous
> versions.
> 
> @Kevin: Would you like to reply here ?
> 
> > > Add a new property "domain-performance-state", which will contain
> > > positive integer values to represent performance levels of the
> > > power-domains as described in this patch.
> > 
> > Why not reference the OPP entries from the domain:
> > 
> > performance-states = <&opp1>, <&opp2>;
> 
> Because that would require additional code in the OPP core to parse
> these then. Right now it is quite straight forward with the bindings I
> presented.
> 
> > Just thinking out loud, not saying that is what you should do. The 
> > continual evolution of power (management) domain, idle state, and OPP 
> > bindings is getting tiring.
> 
> I agree :)

On 20/03/17 09:32, Viresh Kumar wrote:
> Power-domains need to express their active states in DT and what's
> better than OPP table for that.
> 
> This patch allows power-domains to reuse OPP tables to express their
> active states. The "opp-hz" property isn't a required property anymore
> as power-domains may not always use them.
> 
> Add a new property "domain-performance-state", which will contain
> positive integer values to represent performance levels of the
> power-domains as described in this patch.
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  Documentation/devicetree/bindings/opp/opp.txt | 73 ++++++++++++++++++++++++++-
>  1 file changed, 71 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
> index 63725498bd20..d0b95c9e1011 100644
> --- a/Documentation/devicetree/bindings/opp/opp.txt
> +++ b/Documentation/devicetree/bindings/opp/opp.txt
> @@ -76,10 +76,9 @@ This describes the OPPs belonging to a device. This node can have following
>  This defines voltage-current-frequency combinations along with other related
>  properties.
>  
> -Required properties:
> +Optional properties:
>  - opp-hz: Frequency in Hz, expressed as a 64-bit big-endian integer.
>  
> -Optional properties:
>  - opp-microvolt: voltage in micro Volts.
>  
>    A single regulator's voltage is specified with an array of size one or three.
> @@ -154,6 +153,19 @@ properties.
>  
>  - status: Marks the node enabled/disabled.
>  
> +- domain-performance-state: A positive integer value representing the minimum
> +  power-domain performance level required by the device for the OPP node. The

So the above definition is when this field in in the device node rather
than the OPP table entry, right ? For simplicity why not have the
properties named slightly different or just use phandle to an entry in
the device node for this purpose.

> +  The integer value '0' represents the lowest performance level and the higher
> +  values represent higher performance levels. 

needs to be changed as OPP table entry.

>  When present in the OPP table of a
> + power-domain, it represents the performance level of the domain. When present

again "performance level of the domain corresponding to that OPP entry"
on something similar

> +  in the OPP table of a normal device, it represents the performance level of

what do you mean by normal device ? needs description as that's
something new introduced here.

> +  the parent power-domain. The OPP table can contain the
> +  "domain-performance-state" property, only if the device node contains the
> +  "power-domains" or "#power-domain-cells" property. 

Why such a restriction ?

> The OPP nodes aren't
> +  allowed to contain the "domain-performance-state" property partially, i.e.
> +  Either all OPP nodes in the OPP table have the "domain-performance-state"
> +  property or none of them have it.
> +
>  Example 1: Single cluster Dual-core ARM cortex A9, switch DVFS states together.
>  
>  / {
> @@ -528,3 +540,60 @@ Example 5: opp-supported-hw
>  		};
>  	};
>  };
> +
> +Example 7: domain-Performance-state:
> +(example: For 1GHz require domain state 1 and for 1.1 & 1.2 GHz require state 2)
> +
> +/ {
> +	domain_opp_table: opp_table0 {
> +		compatible = "operating-points-v2";
> +
> +		opp@1 {
> +			domain-performance-state = <1>;
> +			opp-microvolt = <975000 970000 985000>;
> +		};
> +		opp@2 {
> +			domain-performance-state = <2>;
> +			opp-microvolt = <1075000 1000000 1085000>;
> +		};
> +	};
> +
> +	foo_domain: power-controller@12340000 {
> +		compatible = "foo,power-controller";
> +		reg = <0x12340000 0x1000>;
> +		#power-domain-cells = <0>;
> +		operating-points-v2 = <&domain_opp_table>;

How does it scale with power domain providers with multiple power domain ?

> +	}
> +
> +	cpu0_opp_table: opp_table1 {
> +		compatible = "operating-points-v2";
> +		opp-shared;
> +
> +		opp@1000000000 {
> +			opp-hz = /bits/ 64 <1000000000>;
> +			domain-performance-state = <1>;
> +		};
> +		opp@1100000000 {
> +			opp-hz = /bits/ 64 <1100000000>;
> +			domain-performance-state = <2>;
> +		};
> +		opp@1200000000 {
> +			opp-hz = /bits/ 64 <1200000000>;
> +			domain-performance-state = <2>;
> +		};
> +	};
> +
> +	cpus {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		cpu@0 {
> +			compatible = "arm,cortex-a9";
> +			reg = <0>;
> +			clocks = <&clk_controller 0>;
> +			clock-names = "cpu";
> +			operating-points-v2 = <&cpu0_opp_table>;

Do we ignore operating-points-v2 above as this device/cpu node contains
power domain which has operating-points-v2 property ? In other words
how do they correlate ?

> +			power-domains = <&foo_domain>;
> +		};
> +	};
> +};
>

On 20/03/17 09:32, Viresh Kumar wrote:
[...]

> +
> +Example 7: domain-Performance-state:
> +(example: For 1GHz require domain state 1 and for 1.1 & 1.2 GHz require state 2)
> +
> +/ {
> +	domain_opp_table: opp_table0 {
> +		compatible = "operating-points-v2";
> +
> +		opp@1 {
> +			domain-performance-state = <1>;
> +			opp-microvolt = <975000 970000 985000>;
> +		};
> +		opp@2 {
> +			domain-performance-state = <2>;
> +			opp-microvolt = <1075000 1000000 1085000>;
> +		};
> +	};
> +
> +	foo_domain: power-controller@12340000 {
> +		compatible = "foo,power-controller";
> +		reg = <0x12340000 0x1000>;
> +		#power-domain-cells = <0>;
> +		operating-points-v2 = <&domain_opp_table>;
> +	}
> +
> +	cpu0_opp_table: opp_table1 {
> +		compatible = "operating-points-v2";
> +		opp-shared;
> +
> +		opp@1000000000 {
> +			opp-hz = /bits/ 64 <1000000000>;
> +			domain-performance-state = <1>;
> +		};
> +		opp@1100000000 {
> +			opp-hz = /bits/ 64 <1100000000>;
> +			domain-performance-state = <2>;
> +		};
> +		opp@1200000000 {
> +			opp-hz = /bits/ 64 <1200000000>;
> +			domain-performance-state = <2>;
> +		};
> +	};
> +
> +	cpus {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		cpu@0 {
> +			compatible = "arm,cortex-a9";
> +			reg = <0>;
> +			clocks = <&clk_controller 0>;
> +			clock-names = "cpu";
> +			operating-points-v2 = <&cpu0_opp_table>;
> +			power-domains = <&foo_domain>;
> +		};
> +	};
> +};


Thinking more about this above example, I think you need more
explanation. So in the above case you have cpu with clock controller,
power-domain and the OPP table info, I can think of few things that need
to be explicit:

1. How does the precedence look like ?

2. Since power-domains with OPP table control the performance state, do
   we ignore clock and operating-points-v2 in the above case completely?

3. Will the power-domain drive the OPP ?

On 12-04-17, 17:49, Sudeep Holla wrote:
> On 20/03/17 09:32, Viresh Kumar wrote:
> > diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
> > index 63725498bd20..d0b95c9e1011 100644
> > --- a/Documentation/devicetree/bindings/opp/opp.txt
> > +++ b/Documentation/devicetree/bindings/opp/opp.txt
> > @@ -76,10 +76,9 @@ This describes the OPPs belonging to a device. This node can have following
> >  This defines voltage-current-frequency combinations along with other related
> >  properties.
> >  
> > -Required properties:
> > +Optional properties:
> >  - opp-hz: Frequency in Hz, expressed as a 64-bit big-endian integer.
> >  
> > -Optional properties:
> >  - opp-microvolt: voltage in micro Volts.
> >  
> >    A single regulator's voltage is specified with an array of size one or three.
> > @@ -154,6 +153,19 @@ properties.
> >  
> >  - status: Marks the node enabled/disabled.
> >  
> > +- domain-performance-state: A positive integer value representing the minimum
> > +  power-domain performance level required by the device for the OPP node. The
> 
> So the above definition is when this field in in the device node rather
> than the OPP table entry, right ?

No. We are updating the opp.txt file here and so it is not about the
device node. The OPP node entries will contain this field for two
cases:
- The OPP table belongs to a power domain
- The OPP table belongs to a device whose power domain supports
  performance-states.

> For simplicity why not have the
> properties named slightly different or just use phandle to an entry in
> the device node for this purpose.

We really need a value here. For example, in case where the OPP table
defines the states of the power-domain itself, we don't have any
phandles to point to.

> > +  The integer value '0' represents the lowest performance level and the higher
> > +  values represent higher performance levels. 
> 
> needs to be changed as OPP table entry.

Not sure I understood what change you are looking for :(

> >  When present in the OPP table of a
> > + power-domain, it represents the performance level of the domain. When present
> 
> again "performance level of the domain corresponding to that OPP entry"
> on something similar

Ok.

> > +  in the OPP table of a normal device, it represents the performance level of
> 
> what do you mean by normal device ? needs description as that's
> something new introduced here.

It should be non-power-domain node.

> > +  the parent power-domain. The OPP table can contain the
> > +  "domain-performance-state" property, only if the device node contains the
> > +  "power-domains" or "#power-domain-cells" property. 
> 
> Why such a restriction ?

Why would we use it for non-power-domain cases? That's not what we
are looking for..

> > The OPP nodes aren't
> > +  allowed to contain the "domain-performance-state" property partially, i.e.
> > +  Either all OPP nodes in the OPP table have the "domain-performance-state"
> > +  property or none of them have it.
> > +
> >  Example 1: Single cluster Dual-core ARM cortex A9, switch DVFS states together.
> >  
> >  / {
> > @@ -528,3 +540,60 @@ Example 5: opp-supported-hw
> >  		};
> >  	};
> >  };
> > +
> > +Example 7: domain-Performance-state:
> > +(example: For 1GHz require domain state 1 and for 1.1 & 1.2 GHz require state 2)
> > +
> > +/ {
> > +	domain_opp_table: opp_table0 {
> > +		compatible = "operating-points-v2";
> > +
> > +		opp@1 {
> > +			domain-performance-state = <1>;
> > +			opp-microvolt = <975000 970000 985000>;
> > +		};
> > +		opp@2 {
> > +			domain-performance-state = <2>;
> > +			opp-microvolt = <1075000 1000000 1085000>;
> > +		};
> > +	};
> > +
> > +	foo_domain: power-controller@12340000 {
> > +		compatible = "foo,power-controller";
> > +		reg = <0x12340000 0x1000>;
> > +		#power-domain-cells = <0>;
> > +		operating-points-v2 = <&domain_opp_table>;
> 
> How does it scale with power domain providers with multiple power domain ?

Devices can't have multiple power domains today. Will see this when
that support is added.

Note that only the power domains can have multiple parent power
domains today.

> > +	}
> > +
> > +	cpu0_opp_table: opp_table1 {
> > +		compatible = "operating-points-v2";
> > +		opp-shared;
> > +
> > +		opp@1000000000 {
> > +			opp-hz = /bits/ 64 <1000000000>;
> > +			domain-performance-state = <1>;
> > +		};
> > +		opp@1100000000 {
> > +			opp-hz = /bits/ 64 <1100000000>;
> > +			domain-performance-state = <2>;
> > +		};
> > +		opp@1200000000 {
> > +			opp-hz = /bits/ 64 <1200000000>;
> > +			domain-performance-state = <2>;
> > +		};
> > +	};
> > +
> > +	cpus {
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +
> > +		cpu@0 {
> > +			compatible = "arm,cortex-a9";
> > +			reg = <0>;
> > +			clocks = <&clk_controller 0>;
> > +			clock-names = "cpu";
> > +			operating-points-v2 = <&cpu0_opp_table>;
> 
> Do we ignore operating-points-v2 above as this device/cpu node contains
> power domain which has operating-points-v2 property ? In other words
> how do they correlate ?

Devices and their power domains can both have their performance
states. Just that to get the device in a particular state, we may need
to get its power domain to a particular state first.

On 12-04-17, 18:05, Sudeep Holla wrote:
> 
> 
> On 20/03/17 09:32, Viresh Kumar wrote:
> [...]
> 
> > +
> > +Example 7: domain-Performance-state:
> > +(example: For 1GHz require domain state 1 and for 1.1 & 1.2 GHz require state 2)
> > +
> > +/ {
> > +	domain_opp_table: opp_table0 {
> > +		compatible = "operating-points-v2";
> > +
> > +		opp@1 {
> > +			domain-performance-state = <1>;
> > +			opp-microvolt = <975000 970000 985000>;
> > +		};
> > +		opp@2 {
> > +			domain-performance-state = <2>;
> > +			opp-microvolt = <1075000 1000000 1085000>;
> > +		};
> > +	};
> > +
> > +	foo_domain: power-controller@12340000 {
> > +		compatible = "foo,power-controller";
> > +		reg = <0x12340000 0x1000>;
> > +		#power-domain-cells = <0>;
> > +		operating-points-v2 = <&domain_opp_table>;
> > +	}
> > +
> > +	cpu0_opp_table: opp_table1 {
> > +		compatible = "operating-points-v2";
> > +		opp-shared;
> > +
> > +		opp@1000000000 {
> > +			opp-hz = /bits/ 64 <1000000000>;
> > +			domain-performance-state = <1>;
> > +		};
> > +		opp@1100000000 {
> > +			opp-hz = /bits/ 64 <1100000000>;
> > +			domain-performance-state = <2>;
> > +		};
> > +		opp@1200000000 {
> > +			opp-hz = /bits/ 64 <1200000000>;
> > +			domain-performance-state = <2>;
> > +		};
> > +	};
> > +
> > +	cpus {
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +
> > +		cpu@0 {
> > +			compatible = "arm,cortex-a9";
> > +			reg = <0>;
> > +			clocks = <&clk_controller 0>;
> > +			clock-names = "cpu";
> > +			operating-points-v2 = <&cpu0_opp_table>;
> > +			power-domains = <&foo_domain>;
> > +		};
> > +	};
> > +};
> 
> 
> Thinking more about this above example, I think you need more
> explanation. So in the above case you have cpu with clock controller,
> power-domain and the OPP table info, I can think of few things that need
> to be explicit:
> 
> 1. How does the precedence look like ?

Just think of the power-domain as a regulator here. If we are
increasing frequency of the device, power-domain needs to be
programmed first followed by the clock.

> 2. Since power-domains with OPP table control the performance state, do

They control performance state of the domains, not the devices.

>    we ignore clock and operating-points-v2 in the above case completely?

No. They are separate.

> 
> 3. Will the power-domain drive the OPP ?

power-domain will driver its own state using its own OPP table.
Devices may fine tune within those states.

On 13/04/17 06:37, Viresh Kumar wrote:
> On 12-04-17, 17:49, Sudeep Holla wrote:
>> On 20/03/17 09:32, Viresh Kumar wrote:
>>> diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
>>> index 63725498bd20..d0b95c9e1011 100644
>>> --- a/Documentation/devicetree/bindings/opp/opp.txt
>>> +++ b/Documentation/devicetree/bindings/opp/opp.txt
>>> @@ -76,10 +76,9 @@ This describes the OPPs belonging to a device. This node can have following
>>>  This defines voltage-current-frequency combinations along with other related
>>>  properties.
>>>  
>>> -Required properties:
>>> +Optional properties:
>>>  - opp-hz: Frequency in Hz, expressed as a 64-bit big-endian integer.
>>>  
>>> -Optional properties:
>>>  - opp-microvolt: voltage in micro Volts.
>>>  
>>>    A single regulator's voltage is specified with an array of size one or three.
>>> @@ -154,6 +153,19 @@ properties.
>>>  
>>>  - status: Marks the node enabled/disabled.
>>>  
>>> +- domain-performance-state: A positive integer value representing the minimum
>>> +  power-domain performance level required by the device for the OPP node. The
>>
>> So the above definition is when this field in in the device node rather
>> than the OPP table entry, right ?
> 
> No. We are updating the opp.txt file here and so it is not about the
> device node. The OPP node entries will contain this field for two
> cases:
> - The OPP table belongs to a power domain
> - The OPP table belongs to a device whose power domain supports
>   performance-states.
> 

Understood.

>> For simplicity why not have the
>> properties named slightly different or just use phandle to an entry in
>> the device node for this purpose.
> 
> We really need a value here. For example, in case where the OPP table
> defines the states of the power-domain itself, we don't have any
> phandles to point to.
> 

OK

>>> +  The integer value '0' represents the lowest performance level and the higher
>>> +  values represent higher performance levels. 
>>
>> needs to be changed as OPP table entry.
> 
> Not sure I understood what change you are looking for :(
> 

Looks like I commented the same thing below, just redundant comment
here. Sorry about that.

>>>  When present in the OPP table of a
>>> + power-domain, it represents the performance level of the domain. When present
>>
>> again "performance level of the domain corresponding to that OPP entry"
>> on something similar
> 
> Ok.
> 
>>> +  in the OPP table of a normal device, it represents the performance level of
>>
>> what do you mean by normal device ? needs description as that's
>> something new introduced here.
> 
> It should be non-power-domain node.
> 

OK

>>> +  the parent power-domain. The OPP table can contain the
>>> +  "domain-performance-state" property, only if the device node contains the
>>> +  "power-domains" or "#power-domain-cells" property. 
>>
>> Why such a restriction ?
> 
> Why would we use it for non-power-domain cases? That's not what we
> are looking for..
> 

OK I was imagining that this would abstract clocks and regulators and
hence thinking of other possibilities.

>>> The OPP nodes aren't
>>> +  allowed to contain the "domain-performance-state" property partially, i.e.
>>> +  Either all OPP nodes in the OPP table have the "domain-performance-state"
>>> +  property or none of them have it.
>>> +
>>>  Example 1: Single cluster Dual-core ARM cortex A9, switch DVFS states together.
>>>  
>>>  / {
>>> @@ -528,3 +540,60 @@ Example 5: opp-supported-hw
>>>  		};
>>>  	};
>>>  };
>>> +
>>> +Example 7: domain-Performance-state:
>>> +(example: For 1GHz require domain state 1 and for 1.1 & 1.2 GHz require state 2)
>>> +
>>> +/ {
>>> +	domain_opp_table: opp_table0 {
>>> +		compatible = "operating-points-v2";
>>> +
>>> +		opp@1 {
>>> +			domain-performance-state = <1>;
>>> +			opp-microvolt = <975000 970000 985000>;
>>> +		};
>>> +		opp@2 {
>>> +			domain-performance-state = <2>;
>>> +			opp-microvolt = <1075000 1000000 1085000>;
>>> +		};
>>> +	};
>>> +
>>> +	foo_domain: power-controller@12340000 {
>>> +		compatible = "foo,power-controller";
>>> +		reg = <0x12340000 0x1000>;
>>> +		#power-domain-cells = <0>;
>>> +		operating-points-v2 = <&domain_opp_table>;
>>
>> How does it scale with power domain providers with multiple power domain ?
> 
> Devices can't have multiple power domains today. Will see this when
> that support is added.
> 

Agreed and I see some working already happening on that, so yes we can
add that later.

What I was referring is about power domain provider with multiple power
domains(simply #power-domain-cells=<1> case as explained in the
power-domain specification.

> Note that only the power domains can have multiple parent power
> domains today.
> 
>>> +	}
>>> +
>>> +	cpu0_opp_table: opp_table1 {
>>> +		compatible = "operating-points-v2";
>>> +		opp-shared;
>>> +
>>> +		opp@1000000000 {
>>> +			opp-hz = /bits/ 64 <1000000000>;
>>> +			domain-performance-state = <1>;
>>> +		};
>>> +		opp@1100000000 {
>>> +			opp-hz = /bits/ 64 <1100000000>;
>>> +			domain-performance-state = <2>;
>>> +		};
>>> +		opp@1200000000 {
>>> +			opp-hz = /bits/ 64 <1200000000>;
>>> +			domain-performance-state = <2>;
>>> +		};
>>> +	};
>>> +
>>> +	cpus {
>>> +		#address-cells = <1>;
>>> +		#size-cells = <0>;
>>> +
>>> +		cpu@0 {
>>> +			compatible = "arm,cortex-a9";
>>> +			reg = <0>;
>>> +			clocks = <&clk_controller 0>;
>>> +			clock-names = "cpu";
>>> +			operating-points-v2 = <&cpu0_opp_table>;
>>
>> Do we ignore operating-points-v2 above as this device/cpu node contains
>> power domain which has operating-points-v2 property ? In other words
>> how do they correlate ?
> 
> Devices and their power domains can both have their performance
> states. Just that to get the device in a particular state, we may need
> to get its power domain to a particular state first.
> 

Yes. To simplify what not we just have power-domain for a device and
change state of that domain to change the performance of that device.
Then put this in the hierarchy. Some thing similar to what we already
have with new domain-idle states. In that way, we can move any
performance control to the domain and abstract the clocks and regulators
from the devices as the first step and from the OSPM view if there's
firmware support.

If we are looking this power-domains with performance as just some
*advanced regulators*, I don't like the complexity added.

I am also looking at how does this align with other specifications like
ACPI and SCMI that are trying to solve similar issues.

On 13/04/17 06:50, Viresh Kumar wrote:
> On 12-04-17, 18:05, Sudeep Holla wrote:
>>
>>
>> On 20/03/17 09:32, Viresh Kumar wrote:

[...]

>>
>> Thinking more about this above example, I think you need more
>> explanation. So in the above case you have cpu with clock controller,
>> power-domain and the OPP table info, I can think of few things that need
>> to be explicit:
>>
>> 1. How does the precedence look like ?
> 
> Just think of the power-domain as a regulator here. If we are
> increasing frequency of the device, power-domain needs to be
> programmed first followed by the clock.
> 

Interesting. My understand of power domain and in particular power
domain performance was that it would control both. The abstract number
you introduce would hide clocks and regulators.

But if the concept treats it just as yet another regulator, we do we
need these at all. Why don't we relate this performance to regulator
values and be done with it ?

Sorry if I am missing to understand something here. I would look this as
replacement for both clocks and regulators, something similar to ACPI
CPPC. If not, it looks unnecessary to me with the information I have got
so far.

>> 2. Since power-domains with OPP table control the performance state, do
> 
> They control performance state of the domains, not the devices.
> 
>>    we ignore clock and operating-points-v2 in the above case completely?
> 
> No. They are separate.
>

Understood now, but still trying to understand the complexity introduced
here.

>>
>> 3. Will the power-domain drive the OPP ?
> 
> power-domain will driver its own state using its own OPP table.
> Devices may fine tune within those states.
> 

I fail to understand here. This makes me think this power domain is same
as regulators as you pointed out earlier. So, we do we need all these
extra things. I was hoping this to be something like ACPI CPPC that hide
away clock and regulators.

On 13-04-17, 14:42, Sudeep Holla wrote:
> What I was referring is about power domain provider with multiple power
> domains(simply #power-domain-cells=<1> case as explained in the
> power-domain specification.

I am not sure if we should be looking to target such a situation for now, as
that would be like this:

Device controlled by Domain A. Domain A itself is controlled by Domain B and
Domain C.

Though we will end up converting the domain-performance-state property to an
array if that is required in near future.

> Yes. To simplify what not we just have power-domain for a device and
> change state of that domain to change the performance of that device.

Consider this case to understand what I have in Mind.

The power domain have its states as A, B, C, D. There can be multiple devices
regulated by that domain and one of the devices have its power states as: A1,
A2, A3, B1, B2, B3, C1, C2, C3, D1, D2, D3 and all these states have different
frequency/voltages.

IOW, the devices can have regulators as well and may want to fine tune within
the domain performance-state.

> Then put this in the hierarchy. Some thing similar to what we already
> have with new domain-idle states. In that way, we can move any
> performance control to the domain and abstract the clocks and regulators
> from the devices as the first step and from the OSPM view if there's
> firmware support.
> 
> If we are looking this power-domains with performance as just some
> *advanced regulators*, I don't like the complexity added.

In the particular case I am trying to solve (Qcom), we have some sort of
regulators which are only programmed by a M3 core. The M3 core needs integer
numbers representing state we want the domain to be in and it will put the
regulators (or whatever) in a particular state.

On 13-04-17, 14:43, Sudeep Holla wrote:
> Interesting. My understand of power domain and in particular power
> domain performance was that it would control both. The abstract number
> you introduce would hide clocks and regulators.
> 
> But if the concept treats it just as yet another regulator, we do we
> need these at all. Why don't we relate this performance to regulator
> values and be done with it ?
> 
> Sorry if I am missing to understand something here. I would look this as
> replacement for both clocks and regulators, something similar to ACPI
> CPPC. If not, it looks unnecessary to me with the information I have got
> so far.

I kind of answered that in the other email.

Some background may be good here. So Qcom tried to solve all this with virtual
regulators, but the problem was that they need to talk in terms of integer
values (1, 2, 3..) and not voltages and so they can't use the regulator
framework straight away. And so we are doing all this.

On 17/04/17 06:27, Viresh Kumar wrote:
> On 13-04-17, 14:42, Sudeep Holla wrote:
>> What I was referring is about power domain provider with multiple power
>> domains(simply #power-domain-cells=<1> case as explained in the
>> power-domain specification.
> 
> I am not sure if we should be looking to target such a situation for now, as
> that would be like this:
> 
> Device controlled by Domain A. Domain A itself is controlled by Domain B and
> Domain C.
> 

No, may be I was not so clear. I am just referring a power controller
that provides say 3 different power domains and are indexed 0 - 2.
The consumer just passes the index along with the phandle for the power
domain info.

> Though we will end up converting the domain-performance-state property to an
> array if that is required in near future.
> 

OK, better to document that so that we know how to extend it. We have
#power-domain-cells=<1> on Juno with SCPI.

>> Yes. To simplify what not we just have power-domain for a device and
>> change state of that domain to change the performance of that device.
> 
> Consider this case to understand what I have in Mind.
> 
> The power domain have its states as A, B, C, D. There can be multiple devices
> regulated by that domain and one of the devices have its power states as: A1,
> A2, A3, B1, B2, B3, C1, C2, C3, D1, D2, D3 and all these states have different
> frequency/voltages.
> 
> IOW, the devices can have regulators as well and may want to fine tune within
> the domain performance-state.
> 

Understood. I would incline towards reusing regulators we that's what is
changed behind the scene. Calling this operating performance point
is misleading and doesn't align well with existing specs/features.

>> Then put this in the hierarchy. Some thing similar to what we already
>> have with new domain-idle states. In that way, we can move any
>> performance control to the domain and abstract the clocks and regulators
>> from the devices as the first step and from the OSPM view if there's
>> firmware support.
>>
>> If we are looking this power-domains with performance as just some
>> *advanced regulators*, I don't like the complexity added.
> 
> In the particular case I am trying to solve (Qcom), we have some sort of
> regulators which are only programmed by a M3 core. The M3 core needs integer
> numbers representing state we want the domain to be in and it will put the
> regulators (or whatever) in a particular state.
> 

Understood. We have exactly same thing with SCPI but it controls both
frequency and voltage referred as operating points. In general, this OPP
terminology is used in SCPI/ACPI/SCMI specifications as both frequency
and voltage control. I am bit worried that this binding might introduce
confusions on the definitions. But it can be reworded/renamed easily if
required.

On 17/04/17 06:33, Viresh Kumar wrote:
> On 13-04-17, 14:43, Sudeep Holla wrote:
>> Interesting. My understand of power domain and in particular power
>> domain performance was that it would control both. The abstract number
>> you introduce would hide clocks and regulators.
>>
>> But if the concept treats it just as yet another regulator, we do we
>> need these at all. Why don't we relate this performance to regulator
>> values and be done with it ?
>>
>> Sorry if I am missing to understand something here. I would look this as
>> replacement for both clocks and regulators, something similar to ACPI
>> CPPC. If not, it looks unnecessary to me with the information I have got
>> so far.
> 
> I kind of answered that in the other email.
> 
> Some background may be good here. So Qcom tried to solve all this with virtual
> regulators, but the problem was that they need to talk in terms of integer
> values (1, 2, 3..) and not voltages and so they can't use the regulator
> framework straight away. And so we are doing all this.
> 

Was it posted externally ? Was there any objections for that approach ?
IMO that's better approach but if I am late to the party, I would like
to read through the discussions that happened on it(if any)

On 18-04-17, 17:01, Sudeep Holla wrote:
> No, may be I was not so clear. I am just referring a power controller
> that provides say 3 different power domains and are indexed 0 - 2.
> The consumer just passes the index along with the phandle for the power
> domain info.

Ahh, I got you now. Will take care of it in next version.

On 18-04-17, 17:03, Sudeep Holla wrote:
> Was it posted externally ? Was there any objections for that approach ?
> IMO that's better approach but if I am late to the party, I would like
> to read through the discussions that happened on it(if any)

Maybe Stephen can tell more about it. AFAIK, there were some offline
discussions around it.

On 18-04-17, 17:01, Sudeep Holla wrote:
> Understood. I would incline towards reusing regulators we that's what is

It can be just a regulator, but it can be anything else as well. That
entity may have its own clock/volt/current tunables, etc.

> changed behind the scene. Calling this operating performance point
> is misleading and doesn't align well with existing specs/features.

Yeah, but there are no voltage levels available here and that doesn't
fit as a regulator then.

> Understood. We have exactly same thing with SCPI but it controls both
> frequency and voltage referred as operating points. In general, this OPP
> terminology is used in SCPI/ACPI/SCMI specifications as both frequency
> and voltage control. I am bit worried that this binding might introduce
> confusions on the definitions. But it can be reworded/renamed easily if
> required.

Yeah, so far we have been looking at OPPs as freq-voltage pairs ONLY
and that is changing. I am not sure if it going in the wrong
direction really. Without frequency also it is an operating point for
the domain. Isn't it?

On 19/04/17 12:47, Viresh Kumar wrote:
> On 18-04-17, 17:01, Sudeep Holla wrote:
>> Understood. I would incline towards reusing regulators we that's what is
> 
> It can be just a regulator, but it can be anything else as well. That
> entity may have its own clock/volt/current tunables, etc.
> 

Agreed.

>> changed behind the scene. Calling this operating performance point
>> is misleading and doesn't align well with existing specs/features.
> 
> Yeah, but there are no voltage levels available here and that doesn't
> fit as a regulator then.
> 

We can't dismiss just based on that. We do have systems where
performance index is mapped to clocks though it may not be 1:1 mapping.
I am not disagreeing here, just trying to understand it better.

>> Understood. We have exactly same thing with SCPI but it controls both
>> frequency and voltage referred as operating points. In general, this OPP
>> terminology is used in SCPI/ACPI/SCMI specifications as both frequency
>> and voltage control. I am bit worried that this binding might introduce
>> confusions on the definitions. But it can be reworded/renamed easily if
>> required.
> 
> Yeah, so far we have been looking at OPPs as freq-voltage pairs ONLY
> and that is changing. I am not sure if it going in the wrong
> direction really. Without frequency also it is an operating point for
> the domain. Isn't it?
> 

Yes, I completely agree. I am not saying the direction is wrong. I am
saying it's confusing and binding needs to be more clear.

On the contrary(playing devil's advocate here), we can treat all
existing regulators alone as OPP then if you strip the voltages and
treat it as abstract number. So if the firmware handles more than just
regulators, I agree. At the same time, I would have preferred firmware
to even abstract the frequency like ACPI CPPC. It would be good to get
more information on what exactly that firmware handles.

I am just more cautious here since we are designing generic bindings and
changing generic code, we need to understand what that firmware supports
and how it may evolve(so that we can maintain DT compatibility)

I did a brief check and wanted to check if this is SMD/RPM regulators ?

On 19-04-17, 14:58, Sudeep Holla wrote:
> On 19/04/17 12:47, Viresh Kumar wrote:
> > On 18-04-17, 17:01, Sudeep Holla wrote:
> >> Understood. I would incline towards reusing regulators we that's what is
> > 
> > It can be just a regulator, but it can be anything else as well. That
> > entity may have its own clock/volt/current tunables, etc.
> > 
> >> changed behind the scene. Calling this operating performance point
> >> is misleading and doesn't align well with existing specs/features.
> > 
> > Yeah, but there are no voltage levels available here and that doesn't
> > fit as a regulator then.
> > 
> 
> We can't dismiss just based on that. We do have systems where
> performance index is mapped to clocks though it may not be 1:1 mapping.
> I am not disagreeing here, just trying to understand it better.

@Stephen: Can you answer here please ?

> >> Understood. We have exactly same thing with SCPI but it controls both
> >> frequency and voltage referred as operating points. In general, this OPP
> >> terminology is used in SCPI/ACPI/SCMI specifications as both frequency
> >> and voltage control. I am bit worried that this binding might introduce
> >> confusions on the definitions. But it can be reworded/renamed easily if
> >> required.
> > 
> > Yeah, so far we have been looking at OPPs as freq-voltage pairs ONLY
> > and that is changing. I am not sure if it going in the wrong
> > direction really. Without frequency also it is an operating point for
> > the domain. Isn't it?
> > 
> 
> Yes, I completely agree. I am not saying the direction is wrong. I am
> saying it's confusing and binding needs to be more clear.

What exactly isn't clear? (Yeah, there had been lots of emails and I
want to know what improvements are you looking for).

> On the contrary(playing devil's advocate here), we can treat all
> existing regulators alone as OPP then if you strip the voltages and
> treat it as abstract number.

But then we are going to have lots of platform specific code which
will program the actual hardware, etc. Which is all handled by the
regulator framework. Also note that the regulator core selects the
common voltage selected by all the children, while we want to select
the highest performance point here.

Even if we have to configure both clock and voltage for the power
domain using standard clk/regulator frameworks, OPP will work just
fine as it will do that then. So, its not that we are bypassing the
regulator framework here. It will be used if we have the voltages
available for the power-domain's performance states.

> So if the firmware handles more than just
> regulators, I agree.

I don't know the internals of that really.

> At the same time, I would have preferred firmware
> to even abstract the frequency like ACPI CPPC.

Frequency isn't required to be configured for the cases I know, but it
can be in future implementations.

> It would be good to get
> more information on what exactly that firmware handles.

@Stephen ?

> I am just more cautious here since we are designing generic bindings and
> changing generic code, we need to understand what that firmware supports
> and how it may evolve(so that we can maintain DT compatibility)

Sure, I am fine with more discussions on it :)

> I did a brief check and wanted to check if this is SMD/RPM regulators ?

Yes, Qcom calls the external core as Resource and Power manager (RPM).

Viresh, Sudeep,

Sorry for jumping in late.

[...]

>> On the contrary(playing devil's advocate here), we can treat all
>> existing regulators alone as OPP then if you strip the voltages and
>> treat it as abstract number.
>
> But then we are going to have lots of platform specific code which
> will program the actual hardware, etc. Which is all handled by the
> regulator framework. Also note that the regulator core selects the
> common voltage selected by all the children, while we want to select
> the highest performance point here.

If I understand correctly, Sudeep is not convinced that this is about
PM domain regulator(s), right?

To me there is no doubt, these regulators is exactly the definition of
PM domain regulators.

That said, long time ago we have decided PM domain regulator shall be
modeled as exactly that. From DT point of view, this means the handle
to the PM domain regulator belongs in the node of the PM domain
controller - and not in each device's node of those belonging to the
PM domain.

Isn't that what this discussion really boils down to? Or maybe I am
not getting it.

>
> Even if we have to configure both clock and voltage for the power
> domain using standard clk/regulator frameworks, OPP will work just
> fine as it will do that then. So, its not that we are bypassing the
> regulator framework here. It will be used if we have the voltages
> available for the power-domain's performance states.
>
>> So if the firmware handles more than just
>> regulators, I agree.
>
> I don't know the internals of that really.
>
>> At the same time, I would have preferred firmware
>> to even abstract the frequency like ACPI CPPC.
>
> Frequency isn't required to be configured for the cases I know, but it
> can be in future implementations.

To me using OPP tables makes sense as it gives us the flexibility that
is needed. If I understand correct, that was also Kevin's point.

[...]

Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On 20-04-17, 10:23, Ulf Hansson wrote:
> Viresh, Sudeep,
> 
> Sorry for jumping in late.
> 
> [...]
> 
> >> On the contrary(playing devil's advocate here), we can treat all
> >> existing regulators alone as OPP then if you strip the voltages and
> >> treat it as abstract number.
> >
> > But then we are going to have lots of platform specific code which
> > will program the actual hardware, etc. Which is all handled by the
> > regulator framework. Also note that the regulator core selects the
> > common voltage selected by all the children, while we want to select
> > the highest performance point here.
> 
> If I understand correctly, Sudeep is not convinced that this is about
> PM domain regulator(s), right?
> 
> To me there is no doubt, these regulators is exactly the definition of
> PM domain regulators.
> 
> That said, long time ago we have decided PM domain regulator shall be
> modeled as exactly that. From DT point of view, this means the handle
> to the PM domain regulator belongs in the node of the PM domain
> controller - and not in each device's node of those belonging to the
> PM domain.
> 
> Isn't that what this discussion really boils down to? Or maybe I am
> not getting it.

Maybe not. I think Sudeep understands that this is about PM domain
regulators only but he is asking why aren't we solving this problem
using regulators framework but performance-levels instead.

> >
> > Even if we have to configure both clock and voltage for the power
> > domain using standard clk/regulator frameworks, OPP will work just
> > fine as it will do that then. So, its not that we are bypassing the
> > regulator framework here. It will be used if we have the voltages
> > available for the power-domain's performance states.
> >
> >> So if the firmware handles more than just
> >> regulators, I agree.
> >
> > I don't know the internals of that really.
> >
> >> At the same time, I would have preferred firmware
> >> to even abstract the frequency like ACPI CPPC.
> >
> > Frequency isn't required to be configured for the cases I know, but it
> > can be in future implementations.
> 
> To me using OPP tables makes sense as it gives us the flexibility that
> is needed. If I understand correct, that was also Kevin's point.

Right.

On 20/04/17 06:25, Viresh Kumar wrote:
> On 19-04-17, 14:58, Sudeep Holla wrote:
>> On 19/04/17 12:47, Viresh Kumar wrote:
>>> On 18-04-17, 17:01, Sudeep Holla wrote:

[...]

>> 
>> Yes, I completely agree. I am not saying the direction is wrong. I
>> am saying it's confusing and binding needs to be more clear.
> 
> What exactly isn't clear? (Yeah, there had been lots of emails and I 
> want to know what improvements are you looking for).
> 

Just that the term performance is closely related to frequency, it needs
to be explicit on what *exactly* it means. As it stands now,
it can be used for OPP as I explain which controls both but as you
clarify that's not what it's designed for.

>> On the contrary(playing devil's advocate here), we can treat all 
>> existing regulators alone as OPP then if you strip the voltages
>> and treat it as abstract number.
> 
> But then we are going to have lots of platform specific code which 
> will program the actual hardware, etc. Which is all handled by the 
> regulator framework. Also note that the regulator core selects the 
> common voltage selected by all the children, while we want to select 
> the highest performance point here.
> 

I am not sure if choosing highest performance point makes it difficult
to fit it in regulator framework. It could be some configuration.
Also IIUC the actual programming is done in the firmware in this case
and I fail to see how that adds lot of platform code.

> Even if we have to configure both clock and voltage for the power 
> domain using standard clk/regulator frameworks, OPP will work just 
> fine as it will do that then. So, its not that we are bypassing the 
> regulator framework here. It will be used if we have the voltages 
> available for the power-domain's performance states.
> 

Yes I understand that.

On 20/04/17 09:23, Ulf Hansson wrote:
> Viresh, Sudeep,
> 
> Sorry for jumping in late.
> 
> [...]
> 
>>> On the contrary(playing devil's advocate here), we can treat all
>>> existing regulators alone as OPP then if you strip the voltages and
>>> treat it as abstract number.
>>
>> But then we are going to have lots of platform specific code which
>> will program the actual hardware, etc. Which is all handled by the
>> regulator framework. Also note that the regulator core selects the
>> common voltage selected by all the children, while we want to select
>> the highest performance point here.
> 
> If I understand correctly, Sudeep is not convinced that this is about
> PM domain regulator(s), right?
> 

No, I am saying that it has to be modeled as regulators or some kind of
advanced regulators. I am against modeling it as some new feature and
using similar terminology that are quite close to OPP/CPPC in which case
it's quite hard not to misunderstand the concepts and eventually use
these bindings incorrectly.

> To me there is no doubt, these regulators is exactly the definition of
> PM domain regulators.
> 

+1

> That said, long time ago we have decided PM domain regulator shall be
> modeled as exactly that. From DT point of view, this means the handle
> to the PM domain regulator belongs in the node of the PM domain
> controller - and not in each device's node of those belonging to the
> PM domain.
> 
> Isn't that what this discussion really boils down to? Or maybe I am
> not getting it.
> 

I completely agree with you on all the above points. I am against the
performance state terminology. Since the regulators and OPP are already
defined in the bindings, all we need to explicitly state(if not already)
is that there are hierarchical.

On 20-04-17, 10:43, Sudeep Holla wrote:
> Just that the term performance is closely related to frequency, it needs
> to be explicit on what *exactly* it means. As it stands now,
> it can be used for OPP as I explain which controls both but as you
> clarify that's not what it's designed for.

We are talking about active states of a power domain here and
*performance* is the best word I got. And yes we can still have
frequency as a configurable here, just that current platforms don't
have it.

> I am not sure if choosing highest performance point makes it difficult
> to fit it in regulator framework. It could be some configuration.

I was just pointing out a difference :)

> Also IIUC the actual programming is done in the firmware in this case
> and I fail to see how that adds lot of platform code.

Oh I meant that for converting general regulator only cases to OPP. No
firmware was involved there.

Viresh Kumar <viresh.kumar@linaro.org> writes:

> On 20-04-17, 10:43, Sudeep Holla wrote:
>> Just that the term performance is closely related to frequency, it needs
>> to be explicit on what *exactly* it means. As it stands now,
>> it can be used for OPP as I explain which controls both but as you
>> clarify that's not what it's designed for.
>
> We are talking about active states of a power domain here and
> *performance* is the best word I got.
>
> And yes we can still have
> frequency as a configurable here, just that current platforms don't
> have it.

It's not that your platforms don't have frequency, it's just that it's
hidden by firmware on an M3 in your particular case.

DT is meant to model hardware, and surely what the M3 is managing is
frequency and/or voltage (IOW, an OPP).

The way I see it, the problem you're trying to solve is complicated just
because you don't know the exat freq and/or voltage because they are
hiddent behind the firware, and all you have control over is an index.  

What if you drop the introduction of the new domain-performance-state
and just stick with OPPs (and phandles to them), and for cases where you
don't know the exact freq/volage pairs, just use indexes and comment
what they refer to:

operating-points = <
	/*
         * NOTE: actual frequency and voltages are managed by
         * firmware and are hidden from HLOS, so we simply use index
         * here to select the OPP
         */
        1 1
	2 2
	3 3
>;

Since selecting the OPP is up to the power-domain driver implementation,
this should be fine, IMO.

This would mean just updating the doc to reflect the "relaxing" of these
fields to reflect indexes in cases where the exact freq/voltages are not
known.

IMO, this would be much simpler, as it avoids adding a new property and
continues to use teriminology that people are already familiar with
around OPPs.

Kevin

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

> On 17/04/17 06:27, Viresh Kumar wrote:
>> On 13-04-17, 14:42, Sudeep Holla wrote:
>>> What I was referring is about power domain provider with multiple power
>>> domains(simply #power-domain-cells=<1> case as explained in the
>>> power-domain specification.
>>
>> I am not sure if we should be looking to target such a situation for now, as
>> that would be like this:
>>
>> Device controlled by Domain A. Domain A itself is controlled by Domain B and
>> Domain C.
>>
> 
> No, may be I was not so clear. I am just referring a power controller
> that provides say 3 different power domains and are indexed 0 - 2.
> The consumer just passes the index along with the phandle for the power
> domain info.
> 
>> Though we will end up converting the domain-performance-state property to an
>> array if that is required in near future.
>>
> 
> OK, better to document that so that we know how to extend it. We have
> #power-domain-cells=<1> on Juno with SCPI.
> 
>>> Yes. To simplify what not we just have power-domain for a device and
>>> change state of that domain to change the performance of that device.
>>
>> Consider this case to understand what I have in Mind.
>>
>> The power domain have its states as A, B, C, D. There can be multiple devices
>> regulated by that domain and one of the devices have its power states as: A1,
>> A2, A3, B1, B2, B3, C1, C2, C3, D1, D2, D3 and all these states have different
>> frequency/voltages.
>>
>> IOW, the devices can have regulators as well and may want to fine tune within
>> the domain performance-state.
>>
> 
> Understood. I would incline towards reusing regulators we that's what is
> changed behind the scene. Calling this operating performance point
> is misleading and doesn't align well with existing specs/features.

[]...
 
>>> If we are looking this power-domains with performance as just some
>>> *advanced regulators*, I don't like the complexity added.

+ Mark

I don;t see any public discussions on why we ruled out using regulators to
support this but maybe there were some offline discussions on this.

Mark, this is a long thread, so just summarizing here to give you the context.

At qualcomm, we have an external M3 core (running its own firmware) which controls
a few voltage rails (including AVS on those). The devices vote for the voltage levels
(or performance levels) they need by passing an integer value to the M3 (not actual
voltage values). Since that didn't fit well with the existing regulator apis it was
proposed we look at modeling these as powerdomain performance levels (and reuse genpd
framework) which is what this series from Viresh is about.

Since the discussion now is moving towards 'why not use regulator framework for this
instead of adding all the complexity with powerdomain performance levels since
these are regulators underneath', I looped you in so you can provide some feedback
on can these really be modeled as some *advanced regulators* with some apis to set some
regulator performance levels (instead of voltage levels).

On Wed, Apr 26, 2017 at 10:02:39AM +0530, Rajendra Nayak wrote:
> > On 17/04/17 06:27, Viresh Kumar wrote:

> >>> If we are looking this power-domains with performance as just some
> >>> *advanced regulators*, I don't like the complexity added.

> + Mark

> I don;t see any public discussions on why we ruled out using regulators to
> support this but maybe there were some offline discussions on this.

> Mark, this is a long thread, so just summarizing here to give you the context.

> At qualcomm, we have an external M3 core (running its own firmware) which controls
> a few voltage rails (including AVS on those). The devices vote for the voltage levels
> (or performance levels) they need by passing an integer value to the M3 (not actual
> voltage values). Since that didn't fit well with the existing regulator apis it was

As I'm getting fed up of saying: if the values you are setting are not
voltages and do not behave like voltages then the hardware should not be
represented as a voltage regulator since if they are represented as
voltage regulators things will expect to be able to control them as
voltage regulators.  This hardware is quite clearly providing OPPs
directly, I would expect this to be handled in the OPP code somehow.

On 26/04/17 14:55, Mark Brown wrote:
> On Wed, Apr 26, 2017 at 10:02:39AM +0530, Rajendra Nayak wrote:
>>> On 17/04/17 06:27, Viresh Kumar wrote:
> 
>>>>> If we are looking this power-domains with performance as just some
>>>>> *advanced regulators*, I don't like the complexity added.
> 
>> + Mark
> 
>> I don;t see any public discussions on why we ruled out using regulators to
>> support this but maybe there were some offline discussions on this.
> 
>> Mark, this is a long thread, so just summarizing here to give you the context.
> 
>> At qualcomm, we have an external M3 core (running its own firmware) which controls
>> a few voltage rails (including AVS on those). The devices vote for the voltage levels

Thanks for explicitly mentioning this, but ...

>> (or performance levels) they need by passing an integer value to the M3 (not actual

you contradict here, is it just voltage or performance(i.e. frequency)
or both ? We need clarity there to choose the right representation.

>> voltage values). Since that didn't fit well with the existing regulator apis it was
> 
> As I'm getting fed up of saying: if the values you are setting are not
> voltages and do not behave like voltages then the hardware should not be
> represented as a voltage regulator since if they are represented as
> voltage regulators things will expect to be able to control them as
> voltage regulators.  This hardware is quite clearly providing OPPs
> directly, I would expect this to be handled in the OPP code somehow.

I agree with you that we need to be absolutely sure on what it actually
represents.

But as more and more platform are pushing such power controls to
dedicated M3 or similar processors, we need abstraction. Though we are
controlling hardware, we do so indirectly. Since there were discussions
around device tree representing hardware vs platform, I tend to think,
we are moving towards platform(something similar to ACPI).

On 04/27/2017 03:12 PM, Sudeep Holla wrote:
[]..

>>
>>> At qualcomm, we have an external M3 core (running its own firmware) which controls
>>> a few voltage rails (including AVS on those). The devices vote for the voltage levels
> 
> Thanks for explicitly mentioning this, but ...
> 
>>> (or performance levels) they need by passing an integer value to the M3 (not actual
> 
> you contradict here, is it just voltage or performance(i.e. frequency)
> or both ? We need clarity there to choose the right representation.

Its just voltage.

On 27-04-17, 16:20, Rajendra Nayak wrote:
> 
> On 04/27/2017 03:12 PM, Sudeep Holla wrote:
> []..
> 
> >>
> >>> At qualcomm, we have an external M3 core (running its own firmware) which controls
> >>> a few voltage rails (including AVS on those). The devices vote for the voltage levels
> > 
> > Thanks for explicitly mentioning this, but ...
> > 
> >>> (or performance levels) they need by passing an integer value to the M3 (not actual
> > 
> > you contradict here, is it just voltage or performance(i.e. frequency)
> > or both ? We need clarity there to choose the right representation.
> 
> Its just voltage.

Right. Its just voltage in this case, but we can't speak of future
platforms here and we have to consider this thing as an operating
performance point only. I still think that this thread is moving in
the right direction, specially after V6 which looks much better.

If we have anything strong against the way V6 is trying to solve it, I
want to talk about it right now and get inputs from all the parties
involved. Scrapping all this work is fine, but I would like to do it
ASAP in that case :)

On 28/04/17 06:00, Viresh Kumar wrote:
> On 27-04-17, 16:20, Rajendra Nayak wrote:
>>
>> On 04/27/2017 03:12 PM, Sudeep Holla wrote:
>> []..
>>
>>>>
>>>>> At qualcomm, we have an external M3 core (running its own firmware) which controls
>>>>> a few voltage rails (including AVS on those). The devices vote for the voltage levels
>>>
>>> Thanks for explicitly mentioning this, but ...
>>>
>>>>> (or performance levels) they need by passing an integer value to the M3 (not actual
>>>
>>> you contradict here, is it just voltage or performance(i.e. frequency)
>>> or both ? We need clarity there to choose the right representation.
>>
>> Its just voltage.
> 
> Right. Its just voltage in this case, but we can't speak of future
> platforms here and we have to consider this thing as an operating
> performance point only. I still think that this thread is moving in
> the right direction, specially after V6 which looks much better.
> 

Just thinking out loud, I can see platforms with have OPPs can move to
this binding in future eliminating the need to specify the clock and
regulators explicitly. So, I am not saying I against this idea, but I
see it might complicate the above case in terms of the precedence that
we consider in DT from backward compatibility.

E.g. if you now use this for just regulators, then I assume you continue
to use clocks. However, that makes it difficult for platforms
implementing *real* OPPs to reuse this binding as they may expect to
skip clock altogether.

Also we may need OPPs(both volt/freq), voltage only and clock only
bindings though all 3 are driven by the firmware and all are at abstract
levels. I am trying to broaden the scope now without having to churn
this binding again in near future.

So I don't totally agree that voltage regulators much have *real*
voltages and not abstract scale. Yes the correct bindings might have
such restrictions but can't we extend it ?

Anyways these are just my opinion.

> If we have anything strong against the way V6 is trying to solve it, I
> want to talk about it right now and get inputs from all the parties
> involved. Scrapping all this work is fine, but I would like to do it
> ASAP in that case :)
> 

As I said I am not against it, but I see it useful for a different
use-case, just not the one you are trying to solve here ;)

On 28-04-17, 10:44, Sudeep Holla wrote:
> Just thinking out loud, I can see platforms with have OPPs can move to
> this binding in future eliminating the need to specify the clock and
> regulators explicitly. So, I am not saying I against this idea, but I
> see it might complicate the above case in terms of the precedence that
> we consider in DT from backward compatibility.
> 
> E.g. if you now use this for just regulators, then I assume you continue
> to use clocks. However, that makes it difficult for platforms
> implementing *real* OPPs to reuse this binding as they may expect to
> skip clock altogether.
> 
> Also we may need OPPs(both volt/freq), voltage only and clock only
> bindings though all 3 are driven by the firmware and all are at abstract
> levels. I am trying to broaden the scope now without having to churn
> this binding again in near future.
> 
> So I don't totally agree that voltage regulators much have *real*
> voltages and not abstract scale. Yes the correct bindings might have
> such restrictions but can't we extend it ?
> 
> Anyways these are just my opinion.

Everyone's opinion has equal merit here :)

I believe that some of your hesitation came from the point that I have
made opp-hz optional. That isn't the case anymore with V6.

Can we please take the discussion to that thread now and see if you
can find similar problems there as well.

Thanks a lot.

On Thu, Apr 27, 2017 at 10:42:49AM +0100, Sudeep Holla wrote:
> On 26/04/17 14:55, Mark Brown wrote:

> > As I'm getting fed up of saying: if the values you are setting are not
> > voltages and do not behave like voltages then the hardware should not be
> > represented as a voltage regulator since if they are represented as
> > voltage regulators things will expect to be able to control them as
> > voltage regulators.  This hardware is quite clearly providing OPPs
> > directly, I would expect this to be handled in the OPP code somehow.

> I agree with you that we need to be absolutely sure on what it actually
> represents.

> But as more and more platform are pushing such power controls to
> dedicated M3 or similar processors, we need abstraction. Though we are
> controlling hardware, we do so indirectly. Since there were discussions
> around device tree representing hardware vs platform, I tend to think,
> we are moving towards platform(something similar to ACPI).

I don't think there's a meaningful hardware/platform distinction here -
in terms of what DT is describing the platform bit is just what the
hardware (the microcontrollers) happen to do, DT doesn't much care about
that though.

On 30/04/17 13:49, Mark Brown wrote:
> On Thu, Apr 27, 2017 at 10:42:49AM +0100, Sudeep Holla wrote:
>> On 26/04/17 14:55, Mark Brown wrote:
> 
>>> As I'm getting fed up of saying: if the values you are setting are not
>>> voltages and do not behave like voltages then the hardware should not be
>>> represented as a voltage regulator since if they are represented as
>>> voltage regulators things will expect to be able to control them as
>>> voltage regulators.  This hardware is quite clearly providing OPPs
>>> directly, I would expect this to be handled in the OPP code somehow.
> 
>> I agree with you that we need to be absolutely sure on what it actually
>> represents.
> 
>> But as more and more platform are pushing such power controls to
>> dedicated M3 or similar processors, we need abstraction. Though we are
>> controlling hardware, we do so indirectly. Since there were discussions
>> around device tree representing hardware vs platform, I tend to think,
>> we are moving towards platform(something similar to ACPI).
> 
> I don't think there's a meaningful hardware/platform distinction here -
> in terms of what DT is describing the platform bit is just what the
> hardware (the microcontrollers) happen to do, 
> 

Yes agreed. It's similar to PSCI or any other platform firmware IMO.

The question is how do we deal with such controls that needs to be done
via the firmware ? We generally plug-in to the existing framework in
Linux using the existing bindings. Most of the time, much simpler
bindings than the one that present complete hardware description.

> DT doesn't much care about that though.

No sure about that, may be doesn't care about the internals, but we need
to care about interface, no ?

On Wed, May 03, 2017 at 12:21:54PM +0100, Sudeep Holla wrote:
> On 30/04/17 13:49, Mark Brown wrote:

> > DT doesn't much care about that though.

> No sure about that, may be doesn't care about the internals, but we need
> to care about interface, no ?

Well, we need to at least describe what's there - my point is that it's
no different to describing a piece of hardware, the fact that it happens
to be implemented as firmware doesn't really change the abstraction
level DT is operating at.