| Message ID | 1510187922-8218-3-git-send-email-pprakash@codeaurora.org (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
On 09/11/17 00:38, 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)) > + This is fine, but just to be safer, is it better to check for all flags to be set as we can't/don't support any partial retention modes. Just curious, how is retention handled on mobile parts. I guess mainline is not important on those parts, but add a note in the commit message that we can make it ACPI agnostic and PSCI param dependent if needed.
On 11/13/2017 5:33 AM, Sudeep Holla wrote: > > On 09/11/17 00:38, 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)) >> + > This is fine, but just to be safer, is it better to check for all > flags to be set as we can't/don't support any partial retention modes. I am not sure I completely understand. If any bit is set in arch_flags, then we lose the context corresponding to that bit, so a full retention state will have no bit set, so that's what we are checking for. Or am i missing something? Thanks, Prashanth
On 14/11/17 16:15, Prakash, Prashanth wrote: > > > On 11/13/2017 5:33 AM, Sudeep Holla wrote: >> >> On 09/11/17 00:38, 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)) >>> + >> This is fine, but just to be safer, is it better to check for all >> flags to be set as we can't/don't support any partial retention modes. > I am not sure I completely understand. > > If any bit is set in arch_flags, then we lose the context corresponding to that bit, so > a full retention state will have no bit set, so that's what we are checking for. > Or am i missing something? > Ah you are right, I somehow misinterpreted the flags exactly to be opposite(i.e. state retained) rather than state lost, sorry for the noise.
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
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(-)