[PATCH 00/17] drm/i915: Redo old gmch irq handling

[PATCH 00/17] drm/i915: Redo old gmch irq handling

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Apparently we have some issues [1] on g4x which smells like irqs not getting
delivered after some point in time. The gen2-4 irq code is rather crusty
so I thought I'd bring it up to the same quality standards as the VLV/CHV
irq code. And to avoid any chances of missing the edges I changed all the
gmch platforms to use the "disable IER -> ack IIR -> enable IER" trick
we use on VLV and CHV. That should be robust with both level and edge
triggered interrupts, and single and double buffered IIR.

I think the only slightly scary bits are the ones touching HWSTAM
programming. While that's not strictly needed for this series, I really
wanted to remove a bunch of duplicat irq setup code, and for that
I wanted to make the HWSTAM programming consistent. We don't actually
use any of the interrupt information written into the status page,
but I have slight concern that the extra status page writes may have
had some unintended effect on seqno coherency. Fingers crossed...

There's potentially more unification we could do on the various gmch
interrupts functions, but as the series was already ballooning out of
control I decided not to pursue that angle very far.

I smoke tested this on 830, pnv, g4x, and ilk.

Series is available here:
git://github.com/vsyrjala/linux.git gmch_irq_redo

Ville Syrjälä (17):
  drm/i915: Clear pipestat consistently
  drm/i915: s/GEN3/GEN5/
  drm/i915: Use GEN3_IRQ_RESET/INIT on gen3/4
  drm/i915: Introduce GEN2_IRQ_RESET/INIT
  drm/i915: Setup EMR first on all gen2-4
  drm/i915: Eliminate PORT_HOTPLUG_EN setup from gen3/4 irq_postinstall
  drm/i915: Unify the appearance of gen3/4 irq_postistall hooks
  drm/i915: Remove NULL dev_priv checks from irq_uninstall
  drm/i915: Mask everything in ring HWSTAM on gen6+ in ringbuffer mode
  drm/i915: Gen3 HWSTAM is actually 32 bits
  drm/i915: Clean up the HWSTAM mess
  drm/i915: Remove duplicated irq_preinstall/uninstall hooks
  drm/i915: Consolidatte intel_check_page_flip() into
    intel_pipe_handle_vblank()
  drm/i915: Move the gen2-4 page flip handling code around
  drm/i915: Simplify the gen2-4 flip_mask handling
  drm/i915: Extract PIPESTAT irq handling into separate functions
  drm/i915: Rewrite GMCH irq handlers to follow the VLV/CHV pattern

 drivers/gpu/drm/i915/i915_irq.c         | 909 ++++++++++++++------------------
 drivers/gpu/drm/i915/intel_ringbuffer.c |   3 +
 2 files changed, 394 insertions(+), 518 deletions(-)

-- 
2.13.0

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

[PATCH 01/17] drm/i915: Clear pipestat consistently

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

We have a lot of different ways of clearing the PIPESTAT registers.
Let's unify it all into one function. There's no magic in PIPESTAT
that would require any of the double clearing and whatnot that
some of the code tries to do. All we can really do is clear the status
bits and disable the enable bits. There is no way to mask anything
so as soon as another event happens the status bit will become set
again, and trying to clear them twice or something can't protect
against that.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 67 ++++++++++++++++++-----------------------
 1 file changed, 30 insertions(+), 37 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index b1c7d1a04612..6daaf47482d4 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1732,6 +1732,19 @@ static bool intel_pipe_handle_vblank(struct drm_i915_private *dev_priv,
 	return ret;
 }
 
+static void i9xx_pipestat_irq_reset(struct drm_i915_private *dev_priv)
+{
+	enum pipe pipe;
+
+	for_each_pipe(dev_priv, pipe) {
+		I915_WRITE(PIPESTAT(pipe),
+			   PIPESTAT_INT_STATUS_MASK |
+			   PIPE_FIFO_UNDERRUN_STATUS);
+
+		dev_priv->pipestat_irq_mask[pipe] = 0;
+	}
+}
+
 static void valleyview_pipestat_irq_ack(struct drm_i915_private *dev_priv,
 					u32 iir, u32 pipe_stats[I915_MAX_PIPES])
 {
@@ -2931,8 +2944,6 @@ static void gen5_gt_irq_reset(struct drm_i915_private *dev_priv)
 
 static void vlv_display_irq_reset(struct drm_i915_private *dev_priv)
 {
-	enum pipe pipe;
-
 	if (IS_CHERRYVIEW(dev_priv))
 		I915_WRITE(DPINVGTT, DPINVGTT_STATUS_MASK_CHV);
 	else
@@ -2941,12 +2952,7 @@ static void vlv_display_irq_reset(struct drm_i915_private *dev_priv)
 	i915_hotplug_interrupt_update_locked(dev_priv, 0xffffffff, 0);
 	I915_WRITE(PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT));
 
-	for_each_pipe(dev_priv, pipe) {
-		I915_WRITE(PIPESTAT(pipe),
-			   PIPE_FIFO_UNDERRUN_STATUS |
-			   PIPESTAT_INT_STATUS_MASK);
-		dev_priv->pipestat_irq_mask[pipe] = 0;
-	}
+	i9xx_pipestat_irq_reset(dev_priv);
 
 	GEN5_IRQ_RESET(VLV_);
 	dev_priv->irq_mask = ~0;
@@ -3604,10 +3610,9 @@ static void ironlake_irq_uninstall(struct drm_device *dev)
 static void i8xx_irq_preinstall(struct drm_device * dev)
 {
 	struct drm_i915_private *dev_priv = to_i915(dev);
-	int pipe;
 
-	for_each_pipe(dev_priv, pipe)
-		I915_WRITE(PIPESTAT(pipe), 0);
+	i9xx_pipestat_irq_reset(dev_priv);
+
 	I915_WRITE16(IMR, 0xffff);
 	I915_WRITE16(IER, 0x0);
 	POSTING_READ16(IER);
@@ -3756,13 +3761,9 @@ static irqreturn_t i8xx_irq_handler(int irq, void *arg)
 static void i8xx_irq_uninstall(struct drm_device * dev)
 {
 	struct drm_i915_private *dev_priv = to_i915(dev);
-	int pipe;
 
-	for_each_pipe(dev_priv, pipe) {
-		/* Clear enable bits; then clear status bits */
-		I915_WRITE(PIPESTAT(pipe), 0);
-		I915_WRITE(PIPESTAT(pipe), I915_READ(PIPESTAT(pipe)));
-	}
+	i9xx_pipestat_irq_reset(dev_priv);
+
 	I915_WRITE16(IMR, 0xffff);
 	I915_WRITE16(IER, 0x0);
 	I915_WRITE16(IIR, I915_READ16(IIR));
@@ -3771,16 +3772,16 @@ static void i8xx_irq_uninstall(struct drm_device * dev)
 static void i915_irq_preinstall(struct drm_device * dev)
 {
 	struct drm_i915_private *dev_priv = to_i915(dev);
-	int pipe;
 
 	if (I915_HAS_HOTPLUG(dev_priv)) {
 		i915_hotplug_interrupt_update(dev_priv, 0xffffffff, 0);
 		I915_WRITE(PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT));
 	}
 
+	i9xx_pipestat_irq_reset(dev_priv);
+
 	I915_WRITE16(HWSTAM, 0xeffe);
-	for_each_pipe(dev_priv, pipe)
-		I915_WRITE(PIPESTAT(pipe), 0);
+
 	I915_WRITE(IMR, 0xffffffff);
 	I915_WRITE(IER, 0x0);
 	POSTING_READ(IER);
@@ -3973,36 +3974,32 @@ static irqreturn_t i915_irq_handler(int irq, void *arg)
 static void i915_irq_uninstall(struct drm_device * dev)
 {
 	struct drm_i915_private *dev_priv = to_i915(dev);
-	int pipe;
 
 	if (I915_HAS_HOTPLUG(dev_priv)) {
 		i915_hotplug_interrupt_update(dev_priv, 0xffffffff, 0);
 		I915_WRITE(PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT));
 	}
 
+	i9xx_pipestat_irq_reset(dev_priv);
+
 	I915_WRITE16(HWSTAM, 0xffff);
-	for_each_pipe(dev_priv, pipe) {
-		/* Clear enable bits; then clear status bits */
-		I915_WRITE(PIPESTAT(pipe), 0);
-		I915_WRITE(PIPESTAT(pipe), I915_READ(PIPESTAT(pipe)));
-	}
+
 	I915_WRITE(IMR, 0xffffffff);
 	I915_WRITE(IER, 0x0);
-
 	I915_WRITE(IIR, I915_READ(IIR));
 }
 
 static void i965_irq_preinstall(struct drm_device * dev)
 {
 	struct drm_i915_private *dev_priv = to_i915(dev);
-	int pipe;
 
 	i915_hotplug_interrupt_update(dev_priv, 0xffffffff, 0);
 	I915_WRITE(PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT));
 
+	i9xx_pipestat_irq_reset(dev_priv);
+
 	I915_WRITE(HWSTAM, 0xeffe);
-	for_each_pipe(dev_priv, pipe)
-		I915_WRITE(PIPESTAT(pipe), 0);
+
 	I915_WRITE(IMR, 0xffffffff);
 	I915_WRITE(IER, 0x0);
 	POSTING_READ(IER);
@@ -4204,7 +4201,6 @@ static irqreturn_t i965_irq_handler(int irq, void *arg)
 static void i965_irq_uninstall(struct drm_device * dev)
 {
 	struct drm_i915_private *dev_priv = to_i915(dev);
-	int pipe;
 
 	if (!dev_priv)
 		return;
@@ -4212,15 +4208,12 @@ static void i965_irq_uninstall(struct drm_device * dev)
 	i915_hotplug_interrupt_update(dev_priv, 0xffffffff, 0);
 	I915_WRITE(PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT));
 
+	i9xx_pipestat_irq_reset(dev_priv);
+
 	I915_WRITE(HWSTAM, 0xffffffff);
-	for_each_pipe(dev_priv, pipe)
-		I915_WRITE(PIPESTAT(pipe), 0);
+
 	I915_WRITE(IMR, 0xffffffff);
 	I915_WRITE(IER, 0x0);
-
-	for_each_pipe(dev_priv, pipe)
-		I915_WRITE(PIPESTAT(pipe),
-			   I915_READ(PIPESTAT(pipe)) & 0x8000ffff);
 	I915_WRITE(IIR, I915_READ(IIR));
 }
 
-- 
2.13.0

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

[PATCH 02/17] drm/i915: s/GEN3/GEN5/

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The GEN5_IRQ_RESET/INIT macros are perfectly suitable even for
gen3/4 hardware as those have 32 bit interrupt registers. Let's
rename the macros to reflect that fact.

Gen2 on the other hand has 16 bit interrupt registers so these
macros aren't really appropriate there.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 44 ++++++++++++++++++++---------------------
 1 file changed, 22 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 6daaf47482d4..8b6b2fce6d87 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -126,7 +126,7 @@ static const u32 hpd_bxt[HPD_NUM_PINS] = {
 	POSTING_READ(GEN8_##type##_IIR(which)); \
 } while (0)
 
-#define GEN5_IRQ_RESET(type) do { \
+#define GEN3_IRQ_RESET(type) do { \
 	I915_WRITE(type##IMR, 0xffffffff); \
 	POSTING_READ(type##IMR); \
 	I915_WRITE(type##IER, 0); \
@@ -139,7 +139,7 @@ static const u32 hpd_bxt[HPD_NUM_PINS] = {
 /*
  * We should clear IMR at preinstall/uninstall, and just check at postinstall.
  */
-static void gen5_assert_iir_is_zero(struct drm_i915_private *dev_priv,
+static void gen3_assert_iir_is_zero(struct drm_i915_private *dev_priv,
 				    i915_reg_t reg)
 {
 	u32 val = I915_READ(reg);
@@ -156,14 +156,14 @@ static void gen5_assert_iir_is_zero(struct drm_i915_private *dev_priv,
 }
 
 #define GEN8_IRQ_INIT_NDX(type, which, imr_val, ier_val) do { \
-	gen5_assert_iir_is_zero(dev_priv, GEN8_##type##_IIR(which)); \
+	gen3_assert_iir_is_zero(dev_priv, GEN8_##type##_IIR(which)); \
 	I915_WRITE(GEN8_##type##_IER(which), (ier_val)); \
 	I915_WRITE(GEN8_##type##_IMR(which), (imr_val)); \
 	POSTING_READ(GEN8_##type##_IMR(which)); \
 } while (0)
 
-#define GEN5_IRQ_INIT(type, imr_val, ier_val) do { \
-	gen5_assert_iir_is_zero(dev_priv, type##IIR); \
+#define GEN3_IRQ_INIT(type, imr_val, ier_val) do { \
+	gen3_assert_iir_is_zero(dev_priv, type##IIR); \
 	I915_WRITE(type##IER, (ier_val)); \
 	I915_WRITE(type##IMR, (imr_val)); \
 	POSTING_READ(type##IMR); \
@@ -2909,7 +2909,7 @@ static void ibx_irq_reset(struct drm_i915_private *dev_priv)
 	if (HAS_PCH_NOP(dev_priv))
 		return;
 
-	GEN5_IRQ_RESET(SDE);
+	GEN3_IRQ_RESET(SDE);
 
 	if (HAS_PCH_CPT(dev_priv) || HAS_PCH_LPT(dev_priv))
 		I915_WRITE(SERR_INT, 0xffffffff);
@@ -2937,9 +2937,9 @@ static void ibx_irq_pre_postinstall(struct drm_device *dev)
 
 static void gen5_gt_irq_reset(struct drm_i915_private *dev_priv)
 {
-	GEN5_IRQ_RESET(GT);
+	GEN3_IRQ_RESET(GT);
 	if (INTEL_GEN(dev_priv) >= 6)
-		GEN5_IRQ_RESET(GEN6_PM);
+		GEN3_IRQ_RESET(GEN6_PM);
 }
 
 static void vlv_display_irq_reset(struct drm_i915_private *dev_priv)
@@ -2954,7 +2954,7 @@ static void vlv_display_irq_reset(struct drm_i915_private *dev_priv)
 
 	i9xx_pipestat_irq_reset(dev_priv);
 
-	GEN5_IRQ_RESET(VLV_);
+	GEN3_IRQ_RESET(VLV_);
 	dev_priv->irq_mask = ~0;
 }
 
@@ -2985,7 +2985,7 @@ static void vlv_display_irq_postinstall(struct drm_i915_private *dev_priv)
 
 	dev_priv->irq_mask = ~enable_mask;
 
-	GEN5_IRQ_INIT(VLV_, dev_priv->irq_mask, enable_mask);
+	GEN3_IRQ_INIT(VLV_, dev_priv->irq_mask, enable_mask);
 }
 
 /* drm_dma.h hooks
@@ -2996,7 +2996,7 @@ static void ironlake_irq_reset(struct drm_device *dev)
 
 	I915_WRITE(HWSTAM, 0xffffffff);
 
-	GEN5_IRQ_RESET(DE);
+	GEN3_IRQ_RESET(DE);
 	if (IS_GEN7(dev_priv))
 		I915_WRITE(GEN7_ERR_INT, 0xffffffff);
 
@@ -3043,9 +3043,9 @@ static void gen8_irq_reset(struct drm_device *dev)
 						   POWER_DOMAIN_PIPE(pipe)))
 			GEN8_IRQ_RESET_NDX(DE_PIPE, pipe);
 
-	GEN5_IRQ_RESET(GEN8_DE_PORT_);
-	GEN5_IRQ_RESET(GEN8_DE_MISC_);
-	GEN5_IRQ_RESET(GEN8_PCU_);
+	GEN3_IRQ_RESET(GEN8_DE_PORT_);
+	GEN3_IRQ_RESET(GEN8_DE_MISC_);
+	GEN3_IRQ_RESET(GEN8_PCU_);
 
 	if (HAS_PCH_SPLIT(dev_priv))
 		ibx_irq_reset(dev_priv);
@@ -3088,7 +3088,7 @@ static void cherryview_irq_preinstall(struct drm_device *dev)
 
 	gen8_gt_irq_reset(dev_priv);
 
-	GEN5_IRQ_RESET(GEN8_PCU_);
+	GEN3_IRQ_RESET(GEN8_PCU_);
 
 	spin_lock_irq(&dev_priv->irq_lock);
 	if (dev_priv->display_irqs_enabled)
@@ -3283,7 +3283,7 @@ static void ibx_irq_postinstall(struct drm_device *dev)
 	else
 		mask = SDE_GMBUS_CPT | SDE_AUX_MASK_CPT;
 
-	gen5_assert_iir_is_zero(dev_priv, SDEIIR);
+	gen3_assert_iir_is_zero(dev_priv, SDEIIR);
 	I915_WRITE(SDEIMR, ~mask);
 
 	if (HAS_PCH_IBX(dev_priv) || HAS_PCH_CPT(dev_priv) ||
@@ -3314,7 +3314,7 @@ static void gen5_gt_irq_postinstall(struct drm_device *dev)
 		gt_irqs |= GT_BLT_USER_INTERRUPT | GT_BSD_USER_INTERRUPT;
 	}
 
-	GEN5_IRQ_INIT(GT, dev_priv->gt_irq_mask, gt_irqs);
+	GEN3_IRQ_INIT(GT, dev_priv->gt_irq_mask, gt_irqs);
 
 	if (INTEL_GEN(dev_priv) >= 6) {
 		/*
@@ -3327,7 +3327,7 @@ static void gen5_gt_irq_postinstall(struct drm_device *dev)
 		}
 
 		dev_priv->pm_imr = 0xffffffff;
-		GEN5_IRQ_INIT(GEN6_PM, dev_priv->pm_imr, pm_irqs);
+		GEN3_IRQ_INIT(GEN6_PM, dev_priv->pm_imr, pm_irqs);
 	}
 }
 
@@ -3361,7 +3361,7 @@ static int ironlake_irq_postinstall(struct drm_device *dev)
 
 	ibx_irq_pre_postinstall(dev);
 
-	GEN5_IRQ_INIT(DE, dev_priv->irq_mask, display_mask | extra_mask);
+	GEN3_IRQ_INIT(DE, dev_priv->irq_mask, display_mask | extra_mask);
 
 	gen5_gt_irq_postinstall(dev);
 
@@ -3502,8 +3502,8 @@ static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)
 					  dev_priv->de_irq_mask[pipe],
 					  de_pipe_enables);
 
-	GEN5_IRQ_INIT(GEN8_DE_PORT_, ~de_port_masked, de_port_enables);
-	GEN5_IRQ_INIT(GEN8_DE_MISC_, ~de_misc_masked, de_misc_masked);
+	GEN3_IRQ_INIT(GEN8_DE_PORT_, ~de_port_masked, de_port_enables);
+	GEN3_IRQ_INIT(GEN8_DE_MISC_, ~de_misc_masked, de_misc_masked);
 
 	if (IS_GEN9_LP(dev_priv))
 		bxt_hpd_detection_setup(dev_priv);
@@ -3589,7 +3589,7 @@ static void cherryview_irq_uninstall(struct drm_device *dev)
 
 	gen8_gt_irq_reset(dev_priv);
 
-	GEN5_IRQ_RESET(GEN8_PCU_);
+	GEN3_IRQ_RESET(GEN8_PCU_);
 
 	spin_lock_irq(&dev_priv->irq_lock);
 	if (dev_priv->display_irqs_enabled)
-- 
2.13.0

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

[PATCH 03/17] drm/i915: Use GEN3_IRQ_RESET/INIT on gen3/4

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Replace the manual IMR+IER+IIR write sequences with the appropriate
GEN3_IRQ_RESET/INIT macro invocations in gen3/4.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 24 ++++++------------------
 1 file changed, 6 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 8b6b2fce6d87..997c1955c88b 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -3782,9 +3782,7 @@ static void i915_irq_preinstall(struct drm_device * dev)
 
 	I915_WRITE16(HWSTAM, 0xeffe);
 
-	I915_WRITE(IMR, 0xffffffff);
-	I915_WRITE(IER, 0x0);
-	POSTING_READ(IER);
+	GEN3_IRQ_RESET();
 }
 
 static int i915_irq_postinstall(struct drm_device *dev)
@@ -3818,9 +3816,7 @@ static int i915_irq_postinstall(struct drm_device *dev)
 		dev_priv->irq_mask &= ~I915_DISPLAY_PORT_INTERRUPT;
 	}
 
-	I915_WRITE(IMR, dev_priv->irq_mask);
-	I915_WRITE(IER, enable_mask);
-	POSTING_READ(IER);
+	GEN3_IRQ_INIT(, dev_priv->irq_mask, enable_mask);
 
 	i915_enable_asle_pipestat(dev_priv);
 
@@ -3984,9 +3980,7 @@ static void i915_irq_uninstall(struct drm_device * dev)
 
 	I915_WRITE16(HWSTAM, 0xffff);
 
-	I915_WRITE(IMR, 0xffffffff);
-	I915_WRITE(IER, 0x0);
-	I915_WRITE(IIR, I915_READ(IIR));
+	GEN3_IRQ_RESET();
 }
 
 static void i965_irq_preinstall(struct drm_device * dev)
@@ -4000,9 +3994,7 @@ static void i965_irq_preinstall(struct drm_device * dev)
 
 	I915_WRITE(HWSTAM, 0xeffe);
 
-	I915_WRITE(IMR, 0xffffffff);
-	I915_WRITE(IER, 0x0);
-	POSTING_READ(IER);
+	GEN3_IRQ_RESET();
 }
 
 static int i965_irq_postinstall(struct drm_device *dev)
@@ -4051,9 +4043,7 @@ static int i965_irq_postinstall(struct drm_device *dev)
 	}
 	I915_WRITE(EMR, error_mask);
 
-	I915_WRITE(IMR, dev_priv->irq_mask);
-	I915_WRITE(IER, enable_mask);
-	POSTING_READ(IER);
+	GEN3_IRQ_INIT(, dev_priv->irq_mask, enable_mask);
 
 	i915_hotplug_interrupt_update(dev_priv, 0xffffffff, 0);
 	POSTING_READ(PORT_HOTPLUG_EN);
@@ -4212,9 +4202,7 @@ static void i965_irq_uninstall(struct drm_device * dev)
 
 	I915_WRITE(HWSTAM, 0xffffffff);
 
-	I915_WRITE(IMR, 0xffffffff);
-	I915_WRITE(IER, 0x0);
-	I915_WRITE(IIR, I915_READ(IIR));
+	GEN3_IRQ_RESET();
 }
 
 /**
-- 
2.13.0

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

[PATCH 04/17] drm/i915: Introduce GEN2_IRQ_RESET/INIT

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Unify the appearance of the gen2 irq code with the gen3+ code by
introducing the GEN2_IRQ_RESET/INIT macros.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 53 +++++++++++++++++++++++++++++++----------
 1 file changed, 41 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 997c1955c88b..204dac7a5529 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -136,6 +136,16 @@ static const u32 hpd_bxt[HPD_NUM_PINS] = {
 	POSTING_READ(type##IIR); \
 } while (0)
 
+#define GEN2_IRQ_RESET(type) do { \
+	I915_WRITE16(type##IMR, 0xffff); \
+	POSTING_READ16(type##IMR); \
+	I915_WRITE16(type##IER, 0); \
+	I915_WRITE16(type##IIR, 0xffff); \
+	POSTING_READ16(type##IIR); \
+	I915_WRITE16(type##IIR, 0xffff); \
+	POSTING_READ16(type##IIR); \
+} while (0)
+
 /*
  * We should clear IMR at preinstall/uninstall, and just check at postinstall.
  */
@@ -155,6 +165,22 @@ static void gen3_assert_iir_is_zero(struct drm_i915_private *dev_priv,
 	POSTING_READ(reg);
 }
 
+static void gen2_assert_iir_is_zero(struct drm_i915_private *dev_priv,
+				    i915_reg_t reg)
+{
+	u16 val = I915_READ16(reg);
+
+	if (val == 0)
+		return;
+
+	WARN(1, "Interrupt register 0x%x is not zero: 0x%08x\n",
+	     i915_mmio_reg_offset(reg), val);
+	I915_WRITE16(reg, 0xffff);
+	POSTING_READ16(reg);
+	I915_WRITE16(reg, 0xffff);
+	POSTING_READ16(reg);
+}
+
 #define GEN8_IRQ_INIT_NDX(type, which, imr_val, ier_val) do { \
 	gen3_assert_iir_is_zero(dev_priv, GEN8_##type##_IIR(which)); \
 	I915_WRITE(GEN8_##type##_IER(which), (ier_val)); \
@@ -169,6 +195,13 @@ static void gen3_assert_iir_is_zero(struct drm_i915_private *dev_priv,
 	POSTING_READ(type##IMR); \
 } while (0)
 
+#define GEN2_IRQ_INIT(type, imr_val, ier_val) do { \
+	gen2_assert_iir_is_zero(dev_priv, type##IIR); \
+	I915_WRITE16(type##IER, (ier_val)); \
+	I915_WRITE16(type##IMR, (imr_val)); \
+	POSTING_READ16(type##IMR); \
+} while (0)
+
 static void gen6_rps_irq_handler(struct drm_i915_private *dev_priv, u32 pm_iir);
 static void gen9_guc_irq_handler(struct drm_i915_private *dev_priv, u32 pm_iir);
 
@@ -3613,14 +3646,13 @@ static void i8xx_irq_preinstall(struct drm_device * dev)
 
 	i9xx_pipestat_irq_reset(dev_priv);
 
-	I915_WRITE16(IMR, 0xffff);
-	I915_WRITE16(IER, 0x0);
-	POSTING_READ16(IER);
+	GEN2_IRQ_RESET();
 }
 
 static int i8xx_irq_postinstall(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = to_i915(dev);
+	u16 enable_mask;
 
 	I915_WRITE16(EMR,
 		     ~(I915_ERROR_PAGE_TABLE | I915_ERROR_MEMORY_REFRESH));
@@ -3631,13 +3663,12 @@ static int i8xx_irq_postinstall(struct drm_device *dev)
 		  I915_DISPLAY_PIPE_B_EVENT_INTERRUPT |
 		  I915_DISPLAY_PLANE_A_FLIP_PENDING_INTERRUPT |
 		  I915_DISPLAY_PLANE_B_FLIP_PENDING_INTERRUPT);
-	I915_WRITE16(IMR, dev_priv->irq_mask);
+	enable_mask =
+		I915_DISPLAY_PIPE_A_EVENT_INTERRUPT |
+		I915_DISPLAY_PIPE_B_EVENT_INTERRUPT |
+		I915_USER_INTERRUPT;
 
-	I915_WRITE16(IER,
-		     I915_DISPLAY_PIPE_A_EVENT_INTERRUPT |
-		     I915_DISPLAY_PIPE_B_EVENT_INTERRUPT |
-		     I915_USER_INTERRUPT);
-	POSTING_READ16(IER);
+	GEN2_IRQ_INIT(, dev_priv->irq_mask, enable_mask);
 
 	/* Interrupt setup is already guaranteed to be single-threaded, this is
 	 * just to make the assert_spin_locked check happy. */
@@ -3764,9 +3795,7 @@ static void i8xx_irq_uninstall(struct drm_device * dev)
 
 	i9xx_pipestat_irq_reset(dev_priv);
 
-	I915_WRITE16(IMR, 0xffff);
-	I915_WRITE16(IER, 0x0);
-	I915_WRITE16(IIR, I915_READ16(IIR));
+	GEN2_IRQ_RESET();
 }
 
 static void i915_irq_preinstall(struct drm_device * dev)
-- 
2.13.0

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

[PATCH 05/17] drm/i915: Setup EMR first on all gen2-4

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Unify the appaerance of the gen2-4 irq postinstall hooks a little
bit by doing the EMR setup first on all the platforms.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 37 +++++++++++++++++++------------------
 1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 204dac7a5529..a597c86b8d19 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -3654,8 +3654,8 @@ static int i8xx_irq_postinstall(struct drm_device *dev)
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	u16 enable_mask;
 
-	I915_WRITE16(EMR,
-		     ~(I915_ERROR_PAGE_TABLE | I915_ERROR_MEMORY_REFRESH));
+	I915_WRITE16(EMR, ~(I915_ERROR_PAGE_TABLE |
+			    I915_ERROR_MEMORY_REFRESH));
 
 	/* Unmask the interrupts that we always want on. */
 	dev_priv->irq_mask =
@@ -3819,7 +3819,8 @@ static int i915_irq_postinstall(struct drm_device *dev)
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	u32 enable_mask;
 
-	I915_WRITE(EMR, ~(I915_ERROR_PAGE_TABLE | I915_ERROR_MEMORY_REFRESH));
+	I915_WRITE(EMR, ~(I915_ERROR_PAGE_TABLE |
+			  I915_ERROR_MEMORY_REFRESH));
 
 	/* Unmask the interrupts that we always want on. */
 	dev_priv->irq_mask =
@@ -4032,6 +4033,21 @@ static int i965_irq_postinstall(struct drm_device *dev)
 	u32 enable_mask;
 	u32 error_mask;
 
+	/*
+	 * Enable some error detection, note the instruction error mask
+	 * bit is reserved, so we leave it masked.
+	 */
+	if (IS_G4X(dev_priv)) {
+		error_mask = ~(GM45_ERROR_PAGE_TABLE |
+			       GM45_ERROR_MEM_PRIV |
+			       GM45_ERROR_CP_PRIV |
+			       I915_ERROR_MEMORY_REFRESH);
+	} else {
+		error_mask = ~(I915_ERROR_PAGE_TABLE |
+			       I915_ERROR_MEMORY_REFRESH);
+	}
+	I915_WRITE(EMR, error_mask);
+
 	/* Unmask the interrupts that we always want on. */
 	dev_priv->irq_mask = ~(I915_ASLE_INTERRUPT |
 			       I915_DISPLAY_PORT_INTERRUPT |
@@ -4057,21 +4073,6 @@ static int i965_irq_postinstall(struct drm_device *dev)
 	i915_enable_pipestat(dev_priv, PIPE_B, PIPE_CRC_DONE_INTERRUPT_STATUS);
 	spin_unlock_irq(&dev_priv->irq_lock);
 
-	/*
-	 * Enable some error detection, note the instruction error mask
-	 * bit is reserved, so we leave it masked.
-	 */
-	if (IS_G4X(dev_priv)) {
-		error_mask = ~(GM45_ERROR_PAGE_TABLE |
-			       GM45_ERROR_MEM_PRIV |
-			       GM45_ERROR_CP_PRIV |
-			       I915_ERROR_MEMORY_REFRESH);
-	} else {
-		error_mask = ~(I915_ERROR_PAGE_TABLE |
-			       I915_ERROR_MEMORY_REFRESH);
-	}
-	I915_WRITE(EMR, error_mask);
-
 	GEN3_IRQ_INIT(, dev_priv->irq_mask, enable_mask);
 
 	i915_hotplug_interrupt_update(dev_priv, 0xffffffff, 0);
-- 
2.13.0

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

[PATCH 06/17] drm/i915: Eliminate PORT_HOTPLUG_EN setup from gen3/4 irq_postinstall

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

We've already cleared PORT_HOTPLUG_EN in the .irq_preinstall hook
so doing it again in the .irq_postinstall is pointless.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index a597c86b8d19..138232ad789d 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -3837,9 +3837,6 @@ static int i915_irq_postinstall(struct drm_device *dev)
 		I915_USER_INTERRUPT;
 
 	if (I915_HAS_HOTPLUG(dev_priv)) {
-		i915_hotplug_interrupt_update(dev_priv, 0xffffffff, 0);
-		POSTING_READ(PORT_HOTPLUG_EN);
-
 		/* Enable in IER... */
 		enable_mask |= I915_DISPLAY_PORT_INTERRUPT;
 		/* and unmask in IMR */
@@ -4075,9 +4072,6 @@ static int i965_irq_postinstall(struct drm_device *dev)
 
 	GEN3_IRQ_INIT(, dev_priv->irq_mask, enable_mask);
 
-	i915_hotplug_interrupt_update(dev_priv, 0xffffffff, 0);
-	POSTING_READ(PORT_HOTPLUG_EN);
-
 	i915_enable_asle_pipestat(dev_priv);
 
 	return 0;
-- 
2.13.0

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

[PATCH 07/17] drm/i915: Unify the appearance of gen3/4 irq_postistall hooks

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Do the irq_mask/enable_mask setup in the same way on gen3/4, and also
reorder the steps to make the code more uniform.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 36 ++++++++++++++++++++----------------
 1 file changed, 20 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 138232ad789d..4abed121623a 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -3845,8 +3845,6 @@ static int i915_irq_postinstall(struct drm_device *dev)
 
 	GEN3_IRQ_INIT(, dev_priv->irq_mask, enable_mask);
 
-	i915_enable_asle_pipestat(dev_priv);
-
 	/* Interrupt setup is already guaranteed to be single-threaded, this is
 	 * just to make the assert_spin_locked check happy. */
 	spin_lock_irq(&dev_priv->irq_lock);
@@ -3854,6 +3852,8 @@ static int i915_irq_postinstall(struct drm_device *dev)
 	i915_enable_pipestat(dev_priv, PIPE_B, PIPE_CRC_DONE_INTERRUPT_STATUS);
 	spin_unlock_irq(&dev_priv->irq_lock);
 
+	i915_enable_asle_pipestat(dev_priv);
+
 	return 0;
 }
 
@@ -4046,22 +4046,28 @@ static int i965_irq_postinstall(struct drm_device *dev)
 	I915_WRITE(EMR, error_mask);
 
 	/* Unmask the interrupts that we always want on. */
-	dev_priv->irq_mask = ~(I915_ASLE_INTERRUPT |
-			       I915_DISPLAY_PORT_INTERRUPT |
-			       I915_DISPLAY_PIPE_A_EVENT_INTERRUPT |
-			       I915_DISPLAY_PIPE_B_EVENT_INTERRUPT |
-			       I915_DISPLAY_PLANE_A_FLIP_PENDING_INTERRUPT |
-			       I915_DISPLAY_PLANE_B_FLIP_PENDING_INTERRUPT |
-			       I915_RENDER_COMMAND_PARSER_ERROR_INTERRUPT);
-
-	enable_mask = ~dev_priv->irq_mask;
-	enable_mask &= ~(I915_DISPLAY_PLANE_A_FLIP_PENDING_INTERRUPT |
-			 I915_DISPLAY_PLANE_B_FLIP_PENDING_INTERRUPT);
-	enable_mask |= I915_USER_INTERRUPT;
+	dev_priv->irq_mask =
+		~(I915_ASLE_INTERRUPT |
+		  I915_DISPLAY_PORT_INTERRUPT |
+		  I915_DISPLAY_PIPE_A_EVENT_INTERRUPT |
+		  I915_DISPLAY_PIPE_B_EVENT_INTERRUPT |
+		  I915_DISPLAY_PLANE_A_FLIP_PENDING_INTERRUPT |
+		  I915_DISPLAY_PLANE_B_FLIP_PENDING_INTERRUPT |
+		  I915_RENDER_COMMAND_PARSER_ERROR_INTERRUPT);
+
+	enable_mask =
+		I915_ASLE_INTERRUPT |
+		I915_DISPLAY_PORT_INTERRUPT |
+		I915_DISPLAY_PIPE_A_EVENT_INTERRUPT |
+		I915_DISPLAY_PIPE_B_EVENT_INTERRUPT |
+		I915_RENDER_COMMAND_PARSER_ERROR_INTERRUPT |
+		I915_USER_INTERRUPT;
 
 	if (IS_G4X(dev_priv))
 		enable_mask |= I915_BSD_USER_INTERRUPT;
 
+	GEN3_IRQ_INIT(, dev_priv->irq_mask, enable_mask);
+
 	/* Interrupt setup is already guaranteed to be single-threaded, this is
 	 * just to make the assert_spin_locked check happy. */
 	spin_lock_irq(&dev_priv->irq_lock);
@@ -4070,8 +4076,6 @@ static int i965_irq_postinstall(struct drm_device *dev)
 	i915_enable_pipestat(dev_priv, PIPE_B, PIPE_CRC_DONE_INTERRUPT_STATUS);
 	spin_unlock_irq(&dev_priv->irq_lock);
 
-	GEN3_IRQ_INIT(, dev_priv->irq_mask, enable_mask);
-
 	i915_enable_asle_pipestat(dev_priv);
 
 	return 0;
-- 
2.13.0

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

[PATCH 08/17] drm/i915: Remove NULL dev_priv checks from irq_uninstall

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

There should be no way to land in irq_uninstall without a
valid dev_priv. Let's kill off the remaining checks, which are
probably some kind of UMS leftovers. Not all the irq_uninstall
hooks even had them anymore.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 19 -------------------
 1 file changed, 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 4abed121623a..b4c9a6a7023a 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -3582,11 +3582,6 @@ static int cherryview_irq_postinstall(struct drm_device *dev)
 
 static void gen8_irq_uninstall(struct drm_device *dev)
 {
-	struct drm_i915_private *dev_priv = to_i915(dev);
-
-	if (!dev_priv)
-		return;
-
 	gen8_irq_reset(dev);
 }
 
@@ -3594,9 +3589,6 @@ static void valleyview_irq_uninstall(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = to_i915(dev);
 
-	if (!dev_priv)
-		return;
-
 	I915_WRITE(VLV_MASTER_IER, 0);
 	POSTING_READ(VLV_MASTER_IER);
 
@@ -3614,9 +3606,6 @@ static void cherryview_irq_uninstall(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = to_i915(dev);
 
-	if (!dev_priv)
-		return;
-
 	I915_WRITE(GEN8_MASTER_IRQ, 0);
 	POSTING_READ(GEN8_MASTER_IRQ);
 
@@ -3632,11 +3621,6 @@ static void cherryview_irq_uninstall(struct drm_device *dev)
 
 static void ironlake_irq_uninstall(struct drm_device *dev)
 {
-	struct drm_i915_private *dev_priv = to_i915(dev);
-
-	if (!dev_priv)
-		return;
-
 	ironlake_irq_reset(dev);
 }
 
@@ -4220,9 +4204,6 @@ static void i965_irq_uninstall(struct drm_device * dev)
 {
 	struct drm_i915_private *dev_priv = to_i915(dev);
 
-	if (!dev_priv)
-		return;
-
 	i915_hotplug_interrupt_update(dev_priv, 0xffffffff, 0);
 	I915_WRITE(PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT));
 
-- 
2.13.0

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

[PATCH 09/17] drm/i915: Mask everything in ring HWSTAM on gen6+ in ringbuffer mode

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The execlist code already masks everything in the ring HWSTAM, but
the ringbuffer code doesn't. Let's go ahead and do that. Pre-gen6
platforms setup HWSTAM during irq setup already since there's just
the one register, and it also contains bits for non-ring interrupts.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 5224b7abb8a3..d7a611c5ce89 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -427,6 +427,9 @@ static void intel_ring_setup_status_page(struct intel_engine_cs *engine)
 		mmio = RING_HWS_PGA(engine->mmio_base);
 	}
 
+	if (INTEL_GEN(dev_priv) >= 6)
+		I915_WRITE(RING_HWSTAM(engine->mmio_base), 0xffffffff);
+
 	I915_WRITE(mmio, engine->status_page.ggtt_offset);
 	POSTING_READ(mmio);
 
-- 
2.13.0

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

[PATCH 10/17] drm/i915: Gen3 HWSTAM is actually 32 bits

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Bspec claims that HWSTAM is only 16 bits on gen3, but the other
interrupts registers are 32 bits and there are 18 valid interrupt
bits. Hence a 16 bit HWSTAM wouldn't be able to contain all the
bits, so it seems the spec is incorrect about the size of the
register. And indeed I can clear bits 16 and 17 just fine with
a 32 bit write. So let's adjust the code to treat the register
as 32 bits.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index b4c9a6a7023a..2b8aeb79a7a8 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -3793,7 +3793,7 @@ static void i915_irq_preinstall(struct drm_device * dev)
 
 	i9xx_pipestat_irq_reset(dev_priv);
 
-	I915_WRITE16(HWSTAM, 0xeffe);
+	I915_WRITE(HWSTAM, 0xffffeffe);
 
 	GEN3_IRQ_RESET();
 }
@@ -3989,7 +3989,7 @@ static void i915_irq_uninstall(struct drm_device * dev)
 
 	i9xx_pipestat_irq_reset(dev_priv);
 
-	I915_WRITE16(HWSTAM, 0xffff);
+	I915_WRITE(HWSTAM, 0xffffffff);
 
 	GEN3_IRQ_RESET();
 }
-- 
2.13.0

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

[PATCH 11/17] drm/i915: Clean up the HWSTAM mess

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Currently we're unmasking some random looking bits in HWSTAM
on gen3/4/5. The two bits we apparently unmask are 0 and 12,
and also bits 16-31 on gen4/5.
What those bits do depends on the gen as follows:
 bit 0: Breakpoint (gen2), ASLE (gen3), reserved (gen4), render user interrupt (gen5)
 bit 12: Sync flush statusa (gen2-4), reserved (gen5)
 bit 16-31: The ones that can unmasked seem to be mostly some
            display stuff on gen4. Bit 18 is the PIPE_CONTROL notify,
	    which might be the only intresting one. On gen5 all the
	    bits are reserved.

So I don't know whether we actually depend on that status page write
somehow. Extra seqno coherency by accident perhaps? Except we don't
even unmask the user interrupt bit in HWSTAM except on gen5, and
sync flush isn't something we use normally, so seems unlikely. So
let's just assume we don't need any of this and mask everything in
HWSTAM.

From gen6 onwards there's a separate HWSTAM for each engine, and so
we deal with them during the engine setup.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 2b8aeb79a7a8..f55f1b61d117 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -3027,7 +3027,8 @@ static void ironlake_irq_reset(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = to_i915(dev);
 
-	I915_WRITE(HWSTAM, 0xffffffff);
+	if (IS_GEN5(dev_priv))
+		I915_WRITE(HWSTAM, 0xffffffff);
 
 	GEN3_IRQ_RESET(DE);
 	if (IS_GEN7(dev_priv))
@@ -3390,8 +3391,6 @@ static int ironlake_irq_postinstall(struct drm_device *dev)
 
 	dev_priv->irq_mask = ~display_mask;
 
-	I915_WRITE(HWSTAM, 0xeffe);
-
 	ibx_irq_pre_postinstall(dev);
 
 	GEN3_IRQ_INIT(DE, dev_priv->irq_mask, display_mask | extra_mask);
@@ -3594,8 +3593,6 @@ static void valleyview_irq_uninstall(struct drm_device *dev)
 
 	gen5_gt_irq_reset(dev_priv);
 
-	I915_WRITE(HWSTAM, 0xffffffff);
-
 	spin_lock_irq(&dev_priv->irq_lock);
 	if (dev_priv->display_irqs_enabled)
 		vlv_display_irq_reset(dev_priv);
@@ -3630,6 +3627,8 @@ static void i8xx_irq_preinstall(struct drm_device * dev)
 
 	i9xx_pipestat_irq_reset(dev_priv);
 
+	I915_WRITE16(HWSTAM, 0xffff);
+
 	GEN2_IRQ_RESET();
 }
 
@@ -3779,6 +3778,8 @@ static void i8xx_irq_uninstall(struct drm_device * dev)
 
 	i9xx_pipestat_irq_reset(dev_priv);
 
+	I915_WRITE16(HWSTAM, 0xffff);
+
 	GEN2_IRQ_RESET();
 }
 
@@ -3793,7 +3794,7 @@ static void i915_irq_preinstall(struct drm_device * dev)
 
 	i9xx_pipestat_irq_reset(dev_priv);
 
-	I915_WRITE(HWSTAM, 0xffffeffe);
+	I915_WRITE(HWSTAM, 0xffffffff);
 
 	GEN3_IRQ_RESET();
 }
@@ -4003,7 +4004,7 @@ static void i965_irq_preinstall(struct drm_device * dev)
 
 	i9xx_pipestat_irq_reset(dev_priv);
 
-	I915_WRITE(HWSTAM, 0xeffe);
+	I915_WRITE(HWSTAM, 0xffffffff);
 
 	GEN3_IRQ_RESET();
 }
-- 
2.13.0

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

[PATCH 12/17] drm/i915: Remove duplicated irq_preinstall/uninstall hooks

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

All the irq_preinstall and irq_uninstall hooks are now identical. Let's
just rename them all the irq_reset and remove the pointless duplicates.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 117 ++++++----------------------------------
 1 file changed, 17 insertions(+), 100 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index f55f1b61d117..5f92d421bb58 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -3039,7 +3039,7 @@ static void ironlake_irq_reset(struct drm_device *dev)
 	ibx_irq_reset(dev_priv);
 }
 
-static void valleyview_irq_preinstall(struct drm_device *dev)
+static void valleyview_irq_reset(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = to_i915(dev);
 
@@ -3113,7 +3113,7 @@ void gen8_irq_power_well_pre_disable(struct drm_i915_private *dev_priv,
 	synchronize_irq(dev_priv->drm.irq);
 }
 
-static void cherryview_irq_preinstall(struct drm_device *dev)
+static void cherryview_irq_reset(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = to_i915(dev);
 
@@ -3579,49 +3579,7 @@ static int cherryview_irq_postinstall(struct drm_device *dev)
 	return 0;
 }
 
-static void gen8_irq_uninstall(struct drm_device *dev)
-{
-	gen8_irq_reset(dev);
-}
-
-static void valleyview_irq_uninstall(struct drm_device *dev)
-{
-	struct drm_i915_private *dev_priv = to_i915(dev);
-
-	I915_WRITE(VLV_MASTER_IER, 0);
-	POSTING_READ(VLV_MASTER_IER);
-
-	gen5_gt_irq_reset(dev_priv);
-
-	spin_lock_irq(&dev_priv->irq_lock);
-	if (dev_priv->display_irqs_enabled)
-		vlv_display_irq_reset(dev_priv);
-	spin_unlock_irq(&dev_priv->irq_lock);
-}
-
-static void cherryview_irq_uninstall(struct drm_device *dev)
-{
-	struct drm_i915_private *dev_priv = to_i915(dev);
-
-	I915_WRITE(GEN8_MASTER_IRQ, 0);
-	POSTING_READ(GEN8_MASTER_IRQ);
-
-	gen8_gt_irq_reset(dev_priv);
-
-	GEN3_IRQ_RESET(GEN8_PCU_);
-
-	spin_lock_irq(&dev_priv->irq_lock);
-	if (dev_priv->display_irqs_enabled)
-		vlv_display_irq_reset(dev_priv);
-	spin_unlock_irq(&dev_priv->irq_lock);
-}
-
-static void ironlake_irq_uninstall(struct drm_device *dev)
-{
-	ironlake_irq_reset(dev);
-}
-
-static void i8xx_irq_preinstall(struct drm_device * dev)
+static void i8xx_irq_reset(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = to_i915(dev);
 
@@ -3772,18 +3730,7 @@ static irqreturn_t i8xx_irq_handler(int irq, void *arg)
 	return ret;
 }
 
-static void i8xx_irq_uninstall(struct drm_device * dev)
-{
-	struct drm_i915_private *dev_priv = to_i915(dev);
-
-	i9xx_pipestat_irq_reset(dev_priv);
-
-	I915_WRITE16(HWSTAM, 0xffff);
-
-	GEN2_IRQ_RESET();
-}
-
-static void i915_irq_preinstall(struct drm_device * dev)
+static void i915_irq_reset(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = to_i915(dev);
 
@@ -3979,23 +3926,7 @@ static irqreturn_t i915_irq_handler(int irq, void *arg)
 	return ret;
 }
 
-static void i915_irq_uninstall(struct drm_device * dev)
-{
-	struct drm_i915_private *dev_priv = to_i915(dev);
-
-	if (I915_HAS_HOTPLUG(dev_priv)) {
-		i915_hotplug_interrupt_update(dev_priv, 0xffffffff, 0);
-		I915_WRITE(PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT));
-	}
-
-	i9xx_pipestat_irq_reset(dev_priv);
-
-	I915_WRITE(HWSTAM, 0xffffffff);
-
-	GEN3_IRQ_RESET();
-}
-
-static void i965_irq_preinstall(struct drm_device * dev)
+static void i965_irq_reset(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = to_i915(dev);
 
@@ -4201,20 +4132,6 @@ static irqreturn_t i965_irq_handler(int irq, void *arg)
 	return ret;
 }
 
-static void i965_irq_uninstall(struct drm_device * dev)
-{
-	struct drm_i915_private *dev_priv = to_i915(dev);
-
-	i915_hotplug_interrupt_update(dev_priv, 0xffffffff, 0);
-	I915_WRITE(PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT));
-
-	i9xx_pipestat_irq_reset(dev_priv);
-
-	I915_WRITE(HWSTAM, 0xffffffff);
-
-	GEN3_IRQ_RESET();
-}
-
 /**
  * intel_irq_init - initializes irq support
  * @dev_priv: i915 device instance
@@ -4295,17 +4212,17 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
 
 	if (IS_CHERRYVIEW(dev_priv)) {
 		dev->driver->irq_handler = cherryview_irq_handler;
-		dev->driver->irq_preinstall = cherryview_irq_preinstall;
+		dev->driver->irq_preinstall = cherryview_irq_reset;
 		dev->driver->irq_postinstall = cherryview_irq_postinstall;
-		dev->driver->irq_uninstall = cherryview_irq_uninstall;
+		dev->driver->irq_uninstall = cherryview_irq_reset;
 		dev->driver->enable_vblank = i965_enable_vblank;
 		dev->driver->disable_vblank = i965_disable_vblank;
 		dev_priv->display.hpd_irq_setup = i915_hpd_irq_setup;
 	} else if (IS_VALLEYVIEW(dev_priv)) {
 		dev->driver->irq_handler = valleyview_irq_handler;
-		dev->driver->irq_preinstall = valleyview_irq_preinstall;
+		dev->driver->irq_preinstall = valleyview_irq_reset;
 		dev->driver->irq_postinstall = valleyview_irq_postinstall;
-		dev->driver->irq_uninstall = valleyview_irq_uninstall;
+		dev->driver->irq_uninstall = valleyview_irq_reset;
 		dev->driver->enable_vblank = i965_enable_vblank;
 		dev->driver->disable_vblank = i965_disable_vblank;
 		dev_priv->display.hpd_irq_setup = i915_hpd_irq_setup;
@@ -4313,7 +4230,7 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
 		dev->driver->irq_handler = gen8_irq_handler;
 		dev->driver->irq_preinstall = gen8_irq_reset;
 		dev->driver->irq_postinstall = gen8_irq_postinstall;
-		dev->driver->irq_uninstall = gen8_irq_uninstall;
+		dev->driver->irq_uninstall = gen8_irq_reset;
 		dev->driver->enable_vblank = gen8_enable_vblank;
 		dev->driver->disable_vblank = gen8_disable_vblank;
 		if (IS_GEN9_LP(dev_priv))
@@ -4327,29 +4244,29 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
 		dev->driver->irq_handler = ironlake_irq_handler;
 		dev->driver->irq_preinstall = ironlake_irq_reset;
 		dev->driver->irq_postinstall = ironlake_irq_postinstall;
-		dev->driver->irq_uninstall = ironlake_irq_uninstall;
+		dev->driver->irq_uninstall = ironlake_irq_reset;
 		dev->driver->enable_vblank = ironlake_enable_vblank;
 		dev->driver->disable_vblank = ironlake_disable_vblank;
 		dev_priv->display.hpd_irq_setup = ilk_hpd_irq_setup;
 	} else {
 		if (IS_GEN2(dev_priv)) {
-			dev->driver->irq_preinstall = i8xx_irq_preinstall;
+			dev->driver->irq_preinstall = i8xx_irq_reset;
 			dev->driver->irq_postinstall = i8xx_irq_postinstall;
 			dev->driver->irq_handler = i8xx_irq_handler;
-			dev->driver->irq_uninstall = i8xx_irq_uninstall;
+			dev->driver->irq_uninstall = i8xx_irq_reset;
 			dev->driver->enable_vblank = i8xx_enable_vblank;
 			dev->driver->disable_vblank = i8xx_disable_vblank;
 		} else if (IS_GEN3(dev_priv)) {
-			dev->driver->irq_preinstall = i915_irq_preinstall;
+			dev->driver->irq_preinstall = i915_irq_reset;
 			dev->driver->irq_postinstall = i915_irq_postinstall;
-			dev->driver->irq_uninstall = i915_irq_uninstall;
+			dev->driver->irq_uninstall = i915_irq_reset;
 			dev->driver->irq_handler = i915_irq_handler;
 			dev->driver->enable_vblank = i8xx_enable_vblank;
 			dev->driver->disable_vblank = i8xx_disable_vblank;
 		} else {
-			dev->driver->irq_preinstall = i965_irq_preinstall;
+			dev->driver->irq_preinstall = i965_irq_reset;
 			dev->driver->irq_postinstall = i965_irq_postinstall;
-			dev->driver->irq_uninstall = i965_irq_uninstall;
+			dev->driver->irq_uninstall = i965_irq_reset;
 			dev->driver->irq_handler = i965_irq_handler;
 			dev->driver->enable_vblank = i965_enable_vblank;
 			dev->driver->disable_vblank = i965_disable_vblank;
-- 
2.13.0

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

[PATCH 13/17] drm/i915: Consolidatte intel_check_page_flip() into intel_pipe_handle_vblank()

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Almost all callers of intel_pipe_handle_vblank() immediately call
intel_check_page_flip() depending on the return value. Let's move
the call into the function itself.

We'll just neeed to deal with the old gmch "flip pending" stuff
in a slightly different way.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 35 +++++++++++++++++++----------------
 1 file changed, 19 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 5f92d421bb58..43eaacaa7d52 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1753,8 +1753,8 @@ static void gen9_guc_irq_handler(struct drm_i915_private *dev_priv, u32 gt_iir)
 	}
 }
 
-static bool intel_pipe_handle_vblank(struct drm_i915_private *dev_priv,
-				     enum pipe pipe)
+static bool _intel_pipe_handle_vblank(struct drm_i915_private *dev_priv,
+				      enum pipe pipe)
 {
 	bool ret;
 
@@ -1765,6 +1765,13 @@ static bool intel_pipe_handle_vblank(struct drm_i915_private *dev_priv,
 	return ret;
 }
 
+static void intel_pipe_handle_vblank(struct drm_i915_private *dev_priv,
+				     enum pipe pipe)
+{
+	if (_intel_pipe_handle_vblank(dev_priv, pipe))
+		intel_check_page_flip(dev_priv, pipe);
+}
+
 static void i9xx_pipestat_irq_reset(struct drm_i915_private *dev_priv)
 {
 	enum pipe pipe;
@@ -1842,9 +1849,8 @@ static void valleyview_pipestat_irq_handler(struct drm_i915_private *dev_priv,
 	enum pipe pipe;
 
 	for_each_pipe(dev_priv, pipe) {
-		if (pipe_stats[pipe] & PIPE_START_VBLANK_INTERRUPT_STATUS &&
-		    intel_pipe_handle_vblank(dev_priv, pipe))
-			intel_check_page_flip(dev_priv, pipe);
+		if (pipe_stats[pipe] & PIPE_START_VBLANK_INTERRUPT_STATUS)
+			intel_pipe_handle_vblank(dev_priv, pipe);
 
 		if (pipe_stats[pipe] & PLANE_FLIP_DONE_INT_STATUS_VLV)
 			intel_finish_page_flip_cs(dev_priv, pipe);
@@ -2299,9 +2305,8 @@ static void ilk_display_irq_handler(struct drm_i915_private *dev_priv,
 		DRM_ERROR("Poison interrupt\n");
 
 	for_each_pipe(dev_priv, pipe) {
-		if (de_iir & DE_PIPE_VBLANK(pipe) &&
-		    intel_pipe_handle_vblank(dev_priv, pipe))
-			intel_check_page_flip(dev_priv, pipe);
+		if (de_iir & DE_PIPE_VBLANK(pipe))
+			intel_pipe_handle_vblank(dev_priv, pipe);
 
 		if (de_iir & DE_PIPE_FIFO_UNDERRUN(pipe))
 			intel_cpu_fifo_underrun_irq_handler(dev_priv, pipe);
@@ -2350,9 +2355,8 @@ static void ivb_display_irq_handler(struct drm_i915_private *dev_priv,
 		intel_opregion_asle_intr(dev_priv);
 
 	for_each_pipe(dev_priv, pipe) {
-		if (de_iir & (DE_PIPE_VBLANK_IVB(pipe)) &&
-		    intel_pipe_handle_vblank(dev_priv, pipe))
-			intel_check_page_flip(dev_priv, pipe);
+		if (de_iir & (DE_PIPE_VBLANK_IVB(pipe)))
+			intel_pipe_handle_vblank(dev_priv, pipe);
 
 		/* plane/pipes map 1:1 on ilk+ */
 		if (de_iir & DE_PLANE_FLIP_DONE_IVB(pipe))
@@ -2551,9 +2555,8 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
 		ret = IRQ_HANDLED;
 		I915_WRITE(GEN8_DE_PIPE_IIR(pipe), iir);
 
-		if (iir & GEN8_PIPE_VBLANK &&
-		    intel_pipe_handle_vblank(dev_priv, pipe))
-			intel_check_page_flip(dev_priv, pipe);
+		if (iir & GEN8_PIPE_VBLANK)
+			intel_pipe_handle_vblank(dev_priv, pipe);
 
 		flip_done = iir;
 		if (INTEL_INFO(dev_priv)->gen >= 9)
@@ -3629,7 +3632,7 @@ static bool i8xx_handle_vblank(struct drm_i915_private *dev_priv,
 {
 	u16 flip_pending = DISPLAY_PLANE_FLIP_PENDING(plane);
 
-	if (!intel_pipe_handle_vblank(dev_priv, pipe))
+	if (!_intel_pipe_handle_vblank(dev_priv, pipe))
 		return false;
 
 	if ((iir & flip_pending) == 0)
@@ -3797,7 +3800,7 @@ static bool i915_handle_vblank(struct drm_i915_private *dev_priv,
 {
 	u32 flip_pending = DISPLAY_PLANE_FLIP_PENDING(plane);
 
-	if (!intel_pipe_handle_vblank(dev_priv, pipe))
+	if (!_intel_pipe_handle_vblank(dev_priv, pipe))
 		return false;
 
 	if ((iir & flip_pending) == 0)
-- 
2.13.0

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

[PATCH 14/17] drm/i915: Move the gen2-4 page flip handling code around

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Move i8xx_handle_vblank() and i915_handle_vblank() to an earlier
location so that we can later collect all the PIPESTAT irq handling
code next to the VLV/CHV PIPESTAT handling code.

While at it s/u32 iir/u16 iir/ for i8xx_handle_vblank() since the
IIR register is only 16 bits on gen2.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 124 ++++++++++++++++++++--------------------
 1 file changed, 62 insertions(+), 62 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 43eaacaa7d52..e18b66d01cad 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1772,6 +1772,68 @@ static void intel_pipe_handle_vblank(struct drm_i915_private *dev_priv,
 		intel_check_page_flip(dev_priv, pipe);
 }
 
+/*
+ * Returns true when a page flip has completed.
+ */
+static bool i8xx_handle_vblank(struct drm_i915_private *dev_priv,
+			       int plane, int pipe, u16 iir)
+{
+	u16 flip_pending = DISPLAY_PLANE_FLIP_PENDING(plane);
+
+	if (!_intel_pipe_handle_vblank(dev_priv, pipe))
+		return false;
+
+	if ((iir & flip_pending) == 0)
+		goto check_page_flip;
+
+	/* We detect FlipDone by looking for the change in PendingFlip from '1'
+	 * to '0' on the following vblank, i.e. IIR has the Pendingflip
+	 * asserted following the MI_DISPLAY_FLIP, but ISR is deasserted, hence
+	 * the flip is completed (no longer pending). Since this doesn't raise
+	 * an interrupt per se, we watch for the change at vblank.
+	 */
+	if (I915_READ16(ISR) & flip_pending)
+		goto check_page_flip;
+
+	intel_finish_page_flip_cs(dev_priv, pipe);
+	return true;
+
+check_page_flip:
+	intel_check_page_flip(dev_priv, pipe);
+	return false;
+}
+
+/*
+ * Returns true when a page flip has completed.
+ */
+static bool i915_handle_vblank(struct drm_i915_private *dev_priv,
+			       int plane, int pipe, u32 iir)
+{
+	u32 flip_pending = DISPLAY_PLANE_FLIP_PENDING(plane);
+
+	if (!_intel_pipe_handle_vblank(dev_priv, pipe))
+		return false;
+
+	if ((iir & flip_pending) == 0)
+		goto check_page_flip;
+
+	/* We detect FlipDone by looking for the change in PendingFlip from '1'
+	 * to '0' on the following vblank, i.e. IIR has the Pendingflip
+	 * asserted following the MI_DISPLAY_FLIP, but ISR is deasserted, hence
+	 * the flip is completed (no longer pending). Since this doesn't raise
+	 * an interrupt per se, we watch for the change at vblank.
+	 */
+	if (I915_READ(ISR) & flip_pending)
+		goto check_page_flip;
+
+	intel_finish_page_flip_cs(dev_priv, pipe);
+	return true;
+
+check_page_flip:
+	intel_check_page_flip(dev_priv, pipe);
+	return false;
+}
+
 static void i9xx_pipestat_irq_reset(struct drm_i915_private *dev_priv)
 {
 	enum pipe pipe;
@@ -3624,37 +3686,6 @@ static int i8xx_irq_postinstall(struct drm_device *dev)
 	return 0;
 }
 
-/*
- * Returns true when a page flip has completed.
- */
-static bool i8xx_handle_vblank(struct drm_i915_private *dev_priv,
-			       int plane, int pipe, u32 iir)
-{
-	u16 flip_pending = DISPLAY_PLANE_FLIP_PENDING(plane);
-
-	if (!_intel_pipe_handle_vblank(dev_priv, pipe))
-		return false;
-
-	if ((iir & flip_pending) == 0)
-		goto check_page_flip;
-
-	/* We detect FlipDone by looking for the change in PendingFlip from '1'
-	 * to '0' on the following vblank, i.e. IIR has the Pendingflip
-	 * asserted following the MI_DISPLAY_FLIP, but ISR is deasserted, hence
-	 * the flip is completed (no longer pending). Since this doesn't raise
-	 * an interrupt per se, we watch for the change at vblank.
-	 */
-	if (I915_READ16(ISR) & flip_pending)
-		goto check_page_flip;
-
-	intel_finish_page_flip_cs(dev_priv, pipe);
-	return true;
-
-check_page_flip:
-	intel_check_page_flip(dev_priv, pipe);
-	return false;
-}
-
 static irqreturn_t i8xx_irq_handler(int irq, void *arg)
 {
 	struct drm_device *dev = arg;
@@ -3792,37 +3823,6 @@ static int i915_irq_postinstall(struct drm_device *dev)
 	return 0;
 }
 
-/*
- * Returns true when a page flip has completed.
- */
-static bool i915_handle_vblank(struct drm_i915_private *dev_priv,
-			       int plane, int pipe, u32 iir)
-{
-	u32 flip_pending = DISPLAY_PLANE_FLIP_PENDING(plane);
-
-	if (!_intel_pipe_handle_vblank(dev_priv, pipe))
-		return false;
-
-	if ((iir & flip_pending) == 0)
-		goto check_page_flip;
-
-	/* We detect FlipDone by looking for the change in PendingFlip from '1'
-	 * to '0' on the following vblank, i.e. IIR has the Pendingflip
-	 * asserted following the MI_DISPLAY_FLIP, but ISR is deasserted, hence
-	 * the flip is completed (no longer pending). Since this doesn't raise
-	 * an interrupt per se, we watch for the change at vblank.
-	 */
-	if (I915_READ(ISR) & flip_pending)
-		goto check_page_flip;
-
-	intel_finish_page_flip_cs(dev_priv, pipe);
-	return true;
-
-check_page_flip:
-	intel_check_page_flip(dev_priv, pipe);
-	return false;
-}
-
 static irqreturn_t i915_irq_handler(int irq, void *arg)
 {
 	struct drm_device *dev = arg;
-- 
2.13.0

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

[PATCH 15/17] drm/i915: Simplify the gen2-4 flip_mask handling

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Replace the complicated "loop multiple times over IIR with different
flip_mask" logic with just clearing the relevant bit from IIR when
we actually handle the interrupt. This results in potentially an extra
IIR write, but so what.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 43 ++++++++++++++++-------------------------
 1 file changed, 17 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index e18b66d01cad..b75b0790e9df 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1772,16 +1772,13 @@ static void intel_pipe_handle_vblank(struct drm_i915_private *dev_priv,
 		intel_check_page_flip(dev_priv, pipe);
 }
 
-/*
- * Returns true when a page flip has completed.
- */
-static bool i8xx_handle_vblank(struct drm_i915_private *dev_priv,
+static void i8xx_handle_vblank(struct drm_i915_private *dev_priv,
 			       int plane, int pipe, u16 iir)
 {
 	u16 flip_pending = DISPLAY_PLANE_FLIP_PENDING(plane);
 
 	if (!_intel_pipe_handle_vblank(dev_priv, pipe))
-		return false;
+		return;
 
 	if ((iir & flip_pending) == 0)
 		goto check_page_flip;
@@ -1795,24 +1792,21 @@ static bool i8xx_handle_vblank(struct drm_i915_private *dev_priv,
 	if (I915_READ16(ISR) & flip_pending)
 		goto check_page_flip;
 
+	I915_WRITE16(IIR, flip_pending);
 	intel_finish_page_flip_cs(dev_priv, pipe);
-	return true;
+	return;
 
 check_page_flip:
 	intel_check_page_flip(dev_priv, pipe);
-	return false;
 }
 
-/*
- * Returns true when a page flip has completed.
- */
-static bool i915_handle_vblank(struct drm_i915_private *dev_priv,
+static void i915_handle_vblank(struct drm_i915_private *dev_priv,
 			       int plane, int pipe, u32 iir)
 {
 	u32 flip_pending = DISPLAY_PLANE_FLIP_PENDING(plane);
 
 	if (!_intel_pipe_handle_vblank(dev_priv, pipe))
-		return false;
+		return;
 
 	if ((iir & flip_pending) == 0)
 		goto check_page_flip;
@@ -1826,12 +1820,12 @@ static bool i915_handle_vblank(struct drm_i915_private *dev_priv,
 	if (I915_READ(ISR) & flip_pending)
 		goto check_page_flip;
 
+	I915_WRITE(IIR, flip_pending);
 	intel_finish_page_flip_cs(dev_priv, pipe);
-	return true;
+	return;
 
 check_page_flip:
 	intel_check_page_flip(dev_priv, pipe);
-	return false;
 }
 
 static void i9xx_pipestat_irq_reset(struct drm_i915_private *dev_priv)
@@ -3693,7 +3687,7 @@ static irqreturn_t i8xx_irq_handler(int irq, void *arg)
 	u16 iir, new_iir;
 	u32 pipe_stats[2];
 	int pipe;
-	u16 flip_mask =
+	const u16 flip_mask =
 		I915_DISPLAY_PLANE_A_FLIP_PENDING_INTERRUPT |
 		I915_DISPLAY_PLANE_B_FLIP_PENDING_INTERRUPT;
 	irqreturn_t ret;
@@ -3742,9 +3736,8 @@ static irqreturn_t i8xx_irq_handler(int irq, void *arg)
 			if (HAS_FBC(dev_priv))
 				plane = !plane;
 
-			if (pipe_stats[pipe] & PIPE_VBLANK_INTERRUPT_STATUS &&
-			    i8xx_handle_vblank(dev_priv, plane, pipe, iir))
-				flip_mask &= ~DISPLAY_PLANE_FLIP_PENDING(plane);
+			if (pipe_stats[pipe] & PIPE_VBLANK_INTERRUPT_STATUS)
+				i8xx_handle_vblank(dev_priv, plane, pipe, iir);
 
 			if (pipe_stats[pipe] & PIPE_CRC_DONE_INTERRUPT_STATUS)
 				i9xx_pipe_crc_irq_handler(dev_priv, pipe);
@@ -3828,7 +3821,7 @@ static irqreturn_t i915_irq_handler(int irq, void *arg)
 	struct drm_device *dev = arg;
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	u32 iir, new_iir, pipe_stats[I915_MAX_PIPES];
-	u32 flip_mask =
+	const u32 flip_mask =
 		I915_DISPLAY_PLANE_A_FLIP_PENDING_INTERRUPT |
 		I915_DISPLAY_PLANE_B_FLIP_PENDING_INTERRUPT;
 	int pipe, ret = IRQ_NONE;
@@ -3887,9 +3880,8 @@ static irqreturn_t i915_irq_handler(int irq, void *arg)
 			if (HAS_FBC(dev_priv))
 				plane = !plane;
 
-			if (pipe_stats[pipe] & PIPE_VBLANK_INTERRUPT_STATUS &&
-			    i915_handle_vblank(dev_priv, plane, pipe, iir))
-				flip_mask &= ~DISPLAY_PLANE_FLIP_PENDING(plane);
+			if (pipe_stats[pipe] & PIPE_VBLANK_INTERRUPT_STATUS)
+				i915_handle_vblank(dev_priv, plane, pipe, iir);
 
 			if (pipe_stats[pipe] & PIPE_LEGACY_BLC_EVENT_STATUS)
 				blc_event = true;
@@ -4032,7 +4024,7 @@ static irqreturn_t i965_irq_handler(int irq, void *arg)
 	u32 iir, new_iir;
 	u32 pipe_stats[I915_MAX_PIPES];
 	int ret = IRQ_NONE, pipe;
-	u32 flip_mask =
+	const u32 flip_mask =
 		I915_DISPLAY_PLANE_A_FLIP_PENDING_INTERRUPT |
 		I915_DISPLAY_PLANE_B_FLIP_PENDING_INTERRUPT;
 
@@ -4092,9 +4084,8 @@ static irqreturn_t i965_irq_handler(int irq, void *arg)
 			notify_ring(dev_priv->engine[VCS]);
 
 		for_each_pipe(dev_priv, pipe) {
-			if (pipe_stats[pipe] & PIPE_START_VBLANK_INTERRUPT_STATUS &&
-			    i915_handle_vblank(dev_priv, pipe, pipe, iir))
-				flip_mask &= ~DISPLAY_PLANE_FLIP_PENDING(pipe);
+			if (pipe_stats[pipe] & PIPE_START_VBLANK_INTERRUPT_STATUS)
+				i915_handle_vblank(dev_priv, pipe, pipe, iir);
 
 			if (pipe_stats[pipe] & PIPE_LEGACY_BLC_EVENT_STATUS)
 				blc_event = true;
-- 
2.13.0

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

[PATCH 16/17] drm/i915: Extract PIPESTAT irq handling into separate functions

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Extract the gen2-4 PIPESTAT irq handling into separate functions just
like we already do on VLV/CHV.

We can share valleyview_pipestat_irq_ack() on all gmch platforms to
actually read and clear the PIPESTAT status bits, so let's rename
it to i9xx_pipestat_irq_ack().

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 219 ++++++++++++++++++----------------------
 1 file changed, 99 insertions(+), 120 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index b75b0790e9df..f00e20902e1c 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1841,8 +1841,8 @@ static void i9xx_pipestat_irq_reset(struct drm_i915_private *dev_priv)
 	}
 }
 
-static void valleyview_pipestat_irq_ack(struct drm_i915_private *dev_priv,
-					u32 iir, u32 pipe_stats[I915_MAX_PIPES])
+static void i9xx_pipestat_irq_ack(struct drm_i915_private *dev_priv,
+				  u32 iir, u32 pipe_stats[I915_MAX_PIPES])
 {
 	int pipe;
 
@@ -1899,6 +1899,82 @@ static void valleyview_pipestat_irq_ack(struct drm_i915_private *dev_priv,
 	spin_unlock(&dev_priv->irq_lock);
 }
 
+static void i8xx_pipestat_irq_handler(struct drm_i915_private *dev_priv,
+				      u16 iir, u32 pipe_stats[I915_MAX_PIPES])
+{
+	enum pipe pipe;
+
+	for_each_pipe(dev_priv, pipe) {
+		enum plane plane = pipe;
+		if (HAS_FBC(dev_priv))
+			plane = !plane;
+
+		if (pipe_stats[pipe] & PIPE_VBLANK_INTERRUPT_STATUS)
+			i8xx_handle_vblank(dev_priv, plane, pipe, iir);
+
+		if (pipe_stats[pipe] & PIPE_CRC_DONE_INTERRUPT_STATUS)
+			i9xx_pipe_crc_irq_handler(dev_priv, pipe);
+
+		if (pipe_stats[pipe] & PIPE_FIFO_UNDERRUN_STATUS)
+			intel_cpu_fifo_underrun_irq_handler(dev_priv, pipe);
+	}
+}
+
+static void i915_pipestat_irq_handler(struct drm_i915_private *dev_priv,
+				      u32 iir, u32 pipe_stats[I915_MAX_PIPES])
+{
+	bool blc_event = false;
+	enum pipe pipe;
+
+	for_each_pipe(dev_priv, pipe) {
+		enum plane plane = pipe;
+		if (HAS_FBC(dev_priv))
+			plane = !plane;
+
+		if (pipe_stats[pipe] & PIPE_VBLANK_INTERRUPT_STATUS)
+			i915_handle_vblank(dev_priv, plane, pipe, iir);
+
+		if (pipe_stats[pipe] & PIPE_LEGACY_BLC_EVENT_STATUS)
+			blc_event = true;
+
+		if (pipe_stats[pipe] & PIPE_CRC_DONE_INTERRUPT_STATUS)
+			i9xx_pipe_crc_irq_handler(dev_priv, pipe);
+
+		if (pipe_stats[pipe] & PIPE_FIFO_UNDERRUN_STATUS)
+			intel_cpu_fifo_underrun_irq_handler(dev_priv, pipe);
+	}
+
+	if (blc_event || (iir & I915_ASLE_INTERRUPT))
+		intel_opregion_asle_intr(dev_priv);
+}
+
+static void i965_pipestat_irq_handler(struct drm_i915_private *dev_priv,
+				      u32 iir, u32 pipe_stats[I915_MAX_PIPES])
+{
+	bool blc_event = false;
+	enum pipe pipe;
+
+	for_each_pipe(dev_priv, pipe) {
+		if (pipe_stats[pipe] & PIPE_START_VBLANK_INTERRUPT_STATUS)
+			i915_handle_vblank(dev_priv, pipe, pipe, iir);
+
+		if (pipe_stats[pipe] & PIPE_LEGACY_BLC_EVENT_STATUS)
+			blc_event = true;
+
+		if (pipe_stats[pipe] & PIPE_CRC_DONE_INTERRUPT_STATUS)
+			i9xx_pipe_crc_irq_handler(dev_priv, pipe);
+
+		if (pipe_stats[pipe] & PIPE_FIFO_UNDERRUN_STATUS)
+			intel_cpu_fifo_underrun_irq_handler(dev_priv, pipe);
+	}
+
+	if (blc_event || (iir & I915_ASLE_INTERRUPT))
+		intel_opregion_asle_intr(dev_priv);
+
+	if (pipe_stats[0] & PIPE_GMBUS_INTERRUPT_STATUS)
+		gmbus_irq_handler(dev_priv);
+}
+
 static void valleyview_pipestat_irq_handler(struct drm_i915_private *dev_priv,
 					    u32 pipe_stats[I915_MAX_PIPES])
 {
@@ -2017,7 +2093,7 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
 
 		/* Call regardless, as some status bits might not be
 		 * signalled in iir */
-		valleyview_pipestat_irq_ack(dev_priv, iir, pipe_stats);
+		i9xx_pipestat_irq_ack(dev_priv, iir, pipe_stats);
 
 		if (iir & (I915_LPE_PIPE_A_INTERRUPT |
 			   I915_LPE_PIPE_B_INTERRUPT))
@@ -2101,7 +2177,7 @@ static irqreturn_t cherryview_irq_handler(int irq, void *arg)
 
 		/* Call regardless, as some status bits might not be
 		 * signalled in iir */
-		valleyview_pipestat_irq_ack(dev_priv, iir, pipe_stats);
+		i9xx_pipestat_irq_ack(dev_priv, iir, pipe_stats);
 
 		if (iir & (I915_LPE_PIPE_A_INTERRUPT |
 			   I915_LPE_PIPE_B_INTERRUPT |
@@ -3685,8 +3761,6 @@ static irqreturn_t i8xx_irq_handler(int irq, void *arg)
 	struct drm_device *dev = arg;
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	u16 iir, new_iir;
-	u32 pipe_stats[2];
-	int pipe;
 	const u16 flip_mask =
 		I915_DISPLAY_PLANE_A_FLIP_PENDING_INTERRUPT |
 		I915_DISPLAY_PLANE_B_FLIP_PENDING_INTERRUPT;
@@ -3704,26 +3778,14 @@ static irqreturn_t i8xx_irq_handler(int irq, void *arg)
 		goto out;
 
 	while (iir & ~flip_mask) {
-		/* Can't rely on pipestat interrupt bit in iir as it might
-		 * have been cleared after the pipestat interrupt was received.
-		 * It doesn't set the bit in iir again, but it still produces
-		 * interrupts (for non-MSI).
-		 */
-		spin_lock(&dev_priv->irq_lock);
+		u32 pipe_stats[I915_MAX_PIPES] = {};
+
 		if (iir & I915_RENDER_COMMAND_PARSER_ERROR_INTERRUPT)
 			DRM_DEBUG("Command parser error, iir 0x%08x\n", iir);
 
-		for_each_pipe(dev_priv, pipe) {
-			i915_reg_t reg = PIPESTAT(pipe);
-			pipe_stats[pipe] = I915_READ(reg);
-
-			/*
-			 * Clear the PIPE*STAT regs before the IIR
-			 */
-			if (pipe_stats[pipe] & 0x8000ffff)
-				I915_WRITE(reg, pipe_stats[pipe]);
-		}
-		spin_unlock(&dev_priv->irq_lock);
+		/* Call regardless, as some status bits might not be
+		 * signalled in iir */
+		i9xx_pipestat_irq_ack(dev_priv, iir, pipe_stats);
 
 		I915_WRITE16(IIR, iir & ~flip_mask);
 		new_iir = I915_READ16(IIR); /* Flush posted writes */
@@ -3731,21 +3793,7 @@ static irqreturn_t i8xx_irq_handler(int irq, void *arg)
 		if (iir & I915_USER_INTERRUPT)
 			notify_ring(dev_priv->engine[RCS]);
 
-		for_each_pipe(dev_priv, pipe) {
-			int plane = pipe;
-			if (HAS_FBC(dev_priv))
-				plane = !plane;
-
-			if (pipe_stats[pipe] & PIPE_VBLANK_INTERRUPT_STATUS)
-				i8xx_handle_vblank(dev_priv, plane, pipe, iir);
-
-			if (pipe_stats[pipe] & PIPE_CRC_DONE_INTERRUPT_STATUS)
-				i9xx_pipe_crc_irq_handler(dev_priv, pipe);
-
-			if (pipe_stats[pipe] & PIPE_FIFO_UNDERRUN_STATUS)
-				intel_cpu_fifo_underrun_irq_handler(dev_priv,
-								    pipe);
-		}
+		i8xx_pipestat_irq_handler(dev_priv, iir, pipe_stats);
 
 		iir = new_iir;
 	}
@@ -3820,11 +3868,11 @@ static irqreturn_t i915_irq_handler(int irq, void *arg)
 {
 	struct drm_device *dev = arg;
 	struct drm_i915_private *dev_priv = to_i915(dev);
-	u32 iir, new_iir, pipe_stats[I915_MAX_PIPES];
+	u32 iir, new_iir;
 	const u32 flip_mask =
 		I915_DISPLAY_PLANE_A_FLIP_PENDING_INTERRUPT |
 		I915_DISPLAY_PLANE_B_FLIP_PENDING_INTERRUPT;
-	int pipe, ret = IRQ_NONE;
+	int ret = IRQ_NONE;
 
 	if (!intel_irqs_enabled(dev_priv))
 		return IRQ_NONE;
@@ -3834,29 +3882,15 @@ static irqreturn_t i915_irq_handler(int irq, void *arg)
 
 	iir = I915_READ(IIR);
 	do {
+		u32 pipe_stats[I915_MAX_PIPES] = {};
 		bool irq_received = (iir & ~flip_mask) != 0;
-		bool blc_event = false;
 
-		/* Can't rely on pipestat interrupt bit in iir as it might
-		 * have been cleared after the pipestat interrupt was received.
-		 * It doesn't set the bit in iir again, but it still produces
-		 * interrupts (for non-MSI).
-		 */
-		spin_lock(&dev_priv->irq_lock);
 		if (iir & I915_RENDER_COMMAND_PARSER_ERROR_INTERRUPT)
 			DRM_DEBUG("Command parser error, iir 0x%08x\n", iir);
 
-		for_each_pipe(dev_priv, pipe) {
-			i915_reg_t reg = PIPESTAT(pipe);
-			pipe_stats[pipe] = I915_READ(reg);
-
-			/* Clear the PIPE*STAT regs before the IIR */
-			if (pipe_stats[pipe] & 0x8000ffff) {
-				I915_WRITE(reg, pipe_stats[pipe]);
-				irq_received = true;
-			}
-		}
-		spin_unlock(&dev_priv->irq_lock);
+		/* Call regardless, as some status bits might not be
+		 * signalled in iir */
+		i9xx_pipestat_irq_ack(dev_priv, iir, pipe_stats);
 
 		if (!irq_received)
 			break;
@@ -3875,27 +3909,7 @@ static irqreturn_t i915_irq_handler(int irq, void *arg)
 		if (iir & I915_USER_INTERRUPT)
 			notify_ring(dev_priv->engine[RCS]);
 
-		for_each_pipe(dev_priv, pipe) {
-			int plane = pipe;
-			if (HAS_FBC(dev_priv))
-				plane = !plane;
-
-			if (pipe_stats[pipe] & PIPE_VBLANK_INTERRUPT_STATUS)
-				i915_handle_vblank(dev_priv, plane, pipe, iir);
-
-			if (pipe_stats[pipe] & PIPE_LEGACY_BLC_EVENT_STATUS)
-				blc_event = true;
-
-			if (pipe_stats[pipe] & PIPE_CRC_DONE_INTERRUPT_STATUS)
-				i9xx_pipe_crc_irq_handler(dev_priv, pipe);
-
-			if (pipe_stats[pipe] & PIPE_FIFO_UNDERRUN_STATUS)
-				intel_cpu_fifo_underrun_irq_handler(dev_priv,
-								    pipe);
-		}
-
-		if (blc_event || (iir & I915_ASLE_INTERRUPT))
-			intel_opregion_asle_intr(dev_priv);
+		i915_pipestat_irq_handler(dev_priv, iir, pipe_stats);
 
 		/* With MSI, interrupts are only generated when iir
 		 * transitions from zero to nonzero.  If another bit got
@@ -4022,8 +4036,7 @@ static irqreturn_t i965_irq_handler(int irq, void *arg)
 	struct drm_device *dev = arg;
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	u32 iir, new_iir;
-	u32 pipe_stats[I915_MAX_PIPES];
-	int ret = IRQ_NONE, pipe;
+	int ret = IRQ_NONE;
 	const u32 flip_mask =
 		I915_DISPLAY_PLANE_A_FLIP_PENDING_INTERRUPT |
 		I915_DISPLAY_PLANE_B_FLIP_PENDING_INTERRUPT;
@@ -4037,31 +4050,15 @@ static irqreturn_t i965_irq_handler(int irq, void *arg)
 	iir = I915_READ(IIR);
 
 	for (;;) {
+		u32 pipe_stats[I915_MAX_PIPES] = {};
 		bool irq_received = (iir & ~flip_mask) != 0;
-		bool blc_event = false;
 
-		/* Can't rely on pipestat interrupt bit in iir as it might
-		 * have been cleared after the pipestat interrupt was received.
-		 * It doesn't set the bit in iir again, but it still produces
-		 * interrupts (for non-MSI).
-		 */
-		spin_lock(&dev_priv->irq_lock);
 		if (iir & I915_RENDER_COMMAND_PARSER_ERROR_INTERRUPT)
 			DRM_DEBUG("Command parser error, iir 0x%08x\n", iir);
 
-		for_each_pipe(dev_priv, pipe) {
-			i915_reg_t reg = PIPESTAT(pipe);
-			pipe_stats[pipe] = I915_READ(reg);
-
-			/*
-			 * Clear the PIPE*STAT regs before the IIR
-			 */
-			if (pipe_stats[pipe] & 0x8000ffff) {
-				I915_WRITE(reg, pipe_stats[pipe]);
-				irq_received = true;
-			}
-		}
-		spin_unlock(&dev_priv->irq_lock);
+		/* Call regardless, as some status bits might not be
+		 * signalled in iir */
+		i9xx_pipestat_irq_ack(dev_priv, iir, pipe_stats);
 
 		if (!irq_received)
 			break;
@@ -4083,25 +4080,7 @@ static irqreturn_t i965_irq_handler(int irq, void *arg)
 		if (iir & I915_BSD_USER_INTERRUPT)
 			notify_ring(dev_priv->engine[VCS]);
 
-		for_each_pipe(dev_priv, pipe) {
-			if (pipe_stats[pipe] & PIPE_START_VBLANK_INTERRUPT_STATUS)
-				i915_handle_vblank(dev_priv, pipe, pipe, iir);
-
-			if (pipe_stats[pipe] & PIPE_LEGACY_BLC_EVENT_STATUS)
-				blc_event = true;
-
-			if (pipe_stats[pipe] & PIPE_CRC_DONE_INTERRUPT_STATUS)
-				i9xx_pipe_crc_irq_handler(dev_priv, pipe);
-
-			if (pipe_stats[pipe] & PIPE_FIFO_UNDERRUN_STATUS)
-				intel_cpu_fifo_underrun_irq_handler(dev_priv, pipe);
-		}
-
-		if (blc_event || (iir & I915_ASLE_INTERRUPT))
-			intel_opregion_asle_intr(dev_priv);
-
-		if (pipe_stats[0] & PIPE_GMBUS_INTERRUPT_STATUS)
-			gmbus_irq_handler(dev_priv);
+		i965_pipestat_irq_handler(dev_priv, iir, pipe_stats);
 
 		/* With MSI, interrupts are only generated when iir
 		 * transitions from zero to nonzero.  If another bit got
-- 
2.13.0

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

[PATCH 17/17] drm/i915: Rewrite GMCH irq handlers to follow the VLV/CHV pattern

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Eliminate the loops from the gen2-3 irq handlers by following the same
trick used for VLV/CHV, ie. clear IER around acking the interrupts.
That way if some IIR bits still remain set we'll get another edge (and
thus another CPU interrupt) when the IER gets restored.

This shouldn't really be necessary when level triggered PCI interrupts
are used (gen2, some gen3), but let's follow the same pattern in
all the handlers so that we don't have to worry about MSI being enabled
or not. And consistency should help avoid confusing the reader as well.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 196 +++++++++++++++++++---------------------
 1 file changed, 94 insertions(+), 102 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index f00e20902e1c..293609384b38 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -3760,11 +3760,7 @@ static irqreturn_t i8xx_irq_handler(int irq, void *arg)
 {
 	struct drm_device *dev = arg;
 	struct drm_i915_private *dev_priv = to_i915(dev);
-	u16 iir, new_iir;
-	const u16 flip_mask =
-		I915_DISPLAY_PLANE_A_FLIP_PENDING_INTERRUPT |
-		I915_DISPLAY_PLANE_B_FLIP_PENDING_INTERRUPT;
-	irqreturn_t ret;
+	irqreturn_t ret = IRQ_NONE;
 
 	if (!intel_irqs_enabled(dev_priv))
 		return IRQ_NONE;
@@ -3772,34 +3768,45 @@ static irqreturn_t i8xx_irq_handler(int irq, void *arg)
 	/* IRQs are synced during runtime_suspend, we don't require a wakeref */
 	disable_rpm_wakeref_asserts(dev_priv);
 
-	ret = IRQ_NONE;
-	iir = I915_READ16(IIR);
-	if (iir == 0)
-		goto out;
-
-	while (iir & ~flip_mask) {
+	do {
+		const u16 flip_mask =
+			I915_DISPLAY_PLANE_A_FLIP_PENDING_INTERRUPT |
+			I915_DISPLAY_PLANE_B_FLIP_PENDING_INTERRUPT;
 		u32 pipe_stats[I915_MAX_PIPES] = {};
+		u16 iir, ier;
 
-		if (iir & I915_RENDER_COMMAND_PARSER_ERROR_INTERRUPT)
-			DRM_DEBUG("Command parser error, iir 0x%08x\n", iir);
+		iir = I915_READ16(IIR);
+		if ((iir & ~flip_mask) == 0)
+			break;
+
+		ret = IRQ_HANDLED;
+
+		/*
+		 * Clear IER while we clear the IIR bits to make
+		 * sure we get another edge if not all IIR bits
+		 * end up cleared.
+		 */
+		ier = I915_READ16(IER);
+		I915_WRITE16(IER, 0);
 
 		/* Call regardless, as some status bits might not be
 		 * signalled in iir */
 		i9xx_pipestat_irq_ack(dev_priv, iir, pipe_stats);
 
-		I915_WRITE16(IIR, iir & ~flip_mask);
-		new_iir = I915_READ16(IIR); /* Flush posted writes */
+		I915_WRITE16(IIR, (iir & ~flip_mask));
+
+		I915_WRITE16(IER, ier);
+		POSTING_READ16(IER);
 
 		if (iir & I915_USER_INTERRUPT)
 			notify_ring(dev_priv->engine[RCS]);
 
-		i8xx_pipestat_irq_handler(dev_priv, iir, pipe_stats);
+		if (iir & I915_RENDER_COMMAND_PARSER_ERROR_INTERRUPT)
+			DRM_DEBUG("Command parser error, iir 0x%08x\n", iir);
 
-		iir = new_iir;
-	}
-	ret = IRQ_HANDLED;
+		i8xx_pipestat_irq_handler(dev_priv, iir, pipe_stats);
+	} while (0);
 
-out:
 	enable_rpm_wakeref_asserts(dev_priv);
 
 	return ret;
@@ -3868,11 +3875,7 @@ static irqreturn_t i915_irq_handler(int irq, void *arg)
 {
 	struct drm_device *dev = arg;
 	struct drm_i915_private *dev_priv = to_i915(dev);
-	u32 iir, new_iir;
-	const u32 flip_mask =
-		I915_DISPLAY_PLANE_A_FLIP_PENDING_INTERRUPT |
-		I915_DISPLAY_PLANE_B_FLIP_PENDING_INTERRUPT;
-	int ret = IRQ_NONE;
+	irqreturn_t ret = IRQ_NONE;
 
 	if (!intel_irqs_enabled(dev_priv))
 		return IRQ_NONE;
@@ -3880,55 +3883,52 @@ static irqreturn_t i915_irq_handler(int irq, void *arg)
 	/* IRQs are synced during runtime_suspend, we don't require a wakeref */
 	disable_rpm_wakeref_asserts(dev_priv);
 
-	iir = I915_READ(IIR);
 	do {
+		const u32 flip_mask =
+			I915_DISPLAY_PLANE_A_FLIP_PENDING_INTERRUPT |
+			I915_DISPLAY_PLANE_B_FLIP_PENDING_INTERRUPT;
 		u32 pipe_stats[I915_MAX_PIPES] = {};
-		bool irq_received = (iir & ~flip_mask) != 0;
+		u32 hotplug_status = 0;
+		u32 iir, ier;
 
-		if (iir & I915_RENDER_COMMAND_PARSER_ERROR_INTERRUPT)
-			DRM_DEBUG("Command parser error, iir 0x%08x\n", iir);
+		iir = I915_READ(IIR);
+		if ((iir & ~flip_mask) == 0)
+			break;
 
-		/* Call regardless, as some status bits might not be
-		 * signalled in iir */
-		i9xx_pipestat_irq_ack(dev_priv, iir, pipe_stats);
+		ret = IRQ_HANDLED;
 
-		if (!irq_received)
-			break;
+		/*
+		 * Clear IER while we clear the IIR bits to make
+		 * sure we get another edge if not all IIR bits
+		 * end up cleared.
+		 */
+		ier = I915_READ(IER);
+		I915_WRITE(IER, 0);
 
-		/* Consume port.  Then clear IIR or we'll miss events */
 		if (I915_HAS_HOTPLUG(dev_priv) &&
-		    iir & I915_DISPLAY_PORT_INTERRUPT) {
-			u32 hotplug_status = i9xx_hpd_irq_ack(dev_priv);
-			if (hotplug_status)
-				i9xx_hpd_irq_handler(dev_priv, hotplug_status);
-		}
+		    iir & I915_DISPLAY_PORT_INTERRUPT)
+			hotplug_status = i9xx_hpd_irq_ack(dev_priv);
+
+		/* Call regardless, as some status bits might not be
+		 * signalled in iir */
+		i9xx_pipestat_irq_ack(dev_priv, iir, pipe_stats);
 
 		I915_WRITE(IIR, iir & ~flip_mask);
-		new_iir = I915_READ(IIR); /* Flush posted writes */
+
+		I915_WRITE(IER, ier);
+		POSTING_READ(IER);
 
 		if (iir & I915_USER_INTERRUPT)
 			notify_ring(dev_priv->engine[RCS]);
 
-		i915_pipestat_irq_handler(dev_priv, iir, pipe_stats);
+		if (iir & I915_RENDER_COMMAND_PARSER_ERROR_INTERRUPT)
+			DRM_DEBUG("Command parser error, iir 0x%08x\n", iir);
 
-		/* With MSI, interrupts are only generated when iir
-		 * transitions from zero to nonzero.  If another bit got
-		 * set while we were handling the existing iir bits, then
-		 * we would never get another interrupt.
-		 *
-		 * This is fine on non-MSI as well, as if we hit this path
-		 * we avoid exiting the interrupt handler only to generate
-		 * another one.
-		 *
-		 * Note that for MSI this could cause a stray interrupt report
-		 * if an interrupt landed in the time between writing IIR and
-		 * the posting read.  This should be rare enough to never
-		 * trigger the 99% of 100,000 interrupts test for disabling
-		 * stray interrupts.
-		 */
-		ret = IRQ_HANDLED;
-		iir = new_iir;
-	} while (iir & ~flip_mask);
+		if (hotplug_status)
+			i9xx_hpd_irq_handler(dev_priv, hotplug_status);
+
+		i915_pipestat_irq_handler(dev_priv, iir, pipe_stats);
+	} while (0);
 
 	enable_rpm_wakeref_asserts(dev_priv);
 
@@ -4035,11 +4035,7 @@ static irqreturn_t i965_irq_handler(int irq, void *arg)
 {
 	struct drm_device *dev = arg;
 	struct drm_i915_private *dev_priv = to_i915(dev);
-	u32 iir, new_iir;
-	int ret = IRQ_NONE;
-	const u32 flip_mask =
-		I915_DISPLAY_PLANE_A_FLIP_PENDING_INTERRUPT |
-		I915_DISPLAY_PLANE_B_FLIP_PENDING_INTERRUPT;
+	irqreturn_t ret = IRQ_NONE;
 
 	if (!intel_irqs_enabled(dev_priv))
 		return IRQ_NONE;
@@ -4047,58 +4043,54 @@ static irqreturn_t i965_irq_handler(int irq, void *arg)
 	/* IRQs are synced during runtime_suspend, we don't require a wakeref */
 	disable_rpm_wakeref_asserts(dev_priv);
 
-	iir = I915_READ(IIR);
-
-	for (;;) {
+	do {
+		const u32 flip_mask =
+			I915_DISPLAY_PLANE_A_FLIP_PENDING_INTERRUPT |
+			I915_DISPLAY_PLANE_B_FLIP_PENDING_INTERRUPT;
 		u32 pipe_stats[I915_MAX_PIPES] = {};
-		bool irq_received = (iir & ~flip_mask) != 0;
-
-		if (iir & I915_RENDER_COMMAND_PARSER_ERROR_INTERRUPT)
-			DRM_DEBUG("Command parser error, iir 0x%08x\n", iir);
-
-		/* Call regardless, as some status bits might not be
-		 * signalled in iir */
-		i9xx_pipestat_irq_ack(dev_priv, iir, pipe_stats);
+		u32 hotplug_status = 0;
+		u32 iir, ier;
 
-		if (!irq_received)
+		iir = I915_READ(IIR);
+		if ((iir & ~flip_mask) == 0)
 			break;
 
 		ret = IRQ_HANDLED;
 
-		/* Consume port.  Then clear IIR or we'll miss events */
-		if (iir & I915_DISPLAY_PORT_INTERRUPT) {
-			u32 hotplug_status = i9xx_hpd_irq_ack(dev_priv);
-			if (hotplug_status)
-				i9xx_hpd_irq_handler(dev_priv, hotplug_status);
-		}
+		/*
+		 * Clear IER while we clear the IIR bits to make
+		 * sure we get another edge if not all IIR bits
+		 * end up cleared.
+		 */
+		ier = I915_READ(IER);
+		I915_WRITE(IER, 0);
+
+		if (iir & I915_DISPLAY_PORT_INTERRUPT)
+			hotplug_status = i9xx_hpd_irq_ack(dev_priv);
+
+		/* Call regardless, as some status bits might not be
+		 * signalled in iir */
+		i9xx_pipestat_irq_ack(dev_priv, iir, pipe_stats);
 
 		I915_WRITE(IIR, iir & ~flip_mask);
-		new_iir = I915_READ(IIR); /* Flush posted writes */
+
+		I915_WRITE(IER, ier);
+		POSTING_READ(IER);
 
 		if (iir & I915_USER_INTERRUPT)
 			notify_ring(dev_priv->engine[RCS]);
+
 		if (iir & I915_BSD_USER_INTERRUPT)
 			notify_ring(dev_priv->engine[VCS]);
 
-		i965_pipestat_irq_handler(dev_priv, iir, pipe_stats);
+		if (iir & I915_RENDER_COMMAND_PARSER_ERROR_INTERRUPT)
+			DRM_DEBUG("Command parser error, iir 0x%08x\n", iir);
 
-		/* With MSI, interrupts are only generated when iir
-		 * transitions from zero to nonzero.  If another bit got
-		 * set while we were handling the existing iir bits, then
-		 * we would never get another interrupt.
-		 *
-		 * This is fine on non-MSI as well, as if we hit this path
-		 * we avoid exiting the interrupt handler only to generate
-		 * another one.
-		 *
-		 * Note that for MSI this could cause a stray interrupt report
-		 * if an interrupt landed in the time between writing IIR and
-		 * the posting read.  This should be rare enough to never
-		 * trigger the 99% of 100,000 interrupts test for disabling
-		 * stray interrupts.
-		 */
-		iir = new_iir;
-	}
+		if (hotplug_status)
+			i9xx_hpd_irq_handler(dev_priv, hotplug_status);
+
+		i965_pipestat_irq_handler(dev_priv, iir, pipe_stats);
+	} while (0);
 
 	enable_rpm_wakeref_asserts(dev_priv);
 
-- 
2.13.0

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

Re: [PATCH 00/17] drm/i915: Redo old gmch irq handling

[Ville Syrjälä]

On Thu, Jun 22, 2017 at 02:55:38PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Apparently we have some issues [1] on g4x which smells like irqs not getting
> delivered after some point in time. The gen2-4 irq code is rather crusty

[1] https://bugs.freedesktop.org/show_bug.cgi?id=101261

> so I thought I'd bring it up to the same quality standards as the VLV/CHV
> irq code. And to avoid any chances of missing the edges I changed all the
> gmch platforms to use the "disable IER -> ack IIR -> enable IER" trick
> we use on VLV and CHV. That should be robust with both level and edge
> triggered interrupts, and single and double buffered IIR.
> 
> I think the only slightly scary bits are the ones touching HWSTAM
> programming. While that's not strictly needed for this series, I really
> wanted to remove a bunch of duplicat irq setup code, and for that
> I wanted to make the HWSTAM programming consistent. We don't actually
> use any of the interrupt information written into the status page,
> but I have slight concern that the extra status page writes may have
> had some unintended effect on seqno coherency. Fingers crossed...
> 
> There's potentially more unification we could do on the various gmch
> interrupts functions, but as the series was already ballooning out of
> control I decided not to pursue that angle very far.
> 
> I smoke tested this on 830, pnv, g4x, and ilk.
> 
> Series is available here:
> git://github.com/vsyrjala/linux.git gmch_irq_redo
> 
> Ville Syrjälä (17):
>   drm/i915: Clear pipestat consistently
>   drm/i915: s/GEN3/GEN5/
>   drm/i915: Use GEN3_IRQ_RESET/INIT on gen3/4
>   drm/i915: Introduce GEN2_IRQ_RESET/INIT
>   drm/i915: Setup EMR first on all gen2-4
>   drm/i915: Eliminate PORT_HOTPLUG_EN setup from gen3/4 irq_postinstall
>   drm/i915: Unify the appearance of gen3/4 irq_postistall hooks
>   drm/i915: Remove NULL dev_priv checks from irq_uninstall
>   drm/i915: Mask everything in ring HWSTAM on gen6+ in ringbuffer mode
>   drm/i915: Gen3 HWSTAM is actually 32 bits
>   drm/i915: Clean up the HWSTAM mess
>   drm/i915: Remove duplicated irq_preinstall/uninstall hooks
>   drm/i915: Consolidatte intel_check_page_flip() into
>     intel_pipe_handle_vblank()
>   drm/i915: Move the gen2-4 page flip handling code around
>   drm/i915: Simplify the gen2-4 flip_mask handling
>   drm/i915: Extract PIPESTAT irq handling into separate functions
>   drm/i915: Rewrite GMCH irq handlers to follow the VLV/CHV pattern
> 
>  drivers/gpu/drm/i915/i915_irq.c         | 909 ++++++++++++++------------------
>  drivers/gpu/drm/i915/intel_ringbuffer.c |   3 +
>  2 files changed, 394 insertions(+), 518 deletions(-)
> 
> -- 
> 2.13.0

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 11/17] drm/i915: Clean up the HWSTAM mess

[Chris Wilson]

Quoting ville.syrjala@linux.intel.com (2017-06-22 12:55:49)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Currently we're unmasking some random looking bits in HWSTAM
> on gen3/4/5. The two bits we apparently unmask are 0 and 12,
> and also bits 16-31 on gen4/5.
> What those bits do depends on the gen as follows:
>  bit 0: Breakpoint (gen2), ASLE (gen3), reserved (gen4), render user interrupt (gen5)
>  bit 12: Sync flush statusa (gen2-4), reserved (gen5)
>  bit 16-31: The ones that can unmasked seem to be mostly some
>             display stuff on gen4. Bit 18 is the PIPE_CONTROL notify,
>             which might be the only intresting one. On gen5 all the
>             bits are reserved.
> 
> So I don't know whether we actually depend on that status page write
> somehow. Extra seqno coherency by accident perhaps? Except we don't
> even unmask the user interrupt bit in HWSTAM except on gen5, and
> sync flush isn't something we use normally, so seems unlikely. So
> let's just assume we don't need any of this and mask everything in
> HWSTAM.

Once upon a time we were discussing that HWSTAM might give us better
seqno coherency. The conclusion for gen6, at least iirc, was that the
HWSTAM was unordered with the MI_STORE_DWORD.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915: Redo old gmch irq handling

[Patchwork]

== Series Details ==

Series: drm/i915: Redo old gmch irq handling
URL   : https://patchwork.freedesktop.org/series/26215/
State : success

== Summary ==

Series 26215v1 drm/i915: Redo old gmch irq handling
https://patchwork.freedesktop.org/api/1.0/series/26215/revisions/1/mbox/

Test gem_exec_suspend:
        Subgroup basic-s4-devices:
                dmesg-warn -> PASS       (fi-kbl-7560u) fdo#100125
Test kms_cursor_legacy:
        Subgroup basic-busy-flip-before-cursor-atomic:
                fail       -> PASS       (fi-snb-2600) fdo#100215

fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125
fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time:439s
fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time:425s
fi-bsw-n3050     total:278  pass:241  dwarn:1   dfail:0   fail:0   skip:36  time:525s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time:509s
fi-byt-j1900     total:278  pass:252  dwarn:2   dfail:0   fail:0   skip:24  time:494s
fi-byt-n2820     total:278  pass:248  dwarn:2   dfail:0   fail:0   skip:28  time:485s
fi-glk-2a        total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time:588s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:436s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:416s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time:415s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:501s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:475s
fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:467s
fi-kbl-7560u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:573s
fi-kbl-r         total:278  pass:259  dwarn:1   dfail:0   fail:0   skip:18  time:581s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:461s
fi-skl-6700hq    total:278  pass:220  dwarn:3   dfail:0   fail:30  skip:24  time:344s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time:467s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:473s
fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time:443s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:542s
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time:399s

7f2c92652872fbbe37c8bca92eccb7218100c21d drm-tip: 2017y-06m-22d-08h-47m-32s UTC integration manifest
6d2871d drm/i915: Rewrite GMCH irq handlers to follow the VLV/CHV pattern
4d61345 drm/i915: Extract PIPESTAT irq handling into separate functions
b5c787b drm/i915: Simplify the gen2-4 flip_mask handling
e0c969f drm/i915: Move the gen2-4 page flip handling code around
1c3f8b5 drm/i915: Consolidatte intel_check_page_flip() into intel_pipe_handle_vblank()
3db6bfb drm/i915: Remove duplicated irq_preinstall/uninstall hooks
6f1030f drm/i915: Clean up the HWSTAM mess
b97c7fc drm/i915: Gen3 HWSTAM is actually 32 bits
c0e71ec drm/i915: Mask everything in ring HWSTAM on gen6+ in ringbuffer mode
32c57c3 drm/i915: Remove NULL dev_priv checks from irq_uninstall
5f33f43f drm/i915: Unify the appearance of gen3/4 irq_postistall hooks
53fb93a drm/i915: Eliminate PORT_HOTPLUG_EN setup from gen3/4 irq_postinstall
4c6b1f1 drm/i915: Setup EMR first on all gen2-4
c5892b4 drm/i915: Introduce GEN2_IRQ_RESET/INIT
0885995 drm/i915: Use GEN3_IRQ_RESET/INIT on gen3/4
14187b2 drm/i915: s/GEN3/GEN5/
8eb1a91 drm/i915: Clear pipestat consistently

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_5025/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 01/17] drm/i915: Clear pipestat consistently

[Chris Wilson]

Quoting ville.syrjala@linux.intel.com (2017-06-22 12:55:39)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> We have a lot of different ways of clearing the PIPESTAT registers.
> Let's unify it all into one function. There's no magic in PIPESTAT
> that would require any of the double clearing and whatnot that
> some of the code tries to do. All we can really do is clear the status
> bits and disable the enable bits. There is no way to mask anything
> so as soon as another event happens the status bit will become set
> again, and trying to clear them twice or something can't protect
> against that.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_irq.c | 67 ++++++++++++++++++-----------------------
>  1 file changed, 30 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index b1c7d1a04612..6daaf47482d4 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1732,6 +1732,19 @@ static bool intel_pipe_handle_vblank(struct drm_i915_private *dev_priv,
>         return ret;
>  }
>  
> +static void i9xx_pipestat_irq_reset(struct drm_i915_private *dev_priv)
> +{
> +       enum pipe pipe;
> +
> +       for_each_pipe(dev_priv, pipe) {
> +               I915_WRITE(PIPESTAT(pipe),
> +                          PIPESTAT_INT_STATUS_MASK |
> +                          PIPE_FIFO_UNDERRUN_STATUS);

Hmm, is this change for i915/i965 significant? Maybe explain it away in
the changelog?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 02/17] drm/i915: s/GEN3/GEN5/

[Chris Wilson]

Quoting ville.syrjala@linux.intel.com (2017-06-22 12:55:40)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The GEN5_IRQ_RESET/INIT macros are perfectly suitable even for
> gen3/4 hardware as those have 32 bit interrupt registers. Let's
> rename the macros to reflect that fact.
> 
> Gen2 on the other hand has 16 bit interrupt registers so these
> macros aren't really appropriate there.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 04/17] drm/i915: Introduce GEN2_IRQ_RESET/INIT

[Chris Wilson]

Quoting ville.syrjala@linux.intel.com (2017-06-22 12:55:42)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Unify the appearance of the gen2 irq code with the gen3+ code by
> introducing the GEN2_IRQ_RESET/INIT macros.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 05/17] drm/i915: Setup EMR first on all gen2-4

[Chris Wilson]

Quoting ville.syrjala@linux.intel.com (2017-06-22 12:55:43)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Unify the appaerance of the gen2-4 irq postinstall hooks a little
> bit by doing the EMR setup first on all the platforms.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 06/17] drm/i915: Eliminate PORT_HOTPLUG_EN setup from gen3/4 irq_postinstall

[Chris Wilson]

Quoting ville.syrjala@linux.intel.com (2017-06-22 12:55:44)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> We've already cleared PORT_HOTPLUG_EN in the .irq_preinstall hook
> so doing it again in the .irq_postinstall is pointless.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 07/17] drm/i915: Unify the appearance of gen3/4 irq_postistall hooks

[Chris Wilson]

Quoting ville.syrjala@linux.intel.com (2017-06-22 12:55:45)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Do the irq_mask/enable_mask setup in the same way on gen3/4, and also
> reorder the steps to make the code more uniform.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 08/17] drm/i915: Remove NULL dev_priv checks from irq_uninstall

[Chris Wilson]

Quoting ville.syrjala@linux.intel.com (2017-06-22 12:55:46)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> There should be no way to land in irq_uninstall without a
> valid dev_priv. Let's kill off the remaining checks, which are
> probably some kind of UMS leftovers. Not all the irq_uninstall
> hooks even had them anymore.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 10/17] drm/i915: Gen3 HWSTAM is actually 32 bits

[Chris Wilson]

Quoting ville.syrjala@linux.intel.com (2017-06-22 12:55:48)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Bspec claims that HWSTAM is only 16 bits on gen3, but the other
> interrupts registers are 32 bits and there are 18 valid interrupt
> bits. Hence a 16 bit HWSTAM wouldn't be able to contain all the
> bits, so it seems the spec is incorrect about the size of the
> register. And indeed I can clear bits 16 and 17 just fine with
> a 32 bit write. So let's adjust the code to treat the register
> as 32 bits.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 12/17] drm/i915: Remove duplicated irq_preinstall/uninstall hooks

[Chris Wilson]

Quoting ville.syrjala@linux.intel.com (2017-06-22 12:55:50)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> All the irq_preinstall and irq_uninstall hooks are now identical. Let's
> just rename them all the irq_reset and remove the pointless duplicates.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Lgtm, but I haven't quite come to terms with HWSTAM, but anyway
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 13/17] drm/i915: Consolidatte intel_check_page_flip() into intel_pipe_handle_vblank()

[Chris Wilson]

Quoting ville.syrjala@linux.intel.com (2017-06-22 12:55:51)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Almost all callers of intel_pipe_handle_vblank() immediately call
> intel_check_page_flip() depending on the return value. Let's move
> the call into the function itself.

Bleugh, dead code (intel_check_page_flip, we don't even notice a pageflip
miss and waitboost anymore).

> We'll just neeed to deal with the old gmch "flip pending" stuff
> in a slightly different way.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 14/17] drm/i915: Move the gen2-4 page flip handling code around

[Chris Wilson]

Quoting ville.syrjala@linux.intel.com (2017-06-22 12:55:52)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Move i8xx_handle_vblank() and i915_handle_vblank() to an earlier
> location so that we can later collect all the PIPESTAT irq handling
> code next to the VLV/CHV PIPESTAT handling code.
> 
> While at it s/u32 iir/u16 iir/ for i8xx_handle_vblank() since the
> IIR register is only 16 bits on gen2.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 15/17] drm/i915: Simplify the gen2-4 flip_mask handling

[Chris Wilson]

Quoting ville.syrjala@linux.intel.com (2017-06-22 12:55:53)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Replace the complicated "loop multiple times over IIR with different
> flip_mask" logic with just clearing the relevant bit from IIR when
> we actually handle the interrupt. This results in potentially an extra
> IIR write, but so what.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Ok then,
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 16/17] drm/i915: Extract PIPESTAT irq handling into separate functions

[Chris Wilson]

Quoting ville.syrjala@linux.intel.com (2017-06-22 12:55:54)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Extract the gen2-4 PIPESTAT irq handling into separate functions just
> like we already do on VLV/CHV.
> 
> We can share valleyview_pipestat_irq_ack() on all gmch platforms to
> actually read and clear the PIPESTAT status bits, so let's rename
> it to i9xx_pipestat_irq_ack().
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

The code reduction is worth it.
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Just reminds me that I still want to reduce the workload on chv where we
check for display underruns after every batch.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 17/17] drm/i915: Rewrite GMCH irq handlers to follow the VLV/CHV pattern

[Chris Wilson]

Quoting ville.syrjala@linux.intel.com (2017-06-22 12:55:55)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Eliminate the loops from the gen2-3 irq handlers by following the same
> trick used for VLV/CHV, ie. clear IER around acking the interrupts.
> That way if some IIR bits still remain set we'll get another edge (and
> thus another CPU interrupt) when the IER gets restored.
> 
> This shouldn't really be necessary when level triggered PCI interrupts
> are used (gen2, some gen3), but let's follow the same pattern in
> all the handlers so that we don't have to worry about MSI being enabled
> or not. And consistency should help avoid confusing the reader as well.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Having a common approach that just works is definitely worth it.
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Interrupts aren't so much of a concern for me for pre-snb, but every
mmio read prior to waking up a waiter should be justified :) Before
ilk, we have to run the entire interrupt handler before userspace can
proceed. Just something to bear in mind, and prune as much as possible.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 00/17] drm/i915: Redo old gmch irq handling

[Chris Wilson]

Quoting ville.syrjala@linux.intel.com (2017-06-22 12:55:38)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Apparently we have some issues [1] on g4x which smells like irqs not getting
> delivered after some point in time. The gen2-4 irq code is rather crusty
> so I thought I'd bring it up to the same quality standards as the VLV/CHV
> irq code. And to avoid any chances of missing the edges I changed all the
> gmch platforms to use the "disable IER -> ack IIR -> enable IER" trick
> we use on VLV and CHV. That should be robust with both level and edge
> triggered interrupts, and single and double buffered IIR.
> 
> I think the only slightly scary bits are the ones touching HWSTAM
> programming. While that's not strictly needed for this series, I really
> wanted to remove a bunch of duplicat irq setup code, and for that
> I wanted to make the HWSTAM programming consistent. We don't actually
> use any of the interrupt information written into the status page,
> but I have slight concern that the extra status page writes may have
> had some unintended effect on seqno coherency. Fingers crossed...

Yeah, I just want to ponder HWSTAM somewhat. One issue is that we have
never taken advantage of it to avoid the uncached mmio read. But
changing it may indeed uncover old coherency issues...
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 17/17] drm/i915: Rewrite GMCH irq handlers to follow the VLV/CHV pattern

[Ville Syrjälä]

On Thu, Jun 22, 2017 at 02:00:49PM +0100, Chris Wilson wrote:
> Quoting ville.syrjala@linux.intel.com (2017-06-22 12:55:55)
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Eliminate the loops from the gen2-3 irq handlers by following the same
> > trick used for VLV/CHV, ie. clear IER around acking the interrupts.
> > That way if some IIR bits still remain set we'll get another edge (and
> > thus another CPU interrupt) when the IER gets restored.
> > 
> > This shouldn't really be necessary when level triggered PCI interrupts
> > are used (gen2, some gen3), but let's follow the same pattern in
> > all the handlers so that we don't have to worry about MSI being enabled
> > or not. And consistency should help avoid confusing the reader as well.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Having a common approach that just works is definitely worth it.
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> Interrupts aren't so much of a concern for me for pre-snb, but every
> mmio read prior to waking up a waiter should be justified :) Before
> ilk, we have to run the entire interrupt handler before userspace can
> proceed. Just something to bear in mind, and prune as much as possible.

I guess we could nuke the IER read at least by storing the value
somewhere. I believe you actually suggested that when I redid the
VLV/CHV code. And I guess ripping out all POSTING_READs could be
a sane thing to do as well.

As for the underrun checks, I guess we could limit those to just happen
when some other pipe interrupt happens. While we're flipping or
reconfiguring planes we should have vblanks enabled anyway, so
we should still be able to catch the most glaring watermark failures
at least. But catching more subtle underruns caused by something like
memory self refresh or perhaps starvation due to heavy GPU memory
access etc. might go unnoticed then.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 00/17] drm/i915: Redo old gmch irq handling

[Ville Syrjälä]

On Thu, Jun 22, 2017 at 02:02:39PM +0100, Chris Wilson wrote:
> Quoting ville.syrjala@linux.intel.com (2017-06-22 12:55:38)
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Apparently we have some issues [1] on g4x which smells like irqs not getting
> > delivered after some point in time. The gen2-4 irq code is rather crusty
> > so I thought I'd bring it up to the same quality standards as the VLV/CHV
> > irq code. And to avoid any chances of missing the edges I changed all the
> > gmch platforms to use the "disable IER -> ack IIR -> enable IER" trick
> > we use on VLV and CHV. That should be robust with both level and edge
> > triggered interrupts, and single and double buffered IIR.
> > 
> > I think the only slightly scary bits are the ones touching HWSTAM
> > programming. While that's not strictly needed for this series, I really
> > wanted to remove a bunch of duplicat irq setup code, and for that
> > I wanted to make the HWSTAM programming consistent. We don't actually
> > use any of the interrupt information written into the status page,
> > but I have slight concern that the extra status page writes may have
> > had some unintended effect on seqno coherency. Fingers crossed...
> 
> Yeah, I just want to ponder HWSTAM somewhat. One issue is that we have
> never taken advantage of it to avoid the uncached mmio read. But
> changing it may indeed uncover old coherency issues...

If I decoded the bits correctly, then I think that danger would only
apply to ilk since that's the only platform where we unmask the user
interrupt in HWSTAM (and just the render user interrupt, the bsd one
remains always masked).

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 02/17] drm/i915: s/GEN3/GEN5/

[Maarten Lankhorst]

Op 22-06-17 om 14:40 schreef Chris Wilson:
> Quoting ville.syrjala@linux.intel.com (2017-06-22 12:55:40)
>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>
>> The GEN5_IRQ_RESET/INIT macros are perfectly suitable even for
>> gen3/4 hardware as those have 32 bit interrupt registers. Let's
>> rename the macros to reflect that fact.
>>
>> Gen2 on the other hand has 16 bit interrupt registers so these
>> macros aren't really appropriate there.
>>
>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> -Chris
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Pretty sure it's s/GEN5/GEN3/ :-)

With that fixed

Acked-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

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

Re: [PATCH 01/17] drm/i915: Clear pipestat consistently

[Ville Syrjälä]

On Thu, Jun 22, 2017 at 01:39:47PM +0100, Chris Wilson wrote:
> Quoting ville.syrjala@linux.intel.com (2017-06-22 12:55:39)
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > We have a lot of different ways of clearing the PIPESTAT registers.
> > Let's unify it all into one function. There's no magic in PIPESTAT
> > that would require any of the double clearing and whatnot that
> > some of the code tries to do. All we can really do is clear the status
> > bits and disable the enable bits. There is no way to mask anything
> > so as soon as another event happens the status bit will become set
> > again, and trying to clear them twice or something can't protect
> > against that.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_irq.c | 67 ++++++++++++++++++-----------------------
> >  1 file changed, 30 insertions(+), 37 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > index b1c7d1a04612..6daaf47482d4 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -1732,6 +1732,19 @@ static bool intel_pipe_handle_vblank(struct drm_i915_private *dev_priv,
> >         return ret;
> >  }
> >  
> > +static void i9xx_pipestat_irq_reset(struct drm_i915_private *dev_priv)
> > +{
> > +       enum pipe pipe;
> > +
> > +       for_each_pipe(dev_priv, pipe) {
> > +               I915_WRITE(PIPESTAT(pipe),
> > +                          PIPESTAT_INT_STATUS_MASK |
> > +                          PIPE_FIFO_UNDERRUN_STATUS);
> 
> Hmm, is this change for i915/i965 significant? Maybe explain it away in
> the changelog?

Sorry missed your question. Which change are we concerned about here
specifically?

As the commit message tried to explain, the "first disable then clear"
thing at least is pointless, and we can just do it in one step.
But maybe that wasn't the part you're thinking about?

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 01/17] drm/i915: Clear pipestat consistently

[Chris Wilson]

Quoting Ville Syrjälä (2017-06-30 12:34:15)
> On Thu, Jun 22, 2017 at 01:39:47PM +0100, Chris Wilson wrote:
> > Quoting ville.syrjala@linux.intel.com (2017-06-22 12:55:39)
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > We have a lot of different ways of clearing the PIPESTAT registers.
> > > Let's unify it all into one function. There's no magic in PIPESTAT
> > > that would require any of the double clearing and whatnot that
> > > some of the code tries to do. All we can really do is clear the status
> > > bits and disable the enable bits. There is no way to mask anything
> > > so as soon as another event happens the status bit will become set
> > > again, and trying to clear them twice or something can't protect
> > > against that.
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_irq.c | 67 ++++++++++++++++++-----------------------
> > >  1 file changed, 30 insertions(+), 37 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > > index b1c7d1a04612..6daaf47482d4 100644
> > > --- a/drivers/gpu/drm/i915/i915_irq.c
> > > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > > @@ -1732,6 +1732,19 @@ static bool intel_pipe_handle_vblank(struct drm_i915_private *dev_priv,
> > >         return ret;
> > >  }
> > >  
> > > +static void i9xx_pipestat_irq_reset(struct drm_i915_private *dev_priv)
> > > +{
> > > +       enum pipe pipe;
> > > +
> > > +       for_each_pipe(dev_priv, pipe) {
> > > +               I915_WRITE(PIPESTAT(pipe),
> > > +                          PIPESTAT_INT_STATUS_MASK |
> > > +                          PIPE_FIFO_UNDERRUN_STATUS);
> > 
> > Hmm, is this change for i915/i965 significant? Maybe explain it away in
> > the changelog?
> 
> Sorry missed your question. Which change are we concerned about here
> specifically?

We didn't set PIPESTAT_INT_STATUS_MASK | PIPE_FIFO_UNDERRUN_STATUS
previously afaics.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 01/17] drm/i915: Clear pipestat consistently

[Ville Syrjälä]

On Fri, Jun 30, 2017 at 12:41:53PM +0100, Chris Wilson wrote:
> Quoting Ville Syrjälä (2017-06-30 12:34:15)
> > On Thu, Jun 22, 2017 at 01:39:47PM +0100, Chris Wilson wrote:
> > > Quoting ville.syrjala@linux.intel.com (2017-06-22 12:55:39)
> > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > 
> > > > We have a lot of different ways of clearing the PIPESTAT registers.
> > > > Let's unify it all into one function. There's no magic in PIPESTAT
> > > > that would require any of the double clearing and whatnot that
> > > > some of the code tries to do. All we can really do is clear the status
> > > > bits and disable the enable bits. There is no way to mask anything
> > > > so as soon as another event happens the status bit will become set
> > > > again, and trying to clear them twice or something can't protect
> > > > against that.
> > > > 
> > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/i915_irq.c | 67 ++++++++++++++++++-----------------------
> > > >  1 file changed, 30 insertions(+), 37 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > > > index b1c7d1a04612..6daaf47482d4 100644
> > > > --- a/drivers/gpu/drm/i915/i915_irq.c
> > > > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > > > @@ -1732,6 +1732,19 @@ static bool intel_pipe_handle_vblank(struct drm_i915_private *dev_priv,
> > > >         return ret;
> > > >  }
> > > >  
> > > > +static void i9xx_pipestat_irq_reset(struct drm_i915_private *dev_priv)
> > > > +{
> > > > +       enum pipe pipe;
> > > > +
> > > > +       for_each_pipe(dev_priv, pipe) {
> > > > +               I915_WRITE(PIPESTAT(pipe),
> > > > +                          PIPESTAT_INT_STATUS_MASK |
> > > > +                          PIPE_FIFO_UNDERRUN_STATUS);
> > > 
> > > Hmm, is this change for i915/i965 significant? Maybe explain it away in
> > > the changelog?
> > 
> > Sorry missed your question. Which change are we concerned about here
> > specifically?
> 
> We didn't set PIPESTAT_INT_STATUS_MASK | PIPE_FIFO_UNDERRUN_STATUS
> previously afaics.

Ah. Those are the sticky status bits, so the earlier
I915_WRITE(PIPESTAT, I915_READ(PIPESTAT)) did the same thing
effectively.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx