Re: [PATCH v3] drm/amdgpu: Increase KIQ invalidate_tlbs timeout

["Christian König" <ckoenig.leichtzumerken@gmail.com>]

Am 31.03.25 um 17:44 schrieb Jay Cornwall:
> 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.
>
> v2: Poll once before msleep
> v3: Fix return value
>
> Signed-off-by: Jay Cornwall <jay.cornwall@amd.com>
> Cc: Kent Russell <kent.russell@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h     |  1 -
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c | 19 ++++++++++++++-----
>  2 files changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index ffca74a476da..3cdb5f8325aa 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -356,7 +356,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

Unrelated to this patch here, but defines like those *must* have an AMDGPU_ prefix.

Please fix in a follow up patch.

Regards,
Christian.

> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> index 464625282872..ecb74ccf1d90 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,10 +759,21 @@ 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) {
> +
> +		r = amdgpu_fence_wait_polling(ring, seq, MAX_KIQ_REG_WAIT);
> +
> +		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;
> -		}
> +		} else
> +			r = 0;
>  	}
>  
>  error_unlock_reset: