I think we probably want to add a comment in v3d_submit_cl_ioctl when we setup the bin job explaining why we postpone looking up the BO list until the render job.
With that, this is: Reviewed-by: Iago Toral Quiroga <[email protected]> Also, unrelated to this patch: should we not use the same v3d_lookup_bos helper in the TFU submit path instead of having a loop looking up individual BOs one by one? Iago El jue, 18-06-2026 a las 12:03 -0300, Maíra Canal escribió: > A submission can expand into a chain of jobs (e.g. bin + render + > cache > clean), but v3d_lookup_bos() only looked up the user's BO list onto > the > *last* job of the submission. Every earlier job was left with > bo_count == 0 and an empty bo[] array. > > As a consequence, when implicit synchronization happens in > v3d_submit_lock_reservations(), earlier jobs get no implicit > dependencies at all, as the loop is gated on job->bo_count and > earlier > jobs don't have any BO attached to them. With that, the RENDER job > could > therefore be dispatched to the hardware and read a BO while another > context > was still writing it, leading to data corruption. > > Fix this by calling v3d_lookup_bos() for each job that references the > submission's BOs, so every job carries its own bo[]/bo_count and > picks > up the correct implicit dependencies during reservation locking. > > Fixes: dffa9b7a78c4 ("drm/v3d: Add missing implicit > synchronization.") > Signed-off-by: Maíra Canal <[email protected]> > --- > drivers/gpu/drm/v3d/v3d_submit.c | 36 +++++++++++++++++------------- > ------ > 1 file changed, 17 insertions(+), 19 deletions(-) > > diff --git a/drivers/gpu/drm/v3d/v3d_submit.c > b/drivers/gpu/drm/v3d/v3d_submit.c > index ee2ac2540ed5..18467e448c91 100644 > --- a/drivers/gpu/drm/v3d/v3d_submit.c > +++ b/drivers/gpu/drm/v3d/v3d_submit.c > @@ -68,7 +68,6 @@ v3d_submit_unlock_reservations(struct v3d_submit > *submit) > /** > * v3d_lookup_bos() - Sets up job->bo[] with the GEM objects > * referenced by the job. > - * @dev: DRM device > * @file_priv: DRM file for this fd > * @job: V3D job being set up > * @bo_handles: GEM handles > @@ -82,23 +81,19 @@ v3d_submit_unlock_reservations(struct v3d_submit > *submit) > * failure, because that will happen at `v3d_job_free()`. > */ > static int > -v3d_lookup_bos(struct v3d_submit *submit, u64 bo_handles, u32 > bo_count) > +v3d_lookup_bos(struct drm_file *file_priv, struct v3d_job *job, > + u64 bo_handles, u32 bo_count) > { > - struct v3d_job *last_job = submit->jobs[submit->job_count - > 1]; > - > - last_job->bo_count = bo_count; > - > - if (!last_job->bo_count) { > - /* See comment on bo_index for why we have to check > - * this. > - */ > - drm_warn(&submit->v3d->drm, "Rendering requires > BOs\n"); > + if (!bo_count) { > + drm_warn(&job->v3d->drm, "Rendering requires > BOs\n"); > return -EINVAL; > } > > - return drm_gem_objects_lookup(submit->file_priv, > + job->bo_count = bo_count; > + > + return drm_gem_objects_lookup(file_priv, > (void __user > *)(uintptr_t)bo_handles, > - last_job->bo_count, &last_job- > >bo); > + job->bo_count, &job->bo); > } > > static void > @@ -446,7 +441,8 @@ v3d_setup_csd_jobs_and_bos(struct v3d_submit > *submit, > if (IS_ERR(clean_job)) > return PTR_ERR(clean_job); > > - return v3d_lookup_bos(submit, args->bo_handles, args- > >bo_handle_count); > + return v3d_lookup_bos(submit->file_priv, &job->base, > + args->bo_handles, args- > >bo_handle_count); > } > > static void > @@ -1085,6 +1081,11 @@ v3d_submit_cl_ioctl(struct drm_device *dev, > void *data, > if (ret) > goto fail; > > + ret = v3d_lookup_bos(submit.file_priv, &render->base, > + args->bo_handles, args- > >bo_handle_count); > + if (ret) > + goto fail; > + > if (args->flags & DRM_V3D_SUBMIT_CL_FLUSH_CACHE) { > clean_job = v3d_submit_add_job(&submit, > V3D_CACHE_CLEAN); > if (IS_ERR(clean_job)) { > @@ -1097,10 +1098,6 @@ v3d_submit_cl_ioctl(struct drm_device *dev, > void *data, > if (ret) > goto fail; > > - ret = v3d_lookup_bos(&submit, args->bo_handles, args- > >bo_handle_count); > - if (ret) > - goto fail; > - > ret = v3d_submit_lock_reservations(&submit); > if (ret) > goto fail; > @@ -1359,7 +1356,8 @@ v3d_submit_cpu_ioctl(struct drm_device *dev, > void *data, > * the CSD and clean jobs in the case of indirect CSD job. > */ > if (args->bo_handle_count) { > - ret = v3d_lookup_bos(&submit, args->bo_handles, > args->bo_handle_count); > + ret = v3d_lookup_bos(submit.file_priv, &cpu_job- > >base, > + args->bo_handles, args- > >bo_handle_count); > if (ret) > goto fail; > } >
