[PATCH] drm/amdgpu: disallow gfxoff until GC IP blocks complete s2idle resume

[Alex Deucher]

On Fri, Oct 21, 2022 at 10:10 AM Liang, Prike <Prike.Liang at amd.com> wrote:
>
> [Public]
>
> -----Original Message-----
> From: Alex Deucher <alexdeucher at gmail.com>
> Sent: Friday, October 21, 2022 9:39 PM
> To: Liang, Prike <Prike.Liang at amd.com>
> Cc: amd-gfx at lists.freedesktop.org; Deucher, Alexander <Alexander.Deucher at amd.com>; Huang, Ray <Ray.Huang at amd.com>
> Subject: Re: [PATCH] drm/amdgpu: disallow gfxoff until GC IP blocks complete s2idle resume
>
> On Thu, Oct 20, 2022 at 10:30 PM Prike Liang <Prike.Liang at amd.com> wrote:
> >
> > In the S2idle suspend/resume phase the gfxoff is keeping functional so
> > some IP blocks will be likely to reinitialize at gfxoff entry and that
> > will result in failing to program GC registers.Therefore, let disallow
> > gfxoff until AMDGPU IPs reinitialized completely.
> >
> > Signed-off-by: Prike Liang <Prike.Liang at amd.com>
> > ---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 4 ++++
> > drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c  | 5 +++++
> >  2 files changed, 9 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > index e0445e8cc342..1624ed15fbc4 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > @@ -4185,6 +4185,10 @@ int amdgpu_device_resume(struct drm_device *dev, bool fbcon)
> >         /* Make sure IB tests flushed */
> >         flush_delayed_work(&adev->delayed_init_work);
> >
> > +       if (adev->in_s0ix) {
> > +               amdgpu_gfx_off_ctrl(adev, true);
> > +               DRM_DEBUG("will enable gfxoff for the mission mode\n");
> > +       }
> >         if (fbcon)
> >
> > drm_fb_helper_set_suspend_unlocked(adev_to_drm(adev)->fb_helper,
> > false);
> >
> > diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> > b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> > index 4fe75dd2b329..3948dc5b1d6a 100644
> > --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> > +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> > @@ -1664,6 +1664,11 @@ static int smu_resume(void *handle)
> >
> >         dev_info(adev->dev, "SMU is resumed successfully!\n");
> >
> > +       if (adev->in_s0ix) {
> > +               amdgpu_gfx_off_ctrl(adev, false);
> > +               dev_dbg(adev->dev, "will disable gfxoff for re-initializing other blocks\n");
> > +       }
> > +
>
> I think it would be better to put this in amdgpu_device.c so it's clear where its match is.  Also for raven based boards this will get missed because they still use the powerplay power code.
>
> Alex
>
> [Prike] I miss this amdgpu_gfx_off_ctrl() is a generic upper layer function but that should also work on Rave series, since on the Rave series will use another message PPSMC_MSG_GpuChangeState exit gfxoff. But it makes sense unify the operation of exiting gfxoff at an upper layer in amdgpu_device.c and I will update it at patch v2.
>

Ok.  If you can move it into amdgpu_device.c that would be great, but
if not, either way, please add comments to these functions noting
where their match is and why this is necessary.

Thanks!

Alex

> Thanks,
> Prike
> >         return 0;
> >  }
> >
> > --
> > 2.25.1
> >