[PATCH v3 5/7] drm/panfrost: Add a new ioctl to submit batches
Alyssa Rosenzweig
alyssa at collabora.com
Fri Jul 2 16:49:55 UTC 2021
> > What is handle? What is point?
>
> Handle is a syncobj handle, point is the point in a syncobj timeline.
> I'll document those fields.
OK.
> > Why is there padding instead of putting point first?
>
> We can move the point field first, but we need to keep the explicit
> padding: the struct has to be 64bit aligned because of the __u64 field
> (which the compiler takes care of) but if we don't have an explicit
> padding, the unused 32bits are undefined, which might cause trouble if
> we extend the struct at some point, since we sort of expect that old
> userspace keep this unused 32bit slot to 0, while new users set
> non-zero values if they have to.
Makes sense. Reordering still probably makes sense.
> > ```
> > > #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?
>
> AFAICT, all other drivers make the no-implicit-dep an opt-in, and I
> didn't want to do things differently in panfrost. But if that's really
> an issue, I can make it an opt-out.
I don't have strong feelings either way. I was just under the
impressions other drivers did this for b/w compat reasons which don't
apply here.
> > 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)
>
> Sounds like a good idea. I'll add a version field and map that
> to a <job_stride,bo_ref_stride,syncobj_ref_stride> tuple.
Cc Steven, does this make sense?
> > > + /**
> > > + * If the submission fails, this encodes the index of the job
> > > + * failed.
> > > + */
> > > + __u32 fail_idx;
> > ```
> >
> > What if multiple jobs fail?
>
> We stop at the first failure. Note that it's not an execution failure,
> but a submission failure (AKA, userspace passed wrong params, like
> invalid BO or synobj handles).
I see, ok.
More information about the dri-devel
mailing list