| Message ID | 20200717185306.60607-2-psampat@linux.ibm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | powernv/idle: Power9 idle cleanup | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch powerpc/merge (c27fe454aff795023d2f3f90f41eb1a3104e614f) |
| snowpatch_ozlabs/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 56 lines checked |
| snowpatch_ozlabs/needsstable | success | Patch has no Fixes tags |
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
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 >
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 {
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(-)