[Public] -----Original Message----- From: Quan, Evan <Evan.Quan@xxxxxxx> Sent: Monday, October 24, 2022 2:31 PM To: Liang, Prike <Prike.Liang@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx Cc: Deucher, Alexander <Alexander.Deucher@xxxxxxx>; Liang, Prike <Prike.Liang@xxxxxxx> Subject: RE: [PATCH v2] drm/amdgpu: disallow gfxoff until GC IP blocks complete s2idle resume [AMD Official Use Only - General] > -----Original Message----- > From: amd-gfx <amd-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of > Prike Liang > Sent: Friday, October 21, 2022 10:47 PM > To: amd-gfx@xxxxxxxxxxxxxxxxxxxxx > Cc: Deucher, Alexander <Alexander.Deucher@xxxxxxx>; Liang, Prike > <Prike.Liang@xxxxxxx> > Subject: [PATCH v2] drm/amdgpu: disallow gfxoff until GC IP blocks > complete s2idle resume > > 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. [Quan, Evan] It seems the issue described here has nothing related with suspend. Instead it happened during resuming only. Right? If so, I would suggest to drop the confusing "suspend" from the description part. Other than that, the patch is reviewed-by: Evan Quan <evan.quan@xxxxxxx> Evan [Prike] Yes this problem only observed on the s2idle resume period, but that's true the s2idle suspend and resume keep the gfxoff being functional. > > Signed-off-by: Prike Liang <Prike.Liang@xxxxxxx> > --- > -v2: Move the operation of exiting gfxoff from smu to higer layer in > amdgpu_device.c. > > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > index 5b8362727226..36c44625932e 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > @@ -3210,6 +3210,12 @@ static int > amdgpu_device_ip_resume_phase2(struct amdgpu_device *adev) > return r; > } > adev->ip_blocks[i].status.hw = true; > + > + if (adev->in_s0ix && adev->ip_blocks[i].version->type == > AMD_IP_BLOCK_TYPE_SMC) { > + amdgpu_gfx_off_ctrl(adev, false); > + DRM_DEBUG("will disable gfxoff for re-initializing > other blocks\n"); > + } > + > } > > return 0; > @@ -4185,6 +4191,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); > > -- > 2.25.1