Skip to contentSkip to conversation

Unify ILK/SNB/IVB/HSW IRQ vfuncs

PATCH In review Paulo Zanoni 4 participants 32 messages Jul 12, 2013 – Jul 19, 2013

Subsystems (2)
DRM drivers Intel i915
lore
Share & tools
lore

Paulo Zanoni

author
#0 lore raw

cover Unify ILK/SNB/IVB/HSW IRQ vfuncs

From: Paulo Zanoni <[email protected]>

Hi

These patches apply on top of -nightly + 4 patches from Daniel Vetter (the 4 unmerged patches from his irq series v2). I imagine we're probably going to merge them soon in their current form, so if we have conflicts, they will be small.

The basic idea here is that the IRQ handling for ILK, SNB, IVB and HSW is actually pretty similar. If you look at the current code it looks very different, but after moving some gen-specific code to separate functions everything gets really similar. So on this series I just merged all the vfuncs.

I imagine not everybody will like all the patches, so we could discard one or two or all if there are objections. The biggest advantage is that we now have less vfuncs to change whenever we want to do work on the interrupt code, and we also have a smaller amount of total source code lines: 151 insertions and 249 deletions. Also, IMHO, the resulting code is easier to maintain.

Another argument in favor of merging all the vfuncs is, for example, a few of Daniel's fixes from his last IRQ rework and the one or two bugs I fix in this series. We had quite a few registers that were exactly the same on SNB/IVB/HSW, but they were handled in completely different ways between SNB (which uses the ILK IRQ handlers) and IVB/HSW (which use the IVB IRQ handlers).

Patches 2-7 were already sent to this mailing list, but they were not on top of Daniel's IRQ rework, so the versions contained here are the new ones, and you can discard that old series.

Flames, bikesheds?

Cheers,
Paulo

Paulo Zanoni (10):
drm/i915: kill ivybridge_irq_preinstall drm/i915: extract ilk_display_irq_handler drm/i915: extract ivb_display_irq_handler drm/i915: don't read or write GEN6_PMIIR on Gen 5 drm/i915: reorganize ironlake_irq_handler drm/i915: POSTING_READ(DEIER) on ivybridge_irq_handler drm/i915: add ILK/SNB support to ivybridge_irq_handler drm/i915: kill ivybridge_enable_vblank drm/i915: kill ivybridge_disable_vblank drm/i915: kill ivybridge_irq_postinstall

 drivers/gpu/drm/i915/i915_irq.c | 400 +++++++++++++++-------------------------
 1 file changed, 151 insertions(+), 249 deletions(-)
Signature · 1 line
1.8.1.2
author
#1 lore raw

PATCH 1/10 drm/i915: kill ivybridge_irq_preinstall

From: Paulo Zanoni <[email protected]>

After Daniel's latest changes it's now equal to
ironlake_irq_preinstall.

Signed-off-by Paulo Zanoni <[email protected]>
---
 drivers/gpu/drm/i915/i915_irq.c | 21 +--------------------
 1 file changed, 1 insertion(+), 20 deletions(-)
drivers/gpu/drm/i915/i915_irq.c +1 -20
index ca9df54..f397f9a 100644
@@ -2090,25 +2090,6 @@ static void ironlake_irq_preinstall(struct drm_device *dev)
 	ibx_irq_preinstall(dev);
 }
 
-static void ivybridge_irq_preinstall(struct drm_device *dev)
-{
-	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
-
-	atomic_set(&dev_priv->irq_received, 0);
-
-	I915_WRITE(HWSTAM, 0xeffe);
-
-	/* XXX hotplug from PCH */
-
-	I915_WRITE(DEIMR, 0xffffffff);
-	I915_WRITE(DEIER, 0x0);
-	POSTING_READ(DEIER);
-
-	gen5_gt_irq_preinstall(dev);
-
-	ibx_irq_preinstall(dev);
-}
-
 static void valleyview_irq_preinstall(struct drm_device *dev)
 {
 	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;

@@ -3116,7 +3097,7 @@ void intel_irq_init(struct drm_device *dev)
 	} else if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev)) {
 		/* Share uninstall handlers with ILK/SNB */
 		dev->driver->irq_handler = ivybridge_irq_handler;
-		dev->driver->irq_preinstall = ivybridge_irq_preinstall;
+		dev->driver->irq_preinstall = ironlake_irq_preinstall;
 		dev->driver->irq_postinstall = ivybridge_irq_postinstall;
 		dev->driver->irq_uninstall = ironlake_irq_uninstall;
 		dev->driver->enable_vblank = ivybridge_enable_vblank;
Signature · 1 line
1.8.1.2
Reviewed-by on v1 1/10
#2 lore raw

Re: [PATCH 01/10] drm/i915: kill ivybridge_irq_preinstall

Quoted 17 lines · Paulo Zanoni

From: Paulo Zanoni <[email protected]>

After Daniel's latest changes it's now equal to
ironlake_irq_preinstall.

Signed-off-by Paulo Zanoni <[email protected]>
---
 drivers/gpu/drm/i915/i915_irq.c | 21 +--------------------
 1 file changed, 1 insertion(+), 20 deletions(-)
drivers/gpu/drm/i915/i915_irq.c +0 -0
index ca9df54..f397f9a 100644
@@ -2090,25 +2090,6 @@ static void ironlake_irq_preinstall(struct drm_device *dev)
 	ibx_irq_preinstall(dev);
 }

drivers/gpu/drm/i915/i915_irq.c static void ironlake_irq_preinstall(struct drm_device *dev) · line 2101

Quoted 10 lines
drivers/gpu/drm/i915/i915_irq.c +0 -9
 
-static void ivybridge_irq_preinstall(struct drm_device *dev)
-{
-	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
-
-	atomic_set(&dev_priv->irq_received, 0);
-
-	I915_WRITE(HWSTAM, 0xeffe);
-
-	/* XXX hotplug from PCH */

This comment will be lost. I couldn't figure out if it is is relevant anymore or not.

Reviewed-by Mika Kuoppala <[email protected]>
Quoted 28 lines
drivers/gpu/drm/i915/i915_irq.c +1 -10
-	I915_WRITE(DEIMR, 0xffffffff);
-	I915_WRITE(DEIER, 0x0);
-	POSTING_READ(DEIER);
-
-	gen5_gt_irq_preinstall(dev);
-
-	ibx_irq_preinstall(dev);
-}
-
 static void valleyview_irq_preinstall(struct drm_device *dev)
 {
 	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;

@@ -3116,7 +3097,7 @@ void intel_irq_init(struct drm_device *dev)
 	} else if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev)) {
 		/* Share uninstall handlers with ILK/SNB */
 		dev->driver->irq_handler = ivybridge_irq_handler;
-		dev->driver->irq_preinstall = ivybridge_irq_preinstall;
+		dev->driver->irq_preinstall = ironlake_irq_preinstall;
 		dev->driver->irq_postinstall = ivybridge_irq_postinstall;
 		dev->driver->irq_uninstall = ironlake_irq_uninstall;
 		dev->driver->enable_vblank = ivybridge_enable_vblank;
Signature · 6 lines
1.8.1.2

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
author
#3 lore raw

PATCH 2/10 drm/i915: extract ilk_display_irq_handler

In reply to Paulo Zanoni

From: Paulo Zanoni <[email protected]>

It's the code that deals with de_iir.

Signed-off-by Paulo Zanoni <[email protected]>
---
 drivers/gpu/drm/i915/i915_irq.c | 104 +++++++++++++++++++++-------------------
 1 file changed, 56 insertions(+), 48 deletions(-)
drivers/gpu/drm/i915/i915_irq.c +56 -48
index f397f9a..39160a2 100644
@@ -1202,6 +1202,60 @@ static void cpt_irq_handler(struct drm_device *dev, u32 pch_iir)
 		cpt_serr_int_handler(dev);
 }
 
+static void ilk_display_irq_handler(struct drm_device *dev, u32 de_iir)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	if (de_iir & DE_AUX_CHANNEL_A)
+		dp_aux_irq_handler(dev);
+
+	if (de_iir & DE_GSE)
+		intel_opregion_asle_intr(dev);
+
+	if (de_iir & DE_PIPEA_VBLANK)
+		drm_handle_vblank(dev, 0);
+
+	if (de_iir & DE_PIPEB_VBLANK)
+		drm_handle_vblank(dev, 1);
+
+	if (de_iir & DE_POISON)
+		DRM_ERROR("Poison interrupt\n");
+
+	if (de_iir & DE_PIPEA_FIFO_UNDERRUN)
+		if (intel_set_cpu_fifo_underrun_reporting(dev, PIPE_A, false))
+			DRM_DEBUG_DRIVER("Pipe A FIFO underrun\n");
+
+	if (de_iir & DE_PIPEB_FIFO_UNDERRUN)
+		if (intel_set_cpu_fifo_underrun_reporting(dev, PIPE_B, false))
+			DRM_DEBUG_DRIVER("Pipe B FIFO underrun\n");
+
+	if (de_iir & DE_PLANEA_FLIP_DONE) {
+		intel_prepare_page_flip(dev, 0);
+		intel_finish_page_flip_plane(dev, 0);
+	}
+
+	if (de_iir & DE_PLANEB_FLIP_DONE) {
+		intel_prepare_page_flip(dev, 1);
+		intel_finish_page_flip_plane(dev, 1);
+	}
+
+	/* check event from PCH */
+	if (de_iir & DE_PCH_EVENT) {
+		u32 pch_iir = I915_READ(SDEIIR);
+
+		if (HAS_PCH_CPT(dev))
+			cpt_irq_handler(dev, pch_iir);
+		else
+			ibx_irq_handler(dev, pch_iir);
+
+		/* should clear PCH hotplug event before clear CPU irq */
+		I915_WRITE(SDEIIR, pch_iir);
+	}
+
+	if (IS_GEN5(dev) && de_iir & DE_PCU_EVENT)
+		ironlake_rps_change_irq_handler(dev);
+}
+
 static irqreturn_t ivybridge_irq_handler(int irq, void *arg)
 {
 	struct drm_device *dev = (struct drm_device *) arg;

@@ -1360,54 +1414,8 @@ static irqreturn_t ironlake_irq_handler(int irq, void *arg)
 	else
 		snb_gt_irq_handler(dev, dev_priv, gt_iir);
 
-	if (de_iir & DE_AUX_CHANNEL_A)
-		dp_aux_irq_handler(dev);
-
-	if (de_iir & DE_GSE)
-		intel_opregion_asle_intr(dev);
-
-	if (de_iir & DE_PIPEA_VBLANK)
-		drm_handle_vblank(dev, 0);
-
-	if (de_iir & DE_PIPEB_VBLANK)
-		drm_handle_vblank(dev, 1);
-
-	if (de_iir & DE_POISON)
-		DRM_ERROR("Poison interrupt\n");
-
-	if (de_iir & DE_PIPEA_FIFO_UNDERRUN)
-		if (intel_set_cpu_fifo_underrun_reporting(dev, PIPE_A, false))
-			DRM_DEBUG_DRIVER("Pipe A FIFO underrun\n");
-
-	if (de_iir & DE_PIPEB_FIFO_UNDERRUN)
-		if (intel_set_cpu_fifo_underrun_reporting(dev, PIPE_B, false))
-			DRM_DEBUG_DRIVER("Pipe B FIFO underrun\n");
-
-	if (de_iir & DE_PLANEA_FLIP_DONE) {
-		intel_prepare_page_flip(dev, 0);
-		intel_finish_page_flip_plane(dev, 0);
-	}
-
-	if (de_iir & DE_PLANEB_FLIP_DONE) {
-		intel_prepare_page_flip(dev, 1);
-		intel_finish_page_flip_plane(dev, 1);
-	}
-
-	/* check event from PCH */
-	if (de_iir & DE_PCH_EVENT) {
-		u32 pch_iir = I915_READ(SDEIIR);
-
-		if (HAS_PCH_CPT(dev))
-			cpt_irq_handler(dev, pch_iir);
-		else
-			ibx_irq_handler(dev, pch_iir);
-
-		/* should clear PCH hotplug event before clear CPU irq */
-		I915_WRITE(SDEIIR, pch_iir);
-	}
-
-	if (IS_GEN5(dev) &&  de_iir & DE_PCU_EVENT)
-		ironlake_rps_change_irq_handler(dev);
+	if (de_iir)
+		ilk_display_irq_handler(dev, de_iir);
 
 	if (IS_GEN6(dev) && pm_iir & GEN6_PM_RPS_EVENTS)
 		gen6_rps_irq_handler(dev_priv, pm_iir);
Signature · 1 line
1.8.1.2
Reviewed-by on v1 2/10
#4 lore raw

Re: [PATCH 02/10] drm/i915: extract ilk_display_irq_handler

Quoted 5 lines · Paulo Zanoni

From: Paulo Zanoni <[email protected]>

It's the code that deals with de_iir.

Signed-off-by Paulo Zanoni <[email protected]>

Minor observation on the whole irq stuff: ilk, ironlake and ivb, ivybridge are both used and I couldn't figure out the pattern.

Reviewed-by Mika Kuoppala <[email protected]>
Quoted 133 lines
---
 drivers/gpu/drm/i915/i915_irq.c | 104 +++++++++++++++++++++-------------------
 1 file changed, 56 insertions(+), 48 deletions(-)
drivers/gpu/drm/i915/i915_irq.c +56 -48
index f397f9a..39160a2 100644
@@ -1202,6 +1202,60 @@ static void cpt_irq_handler(struct drm_device *dev, u32 pch_iir)
 		cpt_serr_int_handler(dev);
 }
 
+static void ilk_display_irq_handler(struct drm_device *dev, u32 de_iir)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	if (de_iir & DE_AUX_CHANNEL_A)
+		dp_aux_irq_handler(dev);
+
+	if (de_iir & DE_GSE)
+		intel_opregion_asle_intr(dev);
+
+	if (de_iir & DE_PIPEA_VBLANK)
+		drm_handle_vblank(dev, 0);
+
+	if (de_iir & DE_PIPEB_VBLANK)
+		drm_handle_vblank(dev, 1);
+
+	if (de_iir & DE_POISON)
+		DRM_ERROR("Poison interrupt\n");
+
+	if (de_iir & DE_PIPEA_FIFO_UNDERRUN)
+		if (intel_set_cpu_fifo_underrun_reporting(dev, PIPE_A, false))
+			DRM_DEBUG_DRIVER("Pipe A FIFO underrun\n");
+
+	if (de_iir & DE_PIPEB_FIFO_UNDERRUN)
+		if (intel_set_cpu_fifo_underrun_reporting(dev, PIPE_B, false))
+			DRM_DEBUG_DRIVER("Pipe B FIFO underrun\n");
+
+	if (de_iir & DE_PLANEA_FLIP_DONE) {
+		intel_prepare_page_flip(dev, 0);
+		intel_finish_page_flip_plane(dev, 0);
+	}
+
+	if (de_iir & DE_PLANEB_FLIP_DONE) {
+		intel_prepare_page_flip(dev, 1);
+		intel_finish_page_flip_plane(dev, 1);
+	}
+
+	/* check event from PCH */
+	if (de_iir & DE_PCH_EVENT) {
+		u32 pch_iir = I915_READ(SDEIIR);
+
+		if (HAS_PCH_CPT(dev))
+			cpt_irq_handler(dev, pch_iir);
+		else
+			ibx_irq_handler(dev, pch_iir);
+
+		/* should clear PCH hotplug event before clear CPU irq */
+		I915_WRITE(SDEIIR, pch_iir);
+	}
+
+	if (IS_GEN5(dev) && de_iir & DE_PCU_EVENT)
+		ironlake_rps_change_irq_handler(dev);
+}
+
 static irqreturn_t ivybridge_irq_handler(int irq, void *arg)
 {
 	struct drm_device *dev = (struct drm_device *) arg;

@@ -1360,54 +1414,8 @@ static irqreturn_t ironlake_irq_handler(int irq, void *arg)
 	else
 		snb_gt_irq_handler(dev, dev_priv, gt_iir);
 
-	if (de_iir & DE_AUX_CHANNEL_A)
-		dp_aux_irq_handler(dev);
-
-	if (de_iir & DE_GSE)
-		intel_opregion_asle_intr(dev);
-
-	if (de_iir & DE_PIPEA_VBLANK)
-		drm_handle_vblank(dev, 0);
-
-	if (de_iir & DE_PIPEB_VBLANK)
-		drm_handle_vblank(dev, 1);
-
-	if (de_iir & DE_POISON)
-		DRM_ERROR("Poison interrupt\n");
-
-	if (de_iir & DE_PIPEA_FIFO_UNDERRUN)
-		if (intel_set_cpu_fifo_underrun_reporting(dev, PIPE_A, false))
-			DRM_DEBUG_DRIVER("Pipe A FIFO underrun\n");
-
-	if (de_iir & DE_PIPEB_FIFO_UNDERRUN)
-		if (intel_set_cpu_fifo_underrun_reporting(dev, PIPE_B, false))
-			DRM_DEBUG_DRIVER("Pipe B FIFO underrun\n");
-
-	if (de_iir & DE_PLANEA_FLIP_DONE) {
-		intel_prepare_page_flip(dev, 0);
-		intel_finish_page_flip_plane(dev, 0);
-	}
-
-	if (de_iir & DE_PLANEB_FLIP_DONE) {
-		intel_prepare_page_flip(dev, 1);
-		intel_finish_page_flip_plane(dev, 1);
-	}
-
-	/* check event from PCH */
-	if (de_iir & DE_PCH_EVENT) {
-		u32 pch_iir = I915_READ(SDEIIR);
-
-		if (HAS_PCH_CPT(dev))
-			cpt_irq_handler(dev, pch_iir);
-		else
-			ibx_irq_handler(dev, pch_iir);
-
-		/* should clear PCH hotplug event before clear CPU irq */
-		I915_WRITE(SDEIIR, pch_iir);
-	}
-
-	if (IS_GEN5(dev) &&  de_iir & DE_PCU_EVENT)
-		ironlake_rps_change_irq_handler(dev);
+	if (de_iir)
+		ilk_display_irq_handler(dev, de_iir);
 
 	if (IS_GEN6(dev) && pm_iir & GEN6_PM_RPS_EVENTS)
 		gen6_rps_irq_handler(dev_priv, pm_iir);
Signature · 6 lines
1.8.1.2

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
author
#5 lore raw

Re: [PATCH 02/10] drm/i915: extract ilk_display_irq_handler

2013/7/19 Mika Kuoppala <[email protected]>:

Quoted 10 lines
Quoted 5 lines · Paulo Zanoni

From: Paulo Zanoni <[email protected]>

It's the code that deals with de_iir.

Signed-off-by Paulo Zanoni <[email protected]>

Minor observation on the whole irq stuff: ilk, ironlake and ivb, ivybridge are both used and I couldn't figure out the pattern.

You mean the function names, right? This is something that confuses me too, all our code is inconsistent and I'd like to know what's the preferred way. We seem to use both ways: vlv_find_best_dpll vs valleyview_crtc_enable, ironlake_crtc_mode_set vs ilk_update_plane, haswell_modeset_global_resources vs ivb_modeset_global_resources. I tend to prefer the shorter versions because I believe these acronyms should be obvious (or easy to discover) for people reading our code, but having some guidance on which one to use on each case would be good. Opinions?

Quoted 2 lines
Reviewed-by Mika Kuoppala <[email protected]>

Thanks for all the reviews!

Quoted 134 lines
Quoted 134 lines
---
 drivers/gpu/drm/i915/i915_irq.c | 104 +++++++++++++++++++++-------------------
 1 file changed, 56 insertions(+), 48 deletions(-)
drivers/gpu/drm/i915/i915_irq.c +56 -49
index f397f9a..39160a2 100644
@@ -1202,6 +1202,60 @@ static void cpt_irq_handler(struct drm_device *dev, u32 pch_iir)
              cpt_serr_int_handler(dev);
 }
 
+static void ilk_display_irq_handler(struct drm_device *dev, u32 de_iir)
+{
+     struct drm_i915_private *dev_priv = dev->dev_private;
+
+     if (de_iir & DE_AUX_CHANNEL_A)
+             dp_aux_irq_handler(dev);
+
+     if (de_iir & DE_GSE)
+             intel_opregion_asle_intr(dev);
+
+     if (de_iir & DE_PIPEA_VBLANK)
+             drm_handle_vblank(dev, 0);
+
+     if (de_iir & DE_PIPEB_VBLANK)
+             drm_handle_vblank(dev, 1);
+
+     if (de_iir & DE_POISON)
+             DRM_ERROR("Poison interrupt\n");
+
+     if (de_iir & DE_PIPEA_FIFO_UNDERRUN)
+             if (intel_set_cpu_fifo_underrun_reporting(dev, PIPE_A, false))
+                     DRM_DEBUG_DRIVER("Pipe A FIFO underrun\n");
+
+     if (de_iir & DE_PIPEB_FIFO_UNDERRUN)
+             if (intel_set_cpu_fifo_underrun_reporting(dev, PIPE_B, false))
+                     DRM_DEBUG_DRIVER("Pipe B FIFO underrun\n");
+
+     if (de_iir & DE_PLANEA_FLIP_DONE) {
+             intel_prepare_page_flip(dev, 0);
+             intel_finish_page_flip_plane(dev, 0);
+     }
+
+     if (de_iir & DE_PLANEB_FLIP_DONE) {
+             intel_prepare_page_flip(dev, 1);
+             intel_finish_page_flip_plane(dev, 1);
+     }
+
+     /* check event from PCH */
+     if (de_iir & DE_PCH_EVENT) {
+             u32 pch_iir = I915_READ(SDEIIR);
+
+             if (HAS_PCH_CPT(dev))
+                     cpt_irq_handler(dev, pch_iir);
+             else
+                     ibx_irq_handler(dev, pch_iir);
+
+             /* should clear PCH hotplug event before clear CPU irq */
+             I915_WRITE(SDEIIR, pch_iir);
+     }
+
+     if (IS_GEN5(dev) && de_iir & DE_PCU_EVENT)
+             ironlake_rps_change_irq_handler(dev);
+}
+
 static irqreturn_t ivybridge_irq_handler(int irq, void *arg)
 {
      struct drm_device *dev = (struct drm_device *) arg;

@@ -1360,54 +1414,8 @@ static irqreturn_t ironlake_irq_handler(int irq, void *arg)
      else
              snb_gt_irq_handler(dev, dev_priv, gt_iir);
 
-     if (de_iir & DE_AUX_CHANNEL_A)
-             dp_aux_irq_handler(dev);
-
-     if (de_iir & DE_GSE)
-             intel_opregion_asle_intr(dev);
-
-     if (de_iir & DE_PIPEA_VBLANK)
-             drm_handle_vblank(dev, 0);
-
-     if (de_iir & DE_PIPEB_VBLANK)
-             drm_handle_vblank(dev, 1);
-
-     if (de_iir & DE_POISON)
-             DRM_ERROR("Poison interrupt\n");
-
-     if (de_iir & DE_PIPEA_FIFO_UNDERRUN)
-             if (intel_set_cpu_fifo_underrun_reporting(dev, PIPE_A, false))
-                     DRM_DEBUG_DRIVER("Pipe A FIFO underrun\n");
-
-     if (de_iir & DE_PIPEB_FIFO_UNDERRUN)
-             if (intel_set_cpu_fifo_underrun_reporting(dev, PIPE_B, false))
-                     DRM_DEBUG_DRIVER("Pipe B FIFO underrun\n");
-
-     if (de_iir & DE_PLANEA_FLIP_DONE) {
-             intel_prepare_page_flip(dev, 0);
-             intel_finish_page_flip_plane(dev, 0);
-     }
-
-     if (de_iir & DE_PLANEB_FLIP_DONE) {
-             intel_prepare_page_flip(dev, 1);
-             intel_finish_page_flip_plane(dev, 1);
-     }
-
-     /* check event from PCH */
-     if (de_iir & DE_PCH_EVENT) {
-             u32 pch_iir = I915_READ(SDEIIR);
-
-             if (HAS_PCH_CPT(dev))
-                     cpt_irq_handler(dev, pch_iir);
-             else
-                     ibx_irq_handler(dev, pch_iir);
-
-             /* should clear PCH hotplug event before clear CPU irq */
-             I915_WRITE(SDEIIR, pch_iir);
-     }
-
-     if (IS_GEN5(dev) &&  de_iir & DE_PCU_EVENT)
-             ironlake_rps_change_irq_handler(dev);
+     if (de_iir)
+             ilk_display_irq_handler(dev, de_iir);
 
      if (IS_GEN6(dev) && pm_iir & GEN6_PM_RPS_EVENTS)
              gen6_rps_irq_handler(dev_priv, pm_iir);
--

1.8.1.2

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Signature · 1 line
Paulo Zanoni
#6 lore raw

Re: [PATCH 02/10] drm/i915: extract ilk_display_irq_handler

Quoted 12 lines · Paulo Zanoni

2013/7/19 Mika Kuoppala <[email protected]>:

Quoted 10 lines
Quoted 5 lines · Paulo Zanoni

From: Paulo Zanoni <[email protected]>

It's the code that deals with de_iir.

Signed-off-by Paulo Zanoni <[email protected]>

Minor observation on the whole irq stuff: ilk, ironlake and ivb, ivybridge are both used and I couldn't figure out the pattern.

Quoted 9 lines

You mean the function names, right? This is something that confuses me too, all our code is inconsistent and I'd like to know what's the preferred way. We seem to use both ways: vlv_find_best_dpll vs valleyview_crtc_enable, ironlake_crtc_mode_set vs ilk_update_plane, haswell_modeset_global_resources vs ivb_modeset_global_resources. I tend to prefer the shorter versions because I believe these acronyms should be obvious (or easy to discover) for people reading our code, but having some guidance on which one to use on each case would be good. Opinions?

I prefer the shorter ones, too. But iirc Jesse is liking the longer ones more, at least he tends to use them in his patches. I guess we'll just have to flag this in patches. New fodder for a good bikeshd ;-)

Cheeers, Daniel

Signature · 3 lines
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
author
#7 lore raw

PATCH 3/10 drm/i915: extract ivb_display_irq_handler

In reply to Paulo Zanoni

From: Paulo Zanoni <[email protected]>

Just like we did with ilk_display_irq_handler.

Signed-off-by Paulo Zanoni <[email protected]>
---
 drivers/gpu/drm/i915/i915_irq.c | 63 +++++++++++++++++++++++------------------
 1 file changed, 35 insertions(+), 28 deletions(-)
drivers/gpu/drm/i915/i915_irq.c +35 -28
index 39160a2..9167219 100644
@@ -1256,13 +1256,46 @@ static void ilk_display_irq_handler(struct drm_device *dev, u32 de_iir)
 		ironlake_rps_change_irq_handler(dev);
 }
 
+static void ivb_display_irq_handler(struct drm_device *dev, u32 de_iir)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	int i;
+
+	if (de_iir & DE_ERR_INT_IVB)
+		ivb_err_int_handler(dev);
+
+	if (de_iir & DE_AUX_CHANNEL_A_IVB)
+		dp_aux_irq_handler(dev);
+
+	if (de_iir & DE_GSE_IVB)
+		intel_opregion_asle_intr(dev);
+
+	for (i = 0; i < 3; i++) {
+		if (de_iir & (DE_PIPEA_VBLANK_IVB << (5 * i)))
+			drm_handle_vblank(dev, i);
+		if (de_iir & (DE_PLANEA_FLIP_DONE_IVB << (5 * i))) {
+			intel_prepare_page_flip(dev, i);
+			intel_finish_page_flip_plane(dev, i);
+		}
+	}
+
+	/* check event from PCH */
+	if (!HAS_PCH_NOP(dev) && (de_iir & DE_PCH_EVENT_IVB)) {
+		u32 pch_iir = I915_READ(SDEIIR);
+
+		cpt_irq_handler(dev, pch_iir);
+
+		/* clear PCH hotplug event before clear CPU irq */
+		I915_WRITE(SDEIIR, pch_iir);
+	}
+}
+
 static irqreturn_t ivybridge_irq_handler(int irq, void *arg)
 {
 	struct drm_device *dev = (struct drm_device *) arg;
 	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
 	u32 de_iir, gt_iir, de_ier, pm_iir, sde_ier = 0;
 	irqreturn_t ret = IRQ_NONE;
-	int i;
 
 	atomic_inc(&dev_priv->irq_received);
 

@@ -1307,33 +1340,7 @@ static irqreturn_t ivybridge_irq_handler(int irq, void *arg)
 
 	de_iir = I915_READ(DEIIR);
 	if (de_iir) {
-		if (de_iir & DE_ERR_INT_IVB)
-			ivb_err_int_handler(dev);
-
-		if (de_iir & DE_AUX_CHANNEL_A_IVB)
-			dp_aux_irq_handler(dev);
-
-		if (de_iir & DE_GSE_IVB)
-			intel_opregion_asle_intr(dev);
-
-		for (i = 0; i < 3; i++) {
-			if (de_iir & (DE_PIPEA_VBLANK_IVB << (5 * i)))
-				drm_handle_vblank(dev, i);
-			if (de_iir & (DE_PLANEA_FLIP_DONE_IVB << (5 * i))) {
-				intel_prepare_page_flip(dev, i);
-				intel_finish_page_flip_plane(dev, i);
-			}
-		}
-
-		/* check event from PCH */
-		if (!HAS_PCH_NOP(dev) && (de_iir & DE_PCH_EVENT_IVB)) {
-			u32 pch_iir = I915_READ(SDEIIR);
-
-			cpt_irq_handler(dev, pch_iir);
-
-			/* clear PCH hotplug event before clear CPU irq */
-			I915_WRITE(SDEIIR, pch_iir);
-		}
+		ivb_display_irq_handler(dev, de_iir);
 
 		I915_WRITE(DEIIR, de_iir);
 		ret = IRQ_HANDLED;
Signature · 1 line
1.8.1.2
Reviewed-by on v1 3/10
#8 lore raw

Re: [PATCH 03/10] drm/i915: extract ivb_display_irq_handler

Quoted 5 lines · Paulo Zanoni

From: Paulo Zanoni <[email protected]>

Just like we did with ilk_display_irq_handler.

Signed-off-by Paulo Zanoni <[email protected]>
Reviewed-by Mika Kuoppala <[email protected]>
Quoted 98 lines
---
 drivers/gpu/drm/i915/i915_irq.c | 63 +++++++++++++++++++++++------------------
 1 file changed, 35 insertions(+), 28 deletions(-)
drivers/gpu/drm/i915/i915_irq.c +35 -28
index 39160a2..9167219 100644
@@ -1256,13 +1256,46 @@ static void ilk_display_irq_handler(struct drm_device *dev, u32 de_iir)
 		ironlake_rps_change_irq_handler(dev);
 }
 
+static void ivb_display_irq_handler(struct drm_device *dev, u32 de_iir)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	int i;
+
+	if (de_iir & DE_ERR_INT_IVB)
+		ivb_err_int_handler(dev);
+
+	if (de_iir & DE_AUX_CHANNEL_A_IVB)
+		dp_aux_irq_handler(dev);
+
+	if (de_iir & DE_GSE_IVB)
+		intel_opregion_asle_intr(dev);
+
+	for (i = 0; i < 3; i++) {
+		if (de_iir & (DE_PIPEA_VBLANK_IVB << (5 * i)))
+			drm_handle_vblank(dev, i);
+		if (de_iir & (DE_PLANEA_FLIP_DONE_IVB << (5 * i))) {
+			intel_prepare_page_flip(dev, i);
+			intel_finish_page_flip_plane(dev, i);
+		}
+	}
+
+	/* check event from PCH */
+	if (!HAS_PCH_NOP(dev) && (de_iir & DE_PCH_EVENT_IVB)) {
+		u32 pch_iir = I915_READ(SDEIIR);
+
+		cpt_irq_handler(dev, pch_iir);
+
+		/* clear PCH hotplug event before clear CPU irq */
+		I915_WRITE(SDEIIR, pch_iir);
+	}
+}
+
 static irqreturn_t ivybridge_irq_handler(int irq, void *arg)
 {
 	struct drm_device *dev = (struct drm_device *) arg;
 	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
 	u32 de_iir, gt_iir, de_ier, pm_iir, sde_ier = 0;
 	irqreturn_t ret = IRQ_NONE;
-	int i;
 
 	atomic_inc(&dev_priv->irq_received);
 

@@ -1307,33 +1340,7 @@ static irqreturn_t ivybridge_irq_handler(int irq, void *arg)
 
 	de_iir = I915_READ(DEIIR);
 	if (de_iir) {
-		if (de_iir & DE_ERR_INT_IVB)
-			ivb_err_int_handler(dev);
-
-		if (de_iir & DE_AUX_CHANNEL_A_IVB)
-			dp_aux_irq_handler(dev);
-
-		if (de_iir & DE_GSE_IVB)
-			intel_opregion_asle_intr(dev);
-
-		for (i = 0; i < 3; i++) {
-			if (de_iir & (DE_PIPEA_VBLANK_IVB << (5 * i)))
-				drm_handle_vblank(dev, i);
-			if (de_iir & (DE_PLANEA_FLIP_DONE_IVB << (5 * i))) {
-				intel_prepare_page_flip(dev, i);
-				intel_finish_page_flip_plane(dev, i);
-			}
-		}
-
-		/* check event from PCH */
-		if (!HAS_PCH_NOP(dev) && (de_iir & DE_PCH_EVENT_IVB)) {
-			u32 pch_iir = I915_READ(SDEIIR);
-
-			cpt_irq_handler(dev, pch_iir);
-
-			/* clear PCH hotplug event before clear CPU irq */
-			I915_WRITE(SDEIIR, pch_iir);
-		}
+		ivb_display_irq_handler(dev, de_iir);
 
 		I915_WRITE(DEIIR, de_iir);
 		ret = IRQ_HANDLED;
Signature · 6 lines
1.8.1.2

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
author
#9 lore raw

PATCH 4/10 drm/i915: don't read or write GEN6_PMIIR on Gen 5

In reply to Paulo Zanoni

From: Paulo Zanoni <[email protected]>

The register doesn't exist on Gen 5.

Signed-off-by Paulo Zanoni <[email protected]>
---
 drivers/gpu/drm/i915/i915_irq.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)
drivers/gpu/drm/i915/i915_irq.c +5 -3
index 9167219..19370db 100644
@@ -1389,7 +1389,7 @@ static irqreturn_t ironlake_irq_handler(int irq, void *arg)
 	struct drm_device *dev = (struct drm_device *) arg;
 	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
 	int ret = IRQ_NONE;
-	u32 de_iir, gt_iir, de_ier, pm_iir, sde_ier;
+	u32 de_iir, gt_iir, de_ier, pm_iir = 0, sde_ier;
 
 	atomic_inc(&dev_priv->irq_received);
 

@@ -1409,7 +1409,8 @@ static irqreturn_t ironlake_irq_handler(int irq, void *arg)
 
 	de_iir = I915_READ(DEIIR);
 	gt_iir = I915_READ(GTIIR);
-	pm_iir = I915_READ(GEN6_PMIIR);
+	if (IS_GEN6(dev))
+		pm_iir = I915_READ(GEN6_PMIIR);
 
 	if (de_iir == 0 && gt_iir == 0 && (!IS_GEN6(dev) || pm_iir == 0))
 		goto done;

@@ -1429,7 +1430,8 @@ static irqreturn_t ironlake_irq_handler(int irq, void *arg)
 
 	I915_WRITE(GTIIR, gt_iir);
 	I915_WRITE(DEIIR, de_iir);
-	I915_WRITE(GEN6_PMIIR, pm_iir);
+	if (IS_GEN6(dev))
+		I915_WRITE(GEN6_PMIIR, pm_iir);
 
 done:
 	I915_WRITE(DEIER, de_ier);
Signature · 1 line
1.8.1.2
#10 lore raw

Re: [PATCH 04/10] drm/i915: don't read or write GEN6_PMIIR on Gen 5

Quoted 3 lines · Paulo Zanoni

From: Paulo Zanoni <[email protected]>

The register doesn't exist on Gen 5.

Whilst you are fixing that, initialize pm_iir = 0 and simply the later checks.
-Chris

Signature · 1 line
Chris Wilson, Intel Open Source Technology Centre
author
#11 lore raw

PATCH 4/9 drm/i915: don't read or write GEN6_PMIIR on Gen 5

From: Paulo Zanoni <[email protected]>

The register doesn't exist on Gen 5.

v2: Simplify checks since pm_iir is always 0 on Gen 5 (Chris)

Signed-off-by Paulo Zanoni <[email protected]>
---
 drivers/gpu/drm/i915/i915_irq.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)
drivers/gpu/drm/i915/i915_irq.c +7 -5
index 9167219..c674dc3 100644
@@ -1389,7 +1389,7 @@ static irqreturn_t ironlake_irq_handler(int irq, void *arg)
 	struct drm_device *dev = (struct drm_device *) arg;
 	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
 	int ret = IRQ_NONE;
-	u32 de_iir, gt_iir, de_ier, pm_iir, sde_ier;
+	u32 de_iir, gt_iir, de_ier, pm_iir = 0, sde_ier;
 
 	atomic_inc(&dev_priv->irq_received);
 

@@ -1409,9 +1409,10 @@ static irqreturn_t ironlake_irq_handler(int irq, void *arg)
 
 	de_iir = I915_READ(DEIIR);
 	gt_iir = I915_READ(GTIIR);
-	pm_iir = I915_READ(GEN6_PMIIR);
+	if (IS_GEN6(dev))
+		pm_iir = I915_READ(GEN6_PMIIR);
 
-	if (de_iir == 0 && gt_iir == 0 && (!IS_GEN6(dev) || pm_iir == 0))
+	if (de_iir == 0 && gt_iir == 0 && pm_iir == 0)
 		goto done;
 
 	ret = IRQ_HANDLED;

@@ -1424,12 +1425,13 @@ static irqreturn_t ironlake_irq_handler(int irq, void *arg)
 	if (de_iir)
 		ilk_display_irq_handler(dev, de_iir);
 
-	if (IS_GEN6(dev) && pm_iir & GEN6_PM_RPS_EVENTS)
+	if (pm_iir & GEN6_PM_RPS_EVENTS)
 		gen6_rps_irq_handler(dev_priv, pm_iir);
 
 	I915_WRITE(GTIIR, gt_iir);
 	I915_WRITE(DEIIR, de_iir);
-	I915_WRITE(GEN6_PMIIR, pm_iir);
+	if (pm_iir)
+		I915_WRITE(GEN6_PMIIR, pm_iir);
 
 done:
 	I915_WRITE(DEIER, de_ier);
Signature · 1 line
1.8.1.2
Reviewed-by on v1 4/9
#12 lore raw

Re: [PATCH 4/9] drm/i915: don't read or write GEN6_PMIIR on Gen 5

Quoted 7 lines · Paulo Zanoni

From: Paulo Zanoni <[email protected]>

The register doesn't exist on Gen 5.

v2: Simplify checks since pm_iir is always 0 on Gen 5 (Chris)

Signed-off-by Paulo Zanoni <[email protected]>
Reviewed-by Mika Kuoppala <[email protected]>
Quoted 53 lines
---
 drivers/gpu/drm/i915/i915_irq.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)
drivers/gpu/drm/i915/i915_irq.c +7 -5
index 9167219..c674dc3 100644
@@ -1389,7 +1389,7 @@ static irqreturn_t ironlake_irq_handler(int irq, void *arg)
 	struct drm_device *dev = (struct drm_device *) arg;
 	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
 	int ret = IRQ_NONE;
-	u32 de_iir, gt_iir, de_ier, pm_iir, sde_ier;
+	u32 de_iir, gt_iir, de_ier, pm_iir = 0, sde_ier;
 
 	atomic_inc(&dev_priv->irq_received);
 

@@ -1409,9 +1409,10 @@ static irqreturn_t ironlake_irq_handler(int irq, void *arg)
 
 	de_iir = I915_READ(DEIIR);
 	gt_iir = I915_READ(GTIIR);
-	pm_iir = I915_READ(GEN6_PMIIR);
+	if (IS_GEN6(dev))
+		pm_iir = I915_READ(GEN6_PMIIR);
 
-	if (de_iir == 0 && gt_iir == 0 && (!IS_GEN6(dev) || pm_iir == 0))
+	if (de_iir == 0 && gt_iir == 0 && pm_iir == 0)
 		goto done;
 
 	ret = IRQ_HANDLED;

@@ -1424,12 +1425,13 @@ static irqreturn_t ironlake_irq_handler(int irq, void *arg)
 	if (de_iir)
 		ilk_display_irq_handler(dev, de_iir);
 
-	if (IS_GEN6(dev) && pm_iir & GEN6_PM_RPS_EVENTS)
+	if (pm_iir & GEN6_PM_RPS_EVENTS)
 		gen6_rps_irq_handler(dev_priv, pm_iir);
 
 	I915_WRITE(GTIIR, gt_iir);
 	I915_WRITE(DEIIR, de_iir);
-	I915_WRITE(GEN6_PMIIR, pm_iir);
+	if (pm_iir)
+		I915_WRITE(GEN6_PMIIR, pm_iir);
 
 done:
 	I915_WRITE(DEIER, de_ier);
Signature · 6 lines
1.8.1.2

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
author
#13 lore raw

PATCH 5/10 drm/i915: reorganize ironlake_irq_handler

In reply to Paulo Zanoni

From: Paulo Zanoni <[email protected]>

The ironlake_irq_handler and ivybridge_irq_handler functions do basically the same thing, but they have different implementation styles. With this patch we reorganize ironlake_irq_handler in a way that makes it look very similar to ivybridge_irq_handler.

One of the advantages of this new function style is that we don't write 0 to the IIR registers anymore.

Signed-off-by Paulo Zanoni <[email protected]>
---
 drivers/gpu/drm/i915/i915_irq.c | 44 ++++++++++++++++++++---------------------
 1 file changed, 22 insertions(+), 22 deletions(-)
drivers/gpu/drm/i915/i915_irq.c +22 -22
index 19370db..7bc36ae 100644
@@ -1407,33 +1407,33 @@ static irqreturn_t ironlake_irq_handler(int irq, void *arg)
 	I915_WRITE(SDEIER, 0);
 	POSTING_READ(SDEIER);
 
-	de_iir = I915_READ(DEIIR);
 	gt_iir = I915_READ(GTIIR);
-	if (IS_GEN6(dev))
-		pm_iir = I915_READ(GEN6_PMIIR);
-
-	if (de_iir == 0 && gt_iir == 0 && (!IS_GEN6(dev) || pm_iir == 0))
-		goto done;
-
-	ret = IRQ_HANDLED;
-
-	if (IS_GEN5(dev))
-		ilk_gt_irq_handler(dev, dev_priv, gt_iir);
-	else
-		snb_gt_irq_handler(dev, dev_priv, gt_iir);
+	if (gt_iir) {
+		if (IS_GEN5(dev))
+			ilk_gt_irq_handler(dev, dev_priv, gt_iir);
+		else
+			snb_gt_irq_handler(dev, dev_priv, gt_iir);
+		I915_WRITE(GTIIR, gt_iir);
+		ret = IRQ_HANDLED;
+	}
 
-	if (de_iir)
+	de_iir = I915_READ(DEIIR);
+	if (de_iir) {
 		ilk_display_irq_handler(dev, de_iir);
+		I915_WRITE(DEIIR, de_iir);
+		ret = IRQ_HANDLED;
+	}
 
-	if (IS_GEN6(dev) && pm_iir & GEN6_PM_RPS_EVENTS)
-		gen6_rps_irq_handler(dev_priv, pm_iir);
-
-	I915_WRITE(GTIIR, gt_iir);
-	I915_WRITE(DEIIR, de_iir);
-	if (IS_GEN6(dev))
-		I915_WRITE(GEN6_PMIIR, pm_iir);
+	if (IS_GEN6(dev)) {
+		pm_iir = I915_READ(GEN6_PMIIR);
+		if (pm_iir) {
+			if (pm_iir & GEN6_PM_RPS_EVENTS)
+				gen6_rps_irq_handler(dev_priv, pm_iir);
+			I915_WRITE(GEN6_PMIIR, pm_iir);
+			ret = IRQ_HANDLED;
+		}
+	}
 
-done:
 	I915_WRITE(DEIER, de_ier);
 	POSTING_READ(DEIER);
 	I915_WRITE(SDEIER, sde_ier);
Signature · 1 line
1.8.1.2
#14 lore raw

Re: [PATCH 05/10] drm/i915: reorganize ironlake_irq_handler

Quoted 9 lines · Paulo Zanoni

From: Paulo Zanoni <[email protected]>

The ironlake_irq_handler and ivybridge_irq_handler functions do basically the same thing, but they have different implementation styles. With this patch we reorganize ironlake_irq_handler in a way that makes it look very similar to ivybridge_irq_handler.

One of the advantages of this new function style is that we don't write 0 to the IIR registers anymore.

Please tightly scope the variables where applicable. -Chris

Signature · 1 line
Chris Wilson, Intel Open Source Technology Centre
author
#15 lore raw

PATCH 5/9 drm/i915: reorganize ironlake_irq_handler

From: Paulo Zanoni <[email protected]>

The ironlake_irq_handler and ivybridge_irq_handler functions do basically the same thing, but they have different implementation styles. With this patch we reorganize ironlake_irq_handler in a way that makes it look very similar to ivybridge_irq_handler.

One of the advantages of this new function style is that we don't write 0 to the IIR registers anymore.

v2: - Rebase due to changes on previous patches
- Move pm_iir to a tighter scope (Chris)

Signed-off-by Paulo Zanoni <[email protected]>
---
 drivers/gpu/drm/i915/i915_irq.c | 46 ++++++++++++++++++++---------------------
 1 file changed, 23 insertions(+), 23 deletions(-)
drivers/gpu/drm/i915/i915_irq.c +23 -23
index c674dc3..88eb380 100644
@@ -1389,7 +1389,7 @@ static irqreturn_t ironlake_irq_handler(int irq, void *arg)
 	struct drm_device *dev = (struct drm_device *) arg;
 	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
 	int ret = IRQ_NONE;
-	u32 de_iir, gt_iir, de_ier, pm_iir = 0, sde_ier;
+	u32 de_iir, gt_iir, de_ier, sde_ier;
 
 	atomic_inc(&dev_priv->irq_received);
 

@@ -1407,33 +1407,33 @@ static irqreturn_t ironlake_irq_handler(int irq, void *arg)
 	I915_WRITE(SDEIER, 0);
 	POSTING_READ(SDEIER);
 
-	de_iir = I915_READ(DEIIR);
 	gt_iir = I915_READ(GTIIR);
-	if (IS_GEN6(dev))
-		pm_iir = I915_READ(GEN6_PMIIR);
-
-	if (de_iir == 0 && gt_iir == 0 && pm_iir == 0)
-		goto done;
-
-	ret = IRQ_HANDLED;
-
-	if (IS_GEN5(dev))
-		ilk_gt_irq_handler(dev, dev_priv, gt_iir);
-	else
-		snb_gt_irq_handler(dev, dev_priv, gt_iir);
+	if (gt_iir) {
+		if (IS_GEN5(dev))
+			ilk_gt_irq_handler(dev, dev_priv, gt_iir);
+		else
+			snb_gt_irq_handler(dev, dev_priv, gt_iir);
+		I915_WRITE(GTIIR, gt_iir);
+		ret = IRQ_HANDLED;
+	}
 
-	if (de_iir)
+	de_iir = I915_READ(DEIIR);
+	if (de_iir) {
 		ilk_display_irq_handler(dev, de_iir);
+		I915_WRITE(DEIIR, de_iir);
+		ret = IRQ_HANDLED;
+	}
 
-	if (pm_iir & GEN6_PM_RPS_EVENTS)
-		gen6_rps_irq_handler(dev_priv, pm_iir);
-
-	I915_WRITE(GTIIR, gt_iir);
-	I915_WRITE(DEIIR, de_iir);
-	if (pm_iir)
-		I915_WRITE(GEN6_PMIIR, pm_iir);
+	if (IS_GEN6(dev)) {
+		u32 pm_iir = I915_READ(GEN6_PMIIR);
+		if (pm_iir) {
+			if (pm_iir & GEN6_PM_RPS_EVENTS)
+				gen6_rps_irq_handler(dev_priv, pm_iir);
+			I915_WRITE(GEN6_PMIIR, pm_iir);
+			ret = IRQ_HANDLED;
+		}
+	}
 
-done:
 	I915_WRITE(DEIER, de_ier);
 	POSTING_READ(DEIER);
 	I915_WRITE(SDEIER, sde_ier);
Signature · 1 line
1.8.1.2
Reviewed-by on v1 5/9
#16 lore raw

Re: [PATCH 5/9] drm/i915: reorganize ironlake_irq_handler

Quoted 7 lines · Paulo Zanoni

From: Paulo Zanoni <[email protected]>

The ironlake_irq_handler and ivybridge_irq_handler functions do basically the same thing, but they have different implementation styles. With this patch we reorganize ironlake_irq_handler in a way that makes it look very similar to ivybridge_irq_handler.

Quoted 7 lines

One of the advantages of this new function style is that we don't write 0 to the IIR registers anymore.

v2: - Rebase due to changes on previous patches
- Move pm_iir to a tighter scope (Chris)

Signed-off-by Paulo Zanoni <[email protected]>
Reviewed-by Mika Kuoppala <[email protected]>
Quoted 80 lines
---
 drivers/gpu/drm/i915/i915_irq.c | 46 ++++++++++++++++++++---------------------
 1 file changed, 23 insertions(+), 23 deletions(-)
drivers/gpu/drm/i915/i915_irq.c +23 -23
index c674dc3..88eb380 100644
@@ -1389,7 +1389,7 @@ static irqreturn_t ironlake_irq_handler(int irq, void *arg)
 	struct drm_device *dev = (struct drm_device *) arg;
 	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
 	int ret = IRQ_NONE;
-	u32 de_iir, gt_iir, de_ier, pm_iir = 0, sde_ier;
+	u32 de_iir, gt_iir, de_ier, sde_ier;
 
 	atomic_inc(&dev_priv->irq_received);
 

@@ -1407,33 +1407,33 @@ static irqreturn_t ironlake_irq_handler(int irq, void *arg)
 	I915_WRITE(SDEIER, 0);
 	POSTING_READ(SDEIER);
 
-	de_iir = I915_READ(DEIIR);
 	gt_iir = I915_READ(GTIIR);
-	if (IS_GEN6(dev))
-		pm_iir = I915_READ(GEN6_PMIIR);
-
-	if (de_iir == 0 && gt_iir == 0 && pm_iir == 0)
-		goto done;
-
-	ret = IRQ_HANDLED;
-
-	if (IS_GEN5(dev))
-		ilk_gt_irq_handler(dev, dev_priv, gt_iir);
-	else
-		snb_gt_irq_handler(dev, dev_priv, gt_iir);
+	if (gt_iir) {
+		if (IS_GEN5(dev))
+			ilk_gt_irq_handler(dev, dev_priv, gt_iir);
+		else
+			snb_gt_irq_handler(dev, dev_priv, gt_iir);
+		I915_WRITE(GTIIR, gt_iir);
+		ret = IRQ_HANDLED;
+	}
 
-	if (de_iir)
+	de_iir = I915_READ(DEIIR);
+	if (de_iir) {
 		ilk_display_irq_handler(dev, de_iir);
+		I915_WRITE(DEIIR, de_iir);
+		ret = IRQ_HANDLED;
+	}
 
-	if (pm_iir & GEN6_PM_RPS_EVENTS)
-		gen6_rps_irq_handler(dev_priv, pm_iir);
-
-	I915_WRITE(GTIIR, gt_iir);
-	I915_WRITE(DEIIR, de_iir);
-	if (pm_iir)
-		I915_WRITE(GEN6_PMIIR, pm_iir);
+	if (IS_GEN6(dev)) {
+		u32 pm_iir = I915_READ(GEN6_PMIIR);
+		if (pm_iir) {
+			if (pm_iir & GEN6_PM_RPS_EVENTS)
+				gen6_rps_irq_handler(dev_priv, pm_iir);
+			I915_WRITE(GEN6_PMIIR, pm_iir);
+			ret = IRQ_HANDLED;
+		}
+	}
 
-done:
 	I915_WRITE(DEIER, de_ier);
 	POSTING_READ(DEIER);
 	I915_WRITE(SDEIER, sde_ier);
Signature · 6 lines
1.8.1.2

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
author
#17 lore raw

PATCH 6/10 drm/i915: POSTING_READ(DEIER) on ivybridge_irq_handler

In reply to Paulo Zanoni

From: Paulo Zanoni <[email protected]>

We have this POSTING_READ inside ironlake_irq_handler. I suppose we also want it on IVB because we want to stop the IRQ handler as soon as possible at this point.

Signed-off-by Paulo Zanoni <[email protected]>
---
 drivers/gpu/drm/i915/i915_irq.c | 1 +
 1 file changed, 1 insertion(+)
drivers/gpu/drm/i915/i915_irq.c +1 -0
index 7bc36ae..bdaab93 100644
@@ -1310,6 +1310,7 @@ static irqreturn_t ivybridge_irq_handler(int irq, void *arg)
 	/* disable master interrupt before clearing iir  */
 	de_ier = I915_READ(DEIER);
 	I915_WRITE(DEIER, de_ier & ~DE_MASTER_IRQ_CONTROL);
+	POSTING_READ(DEIER);
 
 	/* Disable south interrupts. We'll only write to SDEIIR once, so further
 	 * interrupts will will be stored on its back queue, and then we'll be
Signature · 1 line
1.8.1.2
Reviewed-by on v1 6/10
#18 lore raw

Re: [PATCH 06/10] drm/i915: POSTING_READ(DEIER) on ivybridge_irq_handler

Quoted 7 lines · Paulo Zanoni

From: Paulo Zanoni <[email protected]>

We have this POSTING_READ inside ironlake_irq_handler. I suppose we also want it on IVB because we want to stop the IRQ handler as soon as possible at this point.

Signed-off-by Paulo Zanoni <[email protected]>
Reviewed-by Mika Kuoppala <[email protected]>
Quoted 23 lines
---
 drivers/gpu/drm/i915/i915_irq.c | 1 +
 1 file changed, 1 insertion(+)
drivers/gpu/drm/i915/i915_irq.c +1 -0
index 7bc36ae..bdaab93 100644
@@ -1310,6 +1310,7 @@ static irqreturn_t ivybridge_irq_handler(int irq, void *arg)
 	/* disable master interrupt before clearing iir  */
 	de_ier = I915_READ(DEIER);
 	I915_WRITE(DEIER, de_ier & ~DE_MASTER_IRQ_CONTROL);
+	POSTING_READ(DEIER);
 
 	/* Disable south interrupts. We'll only write to SDEIIR once, so further
 	 * interrupts will will be stored on its back queue, and then we'll be
Signature · 6 lines
1.8.1.2

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
author
#19 lore raw

PATCH 7/10 drm/i915: add ILK/SNB support to ivybridge_irq_handler

In reply to Paulo Zanoni

From: Paulo Zanoni <[email protected]>

And then rename it to ironlake_irq_handler. Also move ilk_gt_irq_handler up to avoid forward declarations.

In the previous patches I did small modifications to both ironlake_irq_handler an ivybridge_irq_handler so they became very similar functions. Now it should be very easy to verify that all we need to add ILK/SNB support is to call ilk_gt_irq_handler, call ilk_display_irq_handler and avoid reading pm_iir on gen 5.

Signed-off-by Paulo Zanoni <[email protected]>
---
 drivers/gpu/drm/i915/i915_irq.c | 113 +++++++++++-----------------------------
 1 file changed, 31 insertions(+), 82 deletions(-)
drivers/gpu/drm/i915/i915_irq.c +31 -82
index bdaab93..3d49f0f 100644
@@ -849,6 +849,17 @@ static void ivybridge_parity_error_irq_handler(struct drm_device *dev)
 	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,
+			       u32 gt_iir)
+{
+	if (gt_iir &
+	    (GT_RENDER_USER_INTERRUPT | GT_RENDER_PIPECTL_NOTIFY_INTERRUPT))
+		notify_ring(dev, &dev_priv->ring[RCS]);
+	if (gt_iir & ILK_BSD_USER_INTERRUPT)
+		notify_ring(dev, &dev_priv->ring[VCS]);
+}
+
 static void snb_gt_irq_handler(struct drm_device *dev,
 			       struct drm_i915_private *dev_priv,
 			       u32 gt_iir)

@@ -1290,7 +1301,7 @@ static void ivb_display_irq_handler(struct drm_device *dev, u32 de_iir)
 	}
 }
 
-static irqreturn_t ivybridge_irq_handler(int irq, void *arg)
+static irqreturn_t ironlake_irq_handler(int irq, void *arg)
 {
 	struct drm_device *dev = (struct drm_device *) arg;
 	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;

@@ -1334,27 +1345,34 @@ static irqreturn_t ivybridge_irq_handler(int irq, void *arg)
 
 	gt_iir = I915_READ(GTIIR);
 	if (gt_iir) {
-		snb_gt_irq_handler(dev, dev_priv, gt_iir);
+		if (IS_GEN5(dev))
+			ilk_gt_irq_handler(dev, dev_priv, gt_iir);
+		else
+			snb_gt_irq_handler(dev, dev_priv, gt_iir);
 		I915_WRITE(GTIIR, gt_iir);
 		ret = IRQ_HANDLED;
 	}
 
 	de_iir = I915_READ(DEIIR);
 	if (de_iir) {
-		ivb_display_irq_handler(dev, de_iir);
-
+		if (INTEL_INFO(dev)->gen <= 6)
+			ilk_display_irq_handler(dev, de_iir);
+		else
+			ivb_display_irq_handler(dev, de_iir);
 		I915_WRITE(DEIIR, de_iir);
 		ret = IRQ_HANDLED;
 	}
 
-	pm_iir = I915_READ(GEN6_PMIIR);
-	if (pm_iir) {
-		if (IS_HASWELL(dev))
-			hsw_pm_irq_handler(dev_priv, pm_iir);
-		else if (pm_iir & GEN6_PM_RPS_EVENTS)
-			gen6_rps_irq_handler(dev_priv, pm_iir);
-		I915_WRITE(GEN6_PMIIR, pm_iir);
-		ret = IRQ_HANDLED;
+	if (!IS_GEN5(dev)) {
+		pm_iir = I915_READ(GEN6_PMIIR);
+		if (pm_iir) {
+			if (IS_HASWELL(dev))
+				hsw_pm_irq_handler(dev_priv, pm_iir);
+			else if (pm_iir & GEN6_PM_RPS_EVENTS)
+				gen6_rps_irq_handler(dev_priv, pm_iir);
+			I915_WRITE(GEN6_PMIIR, pm_iir);
+			ret = IRQ_HANDLED;
+		}
 	}
 
 	if (IS_HASWELL(dev)) {

@@ -1374,75 +1392,6 @@ static irqreturn_t ivybridge_irq_handler(int irq, void *arg)
 	return ret;
 }
 
-static void ilk_gt_irq_handler(struct drm_device *dev,
-			       struct drm_i915_private *dev_priv,
-			       u32 gt_iir)
-{
-	if (gt_iir &
-	    (GT_RENDER_USER_INTERRUPT | GT_RENDER_PIPECTL_NOTIFY_INTERRUPT))
-		notify_ring(dev, &dev_priv->ring[RCS]);
-	if (gt_iir & ILK_BSD_USER_INTERRUPT)
-		notify_ring(dev, &dev_priv->ring[VCS]);
-}
-
-static irqreturn_t ironlake_irq_handler(int irq, void *arg)
-{
-	struct drm_device *dev = (struct drm_device *) arg;
-	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
-	int ret = IRQ_NONE;
-	u32 de_iir, gt_iir, de_ier, pm_iir = 0, sde_ier;
-
-	atomic_inc(&dev_priv->irq_received);
-
-	/* disable master interrupt before clearing iir  */
-	de_ier = I915_READ(DEIER);
-	I915_WRITE(DEIER, de_ier & ~DE_MASTER_IRQ_CONTROL);
-	POSTING_READ(DEIER);
-
-	/* Disable south interrupts. We'll only write to SDEIIR once, so further
-	 * interrupts will will be stored on its back queue, and then we'll be
-	 * able to process them after we restore SDEIER (as soon as we restore
-	 * it, we'll get an interrupt if SDEIIR still has something to process
-	 * due to its back queue). */
-	sde_ier = I915_READ(SDEIER);
-	I915_WRITE(SDEIER, 0);
-	POSTING_READ(SDEIER);
-
-	gt_iir = I915_READ(GTIIR);
-	if (gt_iir) {
-		if (IS_GEN5(dev))
-			ilk_gt_irq_handler(dev, dev_priv, gt_iir);
-		else
-			snb_gt_irq_handler(dev, dev_priv, gt_iir);
-		I915_WRITE(GTIIR, gt_iir);
-		ret = IRQ_HANDLED;
-	}
-
-	de_iir = I915_READ(DEIIR);
-	if (de_iir) {
-		ilk_display_irq_handler(dev, de_iir);
-		I915_WRITE(DEIIR, de_iir);
-		ret = IRQ_HANDLED;
-	}
-
-	if (IS_GEN6(dev)) {
-		pm_iir = I915_READ(GEN6_PMIIR);
-		if (pm_iir) {
-			if (pm_iir & GEN6_PM_RPS_EVENTS)
-				gen6_rps_irq_handler(dev_priv, pm_iir);
-			I915_WRITE(GEN6_PMIIR, pm_iir);
-			ret = IRQ_HANDLED;
-		}
-	}
-
-	I915_WRITE(DEIER, de_ier);
-	POSTING_READ(DEIER);
-	I915_WRITE(SDEIER, sde_ier);
-	POSTING_READ(SDEIER);
-
-	return ret;
-}
-
 /**
  * i915_error_work_func - do process context error handling work
  * @work: work struct

@@ -3114,7 +3063,7 @@ void intel_irq_init(struct drm_device *dev)
 		dev_priv->display.hpd_irq_setup = i915_hpd_irq_setup;
 	} else if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev)) {
 		/* Share uninstall handlers with ILK/SNB */
-		dev->driver->irq_handler = ivybridge_irq_handler;
+		dev->driver->irq_handler = ironlake_irq_handler;
 		dev->driver->irq_preinstall = ironlake_irq_preinstall;
 		dev->driver->irq_postinstall = ivybridge_irq_postinstall;
 		dev->driver->irq_uninstall = ironlake_irq_uninstall;
Signature · 1 line
1.8.1.2
author
#20 lore raw

PATCH 7/9 drm/i915: add ILK/SNB support to ivybridge_irq_handler

From: Paulo Zanoni <[email protected]>

And then rename it to ironlake_irq_handler. Also move ilk_gt_irq_handler up to avoid forward declarations.

In the previous patches I did small modifications to both ironlake_irq_handler an ivybridge_irq_handler so they became very similar functions. Now it should be very easy to verify that all we need to add ILK/SNB support is to call ilk_gt_irq_handler, call ilk_display_irq_handler and avoid reading pm_iir on gen 5.

v2: - Rebase due to changes on the previous patches
- Move pm_iir to a tighter scope (Chris)
- Change some Gen checks for readability

Signed-off-by Paulo Zanoni <[email protected]>
---
 drivers/gpu/drm/i915/i915_irq.c | 115 +++++++++++-----------------------------
 1 file changed, 32 insertions(+), 83 deletions(-)
drivers/gpu/drm/i915/i915_irq.c +32 -83
index ba38aa8..f54a02b 100644
@@ -849,6 +849,17 @@ static void ivybridge_parity_error_irq_handler(struct drm_device *dev)
 	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,
+			       u32 gt_iir)
+{
+	if (gt_iir &
+	    (GT_RENDER_USER_INTERRUPT | GT_RENDER_PIPECTL_NOTIFY_INTERRUPT))
+		notify_ring(dev, &dev_priv->ring[RCS]);
+	if (gt_iir & ILK_BSD_USER_INTERRUPT)
+		notify_ring(dev, &dev_priv->ring[VCS]);
+}
+
 static void snb_gt_irq_handler(struct drm_device *dev,
 			       struct drm_i915_private *dev_priv,
 			       u32 gt_iir)

@@ -1290,11 +1301,11 @@ static void ivb_display_irq_handler(struct drm_device *dev, u32 de_iir)
 	}
 }
 
-static irqreturn_t ivybridge_irq_handler(int irq, void *arg)
+static irqreturn_t ironlake_irq_handler(int irq, void *arg)
 {
 	struct drm_device *dev = (struct drm_device *) arg;
 	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
-	u32 de_iir, gt_iir, de_ier, pm_iir, sde_ier = 0;
+	u32 de_iir, gt_iir, de_ier, sde_ier = 0;
 	irqreturn_t ret = IRQ_NONE;
 
 	atomic_inc(&dev_priv->irq_received);

@@ -1334,27 +1345,34 @@ static irqreturn_t ivybridge_irq_handler(int irq, void *arg)
 
 	gt_iir = I915_READ(GTIIR);
 	if (gt_iir) {
-		snb_gt_irq_handler(dev, dev_priv, gt_iir);
+		if (IS_GEN5(dev))
+			ilk_gt_irq_handler(dev, dev_priv, gt_iir);
+		else
+			snb_gt_irq_handler(dev, dev_priv, gt_iir);
 		I915_WRITE(GTIIR, gt_iir);
 		ret = IRQ_HANDLED;
 	}
 
 	de_iir = I915_READ(DEIIR);
 	if (de_iir) {
-		ivb_display_irq_handler(dev, de_iir);
-
+		if (INTEL_INFO(dev)->gen >= 7)
+			ivb_display_irq_handler(dev, de_iir);
+		else
+			ilk_display_irq_handler(dev, de_iir);
 		I915_WRITE(DEIIR, de_iir);
 		ret = IRQ_HANDLED;
 	}
 
-	pm_iir = I915_READ(GEN6_PMIIR);
-	if (pm_iir) {
-		if (IS_HASWELL(dev))
-			hsw_pm_irq_handler(dev_priv, pm_iir);
-		else if (pm_iir & GEN6_PM_RPS_EVENTS)
-			gen6_rps_irq_handler(dev_priv, pm_iir);
-		I915_WRITE(GEN6_PMIIR, pm_iir);
-		ret = IRQ_HANDLED;
+	if (INTEL_INFO(dev)->gen >= 6) {
+		u32 pm_iir = I915_READ(GEN6_PMIIR);
+		if (pm_iir) {
+			if (IS_HASWELL(dev))
+				hsw_pm_irq_handler(dev_priv, pm_iir);
+			else if (pm_iir & GEN6_PM_RPS_EVENTS)
+				gen6_rps_irq_handler(dev_priv, pm_iir);
+			I915_WRITE(GEN6_PMIIR, pm_iir);
+			ret = IRQ_HANDLED;
+		}
 	}
 
 	if (IS_HASWELL(dev)) {

@@ -1374,75 +1392,6 @@ static irqreturn_t ivybridge_irq_handler(int irq, void *arg)
 	return ret;
 }
 
-static void ilk_gt_irq_handler(struct drm_device *dev,
-			       struct drm_i915_private *dev_priv,
-			       u32 gt_iir)
-{
-	if (gt_iir &
-	    (GT_RENDER_USER_INTERRUPT | GT_RENDER_PIPECTL_NOTIFY_INTERRUPT))
-		notify_ring(dev, &dev_priv->ring[RCS]);
-	if (gt_iir & ILK_BSD_USER_INTERRUPT)
-		notify_ring(dev, &dev_priv->ring[VCS]);
-}
-
-static irqreturn_t ironlake_irq_handler(int irq, void *arg)
-{
-	struct drm_device *dev = (struct drm_device *) arg;
-	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
-	int ret = IRQ_NONE;
-	u32 de_iir, gt_iir, de_ier, sde_ier;
-
-	atomic_inc(&dev_priv->irq_received);
-
-	/* disable master interrupt before clearing iir  */
-	de_ier = I915_READ(DEIER);
-	I915_WRITE(DEIER, de_ier & ~DE_MASTER_IRQ_CONTROL);
-	POSTING_READ(DEIER);
-
-	/* Disable south interrupts. We'll only write to SDEIIR once, so further
-	 * interrupts will will be stored on its back queue, and then we'll be
-	 * able to process them after we restore SDEIER (as soon as we restore
-	 * it, we'll get an interrupt if SDEIIR still has something to process
-	 * due to its back queue). */
-	sde_ier = I915_READ(SDEIER);
-	I915_WRITE(SDEIER, 0);
-	POSTING_READ(SDEIER);
-
-	gt_iir = I915_READ(GTIIR);
-	if (gt_iir) {
-		if (IS_GEN5(dev))
-			ilk_gt_irq_handler(dev, dev_priv, gt_iir);
-		else
-			snb_gt_irq_handler(dev, dev_priv, gt_iir);
-		I915_WRITE(GTIIR, gt_iir);
-		ret = IRQ_HANDLED;
-	}
-
-	de_iir = I915_READ(DEIIR);
-	if (de_iir) {
-		ilk_display_irq_handler(dev, de_iir);
-		I915_WRITE(DEIIR, de_iir);
-		ret = IRQ_HANDLED;
-	}
-
-	if (IS_GEN6(dev)) {
-		u32 pm_iir = I915_READ(GEN6_PMIIR);
-		if (pm_iir) {
-			if (pm_iir & GEN6_PM_RPS_EVENTS)
-				gen6_rps_irq_handler(dev_priv, pm_iir);
-			I915_WRITE(GEN6_PMIIR, pm_iir);
-			ret = IRQ_HANDLED;
-		}
-	}
-
-	I915_WRITE(DEIER, de_ier);
-	POSTING_READ(DEIER);
-	I915_WRITE(SDEIER, sde_ier);
-	POSTING_READ(SDEIER);
-
-	return ret;
-}
-
 /**
  * i915_error_work_func - do process context error handling work
  * @work: work struct

@@ -3114,7 +3063,7 @@ void intel_irq_init(struct drm_device *dev)
 		dev_priv->display.hpd_irq_setup = i915_hpd_irq_setup;
 	} else if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev)) {
 		/* Share uninstall handlers with ILK/SNB */
-		dev->driver->irq_handler = ivybridge_irq_handler;
+		dev->driver->irq_handler = ironlake_irq_handler;
 		dev->driver->irq_preinstall = ironlake_irq_preinstall;
 		dev->driver->irq_postinstall = ivybridge_irq_postinstall;
 		dev->driver->irq_uninstall = ironlake_irq_uninstall;
Signature · 1 line
1.8.1.2
Reviewed-by on v1 7/9
#21 lore raw

Re: [PATCH 7/9] drm/i915: add ILK/SNB support to ivybridge_irq_handler

Quoted 11 lines · Paulo Zanoni

From: Paulo Zanoni <[email protected]>

And then rename it to ironlake_irq_handler. Also move ilk_gt_irq_handler up to avoid forward declarations.

In the previous patches I did small modifications to both ironlake_irq_handler an ivybridge_irq_handler so they became very similar functions. Now it should be very easy to verify that all we need to add ILK/SNB support is to call ilk_gt_irq_handler, call ilk_display_irq_handler and avoid reading pm_iir on gen 5.

Quoted 5 lines

v2: - Rebase due to changes on the previous patches
- Move pm_iir to a tighter scope (Chris)
- Change some Gen checks for readability

Signed-off-by Paulo Zanoni <[email protected]>
Reviewed-by Mika Kuoppala <[email protected]>
Quoted 178 lines
---
 drivers/gpu/drm/i915/i915_irq.c | 115 +++++++++++-----------------------------
 1 file changed, 32 insertions(+), 83 deletions(-)
drivers/gpu/drm/i915/i915_irq.c +32 -83
index ba38aa8..f54a02b 100644
@@ -849,6 +849,17 @@ static void ivybridge_parity_error_irq_handler(struct drm_device *dev)
 	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,
+			       u32 gt_iir)
+{
+	if (gt_iir &
+	    (GT_RENDER_USER_INTERRUPT | GT_RENDER_PIPECTL_NOTIFY_INTERRUPT))
+		notify_ring(dev, &dev_priv->ring[RCS]);
+	if (gt_iir & ILK_BSD_USER_INTERRUPT)
+		notify_ring(dev, &dev_priv->ring[VCS]);
+}
+
 static void snb_gt_irq_handler(struct drm_device *dev,
 			       struct drm_i915_private *dev_priv,
 			       u32 gt_iir)

@@ -1290,11 +1301,11 @@ static void ivb_display_irq_handler(struct drm_device *dev, u32 de_iir)
 	}
 }
 
-static irqreturn_t ivybridge_irq_handler(int irq, void *arg)
+static irqreturn_t ironlake_irq_handler(int irq, void *arg)
 {
 	struct drm_device *dev = (struct drm_device *) arg;
 	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
-	u32 de_iir, gt_iir, de_ier, pm_iir, sde_ier = 0;
+	u32 de_iir, gt_iir, de_ier, sde_ier = 0;
 	irqreturn_t ret = IRQ_NONE;
 
 	atomic_inc(&dev_priv->irq_received);

@@ -1334,27 +1345,34 @@ static irqreturn_t ivybridge_irq_handler(int irq, void *arg)
 
 	gt_iir = I915_READ(GTIIR);
 	if (gt_iir) {
-		snb_gt_irq_handler(dev, dev_priv, gt_iir);
+		if (IS_GEN5(dev))
+			ilk_gt_irq_handler(dev, dev_priv, gt_iir);
+		else
+			snb_gt_irq_handler(dev, dev_priv, gt_iir);
 		I915_WRITE(GTIIR, gt_iir);
 		ret = IRQ_HANDLED;
 	}
 
 	de_iir = I915_READ(DEIIR);
 	if (de_iir) {
-		ivb_display_irq_handler(dev, de_iir);
-
+		if (INTEL_INFO(dev)->gen >= 7)
+			ivb_display_irq_handler(dev, de_iir);
+		else
+			ilk_display_irq_handler(dev, de_iir);
 		I915_WRITE(DEIIR, de_iir);
 		ret = IRQ_HANDLED;
 	}
 
-	pm_iir = I915_READ(GEN6_PMIIR);
-	if (pm_iir) {
-		if (IS_HASWELL(dev))
-			hsw_pm_irq_handler(dev_priv, pm_iir);
-		else if (pm_iir & GEN6_PM_RPS_EVENTS)
-			gen6_rps_irq_handler(dev_priv, pm_iir);
-		I915_WRITE(GEN6_PMIIR, pm_iir);
-		ret = IRQ_HANDLED;
+	if (INTEL_INFO(dev)->gen >= 6) {
+		u32 pm_iir = I915_READ(GEN6_PMIIR);
+		if (pm_iir) {
+			if (IS_HASWELL(dev))
+				hsw_pm_irq_handler(dev_priv, pm_iir);
+			else if (pm_iir & GEN6_PM_RPS_EVENTS)
+				gen6_rps_irq_handler(dev_priv, pm_iir);
+			I915_WRITE(GEN6_PMIIR, pm_iir);
+			ret = IRQ_HANDLED;
+		}
 	}
 
 	if (IS_HASWELL(dev)) {

@@ -1374,75 +1392,6 @@ static irqreturn_t ivybridge_irq_handler(int irq, void *arg)
 	return ret;
 }
 
-static void ilk_gt_irq_handler(struct drm_device *dev,
-			       struct drm_i915_private *dev_priv,
-			       u32 gt_iir)
-{
-	if (gt_iir &
-	    (GT_RENDER_USER_INTERRUPT | GT_RENDER_PIPECTL_NOTIFY_INTERRUPT))
-		notify_ring(dev, &dev_priv->ring[RCS]);
-	if (gt_iir & ILK_BSD_USER_INTERRUPT)
-		notify_ring(dev, &dev_priv->ring[VCS]);
-}
-
-static irqreturn_t ironlake_irq_handler(int irq, void *arg)
-{
-	struct drm_device *dev = (struct drm_device *) arg;
-	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
-	int ret = IRQ_NONE;
-	u32 de_iir, gt_iir, de_ier, sde_ier;
-
-	atomic_inc(&dev_priv->irq_received);
-
-	/* disable master interrupt before clearing iir  */
-	de_ier = I915_READ(DEIER);
-	I915_WRITE(DEIER, de_ier & ~DE_MASTER_IRQ_CONTROL);
-	POSTING_READ(DEIER);
-
-	/* Disable south interrupts. We'll only write to SDEIIR once, so further
-	 * interrupts will will be stored on its back queue, and then we'll be
-	 * able to process them after we restore SDEIER (as soon as we restore
-	 * it, we'll get an interrupt if SDEIIR still has something to process
-	 * due to its back queue). */
-	sde_ier = I915_READ(SDEIER);
-	I915_WRITE(SDEIER, 0);
-	POSTING_READ(SDEIER);
-
-	gt_iir = I915_READ(GTIIR);
-	if (gt_iir) {
-		if (IS_GEN5(dev))
-			ilk_gt_irq_handler(dev, dev_priv, gt_iir);
-		else
-			snb_gt_irq_handler(dev, dev_priv, gt_iir);
-		I915_WRITE(GTIIR, gt_iir);
-		ret = IRQ_HANDLED;
-	}
-
-	de_iir = I915_READ(DEIIR);
-	if (de_iir) {
-		ilk_display_irq_handler(dev, de_iir);
-		I915_WRITE(DEIIR, de_iir);
-		ret = IRQ_HANDLED;
-	}
-
-	if (IS_GEN6(dev)) {
-		u32 pm_iir = I915_READ(GEN6_PMIIR);
-		if (pm_iir) {
-			if (pm_iir & GEN6_PM_RPS_EVENTS)
-				gen6_rps_irq_handler(dev_priv, pm_iir);
-			I915_WRITE(GEN6_PMIIR, pm_iir);
-			ret = IRQ_HANDLED;
-		}
-	}
-
-	I915_WRITE(DEIER, de_ier);
-	POSTING_READ(DEIER);
-	I915_WRITE(SDEIER, sde_ier);
-	POSTING_READ(SDEIER);
-
-	return ret;
-}
-
 /**
  * i915_error_work_func - do process context error handling work
  * @work: work struct

@@ -3114,7 +3063,7 @@ void intel_irq_init(struct drm_device *dev)
 		dev_priv->display.hpd_irq_setup = i915_hpd_irq_setup;
 	} else if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev)) {
 		/* Share uninstall handlers with ILK/SNB */
-		dev->driver->irq_handler = ivybridge_irq_handler;
+		dev->driver->irq_handler = ironlake_irq_handler;
 		dev->driver->irq_preinstall = ironlake_irq_preinstall;
 		dev->driver->irq_postinstall = ivybridge_irq_postinstall;
 		dev->driver->irq_uninstall = ironlake_irq_uninstall;
Signature · 6 lines
1.8.1.2

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
author
#22 lore raw

PATCH 8/10 drm/i915: kill ivybridge_enable_vblank

In reply to Paulo Zanoni

From: Paulo Zanoni <[email protected]>

It's the same thing as ironlake_enable_vblank, with the exception of the bit register, so add IVB/HSW support to ironlake_enable_vblank and kill ivybridge_enable_vblank.

Signed-off-by Paulo Zanoni <[email protected]>
---
 drivers/gpu/drm/i915/i915_irq.c | 25 +++++++------------------
 1 file changed, 7 insertions(+), 18 deletions(-)
drivers/gpu/drm/i915/i915_irq.c +7 -18
index 3d49f0f..d26bbb3 100644
@@ -1663,29 +1663,18 @@ static int ironlake_enable_vblank(struct drm_device *dev, int pipe)
 {
 	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
 	unsigned long irqflags;
+	uint32_t bit;
 
 	if (!i915_pipe_enabled(dev, pipe))
 		return -EINVAL;
 
-	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
-	ironlake_enable_display_irq(dev_priv, (pipe == 0) ?
-				    DE_PIPEA_VBLANK : DE_PIPEB_VBLANK);
-	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
-
-	return 0;
-}
-
-static int ivybridge_enable_vblank(struct drm_device *dev, int pipe)
-{
-	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
-	unsigned long irqflags;
-
-	if (!i915_pipe_enabled(dev, pipe))
-		return -EINVAL;
+	if (INTEL_INFO(dev)->gen >= 7)
+		bit = DE_PIPEA_VBLANK_IVB << (pipe * 5);
+	else
+		bit = (pipe == 0) ? DE_PIPEA_VBLANK : DE_PIPEB_VBLANK;
 
 	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
-	ironlake_enable_display_irq(dev_priv,
-				    DE_PIPEA_VBLANK_IVB << (5 * pipe));
+	ironlake_enable_display_irq(dev_priv, bit);
 	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
 
 	return 0;

@@ -3067,7 +3056,7 @@ void intel_irq_init(struct drm_device *dev)
 		dev->driver->irq_preinstall = ironlake_irq_preinstall;
 		dev->driver->irq_postinstall = ivybridge_irq_postinstall;
 		dev->driver->irq_uninstall = ironlake_irq_uninstall;
-		dev->driver->enable_vblank = ivybridge_enable_vblank;
+		dev->driver->enable_vblank = ironlake_enable_vblank;
 		dev->driver->disable_vblank = ivybridge_disable_vblank;
 		dev_priv->display.hpd_irq_setup = ibx_hpd_irq_setup;
 	} else if (HAS_PCH_SPLIT(dev)) {
Signature · 1 line
1.8.1.2
#23 lore raw

Re: [PATCH 08/10] drm/i915: kill ivybridge_enable_vblank

Quoted 5 lines · Paulo Zanoni

From: Paulo Zanoni <[email protected]>

It's the same thing as ironlake_enable_vblank, with the exception of the bit register, so add IVB/HSW support to ironlake_enable_vblank and kill ivybridge_enable_vblank.

If you are a little more creative, you can make the two paths look even move alike.
-Chris

Signature · 1 line
Chris Wilson, Intel Open Source Technology Centre
author
#24 lore raw

PATCH 8/9 drm/i915: kill Ivybridge vblank irq vfuncs

From: Paulo Zanoni <[email protected]>

The IVB funtions are exactly the same as the ILK ones, with the exception of the bit register. So add IVB/HSW support to ironlake_enable_vblank and ironlake_disable_vblank, then kill the ivybridge functions.

Signed-off-by Paulo Zanoni <[email protected]>
---
 drivers/gpu/drm/i915/i915_irq.c | 41 ++++++++---------------------------------
 drivers/gpu/drm/i915/i915_reg.h |  3 +++
 2 files changed, 11 insertions(+), 33 deletions(-)

This replaces patches 8 and 9 from the series. I'm not really sure what Chris had in mind when he mentioned I could be a little more creative, so this is my attempt.

drivers/gpu/drm/i915/i915_irq.c +8 -33
index f54a02b..d084057 100644
@@ -1663,29 +1663,14 @@ static int ironlake_enable_vblank(struct drm_device *dev, int pipe)
 {
 	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
 	unsigned long irqflags;
+	uint32_t bit = (INTEL_INFO(dev)->gen >= 7) ? DE_PIPE_VBLANK_IVB(pipe) :
+						     DE_PIPE_VBLANK_ILK(pipe);
 
 	if (!i915_pipe_enabled(dev, pipe))
 		return -EINVAL;
 
 	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
-	ironlake_enable_display_irq(dev_priv, (pipe == 0) ?
-				    DE_PIPEA_VBLANK : DE_PIPEB_VBLANK);
-	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
-
-	return 0;
-}
-
-static int ivybridge_enable_vblank(struct drm_device *dev, int pipe)
-{
-	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
-	unsigned long irqflags;
-
-	if (!i915_pipe_enabled(dev, pipe))
-		return -EINVAL;
-
-	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
-	ironlake_enable_display_irq(dev_priv,
-				    DE_PIPEA_VBLANK_IVB << (5 * pipe));
+	ironlake_enable_display_irq(dev_priv, bit);
 	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
 
 	return 0;

@@ -1736,21 +1721,11 @@ static void ironlake_disable_vblank(struct drm_device *dev, int pipe)
 {
 	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
 	unsigned long irqflags;
+	uint32_t bit = (INTEL_INFO(dev)->gen >= 7) ? DE_PIPE_VBLANK_IVB(pipe) :
+						     DE_PIPE_VBLANK_ILK(pipe);
 
 	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
-	ironlake_disable_display_irq(dev_priv, (pipe == 0) ?
-				     DE_PIPEA_VBLANK : DE_PIPEB_VBLANK);
-	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
-}
-
-static void ivybridge_disable_vblank(struct drm_device *dev, int pipe)
-{
-	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
-	unsigned long irqflags;
-
-	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
-	ironlake_disable_display_irq(dev_priv,
-				     DE_PIPEA_VBLANK_IVB << (pipe * 5));
+	ironlake_disable_display_irq(dev_priv, bit);
 	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
 }
 

@@ -3067,8 +3042,8 @@ void intel_irq_init(struct drm_device *dev)
 		dev->driver->irq_preinstall = ironlake_irq_preinstall;
 		dev->driver->irq_postinstall = ivybridge_irq_postinstall;
 		dev->driver->irq_uninstall = ironlake_irq_uninstall;
-		dev->driver->enable_vblank = ivybridge_enable_vblank;
-		dev->driver->disable_vblank = ivybridge_disable_vblank;
+		dev->driver->enable_vblank = ironlake_enable_vblank;
+		dev->driver->disable_vblank = ironlake_disable_vblank;
 		dev_priv->display.hpd_irq_setup = ibx_hpd_irq_setup;
 	} else if (HAS_PCH_SPLIT(dev)) {
 		dev->driver->irq_handler = ironlake_irq_handler;
drivers/gpu/drm/i915/i915_reg.h +3 -0
index 9556dff..dd2306e 100644
@@ -3729,6 +3729,9 @@
 #define DE_PLANEA_FLIP_DONE_IVB		(1<<3)
 #define DE_PIPEA_VBLANK_IVB		(1<<0)
 
+#define DE_PIPE_VBLANK_ILK(pipe)	(1 << ((pipe * 8) + 7))
+#define DE_PIPE_VBLANK_IVB(pipe)	(1 << (pipe * 5))
+
 #define VLV_MASTER_IER			0x4400c /* Gunit master IER */
 #define   MASTER_INTERRUPT_ENABLE	(1<<31)
Signature · 1 line
1.8.1.2
Reviewed-by on v1 8/9
#25 lore raw

Re: [PATCH 8/9] drm/i915: kill Ivybridge vblank irq vfuncs

Quoted 8 lines · Paulo Zanoni

From: Paulo Zanoni <[email protected]>

The IVB funtions are exactly the same as the ILK ones, with the exception of the bit register. So add IVB/HSW support to ironlake_enable_vblank and ironlake_disable_vblank, then kill the ivybridge functions.

Signed-off-by Paulo Zanoni <[email protected]>
Reviewed-by Mika Kuoppala <[email protected]>
Quoted 103 lines
---
 drivers/gpu/drm/i915/i915_irq.c | 41 ++++++++---------------------------------
 drivers/gpu/drm/i915/i915_reg.h |  3 +++
 2 files changed, 11 insertions(+), 33 deletions(-)

This replaces patches 8 and 9 from the series. I'm not really sure what Chris had in mind when he mentioned I could be a little more creative, so this is my attempt.

drivers/gpu/drm/i915/i915_irq.c +8 -33
index f54a02b..d084057 100644
@@ -1663,29 +1663,14 @@ static int ironlake_enable_vblank(struct drm_device *dev, int pipe)
 {
 	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
 	unsigned long irqflags;
+	uint32_t bit = (INTEL_INFO(dev)->gen >= 7) ? DE_PIPE_VBLANK_IVB(pipe) :
+						     DE_PIPE_VBLANK_ILK(pipe);
 
 	if (!i915_pipe_enabled(dev, pipe))
 		return -EINVAL;
 
 	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
-	ironlake_enable_display_irq(dev_priv, (pipe == 0) ?
-				    DE_PIPEA_VBLANK : DE_PIPEB_VBLANK);
-	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
-
-	return 0;
-}
-
-static int ivybridge_enable_vblank(struct drm_device *dev, int pipe)
-{
-	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
-	unsigned long irqflags;
-
-	if (!i915_pipe_enabled(dev, pipe))
-		return -EINVAL;
-
-	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
-	ironlake_enable_display_irq(dev_priv,
-				    DE_PIPEA_VBLANK_IVB << (5 * pipe));
+	ironlake_enable_display_irq(dev_priv, bit);
 	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
 
 	return 0;

@@ -1736,21 +1721,11 @@ static void ironlake_disable_vblank(struct drm_device *dev, int pipe)
 {
 	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
 	unsigned long irqflags;
+	uint32_t bit = (INTEL_INFO(dev)->gen >= 7) ? DE_PIPE_VBLANK_IVB(pipe) :
+						     DE_PIPE_VBLANK_ILK(pipe);
 
 	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
-	ironlake_disable_display_irq(dev_priv, (pipe == 0) ?
-				     DE_PIPEA_VBLANK : DE_PIPEB_VBLANK);
-	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
-}
-
-static void ivybridge_disable_vblank(struct drm_device *dev, int pipe)
-{
-	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
-	unsigned long irqflags;
-
-	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
-	ironlake_disable_display_irq(dev_priv,
-				     DE_PIPEA_VBLANK_IVB << (pipe * 5));
+	ironlake_disable_display_irq(dev_priv, bit);
 	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
 }
 

@@ -3067,8 +3042,8 @@ void intel_irq_init(struct drm_device *dev)
 		dev->driver->irq_preinstall = ironlake_irq_preinstall;
 		dev->driver->irq_postinstall = ivybridge_irq_postinstall;
 		dev->driver->irq_uninstall = ironlake_irq_uninstall;
-		dev->driver->enable_vblank = ivybridge_enable_vblank;
-		dev->driver->disable_vblank = ivybridge_disable_vblank;
+		dev->driver->enable_vblank = ironlake_enable_vblank;
+		dev->driver->disable_vblank = ironlake_disable_vblank;
 		dev_priv->display.hpd_irq_setup = ibx_hpd_irq_setup;
 	} else if (HAS_PCH_SPLIT(dev)) {
 		dev->driver->irq_handler = ironlake_irq_handler;
drivers/gpu/drm/i915/i915_reg.h +3 -0
index 9556dff..dd2306e 100644
@@ -3729,6 +3729,9 @@
 #define DE_PLANEA_FLIP_DONE_IVB		(1<<3)
 #define DE_PIPEA_VBLANK_IVB		(1<<0)
 
+#define DE_PIPE_VBLANK_ILK(pipe)	(1 << ((pipe * 8) + 7))
+#define DE_PIPE_VBLANK_IVB(pipe)	(1 << (pipe * 5))
+
 #define VLV_MASTER_IER			0x4400c /* Gunit master IER */
 #define   MASTER_INTERRUPT_ENABLE	(1<<31)
Signature · 6 lines
1.8.1.2

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
author
#26 lore raw

PATCH 9/10 drm/i915: kill ivybridge_disable_vblank

In reply to Paulo Zanoni

From: Paulo Zanoni <[email protected]>

Same reasons as ivybridge_enable_vblank.

Signed-off-by Paulo Zanoni <[email protected]>
---
 drivers/gpu/drm/i915/i915_irq.c | 20 +++++++-------------
 1 file changed, 7 insertions(+), 13 deletions(-)
drivers/gpu/drm/i915/i915_irq.c +7 -13
index d26bbb3..0ad409a 100644
@@ -1725,21 +1725,15 @@ static void ironlake_disable_vblank(struct drm_device *dev, int pipe)
 {
 	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
 	unsigned long irqflags;
+	uint32_t bit;
 
-	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
-	ironlake_disable_display_irq(dev_priv, (pipe == 0) ?
-				     DE_PIPEA_VBLANK : DE_PIPEB_VBLANK);
-	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
-}
-
-static void ivybridge_disable_vblank(struct drm_device *dev, int pipe)
-{
-	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
-	unsigned long irqflags;
+	if (INTEL_INFO(dev)->gen >= 7)
+		bit = DE_PIPEA_VBLANK_IVB << (pipe * 5);
+	else
+		bit = (pipe == 0) ? DE_PIPEA_VBLANK : DE_PIPEB_VBLANK;
 
 	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
-	ironlake_disable_display_irq(dev_priv,
-				     DE_PIPEA_VBLANK_IVB << (pipe * 5));
+	ironlake_disable_display_irq(dev_priv, bit);
 	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
 }
 

@@ -3057,7 +3051,7 @@ void intel_irq_init(struct drm_device *dev)
 		dev->driver->irq_postinstall = ivybridge_irq_postinstall;
 		dev->driver->irq_uninstall = ironlake_irq_uninstall;
 		dev->driver->enable_vblank = ironlake_enable_vblank;
-		dev->driver->disable_vblank = ivybridge_disable_vblank;
+		dev->driver->disable_vblank = ironlake_disable_vblank;
 		dev_priv->display.hpd_irq_setup = ibx_hpd_irq_setup;
 	} else if (HAS_PCH_SPLIT(dev)) {
 		dev->driver->irq_handler = ironlake_irq_handler;
Signature · 1 line
1.8.1.2
author
#27 lore raw

PATCH 10/10 drm/i915: kill ivybridge_irq_postinstall

In reply to Paulo Zanoni

From: Paulo Zanoni <[email protected]>

It was very similar to ironlake_irq_postinstall, so IMHO merging both functions results in a code that is easier to maintain.

With this change, all the irq handler vfuncs between ironlake and ivybridge are now unified.

Signed-off-by Paulo Zanoni <[email protected]>
---
 drivers/gpu/drm/i915/i915_irq.c | 69 ++++++++++++-----------------------------
 1 file changed, 20 insertions(+), 49 deletions(-)
drivers/gpu/drm/i915/i915_irq.c +20 -49
index 0ad409a..ff3fb6d 100644
@@ -2169,21 +2169,33 @@ static void gen5_gt_irq_postinstall(struct drm_device *dev)
 static int ironlake_irq_postinstall(struct drm_device *dev)
 {
 	unsigned long irqflags;
-
 	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
-	/* enable kind of interrupts always enabled */
-	u32 display_mask = DE_MASTER_IRQ_CONTROL | DE_GSE | DE_PCH_EVENT |
-			   DE_PLANEA_FLIP_DONE | DE_PLANEB_FLIP_DONE |
-			   DE_AUX_CHANNEL_A | DE_PIPEB_FIFO_UNDERRUN |
-			   DE_PIPEA_FIFO_UNDERRUN | DE_POISON;
+	u32 display_mask, extra_mask;
+
+	if (INTEL_INFO(dev)->gen >= 7) {
+		display_mask = DE_MASTER_IRQ_CONTROL | DE_GSE_IVB |
+			       DE_PCH_EVENT_IVB | DE_PLANEC_FLIP_DONE_IVB |
+			       DE_PLANEB_FLIP_DONE_IVB |
+			       DE_PLANEA_FLIP_DONE_IVB | DE_AUX_CHANNEL_A_IVB |
+			       DE_ERR_INT_IVB;
+		extra_mask = DE_PIPEC_VBLANK_IVB | DE_PIPEB_VBLANK_IVB |
+			     DE_PIPEA_VBLANK_IVB;
+
+		I915_WRITE(GEN7_ERR_INT, I915_READ(GEN7_ERR_INT));
+	} else {
+		display_mask = DE_MASTER_IRQ_CONTROL | DE_GSE | DE_PCH_EVENT |
+			       DE_PLANEA_FLIP_DONE | DE_PLANEB_FLIP_DONE |
+			       DE_AUX_CHANNEL_A | DE_PIPEB_FIFO_UNDERRUN |
+			       DE_PIPEA_FIFO_UNDERRUN | DE_POISON;
+		extra_mask = DE_PIPEA_VBLANK | DE_PIPEB_VBLANK | DE_PCU_EVENT;
+	}
 
 	dev_priv->irq_mask = ~display_mask;
 
 	/* should always can generate irq */
 	I915_WRITE(DEIIR, I915_READ(DEIIR));
 	I915_WRITE(DEIMR, dev_priv->irq_mask);
-	I915_WRITE(DEIER, display_mask |
-			  DE_PIPEA_VBLANK | DE_PIPEB_VBLANK | DE_PCU_EVENT);
+	I915_WRITE(DEIER, display_mask | extra_mask);
 	POSTING_READ(DEIER);
 
 	gen5_gt_irq_postinstall(dev);

@@ -2204,38 +2216,6 @@ static int ironlake_irq_postinstall(struct drm_device *dev)
 	return 0;
 }
 
-static int ivybridge_irq_postinstall(struct drm_device *dev)
-{
-	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
-	/* enable kind of interrupts always enabled */
-	u32 display_mask =
-		DE_MASTER_IRQ_CONTROL | DE_GSE_IVB | DE_PCH_EVENT_IVB |
-		DE_PLANEC_FLIP_DONE_IVB |
-		DE_PLANEB_FLIP_DONE_IVB |
-		DE_PLANEA_FLIP_DONE_IVB |
-		DE_AUX_CHANNEL_A_IVB |
-		DE_ERR_INT_IVB;
-
-	dev_priv->irq_mask = ~display_mask;
-
-	/* should always can generate irq */
-	I915_WRITE(GEN7_ERR_INT, I915_READ(GEN7_ERR_INT));
-	I915_WRITE(DEIIR, I915_READ(DEIIR));
-	I915_WRITE(DEIMR, dev_priv->irq_mask);
-	I915_WRITE(DEIER,
-		   display_mask |
-		   DE_PIPEC_VBLANK_IVB |
-		   DE_PIPEB_VBLANK_IVB |
-		   DE_PIPEA_VBLANK_IVB);
-	POSTING_READ(DEIER);
-
-	gen5_gt_irq_postinstall(dev);
-
-	ibx_irq_postinstall(dev);
-
-	return 0;
-}
-
 static int valleyview_irq_postinstall(struct drm_device *dev)
 {
 	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;

@@ -3044,15 +3024,6 @@ void intel_irq_init(struct drm_device *dev)
 		dev->driver->enable_vblank = valleyview_enable_vblank;
 		dev->driver->disable_vblank = valleyview_disable_vblank;
 		dev_priv->display.hpd_irq_setup = i915_hpd_irq_setup;
-	} else if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev)) {
-		/* Share uninstall handlers with ILK/SNB */
-		dev->driver->irq_handler = ironlake_irq_handler;
-		dev->driver->irq_preinstall = ironlake_irq_preinstall;
-		dev->driver->irq_postinstall = ivybridge_irq_postinstall;
-		dev->driver->irq_uninstall = ironlake_irq_uninstall;
-		dev->driver->enable_vblank = ironlake_enable_vblank;
-		dev->driver->disable_vblank = ironlake_disable_vblank;
-		dev_priv->display.hpd_irq_setup = ibx_hpd_irq_setup;
 	} else if (HAS_PCH_SPLIT(dev)) {
 		dev->driver->irq_handler = ironlake_irq_handler;
 		dev->driver->irq_preinstall = ironlake_irq_preinstall;
Signature · 1 line
1.8.1.2
#28 lore raw

Re: [PATCH 10/10] drm/i915: kill ivybridge_irq_postinstall

Quoted 9 lines · Paulo Zanoni
drivers/gpu/drm/i915/i915_irq.c +0 -1
index 0ad409a..ff3fb6d 100644
@@ -2169,21 +2169,33 @@ static void gen5_gt_irq_postinstall(struct drm_device *dev)
 static int ironlake_irq_postinstall(struct drm_device *dev)
 {
 	unsigned long irqflags;
-

drivers/gpu/drm/i915/i915_irq.c static void gen5_gt_irq_postinstall(struct drm_device *dev) · line 2176

Quoted 10 lines
drivers/gpu/drm/i915/i915_irq.c +4 -5
 	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
-	/* enable kind of interrupts always enabled */
-	u32 display_mask = DE_MASTER_IRQ_CONTROL | DE_GSE | DE_PCH_EVENT |
-			   DE_PLANEA_FLIP_DONE | DE_PLANEB_FLIP_DONE |
-			   DE_AUX_CHANNEL_A | DE_PIPEB_FIFO_UNDERRUN |
-			   DE_PIPEA_FIFO_UNDERRUN | DE_POISON;
+	u32 display_mask, extra_mask;
+
+	if (INTEL_INFO(dev)->gen >= 7) {
+		display_mask = DE_MASTER_IRQ_CONTROL | DE_GSE_IVB |

If you add a leading bracket here, you make at least one vim user much happier.
-Chris

Signature · 1 line
Chris Wilson, Intel Open Source Technology Centre
author
#29 lore raw

PATCH 9/9 drm/i915: kill ivybridge_irq_postinstall

From: Paulo Zanoni <[email protected]>

It was very similar to ironlake_irq_postinstall, so IMHO merging both functions results in a code that is easier to maintain.

With this change, all the irq handler vfuncs between ironlake and ivybridge are now unified.

v2: Add "(" and ")" to make at least one vim user much happier (Chris)

Signed-off-by Paulo Zanoni <[email protected]>
---
 drivers/gpu/drm/i915/i915_irq.c | 69 ++++++++++++-----------------------------
 1 file changed, 20 insertions(+), 49 deletions(-)
drivers/gpu/drm/i915/i915_irq.c +20 -49
index d084057..8b48a32 100644
@@ -2161,21 +2161,33 @@ static void gen5_gt_irq_postinstall(struct drm_device *dev)
 static int ironlake_irq_postinstall(struct drm_device *dev)
 {
 	unsigned long irqflags;
-
 	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
-	/* enable kind of interrupts always enabled */
-	u32 display_mask = DE_MASTER_IRQ_CONTROL | DE_GSE | DE_PCH_EVENT |
-			   DE_PLANEA_FLIP_DONE | DE_PLANEB_FLIP_DONE |
-			   DE_AUX_CHANNEL_A | DE_PIPEB_FIFO_UNDERRUN |
-			   DE_PIPEA_FIFO_UNDERRUN | DE_POISON;
+	u32 display_mask, extra_mask;
+
+	if (INTEL_INFO(dev)->gen >= 7) {
+		display_mask = (DE_MASTER_IRQ_CONTROL | DE_GSE_IVB |
+				DE_PCH_EVENT_IVB | DE_PLANEC_FLIP_DONE_IVB |
+				DE_PLANEB_FLIP_DONE_IVB |
+				DE_PLANEA_FLIP_DONE_IVB | DE_AUX_CHANNEL_A_IVB |
+				DE_ERR_INT_IVB);
+		extra_mask = (DE_PIPEC_VBLANK_IVB | DE_PIPEB_VBLANK_IVB |
+			      DE_PIPEA_VBLANK_IVB);
+
+		I915_WRITE(GEN7_ERR_INT, I915_READ(GEN7_ERR_INT));
+	} else {
+		display_mask = (DE_MASTER_IRQ_CONTROL | DE_GSE | DE_PCH_EVENT |
+				DE_PLANEA_FLIP_DONE | DE_PLANEB_FLIP_DONE |
+				DE_AUX_CHANNEL_A | DE_PIPEB_FIFO_UNDERRUN |
+				DE_PIPEA_FIFO_UNDERRUN | DE_POISON);
+		extra_mask = DE_PIPEA_VBLANK | DE_PIPEB_VBLANK | DE_PCU_EVENT;
+	}
 
 	dev_priv->irq_mask = ~display_mask;
 
 	/* should always can generate irq */
 	I915_WRITE(DEIIR, I915_READ(DEIIR));
 	I915_WRITE(DEIMR, dev_priv->irq_mask);
-	I915_WRITE(DEIER, display_mask |
-			  DE_PIPEA_VBLANK | DE_PIPEB_VBLANK | DE_PCU_EVENT);
+	I915_WRITE(DEIER, display_mask | extra_mask);
 	POSTING_READ(DEIER);
 
 	gen5_gt_irq_postinstall(dev);

@@ -2196,38 +2208,6 @@ static int ironlake_irq_postinstall(struct drm_device *dev)
 	return 0;
 }
 
-static int ivybridge_irq_postinstall(struct drm_device *dev)
-{
-	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
-	/* enable kind of interrupts always enabled */
-	u32 display_mask =
-		DE_MASTER_IRQ_CONTROL | DE_GSE_IVB | DE_PCH_EVENT_IVB |
-		DE_PLANEC_FLIP_DONE_IVB |
-		DE_PLANEB_FLIP_DONE_IVB |
-		DE_PLANEA_FLIP_DONE_IVB |
-		DE_AUX_CHANNEL_A_IVB |
-		DE_ERR_INT_IVB;
-
-	dev_priv->irq_mask = ~display_mask;
-
-	/* should always can generate irq */
-	I915_WRITE(GEN7_ERR_INT, I915_READ(GEN7_ERR_INT));
-	I915_WRITE(DEIIR, I915_READ(DEIIR));
-	I915_WRITE(DEIMR, dev_priv->irq_mask);
-	I915_WRITE(DEIER,
-		   display_mask |
-		   DE_PIPEC_VBLANK_IVB |
-		   DE_PIPEB_VBLANK_IVB |
-		   DE_PIPEA_VBLANK_IVB);
-	POSTING_READ(DEIER);
-
-	gen5_gt_irq_postinstall(dev);
-
-	ibx_irq_postinstall(dev);
-
-	return 0;
-}
-
 static int valleyview_irq_postinstall(struct drm_device *dev)
 {
 	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;

@@ -3036,15 +3016,6 @@ void intel_irq_init(struct drm_device *dev)
 		dev->driver->enable_vblank = valleyview_enable_vblank;
 		dev->driver->disable_vblank = valleyview_disable_vblank;
 		dev_priv->display.hpd_irq_setup = i915_hpd_irq_setup;
-	} else if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev)) {
-		/* Share uninstall handlers with ILK/SNB */
-		dev->driver->irq_handler = ironlake_irq_handler;
-		dev->driver->irq_preinstall = ironlake_irq_preinstall;
-		dev->driver->irq_postinstall = ivybridge_irq_postinstall;
-		dev->driver->irq_uninstall = ironlake_irq_uninstall;
-		dev->driver->enable_vblank = ironlake_enable_vblank;
-		dev->driver->disable_vblank = ironlake_disable_vblank;
-		dev_priv->display.hpd_irq_setup = ibx_hpd_irq_setup;
 	} else if (HAS_PCH_SPLIT(dev)) {
 		dev->driver->irq_handler = ironlake_irq_handler;
 		dev->driver->irq_preinstall = ironlake_irq_preinstall;
Signature · 1 line
1.8.1.2
Reviewed-by on v1 9/9
#30 lore raw

Re: [PATCH 9/9] drm/i915: kill ivybridge_irq_postinstall

Quoted 11 lines · Paulo Zanoni

From: Paulo Zanoni <[email protected]>

It was very similar to ironlake_irq_postinstall, so IMHO merging both functions results in a code that is easier to maintain.

With this change, all the irq handler vfuncs between ironlake and ivybridge are now unified.

v2: Add "(" and ")" to make at least one vim user much happier (Chris)

Signed-off-by Paulo Zanoni <[email protected]>
Reviewed-by Mika Kuoppala <[email protected]>
Quoted 112 lines
---
 drivers/gpu/drm/i915/i915_irq.c | 69 ++++++++++++-----------------------------
 1 file changed, 20 insertions(+), 49 deletions(-)
drivers/gpu/drm/i915/i915_irq.c +20 -49
index d084057..8b48a32 100644
@@ -2161,21 +2161,33 @@ static void gen5_gt_irq_postinstall(struct drm_device *dev)
 static int ironlake_irq_postinstall(struct drm_device *dev)
 {
 	unsigned long irqflags;
-
 	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
-	/* enable kind of interrupts always enabled */
-	u32 display_mask = DE_MASTER_IRQ_CONTROL | DE_GSE | DE_PCH_EVENT |
-			   DE_PLANEA_FLIP_DONE | DE_PLANEB_FLIP_DONE |
-			   DE_AUX_CHANNEL_A | DE_PIPEB_FIFO_UNDERRUN |
-			   DE_PIPEA_FIFO_UNDERRUN | DE_POISON;
+	u32 display_mask, extra_mask;
+
+	if (INTEL_INFO(dev)->gen >= 7) {
+		display_mask = (DE_MASTER_IRQ_CONTROL | DE_GSE_IVB |
+				DE_PCH_EVENT_IVB | DE_PLANEC_FLIP_DONE_IVB |
+				DE_PLANEB_FLIP_DONE_IVB |
+				DE_PLANEA_FLIP_DONE_IVB | DE_AUX_CHANNEL_A_IVB |
+				DE_ERR_INT_IVB);
+		extra_mask = (DE_PIPEC_VBLANK_IVB | DE_PIPEB_VBLANK_IVB |
+			      DE_PIPEA_VBLANK_IVB);
+
+		I915_WRITE(GEN7_ERR_INT, I915_READ(GEN7_ERR_INT));
+	} else {
+		display_mask = (DE_MASTER_IRQ_CONTROL | DE_GSE | DE_PCH_EVENT |
+				DE_PLANEA_FLIP_DONE | DE_PLANEB_FLIP_DONE |
+				DE_AUX_CHANNEL_A | DE_PIPEB_FIFO_UNDERRUN |
+				DE_PIPEA_FIFO_UNDERRUN | DE_POISON);
+		extra_mask = DE_PIPEA_VBLANK | DE_PIPEB_VBLANK | DE_PCU_EVENT;
+	}
 
 	dev_priv->irq_mask = ~display_mask;
 
 	/* should always can generate irq */
 	I915_WRITE(DEIIR, I915_READ(DEIIR));
 	I915_WRITE(DEIMR, dev_priv->irq_mask);
-	I915_WRITE(DEIER, display_mask |
-			  DE_PIPEA_VBLANK | DE_PIPEB_VBLANK | DE_PCU_EVENT);
+	I915_WRITE(DEIER, display_mask | extra_mask);
 	POSTING_READ(DEIER);
 
 	gen5_gt_irq_postinstall(dev);

@@ -2196,38 +2208,6 @@ static int ironlake_irq_postinstall(struct drm_device *dev)
 	return 0;
 }
 
-static int ivybridge_irq_postinstall(struct drm_device *dev)
-{
-	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
-	/* enable kind of interrupts always enabled */
-	u32 display_mask =
-		DE_MASTER_IRQ_CONTROL | DE_GSE_IVB | DE_PCH_EVENT_IVB |
-		DE_PLANEC_FLIP_DONE_IVB |
-		DE_PLANEB_FLIP_DONE_IVB |
-		DE_PLANEA_FLIP_DONE_IVB |
-		DE_AUX_CHANNEL_A_IVB |
-		DE_ERR_INT_IVB;
-
-	dev_priv->irq_mask = ~display_mask;
-
-	/* should always can generate irq */
-	I915_WRITE(GEN7_ERR_INT, I915_READ(GEN7_ERR_INT));
-	I915_WRITE(DEIIR, I915_READ(DEIIR));
-	I915_WRITE(DEIMR, dev_priv->irq_mask);
-	I915_WRITE(DEIER,
-		   display_mask |
-		   DE_PIPEC_VBLANK_IVB |
-		   DE_PIPEB_VBLANK_IVB |
-		   DE_PIPEA_VBLANK_IVB);
-	POSTING_READ(DEIER);
-
-	gen5_gt_irq_postinstall(dev);
-
-	ibx_irq_postinstall(dev);
-
-	return 0;
-}
-
 static int valleyview_irq_postinstall(struct drm_device *dev)
 {
 	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;

@@ -3036,15 +3016,6 @@ void intel_irq_init(struct drm_device *dev)
 		dev->driver->enable_vblank = valleyview_enable_vblank;
 		dev->driver->disable_vblank = valleyview_disable_vblank;
 		dev_priv->display.hpd_irq_setup = i915_hpd_irq_setup;
-	} else if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev)) {
-		/* Share uninstall handlers with ILK/SNB */
-		dev->driver->irq_handler = ironlake_irq_handler;
-		dev->driver->irq_preinstall = ironlake_irq_preinstall;
-		dev->driver->irq_postinstall = ivybridge_irq_postinstall;
-		dev->driver->irq_uninstall = ironlake_irq_uninstall;
-		dev->driver->enable_vblank = ironlake_enable_vblank;
-		dev->driver->disable_vblank = ironlake_disable_vblank;
-		dev_priv->display.hpd_irq_setup = ibx_hpd_irq_setup;
 	} else if (HAS_PCH_SPLIT(dev)) {
 		dev->driver->irq_handler = ironlake_irq_handler;
 		dev->driver->irq_preinstall = ironlake_irq_preinstall;
Signature · 6 lines
1.8.1.2

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
#31 lore raw

Re: [PATCH 9/9] drm/i915: kill ivybridge_irq_postinstall

Quoted 7 lines · Mika Kuoppala
Quoted 5 lines · Paulo Zanoni

From: Paulo Zanoni <[email protected]>

It was very similar to ironlake_irq_postinstall, so IMHO merging both functions results in a code that is easier to maintain.

Quoted 8 lines
Quoted 6 lines

With this change, all the irq handler vfuncs between ironlake and ivybridge are now unified.

v2: Add "(" and ")" to make at least one vim user much happier (Chris)

Signed-off-by Paulo Zanoni <[email protected]>
Reviewed-by Mika Kuoppala <[email protected]>

Slurped in the entire series, thanks for patches&review. -Daniel

Signature · 3 lines
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch