All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] arm64: dts: sdm845: Add PSCI cpuidle low power states
@ 2018-10-30 17:23 Raju P.L.S.S.S.N
  2018-10-31 23:57 ` Doug Anderson
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Raju P.L.S.S.S.N @ 2018-10-30 17:23 UTC (permalink / raw)
  To: andy.gross, david.brown, linux-arm-msm, linux-soc
  Cc: rnayak, bjorn.andersson, linux-kernel, linux-pm, devicetree,
	sboyd, evgreen, dianders, mka, ilina, Raju P.L.S.S.S.N

Add device bindings for cpuidle states for cpu devices.

Cc: <devicetree@vger.kernel.org>
Signed-off-by: Raju P.L.S.S.S.N <rplsssn@codeaurora.org>
---
Changes in v2
 - Address comments from Doug
---
---
 arch/arm64/boot/dts/qcom/sdm845.dtsi | 62 ++++++++++++++++++++++++++++++++++++
 1 file changed, 62 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index 0c9a2aa..3a8381e 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -96,6 +96,7 @@
 			reg = <0x0 0x0>;
 			enable-method = "psci";
 			next-level-cache = <&L2_0>;
+			cpu-idle-states = <&C0_CPU_PD &C0_CPU_RPD &CLUSTER_PD>;
 			L2_0: l2-cache {
 				compatible = "cache";
 				next-level-cache = <&L3_0>;
@@ -111,6 +112,7 @@
 			reg = <0x0 0x100>;
 			enable-method = "psci";
 			next-level-cache = <&L2_100>;
+			cpu-idle-states = <&C0_CPU_PD &C0_CPU_RPD &CLUSTER_PD>;
 			L2_100: l2-cache {
 				compatible = "cache";
 				next-level-cache = <&L3_0>;
@@ -123,6 +125,7 @@
 			reg = <0x0 0x200>;
 			enable-method = "psci";
 			next-level-cache = <&L2_200>;
+			cpu-idle-states = <&C0_CPU_PD &C0_CPU_RPD &CLUSTER_PD>;
 			L2_200: l2-cache {
 				compatible = "cache";
 				next-level-cache = <&L3_0>;
@@ -135,6 +138,7 @@
 			reg = <0x0 0x300>;
 			enable-method = "psci";
 			next-level-cache = <&L2_300>;
+			cpu-idle-states = <&C0_CPU_PD &C0_CPU_RPD &CLUSTER_PD>;
 			L2_300: l2-cache {
 				compatible = "cache";
 				next-level-cache = <&L3_0>;
@@ -147,6 +151,7 @@
 			reg = <0x0 0x400>;
 			enable-method = "psci";
 			next-level-cache = <&L2_400>;
+			cpu-idle-states = <&C4_CPU_PD &C4_CPU_RPD &CLUSTER_PD>;
 			L2_400: l2-cache {
 				compatible = "cache";
 				next-level-cache = <&L3_0>;
@@ -159,6 +164,7 @@
 			reg = <0x0 0x500>;
 			enable-method = "psci";
 			next-level-cache = <&L2_500>;
+			cpu-idle-states = <&C4_CPU_PD &C4_CPU_RPD &CLUSTER_PD>;
 			L2_500: l2-cache {
 				compatible = "cache";
 				next-level-cache = <&L3_0>;
@@ -171,6 +177,7 @@
 			reg = <0x0 0x600>;
 			enable-method = "psci";
 			next-level-cache = <&L2_600>;
+			cpu-idle-states = <&C4_CPU_PD &C4_CPU_RPD &CLUSTER_PD>;
 			L2_600: l2-cache {
 				compatible = "cache";
 				next-level-cache = <&L3_0>;
@@ -183,11 +190,66 @@
 			reg = <0x0 0x700>;
 			enable-method = "psci";
 			next-level-cache = <&L2_700>;
+			cpu-idle-states = <&C4_CPU_PD &C4_CPU_RPD &CLUSTER_PD>;
 			L2_700: l2-cache {
 				compatible = "cache";
 				next-level-cache = <&L3_0>;
 			};
 		};
+
+		idle-states {
+			entry-method = "psci";
+
+			C0_CPU_PD: c0-power-down {
+				compatible = "arm,idle-state";
+				arm,psci-suspend-param = <0x40000003>;
+				entry-latency-us = <350>;
+				exit-latency-us = <461>;
+				min-residency-us = <1890>;
+				local-timer-stop;
+				idle-state-name = "power-down";
+			};
+
+			C0_CPU_RPD: c0-rail-power-down {
+				compatible = "arm,idle-state";
+				arm,psci-suspend-param = <0x40000004>;
+				entry-latency-us = <360>;
+				exit-latency-us = <531>;
+				min-residency-us = <3934>;
+				local-timer-stop;
+				idle-state-name = "rail-power-down";
+			};
+
+			C4_CPU_PD: c4-power-down {
+				compatible = "arm,idle-state";
+				arm,psci-suspend-param = <0x40000003>;
+				entry-latency-us = <264>;
+				exit-latency-us = <621>;
+				min-residency-us = <952>;
+				local-timer-stop;
+				idle-state-name = "power-down";
+			};
+
+			C4_CPU_RPD: c4-rail-power-down {
+				compatible = "arm,idle-state";
+				arm,psci-suspend-param = <0x40000004>;
+				entry-latency-us = <702>;
+				exit-latency-us = <1061>;
+				min-residency-us = <4488>;
+				local-timer-stop;
+				idle-state-name = "rail-power-down";
+			};
+
+			CLUSTER_PD: cluster-power-down {
+				compatible = "arm,idle-state";
+				arm,psci-suspend-param = <0x400000F4>;
+				entry-latency-us = <3263>;
+				exit-latency-us = <6562>;
+				min-residency-us = <9987>;
+				local-timer-stop;
+				idle-state-name = "cluster-power-down";
+			};
+		};
 	};
 
 	pmu {
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of the Code Aurora Forum, hosted by The Linux Foundation.

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] arm64: dts: sdm845: Add PSCI cpuidle low power states
  2018-10-30 17:23 [PATCH v2] arm64: dts: sdm845: Add PSCI cpuidle low power states Raju P.L.S.S.S.N
@ 2018-10-31 23:57 ` Doug Anderson
  2018-11-01 17:21 ` Evan Green
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 10+ messages in thread
From: Doug Anderson @ 2018-10-31 23:57 UTC (permalink / raw)
  To: Raju P L S S S N
  Cc: Andy Gross, David Brown, linux-arm-msm,
	open list:ARM/QUALCOMM SUPPORT, Rajendra Nayak, Bjorn Andersson,
	LKML, linux-pm, devicetree, Stephen Boyd, Evan Green,
	Matthias Kaehlcke, Lina Iyer

Hi,

On Tue, Oct 30, 2018 at 10:23 AM Raju P.L.S.S.S.N
<rplsssn@codeaurora.org> wrote:
>
> Add device bindings for cpuidle states for cpu devices.
>
> Cc: <devicetree@vger.kernel.org>
> Signed-off-by: Raju P.L.S.S.S.N <rplsssn@codeaurora.org>
> ---
> Changes in v2
>  - Address comments from Doug
> ---
> ---
>  arch/arm64/boot/dts/qcom/sdm845.dtsi | 62 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 62 insertions(+)

I haven't been following all the discussion about idle states closely
enough to know if the contents of this patch is correct.  Thus I won't
provide my Reviewed-by since I expect Evan and/or Lina will provide
theirs again like they did on v1.  However, I'll mention that the nits
I had regarding v1 have been fixed so I'm happy with this patch
landing if others are.  ;-)

-Doug

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] arm64: dts: sdm845: Add PSCI cpuidle low power states
  2018-10-30 17:23 [PATCH v2] arm64: dts: sdm845: Add PSCI cpuidle low power states Raju P.L.S.S.S.N
  2018-10-31 23:57 ` Doug Anderson
@ 2018-11-01 17:21 ` Evan Green
  2018-11-30 15:41 ` Lina Iyer
  2019-05-07 22:20   ` Amit Kucheria
  3 siblings, 0 replies; 10+ messages in thread
From: Evan Green @ 2018-11-01 17:21 UTC (permalink / raw)
  To: rplsssn
  Cc: Andy Gross, David Brown, linux-arm-msm, linux-soc,
	Rajendra Nayak, Bjorn Andersson, linux-kernel, linux-pm,
	devicetree, sboyd, Doug Anderson, mka, Lina Iyer

On Tue, Oct 30, 2018 at 10:23 AM Raju P.L.S.S.S.N
<rplsssn@codeaurora.org> wrote:
>
> Add device bindings for cpuidle states for cpu devices.
>
> Cc: <devicetree@vger.kernel.org>
> Signed-off-by: Raju P.L.S.S.S.N <rplsssn@codeaurora.org>
> ---
> Changes in v2
>  - Address comments from Doug
> ---
> ---
>  arch/arm64/boot/dts/qcom/sdm845.dtsi | 62 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 62 insertions(+)
>

Reviewed-by: Evan Green <evgreen@chromium.org>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] arm64: dts: sdm845: Add PSCI cpuidle low power states
  2018-10-30 17:23 [PATCH v2] arm64: dts: sdm845: Add PSCI cpuidle low power states Raju P.L.S.S.S.N
  2018-10-31 23:57 ` Doug Anderson
  2018-11-01 17:21 ` Evan Green
@ 2018-11-30 15:41 ` Lina Iyer
  2018-12-20 21:40   ` Doug Anderson
  2019-05-07 22:20   ` Amit Kucheria
  3 siblings, 1 reply; 10+ messages in thread
From: Lina Iyer @ 2018-11-30 15:41 UTC (permalink / raw)
  To: Raju P.L.S.S.S.N
  Cc: andy.gross, david.brown, linux-arm-msm, linux-soc, rnayak,
	bjorn.andersson, linux-kernel, linux-pm, devicetree, sboyd,
	evgreen, dianders, mka

On Tue, Oct 30 2018 at 11:23 -0600, Raju P.L.S.S.S.N wrote:
>Add device bindings for cpuidle states for cpu devices.
>
>Cc: <devicetree@vger.kernel.org>
>Signed-off-by: Raju P.L.S.S.S.N <rplsssn@codeaurora.org>
>---
>Changes in v2
> - Address comments from Doug
>---
>---
> arch/arm64/boot/dts/qcom/sdm845.dtsi | 62 ++++++++++++++++++++++++++++++++++++
> 1 file changed, 62 insertions(+)
>
>diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
>index 0c9a2aa..3a8381e 100644
>--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
>+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
>@@ -96,6 +96,7 @@
> 			reg = <0x0 0x0>;
> 			enable-method = "psci";
> 			next-level-cache = <&L2_0>;
>+			cpu-idle-states = <&C0_CPU_PD &C0_CPU_RPD &CLUSTER_PD>;
I think it might be better to use 
<&C0_CPU_PD>, <&C0_CPU_RPD>, <&CLUSTER_PD>
> 			L2_0: l2-cache {
> 				compatible = "cache";
> 				next-level-cache = <&L3_0>;
>@@ -111,6 +112,7 @@
> 			reg = <0x0 0x100>;
> 			enable-method = "psci";
> 			next-level-cache = <&L2_100>;
>+			cpu-idle-states = <&C0_CPU_PD &C0_CPU_RPD &CLUSTER_PD>;
> 			L2_100: l2-cache {
> 				compatible = "cache";
> 				next-level-cache = <&L3_0>;
>@@ -123,6 +125,7 @@
> 			reg = <0x0 0x200>;
> 			enable-method = "psci";
> 			next-level-cache = <&L2_200>;
>+			cpu-idle-states = <&C0_CPU_PD &C0_CPU_RPD &CLUSTER_PD>;
> 			L2_200: l2-cache {
> 				compatible = "cache";
> 				next-level-cache = <&L3_0>;
>@@ -135,6 +138,7 @@
> 			reg = <0x0 0x300>;
> 			enable-method = "psci";
> 			next-level-cache = <&L2_300>;
>+			cpu-idle-states = <&C0_CPU_PD &C0_CPU_RPD &CLUSTER_PD>;
> 			L2_300: l2-cache {
> 				compatible = "cache";
> 				next-level-cache = <&L3_0>;
>@@ -147,6 +151,7 @@
> 			reg = <0x0 0x400>;
> 			enable-method = "psci";
> 			next-level-cache = <&L2_400>;
>+			cpu-idle-states = <&C4_CPU_PD &C4_CPU_RPD &CLUSTER_PD>;
> 			L2_400: l2-cache {
> 				compatible = "cache";
> 				next-level-cache = <&L3_0>;
>@@ -159,6 +164,7 @@
> 			reg = <0x0 0x500>;
> 			enable-method = "psci";
> 			next-level-cache = <&L2_500>;
>+			cpu-idle-states = <&C4_CPU_PD &C4_CPU_RPD &CLUSTER_PD>;
> 			L2_500: l2-cache {
> 				compatible = "cache";
> 				next-level-cache = <&L3_0>;
>@@ -171,6 +177,7 @@
> 			reg = <0x0 0x600>;
> 			enable-method = "psci";
> 			next-level-cache = <&L2_600>;
>+			cpu-idle-states = <&C4_CPU_PD &C4_CPU_RPD &CLUSTER_PD>;
> 			L2_600: l2-cache {
> 				compatible = "cache";
> 				next-level-cache = <&L3_0>;
>@@ -183,11 +190,66 @@
> 			reg = <0x0 0x700>;
> 			enable-method = "psci";
> 			next-level-cache = <&L2_700>;
>+			cpu-idle-states = <&C4_CPU_PD &C4_CPU_RPD &CLUSTER_PD>;
> 			L2_700: l2-cache {
> 				compatible = "cache";
> 				next-level-cache = <&L3_0>;
> 			};
> 		};
>+
>+		idle-states {
>+			entry-method = "psci";
>+
>+			C0_CPU_PD: c0-power-down {
>+				compatible = "arm,idle-state";
>+				arm,psci-suspend-param = <0x40000003>;
>+				entry-latency-us = <350>;
>+				exit-latency-us = <461>;
>+				min-residency-us = <1890>;
>+				local-timer-stop;
>+				idle-state-name = "power-down";
>+			};
>+
>+			C0_CPU_RPD: c0-rail-power-down {
>+				compatible = "arm,idle-state";
>+				arm,psci-suspend-param = <0x40000004>;
>+				entry-latency-us = <360>;
>+				exit-latency-us = <531>;
>+				min-residency-us = <3934>;
>+				local-timer-stop;
>+				idle-state-name = "rail-power-down";
>+			};
>+
>+			C4_CPU_PD: c4-power-down {
>+				compatible = "arm,idle-state";
>+				arm,psci-suspend-param = <0x40000003>;
>+				entry-latency-us = <264>;
>+				exit-latency-us = <621>;
>+				min-residency-us = <952>;
>+				local-timer-stop;
>+				idle-state-name = "power-down";
>+			};
>+
>+			C4_CPU_RPD: c4-rail-power-down {
>+				compatible = "arm,idle-state";
>+				arm,psci-suspend-param = <0x40000004>;
>+				entry-latency-us = <702>;
>+				exit-latency-us = <1061>;
>+				min-residency-us = <4488>;
>+				local-timer-stop;
>+				idle-state-name = "rail-power-down";
>+			};
>+
>+			CLUSTER_PD: cluster-power-down {
>+				compatible = "arm,idle-state";
>+				arm,psci-suspend-param = <0x400000F4>;
>+				entry-latency-us = <3263>;
>+				exit-latency-us = <6562>;
>+				min-residency-us = <9987>;
>+				local-timer-stop;
>+				idle-state-name = "cluster-power-down";
>+			};
>+		};
> 	};
>
> 	pmu {
>-- 
>QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
>of the Code Aurora Forum, hosted by The Linux Foundation.
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] arm64: dts: sdm845: Add PSCI cpuidle low power states
  2018-11-30 15:41 ` Lina Iyer
@ 2018-12-20 21:40   ` Doug Anderson
  2018-12-20 22:55     ` Lina Iyer
  0 siblings, 1 reply; 10+ messages in thread
From: Doug Anderson @ 2018-12-20 21:40 UTC (permalink / raw)
  To: Lina Iyer
  Cc: Raju P.L.S.S.S.N, Andy Gross, David Brown, linux-arm-msm,
	open list:ARM/QUALCOMM SUPPORT, Rajendra Nayak, Bjorn Andersson,
	LKML, linux-pm, devicetree, Stephen Boyd, Evan Green,
	Matthias Kaehlcke

Hi,

On Fri, Nov 30, 2018 at 7:41 AM Lina Iyer <ilina@codeaurora.org> wrote:
>
> On Tue, Oct 30 2018 at 11:23 -0600, Raju P.L.S.S.S.N wrote:
> >Add device bindings for cpuidle states for cpu devices.
> >
> >Cc: <devicetree@vger.kernel.org>
> >Signed-off-by: Raju P.L.S.S.S.N <rplsssn@codeaurora.org>
> >---
> >Changes in v2
> > - Address comments from Doug
> >---
> >---
> > arch/arm64/boot/dts/qcom/sdm845.dtsi | 62 ++++++++++++++++++++++++++++++++++++
> > 1 file changed, 62 insertions(+)
> >
> >diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> >index 0c9a2aa..3a8381e 100644
> >--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> >+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> >@@ -96,6 +96,7 @@
> >                       reg = <0x0 0x0>;
> >                       enable-method = "psci";
> >                       next-level-cache = <&L2_0>;
> >+                      cpu-idle-states = <&C0_CPU_PD &C0_CPU_RPD &CLUSTER_PD>;
> I think it might be better to use
> <&C0_CPU_PD>, <&C0_CPU_RPD>, <&CLUSTER_PD>

I disagree.  All existing examples in both bindings and other device
trees use the syntax that Raju uses.  It doesn't matter
functionality-wise, but I'd much rather match everyone else.  We
should land what Raju has posted and if someone feels super strongly
that the examples in the bindings are wrong then they should attempt
to convince Rob Herring to Ack a change to the examples in the
bindings.  Doesn't seem like a good use of time to me, though.

-Doug

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] arm64: dts: sdm845: Add PSCI cpuidle low power states
  2018-12-20 21:40   ` Doug Anderson
@ 2018-12-20 22:55     ` Lina Iyer
  0 siblings, 0 replies; 10+ messages in thread
From: Lina Iyer @ 2018-12-20 22:55 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Raju P.L.S.S.S.N, Andy Gross, David Brown, linux-arm-msm,
	open list:ARM/QUALCOMM SUPPORT, Rajendra Nayak, Bjorn Andersson,
	LKML, linux-pm, devicetree, Stephen Boyd, Evan Green,
	Matthias Kaehlcke

On Thu, Dec 20 2018 at 14:42 -0700, Doug Anderson wrote:
>Hi,
>
>On Fri, Nov 30, 2018 at 7:41 AM Lina Iyer <ilina@codeaurora.org> wrote:
>>
>> On Tue, Oct 30 2018 at 11:23 -0600, Raju P.L.S.S.S.N wrote:
>> >Add device bindings for cpuidle states for cpu devices.
>> >
>> >Cc: <devicetree@vger.kernel.org>
>> >Signed-off-by: Raju P.L.S.S.S.N <rplsssn@codeaurora.org>
>> >---
>> >Changes in v2
>> > - Address comments from Doug
>> >---
>> >---
>> > arch/arm64/boot/dts/qcom/sdm845.dtsi | 62 ++++++++++++++++++++++++++++++++++++
>> > 1 file changed, 62 insertions(+)
>> >
>> >diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
>> >index 0c9a2aa..3a8381e 100644
>> >--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
>> >+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
>> >@@ -96,6 +96,7 @@
>> >                       reg = <0x0 0x0>;
>> >                       enable-method = "psci";
>> >                       next-level-cache = <&L2_0>;
>> >+                      cpu-idle-states = <&C0_CPU_PD &C0_CPU_RPD &CLUSTER_PD>;
>> I think it might be better to use
>> <&C0_CPU_PD>, <&C0_CPU_RPD>, <&CLUSTER_PD>
>
>I disagree.  All existing examples in both bindings and other device
>trees use the syntax that Raju uses.  It doesn't matter
>functionality-wise, but I'd much rather match everyone else.  We
>should land what Raju has posted and if someone feels super strongly
>that the examples in the bindings are wrong then they should attempt
>to convince Rob Herring to Ack a change to the examples in the
>bindings.  Doesn't seem like a good use of time to me, though.
>
No strong preference from my end.

-- Lina

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] arm64: dts: sdm845: Add PSCI cpuidle low power states
@ 2019-05-07 22:20   ` Amit Kucheria
  0 siblings, 0 replies; 10+ messages in thread
From: Amit Kucheria @ 2019-05-07 22:20 UTC (permalink / raw)
  To: Raju P.L.S.S.S.N
  Cc: Andy Gross, David Brown, linux-arm-msm,
	open list:ARM/QUALCOMM SUPPORT, Rajendra Nayak, Bjorn Andersson,
	LKML, Linux PM list,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Stephen Boyd, evgreen, Douglas Anderson, Matthias Kaehlcke,
	ilina

On Tue, Oct 30, 2018 at 10:53 PM Raju P.L.S.S.S.N
<rplsssn@codeaurora.org> wrote:
>
> Add device bindings for cpuidle states for cpu devices.

Raju: Did this patch fall through the cracks? It would be nice to land
this while Lina works on setting up the infrastructure to do
hierarchical power domains.

> Cc: <devicetree@vger.kernel.org>
> Signed-off-by: Raju P.L.S.S.S.N <rplsssn@codeaurora.org>
> ---
> Changes in v2
>  - Address comments from Doug
> ---
> ---
>  arch/arm64/boot/dts/qcom/sdm845.dtsi | 62 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 62 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> index 0c9a2aa..3a8381e 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> @@ -96,6 +96,7 @@
>                         reg = <0x0 0x0>;
>                         enable-method = "psci";
>                         next-level-cache = <&L2_0>;
> +                       cpu-idle-states = <&C0_CPU_PD &C0_CPU_RPD &CLUSTER_PD>;

May I suggest using more generic names here instead of C0_CPU_PD and
move the QC-specific description to the idle-state-name property? C0
and C4 isn't easy to understand at a glance. Neither is PD and RPD.

Something like big_cpu_retention, big_cpu_sleep, little_cpu_retention,
little_cpu_sleep, cluster_sleep?
Or big_cpu_idle_0, big_cpu_idle_1, big_cpu_idle_2 for states with
increasing breakeven times.

I've commented below on what it might look like.

>                         L2_0: l2-cache {
>                                 compatible = "cache";
>                                 next-level-cache = <&L3_0>;
> @@ -111,6 +112,7 @@
>                         reg = <0x0 0x100>;
>                         enable-method = "psci";
>                         next-level-cache = <&L2_100>;
> +                       cpu-idle-states = <&C0_CPU_PD &C0_CPU_RPD &CLUSTER_PD>;
>                         L2_100: l2-cache {
>                                 compatible = "cache";
>                                 next-level-cache = <&L3_0>;
> @@ -123,6 +125,7 @@
>                         reg = <0x0 0x200>;
>                         enable-method = "psci";
>                         next-level-cache = <&L2_200>;
> +                       cpu-idle-states = <&C0_CPU_PD &C0_CPU_RPD &CLUSTER_PD>;
>                         L2_200: l2-cache {
>                                 compatible = "cache";
>                                 next-level-cache = <&L3_0>;
> @@ -135,6 +138,7 @@
>                         reg = <0x0 0x300>;
>                         enable-method = "psci";
>                         next-level-cache = <&L2_300>;
> +                       cpu-idle-states = <&C0_CPU_PD &C0_CPU_RPD &CLUSTER_PD>;
>                         L2_300: l2-cache {
>                                 compatible = "cache";
>                                 next-level-cache = <&L3_0>;
> @@ -147,6 +151,7 @@
>                         reg = <0x0 0x400>;
>                         enable-method = "psci";
>                         next-level-cache = <&L2_400>;
> +                       cpu-idle-states = <&C4_CPU_PD &C4_CPU_RPD &CLUSTER_PD>;
>                         L2_400: l2-cache {
>                                 compatible = "cache";
>                                 next-level-cache = <&L3_0>;
> @@ -159,6 +164,7 @@
>                         reg = <0x0 0x500>;
>                         enable-method = "psci";
>                         next-level-cache = <&L2_500>;
> +                       cpu-idle-states = <&C4_CPU_PD &C4_CPU_RPD &CLUSTER_PD>;
>                         L2_500: l2-cache {
>                                 compatible = "cache";
>                                 next-level-cache = <&L3_0>;
> @@ -171,6 +177,7 @@
>                         reg = <0x0 0x600>;
>                         enable-method = "psci";
>                         next-level-cache = <&L2_600>;
> +                       cpu-idle-states = <&C4_CPU_PD &C4_CPU_RPD &CLUSTER_PD>;
>                         L2_600: l2-cache {
>                                 compatible = "cache";
>                                 next-level-cache = <&L3_0>;
> @@ -183,11 +190,66 @@
>                         reg = <0x0 0x700>;
>                         enable-method = "psci";
>                         next-level-cache = <&L2_700>;
> +                       cpu-idle-states = <&C4_CPU_PD &C4_CPU_RPD &CLUSTER_PD>;
>                         L2_700: l2-cache {
>                                 compatible = "cache";
>                                 next-level-cache = <&L3_0>;
>                         };
>                 };
> +
> +               idle-states {
> +                       entry-method = "psci";
> +
> +                       C0_CPU_PD: c0-power-down {

big_cpu_retention: big-cpu-retention

> +                               compatible = "arm,idle-state";
> +                               arm,psci-suspend-param = <0x40000003>;
> +                               entry-latency-us = <350>;
> +                               exit-latency-us = <461>;
> +                               min-residency-us = <1890>;
> +                               local-timer-stop;
> +                               idle-state-name = "power-down";

"big-cpu-power-down"

> +                       };
> +
> +                       C0_CPU_RPD: c0-rail-power-down {

big_cpu_sleep: big-cpu-sleep

> +                               compatible = "arm,idle-state";
> +                               arm,psci-suspend-param = <0x40000004>;
> +                               entry-latency-us = <360>;
> +                               exit-latency-us = <531>;
> +                               min-residency-us = <3934>;
> +                               local-timer-stop;
> +                               idle-state-name = "rail-power-down";

"big-cpu-rail-power-down"

> +                       };
> +
> +                       C4_CPU_PD: c4-power-down {

little_cpu_retention: little-cpu-retention

> +                               compatible = "arm,idle-state";
> +                               arm,psci-suspend-param = <0x40000003>;
> +                               entry-latency-us = <264>;
> +                               exit-latency-us = <621>;
> +                               min-residency-us = <952>;
> +                               local-timer-stop;
> +                               idle-state-name = "power-down";

"little-cpu-power-down" and so and so forth. You get the idea.

> +                       };
> +
> +                       C4_CPU_RPD: c4-rail-power-down {
> +                               compatible = "arm,idle-state";
> +                               arm,psci-suspend-param = <0x40000004>;
> +                               entry-latency-us = <702>;
> +                               exit-latency-us = <1061>;
> +                               min-residency-us = <4488>;
> +                               local-timer-stop;
> +                               idle-state-name = "rail-power-down";
> +                       };
> +
> +                       CLUSTER_PD: cluster-power-down {
> +                               compatible = "arm,idle-state";
> +                               arm,psci-suspend-param = <0x400000F4>;
> +                               entry-latency-us = <3263>;
> +                               exit-latency-us = <6562>;
> +                               min-residency-us = <9987>;
> +                               local-timer-stop;
> +                               idle-state-name = "cluster-power-down";
> +                       };
> +               };
>         };
>
>         pmu {
> --
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
> of the Code Aurora Forum, hosted by The Linux Foundation.
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] arm64: dts: sdm845: Add PSCI cpuidle low power states
@ 2019-05-07 22:20   ` Amit Kucheria
  0 siblings, 0 replies; 10+ messages in thread
From: Amit Kucheria @ 2019-05-07 22:20 UTC (permalink / raw)
  To: Raju P.L.S.S.S.N
  Cc: Andy Gross, David Brown, linux-arm-msm,
	open list:ARM/QUALCOMM SUPPORT, Rajendra Nayak, Bjorn Andersson,
	LKML, Linux PM list,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Stephen Boyd, evgreen, Douglas Anderson, Matthias Kaehlcke,
	ilina

On Tue, Oct 30, 2018 at 10:53 PM Raju P.L.S.S.S.N
<rplsssn@codeaurora.org> wrote:
>
> Add device bindings for cpuidle states for cpu devices.

Raju: Did this patch fall through the cracks? It would be nice to land
this while Lina works on setting up the infrastructure to do
hierarchical power domains.

> Cc: <devicetree@vger.kernel.org>
> Signed-off-by: Raju P.L.S.S.S.N <rplsssn@codeaurora.org>
> ---
> Changes in v2
>  - Address comments from Doug
> ---
> ---
>  arch/arm64/boot/dts/qcom/sdm845.dtsi | 62 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 62 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> index 0c9a2aa..3a8381e 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> @@ -96,6 +96,7 @@
>                         reg = <0x0 0x0>;
>                         enable-method = "psci";
>                         next-level-cache = <&L2_0>;
> +                       cpu-idle-states = <&C0_CPU_PD &C0_CPU_RPD &CLUSTER_PD>;

May I suggest using more generic names here instead of C0_CPU_PD and
move the QC-specific description to the idle-state-name property? C0
and C4 isn't easy to understand at a glance. Neither is PD and RPD.

Something like big_cpu_retention, big_cpu_sleep, little_cpu_retention,
little_cpu_sleep, cluster_sleep?
Or big_cpu_idle_0, big_cpu_idle_1, big_cpu_idle_2 for states with
increasing breakeven times.

I've commented below on what it might look like.

>                         L2_0: l2-cache {
>                                 compatible = "cache";
>                                 next-level-cache = <&L3_0>;
> @@ -111,6 +112,7 @@
>                         reg = <0x0 0x100>;
>                         enable-method = "psci";
>                         next-level-cache = <&L2_100>;
> +                       cpu-idle-states = <&C0_CPU_PD &C0_CPU_RPD &CLUSTER_PD>;
>                         L2_100: l2-cache {
>                                 compatible = "cache";
>                                 next-level-cache = <&L3_0>;
> @@ -123,6 +125,7 @@
>                         reg = <0x0 0x200>;
>                         enable-method = "psci";
>                         next-level-cache = <&L2_200>;
> +                       cpu-idle-states = <&C0_CPU_PD &C0_CPU_RPD &CLUSTER_PD>;
>                         L2_200: l2-cache {
>                                 compatible = "cache";
>                                 next-level-cache = <&L3_0>;
> @@ -135,6 +138,7 @@
>                         reg = <0x0 0x300>;
>                         enable-method = "psci";
>                         next-level-cache = <&L2_300>;
> +                       cpu-idle-states = <&C0_CPU_PD &C0_CPU_RPD &CLUSTER_PD>;
>                         L2_300: l2-cache {
>                                 compatible = "cache";
>                                 next-level-cache = <&L3_0>;
> @@ -147,6 +151,7 @@
>                         reg = <0x0 0x400>;
>                         enable-method = "psci";
>                         next-level-cache = <&L2_400>;
> +                       cpu-idle-states = <&C4_CPU_PD &C4_CPU_RPD &CLUSTER_PD>;
>                         L2_400: l2-cache {
>                                 compatible = "cache";
>                                 next-level-cache = <&L3_0>;
> @@ -159,6 +164,7 @@
>                         reg = <0x0 0x500>;
>                         enable-method = "psci";
>                         next-level-cache = <&L2_500>;
> +                       cpu-idle-states = <&C4_CPU_PD &C4_CPU_RPD &CLUSTER_PD>;
>                         L2_500: l2-cache {
>                                 compatible = "cache";
>                                 next-level-cache = <&L3_0>;
> @@ -171,6 +177,7 @@
>                         reg = <0x0 0x600>;
>                         enable-method = "psci";
>                         next-level-cache = <&L2_600>;
> +                       cpu-idle-states = <&C4_CPU_PD &C4_CPU_RPD &CLUSTER_PD>;
>                         L2_600: l2-cache {
>                                 compatible = "cache";
>                                 next-level-cache = <&L3_0>;
> @@ -183,11 +190,66 @@
>                         reg = <0x0 0x700>;
>                         enable-method = "psci";
>                         next-level-cache = <&L2_700>;
> +                       cpu-idle-states = <&C4_CPU_PD &C4_CPU_RPD &CLUSTER_PD>;
>                         L2_700: l2-cache {
>                                 compatible = "cache";
>                                 next-level-cache = <&L3_0>;
>                         };
>                 };
> +
> +               idle-states {
> +                       entry-method = "psci";
> +
> +                       C0_CPU_PD: c0-power-down {

big_cpu_retention: big-cpu-retention

> +                               compatible = "arm,idle-state";
> +                               arm,psci-suspend-param = <0x40000003>;
> +                               entry-latency-us = <350>;
> +                               exit-latency-us = <461>;
> +                               min-residency-us = <1890>;
> +                               local-timer-stop;
> +                               idle-state-name = "power-down";

"big-cpu-power-down"

> +                       };
> +
> +                       C0_CPU_RPD: c0-rail-power-down {

big_cpu_sleep: big-cpu-sleep

> +                               compatible = "arm,idle-state";
> +                               arm,psci-suspend-param = <0x40000004>;
> +                               entry-latency-us = <360>;
> +                               exit-latency-us = <531>;
> +                               min-residency-us = <3934>;
> +                               local-timer-stop;
> +                               idle-state-name = "rail-power-down";

"big-cpu-rail-power-down"

> +                       };
> +
> +                       C4_CPU_PD: c4-power-down {

little_cpu_retention: little-cpu-retention

> +                               compatible = "arm,idle-state";
> +                               arm,psci-suspend-param = <0x40000003>;
> +                               entry-latency-us = <264>;
> +                               exit-latency-us = <621>;
> +                               min-residency-us = <952>;
> +                               local-timer-stop;
> +                               idle-state-name = "power-down";

"little-cpu-power-down" and so and so forth. You get the idea.

> +                       };
> +
> +                       C4_CPU_RPD: c4-rail-power-down {
> +                               compatible = "arm,idle-state";
> +                               arm,psci-suspend-param = <0x40000004>;
> +                               entry-latency-us = <702>;
> +                               exit-latency-us = <1061>;
> +                               min-residency-us = <4488>;
> +                               local-timer-stop;
> +                               idle-state-name = "rail-power-down";
> +                       };
> +
> +                       CLUSTER_PD: cluster-power-down {
> +                               compatible = "arm,idle-state";
> +                               arm,psci-suspend-param = <0x400000F4>;
> +                               entry-latency-us = <3263>;
> +                               exit-latency-us = <6562>;
> +                               min-residency-us = <9987>;
> +                               local-timer-stop;
> +                               idle-state-name = "cluster-power-down";
> +                       };
> +               };
>         };
>
>         pmu {
> --
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
> of the Code Aurora Forum, hosted by The Linux Foundation.
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] arm64: dts: sdm845: Add PSCI cpuidle low power states
  2019-05-07 22:20   ` Amit Kucheria
@ 2019-05-10 11:32     ` Amit Kucheria
  -1 siblings, 0 replies; 10+ messages in thread
From: Amit Kucheria @ 2019-05-10 11:32 UTC (permalink / raw)
  To: Raju P.L.S.S.S.N, mkshah
  Cc: Andy Gross, David Brown, linux-arm-msm,
	open list:ARM/QUALCOMM SUPPORT, Rajendra Nayak, Bjorn Andersson,
	LKML, Linux PM list,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Stephen Boyd, evgreen, Douglas Anderson, Matthias Kaehlcke,
	ilina

On Wed, May 8, 2019 at 3:50 AM Amit Kucheria
<amit.kucheria@verdurent.com> wrote:
>
> On Tue, Oct 30, 2018 at 10:53 PM Raju P.L.S.S.S.N
> <rplsssn@codeaurora.org> wrote:
> >
> > Add device bindings for cpuidle states for cpu devices.
>
> Raju: Did this patch fall through the cracks? It would be nice to land
> this while Lina works on setting up the infrastructure to do
> hierarchical power domains.

After talking offline with Raju, I've added this patch with some
cleanups to my qcom cpuidle series.

> > Cc: <devicetree@vger.kernel.org>
> > Signed-off-by: Raju P.L.S.S.S.N <rplsssn@codeaurora.org>
> > ---
> > Changes in v2
> >  - Address comments from Doug
> > ---
> > ---
> >  arch/arm64/boot/dts/qcom/sdm845.dtsi | 62 ++++++++++++++++++++++++++++++++++++
> >  1 file changed, 62 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > index 0c9a2aa..3a8381e 100644
> > --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > @@ -96,6 +96,7 @@
> >                         reg = <0x0 0x0>;
> >                         enable-method = "psci";
> >                         next-level-cache = <&L2_0>;
> > +                       cpu-idle-states = <&C0_CPU_PD &C0_CPU_RPD &CLUSTER_PD>;
>
> May I suggest using more generic names here instead of C0_CPU_PD and
> move the QC-specific description to the idle-state-name property? C0
> and C4 isn't easy to understand at a glance. Neither is PD and RPD.
>
> Something like big_cpu_retention, big_cpu_sleep, little_cpu_retention,
> little_cpu_sleep, cluster_sleep?
> Or big_cpu_idle_0, big_cpu_idle_1, big_cpu_idle_2 for states with
> increasing breakeven times.
>
> I've commented below on what it might look like.
>
> >                         L2_0: l2-cache {
> >                                 compatible = "cache";
> >                                 next-level-cache = <&L3_0>;
> > @@ -111,6 +112,7 @@
> >                         reg = <0x0 0x100>;
> >                         enable-method = "psci";
> >                         next-level-cache = <&L2_100>;
> > +                       cpu-idle-states = <&C0_CPU_PD &C0_CPU_RPD &CLUSTER_PD>;
> >                         L2_100: l2-cache {
> >                                 compatible = "cache";
> >                                 next-level-cache = <&L3_0>;
> > @@ -123,6 +125,7 @@
> >                         reg = <0x0 0x200>;
> >                         enable-method = "psci";
> >                         next-level-cache = <&L2_200>;
> > +                       cpu-idle-states = <&C0_CPU_PD &C0_CPU_RPD &CLUSTER_PD>;
> >                         L2_200: l2-cache {
> >                                 compatible = "cache";
> >                                 next-level-cache = <&L3_0>;
> > @@ -135,6 +138,7 @@
> >                         reg = <0x0 0x300>;
> >                         enable-method = "psci";
> >                         next-level-cache = <&L2_300>;
> > +                       cpu-idle-states = <&C0_CPU_PD &C0_CPU_RPD &CLUSTER_PD>;
> >                         L2_300: l2-cache {
> >                                 compatible = "cache";
> >                                 next-level-cache = <&L3_0>;
> > @@ -147,6 +151,7 @@
> >                         reg = <0x0 0x400>;
> >                         enable-method = "psci";
> >                         next-level-cache = <&L2_400>;
> > +                       cpu-idle-states = <&C4_CPU_PD &C4_CPU_RPD &CLUSTER_PD>;
> >                         L2_400: l2-cache {
> >                                 compatible = "cache";
> >                                 next-level-cache = <&L3_0>;
> > @@ -159,6 +164,7 @@
> >                         reg = <0x0 0x500>;
> >                         enable-method = "psci";
> >                         next-level-cache = <&L2_500>;
> > +                       cpu-idle-states = <&C4_CPU_PD &C4_CPU_RPD &CLUSTER_PD>;
> >                         L2_500: l2-cache {
> >                                 compatible = "cache";
> >                                 next-level-cache = <&L3_0>;
> > @@ -171,6 +177,7 @@
> >                         reg = <0x0 0x600>;
> >                         enable-method = "psci";
> >                         next-level-cache = <&L2_600>;
> > +                       cpu-idle-states = <&C4_CPU_PD &C4_CPU_RPD &CLUSTER_PD>;
> >                         L2_600: l2-cache {
> >                                 compatible = "cache";
> >                                 next-level-cache = <&L3_0>;
> > @@ -183,11 +190,66 @@
> >                         reg = <0x0 0x700>;
> >                         enable-method = "psci";
> >                         next-level-cache = <&L2_700>;
> > +                       cpu-idle-states = <&C4_CPU_PD &C4_CPU_RPD &CLUSTER_PD>;
> >                         L2_700: l2-cache {
> >                                 compatible = "cache";
> >                                 next-level-cache = <&L3_0>;
> >                         };
> >                 };
> > +
> > +               idle-states {
> > +                       entry-method = "psci";
> > +
> > +                       C0_CPU_PD: c0-power-down {
>
> big_cpu_retention: big-cpu-retention
>
> > +                               compatible = "arm,idle-state";
> > +                               arm,psci-suspend-param = <0x40000003>;
> > +                               entry-latency-us = <350>;
> > +                               exit-latency-us = <461>;
> > +                               min-residency-us = <1890>;
> > +                               local-timer-stop;
> > +                               idle-state-name = "power-down";
>
> "big-cpu-power-down"
>
> > +                       };
> > +
> > +                       C0_CPU_RPD: c0-rail-power-down {
>
> big_cpu_sleep: big-cpu-sleep
>
> > +                               compatible = "arm,idle-state";
> > +                               arm,psci-suspend-param = <0x40000004>;
> > +                               entry-latency-us = <360>;
> > +                               exit-latency-us = <531>;
> > +                               min-residency-us = <3934>;
> > +                               local-timer-stop;
> > +                               idle-state-name = "rail-power-down";
>
> "big-cpu-rail-power-down"
>
> > +                       };
> > +
> > +                       C4_CPU_PD: c4-power-down {
>
> little_cpu_retention: little-cpu-retention
>
> > +                               compatible = "arm,idle-state";
> > +                               arm,psci-suspend-param = <0x40000003>;
> > +                               entry-latency-us = <264>;
> > +                               exit-latency-us = <621>;
> > +                               min-residency-us = <952>;
> > +                               local-timer-stop;
> > +                               idle-state-name = "power-down";
>
> "little-cpu-power-down" and so and so forth. You get the idea.
>
> > +                       };
> > +
> > +                       C4_CPU_RPD: c4-rail-power-down {
> > +                               compatible = "arm,idle-state";
> > +                               arm,psci-suspend-param = <0x40000004>;
> > +                               entry-latency-us = <702>;
> > +                               exit-latency-us = <1061>;
> > +                               min-residency-us = <4488>;
> > +                               local-timer-stop;
> > +                               idle-state-name = "rail-power-down";
> > +                       };
> > +
> > +                       CLUSTER_PD: cluster-power-down {
> > +                               compatible = "arm,idle-state";
> > +                               arm,psci-suspend-param = <0x400000F4>;
> > +                               entry-latency-us = <3263>;
> > +                               exit-latency-us = <6562>;
> > +                               min-residency-us = <9987>;
> > +                               local-timer-stop;
> > +                               idle-state-name = "cluster-power-down";
> > +                       };
> > +               };
> >         };
> >
> >         pmu {
> > --
> > QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
> > of the Code Aurora Forum, hosted by The Linux Foundation.
> >

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] arm64: dts: sdm845: Add PSCI cpuidle low power states
@ 2019-05-10 11:32     ` Amit Kucheria
  0 siblings, 0 replies; 10+ messages in thread
From: Amit Kucheria @ 2019-05-10 11:32 UTC (permalink / raw)
  To: Raju P.L.S.S.S.N, mkshah
  Cc: Andy Gross, David Brown, linux-arm-msm,
	open list:ARM/QUALCOMM SUPPORT, Rajendra Nayak, Bjorn Andersson,
	LKML, Linux PM list,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Stephen Boyd, evgreen, Douglas Anderson, Matthias Kaehlcke,
	ilina

On Wed, May 8, 2019 at 3:50 AM Amit Kucheria
<amit.kucheria@verdurent.com> wrote:
>
> On Tue, Oct 30, 2018 at 10:53 PM Raju P.L.S.S.S.N
> <rplsssn@codeaurora.org> wrote:
> >
> > Add device bindings for cpuidle states for cpu devices.
>
> Raju: Did this patch fall through the cracks? It would be nice to land
> this while Lina works on setting up the infrastructure to do
> hierarchical power domains.

After talking offline with Raju, I've added this patch with some
cleanups to my qcom cpuidle series.

> > Cc: <devicetree@vger.kernel.org>
> > Signed-off-by: Raju P.L.S.S.S.N <rplsssn@codeaurora.org>
> > ---
> > Changes in v2
> >  - Address comments from Doug
> > ---
> > ---
> >  arch/arm64/boot/dts/qcom/sdm845.dtsi | 62 ++++++++++++++++++++++++++++++++++++
> >  1 file changed, 62 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > index 0c9a2aa..3a8381e 100644
> > --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > @@ -96,6 +96,7 @@
> >                         reg = <0x0 0x0>;
> >                         enable-method = "psci";
> >                         next-level-cache = <&L2_0>;
> > +                       cpu-idle-states = <&C0_CPU_PD &C0_CPU_RPD &CLUSTER_PD>;
>
> May I suggest using more generic names here instead of C0_CPU_PD and
> move the QC-specific description to the idle-state-name property? C0
> and C4 isn't easy to understand at a glance. Neither is PD and RPD.
>
> Something like big_cpu_retention, big_cpu_sleep, little_cpu_retention,
> little_cpu_sleep, cluster_sleep?
> Or big_cpu_idle_0, big_cpu_idle_1, big_cpu_idle_2 for states with
> increasing breakeven times.
>
> I've commented below on what it might look like.
>
> >                         L2_0: l2-cache {
> >                                 compatible = "cache";
> >                                 next-level-cache = <&L3_0>;
> > @@ -111,6 +112,7 @@
> >                         reg = <0x0 0x100>;
> >                         enable-method = "psci";
> >                         next-level-cache = <&L2_100>;
> > +                       cpu-idle-states = <&C0_CPU_PD &C0_CPU_RPD &CLUSTER_PD>;
> >                         L2_100: l2-cache {
> >                                 compatible = "cache";
> >                                 next-level-cache = <&L3_0>;
> > @@ -123,6 +125,7 @@
> >                         reg = <0x0 0x200>;
> >                         enable-method = "psci";
> >                         next-level-cache = <&L2_200>;
> > +                       cpu-idle-states = <&C0_CPU_PD &C0_CPU_RPD &CLUSTER_PD>;
> >                         L2_200: l2-cache {
> >                                 compatible = "cache";
> >                                 next-level-cache = <&L3_0>;
> > @@ -135,6 +138,7 @@
> >                         reg = <0x0 0x300>;
> >                         enable-method = "psci";
> >                         next-level-cache = <&L2_300>;
> > +                       cpu-idle-states = <&C0_CPU_PD &C0_CPU_RPD &CLUSTER_PD>;
> >                         L2_300: l2-cache {
> >                                 compatible = "cache";
> >                                 next-level-cache = <&L3_0>;
> > @@ -147,6 +151,7 @@
> >                         reg = <0x0 0x400>;
> >                         enable-method = "psci";
> >                         next-level-cache = <&L2_400>;
> > +                       cpu-idle-states = <&C4_CPU_PD &C4_CPU_RPD &CLUSTER_PD>;
> >                         L2_400: l2-cache {
> >                                 compatible = "cache";
> >                                 next-level-cache = <&L3_0>;
> > @@ -159,6 +164,7 @@
> >                         reg = <0x0 0x500>;
> >                         enable-method = "psci";
> >                         next-level-cache = <&L2_500>;
> > +                       cpu-idle-states = <&C4_CPU_PD &C4_CPU_RPD &CLUSTER_PD>;
> >                         L2_500: l2-cache {
> >                                 compatible = "cache";
> >                                 next-level-cache = <&L3_0>;
> > @@ -171,6 +177,7 @@
> >                         reg = <0x0 0x600>;
> >                         enable-method = "psci";
> >                         next-level-cache = <&L2_600>;
> > +                       cpu-idle-states = <&C4_CPU_PD &C4_CPU_RPD &CLUSTER_PD>;
> >                         L2_600: l2-cache {
> >                                 compatible = "cache";
> >                                 next-level-cache = <&L3_0>;
> > @@ -183,11 +190,66 @@
> >                         reg = <0x0 0x700>;
> >                         enable-method = "psci";
> >                         next-level-cache = <&L2_700>;
> > +                       cpu-idle-states = <&C4_CPU_PD &C4_CPU_RPD &CLUSTER_PD>;
> >                         L2_700: l2-cache {
> >                                 compatible = "cache";
> >                                 next-level-cache = <&L3_0>;
> >                         };
> >                 };
> > +
> > +               idle-states {
> > +                       entry-method = "psci";
> > +
> > +                       C0_CPU_PD: c0-power-down {
>
> big_cpu_retention: big-cpu-retention
>
> > +                               compatible = "arm,idle-state";
> > +                               arm,psci-suspend-param = <0x40000003>;
> > +                               entry-latency-us = <350>;
> > +                               exit-latency-us = <461>;
> > +                               min-residency-us = <1890>;
> > +                               local-timer-stop;
> > +                               idle-state-name = "power-down";
>
> "big-cpu-power-down"
>
> > +                       };
> > +
> > +                       C0_CPU_RPD: c0-rail-power-down {
>
> big_cpu_sleep: big-cpu-sleep
>
> > +                               compatible = "arm,idle-state";
> > +                               arm,psci-suspend-param = <0x40000004>;
> > +                               entry-latency-us = <360>;
> > +                               exit-latency-us = <531>;
> > +                               min-residency-us = <3934>;
> > +                               local-timer-stop;
> > +                               idle-state-name = "rail-power-down";
>
> "big-cpu-rail-power-down"
>
> > +                       };
> > +
> > +                       C4_CPU_PD: c4-power-down {
>
> little_cpu_retention: little-cpu-retention
>
> > +                               compatible = "arm,idle-state";
> > +                               arm,psci-suspend-param = <0x40000003>;
> > +                               entry-latency-us = <264>;
> > +                               exit-latency-us = <621>;
> > +                               min-residency-us = <952>;
> > +                               local-timer-stop;
> > +                               idle-state-name = "power-down";
>
> "little-cpu-power-down" and so and so forth. You get the idea.
>
> > +                       };
> > +
> > +                       C4_CPU_RPD: c4-rail-power-down {
> > +                               compatible = "arm,idle-state";
> > +                               arm,psci-suspend-param = <0x40000004>;
> > +                               entry-latency-us = <702>;
> > +                               exit-latency-us = <1061>;
> > +                               min-residency-us = <4488>;
> > +                               local-timer-stop;
> > +                               idle-state-name = "rail-power-down";
> > +                       };
> > +
> > +                       CLUSTER_PD: cluster-power-down {
> > +                               compatible = "arm,idle-state";
> > +                               arm,psci-suspend-param = <0x400000F4>;
> > +                               entry-latency-us = <3263>;
> > +                               exit-latency-us = <6562>;
> > +                               min-residency-us = <9987>;
> > +                               local-timer-stop;
> > +                               idle-state-name = "cluster-power-down";
> > +                       };
> > +               };
> >         };
> >
> >         pmu {
> > --
> > QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
> > of the Code Aurora Forum, hosted by The Linux Foundation.
> >

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2019-05-10 11:32 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-30 17:23 [PATCH v2] arm64: dts: sdm845: Add PSCI cpuidle low power states Raju P.L.S.S.S.N
2018-10-31 23:57 ` Doug Anderson
2018-11-01 17:21 ` Evan Green
2018-11-30 15:41 ` Lina Iyer
2018-12-20 21:40   ` Doug Anderson
2018-12-20 22:55     ` Lina Iyer
2019-05-07 22:20 ` Amit Kucheria
2019-05-07 22:20   ` Amit Kucheria
2019-05-10 11:32   ` Amit Kucheria
2019-05-10 11:32     ` Amit Kucheria

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.