[PATCH] drm/i915: Convert breadcrumbs spinlock to be irqsafe

* [PATCH] drm/i915: Convert breadcrumbs spinlock to be irqsafe
@ 2016-10-27 16:10 Chris Wilson
  2016-10-27 16:48 ` ✓ Fi.CI.BAT: success for " Patchwork
                   ` (5 more replies)
  0 siblings, 6 replies; 18+ messages in thread
From: Chris Wilson @ 2016-10-27 16:10 UTC (permalink / raw)
  To: intel-gfx

The breadcrumbs are about to be used from within IRQ context sections,
therefore we need to employ the irqsafe spinlock variants.

(This is split out of the defer global seqno allocation patch due to
realisation that we need a more complete conversion if we want to defer
request submission even further.)

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com
---
 drivers/gpu/drm/i915/i915_debugfs.c      | 12 +++++------
 drivers/gpu/drm/i915/intel_breadcrumbs.c | 34 +++++++++++++++++++-------------
 2 files changed, 26 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 925aaedd7fd9..8d3cb8f5ff41 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -683,14 +683,14 @@ static void i915_ring_seqno_info(struct seq_file *m,
 	seq_printf(m, "Current sequence (%s): %x\n",
 		   engine->name, intel_engine_get_seqno(engine));
 
-	spin_lock(&b->lock);
+	spin_lock_irq(&b->lock);
 	for (rb = rb_first(&b->waiters); rb; rb = rb_next(rb)) {
 		struct intel_wait *w = container_of(rb, typeof(*w), node);
 
 		seq_printf(m, "Waiting (%s): %s [%d] on %x\n",
 			   engine->name, w->tsk->comm, w->tsk->pid, w->seqno);
 	}
-	spin_unlock(&b->lock);
+	spin_unlock_irq(&b->lock);
 }
 
 static int i915_gem_seqno_info(struct seq_file *m, void *data)
@@ -1355,14 +1355,14 @@ static int i915_hangcheck_info(struct seq_file *m, void *unused)
 			   yesno(intel_engine_has_waiter(engine)),
 			   yesno(test_bit(engine->id,
 					  &dev_priv->gpu_error.missed_irq_rings)));
-		spin_lock(&b->lock);
+		spin_lock_irq(&b->lock);
 		for (rb = rb_first(&b->waiters); rb; rb = rb_next(rb)) {
 			struct intel_wait *w = container_of(rb, typeof(*w), node);
 
 			seq_printf(m, "\t%s [%d] waiting for %x\n",
 				   w->tsk->comm, w->tsk->pid, w->seqno);
 		}
-		spin_unlock(&b->lock);
+		spin_unlock_irq(&b->lock);
 
 		seq_printf(m, "\tACTHD = 0x%08llx [current 0x%08llx]\n",
 			   (long long)engine->hangcheck.acthd,
@@ -3265,14 +3265,14 @@ static int i915_engine_info(struct seq_file *m, void *unused)
 				   I915_READ(RING_PP_DIR_DCLV(engine)));
 		}
 
-		spin_lock(&b->lock);
+		spin_lock_irq(&b->lock);
 		for (rb = rb_first(&b->waiters); rb; rb = rb_next(rb)) {
 			struct intel_wait *w = container_of(rb, typeof(*w), node);
 
 			seq_printf(m, "\t%s [%d] waiting for %x\n",
 				   w->tsk->comm, w->tsk->pid, w->seqno);
 		}
-		spin_unlock(&b->lock);
+		spin_unlock_irq(&b->lock);
 
 		seq_puts(m, "\n");
 	}
diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c b/drivers/gpu/drm/i915/intel_breadcrumbs.c
index 0d5def0d2dfe..00b25a7cef95 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;
 }
@@ -291,11 +291,12 @@ bool intel_engine_add_wait(struct intel_engine_cs *engine,
 			   struct intel_wait *wait)
 {
 	struct intel_breadcrumbs *b = &engine->breadcrumbs;
+	unsigned long flags;
 	bool first;
 
-	spin_lock(&b->lock);
+	spin_lock_irqsave(&b->lock, flags);
 	first = __intel_engine_add_wait(engine, wait);
-	spin_unlock(&b->lock);
+	spin_unlock_irqrestore(&b->lock, flags);
 
 	return first;
 }
@@ -318,6 +319,7 @@ void intel_engine_remove_wait(struct intel_engine_cs *engine,
 			      struct intel_wait *wait)
 {
 	struct intel_breadcrumbs *b = &engine->breadcrumbs;
+	unsigned long flags;
 
 	/* Quick check to see if this waiter was already decoupled from
 	 * the tree by the bottom-half to avoid contention on the spinlock
@@ -326,7 +328,7 @@ void intel_engine_remove_wait(struct intel_engine_cs *engine,
 	if (RB_EMPTY_NODE(&wait->node))
 		return;
 
-	spin_lock(&b->lock);
+	spin_lock_irqsave(&b->lock, flags);
 
 	if (RB_EMPTY_NODE(&wait->node))
 		goto out_unlock;
@@ -400,7 +402,7 @@ void intel_engine_remove_wait(struct intel_engine_cs *engine,
 	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);
+	spin_unlock_irqrestore(&b->lock, flags);
 }
 
 static bool signal_complete(struct drm_i915_gem_request *request)
@@ -457,6 +459,8 @@ static int intel_breadcrumbs_signaler(void *arg)
 		 */
 		request = READ_ONCE(b->first_signal);
 		if (signal_complete(request)) {
+			unsigned long flags;
+
 			/* Wake up all other completed waiters and select the
 			 * next bottom-half for the next user interrupt.
 			 */
@@ -473,14 +477,14 @@ 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);
+			spin_lock_irqsave(&b->lock, flags);
 			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_irqrestore(&b->lock, flags);
 
 			i915_gem_request_put(request);
 		} else {
@@ -500,6 +504,7 @@ void intel_engine_enable_signaling(struct drm_i915_gem_request *request)
 	struct intel_engine_cs *engine = request->engine;
 	struct intel_breadcrumbs *b = &engine->breadcrumbs;
 	struct rb_node *parent, **p;
+	unsigned long flags;
 	bool first, wakeup;
 
 	/* locked by dma_fence_enable_sw_signaling() */
@@ -511,7 +516,7 @@ 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);
+	spin_lock_irqsave(&b->lock, flags);
 
 	/* First add ourselves into the list of waiters, but register our
 	 * bottom-half as the signaller thread. As per usual, only the oldest
@@ -545,7 +550,7 @@ void intel_engine_enable_signaling(struct drm_i915_gem_request *request)
 	if (first)
 		smp_store_mb(b->first_signal, request);
 
-	spin_unlock(&b->lock);
+	spin_unlock_irqrestore(&b->lock, flags);
 
 	if (wakeup)
 		wake_up_process(b->signaler);
@@ -592,9 +597,10 @@ static void cancel_fake_irq(struct intel_engine_cs *engine)
 void intel_engine_reset_breadcrumbs(struct intel_engine_cs *engine)
 {
 	struct intel_breadcrumbs *b = &engine->breadcrumbs;
+	unsigned long flags;
 
 	cancel_fake_irq(engine);
-	spin_lock(&b->lock);
+	spin_lock_irqsave(&b->lock, flags);
 
 	__intel_breadcrumbs_disable_irq(b);
 	if (intel_engine_has_waiter(engine)) {
@@ -607,7 +613,7 @@ void intel_engine_reset_breadcrumbs(struct intel_engine_cs *engine)
 		irq_disable(engine);
 	}
 
-	spin_unlock(&b->lock);
+	spin_unlock_irqrestore(&b->lock, flags);
 }
 
 void intel_engine_fini_breadcrumbs(struct intel_engine_cs *engine)
-- 
2.10.1

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

^ permalink raw reply related	[flat|nested] 18+ messages in thread