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

[Chris Wilson]

On Tue, Jan 27, 2015 at 12:13:14PM +0000, Tvrtko Ursulin wrote:
> 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.

So you are saying that we have a callback for when the fence is
discarded by userspace and we ignore that opportunity for disabling
interrupt generation, and remove the extra request references?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre