Re: [RFC v3] drm/i915: Android native sync support

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

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@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@intel.com>
>> Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
>> Cc: John Harrison <John.C.Harrison@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?

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

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.

Regards,

Tvrtko

P.S. Thanks for pointing out self-tuning busy loads in another thread - 
it works nicely.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx