El dom, 04-10-2026 a las 12:42 -0300, Maíra Canal escribió:
> vc4_attach_fences() adds the job fence to the reservation of every BO
> the
> job touches, which is a O(bo_count) walk under job_lock. It was there
> because vc4_exec_info could be freed by the job done worker the
> moment the
> lock was dropped.
>
> As vc4_exec_info is refcounted now, hand the fences over after the
> unlock.
>
> Note that the render done handler dropped the fence as soon as the
> job
> completed, so the walk could find exec->fence already cleared. Leave
> the
> fence to be dropped by the release path. The submit path no longer
> needs
> a reference of its own to publish the fence through the sync object
> either.
>
> Signed-off-by: Maíra Canal <[email protected]>
> ---
> drivers/gpu/drm/vc4/vc4_gem.c | 23 ++++++-----------------
> drivers/gpu/drm/vc4/vc4_irq.c | 5 +----
> 2 files changed, 7 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_gem.c
> b/drivers/gpu/drm/vc4/vc4_gem.c
> index 9cf343297f83..ae76868663ea 100644
> --- a/drivers/gpu/drm/vc4/vc4_gem.c
> +++ b/drivers/gpu/drm/vc4/vc4_gem.c
> @@ -639,14 +639,6 @@ vc4_queue_submit(struct drm_device *dev, struct
> vc4_exec_info *exec,
>
> kref_get(&exec->refcount);
>
> - /* The job can complete and drop exec->fence as soon as
> job_lock is
> - * released, so hold our own reference.
> - */
> - if (out_sync)
> - dma_fence_get(&fence->base);
> -
> - vc4_attach_fences(exec);
> -
> list_add_tail(&exec->head, &vc4->bin_job_list);
>
> /* If no bin job was executing and if the render job (if
> any) has the
> @@ -663,12 +655,12 @@ vc4_queue_submit(struct drm_device *dev, struct
> vc4_exec_info *exec,
>
> spin_unlock_irqrestore(&vc4->job_lock, irqflags);
>
> + vc4_attach_fences(exec);
> +
Similar question as before: doesn't this mean we may submit this job
before we have installed the fences in the BO reservations? Wouldn't
that mean there is a small window where the BOs look idle to other
users when they are in fact being used by the GPU?
Iago
> drm_exec_fini(exec_ctx);
>
> - if (out_sync) {
> + if (out_sync)
> drm_syncobj_replace_fence(out_sync, &fence->base);
> - dma_fence_put(&fence->base);
> - }
>
> return 0;
> }
> @@ -862,13 +854,10 @@ vc4_release_exec(struct kref *ref)
> unsigned long irqflags;
> unsigned i;
>
> - /* If we got force-completed because of GPU reset rather
> than
> - * through our IRQ handler, signal the fence now.
> + /* The render done handler signals the fence, so only drop
> the
> + * reference here.
> */
> - if (exec->fence) {
> - dma_fence_signal(exec->fence);
> - dma_fence_put(exec->fence);
> - }
> + dma_fence_put(exec->fence);
>
> if (exec->bo) {
> for (i = 0; i < exec->bo_count; i++) {
> diff --git a/drivers/gpu/drm/vc4/vc4_irq.c
> b/drivers/gpu/drm/vc4/vc4_irq.c
> index 6ced2835dfdc..241249b6582b 100644
> --- a/drivers/gpu/drm/vc4/vc4_irq.c
> +++ b/drivers/gpu/drm/vc4/vc4_irq.c
> @@ -193,11 +193,8 @@ vc4_irq_finish_render_job(struct drm_device
> *dev)
> else if (nextbin && nextbin->perfmon != exec->perfmon)
> vc4_submit_next_bin_job(dev);
>
> - if (exec->fence) {
> + if (exec->fence)
> dma_fence_signal_locked(exec->fence);
> - dma_fence_put(exec->fence);
> - exec->fence = NULL;
> - }
>
> wake_up_all(&vc4->job_wait_queue);
> schedule_work(&vc4->job_done_work);
>
