[Intel-gfx] [PATCH] drm/i915: Android sync points for i915 v2

[Daniel Vetter]

On Tue, Aug 5, 2014 at 6:05 PM, Jesse Barnes <jbarnes at virtuousgeek.org> wrote:
> On Tue, 5 Aug 2014 17:08:22 +0200
> Daniel Vetter <daniel at ffwll.ch> wrote:
>
>> On Tue, Aug 5, 2014 at 4:59 PM, Jesse Barnes <jbarnes at 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.

This comment was specifically about vblank and pageflips, _not_ about
execbuf. At least that's been what I've thought while writing the
original mail and reading your reply. Looks like we have a
misunderstanding here. Since for vblank and pageflip we really can do
it all in the drm core.

>> >> 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?

Well for de-staging someone gets to do a review of the android synpts
interface. And depending upon how syncpts are used in the hwc and gl
extensions to get at those fences and use them we'll have different
requirements for the i915-specific execbuf support. And for the
generic support in the drm core for pageflips.

> 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...

Why that? They should really just provide the userspace interface on top ...
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch