On 6/17/25 05:07, Alex Deucher wrote:
> What we actually care about is the amdgpu_fence object
> so pass that in explicitly to avoid possible mistakes
> in the future.
>
> The job_run_counter handling can be safely removed at this
> point as we no longer support job resubmission.
>
> Signed-off-by: Alex Deucher <[email protected]>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c | 36 +++++++++--------------
> drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c | 5 +++-
> drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h | 4 +--
> 3 files changed, 20 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
> index 569e0e5373927..e88848c14491a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
> @@ -114,14 +114,14 @@ static u32 amdgpu_fence_read(struct amdgpu_ring *ring)
> *
> * @ring: ring the fence is associated with
> * @f: resulting fence object
> - * @job: job the fence is embedded in
> + * @af: amdgpu fence input
> * @flags: flags to pass into the subordinate .emit_fence() call
> *
> * Emits a fence command on the requested ring (all asics).
> * Returns 0 on success, -ENOMEM on failure.
> */
> -int amdgpu_fence_emit(struct amdgpu_ring *ring, struct dma_fence **f, struct
> amdgpu_job *job,
> - unsigned int flags)
> +int amdgpu_fence_emit(struct amdgpu_ring *ring, struct dma_fence **f,
> + struct amdgpu_fence *af, unsigned int flags)
> {
> struct amdgpu_device *adev = ring->adev;
> struct dma_fence *fence;
> @@ -130,36 +130,28 @@ int amdgpu_fence_emit(struct amdgpu_ring *ring, struct
> dma_fence **f, struct amd
> uint32_t seq;
> int r;
>
> - if (job == NULL) {
> - /* create a sperate hw fence */
> + if (!af) {
> + /* create a separate hw fence */
> am_fence = kmem_cache_alloc(amdgpu_fence_slab, GFP_ATOMIC);
> if (am_fence == NULL)
> return -ENOMEM;
I think we should remove the output parameter as well.
An amdgpu_fence can be trivially allocated by the caller.
Apart from that looks good to me.
Regards,
Christian.
> } else {
> - /* take use of job-embedded fence */
> - am_fence = &job->hw_fence;
> + am_fence = af;
> }
> fence = &am_fence->base;
> am_fence->ring = ring;
>
> seq = ++ring->fence_drv.sync_seq;
> - if (job && job->job_run_counter) {
> - /* reinit seq for resubmitted jobs */
> - fence->seqno = seq;
> - /* TO be inline with external fence creation and other drivers
> */
> + if (af) {
> + dma_fence_init(fence, &amdgpu_job_fence_ops,
> + &ring->fence_drv.lock,
> + adev->fence_context + ring->idx, seq);
> + /* Against remove in amdgpu_job_{free, free_cb} */
> dma_fence_get(fence);
> } else {
> - if (job) {
> - dma_fence_init(fence, &amdgpu_job_fence_ops,
> - &ring->fence_drv.lock,
> - adev->fence_context + ring->idx, seq);
> - /* Against remove in amdgpu_job_{free, free_cb} */
> - dma_fence_get(fence);
> - } else {
> - dma_fence_init(fence, &amdgpu_fence_ops,
> - &ring->fence_drv.lock,
> - adev->fence_context + ring->idx, seq);
> - }
> + dma_fence_init(fence, &amdgpu_fence_ops,
> + &ring->fence_drv.lock,
> + adev->fence_context + ring->idx, seq);
> }
>
> amdgpu_ring_emit_fence(ring, ring->fence_drv.gpu_addr,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> index 802743efa3b39..206b70acb29a0 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> @@ -128,6 +128,7 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned
> int num_ibs,
> struct amdgpu_device *adev = ring->adev;
> struct amdgpu_ib *ib = &ibs[0];
> struct dma_fence *tmp = NULL;
> + struct amdgpu_fence *af;
> bool need_ctx_switch;
> struct amdgpu_vm *vm;
> uint64_t fence_ctx;
> @@ -154,6 +155,7 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned
> int num_ibs,
> csa_va = job->csa_va;
> gds_va = job->gds_va;
> init_shadow = job->init_shadow;
> + af = &job->hw_fence;
> } else {
> vm = NULL;
> fence_ctx = 0;
> @@ -161,6 +163,7 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned
> int num_ibs,
> csa_va = 0;
> gds_va = 0;
> init_shadow = false;
> + af = NULL;
> }
>
> if (!ring->sched.ready) {
> @@ -282,7 +285,7 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned
> int num_ibs,
> amdgpu_ring_init_cond_exec(ring, ring->cond_exe_gpu_addr);
> }
>
> - r = amdgpu_fence_emit(ring, f, job, fence_flags);
> + r = amdgpu_fence_emit(ring, f, af, fence_flags);
> if (r) {
> dev_err(adev->dev, "failed to emit fence (%d)\n", r);
> if (job && job->vmid)
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> index e1f25218943a4..9ae522baad8e7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> @@ -157,8 +157,8 @@ void amdgpu_fence_driver_hw_init(struct amdgpu_device
> *adev);
> void amdgpu_fence_driver_hw_fini(struct amdgpu_device *adev);
> int amdgpu_fence_driver_sw_init(struct amdgpu_device *adev);
> void amdgpu_fence_driver_sw_fini(struct amdgpu_device *adev);
> -int amdgpu_fence_emit(struct amdgpu_ring *ring, struct dma_fence **fence,
> struct amdgpu_job *job,
> - unsigned flags);
> +int amdgpu_fence_emit(struct amdgpu_ring *ring, struct dma_fence **f,
> + struct amdgpu_fence *af, unsigned int flags);
> int amdgpu_fence_emit_polling(struct amdgpu_ring *ring, uint32_t *s,
> uint32_t timeout);
> bool amdgpu_fence_process(struct amdgpu_ring *ring);
