dri-devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
To: "Maíra Canal" <mcanal@igalia.com>,
	"Melissa Wen" <mwen@igalia.com>, "Iago Toral" <itoral@igalia.com>,
	"Maarten Lankhorst" <maarten.lankhorst@linux.intel.com>,
	"Maxime Ripard" <mripard@kernel.org>,
	"Thomas Zimmermann" <tzimmermann@suse.de>,
	"David Airlie" <airlied@gmail.com>,
	"Simona Vetter" <simona@ffwll.ch>,
	"Christian König" <ckoenig.leichtzumerken@gmail.com>
Cc: kernel-dev@igalia.com, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH v3 10/14] drm/v3d: Convert submit helpers to operate on struct v3d_submit
Date: Thu, 4 Jun 2026 10:49:35 +0100	[thread overview]
Message-ID: <1dc4a2e7-b30a-490e-bf98-ebdaced60c0b@igalia.com> (raw)
In-Reply-To: <20260603-v3d-sched-misc-fixes-v3-10-d7114bba55a0@igalia.com>


On 03/06/2026 23:25, Maíra Canal wrote:
> Generalize the submission helpers so they act on a whole struct v3d_submit
> (the entire job chain) rather than on individual jobs and a drm_exec. This
> lets a submission of several chained jobs be locked, fenced, and finalized
> as a single unit, and is the groundwork for collapsing the indirect CSD
> path into one chain.
> 
> The following helpers were generalized:
> 
>    - v3d_lookup_bos()
>    - v3d_lock_bo_reservations() (renamed to v3d_submit_lock_reservations()):
>    - v3d_attach_fences_and_unlock_reservation()
>    - v3d_setup_csd_jobs_and_bos()
> 
> Now, the locking helper now iterates over all jobs and locks the union of
> their BOs under one DRM exec, using DRM_EXEC_IGNORE_DUPLICATES to tolerate
> shared BO references. The fence-attach helper similarly walks every job
> and attaches the chain's last fence to all touched BOs.
> 
> Also, v3d_submit_jobs() becomes the single submit-and-finalize entry
> point and callers no longer need to open-code fence attachment,
> reservation unlocking, etc.
> 
> Update CL/TFU/CSD/CPU ioctls to use the new helper signatures. The CPU
> ioctl still uses two struct v3d_submit instances (one for the CPU job,
> one for the indirect CSD jobs) and keeps its manual two-pass fence-attach
> flow. Converting the indirect CSD path into the unified chain is done in
> the next commit.
> 
> No functional change.
> 
> Signed-off-by: Maíra Canal <mcanal@igalia.com>
> ---
>   drivers/gpu/drm/v3d/v3d_submit.c | 340 +++++++++++++++------------------------
>   1 file changed, 128 insertions(+), 212 deletions(-)
> 
> diff --git a/drivers/gpu/drm/v3d/v3d_submit.c b/drivers/gpu/drm/v3d/v3d_submit.c
> index 64eba912dc64..62c23feb8fbb 100644
> --- a/drivers/gpu/drm/v3d/v3d_submit.c
> +++ b/drivers/gpu/drm/v3d/v3d_submit.c
> @@ -20,32 +20,51 @@
>    * to v3d, so we don't attach dma-buf fences to them.
>    */
>   static int
> -v3d_lock_bo_reservations(struct v3d_job *job, struct drm_exec *exec)
> +v3d_submit_lock_reservations(struct v3d_submit *submit)
>   {
> -	int i, ret;
> +	int i, j, ret;
>   
> -	drm_exec_init(exec, DRM_EXEC_INTERRUPTIBLE_WAIT, job->bo_count);
> -	drm_exec_until_all_locked(exec) {
> -		ret = drm_exec_prepare_array(exec, job->bo, job->bo_count, 1);
> -	}
> +	drm_exec_init(&submit->exec,
> +		      DRM_EXEC_INTERRUPTIBLE_WAIT | DRM_EXEC_IGNORE_DUPLICATES, 0);
> +	drm_exec_until_all_locked(&submit->exec) {
> +		for (i = 0; i < submit->job_count; i++) {
> +			struct v3d_job *job = submit->jobs[i];
>   
> -	if (ret)
> -		goto fail;
> -
> -	for (i = 0; i < job->bo_count; i++) {
> -		ret = drm_sched_job_add_implicit_dependencies(&job->base,
> -							      job->bo[i], true);
> +			ret = drm_exec_prepare_array(&submit->exec, job->bo,
> +						     job->bo_count, 1);
> +			if (ret)
> +				break;
> +		}
> +		drm_exec_retry_on_contention(&submit->exec);
>   		if (ret)
>   			goto fail;
>   	}
>   
> +	for (i = 0; i < submit->job_count; i++) {
> +		struct v3d_job *job = submit->jobs[i];
> +
> +		for (j = 0; j < job->bo_count; j++) {
> +			ret = drm_sched_job_add_implicit_dependencies(&job->base,
> +								      job->bo[j],
> +								      true);
> +			if (ret)
> +				goto fail;
> +		}
> +	}
> +
>   	return 0;
>   
>   fail:
> -	drm_exec_fini(exec);
> +	drm_exec_fini(&submit->exec);
>   	return ret;
>   }
>   
> +static void
> +v3d_submit_unlock_reservations(struct v3d_submit *submit)
> +{
> +	drm_exec_fini(&submit->exec);
> +}
> +
>   /**
>    * v3d_lookup_bos() - Sets up job->bo[] with the GEM objects
>    * referenced by the job.
> @@ -63,25 +82,23 @@ v3d_lock_bo_reservations(struct v3d_job *job, struct drm_exec *exec)
>    * failure, because that will happen at `v3d_job_free()`.
>    */
>   static int
> -v3d_lookup_bos(struct drm_device *dev,
> -	       struct drm_file *file_priv,
> -	       struct v3d_job *job,
> -	       u64 bo_handles,
> -	       u32 bo_count)
> +v3d_lookup_bos(struct v3d_submit *submit, u64 bo_handles, u32 bo_count)
>   {
> -	job->bo_count = bo_count;
> +	struct v3d_job *last_job = submit->jobs[submit->job_count - 1];
>   
> -	if (!job->bo_count) {
> +	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(dev, "Rendering requires BOs\n");
> +		drm_warn(&submit->v3d->drm, "Rendering requires BOs\n");
>   		return -EINVAL;
>   	}
>   
> -	return drm_gem_objects_lookup(file_priv,
> +	return drm_gem_objects_lookup(submit->file_priv,
>   				      (void __user *)(uintptr_t)bo_handles,
> -				      job->bo_count, &job->bo);
> +				      last_job->bo_count, &last_job->bo);
>   }
>   
>   static void
> @@ -160,25 +177,6 @@ void v3d_job_put(struct v3d_job *job)
>   	kref_put(&job->refcount, job->free);
>   }
>   
> -static int
> -v3d_job_allocate(struct v3d_dev *v3d, void **container, size_t size)
> -{
> -	*container = kcalloc(1, size, GFP_KERNEL);
> -	if (!*container) {
> -		drm_err(&v3d->drm, "Cannot allocate memory for V3D job.\n");
> -		return -ENOMEM;
> -	}
> -
> -	return 0;
> -}
> -
> -static void
> -v3d_job_deallocate(void **container)
> -{
> -	kfree(*container);
> -	*container = NULL;
> -}
> -
>   static int
>   v3d_job_add_syncobjs(struct v3d_job *job, struct drm_file *file_priv,
>   		     u32 in_sync, struct v3d_submit_ext *se)
> @@ -219,48 +217,6 @@ v3d_job_add_syncobjs(struct v3d_job *job, struct drm_file *file_priv,
>   	return 0;
>   }
>   
> -static int
> -v3d_job_init(struct v3d_dev *v3d, struct drm_file *file_priv,
> -	     struct v3d_job *job, void (*free)(struct kref *ref),
> -	     u32 in_sync, struct v3d_submit_ext *se, enum v3d_queue queue)
> -{
> -	struct v3d_file_priv *v3d_priv = file_priv->driver_priv;
> -	int ret;
> -
> -	job->v3d = v3d;
> -	job->free = free;
> -	job->queue = queue;
> -	job->file_priv = v3d_priv;
> -
> -	ret = drm_sched_job_init(&job->base, &v3d_priv->sched_entity[queue],
> -				 1, v3d_priv, file_priv->client_id);
> -	if (ret)
> -		return ret;
> -
> -	ret = v3d_job_add_syncobjs(job, file_priv, in_sync, se);
> -	if (ret)
> -		goto fail_job_init;
> -
> -	/* CPU jobs don't require hardware resources */
> -	if (queue != V3D_CPU) {
> -		ret = v3d_pm_runtime_get(v3d);
> -		if (ret)
> -			goto fail_job_init;
> -		job->has_pm_ref = true;
> -	}
> -
> -	kref_init(&job->refcount);
> -
> -	job->client_stats = v3d_stats_get(v3d_priv->stats[queue]);
> -	job->global_stats = v3d_stats_get(v3d->queue[queue].stats);
> -
> -	return 0;
> -
> -fail_job_init:
> -	drm_sched_job_cleanup(&job->base);
> -	return ret;
> -}
> -
>   static const struct {
>   	size_t size;
>   	void (*free)(struct kref *ref);
> @@ -363,31 +319,34 @@ v3d_attach_perfmon_to_jobs(struct v3d_submit *submit, u32 perfmon_id)
>   }
>   
>   static void
> -v3d_attach_fences_and_unlock_reservation(struct drm_file *file_priv,
> -					 struct v3d_job *job,
> -					 struct drm_exec *exec,
> -					 u32 out_sync,
> -					 struct v3d_submit_ext *se,
> -					 struct dma_fence *done_fence)
> +v3d_attach_fences_and_unlock_reservation(struct v3d_submit *submit,
> +					 u32 out_sync, struct v3d_submit_ext *se)
>   {
> -	struct drm_syncobj *sync_out;
>   	bool has_multisync = se && (se->flags & DRM_V3D_EXT_ID_MULTI_SYNC);
> -	int i;
> +	struct v3d_job *last_job = submit->jobs[submit->job_count - 1];
> +	struct drm_syncobj *sync_out;
>   
> -	for (i = 0; i < job->bo_count; i++) {
> -		/* XXX: Use shared fences for read-only objects. */
> -		dma_resv_add_fence(job->bo[i]->resv, job->done_fence,
> -				   DMA_RESV_USAGE_WRITE);
> +	/* The submission's last fence covers the entire submission. Attach it
> +	 * to every BO touched by any job in the submission.
> +	 */
> +	for (int i = 0; i < submit->job_count; i++) {
> +		struct v3d_job *job = submit->jobs[i];
> +
> +		for (int j = 0; j < job->bo_count; j++) {
> +			/* XXX: Use shared fences for read-only objects. */
> +			dma_resv_add_fence(job->bo[j]->resv, last_job->done_fence,
> +					   DMA_RESV_USAGE_WRITE);
> +		}
>   	}
>   
> -	drm_exec_fini(exec);
> +	v3d_submit_unlock_reservations(submit);
>   
>   	/* Update the return sync object for the job */
>   	/* If it only supports a single signal semaphore*/
>   	if (!has_multisync) {
> -		sync_out = drm_syncobj_find(file_priv, out_sync);
> +		sync_out = drm_syncobj_find(submit->file_priv, out_sync);
>   		if (sync_out) {
> -			drm_syncobj_replace_fence(sync_out, done_fence);
> +			drm_syncobj_replace_fence(sync_out, last_job->done_fence);
>   			drm_syncobj_put(sync_out);
>   		}
>   		return;
> @@ -395,9 +354,9 @@ v3d_attach_fences_and_unlock_reservation(struct drm_file *file_priv,
>   
>   	/* If multiple semaphores extension is supported */
>   	if (se->out_sync_count) {
> -		for (i = 0; i < se->out_sync_count; i++) {
> +		for (int i = 0; i < se->out_sync_count; i++) {
>   			drm_syncobj_replace_fence(se->out_syncs[i].syncobj,
> -						  done_fence);
> +						  last_job->done_fence);
>   			drm_syncobj_put(se->out_syncs[i].syncobj);
>   		}
>   		kvfree(se->out_syncs);
> @@ -418,7 +377,8 @@ v3d_push_job(struct v3d_job *job)
>   }
>   
>   static int
> -v3d_submit_jobs(struct v3d_submit *submit)
> +v3d_submit_jobs(struct v3d_submit *submit, u32 out_sync,
> +		struct v3d_submit_ext *se)
>   {
>   	struct v3d_dev *v3d = submit->v3d;
>   	int ret = 0;
> @@ -438,52 +398,42 @@ v3d_submit_jobs(struct v3d_submit *submit)
>   		}
>   	}
>   
> +	mutex_unlock(&v3d->sched_lock);
> +
> +	v3d_attach_fences_and_unlock_reservation(submit, out_sync, se);
> +	v3d_submit_put_jobs(submit);
> +
> +	return 0;
> +
>   err:
>   	mutex_unlock(&v3d->sched_lock);
>   	return ret;
>   }
>   
>   static int
> -v3d_setup_csd_jobs_and_bos(struct drm_file *file_priv,
> -			   struct v3d_dev *v3d,
> +v3d_setup_csd_jobs_and_bos(struct v3d_submit *submit,
>   			   struct drm_v3d_submit_csd *args,
> -			   struct v3d_csd_job **job,
> -			   struct v3d_job **clean_job,
> -			   struct v3d_submit_ext *se,
> -			   struct drm_exec *exec)
> +			   struct v3d_submit_ext *se)
>   {
> +	struct v3d_csd_job *job;
> +	struct v3d_job *clean_job;
>   	int ret;
>   
> -	ret = v3d_job_allocate(v3d, (void *)job, sizeof(**job));
> +	job = (struct v3d_csd_job *) v3d_submit_add_job(submit, V3D_CSD);
> +	if (IS_ERR(job))
> +		return PTR_ERR(job);
> +
> +	ret = v3d_job_add_syncobjs(&job->base, submit->file_priv, args->in_sync, se);
>   	if (ret)
>   		return ret;
>   
> -	ret = v3d_job_init(v3d, file_priv, &(*job)->base,
> -			   v3d_job_free, args->in_sync, se, V3D_CSD);
> -	if (ret) {
> -		v3d_job_deallocate((void *)job);
> -		return ret;
> -	}
> +	job->args = *args;
>   
> -	ret = v3d_job_allocate(v3d, (void *)clean_job, sizeof(**clean_job));
> -	if (ret)
> -		return ret;
> +	clean_job = v3d_submit_add_job(submit, V3D_CACHE_CLEAN);
> +	if (IS_ERR(clean_job))
> +		return PTR_ERR(clean_job);
>   
> -	ret = v3d_job_init(v3d, file_priv, *clean_job,
> -			   v3d_job_free, 0, NULL, V3D_CACHE_CLEAN);
> -	if (ret) {
> -		v3d_job_deallocate((void *)clean_job);
> -		return ret;
> -	}
> -
> -	(*job)->args = *args;
> -
> -	ret = v3d_lookup_bos(&v3d->drm, file_priv, *clean_job,
> -			     args->bo_handles, args->bo_handle_count);
> -	if (ret)
> -		return ret;
> -
> -	return v3d_lock_bo_reservations(*clean_job, exec);
> +	return v3d_lookup_bos(submit, args->bo_handles, args->bo_handle_count);
>   }
>   
>   static void
> @@ -1123,33 +1073,22 @@ v3d_submit_cl_ioctl(struct drm_device *dev, void *data,
>   	if (ret)
>   		goto fail;
>   
> -	ret = v3d_lookup_bos(dev, file_priv,
> -			     submit.jobs[submit.job_count - 1],
> -			     args->bo_handles, args->bo_handle_count);
> +	ret = v3d_lookup_bos(&submit, args->bo_handles, args->bo_handle_count);
>   	if (ret)
>   		goto fail;
>   
> -	ret = v3d_lock_bo_reservations(submit.jobs[submit.job_count - 1],
> -				       &submit.exec);
> +	ret = v3d_submit_lock_reservations(&submit);
>   	if (ret)
>   		goto fail;
>   
> -	ret = v3d_submit_jobs(&submit);
> +	ret = v3d_submit_jobs(&submit, args->out_sync, &se);
>   	if (ret)
>   		goto fail_unreserve;
>   
> -	v3d_attach_fences_and_unlock_reservation(file_priv,
> -						 submit.jobs[submit.job_count - 1],
> -						 &submit.exec,
> -						 args->out_sync, &se,
> -						 submit.jobs[submit.job_count - 1]->done_fence);
> -
> -	v3d_submit_put_jobs(&submit);
> -
>   	return 0;
>   
>   fail_unreserve:
> -	drm_exec_fini(&submit.exec);
> +	v3d_submit_unlock_reservations(&submit);
>   fail:
>   	v3d_submit_cleanup_jobs(&submit);
>   	v3d_put_multisync_post_deps(&se);
> @@ -1228,25 +1167,18 @@ v3d_submit_tfu_ioctl(struct drm_device *dev, void *data,
>   		job->base.bo[job->base.bo_count] = bo;
>   	}
>   
> -	ret = v3d_lock_bo_reservations(&job->base, &submit.exec);
> +	ret = v3d_submit_lock_reservations(&submit);
>   	if (ret)
>   		goto fail;
>   
> -	ret = v3d_submit_jobs(&submit);
> +	ret = v3d_submit_jobs(&submit, args->out_sync, &se);
>   	if (ret)
>   		goto fail_unreserve;
>   
> -	v3d_attach_fences_and_unlock_reservation(file_priv,
> -						 &job->base, &submit.exec,
> -						 args->out_sync, &se,
> -						 job->base.done_fence);
> -
> -	v3d_submit_put_jobs(&submit);
> -
>   	return 0;
>   
>   fail_unreserve:
> -	drm_exec_fini(&submit.exec);
> +	v3d_submit_unlock_reservations(&submit);
>   fail:
>   	v3d_submit_cleanup_jobs(&submit);
>   	v3d_put_multisync_post_deps(&se);
> @@ -1270,8 +1202,6 @@ v3d_submit_csd_ioctl(struct drm_device *dev, void *data,
>   	struct v3d_submit submit = { .v3d = to_v3d_dev(dev), .file_priv = file_priv };
>   	struct drm_v3d_submit_csd *args = data;
>   	struct v3d_submit_ext se = {0};
> -	struct v3d_csd_job *job = NULL;
> -	struct v3d_job *clean_job = NULL;
>   	int ret;
>   
>   	trace_v3d_submit_csd_ioctl(dev, args->cfg[5], args->cfg[6]);
> @@ -1297,34 +1227,26 @@ v3d_submit_csd_ioctl(struct drm_device *dev, void *data,
>   		}
>   	}
>   
> -	ret = v3d_setup_csd_jobs_and_bos(file_priv, submit.v3d, args,
> -					 &job, &clean_job, &se, &submit.exec);
> +	ret = v3d_setup_csd_jobs_and_bos(&submit, args, &se);
>   	if (ret)
>   		goto fail;
>   
> -	submit.jobs[submit.job_count++] = &job->base;
> -	submit.jobs[submit.job_count++] = clean_job;
> -
>   	ret = v3d_attach_perfmon_to_jobs(&submit, args->perfmon_id);
>   	if (ret)
> -		goto fail_unreserve;
> +		goto fail;
>   
> -	ret = v3d_submit_jobs(&submit);
> +	ret = v3d_submit_lock_reservations(&submit);
> +	if (ret)
> +		goto fail;
> +
> +	ret = v3d_submit_jobs(&submit, args->out_sync, &se);
>   	if (ret)
>   		goto fail_unreserve;
>   
> -	v3d_attach_fences_and_unlock_reservation(file_priv,
> -						 clean_job,
> -						 &submit.exec,
> -						 args->out_sync, &se,
> -						 clean_job->done_fence);
> -
> -	v3d_submit_put_jobs(&submit);
> -
>   	return 0;
>   
>   fail_unreserve:
> -	drm_exec_fini(&submit.exec);
> +	v3d_submit_unlock_reservations(&submit);
>   fail:
>   	v3d_submit_cleanup_jobs(&submit);
>   	v3d_put_multisync_post_deps(&se);
> @@ -1355,13 +1277,14 @@ v3d_submit_cpu_ioctl(struct drm_device *dev, void *data,
>   		     struct drm_file *file_priv)
>   {
>   	struct v3d_dev *v3d = to_v3d_dev(dev);
> +	struct v3d_submit submit = { .v3d = to_v3d_dev(dev), .file_priv = file_priv };
> +	struct v3d_submit indirect_submit = { .v3d = to_v3d_dev(dev), .file_priv = file_priv };
>   	struct drm_v3d_submit_cpu *args = data;
>   	struct v3d_submit_ext se = {0};
>   	struct v3d_submit_ext *out_se = NULL;

No need to init.

>   	struct v3d_cpu_job *cpu_job = NULL;
>   	struct v3d_csd_job *csd_job = NULL;
>   	struct v3d_job *clean_job = NULL;

Ditto for above three AFAICT.

> -	struct drm_exec exec;
>   	int ret;
>   
>   	if (args->flags && !(args->flags & DRM_V3D_SUBMIT_EXTENSION)) {
> @@ -1369,9 +1292,9 @@ v3d_submit_cpu_ioctl(struct drm_device *dev, void *data,
>   		return -EINVAL;
>   	}
>   
> -	ret = v3d_job_allocate(v3d, (void *)&cpu_job, sizeof(*cpu_job));
> -	if (ret)
> -		return ret;
> +	cpu_job = (struct v3d_cpu_job *) v3d_submit_add_job(&submit, V3D_CPU);
> +	if (IS_ERR(cpu_job))
> +		return PTR_ERR(cpu_job);
>   
>   	if (args->flags & DRM_V3D_SUBMIT_EXTENSION) {
>   		ret = v3d_get_extensions(file_priv, args->extensions, &se, cpu_job);
> @@ -1396,34 +1319,35 @@ v3d_submit_cpu_ioctl(struct drm_device *dev, void *data,
>   
>   	trace_v3d_submit_cpu_ioctl(&v3d->drm, cpu_job->job_type);
>   
> -	ret = v3d_job_init(v3d, file_priv, &cpu_job->base,
> -			   v3d_cpu_job_free, 0, &se, V3D_CPU);
> -	if (ret) {
> -		v3d_job_deallocate((void *)&cpu_job);
> +	ret = v3d_job_add_syncobjs(&cpu_job->base, file_priv, 0, &se);
> +	if (ret)
>   		goto fail;
> -	}
>   
>   	if (cpu_job->job_type == V3D_CPU_JOB_TYPE_INDIRECT_CSD) {
> -		ret = v3d_setup_csd_jobs_and_bos(file_priv, v3d,
> +		ret = v3d_setup_csd_jobs_and_bos(&indirect_submit,
>   						 &cpu_job->indirect_csd.args,
> -						 &cpu_job->indirect_csd.job,
> -						 &cpu_job->indirect_csd.clean_job,
> -						 NULL,
> -						 &cpu_job->indirect_csd.exec);
> +						 NULL);
>   		if (ret)
>   			goto fail;
> +
> +		ret = v3d_submit_lock_reservations(&indirect_submit);
> +		if (ret)
> +			goto fail;
> +
> +		cpu_job->indirect_csd.job = container_of(indirect_submit.jobs[0],
> +							 struct v3d_csd_job, base);
> +		cpu_job->indirect_csd.clean_job = indirect_submit.jobs[1];
> +
> +		clean_job = cpu_job->indirect_csd.clean_job;
> +		csd_job = cpu_job->indirect_csd.job;
>   	}
>   
> -	clean_job = cpu_job->indirect_csd.clean_job;
> -	csd_job = cpu_job->indirect_csd.job;
> -
>   	if (args->bo_handle_count) {
> -		ret = v3d_lookup_bos(dev, file_priv, &cpu_job->base,
> -				     args->bo_handles, args->bo_handle_count);
> +		ret = v3d_lookup_bos(&submit, args->bo_handles, args->bo_handle_count);
>   		if (ret)
>   			goto fail;
>   
> -		ret = v3d_lock_bo_reservations(&cpu_job->base, &exec);
> +		ret = v3d_submit_lock_reservations(&submit);
>   		if (ret)
>   			goto fail;
>   	}
> @@ -1455,36 +1379,28 @@ v3d_submit_cpu_ioctl(struct drm_device *dev, void *data,
>   
>   	out_se = (cpu_job->job_type == V3D_CPU_JOB_TYPE_INDIRECT_CSD) ? NULL : &se;

Move under one of the other two checks for this job type a bit above?

>   
> -	v3d_attach_fences_and_unlock_reservation(file_priv,
> -						 &cpu_job->base,
> -						 &exec, 0,
> -						 out_se, cpu_job->base.done_fence);
> +	v3d_attach_fences_and_unlock_reservation(&submit, 0, out_se);
>   
>   	switch (cpu_job->job_type) {
>   	case V3D_CPU_JOB_TYPE_INDIRECT_CSD:
> -		v3d_attach_fences_and_unlock_reservation(file_priv,
> -							 clean_job,
> -							 &cpu_job->indirect_csd.exec,
> -							 0, &se, clean_job->done_fence);
> +		v3d_attach_fences_and_unlock_reservation(&indirect_submit, 0, &se);
>   		break;
>   	default:
>   		break;
>   	}

Would this switch statement (and the one above it, not shown in the 
diff) look more streamlined as an if block? Would be more compact so 
more readable at least.

Maybe it is temporary code TBF.

Okay:

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>

Regards,

Tvrtko

>   
> -	v3d_job_put(&cpu_job->base);
> -	v3d_job_put(&csd_job->base);
> -	v3d_job_put(clean_job);
> +	v3d_submit_put_jobs(&submit);
> +	v3d_submit_put_jobs(&indirect_submit);
>   
>   	return 0;
>   
>   fail_unreserve:
>   	mutex_unlock(&v3d->sched_lock);
> -	drm_exec_fini(&exec);
> -	drm_exec_fini(&cpu_job->indirect_csd.exec);
> +	v3d_submit_unlock_reservations(&submit);
> +	v3d_submit_unlock_reservations(&indirect_submit);
>   fail:
> -	v3d_job_cleanup((void *)cpu_job);
> -	v3d_job_cleanup((void *)csd_job);
> -	v3d_job_cleanup(clean_job);
> +	v3d_submit_cleanup_jobs(&submit);
> +	v3d_submit_cleanup_jobs(&indirect_submit);
>   	v3d_put_multisync_post_deps(&se);
>   
>   	return ret;
> 


  reply	other threads:[~2026-06-04  9:49 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-03 22:25 [PATCH v3 00/14] drm/v3d: Scheduler and submission fixes and refactoring Maíra Canal
2026-06-03 22:25 ` [PATCH v3 01/14] drm/v3d: Drop unused drm_encoder.h include from v3d_drv.h Maíra Canal
2026-06-03 22:25 ` [PATCH v3 02/14] drm/v3d: Clear queue->active_job when v3d_fence_create() fails Maíra Canal
2026-06-04  2:50   ` sashiko-bot
2026-06-03 22:25 ` [PATCH v3 03/14] drm/v3d: Use inline lock for dma fence initialization Maíra Canal
2026-06-04  2:49   ` sashiko-bot
2026-06-03 22:25 ` [PATCH v3 04/14] drm/v3d: Replace spin_lock_irqsave() with spin_lock() Maíra Canal
2026-06-04  2:51   ` sashiko-bot
2026-06-03 22:25 ` [PATCH v3 05/14] drm/v3d: Extract v3d_job_add_syncobjs() helper Maíra Canal
2026-06-03 22:25 ` [PATCH v3 06/14] drm/v3d: Reject invalid syncobj handles in submit ioctls Maíra Canal
2026-06-04  2:51   ` sashiko-bot
2026-06-03 22:25 ` [PATCH v3 07/14] drm/v3d: Migrate BO reservation locking to DRM exec Maíra Canal
2026-06-03 22:25 ` [PATCH v3 08/14] drm/v3d: Introduce struct v3d_submit and convert CL/TFU/CSD ioctls Maíra Canal
2026-06-04  2:54   ` sashiko-bot
2026-06-04  8:58   ` Tvrtko Ursulin
2026-06-04 11:52     ` Maíra Canal
2026-06-04 12:05       ` Tvrtko Ursulin
2026-06-04 12:10         ` Maíra Canal
2026-06-03 22:25 ` [PATCH v3 09/14] drm/v3d: Make v3d_get_cpu_indirect_csd_params() a pure parser Maíra Canal
2026-06-04  2:53   ` sashiko-bot
2026-06-03 22:25 ` [PATCH v3 10/14] drm/v3d: Convert submit helpers to operate on struct v3d_submit Maíra Canal
2026-06-04  9:49   ` Tvrtko Ursulin [this message]
2026-06-03 22:25 ` [PATCH v3 11/14] drm/v3d: Refactor CPU ioctl into unified submission chain Maíra Canal
2026-06-04  2:58   ` sashiko-bot
2026-06-04  9:54   ` Tvrtko Ursulin
2026-06-03 22:25 ` [PATCH v3 12/14] drm/v3d: Split BO fence attach from syncobj output handling Maíra Canal
2026-06-04  9:59   ` Tvrtko Ursulin
2026-06-03 22:25 ` [PATCH v3 13/14] drm/v3d: Reject invalid out_sync handles in submit ioctls Maíra Canal
2026-06-04  2:59   ` sashiko-bot
2026-06-04 10:12   ` Tvrtko Ursulin
2026-06-03 22:25 ` [PATCH v3 14/14] drm/v3d: Ensure atomic submissions in v3d_submit_jobs() Maíra Canal
2026-06-04 10:25   ` Tvrtko Ursulin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1dc4a2e7-b30a-490e-bf98-ebdaced60c0b@igalia.com \
    --to=tvrtko.ursulin@igalia.com \
    --cc=airlied@gmail.com \
    --cc=ckoenig.leichtzumerken@gmail.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=itoral@igalia.com \
    --cc=kernel-dev@igalia.com \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=mcanal@igalia.com \
    --cc=mripard@kernel.org \
    --cc=mwen@igalia.com \
    --cc=simona@ffwll.ch \
    --cc=tzimmermann@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox