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

[Tvrtko Ursulin]

On 01/22/2015 01:49 PM, Chris Wilson wrote:
> On Thu, Jan 22, 2015 at 01:41:48PM +0000, Tvrtko Ursulin wrote:
>>
>> On 01/22/2015 11:42 AM, Chris Wilson wrote:
>>>> +	if (args->flags & I915_EXEC_FENCE_OUT) {
>>>> +		ret = i915_create_sync_fence_ring(ring, ctx,
>>>> +						  &sync_fence, &fence_fd);
>>>> +		if (ret)
>>>> +			goto sync_err;
>>>> +	}
>>>> +
>>>>   	ret = dev_priv->gt.do_execbuf(dev, file, ring, ctx, args,
>>>>   				      &eb->vmas, batch_obj, exec_start, flags);
>>>
>>> You emit the fence prior to the execution of the batch? Interesting. Not
>>> exactly where I would expect the fence. Both before/after are
>>> justifiable.
>>
>> What do yo consider emitting? To me that is fd_install and that
>> happens after request was successfully submitted. I thought it is
>> tidier to set up required objects before and then install the fence,
>> or discard it, depending on the outcome. You think differently?
>
> i915_create_sync_fence_ring() inserts a breadcrumb into the ring that
> fires before we execute the execbuf (which then gets its own request +
> breadcrumb).
>
> I believe the intention is to hook the fence into the breadcrumb that
> fires after the execbuf, i.e. to add it to the execbuf request rather
> than create a new request all for itself.

You are right, it should be after.

I assumed that ring->add_request() is just to make sure there is a 
request structure for this submission, if some other operation hasn't 
created it already. This was based on my relatively old recollection of 
how this code works. It looks like I need to re-visit this.

Regards,

Tvrtko