[v3] drm/amdgpu: reset asic after system-wide suspend aborted (v3)

[Liang, Prike]

[Public]

> -----Original Message-----
> From: Alex Deucher <alexdeucher at gmail.com>
> Sent: Thursday, December 2, 2021 3:39 AM
> To: Limonciello, Mario <Mario.Limonciello at amd.com>
> Cc: Liang, Prike <Prike.Liang at amd.com>; amd-gfx list <amd-
> gfx at lists.freedesktop.org>; Deucher, Alexander
> <Alexander.Deucher at amd.com>; Lazar, Lijo <Lijo.Lazar at amd.com>; Huang,
> Ray <Ray.Huang at amd.com>
> Subject: Re: [v3] drm/amdgpu: reset asic after system-wide suspend aborted
> (v3)
>
> On Wed, Dec 1, 2021 at 1:46 PM Limonciello, Mario
> <mario.limonciello at amd.com> wrote:
> >
> > On 11/24/2021 23:48, Prike Liang wrote:
> > > Do ASIC reset at the moment Sx suspend aborted behind of amdgpu
> > > suspend to keep AMDGPU in a clean reset state and that can avoid
> > > re-initialize device improperly error. Currently,we just always do
> > > asic reset in the amdgpu resume until sort out the PM abort case.
> > >
> > > v2: Remove incomplete PM abort flag and add GPU hive case check for
> > > GPU reset.
> > >
> > > v3: Some dGPU reset method not support at the early resume time and
> > > temprorary skip the dGPU case.
> > >
> > > Signed-off-by: Prike Liang <Prike.Liang at amd.com>
> > > ---
> > >   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 8 ++++++++
> > >   1 file changed, 8 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > > index 7d4115d..f6e1a6a 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > > @@ -3983,6 +3983,14 @@ int amdgpu_device_resume(struct
> drm_device *dev, bool fbcon)
> > >       if (adev->in_s0ix)
> > >               amdgpu_gfx_state_change_set(adev,
> > > sGpuChangeState_D0Entry);
> > >
> > > +     /*TODO: In order to not let all-always asic reset affect resume latency
> > > +      * need sort out the case which really need asic reset in the resume
> process.
> > > +      * As to the known issue on the system suspend abort behind the
> AMDGPU suspend,
> > > +      * may can sort this case by checking struct suspend_stats which need
> exported
> > > +      * firstly.
> > > +      */
> > > +     if (adev->flags & AMD_IS_APU)
> > > +             amdgpu_asic_reset(adev);
> >
> > Ideally you only want this to happen on S3 right?  So shouldn't there
> > be an extra check for `amdgpu_acpi_is_s0ix_active`?
>
> Shouldn't matter on the resume side.  Only the suspend side.  If we reset in
> suspend, we'd end up disabling gfxoff.  On the resume side, it should safe,
> but the resume paths for various IPs probably are not adequate to deal with
> a reset for S0i3 since they don't re-init as much hardware.  So it's probably
> better to skip this for S0i3.
>
> Alex
>
There's may some IP suspend/resume sequence not program correctly and when abort the s2idle after AMDGPU suspend can see some other error in the SMU/SDMA resume. However, that seems different symptom in the S3 abort case and will handle it separately.

>
> >
> > >       /* post card */
> > >       if (amdgpu_device_need_post(adev)) {
> > >               r = amdgpu_device_asic_init(adev);
> > >
> >