[PATCH 00/12] drm/i915: VLV/CHV irq handler fixes

[PATCH 00/12] drm/i915: VLV/CHV irq handler fixes

[ville.syrjala]

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

The main motivation here is to fix CHV irq handling which got a bit broken
by the removal of the loop. The loop is still gone after I'm done, and
I also killed the loop from the VLV code. That's roughly the first half
of the series.

The second half of the series goes a bit further and splits up the VLV/CHV
irq handlers into clear ack and handle phases. One idea I had long ago that
we could try to move the handling part into a thread, leaving only the ack
part in hard irq context. But this series doesn't go quite that far. Also
I only did the split for VLV/CHV, but if we go ahead with it, we should do
it for all platforms. It's mostly mechanical work anyway.

The whole series is available here:
git://github.com/vsyrjala/linux.git vlv_chv_irq_fixes

Ville Syrjälä (12):
  drm/i915: Use GEN8_MASTER_IRQ_CONTROL consistently
  drm/i915: Set up VLV_MASTER_IER consistently
  drm/i915: Clear VLV_IIR after PIPESTAT
  drm/i915: Clear VLV_MASTER_IER around irq processing
  drm/i915: Clear VLV_IER around irq processing
  drm/i915: Eliminate loop from VLV irq handler
  drm/i915: Move variables to narrower scope in VLV/CHV irq handlers
  drm/i915: Split PORT_HOTPLUG_STAT ack out from i9xx_hpd_irq_handler()
  drm/i915: Split VLV/CVH PIPESTAT handling into ack+handler
  drm/i915: Move gt/pm irq handling out from irq disabled section on VLV
  drm/i915: Eliminate passing dev+dev_priv to {snb,ilk}_gt_irq_handler()
  drm/i915: Split gen8_gt_irq_handler into ack+handle

 drivers/gpu/drm/i915/i915_irq.c | 280 ++++++++++++++++++++++++++--------------
 1 file changed, 183 insertions(+), 97 deletions(-)

-- 
2.7.4

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

[PATCH 01/12] drm/i915: Use GEN8_MASTER_IRQ_CONTROL consistently

[ville.syrjala]

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

Use GEN8_MASTER_IRQ_CONTROL instead of DE_MASTER_IRQ_CONTROL or
MASTER_INTERRUPT_ENABLE with the GEN8_MASTER_IRQ register. They're
all bit 31 so there's no actual bug here, but let's be consistent
which name we use for the bit.

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

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 247d962afabb..165cb4ecb0ad 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1855,7 +1855,7 @@ static irqreturn_t cherryview_irq_handler(int irq, void *arg)
 		 * signalled in iir */
 		valleyview_pipestat_irq_handler(dev, iir);
 
-		I915_WRITE(GEN8_MASTER_IRQ, DE_MASTER_IRQ_CONTROL);
+		I915_WRITE(GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL);
 		POSTING_READ(GEN8_MASTER_IRQ);
 	} while (0);
 
@@ -3813,7 +3813,7 @@ static int gen8_irq_postinstall(struct drm_device *dev)
 	if (HAS_PCH_SPLIT(dev))
 		ibx_irq_postinstall(dev);
 
-	I915_WRITE(GEN8_MASTER_IRQ, DE_MASTER_IRQ_CONTROL);
+	I915_WRITE(GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL);
 	POSTING_READ(GEN8_MASTER_IRQ);
 
 	return 0;
@@ -3830,7 +3830,7 @@ static int cherryview_irq_postinstall(struct drm_device *dev)
 		vlv_display_irq_postinstall(dev_priv);
 	spin_unlock_irq(&dev_priv->irq_lock);
 
-	I915_WRITE(GEN8_MASTER_IRQ, MASTER_INTERRUPT_ENABLE);
+	I915_WRITE(GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL);
 	POSTING_READ(GEN8_MASTER_IRQ);
 
 	return 0;
-- 
2.7.4

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

[PATCH 02/12] drm/i915: Set up VLV_MASTER_IER consistently

[ville.syrjala]

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

We're lacking VLV_MASTER_IER setup from valleyview_irq_preinstall(), so
add it there. Also cargo cult in some POSTING_READ()s to match the other
platforms.

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

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 165cb4ecb0ad..f96a374ea624 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -3355,6 +3355,9 @@ static void valleyview_irq_preinstall(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
+	I915_WRITE(VLV_MASTER_IER, 0);
+	POSTING_READ(VLV_MASTER_IER);
+
 	gen5_gt_irq_reset(dev);
 
 	spin_lock_irq(&dev_priv->irq_lock);
@@ -3724,6 +3727,7 @@ static int valleyview_irq_postinstall(struct drm_device *dev)
 	spin_unlock_irq(&dev_priv->irq_lock);
 
 	I915_WRITE(VLV_MASTER_IER, MASTER_INTERRUPT_ENABLE);
+	POSTING_READ(VLV_MASTER_IER);
 
 	return 0;
 }
@@ -3854,6 +3858,7 @@ static void valleyview_irq_uninstall(struct drm_device *dev)
 		return;
 
 	I915_WRITE(VLV_MASTER_IER, 0);
+	POSTING_READ(VLV_MASTER_IER);
 
 	gen5_gt_irq_reset(dev);
 
-- 
2.7.4

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

[PATCH 03/12] drm/i915: Clear VLV_IIR after PIPESTAT

[ville.syrjala]

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

On VLV/CHV VLV_IIR is not double double buffered, and it doesn't detect
edges from PIPESTAT & co. like it does on gen4. Instead it just
directly latches the level from PIPESTAT & co. That means we must clear
VLV_IIR after PIPESTAT & co. or else we'll get a spurious bit in VLV_IIR
every single time.

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

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index f96a374ea624..52ccb4af5e18 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1789,12 +1789,6 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
 			I915_WRITE(GEN6_PMIIR, pm_iir);
 
 		iir = I915_READ(VLV_IIR);
-		if (iir) {
-			/* Consume port before clearing IIR or we'll miss events */
-			if (iir & I915_DISPLAY_PORT_INTERRUPT)
-				i9xx_hpd_irq_handler(dev);
-			I915_WRITE(VLV_IIR, iir);
-		}
 
 		if (gt_iir == 0 && pm_iir == 0 && iir == 0)
 			goto out;
@@ -1805,9 +1799,20 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
 			snb_gt_irq_handler(dev, dev_priv, gt_iir);
 		if (pm_iir)
 			gen6_rps_irq_handler(dev_priv, pm_iir);
+
+		if (iir & I915_DISPLAY_PORT_INTERRUPT)
+			i9xx_hpd_irq_handler(dev);
+
 		/* Call regardless, as some status bits might not be
 		 * signalled in iir */
 		valleyview_pipestat_irq_handler(dev, iir);
+
+		/*
+		 * VLV_IIR is single buffered, and reflects the level
+		 * from PIPESTAT/PORT_HOTPLUG_STAT, hence clear it last.
+		 */
+		if (iir)
+			I915_WRITE(VLV_IIR, iir);
 	}
 
 out:
@@ -1840,21 +1845,22 @@ static irqreturn_t cherryview_irq_handler(int irq, void *arg)
 
 		I915_WRITE(GEN8_MASTER_IRQ, 0);
 
-		/* Find, clear, then process each source of interrupt */
-
-		if (iir) {
-			/* Consume port before clearing IIR or we'll miss events */
-			if (iir & I915_DISPLAY_PORT_INTERRUPT)
-				i9xx_hpd_irq_handler(dev);
-			I915_WRITE(VLV_IIR, iir);
-		}
-
 		gen8_gt_irq_handler(dev_priv, master_ctl);
 
+		if (iir & I915_DISPLAY_PORT_INTERRUPT)
+			i9xx_hpd_irq_handler(dev);
+
 		/* Call regardless, as some status bits might not be
 		 * signalled in iir */
 		valleyview_pipestat_irq_handler(dev, iir);
 
+		/*
+		 * VLV_IIR is single buffered, and reflects the level
+		 * from PIPESTAT/PORT_HOTPLUG_STAT, hence clear it last.
+		 */
+		if (iir)
+			I915_WRITE(VLV_IIR, iir);
+
 		I915_WRITE(GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL);
 		POSTING_READ(GEN8_MASTER_IRQ);
 	} while (0);
-- 
2.7.4

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

[PATCH 04/12] drm/i915: Clear VLV_MASTER_IER around irq processing

[ville.syrjala]

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

Like on CHV, let's clear out the master irq enable bit when we ack
GT/PM interrupts. This will allow GT/PM interrupts to re-raise the
CPU interrupt if we fail to clear all the bits from the IIR(s).

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

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 52ccb4af5e18..626775039919 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1781,13 +1781,7 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
 		/* Find, clear, then process each source of interrupt */
 
 		gt_iir = I915_READ(GTIIR);
-		if (gt_iir)
-			I915_WRITE(GTIIR, gt_iir);
-
 		pm_iir = I915_READ(GEN6_PMIIR);
-		if (pm_iir)
-			I915_WRITE(GEN6_PMIIR, pm_iir);
-
 		iir = I915_READ(VLV_IIR);
 
 		if (gt_iir == 0 && pm_iir == 0 && iir == 0)
@@ -1795,6 +1789,13 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
 
 		ret = IRQ_HANDLED;
 
+		I915_WRITE(VLV_MASTER_IER, 0);
+
+		if (gt_iir)
+			I915_WRITE(GTIIR, gt_iir);
+		if (pm_iir)
+			I915_WRITE(GEN6_PMIIR, pm_iir);
+
 		if (gt_iir)
 			snb_gt_irq_handler(dev, dev_priv, gt_iir);
 		if (pm_iir)
@@ -1813,6 +1814,9 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
 		 */
 		if (iir)
 			I915_WRITE(VLV_IIR, iir);
+
+		I915_WRITE(VLV_MASTER_IER, MASTER_INTERRUPT_ENABLE);
+		POSTING_READ(VLV_MASTER_IER);
 	}
 
 out:
-- 
2.7.4

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

[PATCH 05/12] drm/i915: Clear VLV_IER around irq processing

[ville.syrjala]

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

On VLV/CHV the master interrupt enable bit only affects GT/PM
interrupts. Display interrupts are not affected by the master
irq control.

Also it seems that the CPU interrupt will only be generated when
the combined result of all GT/PM/display interrupts has a 0->1
edge. We already use the master interrupt enable bit to make sure
GT/PM interrupt can generate such an edge if we don't end up clearing
all IIR bits. We must do the same for display interrupts, and for
that we can simply clear out VLV_IER, and restore after we've acked
all the interrupts we are about to process.

So with both master interrupt enable and VLV_IER cleared out, we will
guarantee that there will be a 0->1 edge if any IIR bits are still set
at the end, and thus another CPU interrupt will be generated.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Fixes: 579de73b048a ("drm/i915: Exit cherryview_irq_handler() after one pass")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 36 +++++++++++++++++++++++++++++++++++-
 1 file changed, 35 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 626775039919..46be03c616f4 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1778,7 +1778,7 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
 	disable_rpm_wakeref_asserts(dev_priv);
 
 	while (true) {
-		/* Find, clear, then process each source of interrupt */
+		u32 ier = 0;
 
 		gt_iir = I915_READ(GTIIR);
 		pm_iir = I915_READ(GEN6_PMIIR);
@@ -1789,7 +1789,22 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
 
 		ret = IRQ_HANDLED;
 
+		/*
+		 * Theory on interrupt generation, based on empirical evidence:
+		 *
+		 * x = ((VLV_IIR & VLV_IER) ||
+		 *      (((GT_IIR & GT_IER) || (GEN6_PMIIR & GEN6_PMIER)) &&
+		 *       (VLV_MASTER_IER & MASTER_INTERRUPT_ENABLE)));
+		 *
+		 * A CPU interrupt will only be raised when 'x' has a 0->1 edge.
+		 * Hence we clear MASTER_INTERRUPT_ENABLE and VLV_IER to
+		 * guarantee the CPU interrupt will be raised again even if we
+		 * don't end up clearing all the VLV_IIR, GT_IIR, GEN6_PMIIR
+		 * bits this time around.
+		 */
 		I915_WRITE(VLV_MASTER_IER, 0);
+		ier = I915_READ(VLV_IER);
+		I915_WRITE(VLV_IER, 0);
 
 		if (gt_iir)
 			I915_WRITE(GTIIR, gt_iir);
@@ -1815,6 +1830,7 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
 		if (iir)
 			I915_WRITE(VLV_IIR, iir);
 
+		I915_WRITE(VLV_IER, ier);
 		I915_WRITE(VLV_MASTER_IER, MASTER_INTERRUPT_ENABLE);
 		POSTING_READ(VLV_MASTER_IER);
 	}
@@ -1839,6 +1855,8 @@ static irqreturn_t cherryview_irq_handler(int irq, void *arg)
 	disable_rpm_wakeref_asserts(dev_priv);
 
 	do {
+		u32 ier = 0;
+
 		master_ctl = I915_READ(GEN8_MASTER_IRQ) & ~GEN8_MASTER_IRQ_CONTROL;
 		iir = I915_READ(VLV_IIR);
 
@@ -1847,7 +1865,22 @@ static irqreturn_t cherryview_irq_handler(int irq, void *arg)
 
 		ret = IRQ_HANDLED;
 
+		/*
+		 * Theory on interrupt generation, based on empirical evidence:
+		 *
+		 * x = ((VLV_IIR & VLV_IER) ||
+		 *      ((GEN8_MASTER_IRQ & ~GEN8_MASTER_IRQ_CONTROL) &&
+		 *       (GEN8_MASTER_IRQ & GEN8_MASTER_IRQ_CONTROL)));
+		 *
+		 * A CPU interrupt will only be raised when 'x' has a 0->1 edge.
+		 * Hence we clear GEN8_MASTER_IRQ_CONTROL and VLV_IER to
+		 * guarantee the CPU interrupt will be raised again even if we
+		 * don't end up clearing all the VLV_IIR and GEN8_MASTER_IRQ_CONTROL
+		 * bits this time around.
+		 */
 		I915_WRITE(GEN8_MASTER_IRQ, 0);
+		ier = I915_READ(VLV_IER);
+		I915_WRITE(VLV_IER, 0);
 
 		gen8_gt_irq_handler(dev_priv, master_ctl);
 
@@ -1865,6 +1898,7 @@ static irqreturn_t cherryview_irq_handler(int irq, void *arg)
 		if (iir)
 			I915_WRITE(VLV_IIR, iir);
 
+		I915_WRITE(VLV_IER, ier);
 		I915_WRITE(GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL);
 		POSTING_READ(GEN8_MASTER_IRQ);
 	} while (0);
-- 
2.7.4

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

[PATCH 06/12] drm/i915: Eliminate loop from VLV irq handler

[ville.syrjala]

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

Now that we've dealt with the races in clearing IIR bits via VLV_IER
and the master interrupt enable, we can go ahead aliminate the loop
from the VLV interrupt handler.

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

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 46be03c616f4..9bfb125fcaea 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1777,7 +1777,7 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
 	/* IRQs are synced during runtime_suspend, we don't require a wakeref */
 	disable_rpm_wakeref_asserts(dev_priv);
 
-	while (true) {
+	do {
 		u32 ier = 0;
 
 		gt_iir = I915_READ(GTIIR);
@@ -1785,7 +1785,7 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
 		iir = I915_READ(VLV_IIR);
 
 		if (gt_iir == 0 && pm_iir == 0 && iir == 0)
-			goto out;
+			break;
 
 		ret = IRQ_HANDLED;
 
@@ -1833,9 +1833,8 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
 		I915_WRITE(VLV_IER, ier);
 		I915_WRITE(VLV_MASTER_IER, MASTER_INTERRUPT_ENABLE);
 		POSTING_READ(VLV_MASTER_IER);
-	}
+	} while (0);
 
-out:
 	enable_rpm_wakeref_asserts(dev_priv);
 
 	return ret;
-- 
2.7.4

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

[PATCH 07/12] drm/i915: Move variables to narrower scope in VLV/CHV irq handlers

[ville.syrjala]

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

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 9bfb125fcaea..523cb41fb283 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1768,7 +1768,6 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
 {
 	struct drm_device *dev = arg;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	u32 iir, gt_iir, pm_iir;
 	irqreturn_t ret = IRQ_NONE;
 
 	if (!intel_irqs_enabled(dev_priv))
@@ -1778,6 +1777,7 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
 	disable_rpm_wakeref_asserts(dev_priv);
 
 	do {
+		u32 iir, gt_iir, pm_iir;
 		u32 ier = 0;
 
 		gt_iir = I915_READ(GTIIR);
@@ -1844,7 +1844,6 @@ static irqreturn_t cherryview_irq_handler(int irq, void *arg)
 {
 	struct drm_device *dev = arg;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	u32 master_ctl, iir;
 	irqreturn_t ret = IRQ_NONE;
 
 	if (!intel_irqs_enabled(dev_priv))
@@ -1854,6 +1853,7 @@ static irqreturn_t cherryview_irq_handler(int irq, void *arg)
 	disable_rpm_wakeref_asserts(dev_priv);
 
 	do {
+		u32 master_ctl, iir;
 		u32 ier = 0;
 
 		master_ctl = I915_READ(GEN8_MASTER_IRQ) & ~GEN8_MASTER_IRQ_CONTROL;
-- 
2.7.4

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

[PATCH 08/12] drm/i915: Split PORT_HOTPLUG_STAT ack out from i9xx_hpd_irq_handler()

[ville.syrjala]

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

Split the VLV/CHV hoplug irq handling to ack and handler phases. This
way we can move the actual irq handling outside the section where
we have disabled the interrupt sources.

For now, we leave things as is for pre-VLV GMCH platforms, but
eventually they could get the same treatment.

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

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 523cb41fb283..1a7b3e97579a 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1723,21 +1723,20 @@ static void valleyview_pipestat_irq_handler(struct drm_device *dev, u32 iir)
 		gmbus_irq_handler(dev);
 }
 
-static void i9xx_hpd_irq_handler(struct drm_device *dev)
+static u32 i9xx_hpd_irq_ack(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv = dev->dev_private;
 	u32 hotplug_status = I915_READ(PORT_HOTPLUG_STAT);
-	u32 pin_mask = 0, long_mask = 0;
 
-	if (!hotplug_status)
-		return;
+	if (hotplug_status)
+		I915_WRITE(PORT_HOTPLUG_STAT, hotplug_status);
 
-	I915_WRITE(PORT_HOTPLUG_STAT, hotplug_status);
-	/*
-	 * Make sure hotplug status is cleared before we clear IIR, or else we
-	 * may miss hotplug events.
-	 */
-	POSTING_READ(PORT_HOTPLUG_STAT);
+	return hotplug_status;
+}
+
+static void i9xx_hpd_irq_handler(struct drm_device *dev,
+				 u32 hotplug_status)
+{
+	u32 pin_mask = 0, long_mask = 0;
 
 	if (IS_G4X(dev) || IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) {
 		u32 hotplug_trigger = hotplug_status & HOTPLUG_INT_STATUS_G4X;
@@ -1778,6 +1777,7 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
 
 	do {
 		u32 iir, gt_iir, pm_iir;
+		u32 hotplug_status = 0;
 		u32 ier = 0;
 
 		gt_iir = I915_READ(GTIIR);
@@ -1817,7 +1817,7 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
 			gen6_rps_irq_handler(dev_priv, pm_iir);
 
 		if (iir & I915_DISPLAY_PORT_INTERRUPT)
-			i9xx_hpd_irq_handler(dev);
+			hotplug_status = i9xx_hpd_irq_ack(dev_priv);
 
 		/* Call regardless, as some status bits might not be
 		 * signalled in iir */
@@ -1833,6 +1833,9 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
 		I915_WRITE(VLV_IER, ier);
 		I915_WRITE(VLV_MASTER_IER, MASTER_INTERRUPT_ENABLE);
 		POSTING_READ(VLV_MASTER_IER);
+
+		if (hotplug_status)
+			i9xx_hpd_irq_handler(dev, hotplug_status);
 	} while (0);
 
 	enable_rpm_wakeref_asserts(dev_priv);
@@ -1854,6 +1857,7 @@ static irqreturn_t cherryview_irq_handler(int irq, void *arg)
 
 	do {
 		u32 master_ctl, iir;
+		u32 hotplug_status = 0;
 		u32 ier = 0;
 
 		master_ctl = I915_READ(GEN8_MASTER_IRQ) & ~GEN8_MASTER_IRQ_CONTROL;
@@ -1884,7 +1888,7 @@ static irqreturn_t cherryview_irq_handler(int irq, void *arg)
 		gen8_gt_irq_handler(dev_priv, master_ctl);
 
 		if (iir & I915_DISPLAY_PORT_INTERRUPT)
-			i9xx_hpd_irq_handler(dev);
+			hotplug_status = i9xx_hpd_irq_ack(dev_priv);
 
 		/* Call regardless, as some status bits might not be
 		 * signalled in iir */
@@ -1900,6 +1904,9 @@ static irqreturn_t cherryview_irq_handler(int irq, void *arg)
 		I915_WRITE(VLV_IER, ier);
 		I915_WRITE(GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL);
 		POSTING_READ(GEN8_MASTER_IRQ);
+
+		if (hotplug_status)
+			i9xx_hpd_irq_handler(dev, hotplug_status);
 	} while (0);
 
 	enable_rpm_wakeref_asserts(dev_priv);
@@ -4257,8 +4264,11 @@ static irqreturn_t i915_irq_handler(int irq, void *arg)
 
 		/* Consume port.  Then clear IIR or we'll miss events */
 		if (I915_HAS_HOTPLUG(dev) &&
-		    iir & I915_DISPLAY_PORT_INTERRUPT)
-			i9xx_hpd_irq_handler(dev);
+		    iir & I915_DISPLAY_PORT_INTERRUPT) {
+			u32 hotplug_status = i9xx_hpd_irq_ack(dev_priv);
+			if (hotplug_status)
+				i9xx_hpd_irq_handler(dev, hotplug_status);
+		}
 
 		I915_WRITE(IIR, iir & ~flip_mask);
 		new_iir = I915_READ(IIR); /* Flush posted writes */
@@ -4487,8 +4497,11 @@ static irqreturn_t i965_irq_handler(int irq, void *arg)
 		ret = IRQ_HANDLED;
 
 		/* Consume port.  Then clear IIR or we'll miss events */
-		if (iir & I915_DISPLAY_PORT_INTERRUPT)
-			i9xx_hpd_irq_handler(dev);
+		if (iir & I915_DISPLAY_PORT_INTERRUPT) {
+			u32 hotplug_status = i9xx_hpd_irq_ack(dev_priv);
+			if (hotplug_status)
+				i9xx_hpd_irq_handler(dev, hotplug_status);
+		}
 
 		I915_WRITE(IIR, iir & ~flip_mask);
 		new_iir = I915_READ(IIR); /* Flush posted writes */
-- 
2.7.4

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

[PATCH 09/12] drm/i915: Split VLV/CVH PIPESTAT handling into ack+handler

[ville.syrjala]

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

Minimize the amount of stuff we do with interrupt sources disabled by
splitting the PIPESTAT irq handling into ack+handler phases.

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

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 1a7b3e97579a..3ac9e7e96cdc 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1644,10 +1644,10 @@ static bool intel_pipe_handle_vblank(struct drm_device *dev, enum pipe pipe)
 	return true;
 }
 
-static void valleyview_pipestat_irq_handler(struct drm_device *dev, u32 iir)
+static void valleyview_pipestat_irq_ack(struct drm_device *dev, u32 iir,
+					u32 pipe_stats[I915_MAX_PIPES])
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	u32 pipe_stats[I915_MAX_PIPES] = { };
 	int pipe;
 
 	spin_lock(&dev_priv->irq_lock);
@@ -1701,6 +1701,13 @@ static void valleyview_pipestat_irq_handler(struct drm_device *dev, u32 iir)
 			I915_WRITE(reg, pipe_stats[pipe]);
 	}
 	spin_unlock(&dev_priv->irq_lock);
+}
+
+static void valleyview_pipestat_irq_handler(struct drm_device *dev,
+					    u32 pipe_stats[I915_MAX_PIPES])
+{
+	struct drm_i915_private *dev_priv = to_i915(dev);
+	enum pipe pipe;
 
 	for_each_pipe(dev_priv, pipe) {
 		if (pipe_stats[pipe] & PIPE_START_VBLANK_INTERRUPT_STATUS &&
@@ -1777,6 +1784,7 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
 
 	do {
 		u32 iir, gt_iir, pm_iir;
+		u32 pipe_stats[I915_MAX_PIPES] = {};
 		u32 hotplug_status = 0;
 		u32 ier = 0;
 
@@ -1821,7 +1829,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_handler(dev, iir);
+		valleyview_pipestat_irq_ack(dev, iir, pipe_stats);
 
 		/*
 		 * VLV_IIR is single buffered, and reflects the level
@@ -1836,6 +1844,8 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
 
 		if (hotplug_status)
 			i9xx_hpd_irq_handler(dev, hotplug_status);
+
+		valleyview_pipestat_irq_handler(dev, pipe_stats);
 	} while (0);
 
 	enable_rpm_wakeref_asserts(dev_priv);
@@ -1857,6 +1867,7 @@ static irqreturn_t cherryview_irq_handler(int irq, void *arg)
 
 	do {
 		u32 master_ctl, iir;
+		u32 pipe_stats[I915_MAX_PIPES] = {};
 		u32 hotplug_status = 0;
 		u32 ier = 0;
 
@@ -1892,7 +1903,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_handler(dev, iir);
+		valleyview_pipestat_irq_ack(dev, iir, pipe_stats);
 
 		/*
 		 * VLV_IIR is single buffered, and reflects the level
@@ -1907,6 +1918,8 @@ static irqreturn_t cherryview_irq_handler(int irq, void *arg)
 
 		if (hotplug_status)
 			i9xx_hpd_irq_handler(dev, hotplug_status);
+
+		valleyview_pipestat_irq_handler(dev, pipe_stats);
 	} while (0);
 
 	enable_rpm_wakeref_asserts(dev_priv);
-- 
2.7.4

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

[PATCH 10/12] drm/i915: Move gt/pm irq handling out from irq disabled section on VLV

[ville.syrjala]

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

No need to actually handle the GT/PM interrupt while we have interrupt
sources disabled. Move the actual processing to happen after we've
restored VLV_IER and master interrupt enable.

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

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 3ac9e7e96cdc..dd8c02a5d490 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1819,11 +1819,6 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
 		if (pm_iir)
 			I915_WRITE(GEN6_PMIIR, pm_iir);
 
-		if (gt_iir)
-			snb_gt_irq_handler(dev, dev_priv, gt_iir);
-		if (pm_iir)
-			gen6_rps_irq_handler(dev_priv, pm_iir);
-
 		if (iir & I915_DISPLAY_PORT_INTERRUPT)
 			hotplug_status = i9xx_hpd_irq_ack(dev_priv);
 
@@ -1842,6 +1837,11 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
 		I915_WRITE(VLV_MASTER_IER, MASTER_INTERRUPT_ENABLE);
 		POSTING_READ(VLV_MASTER_IER);
 
+		if (gt_iir)
+			snb_gt_irq_handler(dev, dev_priv, gt_iir);
+		if (pm_iir)
+			gen6_rps_irq_handler(dev_priv, pm_iir);
+
 		if (hotplug_status)
 			i9xx_hpd_irq_handler(dev, hotplug_status);
 
-- 
2.7.4

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

[PATCH 11/12] drm/i915: Eliminate passing dev+dev_priv to {snb, ilk}_gt_irq_handler()

[ville.syrjala]

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

It looks silly to pass both dev and dev_priv to the snb/ilk gt irq
handlers. Just pass dev_priv.

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

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index dd8c02a5d490..7b07d1745b09 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1264,18 +1264,17 @@ out:
 	mutex_unlock(&dev_priv->dev->struct_mutex);
 }
 
-static void ivybridge_parity_error_irq_handler(struct drm_device *dev, u32 iir)
+static void ivybridge_parity_error_irq_handler(struct drm_i915_private *dev_priv,
+					       u32 iir)
 {
-	struct drm_i915_private *dev_priv = dev->dev_private;
-
-	if (!HAS_L3_DPF(dev))
+	if (!HAS_L3_DPF(dev_priv))
 		return;
 
 	spin_lock(&dev_priv->irq_lock);
-	gen5_disable_gt_irq(dev_priv, GT_PARITY_ERROR(dev));
+	gen5_disable_gt_irq(dev_priv, GT_PARITY_ERROR(dev_priv));
 	spin_unlock(&dev_priv->irq_lock);
 
-	iir &= GT_PARITY_ERROR(dev);
+	iir &= GT_PARITY_ERROR(dev_priv);
 	if (iir & GT_RENDER_L3_PARITY_ERROR_INTERRUPT_S1)
 		dev_priv->l3_parity.which_slice |= 1 << 1;
 
@@ -1285,8 +1284,7 @@ static void ivybridge_parity_error_irq_handler(struct drm_device *dev, u32 iir)
 	queue_work(dev_priv->wq, &dev_priv->l3_parity.error_work);
 }
 
-static void ilk_gt_irq_handler(struct drm_device *dev,
-			       struct drm_i915_private *dev_priv,
+static void ilk_gt_irq_handler(struct drm_i915_private *dev_priv,
 			       u32 gt_iir)
 {
 	if (gt_iir &
@@ -1296,8 +1294,7 @@ static void ilk_gt_irq_handler(struct drm_device *dev,
 		notify_ring(&dev_priv->engine[VCS]);
 }
 
-static void snb_gt_irq_handler(struct drm_device *dev,
-			       struct drm_i915_private *dev_priv,
+static void snb_gt_irq_handler(struct drm_i915_private *dev_priv,
 			       u32 gt_iir)
 {
 
@@ -1314,8 +1311,8 @@ static void snb_gt_irq_handler(struct drm_device *dev,
 		      GT_RENDER_CS_MASTER_ERROR_INTERRUPT))
 		DRM_DEBUG("Command parser error, gt_iir 0x%08x\n", gt_iir);
 
-	if (gt_iir & GT_PARITY_ERROR(dev))
-		ivybridge_parity_error_irq_handler(dev, gt_iir);
+	if (gt_iir & GT_PARITY_ERROR(dev_priv))
+		ivybridge_parity_error_irq_handler(dev_priv, gt_iir);
 }
 
 static __always_inline void
@@ -1838,7 +1835,7 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
 		POSTING_READ(VLV_MASTER_IER);
 
 		if (gt_iir)
-			snb_gt_irq_handler(dev, dev_priv, gt_iir);
+			snb_gt_irq_handler(dev_priv, gt_iir);
 		if (pm_iir)
 			gen6_rps_irq_handler(dev_priv, pm_iir);
 
@@ -2280,9 +2277,9 @@ static irqreturn_t ironlake_irq_handler(int irq, void *arg)
 		I915_WRITE(GTIIR, gt_iir);
 		ret = IRQ_HANDLED;
 		if (INTEL_INFO(dev)->gen >= 6)
-			snb_gt_irq_handler(dev, dev_priv, gt_iir);
+			snb_gt_irq_handler(dev_priv, gt_iir);
 		else
-			ilk_gt_irq_handler(dev, dev_priv, gt_iir);
+			ilk_gt_irq_handler(dev_priv, gt_iir);
 	}
 
 	de_iir = I915_READ(DEIIR);
-- 
2.7.4

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

[PATCH 12/12] drm/i915: Split gen8_gt_irq_handler into ack+handle

[ville.syrjala]

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

As we did on VLV, split the gt irq handling to ack and handler phases on
CHV. Leave the BDW+ codepath mostly intact for now.

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

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 7b07d1745b09..bb1f9a23793a 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1324,60 +1324,45 @@ gen8_cs_irq_handler(struct intel_engine_cs *engine, u32 iir, int test_shift)
 		tasklet_schedule(&engine->irq_tasklet);
 }
 
-static irqreturn_t gen8_gt_irq_handler(struct drm_i915_private *dev_priv,
-				       u32 master_ctl)
+static irqreturn_t gen8_gt_irq_ack(struct drm_i915_private *dev_priv,
+				   u32 master_ctl,
+				   u32 gt_iir[4])
 {
 	irqreturn_t ret = IRQ_NONE;
 
 	if (master_ctl & (GEN8_GT_RCS_IRQ | GEN8_GT_BCS_IRQ)) {
-		u32 iir = I915_READ_FW(GEN8_GT_IIR(0));
-		if (iir) {
-			I915_WRITE_FW(GEN8_GT_IIR(0), iir);
+		gt_iir[0] = I915_READ_FW(GEN8_GT_IIR(0));
+		if (gt_iir[0]) {
+			I915_WRITE_FW(GEN8_GT_IIR(0), gt_iir[0]);
 			ret = IRQ_HANDLED;
-
-			gen8_cs_irq_handler(&dev_priv->engine[RCS],
-					    iir, GEN8_RCS_IRQ_SHIFT);
-
-			gen8_cs_irq_handler(&dev_priv->engine[BCS],
-					    iir, GEN8_BCS_IRQ_SHIFT);
 		} else
 			DRM_ERROR("The master control interrupt lied (GT0)!\n");
 	}
 
 	if (master_ctl & (GEN8_GT_VCS1_IRQ | GEN8_GT_VCS2_IRQ)) {
-		u32 iir = I915_READ_FW(GEN8_GT_IIR(1));
-		if (iir) {
-			I915_WRITE_FW(GEN8_GT_IIR(1), iir);
+		gt_iir[1] = I915_READ_FW(GEN8_GT_IIR(1));
+		if (gt_iir[1]) {
+			I915_WRITE_FW(GEN8_GT_IIR(1), gt_iir[1]);
 			ret = IRQ_HANDLED;
-
-			gen8_cs_irq_handler(&dev_priv->engine[VCS],
-					    iir, GEN8_VCS1_IRQ_SHIFT);
-
-			gen8_cs_irq_handler(&dev_priv->engine[VCS2],
-					    iir, GEN8_VCS2_IRQ_SHIFT);
 		} else
 			DRM_ERROR("The master control interrupt lied (GT1)!\n");
 	}
 
 	if (master_ctl & GEN8_GT_VECS_IRQ) {
-		u32 iir = I915_READ_FW(GEN8_GT_IIR(3));
-		if (iir) {
-			I915_WRITE_FW(GEN8_GT_IIR(3), iir);
+		gt_iir[3] = I915_READ_FW(GEN8_GT_IIR(3));
+		if (gt_iir[3]) {
+			I915_WRITE_FW(GEN8_GT_IIR(3), gt_iir[3]);
 			ret = IRQ_HANDLED;
-
-			gen8_cs_irq_handler(&dev_priv->engine[VECS],
-					    iir, GEN8_VECS_IRQ_SHIFT);
 		} else
 			DRM_ERROR("The master control interrupt lied (GT3)!\n");
 	}
 
 	if (master_ctl & GEN8_GT_PM_IRQ) {
-		u32 iir = I915_READ_FW(GEN8_GT_IIR(2));
-		if (iir & dev_priv->pm_rps_events) {
+		gt_iir[2] = I915_READ_FW(GEN8_GT_IIR(2));
+		if (gt_iir[2] & dev_priv->pm_rps_events) {
 			I915_WRITE_FW(GEN8_GT_IIR(2),
-				      iir & dev_priv->pm_rps_events);
+				      gt_iir[2] & dev_priv->pm_rps_events);
 			ret = IRQ_HANDLED;
-			gen6_rps_irq_handler(dev_priv, iir);
 		} else
 			DRM_ERROR("The master control interrupt lied (PM)!\n");
 	}
@@ -1385,6 +1370,31 @@ static irqreturn_t gen8_gt_irq_handler(struct drm_i915_private *dev_priv,
 	return ret;
 }
 
+static void gen8_gt_irq_handler(struct drm_i915_private *dev_priv,
+				u32 gt_iir[4])
+{
+	if (gt_iir[0]) {
+		gen8_cs_irq_handler(&dev_priv->engine[RCS],
+				    gt_iir[0], GEN8_RCS_IRQ_SHIFT);
+		gen8_cs_irq_handler(&dev_priv->engine[BCS],
+				    gt_iir[0], GEN8_BCS_IRQ_SHIFT);
+	}
+
+	if (gt_iir[1]) {
+		gen8_cs_irq_handler(&dev_priv->engine[VCS],
+				    gt_iir[1], GEN8_VCS1_IRQ_SHIFT);
+		gen8_cs_irq_handler(&dev_priv->engine[VCS2],
+				    gt_iir[1], GEN8_VCS2_IRQ_SHIFT);
+	}
+
+	if (gt_iir[3])
+		gen8_cs_irq_handler(&dev_priv->engine[VECS],
+				    gt_iir[3], GEN8_VECS_IRQ_SHIFT);
+
+	if (gt_iir[2] & dev_priv->pm_rps_events)
+		gen6_rps_irq_handler(dev_priv, gt_iir[2]);
+}
+
 static bool bxt_port_hotplug_long_detect(enum port port, u32 val)
 {
 	switch (port) {
@@ -1864,6 +1874,7 @@ static irqreturn_t cherryview_irq_handler(int irq, void *arg)
 
 	do {
 		u32 master_ctl, iir;
+		u32 gt_iir[4] = {};
 		u32 pipe_stats[I915_MAX_PIPES] = {};
 		u32 hotplug_status = 0;
 		u32 ier = 0;
@@ -1893,7 +1904,7 @@ static irqreturn_t cherryview_irq_handler(int irq, void *arg)
 		ier = I915_READ(VLV_IER);
 		I915_WRITE(VLV_IER, 0);
 
-		gen8_gt_irq_handler(dev_priv, master_ctl);
+		gen8_gt_irq_ack(dev_priv, master_ctl, gt_iir);
 
 		if (iir & I915_DISPLAY_PORT_INTERRUPT)
 			hotplug_status = i9xx_hpd_irq_ack(dev_priv);
@@ -1913,6 +1924,8 @@ static irqreturn_t cherryview_irq_handler(int irq, void *arg)
 		I915_WRITE(GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL);
 		POSTING_READ(GEN8_MASTER_IRQ);
 
+		gen8_gt_irq_handler(dev_priv, gt_iir);
+
 		if (hotplug_status)
 			i9xx_hpd_irq_handler(dev, hotplug_status);
 
@@ -2479,6 +2492,7 @@ static irqreturn_t gen8_irq_handler(int irq, void *arg)
 	struct drm_device *dev = arg;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	u32 master_ctl;
+	u32 gt_iir[4] = {};
 	irqreturn_t ret;
 
 	if (!intel_irqs_enabled(dev_priv))
@@ -2495,7 +2509,8 @@ static irqreturn_t gen8_irq_handler(int irq, void *arg)
 	disable_rpm_wakeref_asserts(dev_priv);
 
 	/* Find, clear, then process each source of interrupt */
-	ret = gen8_gt_irq_handler(dev_priv, master_ctl);
+	ret = gen8_gt_irq_ack(dev_priv, master_ctl, gt_iir);
+	gen8_gt_irq_handler(dev_priv, gt_iir);
 	ret |= gen8_de_irq_handler(dev_priv, master_ctl);
 
 	I915_WRITE_FW(GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL);
-- 
2.7.4

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

Re: [PATCH 05/12] drm/i915: Clear VLV_IER around irq processing

[Chris Wilson]

On Wed, Apr 13, 2016 at 09:19:51PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> On VLV/CHV the master interrupt enable bit only affects GT/PM
> interrupts. Display interrupts are not affected by the master
> irq control.
> 
> Also it seems that the CPU interrupt will only be generated when
> the combined result of all GT/PM/display interrupts has a 0->1
> edge. We already use the master interrupt enable bit to make sure
> GT/PM interrupt can generate such an edge if we don't end up clearing
> all IIR bits. We must do the same for display interrupts, and for
> that we can simply clear out VLV_IER, and restore after we've acked
> all the interrupts we are about to process.
> 
> So with both master interrupt enable and VLV_IER cleared out, we will
> guarantee that there will be a 0->1 edge if any IIR bits are still set
> at the end, and thus another CPU interrupt will be generated.
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Fixes: 579de73b048a ("drm/i915: Exit cherryview_irq_handler() after one pass")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_irq.c | 36 +++++++++++++++++++++++++++++++++++-
>  1 file changed, 35 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 626775039919..46be03c616f4 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1778,7 +1778,7 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
>  	disable_rpm_wakeref_asserts(dev_priv);
>  
>  	while (true) {
> -		/* Find, clear, then process each source of interrupt */
> +		u32 ier = 0;
>  
>  		gt_iir = I915_READ(GTIIR);
>  		pm_iir = I915_READ(GEN6_PMIIR);
> @@ -1789,7 +1789,22 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
>  
>  		ret = IRQ_HANDLED;
>  
> +		/*
> +		 * Theory on interrupt generation, based on empirical evidence:
> +		 *
> +		 * x = ((VLV_IIR & VLV_IER) ||
> +		 *      (((GT_IIR & GT_IER) || (GEN6_PMIIR & GEN6_PMIER)) &&
> +		 *       (VLV_MASTER_IER & MASTER_INTERRUPT_ENABLE)));
> +		 *
> +		 * A CPU interrupt will only be raised when 'x' has a 0->1 edge.
> +		 * Hence we clear MASTER_INTERRUPT_ENABLE and VLV_IER to
> +		 * guarantee the CPU interrupt will be raised again even if we
> +		 * don't end up clearing all the VLV_IIR, GT_IIR, GEN6_PMIIR
> +		 * bits this time around.

Following this logic, we want to enable MASTER_IER before VLV_IER such
that we get an immediate irq if there is a residual VLV_IIR.

> +		 */
>  		I915_WRITE(VLV_MASTER_IER, 0);
> +		ier = I915_READ(VLV_IER);

+wishlist: dev_priv->irq_enabled to save adding another mmio read.

> +		I915_WRITE(VLV_IER, 0);
>  
>  		if (gt_iir)
>  			I915_WRITE(GTIIR, gt_iir);
> @@ -1815,6 +1830,7 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
>  		if (iir)
>  			I915_WRITE(VLV_IIR, iir);
>  
> +		I915_WRITE(VLV_IER, ier);
>  		I915_WRITE(VLV_MASTER_IER, MASTER_INTERRUPT_ENABLE);
>  		POSTING_READ(VLV_MASTER_IER);
>  	}
> 

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 11/12] drm/i915: Eliminate passing dev+dev_priv to {snb, ilk}_gt_irq_handler()

[Daniel Vetter]

On Wed, Apr 13, 2016 at 09:19:57PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> It looks silly to pass both dev and dev_priv to the snb/ilk gt irq
> handlers. Just pass dev_priv.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/i915/i915_irq.c | 27 ++++++++++++---------------
>  1 file changed, 12 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index dd8c02a5d490..7b07d1745b09 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1264,18 +1264,17 @@ out:
>  	mutex_unlock(&dev_priv->dev->struct_mutex);
>  }
>  
> -static void ivybridge_parity_error_irq_handler(struct drm_device *dev, u32 iir)
> +static void ivybridge_parity_error_irq_handler(struct drm_i915_private *dev_priv,
> +					       u32 iir)
>  {
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -
> -	if (!HAS_L3_DPF(dev))
> +	if (!HAS_L3_DPF(dev_priv))
>  		return;
>  
>  	spin_lock(&dev_priv->irq_lock);
> -	gen5_disable_gt_irq(dev_priv, GT_PARITY_ERROR(dev));
> +	gen5_disable_gt_irq(dev_priv, GT_PARITY_ERROR(dev_priv));
>  	spin_unlock(&dev_priv->irq_lock);
>  
> -	iir &= GT_PARITY_ERROR(dev);
> +	iir &= GT_PARITY_ERROR(dev_priv);
>  	if (iir & GT_RENDER_L3_PARITY_ERROR_INTERRUPT_S1)
>  		dev_priv->l3_parity.which_slice |= 1 << 1;
>  
> @@ -1285,8 +1284,7 @@ static void ivybridge_parity_error_irq_handler(struct drm_device *dev, u32 iir)
>  	queue_work(dev_priv->wq, &dev_priv->l3_parity.error_work);
>  }
>  
> -static void ilk_gt_irq_handler(struct drm_device *dev,
> -			       struct drm_i915_private *dev_priv,
> +static void ilk_gt_irq_handler(struct drm_i915_private *dev_priv,
>  			       u32 gt_iir)
>  {
>  	if (gt_iir &
> @@ -1296,8 +1294,7 @@ static void ilk_gt_irq_handler(struct drm_device *dev,
>  		notify_ring(&dev_priv->engine[VCS]);
>  }
>  
> -static void snb_gt_irq_handler(struct drm_device *dev,
> -			       struct drm_i915_private *dev_priv,
> +static void snb_gt_irq_handler(struct drm_i915_private *dev_priv,
>  			       u32 gt_iir)
>  {
>  
> @@ -1314,8 +1311,8 @@ static void snb_gt_irq_handler(struct drm_device *dev,
>  		      GT_RENDER_CS_MASTER_ERROR_INTERRUPT))
>  		DRM_DEBUG("Command parser error, gt_iir 0x%08x\n", gt_iir);
>  
> -	if (gt_iir & GT_PARITY_ERROR(dev))
> -		ivybridge_parity_error_irq_handler(dev, gt_iir);
> +	if (gt_iir & GT_PARITY_ERROR(dev_priv))
> +		ivybridge_parity_error_irq_handler(dev_priv, gt_iir);
>  }
>  
>  static __always_inline void
> @@ -1838,7 +1835,7 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
>  		POSTING_READ(VLV_MASTER_IER);
>  
>  		if (gt_iir)
> -			snb_gt_irq_handler(dev, dev_priv, gt_iir);
> +			snb_gt_irq_handler(dev_priv, gt_iir);
>  		if (pm_iir)
>  			gen6_rps_irq_handler(dev_priv, pm_iir);
>  
> @@ -2280,9 +2277,9 @@ static irqreturn_t ironlake_irq_handler(int irq, void *arg)
>  		I915_WRITE(GTIIR, gt_iir);
>  		ret = IRQ_HANDLED;
>  		if (INTEL_INFO(dev)->gen >= 6)
> -			snb_gt_irq_handler(dev, dev_priv, gt_iir);
> +			snb_gt_irq_handler(dev_priv, gt_iir);
>  		else
> -			ilk_gt_irq_handler(dev, dev_priv, gt_iir);
> +			ilk_gt_irq_handler(dev_priv, gt_iir);
>  	}
>  
>  	de_iir = I915_READ(DEIIR);
> -- 
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 05/12] drm/i915: Clear VLV_IER around irq processing

[Ville Syrjälä]

On Wed, Apr 13, 2016 at 09:53:38PM +0100, Chris Wilson wrote:
> On Wed, Apr 13, 2016 at 09:19:51PM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > On VLV/CHV the master interrupt enable bit only affects GT/PM
> > interrupts. Display interrupts are not affected by the master
> > irq control.
> > 
> > Also it seems that the CPU interrupt will only be generated when
> > the combined result of all GT/PM/display interrupts has a 0->1
> > edge. We already use the master interrupt enable bit to make sure
> > GT/PM interrupt can generate such an edge if we don't end up clearing
> > all IIR bits. We must do the same for display interrupts, and for
> > that we can simply clear out VLV_IER, and restore after we've acked
> > all the interrupts we are about to process.
> > 
> > So with both master interrupt enable and VLV_IER cleared out, we will
> > guarantee that there will be a 0->1 edge if any IIR bits are still set
> > at the end, and thus another CPU interrupt will be generated.
> > 
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > Fixes: 579de73b048a ("drm/i915: Exit cherryview_irq_handler() after one pass")
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_irq.c | 36 +++++++++++++++++++++++++++++++++++-
> >  1 file changed, 35 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > index 626775039919..46be03c616f4 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -1778,7 +1778,7 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
> >  	disable_rpm_wakeref_asserts(dev_priv);
> >  
> >  	while (true) {
> > -		/* Find, clear, then process each source of interrupt */
> > +		u32 ier = 0;
> >  
> >  		gt_iir = I915_READ(GTIIR);
> >  		pm_iir = I915_READ(GEN6_PMIIR);
> > @@ -1789,7 +1789,22 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
> >  
> >  		ret = IRQ_HANDLED;
> >  
> > +		/*
> > +		 * Theory on interrupt generation, based on empirical evidence:
> > +		 *
> > +		 * x = ((VLV_IIR & VLV_IER) ||
> > +		 *      (((GT_IIR & GT_IER) || (GEN6_PMIIR & GEN6_PMIER)) &&
> > +		 *       (VLV_MASTER_IER & MASTER_INTERRUPT_ENABLE)));
> > +		 *
> > +		 * A CPU interrupt will only be raised when 'x' has a 0->1 edge.
> > +		 * Hence we clear MASTER_INTERRUPT_ENABLE and VLV_IER to
> > +		 * guarantee the CPU interrupt will be raised again even if we
> > +		 * don't end up clearing all the VLV_IIR, GT_IIR, GEN6_PMIIR
> > +		 * bits this time around.
> 
> Following this logic, we want to enable MASTER_IER before VLV_IER such
> that we get an immediate irq if there is a residual VLV_IIR.

The order between master irq enable and VLV_IIR shouldn't matter. They
are totally independent of each other. Master irq enable is for GT,
VLV_IER is for display. We just have to make sure both of them are
going to be zero simultaneously, which will guarantee that the CPU
interrupt generation logic will see x==0 at that point.

> 
> > +		 */
> >  		I915_WRITE(VLV_MASTER_IER, 0);
> > +		ier = I915_READ(VLV_IER);
> 
> +wishlist: dev_priv->irq_enabled to save adding another mmio read.

I suppose such a thing could be added.

> 
> > +		I915_WRITE(VLV_IER, 0);
> >  
> >  		if (gt_iir)
> >  			I915_WRITE(GTIIR, gt_iir);
> > @@ -1815,6 +1830,7 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
> >  		if (iir)
> >  			I915_WRITE(VLV_IIR, iir);
> >  
> > +		I915_WRITE(VLV_IER, ier);
> >  		I915_WRITE(VLV_MASTER_IER, MASTER_INTERRUPT_ENABLE);
> >  		POSTING_READ(VLV_MASTER_IER);
> >  	}
> > 
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre

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

Re: [PATCH 05/12] drm/i915: Clear VLV_IER around irq processing

[Chris Wilson]

On Thu, Apr 14, 2016 at 11:22:48AM +0300, Ville Syrjälä wrote:
> On Wed, Apr 13, 2016 at 09:53:38PM +0100, Chris Wilson wrote:
> > On Wed, Apr 13, 2016 at 09:19:51PM +0300, ville.syrjala@linux.intel.com wrote:
> > > +		/*
> > > +		 * Theory on interrupt generation, based on empirical evidence:
> > > +		 *
> > > +		 * x = ((VLV_IIR & VLV_IER) ||
> > > +		 *      (((GT_IIR & GT_IER) || (GEN6_PMIIR & GEN6_PMIER)) &&
> > > +		 *       (VLV_MASTER_IER & MASTER_INTERRUPT_ENABLE)));
> > > +		 *
> > > +		 * A CPU interrupt will only be raised when 'x' has a 0->1 edge.
> > > +		 * Hence we clear MASTER_INTERRUPT_ENABLE and VLV_IER to
> > > +		 * guarantee the CPU interrupt will be raised again even if we
> > > +		 * don't end up clearing all the VLV_IIR, GT_IIR, GEN6_PMIIR
> > > +		 * bits this time around.
> > 
> > Following this logic, we want to enable MASTER_IER before VLV_IER such
> > that we get an immediate irq if there is a residual VLV_IIR.
> 
> The order between master irq enable and VLV_IIR shouldn't matter. They
> are totally independent of each other. Master irq enable is for GT,
> VLV_IER is for display. We just have to make sure both of them are
> going to be zero simultaneously, which will guarantee that the CPU
> interrupt generation logic will see x==0 at that point.

There is no flow from VLV_IER to VLV_MASTER_IER ?

Hmm, I guess it only matters if the interrupt is raised on the leading
edge versus the level. I presumed we had only edge triggered interrupts,
that is the signal is sent fom VLV_IIR & VLV_IER once and will not
result in an interrupt unless VLV_MASTER_IER is enabled.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 05/12] drm/i915: Clear VLV_IER around irq processing

[Ville Syrjälä]

On Thu, Apr 14, 2016 at 09:32:30AM +0100, Chris Wilson wrote:
> On Thu, Apr 14, 2016 at 11:22:48AM +0300, Ville Syrjälä wrote:
> > On Wed, Apr 13, 2016 at 09:53:38PM +0100, Chris Wilson wrote:
> > > On Wed, Apr 13, 2016 at 09:19:51PM +0300, ville.syrjala@linux.intel.com wrote:
> > > > +		/*
> > > > +		 * Theory on interrupt generation, based on empirical evidence:
> > > > +		 *
> > > > +		 * x = ((VLV_IIR & VLV_IER) ||
> > > > +		 *      (((GT_IIR & GT_IER) || (GEN6_PMIIR & GEN6_PMIER)) &&
> > > > +		 *       (VLV_MASTER_IER & MASTER_INTERRUPT_ENABLE)));
> > > > +		 *
> > > > +		 * A CPU interrupt will only be raised when 'x' has a 0->1 edge.
> > > > +		 * Hence we clear MASTER_INTERRUPT_ENABLE and VLV_IER to
> > > > +		 * guarantee the CPU interrupt will be raised again even if we
> > > > +		 * don't end up clearing all the VLV_IIR, GT_IIR, GEN6_PMIIR
> > > > +		 * bits this time around.
> > > 
> > > Following this logic, we want to enable MASTER_IER before VLV_IER such
> > > that we get an immediate irq if there is a residual VLV_IIR.
> > 
> > The order between master irq enable and VLV_IIR shouldn't matter. They
> > are totally independent of each other. Master irq enable is for GT,
> > VLV_IER is for display. We just have to make sure both of them are
> > going to be zero simultaneously, which will guarantee that the CPU
> > interrupt generation logic will see x==0 at that point.
> 
> There is no flow from VLV_IER to VLV_MASTER_IER ?

Nope. The master only handles GT interrupts.

> 
> Hmm, I guess it only matters if the interrupt is raised on the leading
> edge versus the level. I presumed we had only edge triggered interrupts,
> that is the signal is sent fom VLV_IIR & VLV_IER once and will not
> result in an interrupt unless VLV_MASTER_IER is enabled.
> -Chris
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre

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

Re: [PATCH 05/12] drm/i915: Clear VLV_IER around irq processing

[Chris Wilson]

On Wed, Apr 13, 2016 at 09:19:51PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> On VLV/CHV the master interrupt enable bit only affects GT/PM
> interrupts. Display interrupts are not affected by the master
> irq control.
> 
> Also it seems that the CPU interrupt will only be generated when
> the combined result of all GT/PM/display interrupts has a 0->1
> edge. We already use the master interrupt enable bit to make sure
> GT/PM interrupt can generate such an edge if we don't end up clearing
> all IIR bits. We must do the same for display interrupts, and for
> that we can simply clear out VLV_IER, and restore after we've acked
> all the interrupts we are about to process.
> 
> So with both master interrupt enable and VLV_IER cleared out, we will
> guarantee that there will be a 0->1 edge if any IIR bits are still set
> at the end, and thus another CPU interrupt will be generated.
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Fixes: 579de73b048a ("drm/i915: Exit cherryview_irq_handler() after one pass")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Ville is very persuasive,
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Other than wishing the extra POSTING_READ and reading known registers
away, the rest of the series is also
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 05/12] drm/i915: Clear VLV_IER around irq processing

[Ville Syrjälä]

On Thu, Apr 14, 2016 at 10:36:00AM +0100, Chris Wilson wrote:
> On Wed, Apr 13, 2016 at 09:19:51PM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > On VLV/CHV the master interrupt enable bit only affects GT/PM
> > interrupts. Display interrupts are not affected by the master
> > irq control.
> > 
> > Also it seems that the CPU interrupt will only be generated when
> > the combined result of all GT/PM/display interrupts has a 0->1
> > edge. We already use the master interrupt enable bit to make sure
> > GT/PM interrupt can generate such an edge if we don't end up clearing
> > all IIR bits. We must do the same for display interrupts, and for
> > that we can simply clear out VLV_IER, and restore after we've acked
> > all the interrupts we are about to process.
> > 
> > So with both master interrupt enable and VLV_IER cleared out, we will
> > guarantee that there will be a 0->1 edge if any IIR bits are still set
> > at the end, and thus another CPU interrupt will be generated.
> > 
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > Fixes: 579de73b048a ("drm/i915: Exit cherryview_irq_handler() after one pass")
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Ville is very persuasive,
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> Other than wishing the extra POSTING_READ and reading known registers
> away, the rest of the series is also
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Entire series pushed to dinq. Thanks for the review.

Let's keep our fingers crossed that this cures all the
current chv ailments.

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

Re: [PATCH 05/12] drm/i915: Clear VLV_IER around irq processing

[Mika Kuoppala]

ville.syrjala@linux.intel.com writes:

> [ text/plain ]
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> On VLV/CHV the master interrupt enable bit only affects GT/PM
> interrupts. Display interrupts are not affected by the master
> irq control.
>
> Also it seems that the CPU interrupt will only be generated when
> the combined result of all GT/PM/display interrupts has a 0->1
> edge. We already use the master interrupt enable bit to make sure
> GT/PM interrupt can generate such an edge if we don't end up clearing
> all IIR bits. We must do the same for display interrupts, and for
> that we can simply clear out VLV_IER, and restore after we've acked
> all the interrupts we are about to process.
>
> So with both master interrupt enable and VLV_IER cleared out, we will
> guarantee that there will be a 0->1 edge if any IIR bits are still set
> at the end, and thus another CPU interrupt will be generated.
>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Fixes: 579de73b048a ("drm/i915: Exit cherryview_irq_handler() after
> one pass")

I have bisected the bsw ci/bat unstability (frequent gpu hangs) to the
commit pointed by Fixes: And reverting seems to help. 

I am now running the same drv_module_reload loop on top of this
series. It looks good but its too early to draw conclusions.
I will leave it running for the night.

-Mika

> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_irq.c | 36 +++++++++++++++++++++++++++++++++++-
>  1 file changed, 35 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 626775039919..46be03c616f4 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1778,7 +1778,7 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
>  	disable_rpm_wakeref_asserts(dev_priv);
>  
>  	while (true) {
> -		/* Find, clear, then process each source of interrupt */
> +		u32 ier = 0;
>  
>  		gt_iir = I915_READ(GTIIR);
>  		pm_iir = I915_READ(GEN6_PMIIR);
> @@ -1789,7 +1789,22 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
>  
>  		ret = IRQ_HANDLED;
>  
> +		/*
> +		 * Theory on interrupt generation, based on empirical evidence:
> +		 *
> +		 * x = ((VLV_IIR & VLV_IER) ||
> +		 *      (((GT_IIR & GT_IER) || (GEN6_PMIIR & GEN6_PMIER)) &&
> +		 *       (VLV_MASTER_IER & MASTER_INTERRUPT_ENABLE)));
> +		 *
> +		 * A CPU interrupt will only be raised when 'x' has a 0->1 edge.
> +		 * Hence we clear MASTER_INTERRUPT_ENABLE and VLV_IER to
> +		 * guarantee the CPU interrupt will be raised again even if we
> +		 * don't end up clearing all the VLV_IIR, GT_IIR, GEN6_PMIIR
> +		 * bits this time around.
> +		 */
>  		I915_WRITE(VLV_MASTER_IER, 0);
> +		ier = I915_READ(VLV_IER);
> +		I915_WRITE(VLV_IER, 0);
>  
>  		if (gt_iir)
>  			I915_WRITE(GTIIR, gt_iir);
> @@ -1815,6 +1830,7 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
>  		if (iir)
>  			I915_WRITE(VLV_IIR, iir);
>  
> +		I915_WRITE(VLV_IER, ier);
>  		I915_WRITE(VLV_MASTER_IER, MASTER_INTERRUPT_ENABLE);
>  		POSTING_READ(VLV_MASTER_IER);
>  	}
> @@ -1839,6 +1855,8 @@ static irqreturn_t cherryview_irq_handler(int irq, void *arg)
>  	disable_rpm_wakeref_asserts(dev_priv);
>  
>  	do {
> +		u32 ier = 0;
> +
>  		master_ctl = I915_READ(GEN8_MASTER_IRQ) & ~GEN8_MASTER_IRQ_CONTROL;
>  		iir = I915_READ(VLV_IIR);
>  
> @@ -1847,7 +1865,22 @@ static irqreturn_t cherryview_irq_handler(int irq, void *arg)
>  
>  		ret = IRQ_HANDLED;
>  
> +		/*
> +		 * Theory on interrupt generation, based on empirical evidence:
> +		 *
> +		 * x = ((VLV_IIR & VLV_IER) ||
> +		 *      ((GEN8_MASTER_IRQ & ~GEN8_MASTER_IRQ_CONTROL) &&
> +		 *       (GEN8_MASTER_IRQ & GEN8_MASTER_IRQ_CONTROL)));
> +		 *
> +		 * A CPU interrupt will only be raised when 'x' has a 0->1 edge.
> +		 * Hence we clear GEN8_MASTER_IRQ_CONTROL and VLV_IER to
> +		 * guarantee the CPU interrupt will be raised again even if we
> +		 * don't end up clearing all the VLV_IIR and GEN8_MASTER_IRQ_CONTROL
> +		 * bits this time around.
> +		 */
>  		I915_WRITE(GEN8_MASTER_IRQ, 0);
> +		ier = I915_READ(VLV_IER);
> +		I915_WRITE(VLV_IER, 0);
>  
>  		gen8_gt_irq_handler(dev_priv, master_ctl);
>  
> @@ -1865,6 +1898,7 @@ static irqreturn_t cherryview_irq_handler(int irq, void *arg)
>  		if (iir)
>  			I915_WRITE(VLV_IIR, iir);
>  
> +		I915_WRITE(VLV_IER, ier);
>  		I915_WRITE(GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL);
>  		POSTING_READ(GEN8_MASTER_IRQ);
>  	} while (0);
> -- 
> 2.7.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 05/12] drm/i915: Clear VLV_IER around irq processing

[Mika Kuoppala]

Mika Kuoppala <mika.kuoppala@linux.intel.com> writes:

> [ text/plain ]
> ville.syrjala@linux.intel.com writes:
>
>> [ text/plain ]
>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>
>> On VLV/CHV the master interrupt enable bit only affects GT/PM
>> interrupts. Display interrupts are not affected by the master
>> irq control.
>>
>> Also it seems that the CPU interrupt will only be generated when
>> the combined result of all GT/PM/display interrupts has a 0->1
>> edge. We already use the master interrupt enable bit to make sure
>> GT/PM interrupt can generate such an edge if we don't end up clearing
>> all IIR bits. We must do the same for display interrupts, and for
>> that we can simply clear out VLV_IER, and restore after we've acked
>> all the interrupts we are about to process.
>>
>> So with both master interrupt enable and VLV_IER cleared out, we will
>> guarantee that there will be a 0->1 edge if any IIR bits are still set
>> at the end, and thus another CPU interrupt will be generated.
>>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> Fixes: 579de73b048a ("drm/i915: Exit cherryview_irq_handler() after
>> one pass")
>
> I have bisected the bsw ci/bat unstability (frequent gpu hangs) to the
> commit pointed by Fixes: And reverting seems to help. 
>
> I am now running the same drv_module_reload loop on top of this
> series. It looks good but its too early to draw conclusions.
> I will leave it running for the night.
>

Seems to work fine in my tests and as also reflected by CI/bat.

-Mika


> -Mika
>
>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_irq.c | 36 +++++++++++++++++++++++++++++++++++-
>>  1 file changed, 35 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
>> index 626775039919..46be03c616f4 100644
>> --- a/drivers/gpu/drm/i915/i915_irq.c
>> +++ b/drivers/gpu/drm/i915/i915_irq.c
>> @@ -1778,7 +1778,7 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
>>  	disable_rpm_wakeref_asserts(dev_priv);
>>  
>>  	while (true) {
>> -		/* Find, clear, then process each source of interrupt */
>> +		u32 ier = 0;
>>  
>>  		gt_iir = I915_READ(GTIIR);
>>  		pm_iir = I915_READ(GEN6_PMIIR);
>> @@ -1789,7 +1789,22 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
>>  
>>  		ret = IRQ_HANDLED;
>>  
>> +		/*
>> +		 * Theory on interrupt generation, based on empirical evidence:
>> +		 *
>> +		 * x = ((VLV_IIR & VLV_IER) ||
>> +		 *      (((GT_IIR & GT_IER) || (GEN6_PMIIR & GEN6_PMIER)) &&
>> +		 *       (VLV_MASTER_IER & MASTER_INTERRUPT_ENABLE)));
>> +		 *
>> +		 * A CPU interrupt will only be raised when 'x' has a 0->1 edge.
>> +		 * Hence we clear MASTER_INTERRUPT_ENABLE and VLV_IER to
>> +		 * guarantee the CPU interrupt will be raised again even if we
>> +		 * don't end up clearing all the VLV_IIR, GT_IIR, GEN6_PMIIR
>> +		 * bits this time around.
>> +		 */
>>  		I915_WRITE(VLV_MASTER_IER, 0);
>> +		ier = I915_READ(VLV_IER);
>> +		I915_WRITE(VLV_IER, 0);
>>  
>>  		if (gt_iir)
>>  			I915_WRITE(GTIIR, gt_iir);
>> @@ -1815,6 +1830,7 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
>>  		if (iir)
>>  			I915_WRITE(VLV_IIR, iir);
>>  
>> +		I915_WRITE(VLV_IER, ier);
>>  		I915_WRITE(VLV_MASTER_IER, MASTER_INTERRUPT_ENABLE);
>>  		POSTING_READ(VLV_MASTER_IER);
>>  	}
>> @@ -1839,6 +1855,8 @@ static irqreturn_t cherryview_irq_handler(int irq, void *arg)
>>  	disable_rpm_wakeref_asserts(dev_priv);
>>  
>>  	do {
>> +		u32 ier = 0;
>> +
>>  		master_ctl = I915_READ(GEN8_MASTER_IRQ) & ~GEN8_MASTER_IRQ_CONTROL;
>>  		iir = I915_READ(VLV_IIR);
>>  
>> @@ -1847,7 +1865,22 @@ static irqreturn_t cherryview_irq_handler(int irq, void *arg)
>>  
>>  		ret = IRQ_HANDLED;
>>  
>> +		/*
>> +		 * Theory on interrupt generation, based on empirical evidence:
>> +		 *
>> +		 * x = ((VLV_IIR & VLV_IER) ||
>> +		 *      ((GEN8_MASTER_IRQ & ~GEN8_MASTER_IRQ_CONTROL) &&
>> +		 *       (GEN8_MASTER_IRQ & GEN8_MASTER_IRQ_CONTROL)));
>> +		 *
>> +		 * A CPU interrupt will only be raised when 'x' has a 0->1 edge.
>> +		 * Hence we clear GEN8_MASTER_IRQ_CONTROL and VLV_IER to
>> +		 * guarantee the CPU interrupt will be raised again even if we
>> +		 * don't end up clearing all the VLV_IIR and GEN8_MASTER_IRQ_CONTROL
>> +		 * bits this time around.
>> +		 */
>>  		I915_WRITE(GEN8_MASTER_IRQ, 0);
>> +		ier = I915_READ(VLV_IER);
>> +		I915_WRITE(VLV_IER, 0);
>>  
>>  		gen8_gt_irq_handler(dev_priv, master_ctl);
>>  
>> @@ -1865,6 +1898,7 @@ static irqreturn_t cherryview_irq_handler(int irq, void *arg)
>>  		if (iir)
>>  			I915_WRITE(VLV_IIR, iir);
>>  
>> +		I915_WRITE(VLV_IER, ier);
>>  		I915_WRITE(GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL);
>>  		POSTING_READ(GEN8_MASTER_IRQ);
>>  	} while (0);
>> -- 
>> 2.7.4
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx