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@xxxxxxxxxx> > --- > 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 {