powernv: Implement lite variant of stop with ESL=EC=0

From: "Gautham R. Shenoy" <[email protected]>

This patch adds a function named power_enter_stop_lite() that can execute a stop instruction when ESL and EC bits are set to zero in the PSSCR. The function handles the wake-up from idle at the instruction immediately after the stop instruction.

If the flag OPAL_PM_WAKEUP_AT_NEXT_INST[1] is set in the device tree for a stop state, then use the lite variant for that particular stop state.

[1] : The corresponding patch in skiboot that defines OPAL_PM_WAKEUP_AT_NEXT_INST and enables it in the device tree can be found here:
https://lists.ozlabs.org/pipermail/skiboot/2016-September/004805.html

Signed-off-by Gautham R. Shenoy <[email protected]>

---
 arch/powerpc/include/asm/opal-api.h   |  1 +
 arch/powerpc/include/asm/processor.h  |  3 ++-
 arch/powerpc/kernel/idle_book3s.S     | 28 +++++++++++++++++++++++++---
 arch/powerpc/platforms/powernv/idle.c | 17 ++++++++++++++---
 arch/powerpc/platforms/powernv/smp.c  |  2 +-
 drivers/cpuidle/cpuidle-powernv.c     | 24 ++++++++++++++++++++++--
 6 files changed, 65 insertions(+), 10 deletions(-)

arch/powerpc/include/asm/opal-api.h +1 -0

index 0e2e57b..6e5741e 100644
@@ -179,6 +179,7 @@
 #define OPAL_PM_TIMEBASE_STOP		0x00000002
 #define OPAL_PM_LOSE_HYP_CONTEXT	0x00002000
 #define OPAL_PM_LOSE_FULL_CONTEXT	0x00004000
+#define OPAL_PM_WAKEUP_AT_NEXT_INST	0x00008000
 #define OPAL_PM_NAP_ENABLED		0x00010000
 #define OPAL_PM_SLEEP_ENABLED		0x00020000
 #define OPAL_PM_WINKLE_ENABLED		0x00040000

arch/powerpc/include/asm/processor.h +2 -1

index 68e3bf5..e0549a0 100644
@@ -460,7 +460,8 @@ extern int powersave_nap;	/* set if nap mode can be used in idle loop */
 extern unsigned long power7_nap(int check_irq);
 extern unsigned long power7_sleep(void);
 extern unsigned long power7_winkle(void);
-extern unsigned long power9_idle_stop(unsigned long stop_level);
+extern unsigned long power9_idle_stop(unsigned long stop_level,
+				unsigned long exec_lite);
 
 extern void flush_instruction_cache(void);
 extern void hard_reset_now(void);

arch/powerpc/kernel/idle_book3s.S +25 -3

index 32d666b..47ee106 100644
@@ -43,6 +43,8 @@
 #define PSSCR_HV_TEMPLATE	PSSCR_ESL | PSSCR_EC | \
 				PSSCR_PSLL_MASK | PSSCR_TR_MASK | \
 				PSSCR_MTL_MASK
+#define PSSCR_HV_TEMPLATE_LITE	PSSCR_PSLL_MASK | PSSCR_TR_MASK | \
+				 PSSCR_MTL_MASK
 
 	.text
 

@@ -246,6 +248,20 @@ enter_winkle:
 
 	IDLE_STATE_ENTER_SEQ_NORET(PPC_WINKLE)
 
+
+/*
+ * power_enter_stop_lite : This will resume the wake up from
+ * idle at the subsequent instruction.
+ *
+ * Caller should set ESL=EC=0 in PSSCR before calling
+ * this function.
+ *
+ */
+power_enter_stop_lite:
+	IDLE_STATE_ENTER_SEQ(PPC_STOP)
+7:	li	r3,0  /* Since we didn't lose state, return 0 */
+	b 	pnv_wakeup_noloss
+
 /*
  * r3 - requested stop state
  */

@@ -333,13 +349,19 @@ ALT_FTR_SECTION_END_NESTED_IFSET(CPU_FTR_ARCH_207S, 66);		\
 
 /*
  * r3 - requested stop state
+ * r4 - Indicates if the lite variant with ESL=EC=0 should be executed.
  */
 _GLOBAL(power9_idle_stop)
-	LOAD_REG_IMMEDIATE(r4, PSSCR_HV_TEMPLATE)
-	or	r4,r4,r3
+	cmpdi	r4, 1
+	bne	4f
+	LOAD_REG_IMMEDIATE(r4, PSSCR_HV_TEMPLATE_LITE)
+	LOAD_REG_ADDR(r5,power_enter_stop_lite)
+	b 	5f
+4:	LOAD_REG_IMMEDIATE(r4, PSSCR_HV_TEMPLATE)
+	LOAD_REG_ADDR(r5,power_enter_stop)
+5:	or	r4,r4,r3
 	mtspr	SPRN_PSSCR, r4
 	li	r4, 1
-	LOAD_REG_ADDR(r5,power_enter_stop)
 	b	pnv_powersave_common
 	/* No return */
 /*

arch/powerpc/platforms/powernv/idle.c +8 -1

index 479c256..c3d3fed 100644
@@ -244,8 +244,15 @@ static DEVICE_ATTR(fastsleep_workaround_applyonce, 0600,
 static void power9_idle(void)
 {
 	/* Requesting stop state 0 */
-	power9_idle_stop(0);
+	power9_idle_stop(0, 0);
 }
+
+static void power9_idle_lite(void)
+{
+	/* Requesting stop state 0 with ESL=EC=0 */
+	power9_idle_stop(0, 1);
+}
+
 /*
  * First deep stop state. Used to figure out when to save/restore
  * hypervisor context.

@@ -414,8 +421,12 @@ static int __init pnv_init_idle_states(void)
 

Quoted 23 lines

arch/powerpc/platforms/powernv/idle.c +8 -1

index 479c256..c3d3fed 100644
@@ -244,8 +244,15 @@ static DEVICE_ATTR(fastsleep_workaround_applyonce, 0600,
 static void power9_idle(void)
 {
 	/* Requesting stop state 0 */
-	power9_idle_stop(0);
+	power9_idle_stop(0, 0);
 }
+
+static void power9_idle_lite(void)
+{
+	/* Requesting stop state 0 with ESL=EC=0 */
+	power9_idle_stop(0, 1);
+}
+
 /*
  * First deep stop state. Used to figure out when to save/restore
  * hypervisor context.

@@ -414,8 +421,12 @@ static int __init pnv_init_idle_states(void)
 

arch/powerpc/platforms/powernv/idle.c static int __init pnv_init_idle_states(void) · line 429

Quoted 10 lines

arch/powerpc/platforms/powernv/idle.c +6 -2

 	if (supported_cpuidle_states & OPAL_PM_NAP_ENABLED)
 		ppc_md.power_save = power7_idle;
-	else if (supported_cpuidle_states & OPAL_PM_STOP_INST_FAST)
-		ppc_md.power_save = power9_idle;
+	else if (supported_cpuidle_states & OPAL_PM_STOP_INST_FAST) {
+		if (supported_cpuidle_states & OPAL_PM_WAKEUP_AT_NEXT_INST)
+			ppc_md.power_save = power9_idle_lite;
+		else
+			ppc_md.power_save = power9_idle;
+	}

If I am reading this correctly we decide at boot time whether we support wakeup at next instruction and make that the default sleep state. I am a little surprised that these are exclusive. I was expecting power9_idle_lite to be one of the states to go into before power9_idle

Balbir Singh.

From: "Gautham R. Shenoy" <[email protected]>

This patch adds a function named power_enter_stop_lite() that can execute a stop instruction when ESL and EC bits are set to zero in the PSSCR. The function handles the wake-up from idle at the instruction immediately after the stop instruction.

If the flag OPAL_PM_WAKEUP_AT_NEXT_INST[1] is set in the device tree for a stop state, then use the lite variant for that particular stop state.

[1] : The corresponding patch in skiboot that defines OPAL_PM_WAKEUP_AT_NEXT_INST and enables it in the device tree can be found here:
https://lists.ozlabs.org/pipermail/skiboot/2016-September/004805.html

arch/powerpc/kernel/idle_book3s.S +9 -3

@@ -333,13 +349,19 @@ ALT_FTR_SECTION_END_NESTED_IFSET(CPU_FTR_ARCH_207S, 66);		\
 
 /*
  * r3 - requested stop state
+ * r4 - Indicates if the lite variant with ESL=EC=0 should be executed.
  */
 _GLOBAL(power9_idle_stop)
-	LOAD_REG_IMMEDIATE(r4, PSSCR_HV_TEMPLATE)
-	or	r4,r4,r3
+	cmpdi	r4, 1
+	bne	4f
+	LOAD_REG_IMMEDIATE(r4, PSSCR_HV_TEMPLATE_LITE)
+	LOAD_REG_ADDR(r5,power_enter_stop_lite)
+	b 	5f
+4:	LOAD_REG_IMMEDIATE(r4, PSSCR_HV_TEMPLATE)
+	LOAD_REG_ADDR(r5,power_enter_stop)
+5:	or	r4,r4,r3
 	mtspr	SPRN_PSSCR, r4
 	li	r4, 1
-	LOAD_REG_ADDR(r5,power_enter_stop)
 	b	pnv_powersave_common
 	/* No return */
 /*

arch/powerpc/platforms/powernv/idle.c +0 -0

index 479c256..c3d3fed 100644

Quoted 10 lines · Gautham R. Shenoy

From: "Gautham R. Shenoy" <[email protected]>

This patch adds a function named power_enter_stop_lite() that can execute a stop instruction when ESL and EC bits are set to zero in the PSSCR. The function handles the wake-up from idle at the instruction immediately after the stop instruction.

If the flag OPAL_PM_WAKEUP_AT_NEXT_INST[1] is set in the device tree for a stop state, then use the lite variant for that particular stop state.

Hi Gautham,

It seems to me that this would be cleaner if it was modelled as a new stop state? Surely it must have different power saving characteristics to the existing state?

Quoted 5 lines

[1] : The corresponding patch in skiboot that defines OPAL_PM_WAKEUP_AT_NEXT_INST and enables it in the device tree can be found here:
https://lists.ozlabs.org/pipermail/skiboot/2016-September/004805.html

Which says "This will reduce the exit latency of the idle state", and yet it doesn't change any of the latency/residency values?

If all it does is reduce the exit latency, then shouldn't we always use it? Presumably it also saves less power?

arch/powerpc/kernel/idle_book3s.S line 47

Quoted 11 lines

arch/powerpc/kernel/idle_book3s.S +2 -0

index 32d666b..47ee106 100644
@@ -43,6 +43,8 @@
 #define PSSCR_HV_TEMPLATE	PSSCR_ESL | PSSCR_EC | \
 				PSSCR_PSLL_MASK | PSSCR_TR_MASK | \
 				PSSCR_MTL_MASK
+#define PSSCR_HV_TEMPLATE_LITE	PSSCR_PSLL_MASK | PSSCR_TR_MASK | \
+				 PSSCR_MTL_MASK

Why do we have these at all? Firmware tells us the PSSCR values to use in the "ibm,cpu-idle-state-psscr" property.

If we just used what firmware gave us then we wouldn't need the above, or the juggling below.

Quoted 30 lines

arch/powerpc/kernel/idle_book3s.S +9 -3

@@ -333,13 +349,19 @@ ALT_FTR_SECTION_END_NESTED_IFSET(CPU_FTR_ARCH_207S, 66);		\
 
 /*
  * r3 - requested stop state
+ * r4 - Indicates if the lite variant with ESL=EC=0 should be executed.
  */
 _GLOBAL(power9_idle_stop)
-	LOAD_REG_IMMEDIATE(r4, PSSCR_HV_TEMPLATE)
-	or	r4,r4,r3
+	cmpdi	r4, 1
+	bne	4f
+	LOAD_REG_IMMEDIATE(r4, PSSCR_HV_TEMPLATE_LITE)
+	LOAD_REG_ADDR(r5,power_enter_stop_lite)
+	b 	5f
+4:	LOAD_REG_IMMEDIATE(r4, PSSCR_HV_TEMPLATE)
+	LOAD_REG_ADDR(r5,power_enter_stop)
+5:	or	r4,r4,r3
 	mtspr	SPRN_PSSCR, r4
 	li	r4, 1
-	LOAD_REG_ADDR(r5,power_enter_stop)
 	b	pnv_powersave_common
 	/* No return */
 /*

arch/powerpc/platforms/powernv/idle.c +0 -0

index 479c256..c3d3fed 100644

arch/powerpc/platforms/powernv/idle.c static DEVICE_ATTR(fastsleep_workaround_applyonce, 0600, · line 247

Quoted 5 lines

arch/powerpc/platforms/powernv/idle.c +0 -1

@@ -244,8 +244,15 @@ static DEVICE_ATTR(fastsleep_workaround_applyonce, 0600,
 static void power9_idle(void)
 {
 	/* Requesting stop state 0 */
-	power9_idle_stop(0);
 }

That seems like the root of the problem, why aren't we passing a PSSCR value here?

I also don't see us using the psscr-mask property anywhere. Why isn't that a bug?

cheers