[Public] Do we want to actually do a fence_wait_polling before the loop begins like in all of the other functions? I know 5ms isn't a big deal, but the other ones we have are all after a single fence_wait_polling first.
Kent > -----Original Message----- > From: Cornwall, Jay <jay.cornw...@amd.com> > Sent: Friday, March 21, 2025 2:27 PM > To: amd-gfx@lists.freedesktop.org > Cc: Cornwall, Jay <jay.cornw...@amd.com>; Russell, Kent > <kent.russ...@amd.com> > Subject: [PATCH] drm/amdgpu: Increase KIQ invalidate_tlbs timeout > > KIQ invalidate_tlbs request has been seen to marginally exceed the > configured 100 ms timeout on systems under load. > > All other KIQ requests in the driver use a 10 second timeout. Use a > similar timeout implementation on the invalidate_tlbs path. > > Signed-off-by: Jay Cornwall <jay.cornw...@amd.com> > Cc: Kent Russell <kent.russ...@amd.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 - > drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c | 15 +++++++++++---- > 2 files changed, 11 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h > b/drivers/gpu/drm/amd/amdgpu/amdgpu.h > index f68a348dcec9..54bf8780f0b9 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h > @@ -355,7 +355,6 @@ enum amdgpu_kiq_irq { > AMDGPU_CP_KIQ_IRQ_DRIVER0 = 0, > AMDGPU_CP_KIQ_IRQ_LAST > }; > -#define SRIOV_USEC_TIMEOUT 1200000 /* wait 12 * 100ms for SRIOV */ > #define MAX_KIQ_REG_WAIT 5000 /* in usecs, 5ms */ > #define MAX_KIQ_REG_BAILOUT_INTERVAL 5 /* in msecs, 5ms */ > #define MAX_KIQ_REG_TRY 1000 > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c > index 464625282872..047437f625c4 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c > @@ -699,12 +699,10 @@ int amdgpu_gmc_flush_gpu_tlb_pasid(struct > amdgpu_device *adev, uint16_t pasid, > uint32_t flush_type, bool all_hub, > uint32_t inst) > { > - u32 usec_timeout = amdgpu_sriov_vf(adev) ? SRIOV_USEC_TIMEOUT : > - adev->usec_timeout; > struct amdgpu_ring *ring = &adev->gfx.kiq[inst].ring; > struct amdgpu_kiq *kiq = &adev->gfx.kiq[inst]; > unsigned int ndw; > - int r; > + int r, cnt = 0; > uint32_t seq; > > /* > @@ -761,7 +759,16 @@ int amdgpu_gmc_flush_gpu_tlb_pasid(struct > amdgpu_device *adev, uint16_t pasid, > > amdgpu_ring_commit(ring); > spin_unlock(&adev->gfx.kiq[inst].ring_lock); > - if (amdgpu_fence_wait_polling(ring, seq, usec_timeout) < 1) { > + > + might_sleep(); > + while (r < 1 && cnt++ < MAX_KIQ_REG_TRY && > + !amdgpu_reset_pending(adev->reset_domain)) { > + > + msleep(MAX_KIQ_REG_BAILOUT_INTERVAL); > + r = amdgpu_fence_wait_polling(ring, seq, > MAX_KIQ_REG_WAIT); > + } > + > + if (cnt > MAX_KIQ_REG_TRY) { > dev_err(adev->dev, "timeout waiting for kiq fence\n"); > r = -ETIME; > } > -- > 2.34.1