[Public]
> -----Original Message-----
> From: Lazar, Lijo <Lijo.Lazar@xxxxxxx>
> Sent: Tuesday, February 21, 2023 07:20
> To: Limonciello, Mario <Mario.Limonciello@xxxxxxx>; amd-
> gfx@xxxxxxxxxxxxxxxxxxxxx
> Cc: Peter Kopec <pekopec@xxxxxxxxxx>
> 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@xxxxxxxxxx>
> > Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx>
> > ---
> > 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))
