[AMD Official Use Only - General] Hi Christian, http://gerrit-git.amd.com/c/brahma/ec/linux/+/620522 In this patch I saw that there was originally two evict in the suspend and this patch the second one (the one after ip_suspend2) were removed. I am a little bit confused on this, should I just keep the way it is and just add an extra evict in the beginning? Thanks, Shikang. -----Original Message----- From: Christian König <ckoenig.leichtzumerken@xxxxxxxxx> Sent: Thursday, December 8, 2022 5:30 PM To: Fan, Shikang <Shikang.Fan@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx Subject: Re: [PATCH] drm/amdgpu: Add an extra evict_resource call during device_suspend. Am 08.12.22 um 10:25 schrieb Shikang Fan: > - evict_resource is taking too long causing sriov full access mode timeout. > So, add an extra evict_resource in the beginning as an early evict. > - Move the original evict_resource after ip_suspend2. > > Signed-off-by: Shikang Fan <shikang.fan@xxxxxxx> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 12 ++++++++---- > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > index 818fa72c670d..8b7db87cffd9 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > @@ -4111,6 +4111,10 @@ int amdgpu_device_suspend(struct drm_device > *dev, bool fbcon) > > adev->in_suspend = true; > > + r = amdgpu_device_evict_resources(adev); > + if (r) > + return r; > + > if (amdgpu_sriov_vf(adev)) { > amdgpu_virt_fini_data_exchange(adev); > r = amdgpu_virt_request_full_gpu(adev, false); @@ -4135,14 > +4139,14 @@ int amdgpu_device_suspend(struct drm_device *dev, bool fbcon) > if (!adev->in_s0ix) > amdgpu_amdkfd_suspend(adev, adev->in_runpm); > > - r = amdgpu_device_evict_resources(adev); > - if (r) > - return r; > - > amdgpu_fence_driver_hw_fini(adev); > > amdgpu_device_ip_suspend_phase2(adev); > > + r = amdgpu_device_evict_resources(adev); > + if (r) > + return r; > + As noted internally please keep this evict resources call where it was. It makes sense to evict the BOs which were previously pinned by display with the SDMA engine. Only the final eviction of BOs for fw etc.. should be done with the CPU. I suggest to also add a comment to each call explaining why we need it. Regards, Christian. > if (amdgpu_sriov_vf(adev)) > amdgpu_virt_release_full_gpu(adev, false); >
