[Intel-gfx] [PATCH 02/10] drm/i915: Fix up vlv/chv display irq setup

Imre Deak imre.deak at intel.com
Mon Apr 11 16:29:13 UTC 2016


On ma, 2016-04-11 at 16:56 +0300, ville.syrjala at linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> 
> The vlv/chv display irq setup was a bit of mess after I ran out of steam
> when working on it last. Fix it up so that we just have a _reset() and
> _postinstall() hooks for the display irqs, and use those consistently.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_irq.c | 102 ++++++++++------------------------------
>  1 file changed, 24 insertions(+), 78 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 1d21ebfffd4d..a1239fedc086 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -3306,13 +3306,15 @@ static void vlv_display_irq_reset(struct drm_i915_private *dev_priv)
>  {
>  	enum pipe pipe;
>  
> -	i915_hotplug_interrupt_update(dev_priv, 0xFFFFFFFF, 0);
> +	i915_hotplug_interrupt_update_locked(dev_priv, 0xffffffff, 0);
>  	I915_WRITE(PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT));
>  
>  	for_each_pipe(dev_priv, pipe)
>  		I915_WRITE(PIPESTAT(pipe), 0xffff);

Since vlv_display_irq_reset() will be used in place
of valleyview_display_irqs_uninstall()/i915_disable_pipestat()
we'll leave now stale bits in pipestat_irq_mask[pipe]. It's not a
problem since display_irqs_enabled effectively masks these bits, but
for consistency I'd clear them.

The same goes for clearing PIPE_FIFO_UNDERRUN_STATUS.

With that:
Reviewed-by: Imre Deak <imre.deak at intel.com>

>  
>  	GEN5_IRQ_RESET(VLV_);
> +
> +	dev_priv->irq_mask = ~0;
>  }



>  
>  static void valleyview_irq_preinstall(struct drm_device *dev)
> @@ -3323,7 +3325,9 @@ static void valleyview_irq_preinstall(struct drm_device *dev)
>  
>  	I915_WRITE(DPINVGTT, DPINVGTT_STATUS_MASK);
>  
> +	spin_lock_irq(&dev_priv->irq_lock);
>  	vlv_display_irq_reset(dev_priv);
> +	spin_unlock_irq(&dev_priv->irq_lock);
>  }
>  
>  static void gen8_gt_irq_reset(struct drm_i915_private *dev_priv)
> @@ -3398,7 +3402,9 @@ static void cherryview_irq_preinstall(struct drm_device *dev)
>  
>  	I915_WRITE(DPINVGTT, DPINVGTT_STATUS_MASK_CHV);
>  
> +	spin_lock_irq(&dev_priv->irq_lock);
>  	vlv_display_irq_reset(dev_priv);
> +	spin_unlock_irq(&dev_priv->irq_lock);
>  }
>  
>  static u32 intel_hpd_enabled_irqs(struct drm_device *dev,
> @@ -3645,7 +3651,7 @@ static int ironlake_irq_postinstall(struct drm_device *dev)
>  	return 0;
>  }
>  
> -static void valleyview_display_irqs_install(struct drm_i915_private *dev_priv)
> +static void vlv_display_irq_postinstall(struct drm_i915_private *dev_priv)
>  {
>  	u32 pipestat_mask;
>  	u32 iir_mask;
> @@ -3679,40 +3685,6 @@ static void valleyview_display_irqs_install(struct drm_i915_private *dev_priv)
>  	POSTING_READ(VLV_IMR);
>  }
>  
> -static void valleyview_display_irqs_uninstall(struct drm_i915_private *dev_priv)
> -{
> -	u32 pipestat_mask;
> -	u32 iir_mask;
> -	enum pipe pipe;
> -
> -	iir_mask = I915_DISPLAY_PORT_INTERRUPT |
> -		   I915_DISPLAY_PIPE_A_EVENT_INTERRUPT |
> -		   I915_DISPLAY_PIPE_B_EVENT_INTERRUPT;
> -	if (IS_CHERRYVIEW(dev_priv))
> -		iir_mask |= I915_DISPLAY_PIPE_C_EVENT_INTERRUPT;
> -
> -	dev_priv->irq_mask |= iir_mask;
> -	I915_WRITE(VLV_IMR, dev_priv->irq_mask);
> -	I915_WRITE(VLV_IER, ~dev_priv->irq_mask);
> -	I915_WRITE(VLV_IIR, iir_mask);
> -	I915_WRITE(VLV_IIR, iir_mask);
> -	POSTING_READ(VLV_IIR);
> -
> -	pipestat_mask = PLANE_FLIP_DONE_INT_STATUS_VLV |
> -			PIPE_CRC_DONE_INTERRUPT_STATUS;
> -
> -	i915_disable_pipestat(dev_priv, PIPE_A, PIPE_GMBUS_INTERRUPT_STATUS);
> -	for_each_pipe(dev_priv, pipe)
> -		i915_disable_pipestat(dev_priv, pipe, pipestat_mask);
> -
> -	pipestat_mask = PIPESTAT_INT_STATUS_MASK |
> -			PIPE_FIFO_UNDERRUN_STATUS;
> -
> -	for_each_pipe(dev_priv, pipe)
> -		I915_WRITE(PIPESTAT(pipe), pipestat_mask);
> -	POSTING_READ(PIPESTAT(PIPE_A));
> -}
> -
>  void valleyview_enable_display_irqs(struct drm_i915_private *dev_priv)
>  {
>  	assert_spin_locked(&dev_priv->irq_lock);
> @@ -3723,7 +3695,7 @@ void valleyview_enable_display_irqs(struct drm_i915_private *dev_priv)
>  	dev_priv->display_irqs_enabled = true;
>  
>  	if (intel_irqs_enabled(dev_priv))
> -		valleyview_display_irqs_install(dev_priv);
> +		vlv_display_irq_postinstall(dev_priv);
>  }
>  
>  void valleyview_disable_display_irqs(struct drm_i915_private *dev_priv)
> @@ -3736,36 +3708,14 @@ void valleyview_disable_display_irqs(struct drm_i915_private *dev_priv)
>  	dev_priv->display_irqs_enabled = false;
>  
>  	if (intel_irqs_enabled(dev_priv))
> -		valleyview_display_irqs_uninstall(dev_priv);
> +		vlv_display_irq_reset(dev_priv);
>  }
>  
> -static void vlv_display_irq_postinstall(struct drm_i915_private *dev_priv)
> -{
> -	dev_priv->irq_mask = ~0;
> -
> -	i915_hotplug_interrupt_update(dev_priv, 0xffffffff, 0);
> -	POSTING_READ(PORT_HOTPLUG_EN);
> -
> -	I915_WRITE(VLV_IIR, 0xffffffff);
> -	I915_WRITE(VLV_IIR, 0xffffffff);
> -	I915_WRITE(VLV_IER, ~dev_priv->irq_mask);
> -	I915_WRITE(VLV_IMR, dev_priv->irq_mask);
> -	POSTING_READ(VLV_IMR);
> -
> -	/* Interrupt setup is already guaranteed to be single-threaded, this is
> -	 * just to make the assert_spin_locked check happy. */
> -	spin_lock_irq(&dev_priv->irq_lock);
> -	if (dev_priv->display_irqs_enabled)
> -		valleyview_display_irqs_install(dev_priv);
> -	spin_unlock_irq(&dev_priv->irq_lock);
> -}
>  
>  static int valleyview_irq_postinstall(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  
> -	vlv_display_irq_postinstall(dev_priv);
> -
>  	gen5_gt_irq_postinstall(dev);
>  
>  	/* ack & enable invalid PTE error interrupts */
> @@ -3774,6 +3724,10 @@ static int valleyview_irq_postinstall(struct drm_device *dev)
>  	I915_WRITE(DPINVGTT, DPINVGTT_EN_MASK);
>  #endif
>  
> +	spin_lock_irq(&dev_priv->irq_lock);
> +	vlv_display_irq_postinstall(dev_priv);
> +	spin_unlock_irq(&dev_priv->irq_lock);
> +
>  	I915_WRITE(VLV_MASTER_IER, MASTER_INTERRUPT_ENABLE);
>  
>  	return 0;
> @@ -3874,10 +3828,12 @@ static int cherryview_irq_postinstall(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  
> -	vlv_display_irq_postinstall(dev_priv);
> -
>  	gen8_gt_irq_postinstall(dev_priv);
>  
> +	spin_lock_irq(&dev_priv->irq_lock);
> +	vlv_display_irq_postinstall(dev_priv);
> +	spin_unlock_irq(&dev_priv->irq_lock);
> +
>  	I915_WRITE(GEN8_MASTER_IRQ, MASTER_INTERRUPT_ENABLE);
>  	POSTING_READ(GEN8_MASTER_IRQ);
>  
> @@ -3894,20 +3850,6 @@ static void gen8_irq_uninstall(struct drm_device *dev)
>  	gen8_irq_reset(dev);
>  }
>  
> -static void vlv_display_irq_uninstall(struct drm_i915_private *dev_priv)
> -{
> -	/* Interrupt setup is already guaranteed to be single-threaded, this is
> -	 * just to make the assert_spin_locked check happy. */
> -	spin_lock_irq(&dev_priv->irq_lock);
> -	if (dev_priv->display_irqs_enabled)
> -		valleyview_display_irqs_uninstall(dev_priv);
> -	spin_unlock_irq(&dev_priv->irq_lock);
> -
> -	vlv_display_irq_reset(dev_priv);
> -
> -	dev_priv->irq_mask = ~0;
> -}
> -
>  static void valleyview_irq_uninstall(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -3921,7 +3863,9 @@ static void valleyview_irq_uninstall(struct drm_device *dev)
>  
>  	I915_WRITE(HWSTAM, 0xffffffff);
>  
> -	vlv_display_irq_uninstall(dev_priv);
> +	spin_lock_irq(&dev_priv->irq_lock);
> +	vlv_display_irq_reset(dev_priv);
> +	spin_unlock_irq(&dev_priv->irq_lock);
>  }
>  
>  static void cherryview_irq_uninstall(struct drm_device *dev)
> @@ -3938,7 +3882,9 @@ static void cherryview_irq_uninstall(struct drm_device *dev)
>  
>  	GEN5_IRQ_RESET(GEN8_PCU_);
>  
> -	vlv_display_irq_uninstall(dev_priv);
> +	spin_lock_irq(&dev_priv->irq_lock);
> +	vlv_display_irq_reset(dev_priv);
> +	spin_unlock_irq(&dev_priv->irq_lock);
>  }
>  
>  static void ironlake_irq_uninstall(struct drm_device *dev)


More information about the Intel-gfx mailing list