On 10/08/16 18:35, Lina Iyer wrote:
On Wed, Aug 10 2016 at 09:27 -0600, Sudeep Holla wrote:
On 05/08/16 00:05, Lina Iyer wrote:
Define power domain and the power states for the domain as defined by
the PSCI firmware.
The 8916 firmware supports OS initiated method of
powering off the CPU clusters.
How is that related to the this DTS change, more details below ?
Cc: <devicetree@xxxxxxxxxxxxxxx>
Signed-off-by: Lina Iyer <lina.iyer@xxxxxxxxxx>
---
arch/arm64/boot/dts/qcom/msm8916.dtsi | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi
b/arch/arm64/boot/dts/qcom/msm8916.dtsi
index 3029773..eb0aaed 100644
--- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
@@ -64,6 +64,7 @@
next-level-cache = <&L2_0>;
enable-method = "psci";
cpu-idle-states = <&CPU_SPC>;
+ power-domains = <&CPU_PD>;
This is really messy. We need to have idle state information at one
place. I prefer to have a hierarchal representation of power-domains
for CPU with idle-states at each level.
};
I see where are going with that. We need to then isolate idle states
from all devices (including CPU) and put them under the umberella of the
domain/parent idle states.
We also need to remember that domain idle states are not just for CPU
domains. There are generic PM domains that also define their idle
states. For some, that hierarchy may not make sense. So forcing it on
all domains is not correct as well.
Yes again I absolutely agree with that. But just represent the hierarchy
present in the hardware in the DT. If it doesn't exist in the hardware
for some device, then it won't be represented.
CPU1: cpu@1 {
@@ -73,6 +74,7 @@
next-level-cache = <&L2_0>;
enable-method = "psci";
cpu-idle-states = <&CPU_SPC>;
+ power-domains = <&CPU_PD>;
};
CPU2: cpu@2 {
@@ -82,6 +84,7 @@
next-level-cache = <&L2_0>;
enable-method = "psci";
cpu-idle-states = <&CPU_SPC>;
+ power-domains = <&CPU_PD>;
};
CPU3: cpu@3 {
@@ -91,6 +94,7 @@
next-level-cache = <&L2_0>;
enable-method = "psci";
cpu-idle-states = <&CPU_SPC>;
+ power-domains = <&CPU_PD>;
};
L2_0: l2-cache {
@@ -113,6 +117,29 @@
psci {
compatible = "arm,psci-1.0";
method = "smc";
Why is it inside PSCI node ? I don't see a need for that.
If it needs to be here, then amend the binding document.
It is described in patch 13/15.
It is inside PSCI node, because PSCI has the domain controller.
OK, I haven't gone through all the patches, I was just interested in
the binding. Sorry for that.
+
+ CPU_PD: cpu-pd@0 {
+ #power-domain-cells = <0>;
+ domain-idle-states = <&CLUSTER_RET>, <&CLUSTER_PWR_DWN>;
+ };
+
+ domain-states {
+ CLUSTER_RET: domain_ret {
+ compatible = "arm,idle-state";
+ arm,psci-suspend-param = <0x1000010>;
+ entry-latency-us = <500>;
+ exit-latency-us = <500>;
+ min-residency-us = <2000>;
+ };
+
+ CLUSTER_PWR_DWN: domain_gdhs {
+ compatible = "arm,idle-state";
+ arm,psci-suspend-param = <0x1000030>;
+ entry-latency-us = <2000>;
+ exit-latency-us = <2000>;
+ min-residency-us = <6000>;
+ };
+ };
So how do you collapse these states into the cpu level states ?
Why do you have to collapse?
To deal with platform co-ordinated mode of cpu idle support. I think I
gave an example in the other email.
We should be able to cope up with platform co-ordinated mode of idle.
For me, this binding and the representation here is designed only to
address OS co-ordinated mode of idle support but it should be other way
around. Design the bindings that can cater any mode (platform and OS
co-ordinated)
As explained to Brendan on the version2 of the series, OSI and PC are
orthagonal to each other. The idle state definition in the devicetree
exactly matches the unique approaches of these 2 modes.
We don't design bindings that depend on the software approaches or
configurations. It should represent the hardware.
In platform coordinated, the CPU determines the idle state of the domain
and selects the state, hence it makes sense to extend the
cpu-idle-states to cover those domain states.
So, are you saying that old bindings are for platform co-ordinated mode
and these new ones are for OS co-ordinated mode. Make it clear in the
binding document and get explicit approval from the DT maintainers for
that. I really don't like that approach.
In OSI, the CPUs only determine their idle states. When they are done
with their idle state, they bubble up and let the domain choose its idle
state and therefore the domain-idle-states is part of the domain
controller.
I understand that, but I don't like the approach taken to define the DT
bindings. DT should have one or the other form and any mode can be used
as long as firmware is queried and put into appropriate mode of operation.
With this addition platform coordinated representation is not broken. If
your SoC supports both platform and os modes, then you can specify the
idle states of both of them in the DT. The clause in firmware/psci.c
will however, choose OSI if its available. I am not sure we want to
dynamically switch betweeen OSI and PC at runtime.
How ? cpu-idle-states in CPU node can't have both:
1. just it's idles states, and
2. complete list of flattened idle states as it' is present today.
--
Regards,
Sudeep
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
