``` > +/* Syncobj reference passed at job submission time to encode explicit > + * input/output fences. > + */ > +struct drm_panfrost_syncobj_ref { > + __u32 handle; > + __u32 pad; > + __u64 point; > +}; ``` What is handle? What is point? Why is there padding instead of putting point first? ``` > #define PANFROST_BO_REF_EXCLUSIVE 0x1 > +#define PANFROST_BO_REF_NO_IMPLICIT_DEP 0x2 ``` This seems logically backwards. NO_IMPLICIT_DEP makes sense if we're trying to keep backwards compatibility, but here you're crafting a new interface totally from scratch. If anything, isn't BO_REF_IMPLICIT_DEP the flag you'd want? ``` > + /** > + * Stride of the jobs array (needed to ease extension of the > + * BATCH_SUBMIT ioctl). Should be set to > + * sizeof(struct drm_panfrost_job). > + */ > + __u32 job_stride; ... > + /** > + * Stride of the BO and syncobj reference arrays (needed to ease > + * extension of the BATCH_SUBMIT ioctl). Should be set to > + * sizeof(struct drm_panfrost_bo_ref). > + */ > + __u32 bo_ref_stride; > + __u32 syncobj_ref_stride; ``` Hmm. I'm not /opposed/ and I know kbase uses strides but it seems like somewhat unwarranted complexity, and there is a combinatoric explosion here (if jobs, bo refs, and syncobj refs use 3 different versions, as this encoding permits... as opposed to just specifying a UABI version or something like that) ``` > + /** > + * If the submission fails, this encodes the index of the job > + * failed. > + */ > + __u32 fail_idx; ``` What if multiple jobs fail? ``` > + /** > + * ID of the queue to submit those jobs to. 0 is the default > + * submit queue and should always exists. If you need a dedicated > + * queue, create it with DRM_IOCTL_PANFROST_CREATE_SUBMITQUEUE. > + */ > + __u32 queue; ``` s/exists/exist/