Re: [PATCH v3 02/15] drm/amdgpu: complete the guilty fence at the end of a multi ring reset

["Timur Kristóf" <timur.kristof@gmail.com>]

On Wed, 2026-10-28 at 16:56 +0800, Jesse Zhang wrote:
> amdgpu_ring_backup_unprocessed_commands() skips the backup of a ring
> and
> clears ring->guilty_fence when it sees the same guilty fence again,
> to
> avoid replay loops. That happens when the IP block soft reset follows
> a
> failed ring reset of the same job.
> amdgpu_multi_ring_reset_helper_end()
> then restored the guilty ring with ring->guilty_fence, NULL by then,
> so
> neither the guilty fence nor the rest of the ring was completed: the
> hung
> job stayed pending until it timed out again.
> 
> Pass the guilty fence to the end helper, and use it for the guilty
> ring.
> For the other rings, and when the backup didn't drop it, nothing
> changes.
> 
> Signed-off-by: Jesse Zhang <Jesse.Zhang@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c   |  3 ++-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c | 13 ++++++++++---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h |  3 ++-
>  3 files changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c
> index d0b0d3eaffc5..2eea18965fd1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c
> @@ -469,7 +469,8 @@ int amdgpu_device_ip_soft_reset(struct
> amdgpu_ring *guilty_ring,
>  
>  	r = ip_block->version->funcs->soft_reset(ip_block);
>  
> -	r = amdgpu_multi_ring_reset_helper_end(adev, ring_type_mask,
> guilty_ring, r);
> +	r = amdgpu_multi_ring_reset_helper_end(adev, ring_type_mask,
> guilty_ring,
> +					       guilty_fence, r);
>  	amdgpu_device_unlock_reset_domain(adev->reset_domain);
>  
>  	if (r) {
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> index edae764eb0c4..7f0f0ae6650a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> @@ -1038,6 +1038,7 @@ void
> amdgpu_multi_ring_reset_helper_begin(struct amdgpu_device *adev,
>   * @adev: amdgpu device
>   * @ring_type_mask: Bitmask of affected ring types
>   * @guilty_ring: The ring which is guilty of causing a reset, or
> NULL.
> + * @guilty_fence: The fence which didn't signal on the guilty ring.
>   * @ret: Return code from the reset function.
>   *
>   * After calling amdgpu_multi_ring_reset_helper_begin()
> @@ -1050,7 +1051,8 @@ void
> amdgpu_multi_ring_reset_helper_begin(struct amdgpu_device *adev,
>   */
>  int amdgpu_multi_ring_reset_helper_end(struct amdgpu_device *adev,
>  				       const u32 ring_type_mask,
> -				       struct amdgpu_ring
> *guilty_ring, int ret)
> +				       struct amdgpu_ring
> *guilty_ring,
> +				       struct amdgpu_fence
> *guilty_fence, int ret)
>  {
>  	struct amdgpu_ring *ring;
>  	int i, r;
> @@ -1080,8 +1082,13 @@ int amdgpu_multi_ring_reset_helper_end(struct
> amdgpu_device *adev,
>  		if (!(BIT(ring->funcs->type) & ring_type_mask))
>  			continue;
>  
> -		/* Restore contents of the ring */
> -		r = amdgpu_ring_reset_helper_end(ring, ring-
> >guilty_fence);
> +		/*
> +		 * Restore contents of the ring. The backup of the
> guilty ring
> +		 * may have dropped its guilty fence, eg. after a
> failed ring
> +		 * reset with the same fence: still complete it.
> +		 */
> +		r = amdgpu_ring_reset_helper_end(ring, ring ==
> guilty_ring ?
> +						 guilty_fence :
> ring->guilty_fence);

Can you help me understand what was wrong with the line before?
I think guilty_fence == ring->guilty_fence when ring == guilty_ring no?


>  		if (r) {
>  			dev_err(adev->dev,
>  				"Failed to recover ring %s after
> soft reset\n",
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> index 7e6ed43771f2..349c7d503e5f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> @@ -643,7 +643,8 @@ void amdgpu_multi_ring_reset_helper_begin(struct
> amdgpu_device *adev,
>  					  struct amdgpu_fence
> *guilty_fence);
>  int amdgpu_multi_ring_reset_helper_end(struct amdgpu_device *adev,
>  				       const u32 ring_type_mask,
> -				       struct amdgpu_ring
> *guilty_ring, int ret);
> +				       struct amdgpu_ring
> *guilty_ring,
> +				       struct amdgpu_fence
> *guilty_fence, int ret);
>  bool amdgpu_ring_is_reset_type_supported(struct amdgpu_ring *ring,
>  					 u32 reset_type);
>  #endif