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

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

On 01/27/2015 11:40 AM, Chris Wilson wrote:
> On Tue, Jan 27, 2015 at 11:29:36AM +0000, Tvrtko Ursulin wrote:
>> +static void i915_gem_request_unreference_worker(struct work_struct *work)
>> +{
>> +	struct drm_i915_gem_request *req =
>> +		container_of(work, struct drm_i915_gem_request, unref_work);
>> +	struct drm_device *dev = req->ring->dev;
>> +
>> +	mutex_lock(&dev->struct_mutex);
>> +	i915_gem_request_unreference(req);
>> +	mutex_unlock(&dev->struct_mutex);
>> +}
>> +
>> +static int
>> +i915_fence_ring_check(wait_queue_t *wait, unsigned mode, int flags, void *key)
>> +{
>> +	struct drm_i915_gem_request *req = wait->private;
>> +	struct intel_engine_cs *ring = req->ring;
>> +
>> +	if (!i915_gem_request_completed(req, false))
>> +		return 0;
>> +
>> +	fence_signal_locked(&req->fence);
>> +
>> +	__remove_wait_queue(&ring->irq_queue, wait);
>> +	ring->irq_put(ring);
>> +
>> +	INIT_WORK(&req->unref_work, i915_gem_request_unreference_worker);
>> +	schedule_work(&req->unref_work);
>> +
>> +	return 0;
>> +}
>> +
>> +static bool i915_fence_ring_enable_signaling(struct fence *fence)
>> +{
>> +	struct drm_i915_gem_request *req = to_i915_request(fence);
>> +	struct intel_engine_cs *ring = req->ring;
>> +	struct drm_i915_private *dev_priv = ring->dev->dev_private;
>> +	wait_queue_t *wait = &req->wait;
>> +
>> +	/* queue fence wait queue on irq queue and get fence */
>> +	if (i915_gem_request_completed(req, false) ||
>> +	    i915_terminally_wedged(&dev_priv->gpu_error))
>> +		return false;
>> +
>> +	if (!ring->irq_get(ring))
>> +		return false;
>> +
>> +	if (i915_gem_request_completed(req, false)) {
>> +		ring->irq_put(ring);
>> +		return true;
>> +	}
>> +
>> +	wait->flags = 0;
>> +	wait->private = req;
>> +	wait->func = i915_fence_ring_check;
>> +
>> +	i915_gem_request_reference(req);
>> +
>> +	__add_wait_queue(&ring->irq_queue, wait);
>> +
>> +	return true;
>> +}
>
> Explain how fence_release interacts with enable_signalling. Presumably
> either the core fence routines cleanup the outstanding signalling, or we
> are expected to. Doing so will remove the requirement for the extra
> ref/unref (including the worker).

I think normally we would be expected to use fence_get/put on the 
signaling side of things but that is not possible here since struct 
fence is embedded in the request.

So as it is, sync_fence_release will tear everything down with our 
callback still on the irq_queue which is what taking a request reference 
sorts out.

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx