RE: [PATCH] platform/x86/amd: pmc: Add a workaround for an s0i3 issue on Cezanne

[Limonciello, Mario]

[Public]



> -----Original Message-----
> From: Hans de Goede <hdegoede@xxxxxxxxxx>
> Sent: Thursday, November 17, 2022 08:06
> To: Limonciello, Mario <Mario.Limonciello@xxxxxxx>; S-k, Shyam-sundar
> <Shyam-sundar.S-k@xxxxxxx>
> Cc: Mahapatra, Rajib <Rajib.Mahapatra@xxxxxxx>; Raul Rangel
> <rrangel@xxxxxxxxxxxx>; Mark Gross <markgross@xxxxxxxxxx>; platform-
> driver-x86@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH] platform/x86/amd: pmc: Add a workaround for an s0i3
> issue on Cezanne
> 
> Hi Mario,
> 
> On 11/16/22 16:43, Mario Limonciello wrote:
> > Cezanne platforms under the right circumstances have a synchronization
> > problem where attempting to enter s2idle may fail if the x86 cores are
> > put into HLT before hardware resume from the previous attempt has
> > completed.
> >
> > To avoid this issue add a 10-20ms delay before entering s2idle another
> > time. This workaround will only be applied on interrupts that wake the
> > hardware but don't break the s2idle loop.
> >
> > Cc: "Mahapatra, Rajib" <Rajib.Mahapatra@xxxxxxx>
> > Cc: "Raul Rangel" <rrangel@xxxxxxxxxxxx>
> > Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx>
> 
> Thank you for your patch, I've applied this patch to my review-hans
> branch:
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.k
> ernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fpdx86%2Fplatform-
> drivers-x86.git%2Flog%2F%3Fh%3Dreview-
> hans&amp;data=05%7C01%7Cmario.limonciello%40amd.com%7C674f8bf7a8
> 114f83a3b408dac8a4d941%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C
> 0%7C638042907591739047%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLj
> AwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%
> 7C%7C&amp;sdata=XYwl%2FOvUFy%2Bgz9EY9oa35M%2BkLf%2Bud8PKXynQ
> FlrUdoE%3D&amp;reserved=0
> 
> Please let me know if it important to get this as a fix into 6.1,
> I wasn't really planning on doing any more fixes pull-reqs for 6.1,
> but I can do one if necessary.
> 

AFAIK it's a corner case.  I think it can wait until 6.2, but I think it should probably
be Cc to 6.1 stable (which has the ability to run code in the check()) phase.

> Once I've run some tests on this branch the patches there will be
> added to the platform-drivers-x86/for-next branch and eventually
> will be included in the pdx86 pull-request to Linus for the next
> merge-window.
> 
> Regards,
> 
> Hans
> 
> 
> > ---
> >  drivers/platform/x86/amd/pmc.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/drivers/platform/x86/amd/pmc.c
> b/drivers/platform/x86/amd/pmc.c
> > index ef4ae977b8e0..439d282aafd1 100644
> > --- a/drivers/platform/x86/amd/pmc.c
> > +++ b/drivers/platform/x86/amd/pmc.c
> > @@ -739,8 +739,14 @@ static void amd_pmc_s2idle_prepare(void)
> >  static void amd_pmc_s2idle_check(void)
> >  {
> >  	struct amd_pmc_dev *pdev = &pmc;
> > +	struct smu_metrics table;
> >  	int rc;
> >
> > +	/* CZN: Ensure that future s0i3 entry attempts at least 10ms passed
> */
> > +	if (pdev->cpu_id == AMD_CPU_ID_CZN &&
> !get_metrics_table(pdev, &table) &&
> > +	    table.s0i3_last_entry_status)
> > +		usleep_range(10000, 20000);
> > +
> >  	/* Dump the IdleMask before we add to the STB */
> >  	amd_pmc_idlemask_read(pdev, pdev->dev, NULL);
> >