| Message ID | 20240216151006.475077-4-pierre-eric.pelloux-prayer@amd.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | dma-fence, drm, amdgpu new trace events | expand |
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);
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);
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> --- 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(-)