Re: [PATCH v3 2/3] powerpc/powernv/idle: Rename pnv_first_spr_loss_level variable
Hi, On Wed, Jul 22, 2020 at 12:37:41AM +1000, Nicholas Piggin wrote: > Excerpts from Pratik Sampat's message of July 21, 2020 8:29 pm: > > > > > > On 20/07/20 5:27 am, Nicholas Piggin wrote: > >> Excerpts from Pratik Rajesh Sampat's message of July 18, 2020 4:53 am: > >>> Replace the variable name from using "pnv_first_spr_loss_level" to > >>> "pnv_first_fullstate_loss_level". > >>> > >>> As pnv_first_spr_loss_level is supposed to be the earliest state that > >>> has OPAL_PM_LOSE_FULL_CONTEXT set, however as shallow states too loose > >>> SPR values, render an incorrect terminology. > >> It also doesn't lose "full" state at this loss level though. From the > >> architecture it could be called "hv state loss level", but in POWER10 > >> even that is not strictly true. > >> > > Right. Just discovered that deep stop states won't loose full state > > P10 onwards. > > Would it better if we rename it as "pnv_all_spr_loss_state" instead > > so that it stays generic enough while being semantically coherent? > > It doesn't lose all SPRs. It does physically, but for Linux it appears > at least timebase SPRs are retained and that's mostly how it's > documented. > > Maybe there's no really good name for it, but we do call it "deep" stop > in other places, you could call it deep_spr_loss maybe. I don't mind too > much though, whatever Gautham is happy with. Nick's suggestion is fine by me. We can call it deep_spr_loss_state. > > Thanks, > Nick -- Thanks and Regards gautham.
Re: [PATCH v3 2/3] powerpc/powernv/idle: Rename pnv_first_spr_loss_level variable
Excerpts from Pratik Sampat's message of July 21, 2020 8:29 pm: > > > On 20/07/20 5:27 am, Nicholas Piggin wrote: >> Excerpts from Pratik Rajesh Sampat's message of July 18, 2020 4:53 am: >>> Replace the variable name from using "pnv_first_spr_loss_level" to >>> "pnv_first_fullstate_loss_level". >>> >>> As pnv_first_spr_loss_level is supposed to be the earliest state that >>> has OPAL_PM_LOSE_FULL_CONTEXT set, however as shallow states too loose >>> SPR values, render an incorrect terminology. >> It also doesn't lose "full" state at this loss level though. From the >> architecture it could be called "hv state loss level", but in POWER10 >> even that is not strictly true. >> > Right. Just discovered that deep stop states won't loose full state > P10 onwards. > Would it better if we rename it as "pnv_all_spr_loss_state" instead > so that it stays generic enough while being semantically coherent? It doesn't lose all SPRs. It does physically, but for Linux it appears at least timebase SPRs are retained and that's mostly how it's documented. Maybe there's no really good name for it, but we do call it "deep" stop in other places, you could call it deep_spr_loss maybe. I don't mind too much though, whatever Gautham is happy with. Thanks, Nick
Re: [PATCH v3 2/3] powerpc/powernv/idle: Rename pnv_first_spr_loss_level variable
On 20/07/20 5:27 am, Nicholas Piggin wrote:
Excerpts from Pratik Rajesh Sampat's message of July 18, 2020 4:53 am:
Replace the variable name from using "pnv_first_spr_loss_level" to
"pnv_first_fullstate_loss_level".
As pnv_first_spr_loss_level is supposed to be the earliest state that
has OPAL_PM_LOSE_FULL_CONTEXT set, however as shallow states too loose
SPR values, render an incorrect terminology.
It also doesn't lose "full" state at this loss level though. From the
architecture it could be called "hv state loss level", but in POWER10
even that is not strictly true.
Right. Just discovered that deep stop states won't loose full state
P10 onwards.
Would it better if we rename it as "pnv_all_spr_loss_state" instead
so that it stays generic enough while being semantically coherent?
Thanks
Pratik
Signed-off-by: Pratik Rajesh Sampat
---
arch/powerpc/platforms/powernv/idle.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/idle.c
b/arch/powerpc/platforms/powernv/idle.c
index f62904f70fc6..d439e11af101 100644
--- a/arch/powerpc/platforms/powernv/idle.c
+++ b/arch/powerpc/platforms/powernv/idle.c
@@ -48,7 +48,7 @@ static bool default_stop_found;
* First stop state levels when SPR and TB loss can occur.
*/
static u64 pnv_first_tb_loss_level = MAX_STOP_STATE + 1;
-static u64 pnv_first_spr_loss_level = MAX_STOP_STATE + 1;
+static u64 pnv_first_fullstate_loss_level = MAX_STOP_STATE + 1;
/*
* psscr value and mask of the deepest stop idle state.
@@ -657,7 +657,7 @@ static unsigned long power9_idle_stop(unsigned long psscr,
bool mmu_on)
*/
mmcr0 = mfspr(SPRN_MMCR0);
}
- if ((psscr & PSSCR_RL_MASK) >= pnv_first_spr_loss_level) {
+ if ((psscr & PSSCR_RL_MASK) >= pnv_first_fullstate_loss_level) {
sprs.lpcr = mfspr(SPRN_LPCR);
sprs.hfscr = mfspr(SPRN_HFSCR);
sprs.fscr = mfspr(SPRN_FSCR);
@@ -741,7 +741,7 @@ static unsigned long power9_idle_stop(unsigned long psscr,
bool mmu_on)
* just always test PSSCR for SPR/TB state loss.
*/
pls = (psscr & PSSCR_PLS) >> PSSCR_PLS_SHIFT;
- if (likely(pls < pnv_first_spr_loss_level)) {
+ if (likely(pls < pnv_first_fullstate_loss_level)) {
if (sprs_saved)
atomic_stop_thread_idle();
goto out;
@@ -1088,7 +1088,7 @@ static void __init pnv_power9_idle_init(void)
* the deepest loss-less (OPAL_PM_STOP_INST_FAST) stop state.
*/
pnv_first_tb_loss_level = MAX_STOP_STATE + 1;
- pnv_first_spr_loss_level = MAX_STOP_STATE + 1;
+ pnv_first_fullstate_loss_level = MAX_STOP_STATE + 1;
for (i = 0; i < nr_pnv_idle_states; i++) {
int err;
struct pnv_idle_states_t *state = _idle_states[i];
@@ -1099,8 +1099,8 @@ static void __init pnv_power9_idle_init(void)
pnv_first_tb_loss_level = psscr_rl;
if ((state->flags & OPAL_PM_LOSE_FULL_CONTEXT) &&
-(pnv_first_spr_loss_level > psscr_rl))
- pnv_first_spr_loss_level = psscr_rl;
+(pnv_first_fullstate_loss_level > psscr_rl))
+ pnv_first_fullstate_loss_level = psscr_rl;
/*
* The idle code does not deal with TB loss occurring
@@ -,8 +,8 @@ static void __init pnv_power9_idle_init(void)
* compatibility.
*/
if ((state->flags & OPAL_PM_TIMEBASE_STOP) &&
-(pnv_first_spr_loss_level > psscr_rl))
- pnv_first_spr_loss_level = psscr_rl;
+(pnv_first_fullstate_loss_level > psscr_rl))
+ pnv_first_fullstate_loss_level = psscr_rl;
err = validate_psscr_val_mask(>psscr_val,
>psscr_mask,
@@ -1158,7 +1158,7 @@ static void __init pnv_power9_idle_init(void)
}
pr_info("cpuidle-powernv: First stop level that may lose SPRs = 0x%llx\n",
- pnv_first_spr_loss_level);
+ pnv_first_fullstate_loss_level);
pr_info("cpuidle-powernv: First stop level that may lose timebase = 0x%llx\n",
pnv_first_tb_loss_level);
--
2.25.4
Re: [PATCH v3 2/3] powerpc/powernv/idle: Rename pnv_first_spr_loss_level variable
Excerpts from Pratik Rajesh Sampat's message of July 18, 2020 4:53 am:
> Replace the variable name from using "pnv_first_spr_loss_level" to
> "pnv_first_fullstate_loss_level".
>
> As pnv_first_spr_loss_level is supposed to be the earliest state that
> has OPAL_PM_LOSE_FULL_CONTEXT set, however as shallow states too loose
> SPR values, render an incorrect terminology.
It also doesn't lose "full" state at this loss level though. From the
architecture it could be called "hv state loss level", but in POWER10
even that is not strictly true.
>
> Signed-off-by: Pratik Rajesh Sampat
> ---
> arch/powerpc/platforms/powernv/idle.c | 18 +-
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/arch/powerpc/platforms/powernv/idle.c
> b/arch/powerpc/platforms/powernv/idle.c
> index f62904f70fc6..d439e11af101 100644
> --- a/arch/powerpc/platforms/powernv/idle.c
> +++ b/arch/powerpc/platforms/powernv/idle.c
> @@ -48,7 +48,7 @@ static bool default_stop_found;
> * First stop state levels when SPR and TB loss can occur.
> */
> static u64 pnv_first_tb_loss_level = MAX_STOP_STATE + 1;
> -static u64 pnv_first_spr_loss_level = MAX_STOP_STATE + 1;
> +static u64 pnv_first_fullstate_loss_level = MAX_STOP_STATE + 1;
>
> /*
> * psscr value and mask of the deepest stop idle state.
> @@ -657,7 +657,7 @@ static unsigned long power9_idle_stop(unsigned long
> psscr, bool mmu_on)
> */
> mmcr0 = mfspr(SPRN_MMCR0);
> }
> - if ((psscr & PSSCR_RL_MASK) >= pnv_first_spr_loss_level) {
> + if ((psscr & PSSCR_RL_MASK) >= pnv_first_fullstate_loss_level) {
> sprs.lpcr = mfspr(SPRN_LPCR);
> sprs.hfscr = mfspr(SPRN_HFSCR);
> sprs.fscr = mfspr(SPRN_FSCR);
> @@ -741,7 +741,7 @@ static unsigned long power9_idle_stop(unsigned long
> psscr, bool mmu_on)
>* just always test PSSCR for SPR/TB state loss.
>*/
> pls = (psscr & PSSCR_PLS) >> PSSCR_PLS_SHIFT;
> - if (likely(pls < pnv_first_spr_loss_level)) {
> + if (likely(pls < pnv_first_fullstate_loss_level)) {
> if (sprs_saved)
> atomic_stop_thread_idle();
> goto out;
> @@ -1088,7 +1088,7 @@ static void __init pnv_power9_idle_init(void)
>* the deepest loss-less (OPAL_PM_STOP_INST_FAST) stop state.
>*/
> pnv_first_tb_loss_level = MAX_STOP_STATE + 1;
> - pnv_first_spr_loss_level = MAX_STOP_STATE + 1;
> + pnv_first_fullstate_loss_level = MAX_STOP_STATE + 1;
> for (i = 0; i < nr_pnv_idle_states; i++) {
> int err;
> struct pnv_idle_states_t *state = _idle_states[i];
> @@ -1099,8 +1099,8 @@ static void __init pnv_power9_idle_init(void)
> pnv_first_tb_loss_level = psscr_rl;
>
> if ((state->flags & OPAL_PM_LOSE_FULL_CONTEXT) &&
> - (pnv_first_spr_loss_level > psscr_rl))
> - pnv_first_spr_loss_level = psscr_rl;
> + (pnv_first_fullstate_loss_level > psscr_rl))
> + pnv_first_fullstate_loss_level = psscr_rl;
>
> /*
>* The idle code does not deal with TB loss occurring
> @@ -,8 +,8 @@ static void __init pnv_power9_idle_init(void)
>* compatibility.
>*/
> if ((state->flags & OPAL_PM_TIMEBASE_STOP) &&
> - (pnv_first_spr_loss_level > psscr_rl))
> - pnv_first_spr_loss_level = psscr_rl;
> + (pnv_first_fullstate_loss_level > psscr_rl))
> + pnv_first_fullstate_loss_level = psscr_rl;
>
> err = validate_psscr_val_mask(>psscr_val,
> >psscr_mask,
> @@ -1158,7 +1158,7 @@ static void __init pnv_power9_idle_init(void)
> }
>
> pr_info("cpuidle-powernv: First stop level that may lose SPRs =
> 0x%llx\n",
> - pnv_first_spr_loss_level);
> + pnv_first_fullstate_loss_level);
>
> pr_info("cpuidle-powernv: First stop level that may lose timebase =
> 0x%llx\n",
> pnv_first_tb_loss_level);
> --
> 2.25.4
>
>
[PATCH v3 2/3] powerpc/powernv/idle: Rename pnv_first_spr_loss_level variable
Replace the variable name from using "pnv_first_spr_loss_level" to
"pnv_first_fullstate_loss_level".
As pnv_first_spr_loss_level is supposed to be the earliest state that
has OPAL_PM_LOSE_FULL_CONTEXT set, however as shallow states too loose
SPR values, render an incorrect terminology.
Signed-off-by: Pratik Rajesh Sampat
---
arch/powerpc/platforms/powernv/idle.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/idle.c
b/arch/powerpc/platforms/powernv/idle.c
index f62904f70fc6..d439e11af101 100644
--- a/arch/powerpc/platforms/powernv/idle.c
+++ b/arch/powerpc/platforms/powernv/idle.c
@@ -48,7 +48,7 @@ static bool default_stop_found;
* First stop state levels when SPR and TB loss can occur.
*/
static u64 pnv_first_tb_loss_level = MAX_STOP_STATE + 1;
-static u64 pnv_first_spr_loss_level = MAX_STOP_STATE + 1;
+static u64 pnv_first_fullstate_loss_level = MAX_STOP_STATE + 1;
/*
* psscr value and mask of the deepest stop idle state.
@@ -657,7 +657,7 @@ static unsigned long power9_idle_stop(unsigned long psscr,
bool mmu_on)
*/
mmcr0 = mfspr(SPRN_MMCR0);
}
- if ((psscr & PSSCR_RL_MASK) >= pnv_first_spr_loss_level) {
+ if ((psscr & PSSCR_RL_MASK) >= pnv_first_fullstate_loss_level) {
sprs.lpcr = mfspr(SPRN_LPCR);
sprs.hfscr = mfspr(SPRN_HFSCR);
sprs.fscr = mfspr(SPRN_FSCR);
@@ -741,7 +741,7 @@ static unsigned long power9_idle_stop(unsigned long psscr,
bool mmu_on)
* just always test PSSCR for SPR/TB state loss.
*/
pls = (psscr & PSSCR_PLS) >> PSSCR_PLS_SHIFT;
- if (likely(pls < pnv_first_spr_loss_level)) {
+ if (likely(pls < pnv_first_fullstate_loss_level)) {
if (sprs_saved)
atomic_stop_thread_idle();
goto out;
@@ -1088,7 +1088,7 @@ static void __init pnv_power9_idle_init(void)
* the deepest loss-less (OPAL_PM_STOP_INST_FAST) stop state.
*/
pnv_first_tb_loss_level = MAX_STOP_STATE + 1;
- pnv_first_spr_loss_level = MAX_STOP_STATE + 1;
+ pnv_first_fullstate_loss_level = MAX_STOP_STATE + 1;
for (i = 0; i < nr_pnv_idle_states; i++) {
int err;
struct pnv_idle_states_t *state = _idle_states[i];
@@ -1099,8 +1099,8 @@ static void __init pnv_power9_idle_init(void)
pnv_first_tb_loss_level = psscr_rl;
if ((state->flags & OPAL_PM_LOSE_FULL_CONTEXT) &&
-(pnv_first_spr_loss_level > psscr_rl))
- pnv_first_spr_loss_level = psscr_rl;
+(pnv_first_fullstate_loss_level > psscr_rl))
+ pnv_first_fullstate_loss_level = psscr_rl;
/*
* The idle code does not deal with TB loss occurring
@@ -,8 +,8 @@ static void __init pnv_power9_idle_init(void)
* compatibility.
*/
if ((state->flags & OPAL_PM_TIMEBASE_STOP) &&
-(pnv_first_spr_loss_level > psscr_rl))
- pnv_first_spr_loss_level = psscr_rl;
+(pnv_first_fullstate_loss_level > psscr_rl))
+ pnv_first_fullstate_loss_level = psscr_rl;
err = validate_psscr_val_mask(>psscr_val,
>psscr_mask,
@@ -1158,7 +1158,7 @@ static void __init pnv_power9_idle_init(void)
}
pr_info("cpuidle-powernv: First stop level that may lose SPRs =
0x%llx\n",
- pnv_first_spr_loss_level);
+ pnv_first_fullstate_loss_level);
pr_info("cpuidle-powernv: First stop level that may lose timebase =
0x%llx\n",
pnv_first_tb_loss_level);
--
2.25.4
