On 09/30, Iago Toral wrote:

On Wed, 2021-09-29 at 10:43 +0100, Melissa Wen wrote:

...

Move job memory allocation to v3d_job_init function. This aim to facilitate error handling in job initialization, since cleanup steps are similar for all (struct v3d_job)-based types of job involved in a command submission. To generalize v3d_job_init(), this change takes into account that all job structs have the first element a struct v3d_job (bin, render, tfu, csd) or it is a v3d_job itself (clean_job) for pointer casting.

Suggested-by: Iago Toral itoral@igalia.com Signed-off-by: Melissa Wen mwen@igalia.com

---

drivers/gpu/drm/v3d/v3d_gem.c | 115 +++++++++++++-------------------

1 file changed, 42 insertions(+), 73 deletions(-)

diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c index e60fbc28ef29..9cfa6f8d4357 100644 --- a/drivers/gpu/drm/v3d/v3d_gem.c +++ b/drivers/gpu/drm/v3d/v3d_gem.c @@ -392,6 +392,9 @@ v3d_render_job_free(struct kref *ref)

void v3d_job_cleanup(struct v3d_job *job) {

• if (!job)

• return;
  

• drm_sched_job_cleanup(&job->base); v3d_job_put(job);

} @@ -450,12 +453,20 @@ v3d_job_add_deps(struct drm_file *file_priv, struct v3d_job *job,

static int v3d_job_init(struct v3d_dev *v3d, struct drm_file *file_priv,

•     struct v3d_job *job, void (*free)(struct kref *ref),
  

•     void **container, size_t size, void (*free)(struct kref
  

*ref), u32 in_sync, enum v3d_queue queue) { struct v3d_file_priv *v3d_priv = file_priv->driver_priv;

• struct v3d_job *job; int ret;

• *container = kcalloc(1, size, GFP_KERNEL);

• if (!*container) {

• DRM_ERROR("Cannot allocate memory for v3d job.");
  

• return -ENOMEM;
  

• }

• job = *container; job->v3d = v3d; job->free = free;

Right below this hunk we have an early return that doesn't jump to fail:

    ret = pm_runtime_get_sync(v3d->drm.dev);
    if (ret < 0)
            return ret;

so we should kfree(*container) and set it to NULL there, right?

oh, yes. I missed it on porting downstream to upstream.

...

@@ -479,6 +490,9 @@ v3d_job_init(struct v3d_dev *v3d, struct drm_file *file_priv, drm_sched_job_cleanup(&job->base); fail: pm_runtime_put_autosuspend(v3d->drm.dev);

• kfree(*container);
• *container = NULL;
• return ret;

}

(...)

...

@@ -806,29 +789,15 @@ v3d_submit_csd_ioctl(struct drm_device *dev, void *data, return -EINVAL; }

• job = kcalloc(1, sizeof(*job), GFP_KERNEL);
• if (!job)

• return -ENOMEM;
  

• ret = v3d_job_init(v3d, file_priv, &job->base,

• ret = v3d_job_init(v3d, file_priv, (void *)&job, sizeof(*job), v3d_job_free, args->in_sync, V3D_CSD);

• if (ret) {

• kfree(job);
  

• return ret;
  

• }
• clean_job = kcalloc(1, sizeof(*clean_job), GFP_KERNEL);
• if (!clean_job) {

• v3d_job_cleanup(&job->base);
  

• return -ENOMEM;
  

• }

• if (ret)

• goto fail;
  
  

For this to work we need to explicitly initialize clean_job to NULL. Notice that the same applies to the bin and clean jobs in the CL ioctl, but in that case we're already initializing them to NULL.

yes, thanks for pointing it out.

Melissa

...

• ret = v3d_job_init(v3d, file_priv, clean_job, v3d_job_free, 0,

V3D_CACHE_CLEAN);

• if (ret) {

• v3d_job_cleanup(&job->base);
  

• kfree(clean_job);
  

• return ret;
  

• }

• ret = v3d_job_init(v3d, file_priv, (void *)&clean_job,

sizeof(*clean_job),

• 	   v3d_job_free, 0, V3D_CACHE_CLEAN);
  

• if (ret)

• goto fail;
  
  

  job->args = *args;

@@ -877,7 +846,7 @@ v3d_submit_csd_ioctl(struct drm_device *dev, void *data, drm_gem_unlock_reservations(clean_job->bo, clean_job->bo_count, &acquire_ctx); fail:

• v3d_job_cleanup(&job->base);

• v3d_job_cleanup((void *)job); v3d_job_cleanup(clean_job);

  return ret;

On Wed, 2021-09-29 at 10:44 +0100, Melissa Wen wrote:

Add support to attach generic extensions on job submission. This patch is third prep work to enable multiple syncobjs on job submission. With this work, when the job submission interface needs to be extended to accomodate a new feature, we will use a generic extension struct where an id determines the data type to be pointed. The first application is to enable multiples in/out syncobj (next patch), but the base is already done for future features. Therefore, to attach a new feature, a specific extension struct should subclass drm_v3d_extension and update the list of extensions in a job submission.

v2:

• remove redundant elements to subclass struct (Daniel)

Signed-off-by: Melissa Wen mwen@igalia.com

drivers/gpu/drm/v3d/v3d_drv.c | 4 +- drivers/gpu/drm/v3d/v3d_gem.c | 71 +++++++++++++++++++++++++++++++++-- include/uapi/drm/v3d_drm.h | 31 +++++++++++++++ 3 files changed, 100 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/v3d/v3d_drv.c b/drivers/gpu/drm/v3d/v3d_drv.c index c1deab2cf38d..3d6b9bcce2f7 100644 --- a/drivers/gpu/drm/v3d/v3d_drv.c +++ b/drivers/gpu/drm/v3d/v3d_drv.c @@ -83,7 +83,6 @@ static int v3d_get_param_ioctl(struct drm_device *dev, void *data, return 0; }

• switch (args->param) { case DRM_V3D_PARAM_SUPPORTS_TFU: args->value = 1;

@@ -147,7 +146,7 @@ v3d_postclose(struct drm_device *dev, struct drm_file *file) DEFINE_DRM_GEM_FOPS(v3d_drm_fops);

/* DRM_AUTH is required on SUBMIT_CL for now, while we don't have GMP

• • protection between clients. Note that render nodes would be be

• • protection between clients. Note that render nodes would be
  • able to submit CLs that could access BOs from clients

authenticated

• with the master node. The TFU doesn't use the GMP, so it would
• need to stay DRM_AUTH until we do buffer size/offset validation.

@@ -219,7 +218,6 @@ static int v3d_platform_drm_probe(struct platform_device *pdev) u32 mmu_debug; u32 ident1;

• v3d = devm_drm_dev_alloc(dev, &v3d_drm_driver, struct v3d_dev,

drm); if (IS_ERR(v3d)) return PTR_ERR(v3d); diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c index 9cfa6f8d4357..b912419027f7 100644 --- a/drivers/gpu/drm/v3d/v3d_gem.c +++ b/drivers/gpu/drm/v3d/v3d_gem.c @@ -535,6 +535,33 @@ v3d_attach_fences_and_unlock_reservation(struct drm_file *file_priv, } }

+static int +v3d_get_extensions(struct drm_file *file_priv, u64 ext_handles) +{

• struct drm_v3d_extension __user *user_ext;
• user_ext = u64_to_user_ptr(ext_handles);
• while(user_ext) {

• struct drm_v3d_extension ext;
  

• if (copy_from_user(&ext, user_ext, sizeof(ext))) {
  

• 	DRM_DEBUG("Failed to copy submit extension\n");
  

• 	return -EFAULT;
  

• }
  

• switch (ext.id) {
  

• case 0:
  

• default:
  

• 	DRM_DEBUG_DRIVER("Unknown extension id: %d\n",
  

ext.id);

• 	return -EINVAL;
  

• }
  

• user_ext = u64_to_user_ptr(ext.next);
  

• }
• return 0;

+}

/**

• v3d_submit_cl_ioctl() - Submits a job (frame) to the V3D.
• @dev: DRM device

@@ -563,15 +590,24 @@ v3d_submit_cl_ioctl(struct drm_device *dev, void *data,

trace_v3d_submit_cl_ioctl(&v3d->drm, args->rcl_start, args-

...

rcl_end);

• if (args->pad != 0)

• if (args->pad) return -EINVAL;

• if (args->flags != 0 &&

•    args->flags != DRM_V3D_SUBMIT_CL_FLUSH_CACHE) {
  

• if (args->flags &&

•    args->flags & ~(DRM_V3D_SUBMIT_CL_FLUSH_CACHE |
  

• 	    DRM_V3D_SUBMIT_EXTENSION)) {
  

  DRM_INFO("invalid flags: %d\n", args->flags); return -EINVAL; }

• if (args->flags & DRM_V3D_SUBMIT_EXTENSION) {

• ret = v3d_get_extensions(file_priv, args->extensions);
  

• if (ret) {
  

• 	DRM_DEBUG("Failed to get extensions.\n");
  

• 	return ret;
  

• }
  

• }

• ret = v3d_job_init(v3d, file_priv, (void *)&render,

sizeof(*render), v3d_render_job_free, args->in_sync_bcl, V3D_RENDER); if (ret) @@ -700,6 +736,19 @@ v3d_submit_tfu_ioctl(struct drm_device *dev, void *data,

trace_v3d_submit_tfu_ioctl(&v3d->drm, args->iia);

• if (args->flags && !(args->flags & DRM_V3D_SUBMIT_EXTENSION)) {

• DRM_DEBUG("invalid flags: %d\n", args->flags);
  

• return -EINVAL;
  

• }
• if (args->flags & DRM_V3D_SUBMIT_EXTENSION) {

• ret = v3d_get_extensions(file_priv, args->extensions);
  

• if (ret) {
  

• 	DRM_DEBUG("Failed to get extensions.\n");
  

• 	return ret;
  

• }
  

• }
• ret = v3d_job_init(v3d, file_priv, (void *)&job, sizeof(*job), v3d_job_free, args->in_sync, V3D_TFU); if (ret)

@@ -784,11 +833,27 @@ v3d_submit_csd_ioctl(struct drm_device *dev, void *data,

trace_v3d_submit_csd_ioctl(&v3d->drm, args->cfg[5], args-

...

cfg[6]);

• if (args->pad)

• return -EINVAL;
  

• if (!v3d_has_csd(v3d)) { DRM_DEBUG("Attempting CSD submit on non-CSD

hardware\n"); return -EINVAL; }

• if (args->flags && !(args->flags & DRM_V3D_SUBMIT_EXTENSION)) {

• DRM_INFO("invalid flags: %d\n", args->flags);
  

• return -EINVAL;
  

• }
• if (args->flags & DRM_V3D_SUBMIT_EXTENSION) {

• ret = v3d_get_extensions(file_priv, args->extensions);
  

• if (ret) {
  

• 	DRM_DEBUG("Failed to get extensions.\n");
  

• 	return ret;
  

• }
  

• }
• ret = v3d_job_init(v3d, file_priv, (void *)&job, sizeof(*job), v3d_job_free, args->in_sync, V3D_CSD); if (ret)

diff --git a/include/uapi/drm/v3d_drm.h b/include/uapi/drm/v3d_drm.h index 4104f22fb3d3..55b443ca6c0b 100644 --- a/include/uapi/drm/v3d_drm.h +++ b/include/uapi/drm/v3d_drm.h @@ -58,6 +58,20 @@ extern "C" { struct drm_v3d_perfmon_get_values)

#define DRM_V3D_SUBMIT_CL_FLUSH_CACHE 0x01 +#define DRM_V3D_SUBMIT_EXTENSION 0x02

+/* struct drm_v3d_extension - ioctl extensions

• • Linked-list of generic extensions where the id identify which

struct is

• • pointed by ext_data. Therefore, DRM_V3D_EXT_ID_* is used on id to

identify

• • the extension type.
• */

+struct drm_v3d_extension {

• __u64 next;
• __u32 id;

+#define DRM_V3D_EXT_ID_MULTI_SYNC 0x01

• __u32 flags; /* mbz */

+};

/**

• struct drm_v3d_submit_cl - ioctl argument for submitting commands

to the 3D @@ -135,12 +149,16 @@ struct drm_v3d_submit_cl { /* Number of BO handles passed in (size is that times 4). */ __u32 bo_handle_count;

• /* DRM_V3D_SUBMIT_* properties */ __u32 flags;

  /* ID of the perfmon to attach to this job. 0 means no perfmon.

*/ __u32 perfmon_id;

__u32 pad;

• /* Pointer to an array of ioctl extensions*/
• __u64 extensions;

};

/** @@ -248,6 +266,12 @@ struct drm_v3d_submit_tfu { __u32 in_sync; /* Sync object to signal when the TFU job is done. */ __u32 out_sync;

• __u32 flags;
• /* Pointer to an array of ioctl extensions*/
• __u64 extensions;

We want __u64 fields aligned to 64-bit so we should swap the positions of flags and extensions.

};

/* Submits a compute shader for dispatch. This job will block on any @@ -276,6 +300,13 @@ struct drm_v3d_submit_csd {

/* ID of the perfmon to attach to this job. 0 means no perfmon. */ __u32 perfmon_id;

• /* Pointer to an array of ioctl extensions*/
• __u64 extensions;
• __u32 flags;
• __u32 pad;

};

enum {

On Thu, 2021-09-30 at 10:22 +0100, Melissa Wen wrote:

...

...

O 09/30, Iago Toral wrote:

...

On Wed, 2021-09-29 at 10:44 +0100, Melissa Wen wrote:

(...)

...

...

/**

• struct drm_v3d_submit_cl - ioctl argument for submitting

commands to the 3D @@ -135,12 +149,16 @@ struct drm_v3d_submit_cl { /* Number of BO handles passed in (size is that times 4). */ __u32 bo_handle_count;

• /* DRM_V3D_SUBMIT_* properties */ __u32 flags;

  /* ID of the perfmon to attach to this job. 0 means no perfmon.

*/ __u32 perfmon_id;

__u32 pad;

• /* Pointer to an array of ioctl extensions*/
• __u64 extensions;

};

/** @@ -248,6 +266,12 @@ struct drm_v3d_submit_tfu { __u32 in_sync; /* Sync object to signal when the TFU job is done. */ __u32 out_sync;

• __u32 flags;
• /* Pointer to an array of ioctl extensions*/
• __u64 extensions;

We want __u64 fields aligned to 64-bit so we should swap the positions of flags and extensions.

hmm.. not sure. before two arrays of 4 x _u32 elements, we have seven _u32 elements... this is why I counted a odd number of _u32 and put _u32 flags before _u64 extensions... or is it working different for array types?

Ah yes, I was confused by the patch format, but you're right.

For the same reason, I think there is an unalignment issue on submit_csd that would need to change the current interface to solve (afaiu)...

Yes, that one is not aligned, but it is too late to fix now without braking the interface. We have not seen any issues caused by that on 32-bit Raspbian though.

Iago

...

...

};

/* Submits a compute shader for dispatch. This job will block on any @@ -276,6 +300,13 @@ struct drm_v3d_submit_csd {

/* ID of the perfmon to attach to this job. 0 means no perfmon. */ __u32 perfmon_id;

• /* Pointer to an array of ioctl extensions*/
• __u64 extensions;
• __u32 flags;
• __u32 pad;

};

enum {

On Wed, 2021-09-29 at 10:45 +0100, Melissa Wen wrote:

Using the generic extension from the previous patch, a specific multisync extension enables more than one in/out binary syncobj per job submission. Arrays of syncobjs are set in struct drm_v3d_multisync, that also cares of determining the stage for sync (wait deps) according to the job queue.

v2:

• subclass the generic extension struct (Daniel)
• simplify adding dependency conditions to make understandable (Iago)

Signed-off-by: Melissa Wen mwen@igalia.com

drivers/gpu/drm/v3d/v3d_drv.c | 6 +- drivers/gpu/drm/v3d/v3d_drv.h | 23 +++-- drivers/gpu/drm/v3d/v3d_gem.c | 176 ++++++++++++++++++++++++++++++

---

include/uapi/drm/v3d_drm.h | 49 +++++++++- 4 files changed, 224 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/v3d/v3d_drv.c b/drivers/gpu/drm/v3d/v3d_drv.c index 3d6b9bcce2f7..bd46396a1ae0 100644 --- a/drivers/gpu/drm/v3d/v3d_drv.c +++ b/drivers/gpu/drm/v3d/v3d_drv.c @@ -96,6 +96,9 @@ static int v3d_get_param_ioctl(struct drm_device *dev, void *data, case DRM_V3D_PARAM_SUPPORTS_PERFMON: args->value = (v3d->ver >= 40); return 0;

• case DRM_V3D_PARAM_SUPPORTS_MULTISYNC_EXT:

• args->value = 1;
  

• return 0;
  

  default: DRM_DEBUG("Unknown parameter %d\n", args->param); return -EINVAL;

@@ -135,9 +138,8 @@ v3d_postclose(struct drm_device *dev, struct drm_file *file) struct v3d_file_priv *v3d_priv = file->driver_priv; enum v3d_queue q;

• for (q = 0; q < V3D_MAX_QUEUES; q++) {

• for (q = 0; q < V3D_MAX_QUEUES; q++) drm_sched_entity_destroy(&v3d_priv->sched_entity[q]);

• }

  v3d_perfmon_close_file(v3d_priv); kfree(v3d_priv);

diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h index b900a050d5e2..b14ff1e96f49 100644 --- a/drivers/gpu/drm/v3d/v3d_drv.h +++ b/drivers/gpu/drm/v3d/v3d_drv.h @@ -19,15 +19,6 @@ struct reset_control;

#define GMP_GRANULARITY (128 * 1024)

-/* Enum for each of the V3D queues. */ -enum v3d_queue {

• V3D_BIN,
• V3D_RENDER,
• V3D_TFU,
• V3D_CSD,
• V3D_CACHE_CLEAN,

-};

#define V3D_MAX_QUEUES (V3D_CACHE_CLEAN + 1)

struct v3d_queue_state { @@ -294,6 +285,20 @@ struct v3d_csd_job { struct drm_v3d_submit_csd args; };

+struct v3d_submit_outsync {

• struct drm_syncobj *syncobj;

+};

+struct v3d_submit_ext {

• u32 wait_stage;
• u32 in_sync_count;
• u64 in_syncs;
• u32 out_sync_count;
• struct v3d_submit_outsync *out_syncs;

+};

/**

• __wait_for - magic wait macro

diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c index b912419027f7..e92998d39eaa 100644 --- a/drivers/gpu/drm/v3d/v3d_gem.c +++ b/drivers/gpu/drm/v3d/v3d_gem.c @@ -454,11 +454,12 @@ v3d_job_add_deps(struct drm_file *file_priv, struct v3d_job *job, static int v3d_job_init(struct v3d_dev *v3d, struct drm_file *file_priv, void **container, size_t size, void (*free)(struct kref *ref),

•     u32 in_sync, enum v3d_queue queue)
  

•     u32 in_sync, struct v3d_submit_ext *se, enum v3d_queue
  

queue) { struct v3d_file_priv *v3d_priv = file_priv->driver_priv; struct v3d_job *job;

• int ret;

• bool has_multisync = (se && se->in_sync_count);

I think this is not correct... we could be using the multisync interface and pass 0 in_syncs and/or 0 out_syncs... what should determine if we take the multisync path is the presence of the extension alone.

• int ret, i;

  *container = kcalloc(1, size, GFP_KERNEL); if (!*container) {

@@ -479,9 +480,28 @@ v3d_job_init(struct v3d_dev *v3d, struct drm_file *file_priv, if (ret) goto fail;

• ret = v3d_job_add_deps(file_priv, job, in_sync, 0);
• if (ret)

• goto fail_job;
  

• if (has_multisync) {

• if (se->wait_stage == queue) {
  

• 	struct drm_v3d_sem __user *handle =
  

u64_to_user_ptr(se->in_syncs);

• 	for (i = 0; i < se->in_sync_count; i++) {
  

• 		struct drm_v3d_sem in;
  

• 		ret = copy_from_user(&in, handle++,
  

sizeof(in));

• 		if (ret) {
  

• 			DRM_DEBUG("Failed to copy wait
  

dep handle.\n");

• 			goto fail_job;
  

• 		}
  

• 		ret = v3d_job_add_deps(file_priv, job,
  

in.handle, 0);

• 		if (ret)
  

• 			goto fail_job;
  

• 	}
  

• }
  

• } else {

• ret = v3d_job_add_deps(file_priv, job, in_sync, 0);
  

• if (ret)
  

• 	goto fail_job;
  

• }

  kref_init(&job->refcount);

@@ -514,6 +534,7 @@ v3d_attach_fences_and_unlock_reservation(struct drm_file *file_priv, struct v3d_job *job, struct ww_acquire_ctx *acquire_ctx, u32 out_sync,

• 			 struct v3d_submit_ext *se,
  		 struct dma_fence *done_fence)
  

{ struct drm_syncobj *sync_out; @@ -528,6 +549,18 @@ v3d_attach_fences_and_unlock_reservation(struct drm_file *file_priv, drm_gem_unlock_reservations(job->bo, job->bo_count, acquire_ctx);

/* Update the return sync object for the job */

• /* If multiples semaphores is supported */
• if (se && se->out_sync_count) {

• for (i = 0; i < se->out_sync_count; i++) {
  

• 	drm_syncobj_replace_fence(se-
  

...

out_syncs[i].syncobj,

• 				  done_fence);
  

• 	drm_syncobj_put(se->out_syncs[i].syncobj);
  

• }
  

• kvfree(se->out_syncs);
  

• return;
  

• }
• /* Single signal semaphore */ sync_out = drm_syncobj_find(file_priv, out_sync); if (sync_out) { drm_syncobj_replace_fence(sync_out, done_fence);

@@ -535,13 +568,107 @@ v3d_attach_fences_and_unlock_reservation(struct drm_file *file_priv, } }

+static void +v3d_put_multisync_post_deps(struct v3d_submit_ext *se) +{

• unsigned int i;
• for (i = 0; i < se->out_sync_count; i++)

• drm_syncobj_put(se->out_syncs[i].syncobj);
  

• kvfree(se->out_syncs);

+}

static int -v3d_get_extensions(struct drm_file *file_priv, u64 ext_handles) +v3d_get_multisync_post_deps(struct drm_file *file_priv,

• 	    struct v3d_submit_ext *se,
  

• 	    u32 count, u64 handles)
  

+{

• struct drm_v3d_sem __user *post_deps;
• int i, ret;
• se->out_syncs = (struct v3d_submit_outsync *)

• 	kvmalloc_array(count,
  

• 		       sizeof(struct
  

v3d_submit_outsync),

• 		       GFP_KERNEL);
  

• if (!se->out_syncs)

• return -ENOMEM;
  

• post_deps = u64_to_user_ptr(handles);
• for (i = 0; i < count; i++) {

• struct drm_v3d_sem out;
  

• ret = copy_from_user(&out, post_deps++, sizeof(out));
  

• if (ret) {
  

• 	DRM_DEBUG("Failed to copy post dep handles\n");
  

• 	goto fail;
  

• }
  

• se->out_syncs[i].syncobj = drm_syncobj_find(file_priv,
  

• 					    out.handle)
  

;

• if (!se->out_syncs[i].syncobj) {
  

• 	ret = -EINVAL;
  

• 	goto fail;
  

• }
  

• }
• se->out_sync_count = count;
• return 0;

+fail:

• for (i--; i >= 0; i--)

• drm_syncobj_put(se->out_syncs[i].syncobj);
  

• kvfree(se->out_syncs);
• return ret;

+}

+/* Get data for multiple binary semaphores synchronization. Parse syncobj

• • to be signaled when job completes (out_sync).
• */

+static int +v3d_get_multisync_submit_deps(struct drm_file *file_priv,

• 	      struct drm_v3d_extension __user *ext,
  

• 	      void *data)
  

+{

• struct drm_v3d_multi_sync multisync;
• struct v3d_submit_ext *se = data;
• int ret;
• ret = copy_from_user(&multisync, ext, sizeof(multisync));
• if (ret)

• return ret;
  

• if (multisync.pad)

• return -EINVAL;
  

• ret = v3d_get_multisync_post_deps(file_priv, data,

multisync.out_sync_count,

• 			  multisync.out_syncs);
  

• if (ret)

• return ret;
  

• se->in_sync_count = multisync.in_sync_count;
• se->in_syncs = multisync.in_syncs;
• se->wait_stage = multisync.wait_stage;
• return 0;

+}

+/* Whenever userspace sets ioctl extensions, parses data according to the

• • extension id (name). By now, generic ioctl extensions is only

used to

• • support multiple binary semaphores (struct drm_v3d_multi_sync).
• */

Let's remove the sentence that starts with "By now, generic..." it is not really relevant and it is bound to get obsolete when we add more extensions.

+static int +v3d_get_extensions(struct drm_file *file_priv,

•    u64 ext_handles,
  

•    void *data)
  

{ struct drm_v3d_extension __user *user_ext;

• int ret;

  user_ext = u64_to_user_ptr(ext_handles);

• while(user_ext) {

• while (user_ext) { struct drm_v3d_extension ext;

  if (copy_from_user(&ext, user_ext, sizeof(ext))) {

@@ -550,7 +677,11 @@ v3d_get_extensions(struct drm_file *file_priv, u64 ext_handles) }

switch (ext.id) {

• case 0:
  

• case DRM_V3D_EXT_ID_MULTI_SYNC:
  

• 	ret = v3d_get_multisync_submit_deps(file_priv,
  

user_ext, data);

• 	if (ret)
  

• 		return ret;
  

• 	break;
  

  default: DRM_DEBUG_DRIVER("Unknown extension id: %d\n",

ext.id); return -EINVAL; @@ -581,6 +712,7 @@ v3d_submit_cl_ioctl(struct drm_device *dev, void *data, struct v3d_dev *v3d = to_v3d_dev(dev); struct v3d_file_priv *v3d_priv = file_priv->driver_priv; struct drm_v3d_submit_cl *args = data;

• struct v3d_submit_ext se = {0}; struct v3d_bin_job *bin = NULL; struct v3d_render_job *render; struct v3d_job *clean_job = NULL;

@@ -601,7 +733,7 @@ v3d_submit_cl_ioctl(struct drm_device *dev, void *data, }

if (args->flags & DRM_V3D_SUBMIT_EXTENSION) {

• ret = v3d_get_extensions(file_priv, args->extensions);
  

• ret = v3d_get_extensions(file_priv, args->extensions,
  

&se); if (ret) { DRM_DEBUG("Failed to get extensions.\n"); return ret; @@ -609,7 +741,7 @@ v3d_submit_cl_ioctl(struct drm_device *dev, void *data, }

ret = v3d_job_init(v3d, file_priv, (void *)&render, sizeof(*render),

• 	   v3d_render_job_free, args->in_sync_bcl,
  

V3D_RENDER);

• 	   v3d_render_job_free, args->in_sync_bcl, &se,
  

V3D_RENDER);

So here is an example of what I was saying above... if the driver is using the multisync interface with 0 in_syncs, v3d_job_init here will think we are not using multisync and instead try to use args-

in_sync_bcl, which is not what we want. The same issue happens in all

other submission ioctls.

if (ret) goto fail;

@@ -619,7 +751,7 @@ v3d_submit_cl_ioctl(struct drm_device *dev, void *data,

if (args->bcl_start != args->bcl_end) { ret = v3d_job_init(v3d, file_priv, (void *)&bin, sizeof(*bin),

• 		   v3d_job_free, args->in_sync_bcl,
  

V3D_BIN);

• 		   v3d_job_free, args->in_sync_bcl,
  

&se, V3D_BIN);

Same here.

if (ret)
	goto fail;

@@ -633,7 +765,7 @@ v3d_submit_cl_ioctl(struct drm_device *dev, void *data,

if (args->flags & DRM_V3D_SUBMIT_CL_FLUSH_CACHE) { ret = v3d_job_init(v3d, file_priv, (void *)&clean_job, sizeof(*clean_job),

• 		   v3d_job_free, 0, V3D_CACHE_CLEAN);
  

• 		   v3d_job_free, 0, 0,
  

V3D_CACHE_CLEAN); if (ret) goto fail;

@@ -693,6 +825,7 @@ v3d_submit_cl_ioctl(struct drm_device *dev, void *data, last_job, &acquire_ctx, args->out_sync,

• 				 &se,
  			 last_job->done_fence);
  
  

  if (bin)

@@ -711,6 +844,7 @@ v3d_submit_cl_ioctl(struct drm_device *dev, void *data, v3d_job_cleanup((void *)bin); v3d_job_cleanup((void *)render); v3d_job_cleanup(clean_job);

• v3d_put_multisync_post_deps(&se);

  return ret;

} @@ -730,6 +864,7 @@ v3d_submit_tfu_ioctl(struct drm_device *dev, void *data, { struct v3d_dev *v3d = to_v3d_dev(dev); struct drm_v3d_submit_tfu *args = data;

• struct v3d_submit_ext se = {0}; struct v3d_tfu_job *job; struct ww_acquire_ctx acquire_ctx; int ret = 0;

@@ -742,7 +877,7 @@ v3d_submit_tfu_ioctl(struct drm_device *dev, void *data, }

if (args->flags & DRM_V3D_SUBMIT_EXTENSION) {

• ret = v3d_get_extensions(file_priv, args->extensions);
  

• ret = v3d_get_extensions(file_priv, args->extensions,
  

&se); if (ret) { DRM_DEBUG("Failed to get extensions.\n"); return ret; @@ -750,7 +885,7 @@ v3d_submit_tfu_ioctl(struct drm_device *dev, void *data, }

ret = v3d_job_init(v3d, file_priv, (void *)&job, sizeof(*job),

• 	   v3d_job_free, args->in_sync, V3D_TFU);
  

• 	   v3d_job_free, args->in_sync, &se, V3D_TFU);
  

Same here.

if (ret) goto fail;

@@ -798,6 +933,7 @@ v3d_submit_tfu_ioctl(struct drm_device *dev, void *data, v3d_attach_fences_and_unlock_reservation(file_priv, &job->base, &acquire_ctx, args->out_sync,

• 				 &se,
  			 job->base.done_fence);
  
  

  v3d_job_put(&job->base);

@@ -806,6 +942,7 @@ v3d_submit_tfu_ioctl(struct drm_device *dev, void *data,

fail: v3d_job_cleanup((void *)job);

• v3d_put_multisync_post_deps(&se);

  return ret;

} @@ -826,8 +963,9 @@ v3d_submit_csd_ioctl(struct drm_device *dev, void *data, struct v3d_dev *v3d = to_v3d_dev(dev); struct v3d_file_priv *v3d_priv = file_priv->driver_priv; struct drm_v3d_submit_csd *args = data;

• struct v3d_submit_ext se = {0}; struct v3d_csd_job *job;

• struct v3d_job *clean_job;

• struct v3d_job *clean_job = NULL; struct ww_acquire_ctx acquire_ctx; int ret;

@@ -847,7 +985,7 @@ v3d_submit_csd_ioctl(struct drm_device *dev, void *data, }

if (args->flags & DRM_V3D_SUBMIT_EXTENSION) {

• ret = v3d_get_extensions(file_priv, args->extensions);
  

• ret = v3d_get_extensions(file_priv, args->extensions,
  

&se); if (ret) { DRM_DEBUG("Failed to get extensions.\n"); return ret; @@ -855,12 +993,12 @@ v3d_submit_csd_ioctl(struct drm_device *dev, void *data, }

ret = v3d_job_init(v3d, file_priv, (void *)&job, sizeof(*job),

• 	   v3d_job_free, args->in_sync, V3D_CSD);
  

• 	   v3d_job_free, args->in_sync, &se, V3D_CSD);
  

Same here.

if (ret) goto fail;

ret = v3d_job_init(v3d, file_priv, (void *)&clean_job, sizeof(*clean_job),

• 	   v3d_job_free, 0, V3D_CACHE_CLEAN);
  

• 	   v3d_job_free, 0, 0, V3D_CACHE_CLEAN);
  

  if (ret) goto fail;

@@ -899,6 +1037,7 @@ v3d_submit_csd_ioctl(struct drm_device *dev, void *data, clean_job, &acquire_ctx, args->out_sync,

• 				 &se,
  			 clean_job-
  

...

done_fence);

v3d_job_put(&job->base); @@ -913,6 +1052,7 @@ v3d_submit_csd_ioctl(struct drm_device *dev, void *data, fail: v3d_job_cleanup((void *)job); v3d_job_cleanup(clean_job);

• v3d_put_multisync_post_deps(&se);

  return ret;

} diff --git a/include/uapi/drm/v3d_drm.h b/include/uapi/drm/v3d_drm.h index 55b443ca6c0b..3dfc0af8756a 100644 --- a/include/uapi/drm/v3d_drm.h +++ b/include/uapi/drm/v3d_drm.h @@ -73,6 +73,53 @@ struct drm_v3d_extension { __u32 flags; /* mbz */ };

+/* struct drm_v3d_sem - wait/signal semaphore

• • If binary semaphore, it only takes syncobj handle and ignores

flags and

• • point fields. Point is defined for timeline syncobj feature.
• */

+struct drm_v3d_sem {

• __u32 handle; /* syncobj */
• /* rsv below, for future uses */
• __u32 flags;
• __u64 point; /* for timeline sem support */
• __u64 mbz[2]; /* must be zero, rsv */

+};

+/* Enum for each of the V3D queues. */ +enum v3d_queue {

• V3D_BIN,
• V3D_RENDER,
• V3D_TFU,
• V3D_CSD,
• V3D_CACHE_CLEAN,

+};

+/**

• • struct drm_v3d_multi_sync - ioctl extension to add support

multiples

• • syncobjs for commands submission.
• • When an extension of DRM_V3D_EXT_ID_MULTI_SYNC id is defined, it

points to

• • this extension to define wait and signal dependencies, instead of

single

• • in/out sync entries on submitting commands. The field flags is

used to

• • determine the stage to set wait dependencies.
• */

+struct drm_v3d_multi_sync {

• struct drm_v3d_extension base;
• /* Array of wait and signal semaphores */
• __u64 in_syncs;
• __u64 out_syncs;
• /* Number of entries */
• __u32 in_sync_count;
• __u32 out_sync_count;
• /* set the stage (v3d_queue) to sync */
• __u32 wait_stage;
• __u32 pad; /* mbz */

+};

/**

• struct drm_v3d_submit_cl - ioctl argument for submitting commands

to the 3D

• engine.

@@ -228,6 +275,7 @@ enum drm_v3d_param { DRM_V3D_PARAM_SUPPORTS_CSD, DRM_V3D_PARAM_SUPPORTS_CACHE_FLUSH, DRM_V3D_PARAM_SUPPORTS_PERFMON,

• DRM_V3D_PARAM_SUPPORTS_MULTISYNC_EXT,

};

struct drm_v3d_get_param { @@ -271,7 +319,6 @@ struct drm_v3d_submit_tfu {

/* Pointer to an array of ioctl extensions*/ __u64 extensions;

};

/* Submits a compute shader for dispatch. This job will block on any

On 09/30, Melissa Wen wrote:

On 09/30, Iago Toral wrote:

...

On Wed, 2021-09-29 at 10:45 +0100, Melissa Wen wrote:

...

Using the generic extension from the previous patch, a specific multisync extension enables more than one in/out binary syncobj per job submission. Arrays of syncobjs are set in struct drm_v3d_multisync, that also cares of determining the stage for sync (wait deps) according to the job queue.

v2:

• subclass the generic extension struct (Daniel)
• simplify adding dependency conditions to make understandable (Iago)

Signed-off-by: Melissa Wen mwen@igalia.com

drivers/gpu/drm/v3d/v3d_drv.c | 6 +- drivers/gpu/drm/v3d/v3d_drv.h | 23 +++-- drivers/gpu/drm/v3d/v3d_gem.c | 176 ++++++++++++++++++++++++++++++

---

include/uapi/drm/v3d_drm.h | 49 +++++++++- 4 files changed, 224 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/v3d/v3d_drv.c b/drivers/gpu/drm/v3d/v3d_drv.c index 3d6b9bcce2f7..bd46396a1ae0 100644 --- a/drivers/gpu/drm/v3d/v3d_drv.c +++ b/drivers/gpu/drm/v3d/v3d_drv.c @@ -96,6 +96,9 @@ static int v3d_get_param_ioctl(struct drm_device *dev, void *data, case DRM_V3D_PARAM_SUPPORTS_PERFMON: args->value = (v3d->ver >= 40); return 0;

• case DRM_V3D_PARAM_SUPPORTS_MULTISYNC_EXT:

• args->value = 1;
  

• return 0;
  

  default: DRM_DEBUG("Unknown parameter %d\n", args->param); return -EINVAL;

@@ -135,9 +138,8 @@ v3d_postclose(struct drm_device *dev, struct drm_file *file) struct v3d_file_priv *v3d_priv = file->driver_priv; enum v3d_queue q;

• for (q = 0; q < V3D_MAX_QUEUES; q++) {

• for (q = 0; q < V3D_MAX_QUEUES; q++) drm_sched_entity_destroy(&v3d_priv->sched_entity[q]);

• }

  v3d_perfmon_close_file(v3d_priv); kfree(v3d_priv);

diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h index b900a050d5e2..b14ff1e96f49 100644 --- a/drivers/gpu/drm/v3d/v3d_drv.h +++ b/drivers/gpu/drm/v3d/v3d_drv.h @@ -19,15 +19,6 @@ struct reset_control;

#define GMP_GRANULARITY (128 * 1024)

-/* Enum for each of the V3D queues. */ -enum v3d_queue {

• V3D_BIN,
• V3D_RENDER,
• V3D_TFU,
• V3D_CSD,
• V3D_CACHE_CLEAN,

-};

#define V3D_MAX_QUEUES (V3D_CACHE_CLEAN + 1)

struct v3d_queue_state { @@ -294,6 +285,20 @@ struct v3d_csd_job { struct drm_v3d_submit_csd args; };

+struct v3d_submit_outsync {

• struct drm_syncobj *syncobj;

+};

+struct v3d_submit_ext {

• u32 wait_stage;
• u32 in_sync_count;
• u64 in_syncs;
• u32 out_sync_count;
• struct v3d_submit_outsync *out_syncs;

+};

/**

• __wait_for - magic wait macro

diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c index b912419027f7..e92998d39eaa 100644 --- a/drivers/gpu/drm/v3d/v3d_gem.c +++ b/drivers/gpu/drm/v3d/v3d_gem.c @@ -454,11 +454,12 @@ v3d_job_add_deps(struct drm_file *file_priv, struct v3d_job *job, static int v3d_job_init(struct v3d_dev *v3d, struct drm_file *file_priv, void **container, size_t size, void (*free)(struct kref *ref),

•     u32 in_sync, enum v3d_queue queue)
  

•     u32 in_sync, struct v3d_submit_ext *se, enum v3d_queue
  

queue) { struct v3d_file_priv *v3d_priv = file_priv->driver_priv; struct v3d_job *job;

• int ret;

• bool has_multisync = (se && se->in_sync_count);

I think this is not correct... we could be using the multisync interface and pass 0 in_syncs and/or 0 out_syncs... what should determine if we take the multisync path is the presence of the extension alone.

hmm.. yeah. so, I should drop this has_multisync and change conditions to only check if we have a submit_ext (that means we have multisync support) and move the in_sync_count to check if we should add any wait semaphores, as below:

...

...

• int ret, i;

  *container = kcalloc(1, size, GFP_KERNEL); if (!*container) {

@@ -479,9 +480,28 @@ v3d_job_init(struct v3d_dev *v3d, struct drm_file *file_priv, if (ret) goto fail;

• ret = v3d_job_add_deps(file_priv, job, in_sync, 0);
• if (ret)

• goto fail_job;
  

if (se) { if (se->in_sync_count && se->wait_stage == queue) {

never mind. I realized store extension flag on v3d_submit_ext works better for handling multiples wait and signal semaphores. Just sent a v3.

Thanks,

Melissa

...

...

• 	struct drm_v3d_sem __user *handle =
  

u64_to_user_ptr(se->in_syncs);

• 	for (i = 0; i < se->in_sync_count; i++) {
  

• 		struct drm_v3d_sem in;
  

• 		ret = copy_from_user(&in, handle++,
  

sizeof(in));

• 		if (ret) {
  

• 			DRM_DEBUG("Failed to copy wait
  

dep handle.\n");

• 			goto fail_job;
  

• 		}
  

• 		ret = v3d_job_add_deps(file_priv, job,
  

in.handle, 0);

• 		if (ret)
  

• 			goto fail_job;
  

• 	}
  

• }
  

• } else {

• ret = v3d_job_add_deps(file_priv, job, in_sync, 0);
  

• if (ret)
  

• 	goto fail_job;
  

• }

  kref_init(&job->refcount);

@@ -514,6 +534,7 @@ v3d_attach_fences_and_unlock_reservation(struct drm_file *file_priv, struct v3d_job *job, struct ww_acquire_ctx *acquire_ctx, u32 out_sync,

• 			 struct v3d_submit_ext *se,
  		 struct dma_fence *done_fence)
  

{ struct drm_syncobj *sync_out; @@ -528,6 +549,18 @@ v3d_attach_fences_and_unlock_reservation(struct drm_file *file_priv, drm_gem_unlock_reservations(job->bo, job->bo_count, acquire_ctx);

/* Update the return sync object for the job */

• /* If multiples semaphores is supported */
• if (se && se->out_sync_count) {

and drop this out_sync_count condition too ^

same here. In the v3, I reworked a little more to detach multisync support from the number of out_sync to lookup and replace.

...

...

• for (i = 0; i < se->out_sync_count; i++) {
  

• 	drm_syncobj_replace_fence(se-
  

...

out_syncs[i].syncobj,

• 				  done_fence);
  

• 	drm_syncobj_put(se->out_syncs[i].syncobj);
  

• }
  

• kvfree(se->out_syncs);
  

• return;
  

• }
• /* Single signal semaphore */ sync_out = drm_syncobj_find(file_priv, out_sync); if (sync_out) { drm_syncobj_replace_fence(sync_out, done_fence);

@@ -535,13 +568,107 @@ v3d_attach_fences_and_unlock_reservation(struct drm_file *file_priv, } }

+static void +v3d_put_multisync_post_deps(struct v3d_submit_ext *se) +{

• unsigned int i;
• for (i = 0; i < se->out_sync_count; i++)

• drm_syncobj_put(se->out_syncs[i].syncobj);
  

• kvfree(se->out_syncs);

+}

static int -v3d_get_extensions(struct drm_file *file_priv, u64 ext_handles) +v3d_get_multisync_post_deps(struct drm_file *file_priv,

• 	    struct v3d_submit_ext *se,
  

• 	    u32 count, u64 handles)
  

+{

• struct drm_v3d_sem __user *post_deps;
• int i, ret;
• se->out_syncs = (struct v3d_submit_outsync *)

• 	kvmalloc_array(count,
  

• 		       sizeof(struct
  

v3d_submit_outsync),

• 		       GFP_KERNEL);
  

• if (!se->out_syncs)

• return -ENOMEM;
  

• post_deps = u64_to_user_ptr(handles);
• for (i = 0; i < count; i++) {

• struct drm_v3d_sem out;
  

• ret = copy_from_user(&out, post_deps++, sizeof(out));
  

• if (ret) {
  

• 	DRM_DEBUG("Failed to copy post dep handles\n");
  

• 	goto fail;
  

• }
  

• se->out_syncs[i].syncobj = drm_syncobj_find(file_priv,
  

• 					    out.handle)
  

;

• if (!se->out_syncs[i].syncobj) {
  

• 	ret = -EINVAL;
  

• 	goto fail;
  

• }
  

• }
• se->out_sync_count = count;
• return 0;

+fail:

• for (i--; i >= 0; i--)

• drm_syncobj_put(se->out_syncs[i].syncobj);
  

• kvfree(se->out_syncs);
• return ret;

+}

+/* Get data for multiple binary semaphores synchronization. Parse syncobj

• • to be signaled when job completes (out_sync).
• */

+static int +v3d_get_multisync_submit_deps(struct drm_file *file_priv,

• 	      struct drm_v3d_extension __user *ext,
  

• 	      void *data)
  

+{

• struct drm_v3d_multi_sync multisync;
• struct v3d_submit_ext *se = data;
• int ret;
• ret = copy_from_user(&multisync, ext, sizeof(multisync));
• if (ret)

• return ret;
  

• if (multisync.pad)

• return -EINVAL;
  

• ret = v3d_get_multisync_post_deps(file_priv, data,

multisync.out_sync_count,

• 			  multisync.out_syncs);
  

• if (ret)

• return ret;
  

• se->in_sync_count = multisync.in_sync_count;
• se->in_syncs = multisync.in_syncs;
• se->wait_stage = multisync.wait_stage;
• return 0;

+}

+/* Whenever userspace sets ioctl extensions, parses data according to the

• • extension id (name). By now, generic ioctl extensions is only

used to

• • support multiple binary semaphores (struct drm_v3d_multi_sync).
• */

Let's remove the sentence that starts with "By now, generic..." it is not really relevant and it is bound to get obsolete when we add more extensions.

ok. I think I should also move this comment to the previous patch where the function below was created.

...

...

+static int +v3d_get_extensions(struct drm_file *file_priv,

•    u64 ext_handles,
  

•    void *data)
  

{ struct drm_v3d_extension __user *user_ext;

• int ret;

  user_ext = u64_to_user_ptr(ext_handles);

• while(user_ext) {

• while (user_ext) { struct drm_v3d_extension ext;

  if (copy_from_user(&ext, user_ext, sizeof(ext))) {

@@ -550,7 +677,11 @@ v3d_get_extensions(struct drm_file *file_priv, u64 ext_handles) }

switch (ext.id) {

• case 0:
  

• case DRM_V3D_EXT_ID_MULTI_SYNC:
  

• 	ret = v3d_get_multisync_submit_deps(file_priv,
  

user_ext, data);

• 	if (ret)
  

• 		return ret;
  

• 	break;
  

  default: DRM_DEBUG_DRIVER("Unknown extension id: %d\n",

ext.id); return -EINVAL; @@ -581,6 +712,7 @@ v3d_submit_cl_ioctl(struct drm_device *dev, void *data, struct v3d_dev *v3d = to_v3d_dev(dev); struct v3d_file_priv *v3d_priv = file_priv->driver_priv; struct drm_v3d_submit_cl *args = data;

• struct v3d_submit_ext se = {0}; struct v3d_bin_job *bin = NULL; struct v3d_render_job *render; struct v3d_job *clean_job = NULL;

@@ -601,7 +733,7 @@ v3d_submit_cl_ioctl(struct drm_device *dev, void *data, }

if (args->flags & DRM_V3D_SUBMIT_EXTENSION) {

• ret = v3d_get_extensions(file_priv, args->extensions);
  

• ret = v3d_get_extensions(file_priv, args->extensions,
  

&se); if (ret) { DRM_DEBUG("Failed to get extensions.\n"); return ret; @@ -609,7 +741,7 @@ v3d_submit_cl_ioctl(struct drm_device *dev, void *data, }

ret = v3d_job_init(v3d, file_priv, (void *)&render, sizeof(*render),

• 	   v3d_render_job_free, args->in_sync_bcl,
  

V3D_RENDER);

• 	   v3d_render_job_free, args->in_sync_bcl, &se,
  

V3D_RENDER);

So here is an example of what I was saying above... if the driver is using the multisync interface with 0 in_syncs, v3d_job_init here will think we are not using multisync and instead try to use args-

...

in_sync_bcl, which is not what we want. The same issue happens in all

other submission ioctls.

...

if (ret) goto fail;

@@ -619,7 +751,7 @@ v3d_submit_cl_ioctl(struct drm_device *dev, void *data,

if (args->bcl_start != args->bcl_end) { ret = v3d_job_init(v3d, file_priv, (void *)&bin, sizeof(*bin),

• 		   v3d_job_free, args->in_sync_bcl,
  

V3D_BIN);

• 		   v3d_job_free, args->in_sync_bcl,
  

&se, V3D_BIN);

Same here.

...

if (ret)
	goto fail;

@@ -633,7 +765,7 @@ v3d_submit_cl_ioctl(struct drm_device *dev, void *data,

if (args->flags & DRM_V3D_SUBMIT_CL_FLUSH_CACHE) { ret = v3d_job_init(v3d, file_priv, (void *)&clean_job, sizeof(*clean_job),

• 		   v3d_job_free, 0, V3D_CACHE_CLEAN);
  

• 		   v3d_job_free, 0, 0,
  

V3D_CACHE_CLEAN); if (ret) goto fail;

@@ -693,6 +825,7 @@ v3d_submit_cl_ioctl(struct drm_device *dev, void *data, last_job, &acquire_ctx, args->out_sync,

• 				 &se,
  			 last_job->done_fence);
  
  

  if (bin)

@@ -711,6 +844,7 @@ v3d_submit_cl_ioctl(struct drm_device *dev, void *data, v3d_job_cleanup((void *)bin); v3d_job_cleanup((void *)render); v3d_job_cleanup(clean_job);

• v3d_put_multisync_post_deps(&se);

  return ret;

} @@ -730,6 +864,7 @@ v3d_submit_tfu_ioctl(struct drm_device *dev, void *data, { struct v3d_dev *v3d = to_v3d_dev(dev); struct drm_v3d_submit_tfu *args = data;

• struct v3d_submit_ext se = {0}; struct v3d_tfu_job *job; struct ww_acquire_ctx acquire_ctx; int ret = 0;

@@ -742,7 +877,7 @@ v3d_submit_tfu_ioctl(struct drm_device *dev, void *data, }

if (args->flags & DRM_V3D_SUBMIT_EXTENSION) {

• ret = v3d_get_extensions(file_priv, args->extensions);
  

• ret = v3d_get_extensions(file_priv, args->extensions,
  

&se); if (ret) { DRM_DEBUG("Failed to get extensions.\n"); return ret; @@ -750,7 +885,7 @@ v3d_submit_tfu_ioctl(struct drm_device *dev, void *data, }

ret = v3d_job_init(v3d, file_priv, (void *)&job, sizeof(*job),

• 	   v3d_job_free, args->in_sync, V3D_TFU);
  

• 	   v3d_job_free, args->in_sync, &se, V3D_TFU);
  

Same here.

...

if (ret) goto fail;

@@ -798,6 +933,7 @@ v3d_submit_tfu_ioctl(struct drm_device *dev, void *data, v3d_attach_fences_and_unlock_reservation(file_priv, &job->base, &acquire_ctx, args->out_sync,

• 				 &se,
  			 job->base.done_fence);
  
  

  v3d_job_put(&job->base);

@@ -806,6 +942,7 @@ v3d_submit_tfu_ioctl(struct drm_device *dev, void *data,

fail: v3d_job_cleanup((void *)job);

• v3d_put_multisync_post_deps(&se);

  return ret;

} @@ -826,8 +963,9 @@ v3d_submit_csd_ioctl(struct drm_device *dev, void *data, struct v3d_dev *v3d = to_v3d_dev(dev); struct v3d_file_priv *v3d_priv = file_priv->driver_priv; struct drm_v3d_submit_csd *args = data;

• struct v3d_submit_ext se = {0}; struct v3d_csd_job *job;

• struct v3d_job *clean_job;

• struct v3d_job *clean_job = NULL; struct ww_acquire_ctx acquire_ctx; int ret;

@@ -847,7 +985,7 @@ v3d_submit_csd_ioctl(struct drm_device *dev, void *data, }

if (args->flags & DRM_V3D_SUBMIT_EXTENSION) {

• ret = v3d_get_extensions(file_priv, args->extensions);
  

• ret = v3d_get_extensions(file_priv, args->extensions,
  

&se); if (ret) { DRM_DEBUG("Failed to get extensions.\n"); return ret; @@ -855,12 +993,12 @@ v3d_submit_csd_ioctl(struct drm_device *dev, void *data, }

ret = v3d_job_init(v3d, file_priv, (void *)&job, sizeof(*job),

• 	   v3d_job_free, args->in_sync, V3D_CSD);
  

• 	   v3d_job_free, args->in_sync, &se, V3D_CSD);
  

Same here.

...

if (ret) goto fail;

ret = v3d_job_init(v3d, file_priv, (void *)&clean_job, sizeof(*clean_job),

• 	   v3d_job_free, 0, V3D_CACHE_CLEAN);
  

• 	   v3d_job_free, 0, 0, V3D_CACHE_CLEAN);
  

  if (ret) goto fail;

@@ -899,6 +1037,7 @@ v3d_submit_csd_ioctl(struct drm_device *dev, void *data, clean_job, &acquire_ctx, args->out_sync,

• 				 &se,
  			 clean_job-
  

...

done_fence);

v3d_job_put(&job->base); @@ -913,6 +1052,7 @@ v3d_submit_csd_ioctl(struct drm_device *dev, void *data, fail: v3d_job_cleanup((void *)job); v3d_job_cleanup(clean_job);

• v3d_put_multisync_post_deps(&se);

  return ret;

} diff --git a/include/uapi/drm/v3d_drm.h b/include/uapi/drm/v3d_drm.h index 55b443ca6c0b..3dfc0af8756a 100644 --- a/include/uapi/drm/v3d_drm.h +++ b/include/uapi/drm/v3d_drm.h @@ -73,6 +73,53 @@ struct drm_v3d_extension { __u32 flags; /* mbz */ };

+/* struct drm_v3d_sem - wait/signal semaphore

• • If binary semaphore, it only takes syncobj handle and ignores

flags and

• • point fields. Point is defined for timeline syncobj feature.
• */

+struct drm_v3d_sem {

• __u32 handle; /* syncobj */
• /* rsv below, for future uses */
• __u32 flags;
• __u64 point; /* for timeline sem support */
• __u64 mbz[2]; /* must be zero, rsv */

+};

+/* Enum for each of the V3D queues. */ +enum v3d_queue {

• V3D_BIN,
• V3D_RENDER,
• V3D_TFU,
• V3D_CSD,
• V3D_CACHE_CLEAN,

+};

+/**

• • struct drm_v3d_multi_sync - ioctl extension to add support

multiples

• • syncobjs for commands submission.
• • When an extension of DRM_V3D_EXT_ID_MULTI_SYNC id is defined, it

points to

• • this extension to define wait and signal dependencies, instead of

single

• • in/out sync entries on submitting commands. The field flags is

used to

• • determine the stage to set wait dependencies.
• */

+struct drm_v3d_multi_sync {

• struct drm_v3d_extension base;
• /* Array of wait and signal semaphores */
• __u64 in_syncs;
• __u64 out_syncs;
• /* Number of entries */
• __u32 in_sync_count;
• __u32 out_sync_count;
• /* set the stage (v3d_queue) to sync */
• __u32 wait_stage;
• __u32 pad; /* mbz */

+};

/**

• struct drm_v3d_submit_cl - ioctl argument for submitting commands

to the 3D

• engine.

@@ -228,6 +275,7 @@ enum drm_v3d_param { DRM_V3D_PARAM_SUPPORTS_CSD, DRM_V3D_PARAM_SUPPORTS_CACHE_FLUSH, DRM_V3D_PARAM_SUPPORTS_PERFMON,

• DRM_V3D_PARAM_SUPPORTS_MULTISYNC_EXT,

};

struct drm_v3d_get_param { @@ -271,7 +319,6 @@ struct drm_v3d_submit_tfu {

/* Pointer to an array of ioctl extensions*/ __u64 extensions;

};

/* Submits a compute shader for dispatch. This job will block on any