[PATCH 00/10] Unify ILK/SNB/IVB/HSW IRQ vfuncs

[PATCH 00/10] Unify ILK/SNB/IVB/HSW IRQ vfuncs

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

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(-)

-- 
1.8.1.2

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

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

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

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

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index ca9df54..f397f9a 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -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;
-- 
1.8.1.2

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

[Mika Kuoppala]

Paulo Zanoni <przanoni@gmail.com> writes:

> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> After Daniel's latest changes it's now equal to
> ironlake_irq_preinstall.
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_irq.c | 21 +--------------------
>  1 file changed, 1 insertion(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index ca9df54..f397f9a 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -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 */

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

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

> -	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;
> -- 
> 1.8.1.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

It's the code that deals with de_iir.

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

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index f397f9a..39160a2 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -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

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

[Mika Kuoppala]

Paulo Zanoni <przanoni@gmail.com> writes:

> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> It's the code that deals with de_iir.
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

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 <mika.kuoppala@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_irq.c | 104 +++++++++++++++++++++-------------------
>  1 file changed, 56 insertions(+), 48 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index f397f9a..39160a2 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -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
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Paulo Zanoni]

2013/7/19 Mika Kuoppala <mika.kuoppala@linux.intel.com>:
> Paulo Zanoni <przanoni@gmail.com> writes:
>
>> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>>
>> It's the code that deals with de_iir.
>>
>> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> 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?

>
> Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

Thanks for all the reviews!

>
>> ---
>>  drivers/gpu/drm/i915/i915_irq.c | 104 +++++++++++++++++++++-------------------
>>  1 file changed, 56 insertions(+), 48 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
>> index f397f9a..39160a2 100644
>> --- a/drivers/gpu/drm/i915/i915_irq.c
>> +++ b/drivers/gpu/drm/i915/i915_irq.c
>> @@ -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
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni

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

[Daniel Vetter]

On Fri, Jul 19, 2013 at 11:24:21AM -0300, Paulo Zanoni wrote:
> 2013/7/19 Mika Kuoppala <mika.kuoppala@linux.intel.com>:
> > Paulo Zanoni <przanoni@gmail.com> writes:
> >
> >> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> >>
> >> It's the code that deals with de_iir.
> >>
> >> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> >
> > 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?

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
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

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

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

Just like we did with ilk_display_irq_handler.

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

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 39160a2..9167219 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -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;
-- 
1.8.1.2

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

[Mika Kuoppala]

Paulo Zanoni <przanoni@gmail.com> writes:

> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> Just like we did with ilk_display_irq_handler.
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_irq.c | 63 +++++++++++++++++++++++------------------
>  1 file changed, 35 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 39160a2..9167219 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -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;
> -- 
> 1.8.1.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

The register doesn't exist on Gen 5.

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

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 9167219..19370db 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -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);
-- 
1.8.1.2

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

[Chris Wilson]

On Fri, Jul 12, 2013 at 04:35:12PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> The register doesn't exist on Gen 5.

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

-- 
Chris Wilson, Intel Open Source Technology Centre

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

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

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 <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 9167219..c674dc3 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -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);
-- 
1.8.1.2

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

[Mika Kuoppala]

Paulo Zanoni <przanoni@gmail.com> writes:

> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> 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 <paulo.r.zanoni@intel.com>

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_irq.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 9167219..c674dc3 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -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);
> -- 
> 1.8.1.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

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 <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 44 ++++++++++++++++++++---------------------
 1 file changed, 22 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 19370db..7bc36ae 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -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);
-- 
1.8.1.2

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

[Chris Wilson]

On Fri, Jul 12, 2013 at 04:35:13PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> 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

-- 
Chris Wilson, Intel Open Source Technology Centre

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

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

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 <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 46 ++++++++++++++++++++---------------------
 1 file changed, 23 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index c674dc3..88eb380 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -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);
-- 
1.8.1.2

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

[Mika Kuoppala]

Paulo Zanoni <przanoni@gmail.com> writes:

> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> 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 <paulo.r.zanoni@intel.com>

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_irq.c | 46 ++++++++++++++++++++---------------------
>  1 file changed, 23 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index c674dc3..88eb380 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -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);
> -- 
> 1.8.1.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

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 <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 7bc36ae..bdaab93 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -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
-- 
1.8.1.2

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

[Mika Kuoppala]

Paulo Zanoni <przanoni@gmail.com> writes:

> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> 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 <paulo.r.zanoni@intel.com>

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_irq.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 7bc36ae..bdaab93 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -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
> -- 
> 1.8.1.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 07/10] drm/i915: add ILK/SNB support to ivybridge_irq_handler

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

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 <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 113 +++++++++++-----------------------------
 1 file changed, 31 insertions(+), 82 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index bdaab93..3d49f0f 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -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;
-- 
1.8.1.2

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

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

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 <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 115 +++++++++++-----------------------------
 1 file changed, 32 insertions(+), 83 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index ba38aa8..f54a02b 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -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;
-- 
1.8.1.2

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

[Mika Kuoppala]

Paulo Zanoni <przanoni@gmail.com> writes:

> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> 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 <paulo.r.zanoni@intel.com>

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_irq.c | 115 +++++++++++-----------------------------
>  1 file changed, 32 insertions(+), 83 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index ba38aa8..f54a02b 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -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;
> -- 
> 1.8.1.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

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 <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 25 +++++++------------------
 1 file changed, 7 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 3d49f0f..d26bbb3 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -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)) {
-- 
1.8.1.2

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

[Chris Wilson]

On Fri, Jul 12, 2013 at 04:35:16PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> 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

-- 
Chris Wilson, Intel Open Source Technology Centre

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

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

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 <paulo.r.zanoni@intel.com>
---
 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.

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index f54a02b..d084057 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -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;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 9556dff..dd2306e 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -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)
 
-- 
1.8.1.2

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

[Mika Kuoppala]

Paulo Zanoni <przanoni@gmail.com> writes:

> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> 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 <paulo.r.zanoni@intel.com>

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

> ---
>  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.
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index f54a02b..d084057 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -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;
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 9556dff..dd2306e 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -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)
>  
> -- 
> 1.8.1.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 09/10] drm/i915: kill ivybridge_disable_vblank

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

Same reasons as ivybridge_enable_vblank.

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

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index d26bbb3..0ad409a 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -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;
-- 
1.8.1.2

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

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

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 <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 69 ++++++++++++-----------------------------
 1 file changed, 20 insertions(+), 49 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 0ad409a..ff3fb6d 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -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;
-- 
1.8.1.2

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

[Chris Wilson]

On Fri, Jul 12, 2013 at 04:35:18PM -0300, Paulo Zanoni wrote:
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 0ad409a..ff3fb6d 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -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 |

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

-- 
Chris Wilson, Intel Open Source Technology Centre

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

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

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 <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 69 ++++++++++++-----------------------------
 1 file changed, 20 insertions(+), 49 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index d084057..8b48a32 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -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;
-- 
1.8.1.2

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

[Mika Kuoppala]

Paulo Zanoni <przanoni@gmail.com> writes:

> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> 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 <paulo.r.zanoni@intel.com>

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_irq.c | 69 ++++++++++++-----------------------------
>  1 file changed, 20 insertions(+), 49 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index d084057..8b48a32 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -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;
> -- 
> 1.8.1.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Daniel Vetter]

On Fri, Jul 19, 2013 at 04:44:18PM +0300, Mika Kuoppala wrote:
> Paulo Zanoni <przanoni@gmail.com> writes:
> 
> > From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> >
> > 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 <paulo.r.zanoni@intel.com>
> 
> Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

Slurped in the entire series, thanks for patches&review.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch