Re: [PATCH v2 0/3] Power10 basic energy management

On 13/07/20 10:20 pm, Nicholas Piggin wrote:

Excerpts from Pratik Sampat's message of July 13, 2020 8:02 pm:

quoted

Thank you for your comments,

On 13/07/20 10:53 am, Nicholas Piggin wrote:

quoted

Excerpts from Pratik Rajesh Sampat's message of July 10, 2020 3:22 pm:

quoted

Changelog v1 --> v2:
1. Save-restore DAWR and DAWRX unconditionally as they are lost in
shallow idle states too
2. Rename pnv_first_spr_loss_level to pnv_first_fullstate_loss_level to
correct naming terminology

Pratik Rajesh Sampat (3):
    powerpc/powernv/idle: Exclude mfspr on HID1,4,5 on P9 and above
    powerpc/powernv/idle: save-restore DAWR0,DAWRX0 for P10
    powerpc/powernv/idle: Rename pnv_first_spr_loss_level variable

   arch/powerpc/platforms/powernv/idle.c | 34 +++++++++++++++++----------
   1 file changed, 22 insertions(+), 12 deletions(-)

These look okay to me, but the CPU_FTR_ARCH_300 test for
pnv_power9_idle_init() is actually wrong, it should be a PVR test
because idle is not completely architected (not even shallow stop
states, unfortunately).

It doesn't look like we support POWER10 idle correctly yet, and on older
kernels it wouldn't work even if we fixed newer, so ideally the PVR
check would be backported as a fix in the front of the series.

Sadly, we have no OPAL idle driver yet. Hopefully we will before the
next processor shows up :P

Thanks,
Nick

So if I understand this correctly, in powernv/idle.c where we check for
CPU_FTR_ARCH_300, we should rather be making a pvr_version_is(PVR_POWER9)
check instead?

Of course, the P10 PVR and its relevant checks will have to be added then too.

Yes I think so, unfortunately.

Thanks,
Nick

Sure, I'll add these checks in.

Thanks,
Pratik