[Intel-gfx] [PATCH] TGL HAX drm/i915/tgl: Interrupts are overrated

Chris Wilson chris at chris-wilson.co.uk
Fri Oct 4 07:36:37 UTC 2019


Why sleep when you can busywait for an interrupt? Throw out the old irq
handlers, and use irq_poll instead.

References: https://bugs.freedesktop.org/show_bug.cgi?id=111880
Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala at linux.intel.com>
---
 drivers/gpu/drm/i915/Kconfig                 |  1 +
 drivers/gpu/drm/i915/Kconfig.debug           |  5 +++
 drivers/gpu/drm/i915/gt/intel_engine_types.h |  3 ++
 drivers/gpu/drm/i915/gt/intel_gt_irq.c       | 13 ++++--
 drivers/gpu/drm/i915/gt/intel_lrc.c          | 42 ++++++++++++++++++++
 drivers/gpu/drm/i915/i915_drv.h              |  3 ++
 drivers/gpu/drm/i915/i915_irq.c              | 14 ++++++-
 drivers/gpu/drm/i915/i915_pci.c              |  1 -
 include/linux/irq_poll.h                     | 15 ++++++-
 lib/irq_poll.c                               | 13 +++---
 10 files changed, 95 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
index 0d21402945ab..bf2b27b6ebf2 100644
--- a/drivers/gpu/drm/i915/Kconfig
+++ b/drivers/gpu/drm/i915/Kconfig
@@ -13,6 +13,7 @@ config DRM_I915
 	select DRM_PANEL
 	select DRM_MIPI_DSI
 	select RELAY
+	select IRQ_POLL
 	select IRQ_WORK
 	# i915 depends on ACPI_VIDEO when ACPI is enabled
 	# but for select to work, need to select ACPI_VIDEO's dependencies, ick
diff --git a/drivers/gpu/drm/i915/Kconfig.debug b/drivers/gpu/drm/i915/Kconfig.debug
index 4a800faa275c..4d23a84ac2c4 100644
--- a/drivers/gpu/drm/i915/Kconfig.debug
+++ b/drivers/gpu/drm/i915/Kconfig.debug
@@ -44,6 +44,7 @@ config DRM_I915_DEBUG
 	select DRM_I915_SELFTEST
 	select DRM_I915_DEBUG_RUNTIME_PM
 	select DRM_I915_DEBUG_MMIO
+	select TIGERLAKE_DEBUG_IRQ
         default n
         help
           Choose this option to turn on extra driver debugging that may affect
@@ -220,3 +221,7 @@ config DRM_I915_DEBUG_RUNTIME_PM
 	  driver loading, suspend and resume operations.
 
 	  If in doubt, say "N"
+
+config TIGERLAKE_DEBUG_IRQ
+	bool "[TGL] Reduce IRQ functionality for stability"
+	default n
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
index 943f0663837e..53265add81ec 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
@@ -8,6 +8,7 @@
 #define __INTEL_ENGINE_TYPES__
 
 #include <linux/hashtable.h>
+#include <linux/irq_poll.h>
 #include <linux/irq_work.h>
 #include <linux/kref.h>
 #include <linux/list.h>
@@ -330,6 +331,8 @@ struct intel_engine_cs {
 	struct drm_i915_gem_object *default_state;
 	void *pinned_default_state;
 
+	struct irq_poll irq_poll;
+
 	struct {
 		struct intel_ring *ring;
 		struct intel_timeline *timeline;
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_irq.c b/drivers/gpu/drm/i915/gt/intel_gt_irq.c
index 34a4fb624bf7..7d4c2f18576e 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_irq.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_irq.c
@@ -45,6 +45,9 @@ gen11_gt_engine_identity(struct intel_gt *gt,
 
 	lockdep_assert_held(&gt->irq_lock);
 
+	if (WARN_ON(TIGERLAKE_NO_IRQ(gt->i915)))
+		return 0;
+
 	raw_reg_write(regs, GEN11_IIR_REG_SELECTOR(bank), BIT(bit));
 
 	/*
@@ -209,12 +212,14 @@ void gen11_gt_irq_reset(struct intel_gt *gt)
 
 void gen11_gt_irq_postinstall(struct intel_gt *gt)
 {
-	const u32 irqs = GT_RENDER_USER_INTERRUPT | GT_CONTEXT_SWITCH_INTERRUPT;
 	struct intel_uncore *uncore = gt->uncore;
-	const u32 dmask = irqs << 16 | irqs;
-	const u32 smask = irqs << 16;
+	u32 irqs, dmask, smask;
 
-	BUILD_BUG_ON(irqs & 0xffff0000);
+	irqs = GT_RENDER_USER_INTERRUPT | GT_CONTEXT_SWITCH_INTERRUPT;
+	if (TIGERLAKE_NO_IRQ(gt->i915))
+		irqs = 0; /* XXX lalalala */
+	smask = irqs << 16;
+	dmask = smask | irqs;
 
 	/* Enable RCS, BCS, VCS and VECS class interrupts. */
 	intel_uncore_write(uncore, GEN11_RENDER_COPY_INTR_ENABLE, dmask);
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index 431d3b8c3371..de4c494d775e 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -989,6 +989,9 @@ __execlists_schedule_in(struct i915_request *rq)
 	execlists_context_status_change(rq, INTEL_CONTEXT_SCHEDULE_IN);
 	intel_engine_context_in(engine);
 
+	if (TIGERLAKE_NO_IRQ(engine->i915))
+		irq_poll_activate(&engine->irq_poll);
+
 	return engine;
 }
 
@@ -1028,6 +1031,9 @@ __execlists_schedule_out(struct i915_request *rq,
 {
 	struct intel_context * const ce = rq->hw_context;
 
+	if (TIGERLAKE_NO_IRQ(engine->i915))
+		irq_poll_deactivate(&engine->irq_poll);
+
 	intel_engine_context_out(engine);
 	execlists_context_status_change(rq, INTEL_CONTEXT_SCHEDULE_OUT);
 	intel_gt_pm_put(engine->gt);
@@ -1196,6 +1202,8 @@ static void execlists_submit_ports(struct intel_engine_cs *engine)
 	/* we need to manually load the submit queue */
 	if (execlists->ctrl_reg)
 		writel(EL_CTRL_LOAD, execlists->ctrl_reg);
+
+	irq_poll_sched(&engine->irq_poll);
 }
 
 static bool ctx_single_port_submission(const struct intel_context *ce)
@@ -1838,6 +1846,11 @@ gen8_csb_parse(const struct intel_engine_execlists *execlists, const u32 *csb)
 	return *csb & (GEN8_CTX_STATUS_IDLE_ACTIVE | GEN8_CTX_STATUS_PREEMPTED);
 }
 
+static inline bool has_breadcrumbs(struct i915_request *rq)
+{
+	return !list_empty(&rq->hw_context->signals);
+}
+
 static void process_csb(struct intel_engine_cs *engine)
 {
 	struct intel_engine_execlists * const execlists = &engine->execlists;
@@ -1940,6 +1953,10 @@ static void process_csb(struct intel_engine_cs *engine)
 			 */
 			GEM_BUG_ON(!i915_request_completed(*execlists->active) &&
 				   !reset_in_progress(execlists));
+
+			if (has_breadcrumbs(*execlists->active))
+				intel_engine_queue_breadcrumbs(engine);
+
 			execlists_schedule_out(*execlists->active++);
 
 			GEM_BUG_ON(execlists->active - execlists->inflight >
@@ -1987,6 +2004,27 @@ static void execlists_submission_tasklet(unsigned long data)
 	}
 }
 
+static int iop_handler(struct irq_poll *iop, int budget)
+{
+	struct intel_engine_cs *engine =
+		container_of(iop, typeof(*engine), irq_poll);
+	struct intel_engine_execlists *execlists = &engine->execlists;
+	struct tasklet_struct *t = &execlists->tasklet;
+
+	if (execlists->csb_head == READ_ONCE(*execlists->csb_write))
+		return 0;
+
+	if (!tasklet_trylock(t))
+		return 0;
+
+	/* Must wait for any GPU reset in progress. */
+	if (__tasklet_is_enabled(t))
+		t->func(t->data);
+
+	tasklet_unlock(t);
+	return 0;
+}
+
 static void execlists_submission_timer(struct timer_list *timer)
 {
 	struct intel_engine_cs *engine =
@@ -3446,6 +3484,7 @@ void intel_execlists_set_default_submission(struct intel_engine_cs *engine)
 
 static void execlists_destroy(struct intel_engine_cs *engine)
 {
+	irq_poll_disable(&engine->irq_poll);
 	intel_engine_cleanup_common(engine);
 	lrc_destroy_wa_ctx(engine);
 	kfree(engine);
@@ -3532,6 +3571,9 @@ static void rcs_submission_override(struct intel_engine_cs *engine)
 
 int intel_execlists_submission_setup(struct intel_engine_cs *engine)
 {
+	irq_poll_init(&engine->irq_poll, -1, iop_handler);
+	irq_poll_disable(&engine->irq_poll);
+
 	tasklet_init(&engine->execlists.tasklet,
 		     execlists_submission_tasklet, (unsigned long)engine);
 	timer_setup(&engine->execlists.timer, execlists_submission_timer, 0);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 337d8306416a..c505d9e38e98 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2069,6 +2069,9 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
 #define IS_GEN9_LP(dev_priv)	(IS_GEN(dev_priv, 9) && IS_LP(dev_priv))
 #define IS_GEN9_BC(dev_priv)	(IS_GEN(dev_priv, 9) && !IS_LP(dev_priv))
 
+#define TIGERLAKE_NO_IRQ(i915) \
+	IS_ENABLED(CONFIG_TIGERLAKE_DEBUG_IRQ) && IS_TIGERLAKE(i915)
+
 #define HAS_ENGINE(dev_priv, id) (INTEL_INFO(dev_priv)->engine_mask & BIT(id))
 
 #define ENGINE_INSTANCES_MASK(dev_priv, first, count) ({		\
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index bc83f094065a..05060f829981 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -360,6 +360,9 @@ void gen6_enable_rps_interrupts(struct drm_i915_private *dev_priv)
 	if (READ_ONCE(rps->interrupts_enabled))
 		return;
 
+	if (!dev_priv->pm_rps_events)
+		return;
+
 	spin_lock_irq(&gt->irq_lock);
 	WARN_ON_ONCE(rps->pm_iir);
 
@@ -376,7 +379,12 @@ void gen6_enable_rps_interrupts(struct drm_i915_private *dev_priv)
 
 u32 gen6_sanitize_rps_pm_mask(const struct drm_i915_private *i915, u32 mask)
 {
-	return mask & ~i915->gt_pm.rps.pm_intrmsk_mbz;
+	const struct intel_rps *rps = &i915->gt_pm.rps;
+
+	if (!rps->interrupts_enabled)
+		return ~0u;
+
+	return mask & ~rps->pm_intrmsk_mbz;
 }
 
 void gen6_disable_rps_interrupts(struct drm_i915_private *dev_priv)
@@ -4314,7 +4322,9 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
 		dev_priv->gt.pm_guc_events = GUC_INTR_GUC2HOST << 16;
 
 	/* Let's track the enabled rps events */
-	if (IS_VALLEYVIEW(dev_priv))
+	if (TIGERLAKE_NO_IRQ(dev_priv))
+		dev_priv->pm_rps_events = 0;
+	else if (IS_VALLEYVIEW(dev_priv))
 		/* WaGsvRC0ResidencyMethod:vlv */
 		dev_priv->pm_rps_events = GEN6_PM_RP_UP_EI_EXPIRED;
 	else
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index 1cbf3998b361..86be10cc6a70 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -806,7 +806,6 @@ static const struct intel_device_info intel_tigerlake_12_info = {
 	.display.has_modular_fia = 1,
 	.engine_mask =
 		BIT(RCS0) | BIT(BCS0) | BIT(VECS0) | BIT(VCS0) | BIT(VCS2),
-	.has_rps = false, /* XXX disabled for debugging */
 };
 
 #undef GEN
diff --git a/include/linux/irq_poll.h b/include/linux/irq_poll.h
index 16aaeccb65cb..d66d0e0e9379 100644
--- a/include/linux/irq_poll.h
+++ b/include/linux/irq_poll.h
@@ -2,19 +2,22 @@
 #ifndef IRQ_POLL_H
 #define IRQ_POLL_H
 
+#include <linux/atomic.h>
+#include <linux/list.h>
+
 struct irq_poll;
 typedef int (irq_poll_fn)(struct irq_poll *, int);
 
 struct irq_poll {
 	struct list_head list;
 	unsigned long state;
+	atomic_t count;
 	int weight;
 	irq_poll_fn *poll;
 };
 
 enum {
 	IRQ_POLL_F_SCHED	= 0,
-	IRQ_POLL_F_DISABLE	= 1,
 };
 
 extern void irq_poll_sched(struct irq_poll *);
@@ -23,4 +26,14 @@ extern void irq_poll_complete(struct irq_poll *);
 extern void irq_poll_enable(struct irq_poll *);
 extern void irq_poll_disable(struct irq_poll *);
 
+static inline void irq_poll_activate(struct irq_poll *iop)
+{
+	atomic_inc(&iop->count);
+}
+
+static inline void irq_poll_deactivate(struct irq_poll *iop)
+{
+	atomic_dec(&iop->count);
+}
+
 #endif
diff --git a/lib/irq_poll.c b/lib/irq_poll.c
index 2f17b488d58e..59408fa75fa6 100644
--- a/lib/irq_poll.c
+++ b/lib/irq_poll.c
@@ -28,8 +28,9 @@ void irq_poll_sched(struct irq_poll *iop)
 {
 	unsigned long flags;
 
-	if (test_bit(IRQ_POLL_F_DISABLE, &iop->state))
+	if (!atomic_read(&iop->count))
 		return;
+
 	if (test_and_set_bit(IRQ_POLL_F_SCHED, &iop->state))
 		return;
 
@@ -122,7 +123,7 @@ static void __latent_entropy irq_poll_softirq(struct softirq_action *h)
 		 * move the instance around on the list at-will.
 		 */
 		if (work >= weight) {
-			if (test_bit(IRQ_POLL_F_DISABLE, &iop->state))
+			if (!atomic_read(&iop->count))
 				__irq_poll_complete(iop);
 			else
 				list_move_tail(&iop->list, list);
@@ -144,10 +145,9 @@ static void __latent_entropy irq_poll_softirq(struct softirq_action *h)
  **/
 void irq_poll_disable(struct irq_poll *iop)
 {
-	set_bit(IRQ_POLL_F_DISABLE, &iop->state);
+	atomic_set_release(&iop->count, 0);
 	while (test_and_set_bit(IRQ_POLL_F_SCHED, &iop->state))
 		msleep(1);
-	clear_bit(IRQ_POLL_F_DISABLE, &iop->state);
 }
 EXPORT_SYMBOL(irq_poll_disable);
 
@@ -161,9 +161,7 @@ EXPORT_SYMBOL(irq_poll_disable);
  **/
 void irq_poll_enable(struct irq_poll *iop)
 {
-	BUG_ON(!test_bit(IRQ_POLL_F_SCHED, &iop->state));
-	smp_mb__before_atomic();
-	clear_bit_unlock(IRQ_POLL_F_SCHED, &iop->state);
+	irq_poll_activate(iop);
 }
 EXPORT_SYMBOL(irq_poll_enable);
 
@@ -182,6 +180,7 @@ void irq_poll_init(struct irq_poll *iop, int weight, irq_poll_fn *poll_fn)
 	INIT_LIST_HEAD(&iop->list);
 	iop->weight = weight;
 	iop->poll = poll_fn;
+	atomic_set(&iop->count, 1);
 }
 EXPORT_SYMBOL(irq_poll_init);
 
-- 
2.23.0



More information about the Intel-gfx mailing list