Re: [PATCH v3 3/8] amdgpu: use trace_dma_fence_sync_to in amdgpu_fence_sync

["Christian König" <christian.koenig@amd.com>]

Am 16.02.24 um 16:09 schrieb Pierre-Eric Pelloux-Prayer:
> This makes it possible to understand the dependencies between jobs.
> Possible usage of this trace:
> * stuttering issues like Mesa !9189
> * incorrect synchronization: I don't have a link for this one, but having
>    these events was very useful to debug a virtio-gpu / native-context /
>    radeonsi sync issue
>
> I have prototype code using this in UMR, as can be see here:
>     https://gitlab.freedesktop.org/tomstdenis/umr/-/merge_requests/37
>
> v2: add a macro since every caller passes __func__ as the reason parameter
>
> Signed-off-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c | 9 +++++++--
>   drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h | 4 +++-
>   2 files changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
> index 1b013a44ca99..9a3fdc4be51e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
> @@ -30,6 +30,7 @@
>    */
>   
>   #include <linux/dma-fence-chain.h>
> +#include <trace/events/dma_fence.h>
>   
>   #include "amdgpu.h"
>   #include "amdgpu_trace.h"
> @@ -145,14 +146,16 @@ static bool amdgpu_sync_add_later(struct amdgpu_sync *sync, struct dma_fence *f)
>   }
>   
>   /**
> - * amdgpu_sync_fence - remember to sync to this fence
> + * amdgpu_sync_fence_with_reason - remember to sync to this fence
>    *
>    * @sync: sync object to add fence to
>    * @f: fence to sync to
> + * @reason: why do we sync to this fence
>    *
>    * Add the fence to the sync object.
>    */
> -int amdgpu_sync_fence(struct amdgpu_sync *sync, struct dma_fence *f)
> +int amdgpu_sync_fence_with_reason(struct amdgpu_sync *sync, struct dma_fence *f,
> +				  const char *reason)
>   {
>   	struct amdgpu_sync_entry *e;
>   
> @@ -166,6 +169,8 @@ int amdgpu_sync_fence(struct amdgpu_sync *sync, struct dma_fence *f)
>   	if (!e)
>   		return -ENOMEM;
>   
> +	trace_dma_fence_used_as_dependency(f, reason);
> +
>   	hash_add(sync->fences, &e->node, f->context);
>   	e->fence = dma_fence_get(f);
>   	return 0;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h
> index cf1e9e858efd..52e7306801de 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h
> @@ -47,7 +47,9 @@ struct amdgpu_sync {
>   };
>   
>   void amdgpu_sync_create(struct amdgpu_sync *sync);
> -int amdgpu_sync_fence(struct amdgpu_sync *sync, struct dma_fence *f);
> +int amdgpu_sync_fence_with_reason(struct amdgpu_sync *sync, struct dma_fence *f,
> +				  const char *reason);
> +#define amdgpu_sync_fence(s, f) amdgpu_sync_fence_with_reason(s, f, __func__)
>   int amdgpu_sync_resv(struct amdgpu_device *adev, struct amdgpu_sync *sync,
>   		     struct dma_resv *resv, enum amdgpu_sync_mode mode,
>   		     void *owner);