[PATCH 0/2] Avoid calling cpu_pm functions for retention idle states

[PATCH 0/2] Avoid calling cpu_pm functions for retention idle states

[Prashanth Prakash]

CPU_PM_CPU_IDLE_ENTER() treats all idle states whose idx != 0 as a
state that loses some context, but we can have deeper idle states that
doesn't lose any software context. If a CPU is entering such a low power
idle state where it retains the context, then there is no need to call
cpu_pm_enter()/cpu_pm_exit().

Add a new macro(CPU_PM_CPU_IDLE_ENTER_RETENTION) to be used by cpuidle
drivers when they are entering retention state. By not calling cpu_pm_enter
and cpu_pm_exit we reduce the latency involved in entering and exiting
the retention states.

On ARM64 based Qualcomm server platform we measured below overhead for
for calling cpu_pm_enter and cpu_pm_exit for retention states.

workload: stress --hdd #CPUs --hdd-bytes 32M  -t 30
Overhead of cpu_pm_enter - 1.2us(Average), 6.5us(Max)
Overhead of cpu_pm_exit  - 3.1us(Average), 11.1us(Max)

Listed below are 5 functions that were notified on ENTER/EXIT on the
test platform:
      gic_cpu_pm_notifier
      arch_timer_cpu_pm_notify
      cpu_pm_pmu_notify
      hyp_init_cpu_pm_notifier
      fpsimd_cpu_pm_notifier

Prashanth Prakash (2):
  cpuidle: Add new macro to enter a retention idle state
  ARM64 / cpuidle: Use new cpuidle macro for entering retention state

 arch/arm64/kernel/cpuidle.c |  8 +++++++-
 include/linux/cpuidle.h     | 38 +++++++++++++++++++++++---------------
 2 files changed, 30 insertions(+), 16 deletions(-)

-- 
Qualcomm Datacenter Technologies on behalf of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

[PATCH 0/2] Avoid calling cpu_pm functions for retention idle states

[Prashanth Prakash]

CPU_PM_CPU_IDLE_ENTER() treats all idle states whose idx != 0 as a
state that loses some context, but we can have deeper idle states that
doesn't lose any software context. If a CPU is entering such a low power
idle state where it retains the context, then there is no need to call
cpu_pm_enter()/cpu_pm_exit().

Add a new macro(CPU_PM_CPU_IDLE_ENTER_RETENTION) to be used by cpuidle
drivers when they are entering retention state. By not calling cpu_pm_enter
and cpu_pm_exit we reduce the latency involved in entering and exiting
the retention states.

On ARM64 based Qualcomm server platform we measured below overhead for
for calling cpu_pm_enter and cpu_pm_exit for retention states.

workload: stress --hdd #CPUs --hdd-bytes 32M  -t 30
Overhead of cpu_pm_enter - 1.2us(Average), 6.5us(Max)
Overhead of cpu_pm_exit  - 3.1us(Average), 11.1us(Max)

Listed below are 5 functions that were notified on ENTER/EXIT on the
test platform:
      gic_cpu_pm_notifier
      arch_timer_cpu_pm_notify
      cpu_pm_pmu_notify
      hyp_init_cpu_pm_notifier
      fpsimd_cpu_pm_notifier

Prashanth Prakash (2):
  cpuidle: Add new macro to enter a retention idle state
  ARM64 / cpuidle: Use new cpuidle macro for entering retention state

 arch/arm64/kernel/cpuidle.c |  8 +++++++-
 include/linux/cpuidle.h     | 38 +++++++++++++++++++++++---------------
 2 files changed, 30 insertions(+), 16 deletions(-)

-- 
Qualcomm Datacenter Technologies on behalf of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

[PATCH 1/2] cpuidle: Add new macro to enter a retention idle state

[Prashanth Prakash]

If a CPU is entering a low power idle state where it doesn't lose any
context, then there is no need to call cpu_pm_enter()/cpu_pm_exit().
Add a new macro(CPU_PM_CPU_IDLE_ENTER_RETENTION) to be used by cpuidle
drivers when they are entering retention state. By not calling
cpu_pm_enter and cpu_pm_exit we reduce the latency involved in
entering and exiting the retention idle states.

On ARM64 based Qualcomm Server Platform we measured below overhead for
for calling cpu_pm_enter and cpu_pm_exit for retention states.

workload: stress --hdd #CPUs --hdd-bytes 32M  -t 30
        Average overhead of cpu_pm_enter - 1.2us
        Average overhead of cpu_pm_exit  - 3.1us

Signed-off-by: Prashanth Prakash <pprakash@codeaurora.org>
---
 include/linux/cpuidle.h | 38 +++++++++++++++++++++++---------------
 1 file changed, 23 insertions(+), 15 deletions(-)

diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
index 8f7788d..54cbd9d 100644
--- a/include/linux/cpuidle.h
+++ b/include/linux/cpuidle.h
@@ -258,21 +258,29 @@ static inline int cpuidle_register_governor(struct cpuidle_governor *gov)
 #endif
 
 #define CPU_PM_CPU_IDLE_ENTER(low_level_idle_enter, idx)	\
-({								\
-	int __ret;						\
-								\
-	if (!idx) {						\
-		cpu_do_idle();					\
-		return idx;					\
-	}							\
-								\
-	__ret = cpu_pm_enter();					\
-	if (!__ret) {						\
-		__ret = low_level_idle_enter(idx);		\
-		cpu_pm_exit();					\
-	}							\
-								\
-	__ret ? -1 : idx;					\
+	__CPU_PM_CPU_IDLE_ENTER(low_level_idle_enter, idx, 0)
+
+#define CPU_PM_CPU_IDLE_ENTER_RETENTION(low_level_idle_enter, idx)	\
+	__CPU_PM_CPU_IDLE_ENTER(low_level_idle_enter, idx, 1)
+
+#define __CPU_PM_CPU_IDLE_ENTER(low_level_idle_enter, idx, is_retention) \
+({									\
+	int __ret = 0;							\
+									\
+	if (!idx) {							\
+		cpu_do_idle();						\
+		return idx;						\
+	}								\
+									\
+	if (!is_retention)						\
+		__ret =  cpu_pm_enter();				\
+	if (!__ret) {							\
+		__ret = low_level_idle_enter(idx);			\
+		if (!is_retention)					\
+			cpu_pm_exit();					\
+	}								\
+									\
+	__ret ? -1 : idx;						\
 })
 
 #endif /* _LINUX_CPUIDLE_H */
-- 
Qualcomm Datacenter Technologies on behalf of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

[PATCH 1/2] cpuidle: Add new macro to enter a retention idle state

[Prashanth Prakash]

If a CPU is entering a low power idle state where it doesn't lose any
context, then there is no need to call cpu_pm_enter()/cpu_pm_exit().
Add a new macro(CPU_PM_CPU_IDLE_ENTER_RETENTION) to be used by cpuidle
drivers when they are entering retention state. By not calling
cpu_pm_enter and cpu_pm_exit we reduce the latency involved in
entering and exiting the retention idle states.

On ARM64 based Qualcomm Server Platform we measured below overhead for
for calling cpu_pm_enter and cpu_pm_exit for retention states.

workload: stress --hdd #CPUs --hdd-bytes 32M  -t 30
        Average overhead of cpu_pm_enter - 1.2us
        Average overhead of cpu_pm_exit  - 3.1us

Signed-off-by: Prashanth Prakash <pprakash@codeaurora.org>
---
 include/linux/cpuidle.h | 38 +++++++++++++++++++++++---------------
 1 file changed, 23 insertions(+), 15 deletions(-)

diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
index 8f7788d..54cbd9d 100644
--- a/include/linux/cpuidle.h
+++ b/include/linux/cpuidle.h
@@ -258,21 +258,29 @@ static inline int cpuidle_register_governor(struct cpuidle_governor *gov)
 #endif
 
 #define CPU_PM_CPU_IDLE_ENTER(low_level_idle_enter, idx)	\
-({								\
-	int __ret;						\
-								\
-	if (!idx) {						\
-		cpu_do_idle();					\
-		return idx;					\
-	}							\
-								\
-	__ret = cpu_pm_enter();					\
-	if (!__ret) {						\
-		__ret = low_level_idle_enter(idx);		\
-		cpu_pm_exit();					\
-	}							\
-								\
-	__ret ? -1 : idx;					\
+	__CPU_PM_CPU_IDLE_ENTER(low_level_idle_enter, idx, 0)
+
+#define CPU_PM_CPU_IDLE_ENTER_RETENTION(low_level_idle_enter, idx)	\
+	__CPU_PM_CPU_IDLE_ENTER(low_level_idle_enter, idx, 1)
+
+#define __CPU_PM_CPU_IDLE_ENTER(low_level_idle_enter, idx, is_retention) \
+({									\
+	int __ret = 0;							\
+									\
+	if (!idx) {							\
+		cpu_do_idle();						\
+		return idx;						\
+	}								\
+									\
+	if (!is_retention)						\
+		__ret =  cpu_pm_enter();				\
+	if (!__ret) {							\
+		__ret = low_level_idle_enter(idx);			\
+		if (!is_retention)					\
+			cpu_pm_exit();					\
+	}								\
+									\
+	__ret ? -1 : idx;						\
 })
 
 #endif /* _LINUX_CPUIDLE_H */
-- 
Qualcomm Datacenter Technologies on behalf of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

[PATCH 2/2] ARM64 / cpuidle: Use new cpuidle macro for entering retention state

[Prashanth Prakash]

CPU_PM_CPU_IDLE_ENTER_RETENTION skips calling cpu_pm_enter() and
cpu_pm_exit(). By not calling cpu_pm functions in idle entry/exit
paths we can reduce the latency involved in entering and exiting
the low power idle state.

On ARM64 based Qualcomm server platform we measured below overhead
for calling cpu_pm_enter and cpu_pm_exit for retention states.

workload: stress --hdd #CPUs --hdd-bytes 32M  -t 30
	Average overhead of cpu_pm_enter - 1.2us
	Average overhead of cpu_pm_exit  - 3.1us

Signed-off-by: Prashanth Prakash <pprakash@codeaurora.org>
---
 arch/arm64/kernel/cpuidle.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/cpuidle.c b/arch/arm64/kernel/cpuidle.c
index fd69108..f2d1381 100644
--- a/arch/arm64/kernel/cpuidle.c
+++ b/arch/arm64/kernel/cpuidle.c
@@ -47,6 +47,8 @@ int arm_cpuidle_suspend(int index)
 
 #include <acpi/processor.h>
 
+#define ARM64_LPI_IS_RETENTION_STATE(arch_flags) (!(arch_flags))
+
 int acpi_processor_ffh_lpi_probe(unsigned int cpu)
 {
 	return arm_cpuidle_init(cpu);
@@ -54,6 +56,10 @@ int acpi_processor_ffh_lpi_probe(unsigned int cpu)
 
 int acpi_processor_ffh_lpi_enter(struct acpi_lpi_state *lpi)
 {
-	return CPU_PM_CPU_IDLE_ENTER(arm_cpuidle_suspend, lpi->index);
+	if (ARM64_LPI_IS_RETENTION_STATE(lpi->arch_flags))
+		return CPU_PM_CPU_IDLE_ENTER_RETENTION(arm_cpuidle_suspend,
+						lpi->index);
+	else
+		return CPU_PM_CPU_IDLE_ENTER(arm_cpuidle_suspend, lpi->index);
 }
 #endif
-- 
Qualcomm Datacenter Technologies on behalf of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

[PATCH 2/2] ARM64 / cpuidle: Use new cpuidle macro for entering retention state

[Prashanth Prakash]

CPU_PM_CPU_IDLE_ENTER_RETENTION skips calling cpu_pm_enter() and
cpu_pm_exit(). By not calling cpu_pm functions in idle entry/exit
paths we can reduce the latency involved in entering and exiting
the low power idle state.

On ARM64 based Qualcomm server platform we measured below overhead
for calling cpu_pm_enter and cpu_pm_exit for retention states.

workload: stress --hdd #CPUs --hdd-bytes 32M  -t 30
	Average overhead of cpu_pm_enter - 1.2us
	Average overhead of cpu_pm_exit  - 3.1us

Signed-off-by: Prashanth Prakash <pprakash@codeaurora.org>
---
 arch/arm64/kernel/cpuidle.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/cpuidle.c b/arch/arm64/kernel/cpuidle.c
index fd69108..f2d1381 100644
--- a/arch/arm64/kernel/cpuidle.c
+++ b/arch/arm64/kernel/cpuidle.c
@@ -47,6 +47,8 @@ int arm_cpuidle_suspend(int index)
 
 #include <acpi/processor.h>
 
+#define ARM64_LPI_IS_RETENTION_STATE(arch_flags) (!(arch_flags))
+
 int acpi_processor_ffh_lpi_probe(unsigned int cpu)
 {
 	return arm_cpuidle_init(cpu);
@@ -54,6 +56,10 @@ int acpi_processor_ffh_lpi_probe(unsigned int cpu)
 
 int acpi_processor_ffh_lpi_enter(struct acpi_lpi_state *lpi)
 {
-	return CPU_PM_CPU_IDLE_ENTER(arm_cpuidle_suspend, lpi->index);
+	if (ARM64_LPI_IS_RETENTION_STATE(lpi->arch_flags))
+		return CPU_PM_CPU_IDLE_ENTER_RETENTION(arm_cpuidle_suspend,
+						lpi->index);
+	else
+		return CPU_PM_CPU_IDLE_ENTER(arm_cpuidle_suspend, lpi->index);
 }
 #endif
-- 
Qualcomm Datacenter Technologies on behalf of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

Re: [PATCH 2/2] ARM64 / cpuidle: Use new cpuidle macro for entering retention state

[Sudeep Holla]

On 07/11/17 17:35, Prashanth Prakash wrote:
> CPU_PM_CPU_IDLE_ENTER_RETENTION skips calling cpu_pm_enter() and
> cpu_pm_exit(). By not calling cpu_pm functions in idle entry/exit
> paths we can reduce the latency involved in entering and exiting
> the low power idle state.
> 
> On ARM64 based Qualcomm server platform we measured below overhead
> for calling cpu_pm_enter and cpu_pm_exit for retention states.
> 
> workload: stress --hdd #CPUs --hdd-bytes 32M  -t 30
> 	Average overhead of cpu_pm_enter - 1.2us
> 	Average overhead of cpu_pm_exit  - 3.1us
> 
> Signed-off-by: Prashanth Prakash <pprakash@codeaurora.org>
> ---
>  arch/arm64/kernel/cpuidle.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kernel/cpuidle.c b/arch/arm64/kernel/cpuidle.c
> index fd69108..f2d1381 100644
> --- a/arch/arm64/kernel/cpuidle.c
> +++ b/arch/arm64/kernel/cpuidle.c
> @@ -47,6 +47,8 @@ int arm_cpuidle_suspend(int index)
>  
>  #include <acpi/processor.h>
>  
> +#define ARM64_LPI_IS_RETENTION_STATE(arch_flags) (!(arch_flags))
> +

Ideally we could cater both ACPI LPI and DT using PSCI parameters. No ?
Just thinking it out loud as I see no reason to restrict this just to
ACPI LPIs.

-- 
Regards,
Sudeep

[PATCH 2/2] ARM64 / cpuidle: Use new cpuidle macro for entering retention state

[Sudeep Holla]

On 07/11/17 17:35, Prashanth Prakash wrote:
> CPU_PM_CPU_IDLE_ENTER_RETENTION skips calling cpu_pm_enter() and
> cpu_pm_exit(). By not calling cpu_pm functions in idle entry/exit
> paths we can reduce the latency involved in entering and exiting
> the low power idle state.
> 
> On ARM64 based Qualcomm server platform we measured below overhead
> for calling cpu_pm_enter and cpu_pm_exit for retention states.
> 
> workload: stress --hdd #CPUs --hdd-bytes 32M  -t 30
> 	Average overhead of cpu_pm_enter - 1.2us
> 	Average overhead of cpu_pm_exit  - 3.1us
> 
> Signed-off-by: Prashanth Prakash <pprakash@codeaurora.org>
> ---
>  arch/arm64/kernel/cpuidle.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kernel/cpuidle.c b/arch/arm64/kernel/cpuidle.c
> index fd69108..f2d1381 100644
> --- a/arch/arm64/kernel/cpuidle.c
> +++ b/arch/arm64/kernel/cpuidle.c
> @@ -47,6 +47,8 @@ int arm_cpuidle_suspend(int index)
>  
>  #include <acpi/processor.h>
>  
> +#define ARM64_LPI_IS_RETENTION_STATE(arch_flags) (!(arch_flags))
> +

Ideally we could cater both ACPI LPI and DT using PSCI parameters. No ?
Just thinking it out loud as I see no reason to restrict this just to
ACPI LPIs.

-- 
Regards,
Sudeep

Re: [PATCH 1/2] cpuidle: Add new macro to enter a retention idle state

[Sudeep Holla]

On 07/11/17 17:35, Prashanth Prakash wrote:
> If a CPU is entering a low power idle state where it doesn't lose any
> context, then there is no need to call cpu_pm_enter()/cpu_pm_exit().
> Add a new macro(CPU_PM_CPU_IDLE_ENTER_RETENTION) to be used by cpuidle
> drivers when they are entering retention state. By not calling
> cpu_pm_enter and cpu_pm_exit we reduce the latency involved in
> entering and exiting the retention idle states.
> 
> On ARM64 based Qualcomm Server Platform we measured below overhead for
> for calling cpu_pm_enter and cpu_pm_exit for retention states.
> 
> workload: stress --hdd #CPUs --hdd-bytes 32M  -t 30
>         Average overhead of cpu_pm_enter - 1.2us
>         Average overhead of cpu_pm_exit  - 3.1us
> 
> Signed-off-by: Prashanth Prakash <pprakash@codeaurora.org>
> ---
>  include/linux/cpuidle.h | 38 +++++++++++++++++++++++---------------
>  1 file changed, 23 insertions(+), 15 deletions(-)
> 
> diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
> index 8f7788d..54cbd9d 100644
> --- a/include/linux/cpuidle.h
> +++ b/include/linux/cpuidle.h
> @@ -258,21 +258,29 @@ static inline int cpuidle_register_governor(struct cpuidle_governor *gov)
>  #endif
>  
>  #define CPU_PM_CPU_IDLE_ENTER(low_level_idle_enter, idx)	\
> -({								\
> -	int __ret;						\
> -								\
> -	if (!idx) {						\
> -		cpu_do_idle();					\
> -		return idx;					\
> -	}							\
> -								\
> -	__ret = cpu_pm_enter();					\
> -	if (!__ret) {						\
> -		__ret = low_level_idle_enter(idx);		\
> -		cpu_pm_exit();					\
> -	}							\
> -								\
> -	__ret ? -1 : idx;					\
> +	__CPU_PM_CPU_IDLE_ENTER(low_level_idle_enter, idx, 0)
> +
> +#define CPU_PM_CPU_IDLE_ENTER_RETENTION(low_level_idle_enter, idx)	\
> +	__CPU_PM_CPU_IDLE_ENTER(low_level_idle_enter, idx, 1)
> +
> +#define __CPU_PM_CPU_IDLE_ENTER(low_level_idle_enter, idx, is_retention) \
> +({									\
> +	int __ret = 0;							\
> +									\
> +	if (!idx) {							\
> +		cpu_do_idle();						\
> +		return idx;						\
> +	}								\
> +									\
> +	if (!is_retention)						\
> +		__ret =  cpu_pm_enter();				\

I am fine with this change as initial step. But I am wondering if we
will have a retention state which loses partial state ?

The specification has flags to specify that difference but will we see
that in reality is a different question. If we see such hardware, then
we may need to revert this and handle in the callbacks as we can't skip
cpu_pm notifier callbacks all together right ?

-- 
-- 
Regards,
Sudeep

[PATCH 1/2] cpuidle: Add new macro to enter a retention idle state

[Sudeep Holla]

On 07/11/17 17:35, Prashanth Prakash wrote:
> If a CPU is entering a low power idle state where it doesn't lose any
> context, then there is no need to call cpu_pm_enter()/cpu_pm_exit().
> Add a new macro(CPU_PM_CPU_IDLE_ENTER_RETENTION) to be used by cpuidle
> drivers when they are entering retention state. By not calling
> cpu_pm_enter and cpu_pm_exit we reduce the latency involved in
> entering and exiting the retention idle states.
> 
> On ARM64 based Qualcomm Server Platform we measured below overhead for
> for calling cpu_pm_enter and cpu_pm_exit for retention states.
> 
> workload: stress --hdd #CPUs --hdd-bytes 32M  -t 30
>         Average overhead of cpu_pm_enter - 1.2us
>         Average overhead of cpu_pm_exit  - 3.1us
> 
> Signed-off-by: Prashanth Prakash <pprakash@codeaurora.org>
> ---
>  include/linux/cpuidle.h | 38 +++++++++++++++++++++++---------------
>  1 file changed, 23 insertions(+), 15 deletions(-)
> 
> diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
> index 8f7788d..54cbd9d 100644
> --- a/include/linux/cpuidle.h
> +++ b/include/linux/cpuidle.h
> @@ -258,21 +258,29 @@ static inline int cpuidle_register_governor(struct cpuidle_governor *gov)
>  #endif
>  
>  #define CPU_PM_CPU_IDLE_ENTER(low_level_idle_enter, idx)	\
> -({								\
> -	int __ret;						\
> -								\
> -	if (!idx) {						\
> -		cpu_do_idle();					\
> -		return idx;					\
> -	}							\
> -								\
> -	__ret = cpu_pm_enter();					\
> -	if (!__ret) {						\
> -		__ret = low_level_idle_enter(idx);		\
> -		cpu_pm_exit();					\
> -	}							\
> -								\
> -	__ret ? -1 : idx;					\
> +	__CPU_PM_CPU_IDLE_ENTER(low_level_idle_enter, idx, 0)
> +
> +#define CPU_PM_CPU_IDLE_ENTER_RETENTION(low_level_idle_enter, idx)	\
> +	__CPU_PM_CPU_IDLE_ENTER(low_level_idle_enter, idx, 1)
> +
> +#define __CPU_PM_CPU_IDLE_ENTER(low_level_idle_enter, idx, is_retention) \
> +({									\
> +	int __ret = 0;							\
> +									\
> +	if (!idx) {							\
> +		cpu_do_idle();						\
> +		return idx;						\
> +	}								\
> +									\
> +	if (!is_retention)						\
> +		__ret =  cpu_pm_enter();				\

I am fine with this change as initial step. But I am wondering if we
will have a retention state which loses partial state ?

The specification has flags to specify that difference but will we see
that in reality is a different question. If we see such hardware, then
we may need to revert this and handle in the callbacks as we can't skip
cpu_pm notifier callbacks all together right ?

-- 
-- 
Regards,
Sudeep

Re: [PATCH 2/2] ARM64 / cpuidle: Use new cpuidle macro for entering retention state

[Sudeep Holla]

On 08/11/17 14:27, Sudeep Holla wrote:
> 
> 
> On 07/11/17 17:35, Prashanth Prakash wrote:
>> CPU_PM_CPU_IDLE_ENTER_RETENTION skips calling cpu_pm_enter() and
>> cpu_pm_exit(). By not calling cpu_pm functions in idle entry/exit
>> paths we can reduce the latency involved in entering and exiting
>> the low power idle state.
>>
>> On ARM64 based Qualcomm server platform we measured below overhead
>> for calling cpu_pm_enter and cpu_pm_exit for retention states.
>>
>> workload: stress --hdd #CPUs --hdd-bytes 32M  -t 30
>> 	Average overhead of cpu_pm_enter - 1.2us
>> 	Average overhead of cpu_pm_exit  - 3.1us
>>
>> Signed-off-by: Prashanth Prakash <pprakash@codeaurora.org>
>> ---
>>  arch/arm64/kernel/cpuidle.c | 8 +++++++-
>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/kernel/cpuidle.c b/arch/arm64/kernel/cpuidle.c
>> index fd69108..f2d1381 100644
>> --- a/arch/arm64/kernel/cpuidle.c
>> +++ b/arch/arm64/kernel/cpuidle.c
>> @@ -47,6 +47,8 @@ int arm_cpuidle_suspend(int index)
>>  
>>  #include <acpi/processor.h>
>>  
>> +#define ARM64_LPI_IS_RETENTION_STATE(arch_flags) (!(arch_flags))
>> +
> 
> Ideally we could cater both ACPI LPI and DT using PSCI parameters. No ?
> Just thinking it out loud as I see no reason to restrict this just to
> ACPI LPIs.
> 

Just to add, after chatting with Lorenzo, I see PSCI covers only core
states but that then leads to my question in the other patch: do we need
partial state save/restore ever ? If not, then we can still use PSCI
flag in both DT and ACPI.

-- 
Regards,
Sudeep

[PATCH 2/2] ARM64 / cpuidle: Use new cpuidle macro for entering retention state

[Sudeep Holla]

On 08/11/17 14:27, Sudeep Holla wrote:
> 
> 
> On 07/11/17 17:35, Prashanth Prakash wrote:
>> CPU_PM_CPU_IDLE_ENTER_RETENTION skips calling cpu_pm_enter() and
>> cpu_pm_exit(). By not calling cpu_pm functions in idle entry/exit
>> paths we can reduce the latency involved in entering and exiting
>> the low power idle state.
>>
>> On ARM64 based Qualcomm server platform we measured below overhead
>> for calling cpu_pm_enter and cpu_pm_exit for retention states.
>>
>> workload: stress --hdd #CPUs --hdd-bytes 32M  -t 30
>> 	Average overhead of cpu_pm_enter - 1.2us
>> 	Average overhead of cpu_pm_exit  - 3.1us
>>
>> Signed-off-by: Prashanth Prakash <pprakash@codeaurora.org>
>> ---
>>  arch/arm64/kernel/cpuidle.c | 8 +++++++-
>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/kernel/cpuidle.c b/arch/arm64/kernel/cpuidle.c
>> index fd69108..f2d1381 100644
>> --- a/arch/arm64/kernel/cpuidle.c
>> +++ b/arch/arm64/kernel/cpuidle.c
>> @@ -47,6 +47,8 @@ int arm_cpuidle_suspend(int index)
>>  
>>  #include <acpi/processor.h>
>>  
>> +#define ARM64_LPI_IS_RETENTION_STATE(arch_flags) (!(arch_flags))
>> +
> 
> Ideally we could cater both ACPI LPI and DT using PSCI parameters. No ?
> Just thinking it out loud as I see no reason to restrict this just to
> ACPI LPIs.
> 

Just to add, after chatting with Lorenzo, I see PSCI covers only core
states but that then leads to my question in the other patch: do we need
partial state save/restore ever ? If not, then we can still use PSCI
flag in both DT and ACPI.

-- 
Regards,
Sudeep

Re: [PATCH 1/2] cpuidle: Add new macro to enter a retention idle state

[Prakash, Prashanth]

Hi Sudeep,

On 11/8/2017 7:38 AM, Sudeep Holla wrote:
>
> On 07/11/17 17:35, Prashanth Prakash wrote:
>> If a CPU is entering a low power idle state where it doesn't lose any
>> context, then there is no need to call cpu_pm_enter()/cpu_pm_exit().
>> Add a new macro(CPU_PM_CPU_IDLE_ENTER_RETENTION) to be used by cpuidle
>> drivers when they are entering retention state. By not calling
>> cpu_pm_enter and cpu_pm_exit we reduce the latency involved in
>> entering and exiting the retention idle states.
>>
>> On ARM64 based Qualcomm Server Platform we measured below overhead for
>> for calling cpu_pm_enter and cpu_pm_exit for retention states.
>>
>> workload: stress --hdd #CPUs --hdd-bytes 32M  -t 30
>>         Average overhead of cpu_pm_enter - 1.2us
>>         Average overhead of cpu_pm_exit  - 3.1us
>>
>> Signed-off-by: Prashanth Prakash <pprakash@codeaurora.org>
>> ---
>>  include/linux/cpuidle.h | 38 +++++++++++++++++++++++---------------
>>  1 file changed, 23 insertions(+), 15 deletions(-)
>>
>> diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
>> index 8f7788d..54cbd9d 100644
>> --- a/include/linux/cpuidle.h
>> +++ b/include/linux/cpuidle.h
>> @@ -258,21 +258,29 @@ static inline int cpuidle_register_governor(struct cpuidle_governor *gov)
>>  #endif
>>  
>>  #define CPU_PM_CPU_IDLE_ENTER(low_level_idle_enter, idx)	\
>> -({								\
>> -	int __ret;						\
>> -								\
>> -	if (!idx) {						\
>> -		cpu_do_idle();					\
>> -		return idx;					\
>> -	}							\
>> -								\
>> -	__ret = cpu_pm_enter();					\
>> -	if (!__ret) {						\
>> -		__ret = low_level_idle_enter(idx);		\
>> -		cpu_pm_exit();					\
>> -	}							\
>> -								\
>> -	__ret ? -1 : idx;					\
>> +	__CPU_PM_CPU_IDLE_ENTER(low_level_idle_enter, idx, 0)
>> +
>> +#define CPU_PM_CPU_IDLE_ENTER_RETENTION(low_level_idle_enter, idx)	\
>> +	__CPU_PM_CPU_IDLE_ENTER(low_level_idle_enter, idx, 1)
>> +
>> +#define __CPU_PM_CPU_IDLE_ENTER(low_level_idle_enter, idx, is_retention) \
>> +({									\
>> +	int __ret = 0;							\
>> +									\
>> +	if (!idx) {							\
>> +		cpu_do_idle();						\
>> +		return idx;						\
>> +	}								\
>> +									\
>> +	if (!is_retention)						\
>> +		__ret =  cpu_pm_enter();				\
> I am fine with this change as initial step. But I am wondering if we
> will have a retention state which loses partial state ?
>
> The specification has flags to specify that difference but will we see
> that in reality is a different question. If we see such hardware, then
> we may need to revert this and handle in the callbacks as we can't skip
> cpu_pm notifier callbacks all together right ?
I agree. This is a initial and quick first step and will help only fully retention states.

If there are devices with partial retention states and necessary support in cpu
PM framework we can revert this and handle it in more generic way.

--
Thanks,
Prashanth

[PATCH 1/2] cpuidle: Add new macro to enter a retention idle state

[Prakash, Prashanth]

Hi Sudeep,

On 11/8/2017 7:38 AM, Sudeep Holla wrote:
>
> On 07/11/17 17:35, Prashanth Prakash wrote:
>> If a CPU is entering a low power idle state where it doesn't lose any
>> context, then there is no need to call cpu_pm_enter()/cpu_pm_exit().
>> Add a new macro(CPU_PM_CPU_IDLE_ENTER_RETENTION) to be used by cpuidle
>> drivers when they are entering retention state. By not calling
>> cpu_pm_enter and cpu_pm_exit we reduce the latency involved in
>> entering and exiting the retention idle states.
>>
>> On ARM64 based Qualcomm Server Platform we measured below overhead for
>> for calling cpu_pm_enter and cpu_pm_exit for retention states.
>>
>> workload: stress --hdd #CPUs --hdd-bytes 32M  -t 30
>>         Average overhead of cpu_pm_enter - 1.2us
>>         Average overhead of cpu_pm_exit  - 3.1us
>>
>> Signed-off-by: Prashanth Prakash <pprakash@codeaurora.org>
>> ---
>>  include/linux/cpuidle.h | 38 +++++++++++++++++++++++---------------
>>  1 file changed, 23 insertions(+), 15 deletions(-)
>>
>> diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
>> index 8f7788d..54cbd9d 100644
>> --- a/include/linux/cpuidle.h
>> +++ b/include/linux/cpuidle.h
>> @@ -258,21 +258,29 @@ static inline int cpuidle_register_governor(struct cpuidle_governor *gov)
>>  #endif
>>  
>>  #define CPU_PM_CPU_IDLE_ENTER(low_level_idle_enter, idx)	\
>> -({								\
>> -	int __ret;						\
>> -								\
>> -	if (!idx) {						\
>> -		cpu_do_idle();					\
>> -		return idx;					\
>> -	}							\
>> -								\
>> -	__ret = cpu_pm_enter();					\
>> -	if (!__ret) {						\
>> -		__ret = low_level_idle_enter(idx);		\
>> -		cpu_pm_exit();					\
>> -	}							\
>> -								\
>> -	__ret ? -1 : idx;					\
>> +	__CPU_PM_CPU_IDLE_ENTER(low_level_idle_enter, idx, 0)
>> +
>> +#define CPU_PM_CPU_IDLE_ENTER_RETENTION(low_level_idle_enter, idx)	\
>> +	__CPU_PM_CPU_IDLE_ENTER(low_level_idle_enter, idx, 1)
>> +
>> +#define __CPU_PM_CPU_IDLE_ENTER(low_level_idle_enter, idx, is_retention) \
>> +({									\
>> +	int __ret = 0;							\
>> +									\
>> +	if (!idx) {							\
>> +		cpu_do_idle();						\
>> +		return idx;						\
>> +	}								\
>> +									\
>> +	if (!is_retention)						\
>> +		__ret =  cpu_pm_enter();				\
> I am fine with this change as initial step. But I am wondering if we
> will have a retention state which loses partial state ?
>
> The specification has flags to specify that difference but will we see
> that in reality is a different question. If we see such hardware, then
> we may need to revert this and handle in the callbacks as we can't skip
> cpu_pm notifier callbacks all together right ?
I agree. This is a initial and quick first step and will help only fully retention states.

If there are devices with partial retention states and necessary support in cpu
PM framework we can revert this and handle it in more generic way.

--
Thanks,
Prashanth

Re: [PATCH 1/2] cpuidle: Add new macro to enter a retention idle state

[Sudeep Holla]

On 08/11/17 17:15, Prakash, Prashanth wrote:
> Hi Sudeep,
> 
> On 11/8/2017 7:38 AM, Sudeep Holla wrote:

>> I am fine with this change as initial step. But I am wondering if we
>> will have a retention state which loses partial state ?
>>
>> The specification has flags to specify that difference but will we see
>> that in reality is a different question. If we see such hardware, then
>> we may need to revert this and handle in the callbacks as we can't skip
>> cpu_pm notifier callbacks all together right ?
> I agree. This is a initial and quick first step and will help only fully retention states.
> 
> If there are devices with partial retention states and necessary support in cpu
> PM framework we can revert this and handle it in more generic way.
> 

Sounds good. Just wanted to be aware of that fact. Hope we don't get
such crazy hardware(but you never know).

-- 
Regards,
Sudeep

[PATCH 1/2] cpuidle: Add new macro to enter a retention idle state

[Sudeep Holla]

On 08/11/17 17:15, Prakash, Prashanth wrote:
> Hi Sudeep,
> 
> On 11/8/2017 7:38 AM, Sudeep Holla wrote:

>> I am fine with this change as initial step. But I am wondering if we
>> will have a retention state which loses partial state ?
>>
>> The specification has flags to specify that difference but will we see
>> that in reality is a different question. If we see such hardware, then
>> we may need to revert this and handle in the callbacks as we can't skip
>> cpu_pm notifier callbacks all together right ?
> I agree. This is a initial and quick first step and will help only fully retention states.
> 
> If there are devices with partial retention states and necessary support in cpu
> PM framework we can revert this and handle it in more generic way.
> 

Sounds good. Just wanted to be aware of that fact. Hope we don't get
such crazy hardware(but you never know).

-- 
Regards,
Sudeep

Re: [PATCH 1/2] cpuidle: Add new macro to enter a retention idle state

[Rafael J. Wysocki]

On Tuesday, November 7, 2017 6:35:32 PM CET Prashanth Prakash wrote:
> If a CPU is entering a low power idle state where it doesn't lose any
> context, then there is no need to call cpu_pm_enter()/cpu_pm_exit().
> Add a new macro(CPU_PM_CPU_IDLE_ENTER_RETENTION) to be used by cpuidle
> drivers when they are entering retention state. By not calling
> cpu_pm_enter and cpu_pm_exit we reduce the latency involved in
> entering and exiting the retention idle states.
> 
> On ARM64 based Qualcomm Server Platform we measured below overhead for
> for calling cpu_pm_enter and cpu_pm_exit for retention states.
> 
> workload: stress --hdd #CPUs --hdd-bytes 32M  -t 30
>         Average overhead of cpu_pm_enter - 1.2us
>         Average overhead of cpu_pm_exit  - 3.1us
> 
> Signed-off-by: Prashanth Prakash <pprakash@codeaurora.org>
> ---
>  include/linux/cpuidle.h | 38 +++++++++++++++++++++++---------------
>  1 file changed, 23 insertions(+), 15 deletions(-)
> 
> diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
> index 8f7788d..54cbd9d 100644
> --- a/include/linux/cpuidle.h
> +++ b/include/linux/cpuidle.h
> @@ -258,21 +258,29 @@ static inline int cpuidle_register_governor(struct cpuidle_governor *gov)
>  #endif
>  
>  #define CPU_PM_CPU_IDLE_ENTER(low_level_idle_enter, idx)	\
> -({								\
> -	int __ret;						\
> -								\
> -	if (!idx) {						\
> -		cpu_do_idle();					\
> -		return idx;					\
> -	}							\
> -								\
> -	__ret = cpu_pm_enter();					\
> -	if (!__ret) {						\
> -		__ret = low_level_idle_enter(idx);		\
> -		cpu_pm_exit();					\
> -	}							\
> -								\
> -	__ret ? -1 : idx;					\
> +	__CPU_PM_CPU_IDLE_ENTER(low_level_idle_enter, idx, 0)
> +
> +#define CPU_PM_CPU_IDLE_ENTER_RETENTION(low_level_idle_enter, idx)	\
> +	__CPU_PM_CPU_IDLE_ENTER(low_level_idle_enter, idx, 1)
> +
> +#define __CPU_PM_CPU_IDLE_ENTER(low_level_idle_enter, idx, is_retention) \
> +({									\
> +	int __ret = 0;							\
> +									\
> +	if (!idx) {							\
> +		cpu_do_idle();						\
> +		return idx;						\
> +	}								\
> +									\
> +	if (!is_retention)						\
> +		__ret =  cpu_pm_enter();				\
> +	if (!__ret) {							\
> +		__ret = low_level_idle_enter(idx);			\
> +		if (!is_retention)					\
> +			cpu_pm_exit();					\
> +	}								\
> +									\
> +	__ret ? -1 : idx;						\
>  })
>  
>  #endif /* _LINUX_CPUIDLE_H */
> 

The ordering of the macros seems kind of backwards.  The inner-most
ones usually go first (which is more natural for a human reader too).

Thanks,
Rafael

[PATCH 1/2] cpuidle: Add new macro to enter a retention idle state

[Rafael J. Wysocki]

On Tuesday, November 7, 2017 6:35:32 PM CET Prashanth Prakash wrote:
> If a CPU is entering a low power idle state where it doesn't lose any
> context, then there is no need to call cpu_pm_enter()/cpu_pm_exit().
> Add a new macro(CPU_PM_CPU_IDLE_ENTER_RETENTION) to be used by cpuidle
> drivers when they are entering retention state. By not calling
> cpu_pm_enter and cpu_pm_exit we reduce the latency involved in
> entering and exiting the retention idle states.
> 
> On ARM64 based Qualcomm Server Platform we measured below overhead for
> for calling cpu_pm_enter and cpu_pm_exit for retention states.
> 
> workload: stress --hdd #CPUs --hdd-bytes 32M  -t 30
>         Average overhead of cpu_pm_enter - 1.2us
>         Average overhead of cpu_pm_exit  - 3.1us
> 
> Signed-off-by: Prashanth Prakash <pprakash@codeaurora.org>
> ---
>  include/linux/cpuidle.h | 38 +++++++++++++++++++++++---------------
>  1 file changed, 23 insertions(+), 15 deletions(-)
> 
> diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
> index 8f7788d..54cbd9d 100644
> --- a/include/linux/cpuidle.h
> +++ b/include/linux/cpuidle.h
> @@ -258,21 +258,29 @@ static inline int cpuidle_register_governor(struct cpuidle_governor *gov)
>  #endif
>  
>  #define CPU_PM_CPU_IDLE_ENTER(low_level_idle_enter, idx)	\
> -({								\
> -	int __ret;						\
> -								\
> -	if (!idx) {						\
> -		cpu_do_idle();					\
> -		return idx;					\
> -	}							\
> -								\
> -	__ret = cpu_pm_enter();					\
> -	if (!__ret) {						\
> -		__ret = low_level_idle_enter(idx);		\
> -		cpu_pm_exit();					\
> -	}							\
> -								\
> -	__ret ? -1 : idx;					\
> +	__CPU_PM_CPU_IDLE_ENTER(low_level_idle_enter, idx, 0)
> +
> +#define CPU_PM_CPU_IDLE_ENTER_RETENTION(low_level_idle_enter, idx)	\
> +	__CPU_PM_CPU_IDLE_ENTER(low_level_idle_enter, idx, 1)
> +
> +#define __CPU_PM_CPU_IDLE_ENTER(low_level_idle_enter, idx, is_retention) \
> +({									\
> +	int __ret = 0;							\
> +									\
> +	if (!idx) {							\
> +		cpu_do_idle();						\
> +		return idx;						\
> +	}								\
> +									\
> +	if (!is_retention)						\
> +		__ret =  cpu_pm_enter();				\
> +	if (!__ret) {							\
> +		__ret = low_level_idle_enter(idx);			\
> +		if (!is_retention)					\
> +			cpu_pm_exit();					\
> +	}								\
> +									\
> +	__ret ? -1 : idx;						\
>  })
>  
>  #endif /* _LINUX_CPUIDLE_H */
> 

The ordering of the macros seems kind of backwards.  The inner-most
ones usually go first (which is more natural for a human reader too).

Thanks,
Rafael

Re: [PATCH 1/2] cpuidle: Add new macro to enter a retention idle state

[Prakash, Prashanth]

On 11/8/2017 3:47 PM, Rafael J. Wysocki wrote:
> On Tuesday, November 7, 2017 6:35:32 PM CET Prashanth Prakash wrote:
>> If a CPU is entering a low power idle state where it doesn't lose any
>> context, then there is no need to call cpu_pm_enter()/cpu_pm_exit().
>> Add a new macro(CPU_PM_CPU_IDLE_ENTER_RETENTION) to be used by cpuidle
>> drivers when they are entering retention state. By not calling
>> cpu_pm_enter and cpu_pm_exit we reduce the latency involved in
>> entering and exiting the retention idle states.
>>
>> On ARM64 based Qualcomm Server Platform we measured below overhead for
>> for calling cpu_pm_enter and cpu_pm_exit for retention states.
>>
>> workload: stress --hdd #CPUs --hdd-bytes 32M  -t 30
>>         Average overhead of cpu_pm_enter - 1.2us
>>         Average overhead of cpu_pm_exit  - 3.1us
>>
>> Signed-off-by: Prashanth Prakash <pprakash@codeaurora.org>
>> ---
>>  include/linux/cpuidle.h | 38 +++++++++++++++++++++++---------------
>>  1 file changed, 23 insertions(+), 15 deletions(-)
>>
>> diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
>> index 8f7788d..54cbd9d 100644
>> --- a/include/linux/cpuidle.h
>> +++ b/include/linux/cpuidle.h
>> @@ -258,21 +258,29 @@ static inline int cpuidle_register_governor(struct cpuidle_governor *gov)
>>  #endif
>>  
>>  #define CPU_PM_CPU_IDLE_ENTER(low_level_idle_enter, idx)	\
>> -({								\
>> -	int __ret;						\
>> -								\
>> -	if (!idx) {						\
>> -		cpu_do_idle();					\
>> -		return idx;					\
>> -	}							\
>> -								\
>> -	__ret = cpu_pm_enter();					\
>> -	if (!__ret) {						\
>> -		__ret = low_level_idle_enter(idx);		\
>> -		cpu_pm_exit();					\
>> -	}							\
>> -								\
>> -	__ret ? -1 : idx;					\
>> +	__CPU_PM_CPU_IDLE_ENTER(low_level_idle_enter, idx, 0)
>> +
>> +#define CPU_PM_CPU_IDLE_ENTER_RETENTION(low_level_idle_enter, idx)	\
>> +	__CPU_PM_CPU_IDLE_ENTER(low_level_idle_enter, idx, 1)
>> +
>> +#define __CPU_PM_CPU_IDLE_ENTER(low_level_idle_enter, idx, is_retention) \
>> +({									\
>> +	int __ret = 0;							\
>> +									\
>> +	if (!idx) {							\
>> +		cpu_do_idle();						\
>> +		return idx;						\
>> +	}								\
>> +									\
>> +	if (!is_retention)						\
>> +		__ret =  cpu_pm_enter();				\
>> +	if (!__ret) {							\
>> +		__ret = low_level_idle_enter(idx);			\
>> +		if (!is_retention)					\
>> +			cpu_pm_exit();					\
>> +	}								\
>> +									\
>> +	__ret ? -1 : idx;						\
>>  })
>>  
>>  #endif /* _LINUX_CPUIDLE_H */
>>
> The ordering of the macros seems kind of backwards.  The inner-most
> ones usually go first (which is more natural for a human reader too).
I will change it and send out v2.

Thanks,
Prashanth

[PATCH 1/2] cpuidle: Add new macro to enter a retention idle state

[Prakash, Prashanth]

On 11/8/2017 3:47 PM, Rafael J. Wysocki wrote:
> On Tuesday, November 7, 2017 6:35:32 PM CET Prashanth Prakash wrote:
>> If a CPU is entering a low power idle state where it doesn't lose any
>> context, then there is no need to call cpu_pm_enter()/cpu_pm_exit().
>> Add a new macro(CPU_PM_CPU_IDLE_ENTER_RETENTION) to be used by cpuidle
>> drivers when they are entering retention state. By not calling
>> cpu_pm_enter and cpu_pm_exit we reduce the latency involved in
>> entering and exiting the retention idle states.
>>
>> On ARM64 based Qualcomm Server Platform we measured below overhead for
>> for calling cpu_pm_enter and cpu_pm_exit for retention states.
>>
>> workload: stress --hdd #CPUs --hdd-bytes 32M  -t 30
>>         Average overhead of cpu_pm_enter - 1.2us
>>         Average overhead of cpu_pm_exit  - 3.1us
>>
>> Signed-off-by: Prashanth Prakash <pprakash@codeaurora.org>
>> ---
>>  include/linux/cpuidle.h | 38 +++++++++++++++++++++++---------------
>>  1 file changed, 23 insertions(+), 15 deletions(-)
>>
>> diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
>> index 8f7788d..54cbd9d 100644
>> --- a/include/linux/cpuidle.h
>> +++ b/include/linux/cpuidle.h
>> @@ -258,21 +258,29 @@ static inline int cpuidle_register_governor(struct cpuidle_governor *gov)
>>  #endif
>>  
>>  #define CPU_PM_CPU_IDLE_ENTER(low_level_idle_enter, idx)	\
>> -({								\
>> -	int __ret;						\
>> -								\
>> -	if (!idx) {						\
>> -		cpu_do_idle();					\
>> -		return idx;					\
>> -	}							\
>> -								\
>> -	__ret = cpu_pm_enter();					\
>> -	if (!__ret) {						\
>> -		__ret = low_level_idle_enter(idx);		\
>> -		cpu_pm_exit();					\
>> -	}							\
>> -								\
>> -	__ret ? -1 : idx;					\
>> +	__CPU_PM_CPU_IDLE_ENTER(low_level_idle_enter, idx, 0)
>> +
>> +#define CPU_PM_CPU_IDLE_ENTER_RETENTION(low_level_idle_enter, idx)	\
>> +	__CPU_PM_CPU_IDLE_ENTER(low_level_idle_enter, idx, 1)
>> +
>> +#define __CPU_PM_CPU_IDLE_ENTER(low_level_idle_enter, idx, is_retention) \
>> +({									\
>> +	int __ret = 0;							\
>> +									\
>> +	if (!idx) {							\
>> +		cpu_do_idle();						\
>> +		return idx;						\
>> +	}								\
>> +									\
>> +	if (!is_retention)						\
>> +		__ret =  cpu_pm_enter();				\
>> +	if (!__ret) {							\
>> +		__ret = low_level_idle_enter(idx);			\
>> +		if (!is_retention)					\
>> +			cpu_pm_exit();					\
>> +	}								\
>> +									\
>> +	__ret ? -1 : idx;						\
>>  })
>>  
>>  #endif /* _LINUX_CPUIDLE_H */
>>
> The ordering of the macros seems kind of backwards.  The inner-most
> ones usually go first (which is more natural for a human reader too).
I will change it and send out v2.

Thanks,
Prashanth