[PATCH 2/3] drm/amd: Use runtime suspend in lieu regular suspend on supported dGPUs

[Limonciello, Mario]

[Public]



> -----Original Message-----
> From: Lazar, Lijo <Lijo.Lazar at amd.com>
> Sent: Tuesday, February 21, 2023 07:20
> To: Limonciello, Mario <Mario.Limonciello at amd.com>; amd-
> gfx at lists.freedesktop.org
> Cc: Peter Kopec <pekopec at redhat.com>
> Subject: Re: [PATCH 2/3] drm/amd: Use runtime suspend in lieu regular suspend
> on supported dGPUs
> 
> 
> 
> On 2/21/2023 1:46 AM, Mario Limonciello wrote:
> > The PMFW on dGPUs that support BACO will transition them in and out
> > of BACO when video/audio move in out of D3/D0.
> >
> > On the Linux side users can configure what sleep mode to use in
> > `/sys/power/mem_sleep`, but if the host hardware doesn't cut the
> > power rails during this state then calling suspend from Linux may
> > cause a mismatch of behavior.
> >
> > To avoid this, only run the runtime suspend and resume callbacks
> > when the dGPU supports BACO or BOCO and the smart flags didn't return
> > to skip these stages (because already runtime suspended).
> >
> > Cc: Peter Kopec <pekopec at redhat.com>
> > Signed-off-by: Mario Limonciello <mario.limonciello at amd.com>
> > ---
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 9 +++++++--
> >   1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > index c3d3a042946d..fdc1cbf8ad10 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > @@ -2418,8 +2418,11 @@ static int amdgpu_pmops_suspend(struct device
> *dev)
> >   		adev->in_s0ix = true;
> >   	else if (amdgpu_acpi_is_s3_active(adev))
> >   		adev->in_s3 = true;
> > -	if (!adev->in_s0ix && !adev->in_s3)
> > +	if (!adev->in_s0ix && !adev->in_s3) {
> > +		pm_runtime_mark_last_busy(dev);
> > +		pm_runtime_autosuspend(dev);
> 
> This is asking the device to be suspended (from a suspend call and that
> sounds weird).  

I had convinced myself that it was necessary from reading documentation,
but re-reading I believe it should not be necessary if smart suspend is used.

If I drop this patch  then the PMFW should still transition it when the video
turns off.

> Runtime pm handler will assume D3cold scenario and
> explicitly request BACO entry. Wondering what would happen if the
> platform doesn't put it in D3cold under s2Idle for dGPUs (BACO/BOCO).
> 

Higher power consumption I expect.

> Thanks,
> Lijo
> 
> >   		return 0;
> > +	}
> >   	return amdgpu_device_suspend(drm_dev, true);
> >   }
> >
> > @@ -2440,8 +2443,10 @@ static int amdgpu_pmops_resume(struct device
> *dev)
> >   	struct amdgpu_device *adev = drm_to_adev(drm_dev);
> >   	int r;
> >
> > -	if (!adev->in_s0ix && !adev->in_s3)
> > +	if (!adev->in_s0ix && !adev->in_s3) {
> > +		pm_runtime_resume(dev);
> >   		return 0;
> > +	}
> >
> >   	/* Avoids registers access if device is physically gone */
> >   	if (!pci_device_is_present(adev->pdev))