Re: [PATCH v2 3/6] amdgpu: use trace_dma_fence_sync_to in amdgpu_fence_sync
Am 13.02.24 um 16:50 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 The 'reason' param currently uses __func__ but I didn't add a macro for this because it'd be interesting to use more descriptive names for each use of amdgpu_fence_sync. Signed-off-by: Pierre-Eric Pelloux-Prayer --- drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 8 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 14 +++--- drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c | 8 +++- drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c | 4 ++-- drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c | 11 --- drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h | 3 ++- drivers/gpu/drm/amd/amdgpu/amdgpu_umsch_mm.c | 4 ++-- 7 files changed, 28 insertions(+), 24 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c index d17b2452cb1f..fde98e48c84b 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c @@ -491,7 +491,7 @@ static int vm_update_pds(struct amdgpu_vm *vm, struct amdgpu_sync *sync) if (ret) return ret; - return amdgpu_sync_fence(sync, vm->last_update); + return amdgpu_sync_fence(sync, vm->last_update, __func__); __func__ is used so often that it probably deserves a macro. Regards, Christian. } static uint64_t get_pte_flags(struct amdgpu_device *adev, struct kgd_mem *mem) @@ -1251,7 +1251,7 @@ static void unmap_bo_from_gpuvm(struct kgd_mem *mem, amdgpu_vm_clear_freed(adev, vm, _va->last_pt_update); - amdgpu_sync_fence(sync, bo_va->last_pt_update); + amdgpu_sync_fence(sync, bo_va->last_pt_update, __func__); } static int update_gpuvm_pte(struct kgd_mem *mem, @@ -1273,7 +1273,7 @@ static int update_gpuvm_pte(struct kgd_mem *mem, return ret; } - return amdgpu_sync_fence(sync, bo_va->last_pt_update); + return amdgpu_sync_fence(sync, bo_va->last_pt_update, __func__); } static int map_bo_to_gpuvm(struct kgd_mem *mem, @@ -2910,7 +2910,7 @@ int amdgpu_amdkfd_gpuvm_restore_process_bos(void *info, struct dma_fence **ef) } dma_resv_for_each_fence(, bo->tbo.base.resv, DMA_RESV_USAGE_KERNEL, fence) { - ret = amdgpu_sync_fence(_obj, fence); + ret = amdgpu_sync_fence(_obj, fence, __func__); if (ret) { pr_debug("Memory eviction: Sync BO fence failed. Try again\n"); goto validate_map_fail; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c index 6adeddfb3d56..6830892383c3 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c @@ -423,7 +423,7 @@ static int amdgpu_cs_p2_dependencies(struct amdgpu_cs_parser *p, dma_fence_put(old); } - r = amdgpu_sync_fence(>sync, fence); + r = amdgpu_sync_fence(>sync, fence, __func__); dma_fence_put(fence); if (r) return r; @@ -445,7 +445,7 @@ static int amdgpu_syncobj_lookup_and_add(struct amdgpu_cs_parser *p, return r; } - r = amdgpu_sync_fence(>sync, fence); + r = amdgpu_sync_fence(>sync, fence, __func__); dma_fence_put(fence); return r; } @@ -1101,7 +1101,7 @@ static int amdgpu_cs_vm_handling(struct amdgpu_cs_parser *p) if (r) return r; - r = amdgpu_sync_fence(>sync, fpriv->prt_va->last_pt_update); + r = amdgpu_sync_fence(>sync, fpriv->prt_va->last_pt_update, __func__); if (r) return r; @@ -1112,7 +1112,7 @@ static int amdgpu_cs_vm_handling(struct amdgpu_cs_parser *p) if (r) return r; - r = amdgpu_sync_fence(>sync, bo_va->last_pt_update); + r = amdgpu_sync_fence(>sync, bo_va->last_pt_update, __func__); if (r) return r; } @@ -1131,7 +1131,7 @@ static int amdgpu_cs_vm_handling(struct amdgpu_cs_parser *p) if (r) return r; - r = amdgpu_sync_fence(>sync, bo_va->last_pt_update); + r = amdgpu_sync_fence(>sync, bo_va->last_pt_update, __func__); if (r) return r; } @@ -1144,7 +1144,7 @@
[PATCH v2 3/6] amdgpu: use trace_dma_fence_sync_to in amdgpu_fence_sync
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 The 'reason' param currently uses __func__ but I didn't add a macro for this because it'd be interesting to use more descriptive names for each use of amdgpu_fence_sync. Signed-off-by: Pierre-Eric Pelloux-Prayer --- drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 8 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 14 +++--- drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c | 8 +++- drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c | 4 ++-- drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c | 11 --- drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h | 3 ++- drivers/gpu/drm/amd/amdgpu/amdgpu_umsch_mm.c | 4 ++-- 7 files changed, 28 insertions(+), 24 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c index d17b2452cb1f..fde98e48c84b 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c @@ -491,7 +491,7 @@ static int vm_update_pds(struct amdgpu_vm *vm, struct amdgpu_sync *sync) if (ret) return ret; - return amdgpu_sync_fence(sync, vm->last_update); + return amdgpu_sync_fence(sync, vm->last_update, __func__); } static uint64_t get_pte_flags(struct amdgpu_device *adev, struct kgd_mem *mem) @@ -1251,7 +1251,7 @@ static void unmap_bo_from_gpuvm(struct kgd_mem *mem, amdgpu_vm_clear_freed(adev, vm, _va->last_pt_update); - amdgpu_sync_fence(sync, bo_va->last_pt_update); + amdgpu_sync_fence(sync, bo_va->last_pt_update, __func__); } static int update_gpuvm_pte(struct kgd_mem *mem, @@ -1273,7 +1273,7 @@ static int update_gpuvm_pte(struct kgd_mem *mem, return ret; } - return amdgpu_sync_fence(sync, bo_va->last_pt_update); + return amdgpu_sync_fence(sync, bo_va->last_pt_update, __func__); } static int map_bo_to_gpuvm(struct kgd_mem *mem, @@ -2910,7 +2910,7 @@ int amdgpu_amdkfd_gpuvm_restore_process_bos(void *info, struct dma_fence **ef) } dma_resv_for_each_fence(, bo->tbo.base.resv, DMA_RESV_USAGE_KERNEL, fence) { - ret = amdgpu_sync_fence(_obj, fence); + ret = amdgpu_sync_fence(_obj, fence, __func__); if (ret) { pr_debug("Memory eviction: Sync BO fence failed. Try again\n"); goto validate_map_fail; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c index 6adeddfb3d56..6830892383c3 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c @@ -423,7 +423,7 @@ static int amdgpu_cs_p2_dependencies(struct amdgpu_cs_parser *p, dma_fence_put(old); } - r = amdgpu_sync_fence(>sync, fence); + r = amdgpu_sync_fence(>sync, fence, __func__); dma_fence_put(fence); if (r) return r; @@ -445,7 +445,7 @@ static int amdgpu_syncobj_lookup_and_add(struct amdgpu_cs_parser *p, return r; } - r = amdgpu_sync_fence(>sync, fence); + r = amdgpu_sync_fence(>sync, fence, __func__); dma_fence_put(fence); return r; } @@ -1101,7 +1101,7 @@ static int amdgpu_cs_vm_handling(struct amdgpu_cs_parser *p) if (r) return r; - r = amdgpu_sync_fence(>sync, fpriv->prt_va->last_pt_update); + r = amdgpu_sync_fence(>sync, fpriv->prt_va->last_pt_update, __func__); if (r) return r; @@ -1112,7 +1112,7 @@ static int amdgpu_cs_vm_handling(struct amdgpu_cs_parser *p) if (r) return r; - r = amdgpu_sync_fence(>sync, bo_va->last_pt_update); + r = amdgpu_sync_fence(>sync, bo_va->last_pt_update, __func__); if (r) return r; } @@ -1131,7 +1131,7 @@ static int amdgpu_cs_vm_handling(struct amdgpu_cs_parser *p) if (r) return r; - r = amdgpu_sync_fence(>sync, bo_va->last_pt_update); + r = amdgpu_sync_fence(>sync, bo_va->last_pt_update, __func__); if (r) return r; } @@ -1144,7 +1144,7 @@ static int amdgpu_cs_vm_handling(struct amdgpu_cs_parser *p) if (r) return r; - r =