Re: [PATCH] drm/i915: Android sync points for i915 v2

[Jesse Barnes <jbarnes@virtuousgeek.org>]

On Tue, 5 Aug 2014 09:44:00 +0200
Daniel Vetter <daniel@ffwll.ch> wrote:

> On Tue, Aug 5, 2014 at 1:18 AM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> > +#define DRM_IOCTL_I915_GEM_FENCE                       DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GEM_FENCE, struct drm_i915_gem_fence)
> >
> >  /* Allow drivers to submit batchbuffers directly to hardware, relying
> >   * on the security mechanisms provided by hardware.
> > @@ -1066,4 +1068,25 @@ struct drm_i915_gem_userptr {
> >         __u32 handle;
> >  };
> >
> > +/**
> > + * drm_i915_gem_fence - create a fence
> > + * @fd: fd for fence
> > + * @ctx_id: context ID for fence
> > + * @flags: flags for operation
> > + *
> > + * Creates a fence in @fd and returns it to the caller.  This fd can be
> > + * passed around between processes as any other fd, and can be poll'd
> > + * and read for status.
> > + *
> > + * RETURNS:
> > + * A valid fd in the @fd field or an errno on error.
> > + */
> > +struct drm_i915_gem_fence {
> > +       __s32 fd;
> > +       __u32 ctx_id;
> > +       __u32 flags;
> > +       __u32 pad;
> > +       char name[32];
> > +};
> > +
> 
> This doesn't really look like the interface I'd expected. Imo we just
> need to add a flag to execbuf so that userspace can tell the kernel to
> create a fence for that execbuf, and switch one of the leftover rsvd
> fields to __s32 as an outparam for the fd.

Given that I've got a new execbuf coming too, I just wanted to keep
them separate.  Any compelling reason to try to wedge it into execbuf?

> Then we need similar flags for vblank events and pageflips to do the
> same (obviously those are drm core patches) and it's all there. That
> should probably integrated as a special type of drm_event, so that
> drivers don't need to change a single line of code.

Except for actually using the fences...

> Also this should be based on top of Chris' patch to refcount requests
> and make them first-class structures. Then we can simply replace the
> embedded struct kref with a struct fence, i.e. we'll always create a
> fence, but only give userspace an fd handle for it when it asks for
> it.

Yeah I think that was mentioned in the commit.  Once Chris's stuff
lands this should look even simpler.

> For merging there's a few things we need:
> - Some open-source user, either the open-source android-ia project or
> something else.
> - The android syncpt stuff obviously needs to be de-staged. From my
> side that means an ABI review of what's there (and getting the buy-in
> from google guys if we need to change it) plus a full set of testcases
> (if google doesn't already have something we could integrate easily).
> Adding Greg and relevant people.

Yep, I'm hoping Chris has a use for this too in the DDX.  I think
Wayland wants it too.

-- 
Jesse Barnes, Intel Open Source Technology Center