* [PATCH v3 1/3] powerpc/powernv/idle: Replace CPU features checks with PVR checks
2020-07-17 18:53 [PATCH v3 0/3] powernv/idle: Power9 idle cleanup Pratik Rajesh Sampat
@ 2020-07-17 18:53 ` Pratik Rajesh Sampat
2020-07-20 0:00 ` Nicholas Piggin
2020-07-17 18:53 ` [PATCH v3 2/3] powerpc/powernv/idle: Rename pnv_first_spr_loss_level variable Pratik Rajesh Sampat
2020-07-17 18:53 ` [PATCH v3 3/3] powerpc/powernv/idle: Exclude mfspr on HID1, 4, 5 " Pratik Rajesh Sampat
2 siblings, 1 reply; 14+ messages in thread
From: Pratik Rajesh Sampat @ 2020-07-17 18:53 UTC (permalink / raw)
To: mpe, npiggin, benh, paulus, mikey, ego, svaidy, psampat,
pratik.r.sampat, linuxppc-dev, linux-kernel
As the idle framework's architecture is incomplete, hence instead of
checking for just the processor type advertised in the device tree CPU
features; check for the Processor Version Register (PVR) so that finer
granularity can be leveraged while making processor checks.
Signed-off-by: Pratik Rajesh Sampat <psampat@linux.ibm.com>
---
arch/powerpc/platforms/powernv/idle.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c
index 2dd467383a88..f62904f70fc6 100644
--- a/arch/powerpc/platforms/powernv/idle.c
+++ b/arch/powerpc/platforms/powernv/idle.c
@@ -92,7 +92,7 @@ static int pnv_save_sprs_for_deep_states(void)
if (rc != 0)
return rc;
- if (cpu_has_feature(CPU_FTR_ARCH_300)) {
+ if (pvr_version_is(PVR_POWER9)) {
rc = opal_slw_set_reg(pir, P9_STOP_SPR_MSR, msr_val);
if (rc)
return rc;
@@ -116,7 +116,7 @@ static int pnv_save_sprs_for_deep_states(void)
return rc;
/* Only p8 needs to set extra HID regiters */
- if (!cpu_has_feature(CPU_FTR_ARCH_300)) {
+ if (!pvr_version_is(PVR_POWER9)) {
rc = opal_slw_set_reg(pir, SPRN_HID1, hid1_val);
if (rc != 0)
@@ -971,7 +971,7 @@ unsigned long pnv_cpu_offline(unsigned int cpu)
__ppc64_runlatch_off();
- if (cpu_has_feature(CPU_FTR_ARCH_300) && deepest_stop_found) {
+ if (pvr_version_is(PVR_POWER9) && deepest_stop_found) {
unsigned long psscr;
psscr = mfspr(SPRN_PSSCR);
@@ -1175,7 +1175,7 @@ static void __init pnv_disable_deep_states(void)
pr_warn("cpuidle-powernv: Disabling idle states that lose full context\n");
pr_warn("cpuidle-powernv: Idle power-savings, CPU-Hotplug affected\n");
- if (cpu_has_feature(CPU_FTR_ARCH_300) &&
+ if (pvr_version_is(PVR_POWER9) &&
(pnv_deepest_stop_flag & OPAL_PM_LOSE_FULL_CONTEXT)) {
/*
* Use the default stop state for CPU-Hotplug
@@ -1205,7 +1205,7 @@ static void __init pnv_probe_idle_states(void)
return;
}
- if (cpu_has_feature(CPU_FTR_ARCH_300))
+ if (pvr_version_is(PVR_POWER9))
pnv_power9_idle_init();
for (i = 0; i < nr_pnv_idle_states; i++)
@@ -1278,7 +1278,7 @@ static int pnv_parse_cpuidle_dt(void)
pnv_idle_states[i].residency_ns = temp_u32[i];
/* For power9 */
- if (cpu_has_feature(CPU_FTR_ARCH_300)) {
+ if (pvr_version_is(PVR_POWER9)) {
/* Read pm_crtl_val */
if (of_property_read_u64_array(np, "ibm,cpu-idle-state-psscr",
temp_u64, nr_idle_states)) {
@@ -1337,7 +1337,7 @@ static int __init pnv_init_idle_states(void)
if (cpu == cpu_first_thread_sibling(cpu))
p->idle_state = (1 << threads_per_core) - 1;
- if (!cpu_has_feature(CPU_FTR_ARCH_300)) {
+ if (!pvr_version_is(PVR_POWER9)) {
/* P7/P8 nap */
p->thread_idle_state = PNV_THREAD_RUNNING;
} else {
--
2.25.4
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v3 1/3] powerpc/powernv/idle: Replace CPU features checks with PVR checks
2020-07-17 18:53 ` [PATCH v3 1/3] powerpc/powernv/idle: Replace CPU features checks with PVR checks Pratik Rajesh Sampat
@ 2020-07-20 0:00 ` Nicholas Piggin
2020-07-21 10:24 ` Pratik Sampat
0 siblings, 1 reply; 14+ messages in thread
From: Nicholas Piggin @ 2020-07-20 0:00 UTC (permalink / raw)
To: benh, ego, linux-kernel, linuxppc-dev, mikey, mpe, paulus,
pratik.r.sampat, Pratik Rajesh Sampat, svaidy
Excerpts from Pratik Rajesh Sampat's message of July 18, 2020 4:53 am:
> As the idle framework's architecture is incomplete, hence instead of
> checking for just the processor type advertised in the device tree CPU
> features; check for the Processor Version Register (PVR) so that finer
> granularity can be leveraged while making processor checks.
>
> Signed-off-by: Pratik Rajesh Sampat <psampat@linux.ibm.com>
> ---
> arch/powerpc/platforms/powernv/idle.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c
> index 2dd467383a88..f62904f70fc6 100644
> --- a/arch/powerpc/platforms/powernv/idle.c
> +++ b/arch/powerpc/platforms/powernv/idle.c
> @@ -92,7 +92,7 @@ static int pnv_save_sprs_for_deep_states(void)
> if (rc != 0)
> return rc;
>
> - if (cpu_has_feature(CPU_FTR_ARCH_300)) {
> + if (pvr_version_is(PVR_POWER9)) {
> rc = opal_slw_set_reg(pir, P9_STOP_SPR_MSR, msr_val);
> if (rc)
> return rc;
> @@ -116,7 +116,7 @@ static int pnv_save_sprs_for_deep_states(void)
> return rc;
>
> /* Only p8 needs to set extra HID regiters */
> - if (!cpu_has_feature(CPU_FTR_ARCH_300)) {
> + if (!pvr_version_is(PVR_POWER9)) {
>
> rc = opal_slw_set_reg(pir, SPRN_HID1, hid1_val);
> if (rc != 0)
What I think you should do is keep using CPU_FTR_ARCH_300 for this stuff
which is written for power9 and we know is running on power9, because
that's a faster test (static branch and does not have to read PVR. And
then...
> @@ -1205,7 +1205,7 @@ static void __init pnv_probe_idle_states(void)
> return;
> }
>
> - if (cpu_has_feature(CPU_FTR_ARCH_300))
> + if (pvr_version_is(PVR_POWER9))
> pnv_power9_idle_init();
>
> for (i = 0; i < nr_pnv_idle_states; i++)
Here is where you would put the version check. Once we have code that
can also handle P10 (either by testing CPU_FTR_ARCH_31, or by adding
an entirely new power10 idle function), then you can add the P10 version
check here.
Thanks,
Nick
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 1/3] powerpc/powernv/idle: Replace CPU features checks with PVR checks
2020-07-20 0:00 ` Nicholas Piggin
@ 2020-07-21 10:24 ` Pratik Sampat
0 siblings, 0 replies; 14+ messages in thread
From: Pratik Sampat @ 2020-07-21 10:24 UTC (permalink / raw)
To: Nicholas Piggin, benh, ego, linux-kernel, linuxppc-dev, mikey,
mpe, paulus, pratik.r.sampat, svaidy
On 20/07/20 5:30 am, Nicholas Piggin wrote:
> Excerpts from Pratik Rajesh Sampat's message of July 18, 2020 4:53 am:
>> As the idle framework's architecture is incomplete, hence instead of
>> checking for just the processor type advertised in the device tree CPU
>> features; check for the Processor Version Register (PVR) so that finer
>> granularity can be leveraged while making processor checks.
>>
>> Signed-off-by: Pratik Rajesh Sampat <psampat@linux.ibm.com>
>> ---
>> arch/powerpc/platforms/powernv/idle.c | 14 +++++++-------
>> 1 file changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c
>> index 2dd467383a88..f62904f70fc6 100644
>> --- a/arch/powerpc/platforms/powernv/idle.c
>> +++ b/arch/powerpc/platforms/powernv/idle.c
>> @@ -92,7 +92,7 @@ static int pnv_save_sprs_for_deep_states(void)
>> if (rc != 0)
>> return rc;
>>
>> - if (cpu_has_feature(CPU_FTR_ARCH_300)) {
>> + if (pvr_version_is(PVR_POWER9)) {
>> rc = opal_slw_set_reg(pir, P9_STOP_SPR_MSR, msr_val);
>> if (rc)
>> return rc;
>> @@ -116,7 +116,7 @@ static int pnv_save_sprs_for_deep_states(void)
>> return rc;
>>
>> /* Only p8 needs to set extra HID regiters */
>> - if (!cpu_has_feature(CPU_FTR_ARCH_300)) {
>> + if (!pvr_version_is(PVR_POWER9)) {
>>
>> rc = opal_slw_set_reg(pir, SPRN_HID1, hid1_val);
>> if (rc != 0)
> What I think you should do is keep using CPU_FTR_ARCH_300 for this stuff
> which is written for power9 and we know is running on power9, because
> that's a faster test (static branch and does not have to read PVR. And
> then...
>
>> @@ -1205,7 +1205,7 @@ static void __init pnv_probe_idle_states(void)
>> return;
>> }
>>
>> - if (cpu_has_feature(CPU_FTR_ARCH_300))
>> + if (pvr_version_is(PVR_POWER9))
>> pnv_power9_idle_init();
>>
>> for (i = 0; i < nr_pnv_idle_states; i++)
> Here is where you would put the version check. Once we have code that
> can also handle P10 (either by testing CPU_FTR_ARCH_31, or by adding
> an entirely new power10 idle function), then you can add the P10 version
> check here.
Sure, it makes sense to make this check on the top level function and
retain CPU_FTR_ARCH_300 lower in the calls for speed.
I'll make that change.
Thanks
Pratik
> Thanks,
> Nick
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3 2/3] powerpc/powernv/idle: Rename pnv_first_spr_loss_level variable
2020-07-17 18:53 [PATCH v3 0/3] powernv/idle: Power9 idle cleanup Pratik Rajesh Sampat
2020-07-17 18:53 ` [PATCH v3 1/3] powerpc/powernv/idle: Replace CPU features checks with PVR checks Pratik Rajesh Sampat
@ 2020-07-17 18:53 ` Pratik Rajesh Sampat
2020-07-19 23:57 ` Nicholas Piggin
2020-07-17 18:53 ` [PATCH v3 3/3] powerpc/powernv/idle: Exclude mfspr on HID1, 4, 5 " Pratik Rajesh Sampat
2 siblings, 1 reply; 14+ messages in thread
From: Pratik Rajesh Sampat @ 2020-07-17 18:53 UTC (permalink / raw)
To: mpe, npiggin, benh, paulus, mikey, ego, svaidy, psampat,
pratik.r.sampat, linuxppc-dev, linux-kernel
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 <psampat@linux.ibm.com>
---
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 = &pnv_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
@@ -1111,8 +1111,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(&state->psscr_val,
&state->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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v3 2/3] powerpc/powernv/idle: Rename pnv_first_spr_loss_level variable
2020-07-17 18:53 ` [PATCH v3 2/3] powerpc/powernv/idle: Rename pnv_first_spr_loss_level variable Pratik Rajesh Sampat
@ 2020-07-19 23:57 ` Nicholas Piggin
2020-07-21 10:29 ` Pratik Sampat
0 siblings, 1 reply; 14+ messages in thread
From: Nicholas Piggin @ 2020-07-19 23:57 UTC (permalink / raw)
To: benh, ego, linux-kernel, linuxppc-dev, mikey, mpe, paulus,
pratik.r.sampat, Pratik Rajesh Sampat, svaidy
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 <psampat@linux.ibm.com>
> ---
> 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 = &pnv_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
> @@ -1111,8 +1111,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(&state->psscr_val,
> &state->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
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 2/3] powerpc/powernv/idle: Rename pnv_first_spr_loss_level variable
2020-07-19 23:57 ` Nicholas Piggin
@ 2020-07-21 10:29 ` Pratik Sampat
2020-07-21 14:37 ` Nicholas Piggin
0 siblings, 1 reply; 14+ messages in thread
From: Pratik Sampat @ 2020-07-21 10:29 UTC (permalink / raw)
To: Nicholas Piggin, benh, ego, linux-kernel, linuxppc-dev, mikey,
mpe, paulus, pratik.r.sampat, svaidy
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 <psampat@linux.ibm.com>
>> ---
>> 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 = &pnv_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
>> @@ -1111,8 +1111,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(&state->psscr_val,
>> &state->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
>>
>>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 2/3] powerpc/powernv/idle: Rename pnv_first_spr_loss_level variable
2020-07-21 10:29 ` Pratik Sampat
@ 2020-07-21 14:37 ` Nicholas Piggin
2020-07-21 14:55 ` Gautham R Shenoy
0 siblings, 1 reply; 14+ messages in thread
From: Nicholas Piggin @ 2020-07-21 14:37 UTC (permalink / raw)
To: benh, ego, linux-kernel, linuxppc-dev, mikey, mpe, paulus,
pratik.r.sampat, Pratik Sampat, svaidy
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 2/3] powerpc/powernv/idle: Rename pnv_first_spr_loss_level variable
2020-07-21 14:37 ` Nicholas Piggin
@ 2020-07-21 14:55 ` Gautham R Shenoy
0 siblings, 0 replies; 14+ messages in thread
From: Gautham R Shenoy @ 2020-07-21 14:55 UTC (permalink / raw)
To: Nicholas Piggin
Cc: benh, ego, linux-kernel, linuxppc-dev, mikey, mpe, paulus,
pratik.r.sampat, Pratik Sampat, svaidy
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.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 2/3] powerpc/powernv/idle: Rename pnv_first_spr_loss_level variable
@ 2020-07-21 14:55 ` Gautham R Shenoy
0 siblings, 0 replies; 14+ messages in thread
From: Gautham R Shenoy @ 2020-07-21 14:55 UTC (permalink / raw)
To: Nicholas Piggin
Cc: ego, mikey, pratik.r.sampat, linux-kernel, Pratik Sampat, paulus,
linuxppc-dev
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.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3 3/3] powerpc/powernv/idle: Exclude mfspr on HID1,4,5 on P9 and above
2020-07-17 18:53 [PATCH v3 0/3] powernv/idle: Power9 idle cleanup Pratik Rajesh Sampat
@ 2020-07-17 18:53 ` Pratik Rajesh Sampat
2020-07-17 18:53 ` [PATCH v3 2/3] powerpc/powernv/idle: Rename pnv_first_spr_loss_level variable Pratik Rajesh Sampat
2020-07-17 18:53 ` [PATCH v3 3/3] powerpc/powernv/idle: Exclude mfspr on HID1, 4, 5 " Pratik Rajesh Sampat
2 siblings, 0 replies; 14+ messages in thread
From: Pratik Rajesh Sampat @ 2020-07-17 18:53 UTC (permalink / raw)
To: mpe, npiggin, benh, paulus, mikey, ego, svaidy, psampat,
pratik.r.sampat, linuxppc-dev, linux-kernel
POWER9 onwards the support for the registers HID1, HID4, HID5 has been
receded.
Although mfspr on the above registers worked in Power9, In Power10
simulator is unrecognized. Moving their assignment under the
check for machines lower than Power9
Signed-off-by: Pratik Rajesh Sampat <psampat@linux.ibm.com>
Reviewed-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
---
arch/powerpc/platforms/powernv/idle.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c
index d439e11af101..d24d6671f3e8 100644
--- a/arch/powerpc/platforms/powernv/idle.c
+++ b/arch/powerpc/platforms/powernv/idle.c
@@ -73,9 +73,6 @@ static int pnv_save_sprs_for_deep_states(void)
*/
uint64_t lpcr_val = mfspr(SPRN_LPCR);
uint64_t hid0_val = mfspr(SPRN_HID0);
- uint64_t hid1_val = mfspr(SPRN_HID1);
- uint64_t hid4_val = mfspr(SPRN_HID4);
- uint64_t hid5_val = mfspr(SPRN_HID5);
uint64_t hmeer_val = mfspr(SPRN_HMEER);
uint64_t msr_val = MSR_IDLE;
uint64_t psscr_val = pnv_deepest_stop_psscr_val;
@@ -117,6 +114,9 @@ static int pnv_save_sprs_for_deep_states(void)
/* Only p8 needs to set extra HID regiters */
if (!pvr_version_is(PVR_POWER9)) {
+ uint64_t hid1_val = mfspr(SPRN_HID1);
+ uint64_t hid4_val = mfspr(SPRN_HID4);
+ uint64_t hid5_val = mfspr(SPRN_HID5);
rc = opal_slw_set_reg(pir, SPRN_HID1, hid1_val);
if (rc != 0)
--
2.25.4
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v3 3/3] powerpc/powernv/idle: Exclude mfspr on HID1, 4, 5 on P9 and above
@ 2020-07-17 18:53 ` Pratik Rajesh Sampat
0 siblings, 0 replies; 14+ messages in thread
From: Pratik Rajesh Sampat @ 2020-07-17 18:53 UTC (permalink / raw)
To: mpe, npiggin, benh, paulus, mikey, ego, svaidy, psampat,
pratik.r.sampat, linuxppc-dev, linux-kernel
POWER9 onwards the support for the registers HID1, HID4, HID5 has been
receded.
Although mfspr on the above registers worked in Power9, In Power10
simulator is unrecognized. Moving their assignment under the
check for machines lower than Power9
Signed-off-by: Pratik Rajesh Sampat <psampat@linux.ibm.com>
Reviewed-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
---
arch/powerpc/platforms/powernv/idle.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c
index d439e11af101..d24d6671f3e8 100644
--- a/arch/powerpc/platforms/powernv/idle.c
+++ b/arch/powerpc/platforms/powernv/idle.c
@@ -73,9 +73,6 @@ static int pnv_save_sprs_for_deep_states(void)
*/
uint64_t lpcr_val = mfspr(SPRN_LPCR);
uint64_t hid0_val = mfspr(SPRN_HID0);
- uint64_t hid1_val = mfspr(SPRN_HID1);
- uint64_t hid4_val = mfspr(SPRN_HID4);
- uint64_t hid5_val = mfspr(SPRN_HID5);
uint64_t hmeer_val = mfspr(SPRN_HMEER);
uint64_t msr_val = MSR_IDLE;
uint64_t psscr_val = pnv_deepest_stop_psscr_val;
@@ -117,6 +114,9 @@ static int pnv_save_sprs_for_deep_states(void)
/* Only p8 needs to set extra HID regiters */
if (!pvr_version_is(PVR_POWER9)) {
+ uint64_t hid1_val = mfspr(SPRN_HID1);
+ uint64_t hid4_val = mfspr(SPRN_HID4);
+ uint64_t hid5_val = mfspr(SPRN_HID5);
rc = opal_slw_set_reg(pir, SPRN_HID1, hid1_val);
if (rc != 0)
--
2.25.4
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v3 3/3] powerpc/powernv/idle: Exclude mfspr on HID1,4,5 on P9 and above
2020-07-17 18:53 ` [PATCH v3 3/3] powerpc/powernv/idle: Exclude mfspr on HID1, 4, 5 " Pratik Rajesh Sampat
@ 2020-07-20 0:01 ` Nicholas Piggin
-1 siblings, 0 replies; 14+ messages in thread
From: Nicholas Piggin @ 2020-07-20 0:01 UTC (permalink / raw)
To: benh, ego, linux-kernel, linuxppc-dev, mikey, mpe, paulus,
pratik.r.sampat, Pratik Rajesh Sampat, svaidy
Excerpts from Pratik Rajesh Sampat's message of July 18, 2020 4:53 am:
> POWER9 onwards the support for the registers HID1, HID4, HID5 has been
> receded.
> Although mfspr on the above registers worked in Power9, In Power10
> simulator is unrecognized. Moving their assignment under the
> check for machines lower than Power9
>
> Signed-off-by: Pratik Rajesh Sampat <psampat@linux.ibm.com>
> Reviewed-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/platforms/powernv/idle.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c
> index d439e11af101..d24d6671f3e8 100644
> --- a/arch/powerpc/platforms/powernv/idle.c
> +++ b/arch/powerpc/platforms/powernv/idle.c
> @@ -73,9 +73,6 @@ static int pnv_save_sprs_for_deep_states(void)
> */
> uint64_t lpcr_val = mfspr(SPRN_LPCR);
> uint64_t hid0_val = mfspr(SPRN_HID0);
> - uint64_t hid1_val = mfspr(SPRN_HID1);
> - uint64_t hid4_val = mfspr(SPRN_HID4);
> - uint64_t hid5_val = mfspr(SPRN_HID5);
> uint64_t hmeer_val = mfspr(SPRN_HMEER);
> uint64_t msr_val = MSR_IDLE;
> uint64_t psscr_val = pnv_deepest_stop_psscr_val;
> @@ -117,6 +114,9 @@ static int pnv_save_sprs_for_deep_states(void)
>
> /* Only p8 needs to set extra HID regiters */
> if (!pvr_version_is(PVR_POWER9)) {
> + uint64_t hid1_val = mfspr(SPRN_HID1);
> + uint64_t hid4_val = mfspr(SPRN_HID4);
> + uint64_t hid5_val = mfspr(SPRN_HID5);
>
> rc = opal_slw_set_reg(pir, SPRN_HID1, hid1_val);
> if (rc != 0)
> --
> 2.25.4
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 3/3] powerpc/powernv/idle: Exclude mfspr on HID1, 4, 5 on P9 and above
@ 2020-07-20 0:01 ` Nicholas Piggin
0 siblings, 0 replies; 14+ messages in thread
From: Nicholas Piggin @ 2020-07-20 0:01 UTC (permalink / raw)
To: benh, ego, linux-kernel, linuxppc-dev, mikey, mpe, paulus,
pratik.r.sampat, Pratik Rajesh Sampat, svaidy
Excerpts from Pratik Rajesh Sampat's message of July 18, 2020 4:53 am:
> POWER9 onwards the support for the registers HID1, HID4, HID5 has been
> receded.
> Although mfspr on the above registers worked in Power9, In Power10
> simulator is unrecognized. Moving their assignment under the
> check for machines lower than Power9
>
> Signed-off-by: Pratik Rajesh Sampat <psampat@linux.ibm.com>
> Reviewed-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/platforms/powernv/idle.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c
> index d439e11af101..d24d6671f3e8 100644
> --- a/arch/powerpc/platforms/powernv/idle.c
> +++ b/arch/powerpc/platforms/powernv/idle.c
> @@ -73,9 +73,6 @@ static int pnv_save_sprs_for_deep_states(void)
> */
> uint64_t lpcr_val = mfspr(SPRN_LPCR);
> uint64_t hid0_val = mfspr(SPRN_HID0);
> - uint64_t hid1_val = mfspr(SPRN_HID1);
> - uint64_t hid4_val = mfspr(SPRN_HID4);
> - uint64_t hid5_val = mfspr(SPRN_HID5);
> uint64_t hmeer_val = mfspr(SPRN_HMEER);
> uint64_t msr_val = MSR_IDLE;
> uint64_t psscr_val = pnv_deepest_stop_psscr_val;
> @@ -117,6 +114,9 @@ static int pnv_save_sprs_for_deep_states(void)
>
> /* Only p8 needs to set extra HID regiters */
> if (!pvr_version_is(PVR_POWER9)) {
> + uint64_t hid1_val = mfspr(SPRN_HID1);
> + uint64_t hid4_val = mfspr(SPRN_HID4);
> + uint64_t hid5_val = mfspr(SPRN_HID5);
>
> rc = opal_slw_set_reg(pir, SPRN_HID1, hid1_val);
> if (rc != 0)
> --
> 2.25.4
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread