Re: [PATCH v3 1/3] powerpc/powernv/idle: Replace CPU features checks with PVR checks

On 20/07/20 5:30 am, Nicholas Piggin wrote:

Excerpts from Pratik Rajesh Sampat's message of July 18, 2020 4:53 am:

quoted

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 <redacted>
---
  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...

quoted

@@ -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