[Intel-gfx] [RFC v3] drm/i915: Android native sync support

[Daniel Vetter]

On Wed, Jan 28, 2015 at 04:52:53PM +0000, Tvrtko Ursulin wrote:
> 
> On 01/28/2015 09:29 AM, Daniel Vetter wrote:
> >On Tue, Jan 27, 2015 at 11:29:36AM +0000, Tvrtko Ursulin wrote:
> >>From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> >>
> >>Add Android native sync support with fences exported as file descriptors via
> >>the execbuf ioctl (rsvd2 field is used).
> >>
> >>This is a continuation of Jesse Barnes's previous work, squashed to arrive at
> >>the final destination, cleaned up, with some fixes and preliminary light
> >>testing.
> >>
> >>GEM requests are extended with fence structures which are associated with
> >>Android sync fences exported to user space via file descriptors. Fences which
> >>are waited upon, and while exported to userspace, are referenced and added to
> >>the irq_queue so they are signalled when requests are completed. There is no
> >>overhead apart from the where fences are not requested.
> >>
> >>Based on patches by Jesse Barnes:
> >>    drm/i915: Android sync points for i915 v3
> >>    drm/i915: add fences to the request struct
> >>    drm/i915: sync fence fixes/updates
> >>
> >>To do:
> >>    * Extend driver data with context id / ring id (TBD).
> >>
> >>v2:
> >>    * Code review comments. (Chris Wilson)
> >>    * ring->add_request() was a wrong thing to call - rebase on top of John
> >>      Harrison's (drm/i915: Early alloc request) to ensure correct request is
> >>      present before creating a fence.
> >>    * Take a request reference from signalling path as well to ensure request
> >>      sticks around while fence is on the request completion wait queue.
> >>
> >>v3:
> >>    * Use worker to unreference on completion so it can lock the mutex from
> >>      interrupt context.
> >>
> >>Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> >>Cc: Jesse Barnes <jbarnes at virtuousgeek.org>
> >>Cc: John Harrison <John.C.Harrison at Intel.com>
> >
> >btw for merging we need to split the conversion to fences out from the
> >actual userspace interface. And imo we should replace a lot of our
> >existing usage of i915_gem_request with fences within the driver, too. Not
> >tacked on the side like in your patch here. All the recent refactoring
> >have been aiming to match i915 requests to the internal fence interfaces,
> >so we should be pretty much there.
> 
> Ok I did not realize this. It did not make sense to me to split it out if
> the only access point to create them is via Android native sync, but from
> what you are saying fences should be initialized and active for all requests
> all the time. With the native sync part established on demand.
> 
> In what respects has the refactoring been aligning requests and fences?

requests are also refcounted, like fences. The actual interface for
waiting might still have a slight mismatch.

> >We also need this to be able to integrate with the scheduler properly - if
> >that needs to deal both with fences and i915 requests separately it'll be
> >a bit more messy. If it's all just fences the code should be streamlined a
> >lot.
> 
> Requests will remain as main data structure representing the unit of GPU
> work?

Yes. requests will be just a subclass of fences (through structure
embedding).

> Is so, it sounds logical that fences are associated (or aggregated) with
> requests. I don't see how scheduler would work with fences and not requests.

The important part is that the scheduler can work with fences which are
_not_ i915 requests (e.g. from the camera pipeline or similar things).
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch