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

[Jesse Barnes <jbarnes@virtuousgeek.org>]

On Tue, 5 Aug 2014 17:08:22 +0200
Daniel Vetter <daniel@ffwll.ch> wrote:

> On Tue, Aug 5, 2014 at 4:59 PM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> >> 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?
> 
> The new execbuf is for svm, and there we obviously need fences. But we
> also need proper fence support everywhere else (hence also the comment
> that we need support for fences in drm events).
> 
> >> 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...
> 
> Actually no, nothing needed - drivers already signal drm_events in all
> the right places, so we really only need to change
> drm_send_vblank_event. And ofc we need to rework the code in the
> pageflip/atomic/vblank_wait ioctl code in the drm core to create a
> fence (and return it to userspace) instead of a normal drm event.

Actually yes.  You get back a fence object and want to do something
with it, right?  That means new code.  Plus modifying current execbuf
users that want fences to pass in a flag.

> >> 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.
> 
> Well since syncpts are originally from Android I'd really prefere an
> Android based implementation - otherwise we might create something by
> accident that's not suitable for Android.

I don't see how using the Android sync points API might make something
not suitable for Android?

But yes, I want the Android guys to try this out too.  I've already
pinged them internally to check things out.  Probably the biggest
remaining opens there would be having a timeline for the display side
of things too that covers vblank and page flip events as fences in a
separate namespace.

Which makes me think going back to just using the Android structs
directly might be easier...

-- 
Jesse Barnes, Intel Open Source Technology Center