[PATCH 2/2] drm/i915: Initialize pm_intr_keep during intel_irq_init for GuC

Sagar Arun Kamble sagar.a.kamble at intel.com
Thu Jan 26 16:42:38 UTC 2017


From: "sagar.a.kamble at intel.com" <sagar.a.kamble at intel.com>

Driver needs to ensure that it doesn't mask the PM interrupts, which are
unmasked/needed by GuC firmware. For that, Driver maintains a bitmask of
interrupts to be kept unmasked, pm_intr_keep.

pm_intr_keep was determined across GuC load. GuC gets loaded in different
scenarios and it is not going to change the pm_intr_keep so this patch
moves its setup to intel_irq_init.

This patch fixes incorrect RPS masking and solves bunch of SKL GT3
performance regressions as Host RPS was taking frequency down when GuC is
enabled. This was happening due to signed last_adj that becomes negative
after 32 UP interrupts and makes cur_freq=min starting the ramp again.
Next patch in the series avoids such frequency transitions.

Cc: Szwichtenberg, Radoslaw <radoslaw.szwichtenberg at intel.com>
Cc: Tomi Sarvela <tomi.p.sarvela at intel.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97017
Signed-off-by: Sagar Arun Kamble <sagar.a.kamble at intel.com>
Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_irq.c         | 24 ++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_reg.h         |  3 ++-
 drivers/gpu/drm/i915/intel_guc_loader.c | 26 --------------------------
 3 files changed, 26 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 6fefc34..eb972fc 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -4199,6 +4199,30 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
 	if (INTEL_INFO(dev_priv)->gen >= 8)
 		dev_priv->rps.pm_intr_keep |= GEN8_PMINTR_REDIRECT_TO_GUC;
 
+	/*
+	 * The REDIRECT_TO_GUC bit of the PMINTRMSK register directs all
+	 * (unmasked) PM interrupts to the GuC. All other bits of this
+	 * register *disable* generation of a specific interrupt.
+	 *
+	 * 'pm_intr_keep' indicates bits that are NOT to be set when
+	 * writing to the PM interrupt mask register, i.e. interrupts
+	 * that must not be disabled.
+	 *
+	 * If the GuC is handling these interrupts, then we must not let
+	 * the PM code disable ANY interrupt that the GuC is expecting.
+	 * So for each ENABLED (0) bit in this register, we must SET the
+	 * bit in pm_intr_keep so that it's left enabled for the GuC.
+	 * GuC needs ARAT expired interrupt unmasked hence it is set in
+	 * pm_intr_keep.
+	 *
+	 * Here we CLEAR REDIRECT_TO_GUC bit in pm_intr_keep, which will
+	 * result in the register bit being left SET!
+	 */
+	if (HAS_GUC_SCHED(dev_priv)) {
+		dev_priv->rps.pm_intr_keep |= ARAT_EXPIRED_INTRMSK;
+		dev_priv->rps.pm_intr_keep &= ~GEN8_PMINTR_REDIRECT_TO_GUC;
+	}
+
 	if (IS_GEN2(dev_priv)) {
 		/* Gen2 doesn't have a hardware frame counter */
 		dev->max_vblank_count = 0;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 72f9f36..822c147 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7395,7 +7395,8 @@ enum {
 #define VLV_RCEDATA				_MMIO(0xA0BC)
 #define GEN6_RC6pp_THRESHOLD			_MMIO(0xA0C0)
 #define GEN6_PMINTRMSK				_MMIO(0xA168)
-#define   GEN8_PMINTR_REDIRECT_TO_GUC		  (1<<31)
+#define   GEN8_PMINTR_REDIRECT_TO_GUC		(1<<31)
+#define   ARAT_EXPIRED_INTRMSK			(1<<9)
 #define GEN8_MISC_CTRL0				_MMIO(0xA180)
 #define VLV_PWRDWNUPCTL				_MMIO(0xA294)
 #define GEN9_MEDIA_PG_IDLE_HYSTERESIS		_MMIO(0xA0C4)
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
index 2f1cf9a..0eaa8e4 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -114,7 +114,6 @@ static void guc_interrupts_capture(struct drm_i915_private *dev_priv)
 	struct intel_engine_cs *engine;
 	enum intel_engine_id id;
 	int irqs;
-	u32 tmp;
 
 	/* tell all command streamers to forward interrupts (but not vblank) to GuC */
 	irqs = _MASKED_BIT_ENABLE(GFX_INTERRUPT_STEERING);
@@ -128,31 +127,6 @@ static void guc_interrupts_capture(struct drm_i915_private *dev_priv)
 	I915_WRITE(GUC_BCS_RCS_IER, ~irqs);
 	I915_WRITE(GUC_VCS2_VCS1_IER, ~irqs);
 	I915_WRITE(GUC_WD_VECS_IER, ~irqs);
-
-	/*
-	 * The REDIRECT_TO_GUC bit of the PMINTRMSK register directs all
-	 * (unmasked) PM interrupts to the GuC. All other bits of this
-	 * register *disable* generation of a specific interrupt.
-	 *
-	 * 'pm_intr_keep' indicates bits that are NOT to be set when
-	 * writing to the PM interrupt mask register, i.e. interrupts
-	 * that must not be disabled.
-	 *
-	 * If the GuC is handling these interrupts, then we must not let
-	 * the PM code disable ANY interrupt that the GuC is expecting.
-	 * So for each ENABLED (0) bit in this register, we must SET the
-	 * bit in pm_intr_keep so that it's left enabled for the GuC.
-	 *
-	 * OTOH the REDIRECT_TO_GUC bit is initially SET in pm_intr_keep
-	 * (so interrupts go to the DISPLAY unit at first); but here we
-	 * need to CLEAR that bit, which will result in the register bit
-	 * being left SET!
-	 */
-	tmp = I915_READ(GEN6_PMINTRMSK);
-	if (tmp & GEN8_PMINTR_REDIRECT_TO_GUC) {
-		dev_priv->rps.pm_intr_keep |= ~tmp;
-		dev_priv->rps.pm_intr_keep &= ~GEN8_PMINTR_REDIRECT_TO_GUC;
-	}
 }
 
 static u32 get_gttype(struct drm_i915_private *dev_priv)
-- 
1.9.1



More information about the Intel-gfx-trybot mailing list