Re: [RFC 3/3] powernv/cpuidle : Introduce capability for firmware-enabled-stop

2020-04-27 Thread Gautham R Shenoy
On Sun, Apr 26, 2020 at 09:10:27PM -0500, Abhishek Goel wrote:
> This patch introduces the capability for firmware to handle the stop
> states instead. A bit is set based on the discovery of the feature
> and correspondingly also the responsibility to handle the stop states.
> 
> If Kernel does not know about stop version, it can fallback to opal for
> idle stop support if firmware-stop-supported property is present.
> 
> Earlier this patch was posted as part of this series :
> https://lkml.org/lkml/2020/3/4/589
> 
> Signed-off-by: Pratik Rajesh Sampat 
> Signed-off-by: Abhishek Goel 
> ---
> 
>  v1->v2: This patch is newly added in this series.
> 
>  arch/powerpc/include/asm/processor.h  |  1 +
>  arch/powerpc/kernel/dt_cpu_ftrs.c |  8 
>  arch/powerpc/platforms/powernv/idle.c | 27 ---
>  3 files changed, 25 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/processor.h 
> b/arch/powerpc/include/asm/processor.h
> index 66fa20476d0e..d5c6532b11ef 100644
> --- a/arch/powerpc/include/asm/processor.h
> +++ b/arch/powerpc/include/asm/processor.h
> @@ -430,6 +430,7 @@ extern unsigned long cpuidle_disable;
>  enum idle_boot_override {IDLE_NO_OVERRIDE = 0, IDLE_POWERSAVE_OFF};
> 
>  #define STOP_ENABLE  0x0001
> +#define FIRMWARE_STOP_ENABLE 0x0010
> 
>  #define STOP_VERSION_P9   0x1
> 
> diff --git a/arch/powerpc/kernel/dt_cpu_ftrs.c 
> b/arch/powerpc/kernel/dt_cpu_ftrs.c
> index db1a525e090d..ff4a87b79702 100644
> --- a/arch/powerpc/kernel/dt_cpu_ftrs.c
> +++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
> @@ -298,6 +298,13 @@ static int __init feat_enable_idle_stop(struct 
> dt_cpu_feature *f)
>   return 1;
>  }
> 
> +static int __init feat_enable_firmware_stop(struct dt_cpu_feature *f)
> +{
> + stop_dep.cpuidle_prop |= FIRMWARE_STOP_ENABLE;
> +
> + return 1;
> +}
> +
>  static int __init feat_enable_mmu_hash(struct dt_cpu_feature *f)
>  {
>   u64 lpcr;
> @@ -592,6 +599,7 @@ static struct dt_cpu_feature_match __initdata
>   {"idle-nap", feat_enable_idle_nap, 0},
>   {"alignment-interrupt-dsisr", feat_enable_align_dsisr, 0},
>   {"idle-stop", feat_enable_idle_stop, 0},
> + {"firmware-stop-supported", feat_enable_firmware_stop, 0},
>   {"machine-check-power8", feat_enable_mce_power8, 0},
>   {"performance-monitor-power8", feat_enable_pmu_power8, 0},
>   {"data-stream-control-register", feat_enable_dscr, CPU_FTR_DSCR},
> diff --git a/arch/powerpc/platforms/powernv/idle.c 
> b/arch/powerpc/platforms/powernv/idle.c
> index 538f0842ac3f..0de5de81902e 100644
> --- a/arch/powerpc/platforms/powernv/idle.c
> +++ b/arch/powerpc/platforms/powernv/idle.c
> @@ -633,16 +633,6 @@ static unsigned long power9_idle_stop(unsigned long 
> psscr, bool mmu_on)
>   unsigned long mmcr0 = 0;
>   struct p9_sprs sprs = {}; /* avoid false used-uninitialised */
>   bool sprs_saved = false;
> - int rc = 0;
> -
> - /*
> -  * Kernel takes decision whether to make OPAL call or not. This logic
> -  * will be combined with the logic for BE opal to take decision.
> -  */
> - if (firmware_stop_supported) {
> - rc = opal_cpu_idle(cpu_to_be64(__pa()), (uint64_t) psscr);
> - goto out;
> - }
> 
>   if (!(psscr & (PSSCR_EC|PSSCR_ESL))) {
>   /* EC=ESL=0 case */
> @@ -835,6 +825,19 @@ static unsigned long power9_idle_stop(unsigned long 
> psscr, bool mmu_on)
>   return srr1;
>  }
> 
> +static unsigned long power9_firmware_idle_stop(unsigned long psscr, bool 
> mmu_on)
> +{
> + unsigned long srr1;
> + int rc;
> +
> + rc = opal_cpu_idle(cpu_to_be64(__pa()), (uint64_t) psscr);
> +
> + if (mmu_on)
> + mtmsr(MSR_KERNEL);
> + return srr1;
> +
> +}
> +
>  #ifdef CONFIG_HOTPLUG_CPU
>  static unsigned long power9_offline_stop(unsigned long psscr)
>  {
> @@ -1394,9 +1397,11 @@ static int __init pnv_init_idle_states(void)
>   !(stop_dep.cpuidle_prop & STOP_ENABLE))
>   goto out;
> 
> - /* Check for supported version in kernel */
> + /* Check for supported version in kernel or fallback to kernel*/
>   if (stop_dep.stop_version & STOP_VERSION_P9) {
>   stop_dep.idle_stop = power9_idle_stop;
> + } else if (stop_dep.cpuidle_prop & FIRMWARE_STOP_ENABLE) {
> + stop_dep.idle_stop = power9_firmware_idle_stop;

Ok, so in this patch you first check if the "idle-stop" feature is
available. Only otherwise you fallback to the OPAL based cpuidle
driver.

This looks ok to me.


>   } else {
>   stop_dep.idle_stop = NULL;
>   goto out;
> -- 
> 2.17.1
> 

--
Thanks and Regards
gautham.


[RFC 3/3] powernv/cpuidle : Introduce capability for firmware-enabled-stop

2020-04-26 Thread Abhishek Goel
This patch introduces the capability for firmware to handle the stop
states instead. A bit is set based on the discovery of the feature
and correspondingly also the responsibility to handle the stop states.

If Kernel does not know about stop version, it can fallback to opal for
idle stop support if firmware-stop-supported property is present.

Earlier this patch was posted as part of this series :
https://lkml.org/lkml/2020/3/4/589

Signed-off-by: Pratik Rajesh Sampat 
Signed-off-by: Abhishek Goel 
---

 v1->v2: This patch is newly added in this series.

 arch/powerpc/include/asm/processor.h  |  1 +
 arch/powerpc/kernel/dt_cpu_ftrs.c |  8 
 arch/powerpc/platforms/powernv/idle.c | 27 ---
 3 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/arch/powerpc/include/asm/processor.h 
b/arch/powerpc/include/asm/processor.h
index 66fa20476d0e..d5c6532b11ef 100644
--- a/arch/powerpc/include/asm/processor.h
+++ b/arch/powerpc/include/asm/processor.h
@@ -430,6 +430,7 @@ extern unsigned long cpuidle_disable;
 enum idle_boot_override {IDLE_NO_OVERRIDE = 0, IDLE_POWERSAVE_OFF};
 
 #define STOP_ENABLE0x0001
+#define FIRMWARE_STOP_ENABLE   0x0010
 
 #define STOP_VERSION_P9   0x1
 
diff --git a/arch/powerpc/kernel/dt_cpu_ftrs.c 
b/arch/powerpc/kernel/dt_cpu_ftrs.c
index db1a525e090d..ff4a87b79702 100644
--- a/arch/powerpc/kernel/dt_cpu_ftrs.c
+++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
@@ -298,6 +298,13 @@ static int __init feat_enable_idle_stop(struct 
dt_cpu_feature *f)
return 1;
 }
 
+static int __init feat_enable_firmware_stop(struct dt_cpu_feature *f)
+{
+   stop_dep.cpuidle_prop |= FIRMWARE_STOP_ENABLE;
+
+   return 1;
+}
+
 static int __init feat_enable_mmu_hash(struct dt_cpu_feature *f)
 {
u64 lpcr;
@@ -592,6 +599,7 @@ static struct dt_cpu_feature_match __initdata
{"idle-nap", feat_enable_idle_nap, 0},
{"alignment-interrupt-dsisr", feat_enable_align_dsisr, 0},
{"idle-stop", feat_enable_idle_stop, 0},
+   {"firmware-stop-supported", feat_enable_firmware_stop, 0},
{"machine-check-power8", feat_enable_mce_power8, 0},
{"performance-monitor-power8", feat_enable_pmu_power8, 0},
{"data-stream-control-register", feat_enable_dscr, CPU_FTR_DSCR},
diff --git a/arch/powerpc/platforms/powernv/idle.c 
b/arch/powerpc/platforms/powernv/idle.c
index 538f0842ac3f..0de5de81902e 100644
--- a/arch/powerpc/platforms/powernv/idle.c
+++ b/arch/powerpc/platforms/powernv/idle.c
@@ -633,16 +633,6 @@ static unsigned long power9_idle_stop(unsigned long psscr, 
bool mmu_on)
unsigned long mmcr0 = 0;
struct p9_sprs sprs = {}; /* avoid false used-uninitialised */
bool sprs_saved = false;
-   int rc = 0;
-
-   /*
-* Kernel takes decision whether to make OPAL call or not. This logic
-* will be combined with the logic for BE opal to take decision.
-*/
-   if (firmware_stop_supported) {
-   rc = opal_cpu_idle(cpu_to_be64(__pa()), (uint64_t) psscr);
-   goto out;
-   }
 
if (!(psscr & (PSSCR_EC|PSSCR_ESL))) {
/* EC=ESL=0 case */
@@ -835,6 +825,19 @@ static unsigned long power9_idle_stop(unsigned long psscr, 
bool mmu_on)
return srr1;
 }
 
+static unsigned long power9_firmware_idle_stop(unsigned long psscr, bool 
mmu_on)
+{
+   unsigned long srr1;
+   int rc;
+
+   rc = opal_cpu_idle(cpu_to_be64(__pa()), (uint64_t) psscr);
+
+   if (mmu_on)
+   mtmsr(MSR_KERNEL);
+   return srr1;
+
+}
+
 #ifdef CONFIG_HOTPLUG_CPU
 static unsigned long power9_offline_stop(unsigned long psscr)
 {
@@ -1394,9 +1397,11 @@ static int __init pnv_init_idle_states(void)
!(stop_dep.cpuidle_prop & STOP_ENABLE))
goto out;
 
-   /* Check for supported version in kernel */
+   /* Check for supported version in kernel or fallback to kernel*/
if (stop_dep.stop_version & STOP_VERSION_P9) {
stop_dep.idle_stop = power9_idle_stop;
+   } else if (stop_dep.cpuidle_prop & FIRMWARE_STOP_ENABLE) {
+   stop_dep.idle_stop = power9_firmware_idle_stop;
} else {
stop_dep.idle_stop = NULL;
goto out;
-- 
2.17.1