diff mbox

[06/20] drm/i915: properly clear IIR at irq_uninstall on Gen5+

Message ID 1394233836-3827-7-git-send-email-przanoni@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Paulo Zanoni March 7, 2014, 11:10 p.m. UTC 
From: Paulo Zanoni <paulo.r.zanoni@intel.com>

The IRQ_INIT and IRQ_FINI macros are basically the same thing, with
the exception that IRQ_FINI doesn't properly clear IIR twice and
doesn't have as many POSTING_READs as IRQ_INIT. So rename the macro to
IRQ_RESET and use it everywhere.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 76 +++++++++++++++--------------------------
 1 file changed, 28 insertions(+), 48 deletions(-)

Comments

Chris Wilson March 11, 2014, 8:25 a.m. UTC | #1 
On Fri, Mar 07, 2014 at 08:10:22PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> The IRQ_INIT and IRQ_FINI macros are basically the same thing, with
> the exception that IRQ_FINI doesn't properly clear IIR twice and
> doesn't have as many POSTING_READs as IRQ_INIT. So rename the macro to
> IRQ_RESET and use it everywhere.

"the macro". The previous sentence talks about a pair of macros, so it
not clear to which you refer to here. By deduction and confirmed by
inspection you mean s/IRQ_INIT/IRQ_FINI/.
-Chris
Ben Widawsky March 18, 2014, 5:20 p.m. UTC | #2 
On Fri, Mar 07, 2014 at 08:10:22PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> The IRQ_INIT and IRQ_FINI macros are basically the same thing, with
> the exception that IRQ_FINI doesn't properly clear IIR twice and
> doesn't have as many POSTING_READs as IRQ_INIT. So rename the macro to
> IRQ_RESET and use it everywhere.

Make me happy and call it GEN5_IRQ_DISABLE(). Also note, I've argued
with the, "doesn't properly clear IIR twice" in a previous patch, but
that's not a big deal.

> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_irq.c | 76 +++++++++++++++--------------------------
>  1 file changed, 28 insertions(+), 48 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index f681462..73f1125 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -85,7 +85,7 @@ static const u32 hpd_status_i915[] = { /* i915 and valleyview are the same */
>   * Also, make sure callers of these macros have something equivalent to a
>   * POSTING_READ on the IIR register.
>   * */
> -#define GEN8_IRQ_INIT_NDX(type, which) do { \
> +#define GEN8_IRQ_RESET_NDX(type, which) do { \
>  	I915_WRITE(GEN8_##type##_IMR(which), 0xffffffff); \
>  	POSTING_READ(GEN8_##type##_IMR(which)); \
>  	I915_WRITE(GEN8_##type##_IER(which), 0); \
> @@ -94,7 +94,7 @@ static const u32 hpd_status_i915[] = { /* i915 and valleyview are the same */
>  	I915_WRITE(GEN8_##type##_IIR(which), 0xffffffff); \
>  } while (0)
>  
> -#define GEN5_IRQ_INIT(type) do { \
> +#define GEN5_IRQ_RESET(type) do { \
>  	I915_WRITE(type##IMR, 0xffffffff); \
>  	POSTING_READ(type##IMR); \
>  	I915_WRITE(type##IER, 0); \
> @@ -103,12 +103,6 @@ static const u32 hpd_status_i915[] = { /* i915 and valleyview are the same */
>  	I915_WRITE(type##IIR, 0xffffffff); \
>  } while (0)
>  
> -#define GEN5_IRQ_FINI(type) do { \
> -	I915_WRITE(type##IMR, 0xffffffff); \
> -	I915_WRITE(type##IER, 0); \
> -	I915_WRITE(type##IIR, I915_READ(type##IIR)); \
> -} while (0)
> -
>  /* For display hotplug interrupt */
>  static void
>  ironlake_enable_display_irq(drm_i915_private_t *dev_priv, u32 mask)
> @@ -2794,7 +2788,7 @@ static void ibx_irq_preinstall(struct drm_device *dev)
>  	if (HAS_PCH_NOP(dev))
>  		return;
>  
> -	GEN5_IRQ_INIT(SDE);
> +	GEN5_IRQ_RESET(SDE);
>  	/*
>  	 * SDEIER is also touched by the interrupt handler to work around missed
>  	 * PCH interrupts. Hence we can't update it after the interrupt handler
> @@ -2809,9 +2803,9 @@ static void gen5_gt_irq_preinstall(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  
> -	GEN5_IRQ_INIT(GT);
> +	GEN5_IRQ_RESET(GT);
>  	if (INTEL_INFO(dev)->gen >= 6)
> -		GEN5_IRQ_INIT(GEN6_PM);
> +		GEN5_IRQ_RESET(GEN6_PM);
>  	POSTING_READ(GTIIR);
>  }
>  
> @@ -2823,7 +2817,7 @@ static void ironlake_irq_preinstall(struct drm_device *dev)
>  
>  	I915_WRITE(HWSTAM, 0xeffe);
>  
> -	GEN5_IRQ_INIT(DE);
> +	GEN5_IRQ_RESET(DE);
>  
>  	gen5_gt_irq_preinstall(dev);
>  
> @@ -2867,18 +2861,18 @@ static void gen8_irq_preinstall(struct drm_device *dev)
>  	I915_WRITE(GEN8_MASTER_IRQ, 0);
>  	POSTING_READ(GEN8_MASTER_IRQ);
>  
> -	GEN8_IRQ_INIT_NDX(GT, 0);
> -	GEN8_IRQ_INIT_NDX(GT, 1);
> -	GEN8_IRQ_INIT_NDX(GT, 2);
> -	GEN8_IRQ_INIT_NDX(GT, 3);
> +	GEN8_IRQ_RESET_NDX(GT, 0);
> +	GEN8_IRQ_RESET_NDX(GT, 1);
> +	GEN8_IRQ_RESET_NDX(GT, 2);
> +	GEN8_IRQ_RESET_NDX(GT, 3);
>  
>  	for_each_pipe(pipe) {
> -		GEN8_IRQ_INIT_NDX(DE_PIPE, pipe);
> +		GEN8_IRQ_RESET_NDX(DE_PIPE, pipe);
>  	}
>  
> -	GEN5_IRQ_INIT(GEN8_DE_PORT_);
> -	GEN5_IRQ_INIT(GEN8_DE_MISC_);
> -	GEN5_IRQ_INIT(GEN8_PCU_);
> +	GEN5_IRQ_RESET(GEN8_DE_PORT_);
> +	GEN5_IRQ_RESET(GEN8_DE_MISC_);
> +	GEN5_IRQ_RESET(GEN8_PCU_);
>  	POSTING_READ(GEN8_PCU_IIR);
>  
>  	ibx_irq_preinstall(dev);
> @@ -3237,32 +3231,17 @@ static void gen8_irq_uninstall(struct drm_device *dev)
>  
>  	I915_WRITE(GEN8_MASTER_IRQ, 0);
>  
> -#define GEN8_IRQ_FINI_NDX(type, which) do { \
> -		I915_WRITE(GEN8_##type##_IMR(which), 0xffffffff); \
> -		I915_WRITE(GEN8_##type##_IER(which), 0); \
> -		I915_WRITE(GEN8_##type##_IIR(which), 0xffffffff); \
> -	} while (0)
> -
> -#define GEN8_IRQ_FINI(type) do { \
> -		I915_WRITE(GEN8_##type##_IMR, 0xffffffff); \
> -		I915_WRITE(GEN8_##type##_IER, 0); \
> -		I915_WRITE(GEN8_##type##_IIR, 0xffffffff); \
> -	} while (0)
> +	GEN8_IRQ_RESET_NDX(GT, 0);
> +	GEN8_IRQ_RESET_NDX(GT, 1);
> +	GEN8_IRQ_RESET_NDX(GT, 2);
> +	GEN8_IRQ_RESET_NDX(GT, 3);
>  
> -	GEN8_IRQ_FINI_NDX(GT, 0);
> -	GEN8_IRQ_FINI_NDX(GT, 1);
> -	GEN8_IRQ_FINI_NDX(GT, 2);
> -	GEN8_IRQ_FINI_NDX(GT, 3);
> -
> -	for_each_pipe(pipe) {
> -		GEN8_IRQ_FINI_NDX(DE_PIPE, pipe);
> -	}
> +	for_each_pipe(pipe)
> +		GEN8_IRQ_RESET_NDX(DE_PIPE, pipe);
>  
> -	GEN8_IRQ_FINI(DE_PORT);
> -	GEN8_IRQ_FINI(DE_MISC);
> -	GEN8_IRQ_FINI(PCU);
> -#undef GEN8_IRQ_FINI
> -#undef GEN8_IRQ_FINI_NDX
> +	GEN5_IRQ_RESET(GEN8_DE_PORT_);
> +	GEN5_IRQ_RESET(GEN8_DE_MISC_);
> +	GEN5_IRQ_RESET(GEN8_PCU_);
>  
>  	POSTING_READ(GEN8_PCU_IIR);
>  }
> @@ -3309,18 +3288,19 @@ static void ironlake_irq_uninstall(struct drm_device *dev)
>  
>  	I915_WRITE(HWSTAM, 0xffffffff);
>  
> -	GEN5_IRQ_FINI(DE);
> +	GEN5_IRQ_RESET(DE);
>  	if (IS_GEN7(dev))
>  		I915_WRITE(GEN7_ERR_INT, I915_READ(GEN7_ERR_INT));
>  
> -	GEN5_IRQ_FINI(GT);
> +	GEN5_IRQ_RESET(GT);
>  	if (INTEL_INFO(dev)->gen >= 6)
> -		GEN5_IRQ_FINI(GEN6_PM);
> +		GEN5_IRQ_RESET(GEN6_PM);
> +	POSTING_READ(GTIIR);
>  
>  	if (HAS_PCH_NOP(dev))
>  		return;
>  
> -	GEN5_IRQ_FINI(SDE);
> +	GEN5_IRQ_RESET(SDE);
>  	if (HAS_PCH_CPT(dev) || HAS_PCH_LPT(dev))
>  		I915_WRITE(SERR_INT, I915_READ(SERR_INT));
>  }
> -- 
> 1.8.5.3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index f681462..73f1125 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -85,7 +85,7 @@  static const u32 hpd_status_i915[] = { /* i915 and valleyview are the same */
  * Also, make sure callers of these macros have something equivalent to a
  * POSTING_READ on the IIR register.
  * */
-#define GEN8_IRQ_INIT_NDX(type, which) do { \
+#define GEN8_IRQ_RESET_NDX(type, which) do { \
 	I915_WRITE(GEN8_##type##_IMR(which), 0xffffffff); \
 	POSTING_READ(GEN8_##type##_IMR(which)); \
 	I915_WRITE(GEN8_##type##_IER(which), 0); \
@@ -94,7 +94,7 @@  static const u32 hpd_status_i915[] = { /* i915 and valleyview are the same */
 	I915_WRITE(GEN8_##type##_IIR(which), 0xffffffff); \
 } while (0)
 
-#define GEN5_IRQ_INIT(type) do { \
+#define GEN5_IRQ_RESET(type) do { \
 	I915_WRITE(type##IMR, 0xffffffff); \
 	POSTING_READ(type##IMR); \
 	I915_WRITE(type##IER, 0); \
@@ -103,12 +103,6 @@  static const u32 hpd_status_i915[] = { /* i915 and valleyview are the same */
 	I915_WRITE(type##IIR, 0xffffffff); \
 } while (0)
 
-#define GEN5_IRQ_FINI(type) do { \
-	I915_WRITE(type##IMR, 0xffffffff); \
-	I915_WRITE(type##IER, 0); \
-	I915_WRITE(type##IIR, I915_READ(type##IIR)); \
-} while (0)
-
 /* For display hotplug interrupt */
 static void
 ironlake_enable_display_irq(drm_i915_private_t *dev_priv, u32 mask)
@@ -2794,7 +2788,7 @@  static void ibx_irq_preinstall(struct drm_device *dev)
 	if (HAS_PCH_NOP(dev))
 		return;
 
-	GEN5_IRQ_INIT(SDE);
+	GEN5_IRQ_RESET(SDE);
 	/*
 	 * SDEIER is also touched by the interrupt handler to work around missed
 	 * PCH interrupts. Hence we can't update it after the interrupt handler
@@ -2809,9 +2803,9 @@  static void gen5_gt_irq_preinstall(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
-	GEN5_IRQ_INIT(GT);
+	GEN5_IRQ_RESET(GT);
 	if (INTEL_INFO(dev)->gen >= 6)
-		GEN5_IRQ_INIT(GEN6_PM);
+		GEN5_IRQ_RESET(GEN6_PM);
 	POSTING_READ(GTIIR);
 }
 
@@ -2823,7 +2817,7 @@  static void ironlake_irq_preinstall(struct drm_device *dev)
 
 	I915_WRITE(HWSTAM, 0xeffe);
 
-	GEN5_IRQ_INIT(DE);
+	GEN5_IRQ_RESET(DE);
 
 	gen5_gt_irq_preinstall(dev);
 
@@ -2867,18 +2861,18 @@  static void gen8_irq_preinstall(struct drm_device *dev)
 	I915_WRITE(GEN8_MASTER_IRQ, 0);
 	POSTING_READ(GEN8_MASTER_IRQ);
 
-	GEN8_IRQ_INIT_NDX(GT, 0);
-	GEN8_IRQ_INIT_NDX(GT, 1);
-	GEN8_IRQ_INIT_NDX(GT, 2);
-	GEN8_IRQ_INIT_NDX(GT, 3);
+	GEN8_IRQ_RESET_NDX(GT, 0);
+	GEN8_IRQ_RESET_NDX(GT, 1);
+	GEN8_IRQ_RESET_NDX(GT, 2);
+	GEN8_IRQ_RESET_NDX(GT, 3);
 
 	for_each_pipe(pipe) {
-		GEN8_IRQ_INIT_NDX(DE_PIPE, pipe);
+		GEN8_IRQ_RESET_NDX(DE_PIPE, pipe);
 	}
 
-	GEN5_IRQ_INIT(GEN8_DE_PORT_);
-	GEN5_IRQ_INIT(GEN8_DE_MISC_);
-	GEN5_IRQ_INIT(GEN8_PCU_);
+	GEN5_IRQ_RESET(GEN8_DE_PORT_);
+	GEN5_IRQ_RESET(GEN8_DE_MISC_);
+	GEN5_IRQ_RESET(GEN8_PCU_);
 	POSTING_READ(GEN8_PCU_IIR);
 
 	ibx_irq_preinstall(dev);
@@ -3237,32 +3231,17 @@  static void gen8_irq_uninstall(struct drm_device *dev)
 
 	I915_WRITE(GEN8_MASTER_IRQ, 0);
 
-#define GEN8_IRQ_FINI_NDX(type, which) do { \
-		I915_WRITE(GEN8_##type##_IMR(which), 0xffffffff); \
-		I915_WRITE(GEN8_##type##_IER(which), 0); \
-		I915_WRITE(GEN8_##type##_IIR(which), 0xffffffff); \
-	} while (0)
-
-#define GEN8_IRQ_FINI(type) do { \
-		I915_WRITE(GEN8_##type##_IMR, 0xffffffff); \
-		I915_WRITE(GEN8_##type##_IER, 0); \
-		I915_WRITE(GEN8_##type##_IIR, 0xffffffff); \
-	} while (0)
+	GEN8_IRQ_RESET_NDX(GT, 0);
+	GEN8_IRQ_RESET_NDX(GT, 1);
+	GEN8_IRQ_RESET_NDX(GT, 2);
+	GEN8_IRQ_RESET_NDX(GT, 3);
 
-	GEN8_IRQ_FINI_NDX(GT, 0);
-	GEN8_IRQ_FINI_NDX(GT, 1);
-	GEN8_IRQ_FINI_NDX(GT, 2);
-	GEN8_IRQ_FINI_NDX(GT, 3);
-
-	for_each_pipe(pipe) {
-		GEN8_IRQ_FINI_NDX(DE_PIPE, pipe);
-	}
+	for_each_pipe(pipe)
+		GEN8_IRQ_RESET_NDX(DE_PIPE, pipe);
 
-	GEN8_IRQ_FINI(DE_PORT);
-	GEN8_IRQ_FINI(DE_MISC);
-	GEN8_IRQ_FINI(PCU);
-#undef GEN8_IRQ_FINI
-#undef GEN8_IRQ_FINI_NDX
+	GEN5_IRQ_RESET(GEN8_DE_PORT_);
+	GEN5_IRQ_RESET(GEN8_DE_MISC_);
+	GEN5_IRQ_RESET(GEN8_PCU_);
 
 	POSTING_READ(GEN8_PCU_IIR);
 }
@@ -3309,18 +3288,19 @@  static void ironlake_irq_uninstall(struct drm_device *dev)
 
 	I915_WRITE(HWSTAM, 0xffffffff);
 
-	GEN5_IRQ_FINI(DE);
+	GEN5_IRQ_RESET(DE);
 	if (IS_GEN7(dev))
 		I915_WRITE(GEN7_ERR_INT, I915_READ(GEN7_ERR_INT));
 
-	GEN5_IRQ_FINI(GT);
+	GEN5_IRQ_RESET(GT);
 	if (INTEL_INFO(dev)->gen >= 6)
-		GEN5_IRQ_FINI(GEN6_PM);
+		GEN5_IRQ_RESET(GEN6_PM);
+	POSTING_READ(GTIIR);
 
 	if (HAS_PCH_NOP(dev))
 		return;
 
-	GEN5_IRQ_FINI(SDE);
+	GEN5_IRQ_RESET(SDE);
 	if (HAS_PCH_CPT(dev) || HAS_PCH_LPT(dev))
 		I915_WRITE(SERR_INT, I915_READ(SERR_INT));
 }