LinuxLists.cc - [PATCH v2 0/9] qcom: Add cpuidle to some platforms

[permalink] [raw]

Subject: [PATCH v2 1/9] arm64: dts: fsl: ls1028a: Fix entry-method property to reflect documentation

The idle-states binding documentation[1] mentions that the
'entry-method' property is required on 64-bit platforms and must be set
to "psci".

We fixed up all uses of the entry-method property in
commit e9880240e4f4 ("arm64: dts: Fix various entry-method properties to
reflect documentation"). But a new one has appeared. Fix it up.

[1] Documentation/devicetree/bindings/arm/idle-states.txt (see
idle-states node)

Signed-off-by: Amit Kucheria <[email protected]>
Acked-by: Sudeep Holla <[email protected]>
Reviewed-by: Bjorn Andersson <[email protected]>
Reviewed-by: Niklas Cassel <[email protected]>
---
arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
index 2896bbcfa3bb..42e7822a0227 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
@@ -51,7 +51,7 @@
* PSCI node is not added default, U-boot will add missing
* parts if it determines to use PSCI.
*/
- entry-method = "arm,psci";
+ entry-method = "psci";

CPU_PH20: cpu-ph20 {
compatible = "arm,idle-state";
--
2.17.1

2019-05-21 09:37:21

[permalink] [raw]

Subject: [PATCH v2 5/9] arm64: dts: qcom: qcs404: Add PSCI cpuidle low power states

From: Niklas Cassel <[email protected]>

Add device bindings for cpuidle states for cpu devices.

Signed-off-by: Niklas Cassel <[email protected]>
Reviewed-by: Vinod Koul <[email protected]>
[rename the idle-states to more generic names and fixups]
Signed-off-by: Amit Kucheria <[email protected]>
Acked-by: Daniel Lezcano <[email protected]>
---
arch/arm64/boot/dts/qcom/qcs404.dtsi | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/qcs404.dtsi b/arch/arm64/boot/dts/qcom/qcs404.dtsi
index e8fd26633d57..0a9b29af64c2 100644
--- a/arch/arm64/boot/dts/qcom/qcs404.dtsi
+++ b/arch/arm64/boot/dts/qcom/qcs404.dtsi
@@ -30,6 +30,7 @@
compatible = "arm,cortex-a53";
reg = <0x100>;
enable-method = "psci";
+ cpu-idle-states = <&CPU_SLEEP_0>;
next-level-cache = <&L2_0>;
};

@@ -38,6 +39,7 @@
compatible = "arm,cortex-a53";
reg = <0x101>;
enable-method = "psci";
+ cpu-idle-states = <&CPU_SLEEP_0>;
next-level-cache = <&L2_0>;
};

@@ -46,6 +48,7 @@
compatible = "arm,cortex-a53";
reg = <0x102>;
enable-method = "psci";
+ cpu-idle-states = <&CPU_SLEEP_0>;
next-level-cache = <&L2_0>;
};

@@ -54,6 +57,7 @@
compatible = "arm,cortex-a53";
reg = <0x103>;
enable-method = "psci";
+ cpu-idle-states = <&CPU_SLEEP_0>;
next-level-cache = <&L2_0>;
};

@@ -61,6 +65,20 @@
compatible = "cache";
cache-level = <2>;
};
+
+ idle-states {
+ entry-method = "psci";
+
+ CPU_SLEEP_0: cpu-sleep-0 {
+ compatible = "arm,idle-state";
+ idle-state-name = "standalone-power-collapse";
+ arm,psci-suspend-param = <0x40000003>;
+ entry-latency-us = <125>;
+ exit-latency-us = <180>;
+ min-residency-us = <595>;
+ local-timer-stop;
+ };
+ };
};

firmware {
--
2.17.1

2019-05-21 09:37:23

[permalink] [raw]

Subject: [PATCH v2 4/9] arm64: dts: qcom: msm8916: Use more generic idle state names

Instead of using Qualcomm-specific terminology, use generic node names
for the idle states that are easier to understand. Move the description
into the "idle-state-name" property.

Signed-off-by: Amit Kucheria <[email protected]>
Reviewed-by: Niklas Cassel <[email protected]>
Acked-by: Daniel Lezcano <[email protected]>
---
arch/arm64/boot/dts/qcom/msm8916.dtsi | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi
index 82ea5b8b37a2..3a8c6c4fcf15 100644
--- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
@@ -110,7 +110,7 @@
reg = <0x0>;
next-level-cache = <&L2_0>;
enable-method = "psci";
- cpu-idle-states = <&CPU_SPC>;
+ cpu-idle-states = <&CPU_SLEEP_0>;
clocks = <&apcs>;
operating-points-v2 = <&cpu_opp_table>;
#cooling-cells = <2>;
@@ -122,7 +122,7 @@
reg = <0x1>;
next-level-cache = <&L2_0>;
enable-method = "psci";
- cpu-idle-states = <&CPU_SPC>;
+ cpu-idle-states = <&CPU_SLEEP_0>;
clocks = <&apcs>;
operating-points-v2 = <&cpu_opp_table>;
#cooling-cells = <2>;
@@ -134,7 +134,7 @@
reg = <0x2>;
next-level-cache = <&L2_0>;
enable-method = "psci";
- cpu-idle-states = <&CPU_SPC>;
+ cpu-idle-states = <&CPU_SLEEP_0>;
clocks = <&apcs>;
operating-points-v2 = <&cpu_opp_table>;
#cooling-cells = <2>;
@@ -146,7 +146,7 @@
reg = <0x3>;
next-level-cache = <&L2_0>;
enable-method = "psci";
- cpu-idle-states = <&CPU_SPC>;
+ cpu-idle-states = <&CPU_SLEEP_0>;
clocks = <&apcs>;
operating-points-v2 = <&cpu_opp_table>;
#cooling-cells = <2>;
@@ -160,8 +160,9 @@
idle-states {
entry-method = "psci";

- CPU_SPC: spc {
+ CPU_SLEEP_0: cpu-sleep-0 {
compatible = "arm,idle-state";
+ idle-state-name = "standalone-power-collapse";
arm,psci-suspend-param = <0x40000002>;
entry-latency-us = <130>;
exit-latency-us = <150>;
--
2.17.1

2019-05-21 09:37:27

[permalink] [raw]

Subject: [PATCH v2 6/9] arm64: dts: qcom: msm8996: Add PSCI cpuidle low power states

Add device bindings for cpuidle states for cpu devices.

msm8996 features 4 cpus - 2 in each cluster. However, all cpus implement
the same microarchitecture and the two clusters only differ in the
maximum frequency attainable by the CPUs.

Signed-off-by: Amit Kucheria <[email protected]>
---
arch/arm64/boot/dts/qcom/msm8996.dtsi | 17 +++++++++++++++++
1 file changed, 17 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
index c761269caf80..4f2fb7885f39 100644
--- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
@@ -95,6 +95,7 @@
compatible = "qcom,kryo";
reg = <0x0 0x0>;
enable-method = "psci";
+ cpu-idle-states = <&CPU_SLEEP_0>;
next-level-cache = <&L2_0>;
L2_0: l2-cache {
compatible = "cache";
@@ -107,6 +108,7 @@
compatible = "qcom,kryo";
reg = <0x0 0x1>;
enable-method = "psci";
+ cpu-idle-states = <&CPU_SLEEP_0>;
next-level-cache = <&L2_0>;
};

@@ -115,6 +117,7 @@
compatible = "qcom,kryo";
reg = <0x0 0x100>;
enable-method = "psci";
+ cpu-idle-states = <&CPU_SLEEP_0>;
next-level-cache = <&L2_1>;
L2_1: l2-cache {
compatible = "cache";
@@ -127,6 +130,7 @@
compatible = "qcom,kryo";
reg = <0x0 0x101>;
enable-method = "psci";
+ cpu-idle-states = <&CPU_SLEEP_0>;
next-level-cache = <&L2_1>;
};

@@ -151,6 +155,19 @@
};
};
};
+
+ idle-states {
+ entry-method = "psci";
+
+ CPU_SLEEP_0: cpu-sleep-0 {
+ compatible = "arm,idle-state";
+ idle-state-name = "standalone-power-collapse";
+ arm,psci-suspend-param = <0x00000004>;
+ entry-latency-us = <40>;
+ exit-latency-us = <80>;
+ min-residency-us = <300>;
+ };
+ };
};

thermal-zones {
--
2.17.1

2019-05-21 09:37:37

[permalink] [raw]

Subject: [PATCH v2 3/9] arm64: dts: qcom: msm8916: Add entry-method property for the idle-states node

The idle-states binding documentation[1] mentions that the
'entry-method' property is required on 64-bit platforms and must be set
to "psci".

[1] Documentation/devicetree/bindings/arm/idle-states.txt (see
idle-states node)

Signed-off-by: Amit Kucheria <[email protected]>
Reviewed-by: Niklas Cassel <[email protected]>
Acked-by: Daniel Lezcano <[email protected]>
---
arch/arm64/boot/dts/qcom/msm8916.dtsi | 2 ++
1 file changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi
index 0803ca8c02da..82ea5b8b37a2 100644
--- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
@@ -158,6 +158,8 @@
};

idle-states {
+ entry-method = "psci";
+
CPU_SPC: spc {
compatible = "arm,idle-state";
arm,psci-suspend-param = <0x40000002>;
--
2.17.1

2019-05-21 09:37:37

[permalink] [raw]

Subject: [PATCH v2 8/9] arm64: dts: qcom: sdm845: Add PSCI cpuidle low power states

From: "Raju P.L.S.S.S.N" <[email protected]>

Add device bindings for cpuidle states for cpu devices.

Cc: <[email protected]>
Signed-off-by: Raju P.L.S.S.S.N <[email protected]>
Reviewed-by: Evan Green <[email protected]>
[amit: rename the idle-states to more generic names and fixups]
Signed-off-by: Amit Kucheria <[email protected]>
Acked-by: Daniel Lezcano <[email protected]>
---
arch/arm64/boot/dts/qcom/sdm845.dtsi | 69 ++++++++++++++++++++++++++++
1 file changed, 69 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index 5308f1671824..a0ae6bf033ee 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -119,6 +119,7 @@
compatible = "qcom,kryo385";
reg = <0x0 0x0>;
enable-method = "psci";
+ cpu-idle-states = <&LITTLE_CPU_SLEEP_0 &LITTLE_CPU_SLEEP_1 &CLUSTER_SLEEP_0>;
qcom,freq-domain = <&cpufreq_hw 0>;
#cooling-cells = <2>;
next-level-cache = <&L2_0>;
@@ -136,6 +137,8 @@
compatible = "qcom,kryo385";
reg = <0x0 0x100>;
enable-method = "psci";
+ cpu-idle-states = <&LITTLE_CPU_SLEEP_0 &LITTLE_CPU_SLEEP_1
+ &CLUSTER_SLEEP_0>;
qcom,freq-domain = <&cpufreq_hw 0>;
#cooling-cells = <2>;
next-level-cache = <&L2_100>;
@@ -150,6 +153,8 @@
compatible = "qcom,kryo385";
reg = <0x0 0x200>;
enable-method = "psci";
+ cpu-idle-states = <&LITTLE_CPU_SLEEP_0 &LITTLE_CPU_SLEEP_1
+ &CLUSTER_SLEEP_0>;
qcom,freq-domain = <&cpufreq_hw 0>;
#cooling-cells = <2>;
next-level-cache = <&L2_200>;
@@ -164,6 +169,8 @@
compatible = "qcom,kryo385";
reg = <0x0 0x300>;
enable-method = "psci";
+ cpu-idle-states = <&LITTLE_CPU_SLEEP_0 &LITTLE_CPU_SLEEP_1
+ &CLUSTER_SLEEP_0>;
qcom,freq-domain = <&cpufreq_hw 0>;
#cooling-cells = <2>;
next-level-cache = <&L2_300>;
@@ -178,6 +185,8 @@
compatible = "qcom,kryo385";
reg = <0x0 0x400>;
enable-method = "psci";
+ cpu-idle-states = <&BIG_CPU_SLEEP_0 &BIG_CPU_SLEEP_1
+ &CLUSTER_SLEEP_0>;
qcom,freq-domain = <&cpufreq_hw 1>;
#cooling-cells = <2>;
next-level-cache = <&L2_400>;
@@ -192,6 +201,8 @@
compatible = "qcom,kryo385";
reg = <0x0 0x500>;
enable-method = "psci";
+ cpu-idle-states = <&BIG_CPU_SLEEP_0 &BIG_CPU_SLEEP_1
+ &CLUSTER_SLEEP_0>;
qcom,freq-domain = <&cpufreq_hw 1>;
#cooling-cells = <2>;
next-level-cache = <&L2_500>;
@@ -206,6 +217,8 @@
compatible = "qcom,kryo385";
reg = <0x0 0x600>;
enable-method = "psci";
+ cpu-idle-states = <&BIG_CPU_SLEEP_0 &BIG_CPU_SLEEP_1
+ &CLUSTER_SLEEP_0>;
qcom,freq-domain = <&cpufreq_hw 1>;
#cooling-cells = <2>;
next-level-cache = <&L2_600>;
@@ -220,6 +233,8 @@
compatible = "qcom,kryo385";
reg = <0x0 0x700>;
enable-method = "psci";
+ cpu-idle-states = <&BIG_CPU_SLEEP_0 &BIG_CPU_SLEEP_1
+ &CLUSTER_SLEEP_0>;
qcom,freq-domain = <&cpufreq_hw 1>;
#cooling-cells = <2>;
next-level-cache = <&L2_700>;
@@ -228,6 +243,60 @@
next-level-cache = <&L3_0>;
};
};
+
+ idle-states {
+ entry-method = "psci";
+
+ LITTLE_CPU_SLEEP_0: cpu-sleep-0-0 {
+ compatible = "arm,idle-state";
+ idle-state-name = "little-power-down";
+ arm,psci-suspend-param = <0x40000003>;
+ entry-latency-us = <350>;
+ exit-latency-us = <461>;
+ min-residency-us = <1890>;
+ local-timer-stop;
+ };
+
+ LITTLE_CPU_SLEEP_1: cpu-sleep-0-1 {
+ compatible = "arm,idle-state";
+ idle-state-name = "little-rail-power-down";
+ arm,psci-suspend-param = <0x40000004>;
+ entry-latency-us = <360>;
+ exit-latency-us = <531>;
+ min-residency-us = <3934>;
+ local-timer-stop;
+ };
+
+ BIG_CPU_SLEEP_0: cpu-sleep-1-0 {
+ compatible = "arm,idle-state";
+ idle-state-name = "big-power-down";
+ arm,psci-suspend-param = <0x40000003>;
+ entry-latency-us = <264>;
+ exit-latency-us = <621>;
+ min-residency-us = <952>;
+ local-timer-stop;
+ };
+
+ BIG_CPU_SLEEP_1: cpu-sleep-1-1 {
+ compatible = "arm,idle-state";
+ idle-state-name = "big-rail-power-down";
+ arm,psci-suspend-param = <0x40000004>;
+ entry-latency-us = <702>;
+ exit-latency-us = <1061>;
+ min-residency-us = <4488>;
+ local-timer-stop;
+ };
+
+ CLUSTER_SLEEP_0: cluster-sleep-0 {
+ compatible = "arm,idle-state";
+ idle-state-name = "cluster-power-down";
+ arm,psci-suspend-param = <0x400000F4>;
+ entry-latency-us = <3263>;
+ exit-latency-us = <6562>;
+ min-residency-us = <9987>;
+ local-timer-stop;
+ };
+ };
};

pmu {
--
2.17.1

2019-05-21 09:38:06

[permalink] [raw]

Subject: [PATCH v2 2/9] Documentation: arm: Link idle-states binding to "enable-method" property

The "enable-method" property for cpu nodes needs to be "psci" for CPU
idle management to be setup correctly.

Add a note to the binding documentation to this effect to make it
obvious.

Signed-off-by: Amit Kucheria <[email protected]>
Acked-by: Sudeep Holla <[email protected]>
---
.../devicetree/bindings/arm/idle-states.txt | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/arm/idle-states.txt b/Documentation/devicetree/bindings/arm/idle-states.txt
index 45730ba60af5..3bdbe675b9e6 100644
--- a/Documentation/devicetree/bindings/arm/idle-states.txt
+++ b/Documentation/devicetree/bindings/arm/idle-states.txt
@@ -241,9 +241,13 @@ processor idle states, defined as device tree nodes, are listed.
- "psci"
# On ARM 32-bit systems this property is optional

-The nodes describing the idle states (state) can only be defined within the
-idle-states node, any other configuration is considered invalid and therefore
-must be ignored.
+This assumes that the "enable-method" property is set to "psci" in the cpu
+node[6] that is responsible for setting up CPU idle management in the OS
+implementation.
+
+The nodes describing the idle states (state) can only be defined
+within the idle-states node, any other configuration is considered invalid
+and therefore must be ignored.

===========================================
4 - state node
@@ -697,3 +701,6 @@ cpus {

[5] Devicetree Specification
https://www.devicetree.org/specifications/
+
+[6] ARM Linux Kernel documentation - Booting AArch64 Linux
+ Documentation/arm64/booting.txt
--
2.17.1

2019-05-21 09:38:44

[permalink] [raw]

Subject: [PATCH v2 7/9] arm64: dts: qcom: msm8998: Add PSCI cpuidle low power states

Add device bindings for cpuidle states for cpu devices.

Cc: Marc Gonzalez <[email protected]>
Signed-off-by: Amit Kucheria <[email protected]>
Acked-by: Daniel Lezcano <[email protected]>
---
arch/arm64/boot/dts/qcom/msm8998.dtsi | 50 +++++++++++++++++++++++++++
1 file changed, 50 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi b/arch/arm64/boot/dts/qcom/msm8998.dtsi
index 3fd0769fe648..54810980fcf9 100644
--- a/arch/arm64/boot/dts/qcom/msm8998.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi
@@ -78,6 +78,7 @@
compatible = "arm,armv8";
reg = <0x0 0x0>;
enable-method = "psci";
+ cpu-idle-states = <&LITTLE_CPU_SLEEP_0 &LITTLE_CPU_SLEEP_1>;
efficiency = <1024>;
next-level-cache = <&L2_0>;
L2_0: l2-cache {
@@ -97,6 +98,7 @@
compatible = "arm,armv8";
reg = <0x0 0x1>;
enable-method = "psci";
+ cpu-idle-states = <&LITTLE_CPU_SLEEP_0 &LITTLE_CPU_SLEEP_1>;
efficiency = <1024>;
next-level-cache = <&L2_0>;
L1_I_1: l1-icache {
@@ -112,6 +114,7 @@
compatible = "arm,armv8";
reg = <0x0 0x2>;
enable-method = "psci";
+ cpu-idle-states = <&LITTLE_CPU_SLEEP_0 &LITTLE_CPU_SLEEP_1>;
efficiency = <1024>;
next-level-cache = <&L2_0>;
L1_I_2: l1-icache {
@@ -127,6 +130,7 @@
compatible = "arm,armv8";
reg = <0x0 0x3>;
enable-method = "psci";
+ cpu-idle-states = <&LITTLE_CPU_SLEEP_0 &LITTLE_CPU_SLEEP_1>;
efficiency = <1024>;
next-level-cache = <&L2_0>;
L1_I_3: l1-icache {
@@ -142,6 +146,7 @@
compatible = "arm,armv8";
reg = <0x0 0x100>;
enable-method = "psci";
+ cpu-idle-states = <&BIG_CPU_SLEEP_0 &BIG_CPU_SLEEP_1>;
efficiency = <1536>;
next-level-cache = <&L2_1>;
L2_1: l2-cache {
@@ -161,6 +166,7 @@
compatible = "arm,armv8";
reg = <0x0 0x101>;
enable-method = "psci";
+ cpu-idle-states = <&BIG_CPU_SLEEP_0 &BIG_CPU_SLEEP_1>;
efficiency = <1536>;
next-level-cache = <&L2_1>;
L1_I_101: l1-icache {
@@ -176,6 +182,7 @@
compatible = "arm,armv8";
reg = <0x0 0x102>;
enable-method = "psci";
+ cpu-idle-states = <&BIG_CPU_SLEEP_0 &BIG_CPU_SLEEP_1>;
efficiency = <1536>;
next-level-cache = <&L2_1>;
L1_I_102: l1-icache {
@@ -191,6 +198,7 @@
compatible = "arm,armv8";
reg = <0x0 0x103>;
enable-method = "psci";
+ cpu-idle-states = <&BIG_CPU_SLEEP_0 &BIG_CPU_SLEEP_1>;
efficiency = <1536>;
next-level-cache = <&L2_1>;
L1_I_103: l1-icache {
@@ -238,6 +246,48 @@
};
};
};
+
+ idle-states {
+ entry-method = "psci";
+
+ LITTLE_CPU_SLEEP_0: cpu-sleep-0-0 {
+ compatible = "arm,idle-state";
+ idle-state-name = "little-retention";
+ arm,psci-suspend-param = <0x00000002>;
+ entry-latency-us = <43>;
+ exit-latency-us = <86>;
+ min-residency-us = <200>;
+ };
+
+ LITTLE_CPU_SLEEP_1: cpu-sleep-0-1 {
+ compatible = "arm,idle-state";
+ idle-state-name = "little-power-collapse";
+ arm,psci-suspend-param = <0x00000003>;
+ entry-latency-us = <100>;
+ exit-latency-us = <612>;
+ min-residency-us = <1000>;
+ local-timer-stop;
+ };
+
+ BIG_CPU_SLEEP_0: cpu-sleep-1-0 {
+ compatible = "arm,idle-state";
+ idle-state-name = "big-retention";
+ arm,psci-suspend-param = <0x00000002>;
+ entry-latency-us = <41>;
+ exit-latency-us = <82>;
+ min-residency-us = <200>;
+ };
+
+ BIG_CPU_SLEEP_1: cpu-sleep-1-1 {
+ compatible = "arm,idle-state";
+ idle-state-name = "big-power-collapse";
+ arm,psci-suspend-param = <0x00000003>;
+ entry-latency-us = <100>;
+ exit-latency-us = <525>;
+ min-residency-us = <1000>;
+ local-timer-stop;
+ };
+ };
};

firmware {
--
2.17.1

2019-05-21 09:38:47

[permalink] [raw]

Subject: [PATCH v2 9/9] arm64: dts: msm8996: Add proper capacity scaling for the cpus

msm8996 features 4 cpus - 2 in each cluster. However, all cpus implement
the same microarchitecture and the two clusters only differ in the
maximum frequency attainable by the CPUs.

Add capacity-dmips-mhz property to allow the topology code to determine
the actual capacity by taking into account the highest frequency for
each CPU.

Signed-off-by: Amit Kucheria <[email protected]>
Suggested-by: Daniel Lezcano <[email protected]>
---
arch/arm64/boot/dts/qcom/msm8996.dtsi | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
index 4f2fb7885f39..e0e8f30ce11a 100644
--- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
@@ -96,6 +96,7 @@
reg = <0x0 0x0>;
enable-method = "psci";
cpu-idle-states = <&CPU_SLEEP_0>;
+ capacity-dmips-mhz = <1024>;
next-level-cache = <&L2_0>;
L2_0: l2-cache {
compatible = "cache";
@@ -109,6 +110,7 @@
reg = <0x0 0x1>;
enable-method = "psci";
cpu-idle-states = <&CPU_SLEEP_0>;
+ capacity-dmips-mhz = <1024>;
next-level-cache = <&L2_0>;
};

@@ -118,6 +120,7 @@
reg = <0x0 0x100>;
enable-method = "psci";
cpu-idle-states = <&CPU_SLEEP_0>;
+ capacity-dmips-mhz = <1024>;
next-level-cache = <&L2_1>;
L2_1: l2-cache {
compatible = "cache";
@@ -131,6 +134,7 @@
reg = <0x0 0x101>;
enable-method = "psci";
cpu-idle-states = <&CPU_SLEEP_0>;
+ capacity-dmips-mhz = <1024>;
next-level-cache = <&L2_1>;
};

--
2.17.1

2019-05-21 10:16:39

by Daniel Lezcano

[permalink] [raw]

Subject: Re: [PATCH v2 6/9] arm64: dts: qcom: msm8996: Add PSCI cpuidle low power states

On 21/05/2019 11:35, Amit Kucheria wrote:
> Add device bindings for cpuidle states for cpu devices.
>
> msm8996 features 4 cpus - 2 in each cluster. However, all cpus implement
> the same microarchitecture and the two clusters only differ in the
> maximum frequency attainable by the CPUs.
>
> Signed-off-by: Amit Kucheria <[email protected]>

Acked-by: Daniel Lezcano <[email protected]>

> ---
> arch/arm64/boot/dts/qcom/msm8996.dtsi | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
> index c761269caf80..4f2fb7885f39 100644
> --- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
> @@ -95,6 +95,7 @@
> compatible = "qcom,kryo";
> reg = <0x0 0x0>;
> enable-method = "psci";
> + cpu-idle-states = <&CPU_SLEEP_0>;
> next-level-cache = <&L2_0>;
> L2_0: l2-cache {
> compatible = "cache";
> @@ -107,6 +108,7 @@
> compatible = "qcom,kryo";
> reg = <0x0 0x1>;
> enable-method = "psci";
> + cpu-idle-states = <&CPU_SLEEP_0>;
> next-level-cache = <&L2_0>;
> };
>
> @@ -115,6 +117,7 @@
> compatible = "qcom,kryo";
> reg = <0x0 0x100>;
> enable-method = "psci";
> + cpu-idle-states = <&CPU_SLEEP_0>;
> next-level-cache = <&L2_1>;
> L2_1: l2-cache {
> compatible = "cache";
> @@ -127,6 +130,7 @@
> compatible = "qcom,kryo";
> reg = <0x0 0x101>;
> enable-method = "psci";
> + cpu-idle-states = <&CPU_SLEEP_0>;
> next-level-cache = <&L2_1>;
> };
>
> @@ -151,6 +155,19 @@
> };
> };
> };
> +
> + idle-states {
> + entry-method = "psci";
> +
> + CPU_SLEEP_0: cpu-sleep-0 {
> + compatible = "arm,idle-state";
> + idle-state-name = "standalone-power-collapse";
> + arm,psci-suspend-param = <0x00000004>;
> + entry-latency-us = <40>;
> + exit-latency-us = <80>;
> + min-residency-us = <300>;
> + };
> + };
> };
>
> thermal-zones {
>

--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

2019-05-21 10:16:51

by Daniel Lezcano

[permalink] [raw]

Subject: Re: [PATCH v2 9/9] arm64: dts: msm8996: Add proper capacity scaling for the cpus

On 21/05/2019 11:35, Amit Kucheria wrote:
> msm8996 features 4 cpus - 2 in each cluster. However, all cpus implement
> the same microarchitecture and the two clusters only differ in the
> maximum frequency attainable by the CPUs.
>
> Add capacity-dmips-mhz property to allow the topology code to determine
> the actual capacity by taking into account the highest frequency for
> each CPU.
>
> Signed-off-by: Amit Kucheria <[email protected]>
> Suggested-by: Daniel Lezcano <[email protected]>

Acked-by: Daniel Lezcano <[email protected]>

> ---
> arch/arm64/boot/dts/qcom/msm8996.dtsi | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
> index 4f2fb7885f39..e0e8f30ce11a 100644
> --- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
> @@ -96,6 +96,7 @@
> reg = <0x0 0x0>;
> enable-method = "psci";
> cpu-idle-states = <&CPU_SLEEP_0>;
> + capacity-dmips-mhz = <1024>;
> next-level-cache = <&L2_0>;
> L2_0: l2-cache {
> compatible = "cache";
> @@ -109,6 +110,7 @@
> reg = <0x0 0x1>;
> enable-method = "psci";
> cpu-idle-states = <&CPU_SLEEP_0>;
> + capacity-dmips-mhz = <1024>;
> next-level-cache = <&L2_0>;
> };
>
> @@ -118,6 +120,7 @@
> reg = <0x0 0x100>;
> enable-method = "psci";
> cpu-idle-states = <&CPU_SLEEP_0>;
> + capacity-dmips-mhz = <1024>;
> next-level-cache = <&L2_1>;
> L2_1: l2-cache {
> compatible = "cache";
> @@ -131,6 +134,7 @@
> reg = <0x0 0x101>;
> enable-method = "psci";
> cpu-idle-states = <&CPU_SLEEP_0>;
> + capacity-dmips-mhz = <1024>;
> next-level-cache = <&L2_1>;
> };
>
>

--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

2019-05-21 12:05:50

by Marc Gonzalez

[permalink] [raw]

Subject: Re: [PATCH v2 7/9] arm64: dts: qcom: msm8998: Add PSCI cpuidle low power states

On 21/05/2019 11:35, Amit Kucheria wrote:

> Add device bindings for cpuidle states for cpu devices.
>
> Cc: Marc Gonzalez <[email protected]>
> Signed-off-by: Amit Kucheria <[email protected]>
> Acked-by: Daniel Lezcano <[email protected]>
> ---
> arch/arm64/boot/dts/qcom/msm8998.dtsi | 50 +++++++++++++++++++++++++++
> 1 file changed, 50 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi b/arch/arm64/boot/dts/qcom/msm8998.dtsi
> index 3fd0769fe648..54810980fcf9 100644
> --- a/arch/arm64/boot/dts/qcom/msm8998.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi
> @@ -78,6 +78,7 @@
> compatible = "arm,armv8";
> reg = <0x0 0x0>;
> enable-method = "psci";
> + cpu-idle-states = <&LITTLE_CPU_SLEEP_0 &LITTLE_CPU_SLEEP_1>;
> efficiency = <1024>;
> next-level-cache = <&L2_0>;
> L2_0: l2-cache {

NB: this patch does not apply cleanly to v5.2-rc1 ;-)

86f93c93dd50 arm64: dts: msm8998: efficiency is not valid property

commit 86f93c93dd5005f0aeb8ce84c2113e21a6006c7d
Author: Amit Kucheria <[email protected]>
AuthorDate: Fri Mar 29 15:42:08 2019 +0530
Commit: Andy Gross <[email protected]>
CommitDate: Tue Apr 9 23:08:17 2019 -0500

After manually fixing up the trivial conflict, the DTB builds without errors.

I then enable
+CONFIG_CPU_IDLE=y
+CONFIG_ARM_CPUIDLE=y

(because I'm using a board-specific tiny defconfig)

And... the system starts to boot, hangs a few seconds, then silently reboots:

scsi 0:0:0:3: Direct-Access SAMSUNG KLUBG4G1CE-B0B1 0800 PQ: 0 ANSI: 6
sd 0:0:0:1: [sdb] 16384-byte physical blocks
sd 0:0:0:2: [sdc] 2048 4096-byte logical blocks: (8.39 MB/8.00 MiB)
sd 0:0:0:2: [sdc] 16384-byte physical blocks

Format: Log Type - Time(microsec) - Message - Optional Info
Log Type: B - Since Boot(Power On Reset), D - Delta, S - Statistic
S - QC_IMAGE_VERSION_STRING=BOOT.XF.1.2.2-00157-M8998LZB-1
S - IMAGE_VARIANT_STRING=Msm8998LA

Looks like the "helpful" behavior of the secure OS...

Regards.

2019-05-21 16:12:02

by Marc Gonzalez

[permalink] [raw]

Subject: Re: [PATCH v2 7/9] arm64: dts: qcom: msm8998: Add PSCI cpuidle low power states

On 21/05/2019 14:03, Marc Gonzalez wrote:

> the system starts to boot, hangs a few seconds, then silently reboots

Using extremely high-tech debugging tools (i.e. spraying printk left and right)
I traced this one down to:

psci_cpu_suspend_enter: 435
psci_cpu_suspend: 171
psci_cpu_suspend: __invoke_psci_fn_smc c4000001
__invoke_psci_fn_smc: id=c4000001 3 0 0
/*** we never return from arm_smccc_smc() ***/

The following dmesg log caught my eye, and might be relevant:

ARM_SMCCC_ARCH_WORKAROUND_1 missing from firmware

If I revert your patch, psci_cpu_suspend_enter() is never called,
so we don't tickle the arm_smccc_smc() monster.

Could it be that my FW doesn't support PSCI?

Regards.

2019-05-22 03:32:21

[permalink] [raw]

Subject: Re: [PATCH v2 3/9] arm64: dts: qcom: msm8916: Add entry-method property for the idle-states node

On Tue 21 May 02:35 PDT 2019, Amit Kucheria wrote:

> The idle-states binding documentation[1] mentions that the
> 'entry-method' property is required on 64-bit platforms and must be set
> to "psci".
>
> [1] Documentation/devicetree/bindings/arm/idle-states.txt (see
> idle-states node)
>
> Signed-off-by: Amit Kucheria <[email protected]>
> Reviewed-by: Niklas Cassel <[email protected]>
> Acked-by: Daniel Lezcano <[email protected]>

Picked up

Regards,
Bjorn

> ---
> arch/arm64/boot/dts/qcom/msm8916.dtsi | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi
> index 0803ca8c02da..82ea5b8b37a2 100644
> --- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
> @@ -158,6 +158,8 @@
> };
>
> idle-states {
> + entry-method = "psci";
> +
> CPU_SPC: spc {
> compatible = "arm,idle-state";
> arm,psci-suspend-param = <0x40000002>;
> --
> 2.17.1
>

2019-05-22 03:33:01

[permalink] [raw]

Subject: Re: [PATCH v2 4/9] arm64: dts: qcom: msm8916: Use more generic idle state names

On Tue 21 May 02:35 PDT 2019, Amit Kucheria wrote:

> Instead of using Qualcomm-specific terminology, use generic node names
> for the idle states that are easier to understand. Move the description
> into the "idle-state-name" property.
>
> Signed-off-by: Amit Kucheria <[email protected]>
> Reviewed-by: Niklas Cassel <[email protected]>
> Acked-by: Daniel Lezcano <[email protected]>
> ---

Picked up

Regards,
Bjorn

> arch/arm64/boot/dts/qcom/msm8916.dtsi | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi
> index 82ea5b8b37a2..3a8c6c4fcf15 100644
> --- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
> @@ -110,7 +110,7 @@
> reg = <0x0>;
> next-level-cache = <&L2_0>;
> enable-method = "psci";
> - cpu-idle-states = <&CPU_SPC>;
> + cpu-idle-states = <&CPU_SLEEP_0>;
> clocks = <&apcs>;
> operating-points-v2 = <&cpu_opp_table>;
> #cooling-cells = <2>;
> @@ -122,7 +122,7 @@
> reg = <0x1>;
> next-level-cache = <&L2_0>;
> enable-method = "psci";
> - cpu-idle-states = <&CPU_SPC>;
> + cpu-idle-states = <&CPU_SLEEP_0>;
> clocks = <&apcs>;
> operating-points-v2 = <&cpu_opp_table>;
> #cooling-cells = <2>;
> @@ -134,7 +134,7 @@
> reg = <0x2>;
> next-level-cache = <&L2_0>;
> enable-method = "psci";
> - cpu-idle-states = <&CPU_SPC>;
> + cpu-idle-states = <&CPU_SLEEP_0>;
> clocks = <&apcs>;
> operating-points-v2 = <&cpu_opp_table>;
> #cooling-cells = <2>;
> @@ -146,7 +146,7 @@
> reg = <0x3>;
> next-level-cache = <&L2_0>;
> enable-method = "psci";
> - cpu-idle-states = <&CPU_SPC>;
> + cpu-idle-states = <&CPU_SLEEP_0>;
> clocks = <&apcs>;
> operating-points-v2 = <&cpu_opp_table>;
> #cooling-cells = <2>;
> @@ -160,8 +160,9 @@
> idle-states {
> entry-method = "psci";
>
> - CPU_SPC: spc {
> + CPU_SLEEP_0: cpu-sleep-0 {
> compatible = "arm,idle-state";
> + idle-state-name = "standalone-power-collapse";
> arm,psci-suspend-param = <0x40000002>;
> entry-latency-us = <130>;
> exit-latency-us = <150>;
> --
> 2.17.1
>

2019-05-22 03:34:07

[permalink] [raw]

Subject: Re: [PATCH v2 5/9] arm64: dts: qcom: qcs404: Add PSCI cpuidle low power states

On Tue 21 May 02:35 PDT 2019, Amit Kucheria wrote:

> From: Niklas Cassel <[email protected]>
>
> Add device bindings for cpuidle states for cpu devices.
>
> Signed-off-by: Niklas Cassel <[email protected]>
> Reviewed-by: Vinod Koul <[email protected]>
> [rename the idle-states to more generic names and fixups]
> Signed-off-by: Amit Kucheria <[email protected]>
> Acked-by: Daniel Lezcano <[email protected]>
> ---

Applied

Regards,
Bjorn

> arch/arm64/boot/dts/qcom/qcs404.dtsi | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/qcs404.dtsi b/arch/arm64/boot/dts/qcom/qcs404.dtsi
> index e8fd26633d57..0a9b29af64c2 100644
> --- a/arch/arm64/boot/dts/qcom/qcs404.dtsi
> +++ b/arch/arm64/boot/dts/qcom/qcs404.dtsi
> @@ -30,6 +30,7 @@
> compatible = "arm,cortex-a53";
> reg = <0x100>;
> enable-method = "psci";
> + cpu-idle-states = <&CPU_SLEEP_0>;
> next-level-cache = <&L2_0>;
> };
>
> @@ -38,6 +39,7 @@
> compatible = "arm,cortex-a53";
> reg = <0x101>;
> enable-method = "psci";
> + cpu-idle-states = <&CPU_SLEEP_0>;
> next-level-cache = <&L2_0>;
> };
>
> @@ -46,6 +48,7 @@
> compatible = "arm,cortex-a53";
> reg = <0x102>;
> enable-method = "psci";
> + cpu-idle-states = <&CPU_SLEEP_0>;
> next-level-cache = <&L2_0>;
> };
>
> @@ -54,6 +57,7 @@
> compatible = "arm,cortex-a53";
> reg = <0x103>;
> enable-method = "psci";
> + cpu-idle-states = <&CPU_SLEEP_0>;
> next-level-cache = <&L2_0>;
> };
>
> @@ -61,6 +65,20 @@
> compatible = "cache";
> cache-level = <2>;
> };
> +
> + idle-states {
> + entry-method = "psci";
> +
> + CPU_SLEEP_0: cpu-sleep-0 {
> + compatible = "arm,idle-state";
> + idle-state-name = "standalone-power-collapse";
> + arm,psci-suspend-param = <0x40000003>;
> + entry-latency-us = <125>;
> + exit-latency-us = <180>;
> + min-residency-us = <595>;
> + local-timer-stop;
> + };
> + };
> };
>
> firmware {
> --
> 2.17.1
>

2019-05-22 03:51:19

[permalink] [raw]

Subject: Re: [PATCH v2 6/9] arm64: dts: qcom: msm8996: Add PSCI cpuidle low power states

On Tue 21 May 02:35 PDT 2019, Amit Kucheria wrote:

> Add device bindings for cpuidle states for cpu devices.
>
> msm8996 features 4 cpus - 2 in each cluster. However, all cpus implement
> the same microarchitecture and the two clusters only differ in the
> maximum frequency attainable by the CPUs.
>
> Signed-off-by: Amit Kucheria <[email protected]>

Applied

Regards,
Bjorn

> ---
> arch/arm64/boot/dts/qcom/msm8996.dtsi | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
> index c761269caf80..4f2fb7885f39 100644
> --- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
> @@ -95,6 +95,7 @@
> compatible = "qcom,kryo";
> reg = <0x0 0x0>;
> enable-method = "psci";
> + cpu-idle-states = <&CPU_SLEEP_0>;
> next-level-cache = <&L2_0>;
> L2_0: l2-cache {
> compatible = "cache";
> @@ -107,6 +108,7 @@
> compatible = "qcom,kryo";
> reg = <0x0 0x1>;
> enable-method = "psci";
> + cpu-idle-states = <&CPU_SLEEP_0>;
> next-level-cache = <&L2_0>;
> };
>
> @@ -115,6 +117,7 @@
> compatible = "qcom,kryo";
> reg = <0x0 0x100>;
> enable-method = "psci";
> + cpu-idle-states = <&CPU_SLEEP_0>;
> next-level-cache = <&L2_1>;
> L2_1: l2-cache {
> compatible = "cache";
> @@ -127,6 +130,7 @@
> compatible = "qcom,kryo";
> reg = <0x0 0x101>;
> enable-method = "psci";
> + cpu-idle-states = <&CPU_SLEEP_0>;
> next-level-cache = <&L2_1>;
> };
>
> @@ -151,6 +155,19 @@
> };
> };
> };
> +
> + idle-states {
> + entry-method = "psci";
> +
> + CPU_SLEEP_0: cpu-sleep-0 {
> + compatible = "arm,idle-state";
> + idle-state-name = "standalone-power-collapse";
> + arm,psci-suspend-param = <0x00000004>;
> + entry-latency-us = <40>;
> + exit-latency-us = <80>;
> + min-residency-us = <300>;
> + };
> + };
> };
>
> thermal-zones {
> --
> 2.17.1
>

2019-05-22 03:52:09

[permalink] [raw]

Subject: Re: [PATCH v2 9/9] arm64: dts: msm8996: Add proper capacity scaling for the cpus

On Tue 21 May 02:35 PDT 2019, Amit Kucheria wrote:

> msm8996 features 4 cpus - 2 in each cluster. However, all cpus implement
> the same microarchitecture and the two clusters only differ in the
> maximum frequency attainable by the CPUs.
>
> Add capacity-dmips-mhz property to allow the topology code to determine
> the actual capacity by taking into account the highest frequency for
> each CPU.
>
> Signed-off-by: Amit Kucheria <[email protected]>
> Suggested-by: Daniel Lezcano <[email protected]>

Applied

Regards,
Bjorn

> ---
> arch/arm64/boot/dts/qcom/msm8996.dtsi | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
> index 4f2fb7885f39..e0e8f30ce11a 100644
> --- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
> @@ -96,6 +96,7 @@
> reg = <0x0 0x0>;
> enable-method = "psci";
> cpu-idle-states = <&CPU_SLEEP_0>;
> + capacity-dmips-mhz = <1024>;
> next-level-cache = <&L2_0>;
> L2_0: l2-cache {
> compatible = "cache";
> @@ -109,6 +110,7 @@
> reg = <0x0 0x1>;
> enable-method = "psci";
> cpu-idle-states = <&CPU_SLEEP_0>;
> + capacity-dmips-mhz = <1024>;
> next-level-cache = <&L2_0>;
> };
>
> @@ -118,6 +120,7 @@
> reg = <0x0 0x100>;
> enable-method = "psci";
> cpu-idle-states = <&CPU_SLEEP_0>;
> + capacity-dmips-mhz = <1024>;
> next-level-cache = <&L2_1>;
> L2_1: l2-cache {
> compatible = "cache";
> @@ -131,6 +134,7 @@
> reg = <0x0 0x101>;
> enable-method = "psci";
> cpu-idle-states = <&CPU_SLEEP_0>;
> + capacity-dmips-mhz = <1024>;
> next-level-cache = <&L2_1>;
> };
>
> --
> 2.17.1
>

2019-05-22 04:01:28

[permalink] [raw]

Subject: Re: [PATCH v2 8/9] arm64: dts: qcom: sdm845: Add PSCI cpuidle low power states

On Tue 21 May 02:35 PDT 2019, Amit Kucheria wrote:

> From: "Raju P.L.S.S.S.N" <[email protected]>
>
> Add device bindings for cpuidle states for cpu devices.
>
> Cc: <[email protected]>
> Signed-off-by: Raju P.L.S.S.S.N <[email protected]>
> Reviewed-by: Evan Green <[email protected]>
> [amit: rename the idle-states to more generic names and fixups]
> Signed-off-by: Amit Kucheria <[email protected]>
> Acked-by: Daniel Lezcano <[email protected]>
> ---

Applied

Thanks,
Bjorn

> arch/arm64/boot/dts/qcom/sdm845.dtsi | 69 ++++++++++++++++++++++++++++
> 1 file changed, 69 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> index 5308f1671824..a0ae6bf033ee 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> @@ -119,6 +119,7 @@
> compatible = "qcom,kryo385";
> reg = <0x0 0x0>;
> enable-method = "psci";
> + cpu-idle-states = <&LITTLE_CPU_SLEEP_0 &LITTLE_CPU_SLEEP_1 &CLUSTER_SLEEP_0>;
> qcom,freq-domain = <&cpufreq_hw 0>;
> #cooling-cells = <2>;
> next-level-cache = <&L2_0>;
> @@ -136,6 +137,8 @@
> compatible = "qcom,kryo385";
> reg = <0x0 0x100>;
> enable-method = "psci";
> + cpu-idle-states = <&LITTLE_CPU_SLEEP_0 &LITTLE_CPU_SLEEP_1
> + &CLUSTER_SLEEP_0>;
> qcom,freq-domain = <&cpufreq_hw 0>;
> #cooling-cells = <2>;
> next-level-cache = <&L2_100>;
> @@ -150,6 +153,8 @@
> compatible = "qcom,kryo385";
> reg = <0x0 0x200>;
> enable-method = "psci";
> + cpu-idle-states = <&LITTLE_CPU_SLEEP_0 &LITTLE_CPU_SLEEP_1
> + &CLUSTER_SLEEP_0>;
> qcom,freq-domain = <&cpufreq_hw 0>;
> #cooling-cells = <2>;
> next-level-cache = <&L2_200>;
> @@ -164,6 +169,8 @@
> compatible = "qcom,kryo385";
> reg = <0x0 0x300>;
> enable-method = "psci";
> + cpu-idle-states = <&LITTLE_CPU_SLEEP_0 &LITTLE_CPU_SLEEP_1
> + &CLUSTER_SLEEP_0>;
> qcom,freq-domain = <&cpufreq_hw 0>;
> #cooling-cells = <2>;
> next-level-cache = <&L2_300>;
> @@ -178,6 +185,8 @@
> compatible = "qcom,kryo385";
> reg = <0x0 0x400>;
> enable-method = "psci";
> + cpu-idle-states = <&BIG_CPU_SLEEP_0 &BIG_CPU_SLEEP_1
> + &CLUSTER_SLEEP_0>;
> qcom,freq-domain = <&cpufreq_hw 1>;
> #cooling-cells = <2>;
> next-level-cache = <&L2_400>;
> @@ -192,6 +201,8 @@
> compatible = "qcom,kryo385";
> reg = <0x0 0x500>;
> enable-method = "psci";
> + cpu-idle-states = <&BIG_CPU_SLEEP_0 &BIG_CPU_SLEEP_1
> + &CLUSTER_SLEEP_0>;
> qcom,freq-domain = <&cpufreq_hw 1>;
> #cooling-cells = <2>;
> next-level-cache = <&L2_500>;
> @@ -206,6 +217,8 @@
> compatible = "qcom,kryo385";
> reg = <0x0 0x600>;
> enable-method = "psci";
> + cpu-idle-states = <&BIG_CPU_SLEEP_0 &BIG_CPU_SLEEP_1
> + &CLUSTER_SLEEP_0>;
> qcom,freq-domain = <&cpufreq_hw 1>;
> #cooling-cells = <2>;
> next-level-cache = <&L2_600>;
> @@ -220,6 +233,8 @@
> compatible = "qcom,kryo385";
> reg = <0x0 0x700>;
> enable-method = "psci";
> + cpu-idle-states = <&BIG_CPU_SLEEP_0 &BIG_CPU_SLEEP_1
> + &CLUSTER_SLEEP_0>;
> qcom,freq-domain = <&cpufreq_hw 1>;
> #cooling-cells = <2>;
> next-level-cache = <&L2_700>;
> @@ -228,6 +243,60 @@
> next-level-cache = <&L3_0>;
> };
> };
> +
> + idle-states {
> + entry-method = "psci";
> +
> + LITTLE_CPU_SLEEP_0: cpu-sleep-0-0 {
> + compatible = "arm,idle-state";
> + idle-state-name = "little-power-down";
> + arm,psci-suspend-param = <0x40000003>;
> + entry-latency-us = <350>;
> + exit-latency-us = <461>;
> + min-residency-us = <1890>;
> + local-timer-stop;
> + };
> +
> + LITTLE_CPU_SLEEP_1: cpu-sleep-0-1 {
> + compatible = "arm,idle-state";
> + idle-state-name = "little-rail-power-down";
> + arm,psci-suspend-param = <0x40000004>;
> + entry-latency-us = <360>;
> + exit-latency-us = <531>;
> + min-residency-us = <3934>;
> + local-timer-stop;
> + };
> +
> + BIG_CPU_SLEEP_0: cpu-sleep-1-0 {
> + compatible = "arm,idle-state";
> + idle-state-name = "big-power-down";
> + arm,psci-suspend-param = <0x40000003>;
> + entry-latency-us = <264>;
> + exit-latency-us = <621>;
> + min-residency-us = <952>;
> + local-timer-stop;
> + };
> +
> + BIG_CPU_SLEEP_1: cpu-sleep-1-1 {
> + compatible = "arm,idle-state";
> + idle-state-name = "big-rail-power-down";
> + arm,psci-suspend-param = <0x40000004>;
> + entry-latency-us = <702>;
> + exit-latency-us = <1061>;
> + min-residency-us = <4488>;
> + local-timer-stop;
> + };
> +
> + CLUSTER_SLEEP_0: cluster-sleep-0 {
> + compatible = "arm,idle-state";
> + idle-state-name = "cluster-power-down";
> + arm,psci-suspend-param = <0x400000F4>;
> + entry-latency-us = <3263>;
> + exit-latency-us = <6562>;
> + min-residency-us = <9987>;
> + local-timer-stop;
> + };
> + };
> };
>
> pmu {
> --
> 2.17.1
>

2019-05-23 21:26:15

by Niklas Cassel

[permalink] [raw]

Subject: Re: [PATCH v2 6/9] arm64: dts: qcom: msm8996: Add PSCI cpuidle low power states

On Tue, May 21, 2019 at 03:05:16PM +0530, Amit Kucheria wrote:
> Add device bindings for cpuidle states for cpu devices.
>
> msm8996 features 4 cpus - 2 in each cluster. However, all cpus implement
> the same microarchitecture and the two clusters only differ in the
> maximum frequency attainable by the CPUs.
>
> Signed-off-by: Amit Kucheria <[email protected]>
> ---
> arch/arm64/boot/dts/qcom/msm8996.dtsi | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
> index c761269caf80..4f2fb7885f39 100644
> --- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
> @@ -95,6 +95,7 @@
> compatible = "qcom,kryo";
> reg = <0x0 0x0>;
> enable-method = "psci";
> + cpu-idle-states = <&CPU_SLEEP_0>;
> next-level-cache = <&L2_0>;
> L2_0: l2-cache {
> compatible = "cache";
> @@ -107,6 +108,7 @@
> compatible = "qcom,kryo";
> reg = <0x0 0x1>;
> enable-method = "psci";
> + cpu-idle-states = <&CPU_SLEEP_0>;
> next-level-cache = <&L2_0>;
> };
>
> @@ -115,6 +117,7 @@
> compatible = "qcom,kryo";
> reg = <0x0 0x100>;
> enable-method = "psci";
> + cpu-idle-states = <&CPU_SLEEP_0>;
> next-level-cache = <&L2_1>;
> L2_1: l2-cache {
> compatible = "cache";
> @@ -127,6 +130,7 @@
> compatible = "qcom,kryo";
> reg = <0x0 0x101>;
> enable-method = "psci";
> + cpu-idle-states = <&CPU_SLEEP_0>;
> next-level-cache = <&L2_1>;
> };
>
> @@ -151,6 +155,19 @@
> };
> };
> };
> +
> + idle-states {
> + entry-method = "psci";
> +
> + CPU_SLEEP_0: cpu-sleep-0 {
> + compatible = "arm,idle-state";
> + idle-state-name = "standalone-power-collapse";
> + arm,psci-suspend-param = <0x00000004>;
> + entry-latency-us = <40>;
> + exit-latency-us = <80>;

Hello Amit,

Looking at this line of code in msm-4.14:
https://source.codeaurora.org/quic/la/kernel/msm-4.14/tree/drivers/cpuidle/lpm-levels.c?h=LA.UM.7.1.r1-14000-sm8150.0#n993

And seeing the equivalent in msm-4.4:
https://source.codeaurora.org/quic/la/kernel/msm-4.4/tree/drivers/cpuidle/lpm-levels.c?h=msm-4.4#n1080

It becomes obvious that

qcom,time-overhead == entry-latency-us + exit-latency-us
and
qcom,latency-us == exit-latency-us

which means that

entry-latency-us == qcom,time-overhead - qcom,latency-us

Using this formula, with the numbers from downstream SDM845:
https://source.codeaurora.org/quic/la/kernel/msm-4.9/tree/arch/arm64/boot/dts/qcom/sdm845-pm.dtsi?h=msm-4.9#n123

qcom,latency-us = <621>;
qcom,time-overhead = <885>;

885 - 621 = 264

we end up with the same values that Raju
has in his submission for upstream SDM845:
https://patchwork.kernel.org/patch/10953253/

entry-latency-us = <264>;
exit-latency-us = <621>;

Which for msm8996:

qcom,latency-us = <80>;
qcom,time-overhead = <210>;

gives:

entry-latency-us = <130>
exit-latency-us = <80>;

> + min-residency-us = <300>;
> + };
> + };
> };
>
> thermal-zones {
> --
> 2.17.1
>

2019-06-13 23:14:12

by Rob Herring (Arm)

[permalink] [raw]

Subject: Re: [PATCH v2 2/9] Documentation: arm: Link idle-states binding to "enable-method" property

On Tue, 21 May 2019 15:05:12 +0530, Amit Kucheria wrote:
> The "enable-method" property for cpu nodes needs to be "psci" for CPU
> idle management to be setup correctly.
>
> Add a note to the binding documentation to this effect to make it
> obvious.
>
> Signed-off-by: Amit Kucheria <[email protected]>
> Acked-by: Sudeep Holla <[email protected]>
> ---
> .../devicetree/bindings/arm/idle-states.txt | 13 ++++++++++---
> 1 file changed, 10 insertions(+), 3 deletions(-)
>

Applied, thanks.

Rob

2019-09-30 22:32:55

[permalink] [raw]

Subject: Re: [PATCH v2 7/9] arm64: dts: qcom: msm8998: Add PSCI cpuidle low power states

Amit, the merged version of the below change causes a boot failure
(nasty hang, sometimes with RCU stalls) on the msm8998 laptops. Oddly
enough, it seems to be resolved if I remove the cpu-idle-states
property from one of the cpu nodes.

I see no issues with the msm8998 MTP.

Do you have any suggestions on how we might debug this?

On Tue, May 21, 2019 at 3:38 AM Amit Kucheria <[email protected]> wrote:
>
> Add device bindings for cpuidle states for cpu devices.
>
> Cc: Marc Gonzalez <[email protected]>
> Signed-off-by: Amit Kucheria <[email protected]>
> Acked-by: Daniel Lezcano <[email protected]>
> ---
> arch/arm64/boot/dts/qcom/msm8998.dtsi | 50 +++++++++++++++++++++++++++
> 1 file changed, 50 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi b/arch/arm64/boot/dts/qcom/msm8998.dtsi
> index 3fd0769fe648..54810980fcf9 100644
> --- a/arch/arm64/boot/dts/qcom/msm8998.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi
> @@ -78,6 +78,7 @@
> compatible = "arm,armv8";
> reg = <0x0 0x0>;
> enable-method = "psci";
> + cpu-idle-states = <&LITTLE_CPU_SLEEP_0 &LITTLE_CPU_SLEEP_1>;
> efficiency = <1024>;
> next-level-cache = <&L2_0>;
> L2_0: l2-cache {
> @@ -97,6 +98,7 @@
> compatible = "arm,armv8";
> reg = <0x0 0x1>;
> enable-method = "psci";
> + cpu-idle-states = <&LITTLE_CPU_SLEEP_0 &LITTLE_CPU_SLEEP_1>;
> efficiency = <1024>;
> next-level-cache = <&L2_0>;
> L1_I_1: l1-icache {
> @@ -112,6 +114,7 @@
> compatible = "arm,armv8";
> reg = <0x0 0x2>;
> enable-method = "psci";
> + cpu-idle-states = <&LITTLE_CPU_SLEEP_0 &LITTLE_CPU_SLEEP_1>;
> efficiency = <1024>;
> next-level-cache = <&L2_0>;
> L1_I_2: l1-icache {
> @@ -127,6 +130,7 @@
> compatible = "arm,armv8";
> reg = <0x0 0x3>;
> enable-method = "psci";
> + cpu-idle-states = <&LITTLE_CPU_SLEEP_0 &LITTLE_CPU_SLEEP_1>;
> efficiency = <1024>;
> next-level-cache = <&L2_0>;
> L1_I_3: l1-icache {
> @@ -142,6 +146,7 @@
> compatible = "arm,armv8";
> reg = <0x0 0x100>;
> enable-method = "psci";
> + cpu-idle-states = <&BIG_CPU_SLEEP_0 &BIG_CPU_SLEEP_1>;
> efficiency = <1536>;
> next-level-cache = <&L2_1>;
> L2_1: l2-cache {
> @@ -161,6 +166,7 @@
> compatible = "arm,armv8";
> reg = <0x0 0x101>;
> enable-method = "psci";
> + cpu-idle-states = <&BIG_CPU_SLEEP_0 &BIG_CPU_SLEEP_1>;
> efficiency = <1536>;
> next-level-cache = <&L2_1>;
> L1_I_101: l1-icache {
> @@ -176,6 +182,7 @@
> compatible = "arm,armv8";
> reg = <0x0 0x102>;
> enable-method = "psci";
> + cpu-idle-states = <&BIG_CPU_SLEEP_0 &BIG_CPU_SLEEP_1>;
> efficiency = <1536>;
> next-level-cache = <&L2_1>;
> L1_I_102: l1-icache {
> @@ -191,6 +198,7 @@
> compatible = "arm,armv8";
> reg = <0x0 0x103>;
> enable-method = "psci";
> + cpu-idle-states = <&BIG_CPU_SLEEP_0 &BIG_CPU_SLEEP_1>;
> efficiency = <1536>;
> next-level-cache = <&L2_1>;
> L1_I_103: l1-icache {
> @@ -238,6 +246,48 @@
> };
> };
> };
> +
> + idle-states {
> + entry-method = "psci";
> +
> + LITTLE_CPU_SLEEP_0: cpu-sleep-0-0 {
> + compatible = "arm,idle-state";
> + idle-state-name = "little-retention";
> + arm,psci-suspend-param = <0x00000002>;
> + entry-latency-us = <43>;
> + exit-latency-us = <86>;
> + min-residency-us = <200>;
> + };
> +
> + LITTLE_CPU_SLEEP_1: cpu-sleep-0-1 {
> + compatible = "arm,idle-state";
> + idle-state-name = "little-power-collapse";
> + arm,psci-suspend-param = <0x00000003>;
> + entry-latency-us = <100>;
> + exit-latency-us = <612>;
> + min-residency-us = <1000>;
> + local-timer-stop;
> + };
> +
> + BIG_CPU_SLEEP_0: cpu-sleep-1-0 {
> + compatible = "arm,idle-state";
> + idle-state-name = "big-retention";
> + arm,psci-suspend-param = <0x00000002>;
> + entry-latency-us = <41>;
> + exit-latency-us = <82>;
> + min-residency-us = <200>;
> + };
> +
> + BIG_CPU_SLEEP_1: cpu-sleep-1-1 {
> + compatible = "arm,idle-state";
> + idle-state-name = "big-power-collapse";
> + arm,psci-suspend-param = <0x00000003>;
> + entry-latency-us = <100>;
> + exit-latency-us = <525>;
> + min-residency-us = <1000>;
> + local-timer-stop;
> + };
> + };
> };
>
> firmware {
> --
> 2.17.1
>

2019-09-30 22:46:05

[permalink] [raw]

Subject: Re: [PATCH v2 7/9] arm64: dts: qcom: msm8998: Add PSCI cpuidle low power states

Can you try removing just the *SLEEP_1 states from the cpu-idle-states
property? I want to understand if this is triggered just by the deeper
C-state.

On Tue, Oct 1, 2019 at 3:50 AM Jeffrey Hugo <[email protected]> wrote:
>
> Amit, the merged version of the below change causes a boot failure
> (nasty hang, sometimes with RCU stalls) on the msm8998 laptops. Oddly
> enough, it seems to be resolved if I remove the cpu-idle-states
> property from one of the cpu nodes.
>
> I see no issues with the msm8998 MTP.
>
> Do you have any suggestions on how we might debug this?
>
> On Tue, May 21, 2019 at 3:38 AM Amit Kucheria <[email protected]> wrote:
> >
> > Add device bindings for cpuidle states for cpu devices.
> >
> > Cc: Marc Gonzalez <[email protected]>
> > Signed-off-by: Amit Kucheria <[email protected]>
> > Acked-by: Daniel Lezcano <[email protected]>
> > ---
> > arch/arm64/boot/dts/qcom/msm8998.dtsi | 50 +++++++++++++++++++++++++++
> > 1 file changed, 50 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi b/arch/arm64/boot/dts/qcom/msm8998.dtsi
> > index 3fd0769fe648..54810980fcf9 100644
> > --- a/arch/arm64/boot/dts/qcom/msm8998.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi
> > @@ -78,6 +78,7 @@
> > compatible = "arm,armv8";
> > reg = <0x0 0x0>;
> > enable-method = "psci";
> > + cpu-idle-states = <&LITTLE_CPU_SLEEP_0 &LITTLE_CPU_SLEEP_1>;
> > efficiency = <1024>;
> > next-level-cache = <&L2_0>;
> > L2_0: l2-cache {
> > @@ -97,6 +98,7 @@
> > compatible = "arm,armv8";
> > reg = <0x0 0x1>;
> > enable-method = "psci";
> > + cpu-idle-states = <&LITTLE_CPU_SLEEP_0 &LITTLE_CPU_SLEEP_1>;
> > efficiency = <1024>;
> > next-level-cache = <&L2_0>;
> > L1_I_1: l1-icache {
> > @@ -112,6 +114,7 @@
> > compatible = "arm,armv8";
> > reg = <0x0 0x2>;
> > enable-method = "psci";
> > + cpu-idle-states = <&LITTLE_CPU_SLEEP_0 &LITTLE_CPU_SLEEP_1>;
> > efficiency = <1024>;
> > next-level-cache = <&L2_0>;
> > L1_I_2: l1-icache {
> > @@ -127,6 +130,7 @@
> > compatible = "arm,armv8";
> > reg = <0x0 0x3>;
> > enable-method = "psci";
> > + cpu-idle-states = <&LITTLE_CPU_SLEEP_0 &LITTLE_CPU_SLEEP_1>;
> > efficiency = <1024>;
> > next-level-cache = <&L2_0>;
> > L1_I_3: l1-icache {
> > @@ -142,6 +146,7 @@
> > compatible = "arm,armv8";
> > reg = <0x0 0x100>;
> > enable-method = "psci";
> > + cpu-idle-states = <&BIG_CPU_SLEEP_0 &BIG_CPU_SLEEP_1>;
> > efficiency = <1536>;
> > next-level-cache = <&L2_1>;
> > L2_1: l2-cache {
> > @@ -161,6 +166,7 @@
> > compatible = "arm,armv8";
> > reg = <0x0 0x101>;
> > enable-method = "psci";
> > + cpu-idle-states = <&BIG_CPU_SLEEP_0 &BIG_CPU_SLEEP_1>;
> > efficiency = <1536>;
> > next-level-cache = <&L2_1>;
> > L1_I_101: l1-icache {
> > @@ -176,6 +182,7 @@
> > compatible = "arm,armv8";
> > reg = <0x0 0x102>;
> > enable-method = "psci";
> > + cpu-idle-states = <&BIG_CPU_SLEEP_0 &BIG_CPU_SLEEP_1>;
> > efficiency = <1536>;
> > next-level-cache = <&L2_1>;
> > L1_I_102: l1-icache {
> > @@ -191,6 +198,7 @@
> > compatible = "arm,armv8";
> > reg = <0x0 0x103>;
> > enable-method = "psci";
> > + cpu-idle-states = <&BIG_CPU_SLEEP_0 &BIG_CPU_SLEEP_1>;
> > efficiency = <1536>;
> > next-level-cache = <&L2_1>;
> > L1_I_103: l1-icache {
> > @@ -238,6 +246,48 @@
> > };
> > };
> > };
> > +
> > + idle-states {
> > + entry-method = "psci";
> > +
> > + LITTLE_CPU_SLEEP_0: cpu-sleep-0-0 {
> > + compatible = "arm,idle-state";
> > + idle-state-name = "little-retention";
> > + arm,psci-suspend-param = <0x00000002>;
> > + entry-latency-us = <43>;
> > + exit-latency-us = <86>;
> > + min-residency-us = <200>;
> > + };
> > +
> > + LITTLE_CPU_SLEEP_1: cpu-sleep-0-1 {
> > + compatible = "arm,idle-state";
> > + idle-state-name = "little-power-collapse";
> > + arm,psci-suspend-param = <0x00000003>;
> > + entry-latency-us = <100>;
> > + exit-latency-us = <612>;
> > + min-residency-us = <1000>;
> > + local-timer-stop;
> > + };
> > +
> > + BIG_CPU_SLEEP_0: cpu-sleep-1-0 {
> > + compatible = "arm,idle-state";
> > + idle-state-name = "big-retention";
> > + arm,psci-suspend-param = <0x00000002>;
> > + entry-latency-us = <41>;
> > + exit-latency-us = <82>;
> > + min-residency-us = <200>;
> > + };
> > +
> > + BIG_CPU_SLEEP_1: cpu-sleep-1-1 {
> > + compatible = "arm,idle-state";
> > + idle-state-name = "big-power-collapse";
> > + arm,psci-suspend-param = <0x00000003>;
> > + entry-latency-us = <100>;
> > + exit-latency-us = <525>;
> > + min-residency-us = <1000>;
> > + local-timer-stop;
> > + };
> > + };
> > };
> >
> > firmware {
> > --
> > 2.17.1
> >

2019-10-01 14:23:12

[permalink] [raw]

Subject: Re: [PATCH v2 7/9] arm64: dts: qcom: msm8998: Add PSCI cpuidle low power states

On Mon, Sep 30, 2019 at 4:44 PM Amit Kucheria <[email protected]> wrote:
>
> Can you try removing just the *SLEEP_1 states from the cpu-idle-states
> property? I want to understand if this is triggered just by the deeper
> C-state.

Still seeing the issue with just the SLEEP_0 states. For reference,
Bjorn suggested adding kpti=no to the command line, which also
appeared to have no effect on the issue.

>
> On Tue, Oct 1, 2019 at 3:50 AM Jeffrey Hugo <[email protected]> wrote:
> >
> > Amit, the merged version of the below change causes a boot failure
> > (nasty hang, sometimes with RCU stalls) on the msm8998 laptops. Oddly
> > enough, it seems to be resolved if I remove the cpu-idle-states
> > property from one of the cpu nodes.
> >
> > I see no issues with the msm8998 MTP.
> >
> > Do you have any suggestions on how we might debug this?
> >
> > On Tue, May 21, 2019 at 3:38 AM Amit Kucheria <[email protected]> wrote:
> > >
> > > Add device bindings for cpuidle states for cpu devices.
> > >
> > > Cc: Marc Gonzalez <[email protected]>
> > > Signed-off-by: Amit Kucheria <[email protected]>
> > > Acked-by: Daniel Lezcano <[email protected]>
> > > ---
> > > arch/arm64/boot/dts/qcom/msm8998.dtsi | 50 +++++++++++++++++++++++++++
> > > 1 file changed, 50 insertions(+)
> > >
> > > diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi b/arch/arm64/boot/dts/qcom/msm8998.dtsi
> > > index 3fd0769fe648..54810980fcf9 100644
> > > --- a/arch/arm64/boot/dts/qcom/msm8998.dtsi
> > > +++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi
> > > @@ -78,6 +78,7 @@
> > > compatible = "arm,armv8";
> > > reg = <0x0 0x0>;
> > > enable-method = "psci";
> > > + cpu-idle-states = <&LITTLE_CPU_SLEEP_0 &LITTLE_CPU_SLEEP_1>;
> > > efficiency = <1024>;
> > > next-level-cache = <&L2_0>;
> > > L2_0: l2-cache {
> > > @@ -97,6 +98,7 @@
> > > compatible = "arm,armv8";
> > > reg = <0x0 0x1>;
> > > enable-method = "psci";
> > > + cpu-idle-states = <&LITTLE_CPU_SLEEP_0 &LITTLE_CPU_SLEEP_1>;
> > > efficiency = <1024>;
> > > next-level-cache = <&L2_0>;
> > > L1_I_1: l1-icache {
> > > @@ -112,6 +114,7 @@
> > > compatible = "arm,armv8";
> > > reg = <0x0 0x2>;
> > > enable-method = "psci";
> > > + cpu-idle-states = <&LITTLE_CPU_SLEEP_0 &LITTLE_CPU_SLEEP_1>;
> > > efficiency = <1024>;
> > > next-level-cache = <&L2_0>;
> > > L1_I_2: l1-icache {
> > > @@ -127,6 +130,7 @@
> > > compatible = "arm,armv8";
> > > reg = <0x0 0x3>;
> > > enable-method = "psci";
> > > + cpu-idle-states = <&LITTLE_CPU_SLEEP_0 &LITTLE_CPU_SLEEP_1>;
> > > efficiency = <1024>;
> > > next-level-cache = <&L2_0>;
> > > L1_I_3: l1-icache {
> > > @@ -142,6 +146,7 @@
> > > compatible = "arm,armv8";
> > > reg = <0x0 0x100>;
> > > enable-method = "psci";
> > > + cpu-idle-states = <&BIG_CPU_SLEEP_0 &BIG_CPU_SLEEP_1>;
> > > efficiency = <1536>;
> > > next-level-cache = <&L2_1>;
> > > L2_1: l2-cache {
> > > @@ -161,6 +166,7 @@
> > > compatible = "arm,armv8";
> > > reg = <0x0 0x101>;
> > > enable-method = "psci";
> > > + cpu-idle-states = <&BIG_CPU_SLEEP_0 &BIG_CPU_SLEEP_1>;
> > > efficiency = <1536>;
> > > next-level-cache = <&L2_1>;
> > > L1_I_101: l1-icache {
> > > @@ -176,6 +182,7 @@
> > > compatible = "arm,armv8";
> > > reg = <0x0 0x102>;
> > > enable-method = "psci";
> > > + cpu-idle-states = <&BIG_CPU_SLEEP_0 &BIG_CPU_SLEEP_1>;
> > > efficiency = <1536>;
> > > next-level-cache = <&L2_1>;
> > > L1_I_102: l1-icache {
> > > @@ -191,6 +198,7 @@
> > > compatible = "arm,armv8";
> > > reg = <0x0 0x103>;
> > > enable-method = "psci";
> > > + cpu-idle-states = <&BIG_CPU_SLEEP_0 &BIG_CPU_SLEEP_1>;
> > > efficiency = <1536>;
> > > next-level-cache = <&L2_1>;
> > > L1_I_103: l1-icache {
> > > @@ -238,6 +246,48 @@
> > > };
> > > };
> > > };
> > > +
> > > + idle-states {
> > > + entry-method = "psci";
> > > +
> > > + LITTLE_CPU_SLEEP_0: cpu-sleep-0-0 {
> > > + compatible = "arm,idle-state";
> > > + idle-state-name = "little-retention";
> > > + arm,psci-suspend-param = <0x00000002>;
> > > + entry-latency-us = <43>;
> > > + exit-latency-us = <86>;
> > > + min-residency-us = <200>;
> > > + };
> > > +
> > > + LITTLE_CPU_SLEEP_1: cpu-sleep-0-1 {
> > > + compatible = "arm,idle-state";
> > > + idle-state-name = "little-power-collapse";
> > > + arm,psci-suspend-param = <0x00000003>;
> > > + entry-latency-us = <100>;
> > > + exit-latency-us = <612>;
> > > + min-residency-us = <1000>;
> > > + local-timer-stop;
> > > + };
> > > +
> > > + BIG_CPU_SLEEP_0: cpu-sleep-1-0 {
> > > + compatible = "arm,idle-state";
> > > + idle-state-name = "big-retention";
> > > + arm,psci-suspend-param = <0x00000002>;
> > > + entry-latency-us = <41>;
> > > + exit-latency-us = <82>;
> > > + min-residency-us = <200>;
> > > + };
> > > +
> > > + BIG_CPU_SLEEP_1: cpu-sleep-1-1 {
> > > + compatible = "arm,idle-state";
> > > + idle-state-name = "big-power-collapse";
> > > + arm,psci-suspend-param = <0x00000003>;
> > > + entry-latency-us = <100>;
> > > + exit-latency-us = <525>;
> > > + min-residency-us = <1000>;
> > > + local-timer-stop;
> > > + };
> > > + };
> > > };
> > >
> > > firmware {
> > > --
> > > 2.17.1
> > >

2019-10-02 09:20:44

by Niklas Cassel

[permalink] [raw]

Subject: Re: [PATCH v2 7/9] arm64: dts: qcom: msm8998: Add PSCI cpuidle low power states

On Mon, Sep 30, 2019 at 04:20:15PM -0600, Jeffrey Hugo wrote:
> Amit, the merged version of the below change causes a boot failure
> (nasty hang, sometimes with RCU stalls) on the msm8998 laptops. Oddly
> enough, it seems to be resolved if I remove the cpu-idle-states
> property from one of the cpu nodes.
>
> I see no issues with the msm8998 MTP.

Hello Jeffrey, Amit,

If the PSCI idle states work properly on the msm8998 devboard (MTP),
but causes crashes on msm8998 laptops, the only logical change is
that the PSCI firmware is different between the two devices.

Kind regards,
Niklas

2019-10-02 09:30:22

by Niklas Cassel

[permalink] [raw]

Subject: Re: [PATCH v2 7/9] arm64: dts: qcom: msm8998: Add PSCI cpuidle low power states

On Wed, Oct 02, 2019 at 11:19:50AM +0200, Niklas Cassel wrote:
> On Mon, Sep 30, 2019 at 04:20:15PM -0600, Jeffrey Hugo wrote:
> > Amit, the merged version of the below change causes a boot failure
> > (nasty hang, sometimes with RCU stalls) on the msm8998 laptops. Oddly
> > enough, it seems to be resolved if I remove the cpu-idle-states
> > property from one of the cpu nodes.
> >
> > I see no issues with the msm8998 MTP.
>
> Hello Jeffrey, Amit,
>
> If the PSCI idle states work properly on the msm8998 devboard (MTP),
> but causes crashes on msm8998 laptops, the only logical change is
> that the PSCI firmware is different between the two devices.

Since the msm8998 laptops boot using ACPI, perhaps these laptops
doesn't support PSCI/have any PSCI firmware at all.

Kind regards,
Niklas

2019-10-02 18:19:53

[permalink] [raw]

Subject: Re: [PATCH v2 7/9] arm64: dts: qcom: msm8998: Add PSCI cpuidle low power states

On Wed, Oct 2, 2019 at 3:27 AM Niklas Cassel <[email protected]> wrote:
>
> On Wed, Oct 02, 2019 at 11:19:50AM +0200, Niklas Cassel wrote:
> > On Mon, Sep 30, 2019 at 04:20:15PM -0600, Jeffrey Hugo wrote:
> > > Amit, the merged version of the below change causes a boot failure
> > > (nasty hang, sometimes with RCU stalls) on the msm8998 laptops. Oddly
> > > enough, it seems to be resolved if I remove the cpu-idle-states
> > > property from one of the cpu nodes.
> > >
> > > I see no issues with the msm8998 MTP.
> >
> > Hello Jeffrey, Amit,
> >
> > If the PSCI idle states work properly on the msm8998 devboard (MTP),
> > but causes crashes on msm8998 laptops, the only logical change is
> > that the PSCI firmware is different between the two devices.
>
> Since the msm8998 laptops boot using ACPI, perhaps these laptops
> doesn't support PSCI/have any PSCI firmware at all.

They have PSCI. If there was no PSCI, I would expect the PSCI
get_version request from Linux to fail, and all PSCI functionality to
be disabled.

However, your mention about ACPI sparked a thought. ACPI describes
the idle states, along with the PSCI info, in the ACPI0007 devices.
Those exist on the laptops, and the info mostly correlates with Amit's
patch (ACPI seems to be a bit more conservative about the latencies,
and describes one additional deeper state). However, upon a detailed
analysis of the ACPI description, I did find something relevant - the
retention state is not enabled.

So, I hacked out the retention state from Amit's patch, and I did not
observe a hang. I used sysfs, and appeared able to validate that the
power collapse state was being used successfully.

I'm guessing that something is weird with the laptops, where the CPUs
can go into retention, but not come out, thus causing issues.

I'll post a patch to fix up the laptops. Thanks for all the help.

2019-10-04 07:56:04

[permalink] [raw]

Subject: Re: [PATCH v2 7/9] arm64: dts: qcom: msm8998: Add PSCI cpuidle low power states

On Wed, Oct 2, 2019 at 11:48 PM Jeffrey Hugo <[email protected]> wrote:
>
> On Wed, Oct 2, 2019 at 3:27 AM Niklas Cassel <[email protected]> wrote:
> >
> > On Wed, Oct 02, 2019 at 11:19:50AM +0200, Niklas Cassel wrote:
> > > On Mon, Sep 30, 2019 at 04:20:15PM -0600, Jeffrey Hugo wrote:
> > > > Amit, the merged version of the below change causes a boot failure
> > > > (nasty hang, sometimes with RCU stalls) on the msm8998 laptops. Oddly
> > > > enough, it seems to be resolved if I remove the cpu-idle-states
> > > > property from one of the cpu nodes.
> > > >
> > > > I see no issues with the msm8998 MTP.
> > >
> > > Hello Jeffrey, Amit,
> > >
> > > If the PSCI idle states work properly on the msm8998 devboard (MTP),
> > > but causes crashes on msm8998 laptops, the only logical change is
> > > that the PSCI firmware is different between the two devices.
> >
> > Since the msm8998 laptops boot using ACPI, perhaps these laptops
> > doesn't support PSCI/have any PSCI firmware at all.
>
> They have PSCI. If there was no PSCI, I would expect the PSCI
> get_version request from Linux to fail, and all PSCI functionality to
> be disabled.
>
> However, your mention about ACPI sparked a thought. ACPI describes
> the idle states, along with the PSCI info, in the ACPI0007 devices.
> Those exist on the laptops, and the info mostly correlates with Amit's
> patch (ACPI seems to be a bit more conservative about the latencies,
> and describes one additional deeper state). However, upon a detailed
> analysis of the ACPI description, I did find something relevant - the
> retention state is not enabled.
>
> So, I hacked out the retention state from Amit's patch, and I did not
> observe a hang. I used sysfs, and appeared able to validate that the
> power collapse state was being used successfully.

Interesting that the shallower sleep state was causing problems.
Usually, it is the deeper states that cause problems. So you plan to
override the idle states table in the board-specific DT?

Why does the platform even rely on DT? Shouldn't we use the ACPI tables instead?

> I'm guessing that something is weird with the laptops, where the CPUs
> can go into retention, but not come out, thus causing issues.
>
> I'll post a patch to fix up the laptops. Thanks for all the help.

2019-10-04 08:04:09