diff mbox series

[V3,2/7] PM / OPP: Introduce "domain-performance-state" binding to OPP nodes

Message ID ceb1bf5f696138c30b30743c24b619336d438d7c.1487926924.git.viresh.kumar@linaro.org
State New
Headers show
Series [V3,1/7] PM / Domains: Introduce "performance-states" binding | expand

Commit Message

Viresh Kumar Feb. 24, 2017, 9:06 a.m. UTC
If the consumers don't need the capability of switching to different
domain performance states at runtime, then they can simply define their
required domain performance state in their nodes directly.

But if the device needs the capability of switching to different domain
performance states, as they may need to support different clock rates,
then the per OPP node can be used to contain that information.

This patch introduces the domain-performance-state (already defined by
Power Domain bindings) to the per OPP node.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>

Tested-by: Rajendra Nayak <rnayak@codeaurora.org>

---
 Documentation/devicetree/bindings/opp/opp.txt | 64 +++++++++++++++++++++++++++
 1 file changed, 64 insertions(+)

-- 
2.7.1.410.g6faf27b

--
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

Comments

Rob Herring Feb. 28, 2017, 12:39 a.m. UTC | #1
On Fri, Feb 24, 2017 at 02:36:34PM +0530, Viresh Kumar wrote:
> If the consumers don't need the capability of switching to different

> domain performance states at runtime, then they can simply define their

> required domain performance state in their nodes directly.

> 

> But if the device needs the capability of switching to different domain

> performance states, as they may need to support different clock rates,

> then the per OPP node can be used to contain that information.

> 

> This patch introduces the domain-performance-state (already defined by

> Power Domain bindings) to the per OPP node.

> 


We already have OPP voltages, why are those not sufficient?

> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>

> Tested-by: Rajendra Nayak <rnayak@codeaurora.org>

> ---

>  Documentation/devicetree/bindings/opp/opp.txt | 64 +++++++++++++++++++++++++++

>  1 file changed, 64 insertions(+)

> 

> diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt

> index 9f5ca4457b5f..7f6bb52521b6 100644

> --- a/Documentation/devicetree/bindings/opp/opp.txt

> +++ b/Documentation/devicetree/bindings/opp/opp.txt

> @@ -154,6 +154,15 @@ properties.

>  

>  - status: Marks the node enabled/disabled.

>  

> +- domain-performance-state: A positive integer value representing the minimum

> +  performance level (of the parent domain) required by the consumer as defined

> +  by ../power/power_domain.txt binding document. The OPP nodes can contain the

> +  "domain-performance-state" property, only if the device node contains a

> +  "power-domains" property. 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 +537,58 @@ 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)

> +

> +/ {

> +	cpu0_opp_table: opp_table0 {

> +		compatible = "operating-points-v2";

> +		opp-shared;

> +

> +		opp@1000000000 {

> +			opp-hz = /bits/ 64 <1000000000>;

> +			domain-performance-state = <1>;


Thinking about this some more, there's a problem here that you have no 
link to foo_domain. I guess that resides in the cpu's node?

Perhaps instead of a number, this should be a phandle to pstate@1. Then 
you just get the parent if you need to know the domain.

> +		};

> +		opp@1100000000 {

> +			opp-hz = /bits/ 64 <1100000000>;

> +			domain-performance-state = <2>;

> +		};

> +		opp@1200000000 {

> +			opp-hz = /bits/ 64 <1200000000>;

> +			domain-performance-state = <2>;

> +		};

> +	};

> +

> +	foo_domain: power-controller@12340000 {

> +		compatible = "foo,power-controller";

> +		reg = <0x12340000 0x1000>;

> +		#power-domain-cells = <0>;

> +

> +		performance-states {

> +			compatible = "domain-performance-state";

> +			pstate@1 {

> +				reg = <1>;

> +				domain-microvolt = <970000 975000 985000>;

> +			};

> +			pstate@2 {

> +				reg = <2>;

> +				domain-microvolt = <1000000 1075000 1085000>;

> +			};

> +		};

> +	}

> +

> +	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>;

> +		};

> +	};

> +};

> -- 

> 2.7.1.410.g6faf27b

> 

--
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
Viresh Kumar Feb. 28, 2017, 6:57 a.m. UTC | #2
On 27-02-17, 18:39, Rob Herring wrote:
> On Fri, Feb 24, 2017 at 02:36:34PM +0530, Viresh Kumar wrote:

> > If the consumers don't need the capability of switching to different

> > domain performance states at runtime, then they can simply define their

> > required domain performance state in their nodes directly.

> > 

> > But if the device needs the capability of switching to different domain

> > performance states, as they may need to support different clock rates,

> > then the per OPP node can be used to contain that information.

> > 

> > This patch introduces the domain-performance-state (already defined by

> > Power Domain bindings) to the per OPP node.

> > 

> 

> We already have OPP voltages, why are those not sufficient?


Those are for the regulator that ONLY controls the device, and
domain-performance-state belongs to the parent domain which controls many
devices.

> > +Example 7: domain-Performance-state:

> > +(example: For 1GHz require domain state 1 and for 1.1 & 1.2 GHz require state 2)

> > +

> > +/ {

> > +	cpu0_opp_table: opp_table0 {

> > +		compatible = "operating-points-v2";

> > +		opp-shared;

> > +

> > +		opp@1000000000 {

> > +			opp-hz = /bits/ 64 <1000000000>;

> 

> Thinking about this some more, there's a problem here that you have no 

> link to foo_domain. I guess that resides in the cpu's node?


Right, the "cpus" node below demonstrates that.

> > +	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>;

> > +		};

> > +	};

> > +};


> > +			domain-performance-state = <1>;


> Perhaps instead of a number, this should be a phandle to pstate@1. Then 

> you just get the parent if you need to know the domain.


That's what I did in V2, but then I turned it down considering the parent/child
relationships we may have.

There are multiple cases we can have:

A.) DeviceX  --->  Parent-domain-1 (Contains Perfomance states)

B.) DeviceX  --->  Parent-domain-1  ---> Parent domain-2 (Contains Perfomance states)

                                    ---> Parent domain-2 (Contains Perfomance states)
                                    |
                                    |
C.) DeviceX  --->  Parent-domain-1  |
                                    |
                                    |
                                    ---> Parent domain-3 (Contains Perfomance states)


The case A.) represents a simple case where the parent domain of the device
contains the performance states. The phandle can work pretty well in this case.
But the other cases B.) and C.) are a bit complicated as the direct parent
domain doesn't allow changing the performance states, but its parents. And so I
went ahead with numbers instead of phandles. Yes, we will still be able to get
to the performance state node with the help of phandles, but will that be the
right thing to do ?

-- 
viresh
--
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
Rob Herring Feb. 28, 2017, 2:10 p.m. UTC | #3
On Tue, Feb 28, 2017 at 12:57 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> On 27-02-17, 18:39, Rob Herring wrote:

>> On Fri, Feb 24, 2017 at 02:36:34PM +0530, Viresh Kumar wrote:

>> > If the consumers don't need the capability of switching to different

>> > domain performance states at runtime, then they can simply define their

>> > required domain performance state in their nodes directly.

>> >

>> > But if the device needs the capability of switching to different domain

>> > performance states, as they may need to support different clock rates,

>> > then the per OPP node can be used to contain that information.

>> >

>> > This patch introduces the domain-performance-state (already defined by

>> > Power Domain bindings) to the per OPP node.

>> >

>>

>> We already have OPP voltages, why are those not sufficient?

>

> Those are for the regulator that ONLY controls the device, and

> domain-performance-state belongs to the parent domain which controls many

> devices.

>

>> > +Example 7: domain-Performance-state:

>> > +(example: For 1GHz require domain state 1 and for 1.1 & 1.2 GHz require state 2)

>> > +

>> > +/ {

>> > +   cpu0_opp_table: opp_table0 {

>> > +           compatible = "operating-points-v2";

>> > +           opp-shared;

>> > +

>> > +           opp@1000000000 {

>> > +                   opp-hz = /bits/ 64 <1000000000>;

>>

>> Thinking about this some more, there's a problem here that you have no

>> link to foo_domain. I guess that resides in the cpu's node?

>

> Right, the "cpus" node below demonstrates that.

>

>> > +   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>;

>> > +           };

>> > +   };

>> > +};

>

>> > +                   domain-performance-state = <1>;

>

>> Perhaps instead of a number, this should be a phandle to pstate@1. Then

>> you just get the parent if you need to know the domain.

>

> That's what I did in V2, but then I turned it down considering the parent/child

> relationships we may have.

>

> There are multiple cases we can have:

>

> A.) DeviceX  --->  Parent-domain-1 (Contains Perfomance states)

>

> B.) DeviceX  --->  Parent-domain-1  ---> Parent domain-2 (Contains Perfomance states)

>

>                                     ---> Parent domain-2 (Contains Perfomance states)

>                                     |

>                                     |

> C.) DeviceX  --->  Parent-domain-1  |

>                                     |

>                                     |

>                                     ---> Parent domain-3 (Contains Perfomance states)


I'm a bit confused. How does a domain have 2 parent domains?

You have the same problem either way. If I have performance state 2
for the device, that corresponds to domain 2 or 3?

> The case A.) represents a simple case where the parent domain of the device

> contains the performance states. The phandle can work pretty well in this case.

> But the other cases B.) and C.) are a bit complicated as the direct parent

> domain doesn't allow changing the performance states, but its parents. And so I

> went ahead with numbers instead of phandles. Yes, we will still be able to get

> to the performance state node with the help of phandles, but will that be the

> right thing to do ?

>

> --

> viresh

--
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
Ulf Hansson Feb. 28, 2017, 3:14 p.m. UTC | #4
[...]

>>                                     ---> Parent domain-2 (Contains Perfomance states)

>>                                     |

>>                                     |

>> C.) DeviceX  --->  Parent-domain-1  |

>>                                     |

>>                                     |

>>                                     ---> Parent domain-3 (Contains Perfomance states)

>

> I'm a bit confused. How does a domain have 2 parent domains?


This comes from the early design of the generic PM domain, thus I
assume we have some HW with such complex PM topology. However, I don't
know if it is actually being used.

Moreover, the corresponding DT bindings for "power-domains" parents,
can easily be extended to cover more than one parent. See more in
Documentation/devicetree/bindings/power/power_domain.txt

[...]

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
Rob Herring Feb. 28, 2017, 3:52 p.m. UTC | #5
On Tue, Feb 28, 2017 at 9:14 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> [...]

>

>>>                                     ---> Parent domain-2 (Contains Perfomance states)

>>>                                     |

>>>                                     |

>>> C.) DeviceX  --->  Parent-domain-1  |

>>>                                     |

>>>                                     |

>>>                                     ---> Parent domain-3 (Contains Perfomance states)

>>

>> I'm a bit confused. How does a domain have 2 parent domains?

>

> This comes from the early design of the generic PM domain, thus I

> assume we have some HW with such complex PM topology. However, I don't

> know if it is actually being used.

>

> Moreover, the corresponding DT bindings for "power-domains" parents,

> can easily be extended to cover more than one parent. See more in

> Documentation/devicetree/bindings/power/power_domain.txt


I could easily see device having 2 power domains. For example a cpu
may have separate domains for RAM/caches and logic. And nesting of
power domains is certainly common, but a power domain being contained
in 2 different parents? I don't even see how that is possible in the
physical design. Now if we're mixing PM and power domains again and
the cpu device is pointing to the cpu PM domain which contains 2 power
domains, then certainly that is possible.

Rob
--
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
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
index 9f5ca4457b5f..7f6bb52521b6 100644
--- a/Documentation/devicetree/bindings/opp/opp.txt
+++ b/Documentation/devicetree/bindings/opp/opp.txt
@@ -154,6 +154,15 @@  properties.
 
 - status: Marks the node enabled/disabled.
 
+- domain-performance-state: A positive integer value representing the minimum
+  performance level (of the parent domain) required by the consumer as defined
+  by ../power/power_domain.txt binding document. The OPP nodes can contain the
+  "domain-performance-state" property, only if the device node contains a
+  "power-domains" property. 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 +537,58 @@  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)
+
+/ {
+	cpu0_opp_table: opp_table0 {
+		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>;
+		};
+	};
+
+	foo_domain: power-controller@12340000 {
+		compatible = "foo,power-controller";
+		reg = <0x12340000 0x1000>;
+		#power-domain-cells = <0>;
+
+		performance-states {
+			compatible = "domain-performance-state";
+			pstate@1 {
+				reg = <1>;
+				domain-microvolt = <970000 975000 985000>;
+			};
+			pstate@2 {
+				reg = <2>;
+				domain-microvolt = <1000000 1075000 1085000>;
+			};
+		};
+	}
+
+	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>;
+		};
+	};
+};