* [PATCH RFT v2 1/3] arm64: dts: qcom: sc8280xp: Add lower cluster idle states
2023-12-20 22:12 [PATCH RFT v2 0/3] Fix up SC8280XP idle states Konrad Dybcio
@ 2023-12-20 22:12 ` Konrad Dybcio
2023-12-20 22:12 ` [PATCH RFT v2 2/3] arm64: dts: qcom: sc8280xp: Add missing CPU " Konrad Dybcio
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Konrad Dybcio @ 2023-12-20 22:12 UTC (permalink / raw)
To: Andy Gross, Bjorn Andersson, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Johan Hovold
Cc: Marijn Suijten, linux-arm-msm, devicetree, linux-kernel,
Konrad Dybcio, Krzysztof Kozlowski, Konrad Dybcio
Apart from a total LLCC + APSS power collapse, SC8280XP can also put
either the DSU rail (CPU + L3), or VDD_CX in power collapse.
Add support for these lower idle states to allow more flexibility.
Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 20 ++++++++++++++++++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
index febf28356ff8..ca56bbac4353 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
@@ -286,7 +286,23 @@ BIG_CPU_SLEEP_0: cpu-sleep-1-0 {
};
domain-idle-states {
- CLUSTER_SLEEP_0: cluster-sleep-0 {
+ CLUSTER_SLEEP_APSS_OFF: cluster-sleep-0 {
+ compatible = "domain-idle-state";
+ arm,psci-suspend-param = <0x41000044>;
+ entry-latency-us = <2752>;
+ exit-latency-us = <3048>;
+ min-residency-us = <6118>;
+ };
+
+ CLUSTER_SLEEP_CX_OFF: cluster-sleep-1 {
+ compatible = "domain-idle-state";
+ arm,psci-suspend-param = <0x41002344>;
+ entry-latency-us = <3263>;
+ exit-latency-us = <4562>;
+ min-residency-us = <8467>;
+ };
+
+ CLUSTER_SLEEP_AOSS_SLEEP: cluster-sleep-2 {
compatible = "domain-idle-state";
arm,psci-suspend-param = <0x4100c344>;
entry-latency-us = <3263>;
@@ -634,7 +650,7 @@ CPU_PD7: power-domain-cpu7 {
CLUSTER_PD: power-domain-cpu-cluster0 {
#power-domain-cells = <0>;
- domain-idle-states = <&CLUSTER_SLEEP_0>;
+ domain-idle-states = <&CLUSTER_SLEEP_APSS_OFF &CLUSTER_SLEEP_CX_OFF &CLUSTER_SLEEP_AOSS_SLEEP>;
};
};
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH RFT v2 2/3] arm64: dts: qcom: sc8280xp: Add missing CPU idle states
2023-12-20 22:12 [PATCH RFT v2 0/3] Fix up SC8280XP idle states Konrad Dybcio
2023-12-20 22:12 ` [PATCH RFT v2 1/3] arm64: dts: qcom: sc8280xp: Add lower cluster " Konrad Dybcio
@ 2023-12-20 22:12 ` Konrad Dybcio
2023-12-20 22:12 ` [PATCH RFT v2 3/3] arm64: dts: qcom: sc8280xp: Fix up idle state periods Konrad Dybcio
2024-02-14 5:35 ` [PATCH RFT v2 0/3] Fix up SC8280XP idle states Bjorn Andersson
3 siblings, 0 replies; 6+ messages in thread
From: Konrad Dybcio @ 2023-12-20 22:12 UTC (permalink / raw)
To: Andy Gross, Bjorn Andersson, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Johan Hovold
Cc: Marijn Suijten, linux-arm-msm, devicetree, linux-kernel,
Konrad Dybcio, Krzysztof Kozlowski, Konrad Dybcio
Between WFI (C1) and rail power collapse (C4), the Cortex cores on the
SC8280XP can also be shut down on their own (C3).
Describe that missing idle state.
Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 36 ++++++++++++++++++++++++++--------
1 file changed, 28 insertions(+), 8 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
index ca56bbac4353..62f3f33220ff 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
@@ -265,6 +265,16 @@ idle-states {
entry-method = "psci";
LITTLE_CPU_SLEEP_0: cpu-sleep-0-0 {
+ compatible = "arm,idle-state";
+ idle-state-name = "little-power-collapse";
+ arm,psci-suspend-param = <0x40000003>;
+ entry-latency-us = <549>;
+ exit-latency-us = <901>;
+ min-residency-us = <1774>;
+ local-timer-stop;
+ };
+
+ LITTLE_CPU_SLEEP_1: cpu-sleep-0-1 {
compatible = "arm,idle-state";
idle-state-name = "little-rail-power-collapse";
arm,psci-suspend-param = <0x40000004>;
@@ -275,6 +285,16 @@ LITTLE_CPU_SLEEP_0: cpu-sleep-0-0 {
};
BIG_CPU_SLEEP_0: cpu-sleep-1-0 {
+ compatible = "arm,idle-state";
+ idle-state-name = "big-power-collapse";
+ arm,psci-suspend-param = <0x40000003>;
+ entry-latency-us = <523>;
+ exit-latency-us = <1244>;
+ min-residency-us = <2207>;
+ local-timer-stop;
+ };
+
+ BIG_CPU_SLEEP_1: cpu-sleep-1-1 {
compatible = "arm,idle-state";
idle-state-name = "big-rail-power-collapse";
arm,psci-suspend-param = <0x40000004>;
@@ -603,49 +623,49 @@ psci {
CPU_PD0: power-domain-cpu0 {
#power-domain-cells = <0>;
power-domains = <&CLUSTER_PD>;
- domain-idle-states = <&LITTLE_CPU_SLEEP_0>;
+ domain-idle-states = <&LITTLE_CPU_SLEEP_0 &LITTLE_CPU_SLEEP_1>;
};
CPU_PD1: power-domain-cpu1 {
#power-domain-cells = <0>;
power-domains = <&CLUSTER_PD>;
- domain-idle-states = <&LITTLE_CPU_SLEEP_0>;
+ domain-idle-states = <&LITTLE_CPU_SLEEP_0 &LITTLE_CPU_SLEEP_1>;
};
CPU_PD2: power-domain-cpu2 {
#power-domain-cells = <0>;
power-domains = <&CLUSTER_PD>;
- domain-idle-states = <&LITTLE_CPU_SLEEP_0>;
+ domain-idle-states = <&LITTLE_CPU_SLEEP_0 &LITTLE_CPU_SLEEP_1>;
};
CPU_PD3: power-domain-cpu3 {
#power-domain-cells = <0>;
power-domains = <&CLUSTER_PD>;
- domain-idle-states = <&LITTLE_CPU_SLEEP_0>;
+ domain-idle-states = <&LITTLE_CPU_SLEEP_0 &LITTLE_CPU_SLEEP_1>;
};
CPU_PD4: power-domain-cpu4 {
#power-domain-cells = <0>;
power-domains = <&CLUSTER_PD>;
- domain-idle-states = <&BIG_CPU_SLEEP_0>;
+ domain-idle-states = <&BIG_CPU_SLEEP_0 &BIG_CPU_SLEEP_1>;
};
CPU_PD5: power-domain-cpu5 {
#power-domain-cells = <0>;
power-domains = <&CLUSTER_PD>;
- domain-idle-states = <&BIG_CPU_SLEEP_0>;
+ domain-idle-states = <&BIG_CPU_SLEEP_0 &BIG_CPU_SLEEP_1>;
};
CPU_PD6: power-domain-cpu6 {
#power-domain-cells = <0>;
power-domains = <&CLUSTER_PD>;
- domain-idle-states = <&BIG_CPU_SLEEP_0>;
+ domain-idle-states = <&BIG_CPU_SLEEP_0 &BIG_CPU_SLEEP_1>;
};
CPU_PD7: power-domain-cpu7 {
#power-domain-cells = <0>;
power-domains = <&CLUSTER_PD>;
- domain-idle-states = <&BIG_CPU_SLEEP_0>;
+ domain-idle-states = <&BIG_CPU_SLEEP_0 &BIG_CPU_SLEEP_1>;
};
CLUSTER_PD: power-domain-cpu-cluster0 {
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH RFT v2 3/3] arm64: dts: qcom: sc8280xp: Fix up idle state periods
2023-12-20 22:12 [PATCH RFT v2 0/3] Fix up SC8280XP idle states Konrad Dybcio
2023-12-20 22:12 ` [PATCH RFT v2 1/3] arm64: dts: qcom: sc8280xp: Add lower cluster " Konrad Dybcio
2023-12-20 22:12 ` [PATCH RFT v2 2/3] arm64: dts: qcom: sc8280xp: Add missing CPU " Konrad Dybcio
@ 2023-12-20 22:12 ` Konrad Dybcio
2024-02-14 5:35 ` [PATCH RFT v2 0/3] Fix up SC8280XP idle states Bjorn Andersson
3 siblings, 0 replies; 6+ messages in thread
From: Konrad Dybcio @ 2023-12-20 22:12 UTC (permalink / raw)
To: Andy Gross, Bjorn Andersson, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Johan Hovold
Cc: Marijn Suijten, linux-arm-msm, devicetree, linux-kernel,
Konrad Dybcio, Krzysztof Kozlowski, Konrad Dybcio
Some of the values were wrong, which may have painted a wrong picture
to the scheduler. Fix it.
Fixes: 152d1faf1e2f ("arm64: dts: qcom: add SC8280XP platform")
Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
index 62f3f33220ff..7625db4512c7 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
@@ -278,9 +278,9 @@ LITTLE_CPU_SLEEP_1: cpu-sleep-0-1 {
compatible = "arm,idle-state";
idle-state-name = "little-rail-power-collapse";
arm,psci-suspend-param = <0x40000004>;
- entry-latency-us = <355>;
- exit-latency-us = <909>;
- min-residency-us = <3934>;
+ entry-latency-us = <702>;
+ exit-latency-us = <915>;
+ min-residency-us = <4001>;
local-timer-stop;
};
@@ -298,9 +298,9 @@ BIG_CPU_SLEEP_1: cpu-sleep-1-1 {
compatible = "arm,idle-state";
idle-state-name = "big-rail-power-collapse";
arm,psci-suspend-param = <0x40000004>;
- entry-latency-us = <241>;
- exit-latency-us = <1461>;
- min-residency-us = <4488>;
+ entry-latency-us = <526>;
+ exit-latency-us = <1854>;
+ min-residency-us = <5555>;
local-timer-stop;
};
};
@@ -325,9 +325,9 @@ CLUSTER_SLEEP_CX_OFF: cluster-sleep-1 {
CLUSTER_SLEEP_AOSS_SLEEP: cluster-sleep-2 {
compatible = "domain-idle-state";
arm,psci-suspend-param = <0x4100c344>;
- entry-latency-us = <3263>;
+ entry-latency-us = <3638>;
exit-latency-us = <6562>;
- min-residency-us = <9987>;
+ min-residency-us = <9826>;
};
};
};
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH RFT v2 0/3] Fix up SC8280XP idle states
2023-12-20 22:12 [PATCH RFT v2 0/3] Fix up SC8280XP idle states Konrad Dybcio
` (2 preceding siblings ...)
2023-12-20 22:12 ` [PATCH RFT v2 3/3] arm64: dts: qcom: sc8280xp: Fix up idle state periods Konrad Dybcio
@ 2024-02-14 5:35 ` Bjorn Andersson
2024-02-15 7:38 ` Johan Hovold
3 siblings, 1 reply; 6+ messages in thread
From: Bjorn Andersson @ 2024-02-14 5:35 UTC (permalink / raw)
To: Konrad Dybcio
Cc: Andy Gross, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Johan Hovold, Marijn Suijten, linux-arm-msm, devicetree,
linux-kernel, Konrad Dybcio, Krzysztof Kozlowski
On Wed, Dec 20, 2023 at 11:12:53PM +0100, Konrad Dybcio wrote:
> Comparing the data available in the downstream sources with what's there
> upstream, it was easy to spot some differences. This series aligns what
> we have upstream with what is there on the vendor kernel.
>
> The big asterisk there is that the downstream sources for SC8280XP can't
> always be trusted. A simple test shows that the lower idle states that
> were previously missing are implemented in the firmware (Linux reports no
> errors and enters them).
>
> HOWEVER
>
> The only cluster idle state that's been present until now (the deepest
> one) is now barely used if at all, as the scheduler seems to deem it
> inefficient or so.
>
> Hence, a request for testing and comments, especially from those who
> use the X13s daily or have reliable setup to measure the power usage.
>
> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
What did we conclude on this one? Does the extra state make sense?
The last patch looks useful...
Regards,
Bjorn
> ---
> Changes in v2:
> - Rename the idle states
> - Drop RFC, confirmed with Qualcomm
> - Rebase
> - Link to v1: https://lore.kernel.org/r/20230619-topic-sc8280xp-idle-v1-0-35a8b98451d0@linaro.org
>
> ---
> Konrad Dybcio (3):
> arm64: dts: qcom: sc8280xp: Add lower cluster idle states
> arm64: dts: qcom: sc8280xp: Add missing CPU idle states
> arm64: dts: qcom: sc8280xp: Fix up idle state periods
>
> arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 72 +++++++++++++++++++++++++---------
> 1 file changed, 54 insertions(+), 18 deletions(-)
> ---
> base-commit: 20d857259d7d10cd0d5e8b60608455986167cfad
> change-id: 20230619-topic-sc8280xp-idle-00fc007234c8
>
> Best regards,
> --
> Konrad Dybcio <konrad.dybcio@linaro.org>
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH RFT v2 0/3] Fix up SC8280XP idle states
2024-02-14 5:35 ` [PATCH RFT v2 0/3] Fix up SC8280XP idle states Bjorn Andersson
@ 2024-02-15 7:38 ` Johan Hovold
0 siblings, 0 replies; 6+ messages in thread
From: Johan Hovold @ 2024-02-15 7:38 UTC (permalink / raw)
To: Bjorn Andersson
Cc: Konrad Dybcio, Andy Gross, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Johan Hovold, Marijn Suijten, linux-arm-msm,
devicetree, linux-kernel, Konrad Dybcio, Krzysztof Kozlowski
On Tue, Feb 13, 2024 at 11:35:06PM -0600, Bjorn Andersson wrote:
> On Wed, Dec 20, 2023 at 11:12:53PM +0100, Konrad Dybcio wrote:
> > Comparing the data available in the downstream sources with what's there
> > upstream, it was easy to spot some differences. This series aligns what
> > we have upstream with what is there on the vendor kernel.
> >
> > The big asterisk there is that the downstream sources for SC8280XP can't
> > always be trusted. A simple test shows that the lower idle states that
> > were previously missing are implemented in the firmware (Linux reports no
> > errors and enters them).
> >
> > HOWEVER
> >
> > The only cluster idle state that's been present until now (the deepest
> > one) is now barely used if at all, as the scheduler seems to deem it
> > inefficient or so.
> >
> > Hence, a request for testing and comments, especially from those who
> > use the X13s daily or have reliable setup to measure the power usage.
> >
> > Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
>
> What did we conclude on this one? Does the extra state make sense?
> The last patch looks useful...
I asked Konrad a while back to provide some performance numbers to
accompany this change.
I think he said that this series made no difference in either direction,
but IIUC that only after a really quick attempt at evaluating the
impact during a meeting we had.
Johan
^ permalink raw reply [flat|nested] 6+ messages in thread