Re: [PATCH v3 3/3] powernv: Pass PSSCR value and mask to power9_idle_stop

[Gautham R Shenoy]

Hi Michael,

On Wed, Nov 23, 2016 at 08:51:10PM +1100, Michael Ellerman wrote:
> "Gautham R. Shenoy"  writes:
> 
> > From: "Gautham R. Shenoy" 
> >
> > The power9_idle_stop method currently takes only the requested stop
> > level as a parameter and picks up the rest of the PSSCR bits from a
> > hand-coded macro. This is not a very flexible design, especially when
> > the firmware has the capability to communicate the psscr value and the
> > mask associated with a particular stop state via device tree.
> >
> > This patch modifies the power9_idle_stop API to take as parameters the
> > PSSCR value and the PSSCR mask corresponding to the stop state that
> > needs to be set. These PSSCR value and mask are respectively obtained
> > by parsing the "ibm,cpu-idle-state-psscr" and
> > "ibm,cpu-idle-state-psscr-mask" fields from the device tree.
> >
> > In addition to this, the patch adds support for handling stop states
> > for which ESL and EC bits in the PSSCR are zero. As per the
> > architecture, a wakeup from these stop states resumes execution from
> > the subsequent instruction as opposed to waking up at the System
> > Vector.
> >
> > The older firmware sets only the Requested Level (RL) field in the
> > psscr and psscr-mask exposed in the device tree. For older firmware
> > where psscr-mask=0xf, this patch will set the default sane values that
> > the set for for remaining PSSCR fields (i.e PSLL, MTL, ESL, EC, and
> > TR).
> 
> So we're using psscr-mas=0xf as a signal that we're running on old
> firmware.
> 
> That's OK I think, but please send a patch to document it in the device
> tree binding.
> 
> And call it out below in the code.

Sure will do this! Thanks for reviewing the code.

> 
> cheers
> 

--
Thanks and Regards
gautham.

Re: [PATCH v3 3/3] powernv: Pass PSSCR value and mask to power9_idle_stop

[Michael Ellerman]

"Gautham R. Shenoy"  writes:

> From: "Gautham R. Shenoy" 
>
> The power9_idle_stop method currently takes only the requested stop
> level as a parameter and picks up the rest of the PSSCR bits from a
> hand-coded macro. This is not a very flexible design, especially when
> the firmware has the capability to communicate the psscr value and the
> mask associated with a particular stop state via device tree.
>
> This patch modifies the power9_idle_stop API to take as parameters the
> PSSCR value and the PSSCR mask corresponding to the stop state that
> needs to be set. These PSSCR value and mask are respectively obtained
> by parsing the "ibm,cpu-idle-state-psscr" and
> "ibm,cpu-idle-state-psscr-mask" fields from the device tree.
>
> In addition to this, the patch adds support for handling stop states
> for which ESL and EC bits in the PSSCR are zero. As per the
> architecture, a wakeup from these stop states resumes execution from
> the subsequent instruction as opposed to waking up at the System
> Vector.
>
> The older firmware sets only the Requested Level (RL) field in the
> psscr and psscr-mask exposed in the device tree. For older firmware
> where psscr-mask=0xf, this patch will set the default sane values that
> the set for for remaining PSSCR fields (i.e PSLL, MTL, ESL, EC, and
> TR).

So we're using psscr-mas=0xf as a signal that we're running on old
firmware.

That's OK I think, but please send a patch to document it in the device
tree binding.

And call it out below in the code.

cheers

[PATCH v3 3/3] powernv: Pass PSSCR value and mask to power9_idle_stop

[Gautham R. Shenoy]

From: "Gautham R. Shenoy" 

The power9_idle_stop method currently takes only the requested stop
level as a parameter and picks up the rest of the PSSCR bits from a
hand-coded macro. This is not a very flexible design, especially when
the firmware has the capability to communicate the psscr value and the
mask associated with a particular stop state via device tree.

This patch modifies the power9_idle_stop API to take as parameters the
PSSCR value and the PSSCR mask corresponding to the stop state that
needs to be set. These PSSCR value and mask are respectively obtained
by parsing the "ibm,cpu-idle-state-psscr" and
"ibm,cpu-idle-state-psscr-mask" fields from the device tree.

In addition to this, the patch adds support for handling stop states
for which ESL and EC bits in the PSSCR are zero. As per the
architecture, a wakeup from these stop states resumes execution from
the subsequent instruction as opposed to waking up at the System
Vector.

The older firmware sets only the Requested Level (RL) field in the
psscr and psscr-mask exposed in the device tree. For older firmware
where psscr-mask=0xf, this patch will set the default sane values that
the set for for remaining PSSCR fields (i.e PSLL, MTL, ESL, EC, and
TR).

This skiboot patch that exports fully populated PSSCR values and the
mask for all the stop states can be found here:
https://lists.ozlabs.org/pipermail/skiboot/2016-September/004869.html

Signed-off-by: Gautham R. Shenoy 
---
 arch/powerpc/include/asm/cpuidle.h   | 37 +++
 arch/powerpc/include/asm/processor.h |  3 +-
 arch/powerpc/kernel/idle_book3s.S| 31 +++-
 arch/powerpc/platforms/powernv/idle.c| 81 ++--
 arch/powerpc/platforms/powernv/powernv.h |  3 +-
 arch/powerpc/platforms/powernv/smp.c | 14 +++---
 drivers/cpuidle/cpuidle-powernv.c| 40 +++-
 7 files changed, 165 insertions(+), 44 deletions(-)

diff --git a/arch/powerpc/include/asm/cpuidle.h 
b/arch/powerpc/include/asm/cpuidle.h
index 0a3255b..41b5d27 100644
--- a/arch/powerpc/include/asm/cpuidle.h
+++ b/arch/powerpc/include/asm/cpuidle.h
@@ -10,11 +10,48 @@
 #define PNV_CORE_IDLE_LOCK_BIT  0x100
 #define PNV_CORE_IDLE_THREAD_BITS   0x0FF
 
+/*
+ * By default we set the ESL and EC bits in the PSSCR.
+ *
+ * The MTL and PSLL are set to the maximum value possible as per the
+ * ISA, i.e 15.
+ *
+ * The Transition Rate is set to the Maximum value 3.
+ */
+#define PSSCR_HV_DEFAULT_VAL(PSSCR_ESL | PSSCR_EC |\
+   PSSCR_PSLL_MASK | PSSCR_TR_MASK |   \
+   PSSCR_MTL_MASK)
+
+#define PSSCR_HV_DEFAULT_MASK   (PSSCR_ESL | PSSCR_EC |\
+   PSSCR_PSLL_MASK | PSSCR_TR_MASK |   \
+   PSSCR_MTL_MASK | PSSCR_RL_MASK)
+
 #ifndef __ASSEMBLY__
 extern u32 pnv_fastsleep_workaround_at_entry[];
 extern u32 pnv_fastsleep_workaround_at_exit[];
 
 extern u64 pnv_first_deep_stop_state;
+
+/*
+ * Older firmware only populates the RL field in psscr_val and
+ * psscr_mask.
+ *
+ * Set the remaining fields to the default value in case of the
+ * older firmware.
+ */
+static inline u64 compute_psscr_val(u64 psscr_val, u64 psscr_mask)
+{
+   if (psscr_mask == 0xf)
+   return psscr_val | PSSCR_HV_DEFAULT_VAL;
+   return psscr_val;
+}
+
+static inline u64 compute_psscr_mask(u64 psscr_mask)
+{
+   if (psscr_mask == 0xf)
+   return PSSCR_HV_DEFAULT_MASK;
+   return psscr_mask;
+}
 #endif
 
 #endif
diff --git a/arch/powerpc/include/asm/processor.h 
b/arch/powerpc/include/asm/processor.h
index c07c31b..422becd 100644
--- a/arch/powerpc/include/asm/processor.h
+++ b/arch/powerpc/include/asm/processor.h
@@ -458,7 +458,8 @@ static inline unsigned long get_clean_sp(unsigned long sp, 
int is_32)
 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_psscr_val,
+ unsigned long stop_psscr_mask);
 
 extern void flush_instruction_cache(void);
 extern void hard_reset_now(void);
diff --git a/arch/powerpc/kernel/idle_book3s.S 
b/arch/powerpc/kernel/idle_book3s.S
index be90e2f..37ee533 100644
--- a/arch/powerpc/kernel/idle_book3s.S
+++ b/arch/powerpc/kernel/idle_book3s.S
@@ -40,9 +40,7 @@
 #define _WORC  GPR11
 #define _PTCR  GPR12
 
-#define PSSCR_HV_TEMPLATE  PSSCR_ESL | PSSCR_EC | \
-   PSSCR_PSLL_MASK | PSSCR_TR_MASK | \
-   PSSCR_MTL_MASK
+#define PSSCR_EC_ESL_MASK_SHIFTED  (PSSCR_EC | PSSCR_ESL) >> 16
 
.text
 
@@ -264,7 +262,7 @@ enter_winkle:
IDLE_STATE_ENTER_SEQ_NORET(PPC_WINKLE)
 
 /*
- * r3 - requested stop state
+