Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Michael Ellerman <mpe@ellerman.id.au>
To:     Pratik Sampat <psampat@linux.ibm.com>,
        Christophe Leroy <christophe.leroy@csgroup.eu>,
        linuxppc-dev@lists.ozlabs.org, npiggin@gmail.com,
        mikey@neuling.org, ego@linux.vnet.ibm.com, svaidy@linux.ibm.com,
        linux-kernel@vger.kernel.org, pratik.r.sampat@gmail.com
Subject: Re: [PATCH] Revert "powerpc/powernv/idle: Replace CPU feature check with PVR check"
In-Reply-To: <170e1919-bc45-6b99-dc4d-713418c98be1@linux.ibm.com>
References: <20200826082918.89306-1-psampat@linux.ibm.com> <1fb7fcef-a39d-d36e-35d5-021a5c9ea82c@csgroup.eu> <170e1919-bc45-6b99-dc4d-713418c98be1@linux.ibm.com>
Date:   Thu, 27 Aug 2020 16:07:39 +1000
Message-ID: <87imd439lg.fsf@mpe.ellerman.id.au>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Pratik Sampat <psampat@linux.ibm.com> writes:
> On 26/08/20 2:07 pm, Christophe Leroy wrote:
>> Le 26/08/2020 =C3=A0 10:29, Pratik Rajesh Sampat a =C3=A9crit=C2=A0:
>>> Cpuidle stop state implementation has minor optimizations for P10
>>> where hardware preserves more SPR registers compared to P9.
>>> The current P9 driver works for P10, although does few extra
>>> save-restores. P9 driver can provide the required power management
>>> features like SMT thread folding and core level power savings
>>> on a P10 platform.
>>>
>>> Until the P10 stop driver is available, revert the commit which
>>> allows for only P9 systems to utilize cpuidle and blocks all
>>> idle stop states for P10.
>>> Cpu idle states are enabled and tested on the P10 platform
>>> with this fix.
>>>
>>> This reverts commit 8747bf36f312356f8a295a0c39ff092d65ce75ae.
>>>
>>> Fixes: 8747bf36f312 ("powerpc/powernv/idle: Replace CPU feature check=20
>>> with PVR check")
>>> Signed-off-by: Pratik Rajesh Sampat <psampat@linux.ibm.com>
>>> ---
>>> =C2=A0 @mpe: This revert would resolve a staging issue wherein the P10 =
stop
>>> =C2=A0 driver is not yet ready while cpuidle stop states need not be bl=
ocked
>>> =C2=A0 on 5.9 for Power10 systems which could cause SMT folding related
>>> =C2=A0 performance issues.
>>>
>>> =C2=A0 The P10 stop driver is in the works here:
>>> https://lists.ozlabs.org/pipermail/linuxppc-dev/2020-August/216773.html
>>>
>>> =C2=A0 arch/powerpc/platforms/powernv/idle.c | 2 +-
>>> =C2=A0 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/arch/powerpc/platforms/powernv/idle.c=20
>>> b/arch/powerpc/platforms/powernv/idle.c
>>> index 77513a80cef9..345ab062b21a 100644
>>> --- a/arch/powerpc/platforms/powernv/idle.c
>>> +++ b/arch/powerpc/platforms/powernv/idle.c
>>> @@ -1223,7 +1223,7 @@ static void __init pnv_probe_idle_states(void)
>>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 return;
>>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 }
>>> =C2=A0 -=C2=A0=C2=A0=C2=A0 if (pvr_version_is(PVR_POWER9))
>>> +=C2=A0=C2=A0=C2=A0 if (cpu_has_feature(CPU_FTR_ARCH_300))
>>
>> Why not something like:
>>
>> =C2=A0=C2=A0=C2=A0=C2=A0if (pvr_version_is(PVR_POWER9) || pvr_version_is=
(PVR_POWER10))
>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 pnv_power9_idle_init();=20
>
> In order to use PVR_POWER10 I would need to define it under
> arch/powerpc/include/asm/reg.h, which is not present in 5.9 yet.
>
> However, if it okay with @mpe I could split out Nick's P10 stop driver
> (https://lists.ozlabs.org/pipermail/linuxppc-dev/2020-August/216773.html)
> into two parts:

I'll just take this for now, it's the simplest option.

cheers