[PATCH v2 3/4] drm/v3d: add generic ioctl extension

[Iago Toral]

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 {