All of lore.kernel.org
 help / color / mirror / Atom feed
From: Niklas Cassel <niklas.cassel@linaro.org>
To: Marc Gonzalez <marc.w.gonzalez@free.fr>
Cc: Bjorn Andersson <bjorn.andersson@linaro.org>,
	Amit Kucheria <amit.kucheria@linaro.org>,
	MSM <linux-arm-msm@vger.kernel.org>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>,
	Daniel Lezcano <daniel.lezcano@linaro.org>,
	Sibi Sankar <sibis@codeaurora.org>,
	Jeffrey Hugo <jhugo@codeaurora.org>,
	Andy Gross <agross@kernel.org>
Subject: Re: [PATCH v3] arm64: dts: qcom: msm8998: Add PSCI cpuidle low power states
Date: Thu, 23 May 2019 23:46:19 +0200	[thread overview]
Message-ID: <20190523214619.GB25133@centauri> (raw)
In-Reply-To: <346cd9f0-583d-f467-83d0-e73768bf5aac@free.fr>

On Thu, May 23, 2019 at 10:36:51AM +0200, Marc Gonzalez wrote:
> From: Amit Kucheria <amit.kucheria@linaro.org>
> 
> Add device bindings for cpuidle states for cpu devices.
> 
> [marc: rebase and fix arm,psci-suspend-param for power-collapse]
> Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
> Signed-off-by: Marc Gonzalez <marc.w.gonzalez@free.fr>
> ---
> Bjorn, this is an updated/fixed (as documented above) version of
> [PATCH v2 7/9] arm64: dts: qcom: msm8998: Add PSCI cpuidle low power states
> ---
>  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 412195b9794c..224f84e39204 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>;
>  			next-level-cache = <&L2_0>;
>  			L2_0: l2-cache {
>  				compatible = "arm,arch-cache";
> @@ -96,6 +97,7 @@
>  			compatible = "arm,armv8";
>  			reg = <0x0 0x1>;
>  			enable-method = "psci";
> +			cpu-idle-states = <&LITTLE_CPU_SLEEP_0 &LITTLE_CPU_SLEEP_1>;
>  			next-level-cache = <&L2_0>;
>  			L1_I_1: l1-icache {
>  				compatible = "arm,arch-cache";
> @@ -110,6 +112,7 @@
>  			compatible = "arm,armv8";
>  			reg = <0x0 0x2>;
>  			enable-method = "psci";
> +			cpu-idle-states = <&LITTLE_CPU_SLEEP_0 &LITTLE_CPU_SLEEP_1>;
>  			next-level-cache = <&L2_0>;
>  			L1_I_2: l1-icache {
>  				compatible = "arm,arch-cache";
> @@ -124,6 +127,7 @@
>  			compatible = "arm,armv8";
>  			reg = <0x0 0x3>;
>  			enable-method = "psci";
> +			cpu-idle-states = <&LITTLE_CPU_SLEEP_0 &LITTLE_CPU_SLEEP_1>;
>  			next-level-cache = <&L2_0>;
>  			L1_I_3: l1-icache {
>  				compatible = "arm,arch-cache";
> @@ -138,6 +142,7 @@
>  			compatible = "arm,armv8";
>  			reg = <0x0 0x100>;
>  			enable-method = "psci";
> +			cpu-idle-states = <&BIG_CPU_SLEEP_0 &BIG_CPU_SLEEP_1>;
>  			next-level-cache = <&L2_1>;
>  			L2_1: l2-cache {
>  				compatible = "arm,arch-cache";
> @@ -156,6 +161,7 @@
>  			compatible = "arm,armv8";
>  			reg = <0x0 0x101>;
>  			enable-method = "psci";
> +			cpu-idle-states = <&BIG_CPU_SLEEP_0 &BIG_CPU_SLEEP_1>;
>  			next-level-cache = <&L2_1>;
>  			L1_I_101: l1-icache {
>  				compatible = "arm,arch-cache";
> @@ -170,6 +176,7 @@
>  			compatible = "arm,armv8";
>  			reg = <0x0 0x102>;
>  			enable-method = "psci";
> +			cpu-idle-states = <&BIG_CPU_SLEEP_0 &BIG_CPU_SLEEP_1>;
>  			next-level-cache = <&L2_1>;
>  			L1_I_102: l1-icache {
>  				compatible = "arm,arch-cache";
> @@ -184,6 +191,7 @@
>  			compatible = "arm,armv8";
>  			reg = <0x0 0x103>;
>  			enable-method = "psci";
> +			cpu-idle-states = <&BIG_CPU_SLEEP_0 &BIG_CPU_SLEEP_1>;
>  			next-level-cache = <&L2_1>;
>  			L1_I_103: l1-icache {
>  				compatible = "arm,arch-cache";
> @@ -230,6 +238,48 @@
>  				};
>  			};
>  		};
> +

Hello Marc, 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>;

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

Which for little cluster retention:

https://source.codeaurora.org/quic/la/kernel/msm-4.4/tree/arch/arm/boot/dts/qcom/msm8998-pm.dtsi?h=msm-4.4#n153

qcom,latency-us = <86>;
qcom,time-overhead = <167>;

gives:

entry-latency-us = <81>;
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 = <0x40000003>;
> +				entry-latency-us = <100>;
> +				exit-latency-us = <612>;

Which for little power collapse:

https://source.codeaurora.org/quic/la/kernel/msm-4.4/tree/arch/arm/boot/dts/qcom/msm8998-pm.dtsi?h=msm-4.4#n163

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

gives:

entry-latency-us = <273>;
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>;

Which for big retention:

https://source.codeaurora.org/quic/la/kernel/msm-4.4/tree/arch/arm/boot/dts/qcom/msm8998-pm.dtsi?h=msm-4.4#n246

qcom,latency-us = <82>;
qcom,time-overhead = <161>;

gives:

entry-latency-us = <79>;
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 = <0x40000003>;
> +				entry-latency-us = <100>;
> +				exit-latency-us = <525>;
> +				min-residency-us = <1000>;

Which for big power collapse:

https://source.codeaurora.org/quic/la/kernel/msm-4.4/tree/arch/arm/boot/dts/qcom/msm8998-pm.dtsi?h=msm-4.4#n256

qcom,latency-us = <525>;
qcom,time-overhead = <861>;

gives:

entry-latency-us = <336>;
exit-latency-us = <525>;

> +				local-timer-stop;
> +			};
> +		};
>  	};
>  
>  	firmware {
> -- 
> 2.17.1

WARNING: multiple messages have this Message-ID (diff)
From: Niklas Cassel <niklas.cassel@linaro.org>
To: Marc Gonzalez <marc.w.gonzalez@free.fr>
Cc: Jeffrey Hugo <jhugo@codeaurora.org>,
	MSM <linux-arm-msm@vger.kernel.org>,
	Daniel Lezcano <daniel.lezcano@linaro.org>,
	Amit Kucheria <amit.kucheria@linaro.org>,
	Bjorn Andersson <bjorn.andersson@linaro.org>,
	Andy Gross <agross@kernel.org>,
	Sibi Sankar <sibis@codeaurora.org>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH v3] arm64: dts: qcom: msm8998: Add PSCI cpuidle low power states
Date: Thu, 23 May 2019 23:46:19 +0200	[thread overview]
Message-ID: <20190523214619.GB25133@centauri> (raw)
In-Reply-To: <346cd9f0-583d-f467-83d0-e73768bf5aac@free.fr>

On Thu, May 23, 2019 at 10:36:51AM +0200, Marc Gonzalez wrote:
> From: Amit Kucheria <amit.kucheria@linaro.org>
> 
> Add device bindings for cpuidle states for cpu devices.
> 
> [marc: rebase and fix arm,psci-suspend-param for power-collapse]
> Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
> Signed-off-by: Marc Gonzalez <marc.w.gonzalez@free.fr>
> ---
> Bjorn, this is an updated/fixed (as documented above) version of
> [PATCH v2 7/9] arm64: dts: qcom: msm8998: Add PSCI cpuidle low power states
> ---
>  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 412195b9794c..224f84e39204 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>;
>  			next-level-cache = <&L2_0>;
>  			L2_0: l2-cache {
>  				compatible = "arm,arch-cache";
> @@ -96,6 +97,7 @@
>  			compatible = "arm,armv8";
>  			reg = <0x0 0x1>;
>  			enable-method = "psci";
> +			cpu-idle-states = <&LITTLE_CPU_SLEEP_0 &LITTLE_CPU_SLEEP_1>;
>  			next-level-cache = <&L2_0>;
>  			L1_I_1: l1-icache {
>  				compatible = "arm,arch-cache";
> @@ -110,6 +112,7 @@
>  			compatible = "arm,armv8";
>  			reg = <0x0 0x2>;
>  			enable-method = "psci";
> +			cpu-idle-states = <&LITTLE_CPU_SLEEP_0 &LITTLE_CPU_SLEEP_1>;
>  			next-level-cache = <&L2_0>;
>  			L1_I_2: l1-icache {
>  				compatible = "arm,arch-cache";
> @@ -124,6 +127,7 @@
>  			compatible = "arm,armv8";
>  			reg = <0x0 0x3>;
>  			enable-method = "psci";
> +			cpu-idle-states = <&LITTLE_CPU_SLEEP_0 &LITTLE_CPU_SLEEP_1>;
>  			next-level-cache = <&L2_0>;
>  			L1_I_3: l1-icache {
>  				compatible = "arm,arch-cache";
> @@ -138,6 +142,7 @@
>  			compatible = "arm,armv8";
>  			reg = <0x0 0x100>;
>  			enable-method = "psci";
> +			cpu-idle-states = <&BIG_CPU_SLEEP_0 &BIG_CPU_SLEEP_1>;
>  			next-level-cache = <&L2_1>;
>  			L2_1: l2-cache {
>  				compatible = "arm,arch-cache";
> @@ -156,6 +161,7 @@
>  			compatible = "arm,armv8";
>  			reg = <0x0 0x101>;
>  			enable-method = "psci";
> +			cpu-idle-states = <&BIG_CPU_SLEEP_0 &BIG_CPU_SLEEP_1>;
>  			next-level-cache = <&L2_1>;
>  			L1_I_101: l1-icache {
>  				compatible = "arm,arch-cache";
> @@ -170,6 +176,7 @@
>  			compatible = "arm,armv8";
>  			reg = <0x0 0x102>;
>  			enable-method = "psci";
> +			cpu-idle-states = <&BIG_CPU_SLEEP_0 &BIG_CPU_SLEEP_1>;
>  			next-level-cache = <&L2_1>;
>  			L1_I_102: l1-icache {
>  				compatible = "arm,arch-cache";
> @@ -184,6 +191,7 @@
>  			compatible = "arm,armv8";
>  			reg = <0x0 0x103>;
>  			enable-method = "psci";
> +			cpu-idle-states = <&BIG_CPU_SLEEP_0 &BIG_CPU_SLEEP_1>;
>  			next-level-cache = <&L2_1>;
>  			L1_I_103: l1-icache {
>  				compatible = "arm,arch-cache";
> @@ -230,6 +238,48 @@
>  				};
>  			};
>  		};
> +

Hello Marc, 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>;

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

Which for little cluster retention:

https://source.codeaurora.org/quic/la/kernel/msm-4.4/tree/arch/arm/boot/dts/qcom/msm8998-pm.dtsi?h=msm-4.4#n153

qcom,latency-us = <86>;
qcom,time-overhead = <167>;

gives:

entry-latency-us = <81>;
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 = <0x40000003>;
> +				entry-latency-us = <100>;
> +				exit-latency-us = <612>;

Which for little power collapse:

https://source.codeaurora.org/quic/la/kernel/msm-4.4/tree/arch/arm/boot/dts/qcom/msm8998-pm.dtsi?h=msm-4.4#n163

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

gives:

entry-latency-us = <273>;
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>;

Which for big retention:

https://source.codeaurora.org/quic/la/kernel/msm-4.4/tree/arch/arm/boot/dts/qcom/msm8998-pm.dtsi?h=msm-4.4#n246

qcom,latency-us = <82>;
qcom,time-overhead = <161>;

gives:

entry-latency-us = <79>;
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 = <0x40000003>;
> +				entry-latency-us = <100>;
> +				exit-latency-us = <525>;
> +				min-residency-us = <1000>;

Which for big power collapse:

https://source.codeaurora.org/quic/la/kernel/msm-4.4/tree/arch/arm/boot/dts/qcom/msm8998-pm.dtsi?h=msm-4.4#n256

qcom,latency-us = <525>;
qcom,time-overhead = <861>;

gives:

entry-latency-us = <336>;
exit-latency-us = <525>;

> +				local-timer-stop;
> +			};
> +		};
>  	};
>  
>  	firmware {
> -- 
> 2.17.1

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2019-05-23 21:46 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-23  8:36 [PATCH v3] arm64: dts: qcom: msm8998: Add PSCI cpuidle low power states Marc Gonzalez
2019-05-23  8:36 ` Marc Gonzalez
2019-05-23 21:46 ` Niklas Cassel [this message]
2019-05-23 21:46   ` Niklas Cassel
2019-05-24 11:33   ` Marc Gonzalez
2019-05-24 11:33     ` Marc Gonzalez
2019-05-24 12:32   ` [PATCH v4] " Marc Gonzalez
2019-05-24 12:32     ` Marc Gonzalez
2019-05-29 11:08     ` Marc Gonzalez
2019-05-29 11:08       ` Marc Gonzalez
2019-05-29 17:25       ` Lorenzo Pieralisi
2019-05-29 17:25         ` Lorenzo Pieralisi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190523214619.GB25133@centauri \
    --to=niklas.cassel@linaro.org \
    --cc=agross@kernel.org \
    --cc=amit.kucheria@linaro.org \
    --cc=bjorn.andersson@linaro.org \
    --cc=daniel.lezcano@linaro.org \
    --cc=jhugo@codeaurora.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=marc.w.gonzalez@free.fr \
    --cc=sibis@codeaurora.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.