[Intel-gfx] [PATCH 2/3] drm/i915/guc: Always enable the breadcrumbs irq

Chris Wilson chris at chris-wilson.co.uk
Fri Oct 20 21:06:39 UTC 2017


The execlists emulation on top of the GuC (used for scheduling and
preemption) depends on the MI_USER_INTERRUPT for its notifications and
tasklet action. As we always employ the irq, there is no advantage in
ever disabling it while we are using the GuC, so allow us to arm the
breadcrumb irq when enabling GuC submission and disarm upon disabling.
The impact should be lessened by the delayed irq disabling we do (we
only disable after receiving an interrupt for which no one was wanting),
but allowing guc to explicitly manage the irq in relation to itself is
simpler and prevents an issue with losing an interrupt for preemption
as it is not coupled to an active request.

Internally, we add a reference counter (breadcrumbs.irq_users) as a
simple mechanism to allow GuC to keep the breadcrumb irq enabled. It
also means that the missed-breadcrumb timer is permanently active. To
improve, we need to hook into the parking facility of intel_engines so
that we only enable the breadcrumbs while the GT is active (one step
better would be to individually park/unpark each engine).

Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
Cc: MichaƂ Winiarski <michal.winiarski at intel.com>,
Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
---
 drivers/gpu/drm/i915/i915_guc_submission.c | 15 ++++++++++
 drivers/gpu/drm/i915/i915_irq.c            |  3 +-
 drivers/gpu/drm/i915/intel_breadcrumbs.c   | 44 +++++++++++++++++++++++-------
 drivers/gpu/drm/i915/intel_ringbuffer.h    |  5 +++-
 4 files changed, 55 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index a2e8114b739d..2c27ec11ef65 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -1094,6 +1094,16 @@ static void guc_interrupts_release(struct drm_i915_private *dev_priv)
 	rps->pm_intrmsk_mbz &= ~ARAT_EXPIRED_INTRMSK;
 }
 
+static void i915_guc_submission_park(struct intel_engine_cs *engine)
+{
+	intel_engine_unpin_breadcrumbs_irq(engine);
+}
+
+static void i915_guc_submission_unpark(struct intel_engine_cs *engine)
+{
+	intel_engine_pin_breadcrumbs_irq(engine);
+}
+
 int i915_guc_submission_enable(struct drm_i915_private *dev_priv)
 {
 	struct intel_guc *guc = &dev_priv->guc;
@@ -1151,6 +1161,9 @@ int i915_guc_submission_enable(struct drm_i915_private *dev_priv)
 		execlists->irq_tasklet.func = i915_guc_irq_handler;
 		clear_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted);
 		tasklet_schedule(&execlists->irq_tasklet);
+
+		engine->park = i915_guc_submission_park;
+		engine->unpark = i915_guc_submission_unpark;
 	}
 
 	return 0;
@@ -1165,6 +1178,8 @@ void i915_guc_submission_disable(struct drm_i915_private *dev_priv)
 {
 	struct intel_guc *guc = &dev_priv->guc;
 
+	GEM_BUG_ON(dev_priv->gt.awake); /* GT should be parked first */
+
 	guc_interrupts_release(dev_priv);
 
 	/* Revert back to manual ELSP submission */
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index b1296a55c1e4..330bd3a3b46e 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1101,7 +1101,8 @@ static void notify_ring(struct intel_engine_cs *engine)
 		if (wakeup)
 			wake_up_process(wait->tsk);
 	} else {
-		__intel_engine_disarm_breadcrumbs(engine);
+		if (engine->breadcrumbs.irq_armed)
+			__intel_engine_disarm_breadcrumbs(engine);
 	}
 	spin_unlock(&engine->breadcrumbs.irq_lock);
 
diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c b/drivers/gpu/drm/i915/intel_breadcrumbs.c
index 48e1ba01ccf8..51cfd4276dd3 100644
--- a/drivers/gpu/drm/i915/intel_breadcrumbs.c
+++ b/drivers/gpu/drm/i915/intel_breadcrumbs.c
@@ -123,7 +123,7 @@ static void intel_breadcrumbs_fake_irq(struct timer_list *t)
 	 */
 
 	spin_lock_irq(&b->irq_lock);
-	if (!__intel_breadcrumbs_wakeup(b))
+	if (b->irq_armed && !__intel_breadcrumbs_wakeup(b))
 		__intel_engine_disarm_breadcrumbs(engine);
 	spin_unlock_irq(&b->irq_lock);
 	if (!b->irq_armed)
@@ -171,15 +171,36 @@ void __intel_engine_disarm_breadcrumbs(struct intel_engine_cs *engine)
 
 	lockdep_assert_held(&b->irq_lock);
 	GEM_BUG_ON(b->irq_wait);
+	GEM_BUG_ON(!b->irq_armed);
 
-	if (b->irq_enabled) {
+	GEM_BUG_ON(!b->irq_enabled);
+	if (!--b->irq_enabled)
 		irq_disable(engine);
-		b->irq_enabled = false;
-	}
 
 	b->irq_armed = false;
 }
 
+void intel_engine_pin_breadcrumbs_irq(struct intel_engine_cs *engine)
+{
+	struct intel_breadcrumbs *b = &engine->breadcrumbs;
+
+	spin_lock_irq(&b->irq_lock);
+	if (!b->irq_enabled++)
+		irq_enable(engine);
+	spin_unlock_irq(&b->irq_lock);
+}
+
+void intel_engine_unpin_breadcrumbs_irq(struct intel_engine_cs *engine)
+{
+	struct intel_breadcrumbs *b = &engine->breadcrumbs;
+
+	spin_lock_irq(&b->irq_lock);
+	GEM_BUG_ON(!b->irq_enabled);
+	if (!--b->irq_enabled)
+		irq_disable(engine);
+	spin_unlock_irq(&b->irq_lock);
+}
+
 void intel_engine_disarm_breadcrumbs(struct intel_engine_cs *engine)
 {
 	struct intel_breadcrumbs *b = &engine->breadcrumbs;
@@ -241,6 +262,9 @@ static bool __intel_breadcrumbs_enable_irq(struct intel_breadcrumbs *b)
 	struct intel_engine_cs *engine =
 		container_of(b, struct intel_engine_cs, breadcrumbs);
 	struct drm_i915_private *i915 = engine->i915;
+	bool enabled;
+
+	GEM_BUG_ON(!intel_irqs_enabled(i915));
 
 	lockdep_assert_held(&b->irq_lock);
 	if (b->irq_armed)
@@ -252,7 +276,6 @@ static bool __intel_breadcrumbs_enable_irq(struct intel_breadcrumbs *b)
 	 * the irq.
 	 */
 	b->irq_armed = true;
-	GEM_BUG_ON(b->irq_enabled);
 
 	if (I915_SELFTEST_ONLY(b->mock)) {
 		/* For our mock objects we want to avoid interaction
@@ -273,14 +296,15 @@ static bool __intel_breadcrumbs_enable_irq(struct intel_breadcrumbs *b)
 	 */
 
 	/* No interrupts? Kick the waiter every jiffie! */
-	if (intel_irqs_enabled(i915)) {
-		if (!test_bit(engine->id, &i915->gpu_error.test_irq_rings))
-			irq_enable(engine);
-		b->irq_enabled = true;
+	enabled = false;
+	if (!b->irq_enabled++ &&
+	    !test_bit(engine->id, &i915->gpu_error.test_irq_rings)) {
+		irq_enable(engine);
+		enabled = true;
 	}
 
 	enable_fake_irq(b);
-	return true;
+	return enabled;
 }
 
 static inline struct intel_wait *to_wait(struct rb_node *node)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 240814b60a65..2f7b1a86f374 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -331,9 +331,9 @@ struct intel_engine_cs {
 		struct timer_list hangcheck; /* detect missed interrupts */
 
 		unsigned int hangcheck_interrupts;
+		unsigned int irq_enabled;
 
 		bool irq_armed : 1;
-		bool irq_enabled : 1;
 		I915_SELFTEST_DECLARE(bool mock : 1);
 	} breadcrumbs;
 
@@ -818,6 +818,9 @@ unsigned int intel_engine_wakeup(struct intel_engine_cs *engine);
 #define ENGINE_WAKEUP_WAITER BIT(0)
 #define ENGINE_WAKEUP_ASLEEP BIT(1)
 
+void intel_engine_pin_breadcrumbs_irq(struct intel_engine_cs *engine);
+void intel_engine_unpin_breadcrumbs_irq(struct intel_engine_cs *engine);
+
 void __intel_engine_disarm_breadcrumbs(struct intel_engine_cs *engine);
 void intel_engine_disarm_breadcrumbs(struct intel_engine_cs *engine);
 
-- 
2.15.0.rc1



More information about the Intel-gfx mailing list