On Tue, Feb 10, 2026 at 10:02 AM Srinivasan Shanmugam <[email protected]> wrote: > > dma_fence_wait(old, false) is not interruptible and cannot return an > error. Drop the unreachable error handling in amdgpu_fence_emit(). > > Since the function can no longer fail, convert amdgpu_fence_emit() to > return void and remove return value handling from all callers. > > Suggested-by: Christian König <[email protected]> > Cc: Alex Deucher <[email protected]> > Signed-off-by: Srinivasan Shanmugam <[email protected]> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c | 12 ++++-------- > drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c | 9 +-------- > drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h | 4 ++-- > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 7 ++----- > 4 files changed, 9 insertions(+), 23 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c > index 1054d66c54fa..4d2052347aa1 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c > @@ -107,16 +107,14 @@ static void amdgpu_fence_save_fence_wptr_end(struct > amdgpu_fence *af) > * @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 amdgpu_fence *af, > - unsigned int flags) > +void amdgpu_fence_emit(struct amdgpu_ring *ring, struct amdgpu_fence *af, > + unsigned int flags) > { > struct amdgpu_device *adev = ring->adev; > struct dma_fence *fence; > struct dma_fence __rcu **ptr; > uint32_t seq; > - int r; > > fence = &af->base; > af->ring = ring; > @@ -141,10 +139,8 @@ int amdgpu_fence_emit(struct amdgpu_ring *ring, struct > amdgpu_fence *af, > rcu_read_unlock(); > > if (old) { > - r = dma_fence_wait(old, false);
Might want a comment here as to why it's safe to ignore the return value. > + dma_fence_wait(old, false); > dma_fence_put(old); > - if (r) > - return r; > } > } > > @@ -155,7 +151,7 @@ int amdgpu_fence_emit(struct amdgpu_ring *ring, struct > amdgpu_fence *af, > */ > rcu_assign_pointer(*ptr, dma_fence_get(fence)); > > - return 0; > + return; This can be dropped since the function is void. Other than that, looks good to me. With my comments fixed, the series is: Reviewed-by: Alex Deucher <[email protected]> > } > > /** > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c > index bfa64cd7a62d..07b43a498592 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c > @@ -297,14 +297,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, af, fence_flags); > - if (r) { > - dev_err(adev->dev, "failed to emit fence (%d)\n", r); > - if (job && job->vmid) > - amdgpu_vmid_reset(adev, ring->vm_hub, job->vmid); > - amdgpu_ring_undo(ring); > - goto free_fence; > - } > + amdgpu_fence_emit(ring, af, fence_flags); > *f = &af->base; > /* get a ref for the job */ > if (job) > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h > b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h > index 1abd8fdb5cef..5a82db0888f0 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h > @@ -172,8 +172,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 amdgpu_fence *af, > - unsigned int flags); > +void amdgpu_fence_emit(struct amdgpu_ring *ring, 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); > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > index 807f8bcc7de5..2be65bd6b39c 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > @@ -783,8 +783,7 @@ int amdgpu_vm_flush(struct amdgpu_ring *ring, struct > amdgpu_job *job, > bool cleaner_shader_needed = false; > bool pasid_mapping_needed = false; > struct dma_fence *fence = NULL; > - unsigned int patch; > - int r; > + unsigned int patch = 0; > > if (amdgpu_vmid_had_gpu_reset(adev, id)) { > gds_switch_needed = true; > @@ -845,9 +844,7 @@ int amdgpu_vm_flush(struct amdgpu_ring *ring, struct > amdgpu_job *job, > } > > if (vm_flush_needed || pasid_mapping_needed || cleaner_shader_needed) > { > - r = amdgpu_fence_emit(ring, job->hw_vm_fence, 0); > - if (r) > - return r; > + amdgpu_fence_emit(ring, job->hw_vm_fence, 0); > fence = &job->hw_vm_fence->base; > /* get a ref for the job */ > dma_fence_get(fence); > -- > 2.34.1 >
