[PATCH 27/27] drm/i915: Move per-fence spinlock over to shared breadcrumb lock

Chris Wilson chris at chris-wilson.co.uk
Sat Sep 3 13:08:40 UTC 2016


Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_gem_request.c  |  5 +-
 drivers/gpu/drm/i915/i915_gem_request.h  |  1 -
 drivers/gpu/drm/i915/intel_breadcrumbs.c | 81 +++++++++++++++++---------------
 3 files changed, 47 insertions(+), 40 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
index 2ec27c5c74af..ab61aec00522 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.c
+++ b/drivers/gpu/drm/i915/i915_gem_request.c
@@ -193,6 +193,8 @@ static void i915_gem_request_retire(struct drm_i915_gem_request *request)
 	}
 
 	i915_gem_context_put(request->ctx);
+
+	fence_signal(&request->fence);
 	i915_gem_request_put(request);
 }
 
@@ -392,10 +394,9 @@ i915_gem_request_alloc(struct intel_engine_cs *engine,
 	if (ret)
 		goto err;
 
-	spin_lock_init(&req->lock);
 	fence_init(&req->fence,
 		   &i915_fence_ops,
-		   &req->lock,
+		   &engine->breadcrumbs.lock,
 		   engine->timeline->fence_context,
 		   seqno);
 
diff --git a/drivers/gpu/drm/i915/i915_gem_request.h b/drivers/gpu/drm/i915/i915_gem_request.h
index 48ba301d1ff6..ca7c85066da7 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.h
+++ b/drivers/gpu/drm/i915/i915_gem_request.h
@@ -63,7 +63,6 @@ struct intel_signal_node {
  */
 struct drm_i915_gem_request {
 	struct fence fence;
-	spinlock_t lock;
 
 	/** On Which ring this request was generated */
 	struct drm_i915_private *i915;
diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c b/drivers/gpu/drm/i915/intel_breadcrumbs.c
index 9ad1028681cf..79661cd56756 100644
--- a/drivers/gpu/drm/i915/intel_breadcrumbs.c
+++ b/drivers/gpu/drm/i915/intel_breadcrumbs.c
@@ -83,16 +83,16 @@ static void irq_enable(struct intel_engine_cs *engine)
 	 */
 	engine->breadcrumbs.irq_posted = true;
 
-	spin_lock_irq(&engine->i915->irq_lock);
+	spin_lock(&engine->i915->irq_lock);
 	engine->irq_enable(engine);
-	spin_unlock_irq(&engine->i915->irq_lock);
+	spin_unlock(&engine->i915->irq_lock);
 }
 
 static void irq_disable(struct intel_engine_cs *engine)
 {
-	spin_lock_irq(&engine->i915->irq_lock);
+	spin_lock(&engine->i915->irq_lock);
 	engine->irq_disable(engine);
-	spin_unlock_irq(&engine->i915->irq_lock);
+	spin_unlock(&engine->i915->irq_lock);
 
 	engine->breadcrumbs.irq_posted = false;
 }
@@ -293,9 +293,9 @@ bool intel_engine_add_wait(struct intel_engine_cs *engine,
 	struct intel_breadcrumbs *b = &engine->breadcrumbs;
 	bool first;
 
-	spin_lock(&b->lock);
+	spin_lock_irq(&b->lock);
 	first = __intel_engine_add_wait(engine, wait);
-	spin_unlock(&b->lock);
+	spin_unlock_irq(&b->lock);
 
 	return first;
 }
@@ -314,22 +314,12 @@ static inline int wakeup_priority(struct intel_breadcrumbs *b,
 		return tsk->prio;
 }
 
-void intel_engine_remove_wait(struct intel_engine_cs *engine,
-			      struct intel_wait *wait)
+static void __intel_engine_remove_wait(struct intel_breadcrumbs *b,
+				       struct intel_wait *wait)
 {
-	struct intel_breadcrumbs *b = &engine->breadcrumbs;
-
-	/* Quick check to see if this waiter was already decoupled from
-	 * the tree by the bottom-half to avoid contention on the spinlock
-	 * by the herd.
-	 */
-	if (RB_EMPTY_NODE(&wait->node))
-		return;
-
-	spin_lock(&b->lock);
-
+	assert_spin_locked(&b->lock);
 	if (RB_EMPTY_NODE(&wait->node))
-		goto out_unlock;
+		goto out;
 
 	if (b->first_wait == wait) {
 		const int priority = wakeup_priority(b, wait->tsk);
@@ -346,6 +336,11 @@ void intel_engine_remove_wait(struct intel_engine_cs *engine,
 		 */
 		next = rb_next(&wait->node);
 		if (chain_wakeup(next, priority)) {
+			struct intel_engine_cs *engine =
+				container_of(b,
+					     struct intel_engine_cs,
+					     breadcrumbs);
+
 			/* If the next waiter is already complete,
 			 * wake it up and continue onto the next waiter. So
 			 * if have a small herd, they will wake up in parallel
@@ -395,12 +390,28 @@ void intel_engine_remove_wait(struct intel_engine_cs *engine,
 	GEM_BUG_ON(RB_EMPTY_NODE(&wait->node));
 	rb_erase(&wait->node, &b->waiters);
 
-out_unlock:
+out:
 	GEM_BUG_ON(b->first_wait == wait);
 	GEM_BUG_ON(rb_first(&b->waiters) !=
 		   (b->first_wait ? &b->first_wait->node : NULL));
 	GEM_BUG_ON(!rcu_access_pointer(b->irq_seqno_bh) ^ RB_EMPTY_ROOT(&b->waiters));
-	spin_unlock(&b->lock);
+}
+
+void intel_engine_remove_wait(struct intel_engine_cs *engine,
+			      struct intel_wait *wait)
+{
+	struct intel_breadcrumbs *b = &engine->breadcrumbs;
+
+	/* Quick check to see if this waiter was already decoupled from
+	 * the tree by the bottom-half to avoid contention on the spinlock
+	 * by the herd.
+	 */
+	if (RB_EMPTY_NODE(&wait->node))
+		return;
+
+	spin_lock_irq(&b->lock);
+	__intel_engine_remove_wait(b, wait);
+	spin_unlock_irq(&b->lock);
 }
 
 static bool signal_complete(struct drm_i915_gem_request *request)
@@ -437,8 +448,7 @@ static void signaler_set_rtpriority(void)
 
 static int intel_breadcrumbs_signaler(void *arg)
 {
-	struct intel_engine_cs *engine = arg;
-	struct intel_breadcrumbs *b = &engine->breadcrumbs;
+	struct intel_breadcrumbs *b = arg;
 	struct drm_i915_gem_request *request;
 
 	/* Install ourselves with high priority to reduce signalling latency */
@@ -457,15 +467,15 @@ static int intel_breadcrumbs_signaler(void *arg)
 		 */
 		request = READ_ONCE(b->first_signal);
 		if (signal_complete(request)) {
+			local_bh_disable();
+			spin_lock_irq(&b->lock);
+
 			/* Wake up all other completed waiters and select the
 			 * next bottom-half for the next user interrupt.
 			 */
-			intel_engine_remove_wait(engine,
-						 &request->signaling.wait);
+			__intel_engine_remove_wait(b, &request->signaling.wait);
 
-			local_bh_disable();
-			fence_signal(&request->fence);
-			local_bh_enable(); /* kick start the tasklets */
+			fence_signal_locked(&request->fence);
 
 			/* Find the next oldest signal. Note that as we have
 			 * not been holding the lock, another client may
@@ -473,14 +483,15 @@ static int intel_breadcrumbs_signaler(void *arg)
 			 * we just completed - so double check we are still
 			 * the oldest before picking the next one.
 			 */
-			spin_lock(&b->lock);
 			if (request == b->first_signal) {
 				struct rb_node *rb =
 					rb_next(&request->signaling.node);
 				b->first_signal = rb ? to_signaler(rb) : NULL;
 			}
 			rb_erase(&request->signaling.node, &b->signals);
-			spin_unlock(&b->lock);
+
+			spin_unlock_irq(&b->lock);
+			local_bh_enable(); /* kick start the tasklets */
 
 			i915_gem_request_put(request);
 		} else {
@@ -503,7 +514,7 @@ void intel_engine_enable_signaling(struct drm_i915_gem_request *request)
 	bool first, wakeup;
 
 	/* locked by fence_enable_sw_signaling() */
-	assert_spin_locked(&request->lock);
+	assert_spin_locked(&b->lock);
 	if (!request->global_seqno)
 		return;
 
@@ -511,8 +522,6 @@ void intel_engine_enable_signaling(struct drm_i915_gem_request *request)
 	request->signaling.wait.seqno = request->global_seqno;
 	i915_gem_request_get(request);
 
-	spin_lock(&b->lock);
-
 	/* First add ourselves into the list of waiters, but register our
 	 * bottom-half as the signaller thread. As per usual, only the oldest
 	 * waiter (not just signaller) is tasked as the bottom-half waking
@@ -545,8 +554,6 @@ void intel_engine_enable_signaling(struct drm_i915_gem_request *request)
 	if (first)
 		smp_store_mb(b->first_signal, request);
 
-	spin_unlock(&b->lock);
-
 	if (wakeup)
 		wake_up_process(b->signaler);
 }
@@ -570,7 +577,7 @@ int intel_engine_init_breadcrumbs(struct intel_engine_cs *engine)
 	 * we create a thread to do the coherent seqno dance after the
 	 * interrupt and then signal the waitqueue (via the dma-buf/fence).
 	 */
-	tsk = kthread_run(intel_breadcrumbs_signaler, engine,
+	tsk = kthread_run(intel_breadcrumbs_signaler, b,
 			  "i915/signal:%d", engine->id);
 	if (IS_ERR(tsk))
 		return PTR_ERR(tsk);
-- 
2.9.3



More information about the Intel-gfx-trybot mailing list