Re: [PATCH 1/6] drm/i915: add ibx_irq_preinstall

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, Jun 05, 2013 at 02:21:51PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> So we can remove some duplicate code. All the PCHs are very similar
> and right now the code is the same. I plan to add more code, so we
> would have more duplicated code.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Queued for -next, thanks for the patch. For the other prep patches I'd
like to review our interrupt code first a bit, like describe in my reply
to the vecs enabling patches.
-Daniel
> ---
>  drivers/gpu/drm/i915/i915_irq.c | 44 ++++++++++++++++++++---------------------
>  1 file changed, 21 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 63996aa..c482e8a 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -2472,6 +2472,25 @@ void i915_hangcheck_elapsed(unsigned long data)
>  					   DRM_I915_HANGCHECK_JIFFIES));
>  }
>  
> +static void ibx_irq_preinstall(struct drm_device *dev)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +
> +	if (HAS_PCH_NOP(dev))
> +		return;
> +
> +	/* south display irq */
> +	I915_WRITE(SDEIMR, 0xffffffff);
> +	/*
> +	 * SDEIER is also touched by the interrupt handler to work around missed
> +	 * PCH interrupts. Hence we can't update it after the interrupt handler
> +	 * is enabled - instead we unconditionally enable all PCH interrupt
> +	 * sources here, but then only unmask them as needed with SDEIMR.
> +	 */
> +	I915_WRITE(SDEIER, 0xffffffff);
> +	POSTING_READ(SDEIER);
> +}
> +
>  /* drm_dma.h hooks
>  */
>  static void ironlake_irq_preinstall(struct drm_device *dev)
> @@ -2493,16 +2512,7 @@ static void ironlake_irq_preinstall(struct drm_device *dev)
>  	I915_WRITE(GTIER, 0x0);
>  	POSTING_READ(GTIER);
>  
> -	/* south display irq */
> -	I915_WRITE(SDEIMR, 0xffffffff);
> -	/*
> -	 * SDEIER is also touched by the interrupt handler to work around missed
> -	 * PCH interrupts. Hence we can't update it after the interrupt handler
> -	 * is enabled - instead we unconditionally enable all PCH interrupt
> -	 * sources here, but then only unmask them as needed with SDEIMR.
> -	 */
> -	I915_WRITE(SDEIER, 0xffffffff);
> -	POSTING_READ(SDEIER);
> +	ibx_irq_preinstall(dev);
>  }
>  
>  static void ivybridge_irq_preinstall(struct drm_device *dev)
> @@ -2529,19 +2539,7 @@ static void ivybridge_irq_preinstall(struct drm_device *dev)
>  	I915_WRITE(GEN6_PMIER, 0x0);
>  	POSTING_READ(GEN6_PMIER);
>  
> -	if (HAS_PCH_NOP(dev))
> -		return;
> -
> -	/* south display irq */
> -	I915_WRITE(SDEIMR, 0xffffffff);
> -	/*
> -	 * SDEIER is also touched by the interrupt handler to work around missed
> -	 * PCH interrupts. Hence we can't update it after the interrupt handler
> -	 * is enabled - instead we unconditionally enable all PCH interrupt
> -	 * sources here, but then only unmask them as needed with SDEIMR.
> -	 */
> -	I915_WRITE(SDEIER, 0xffffffff);
> -	POSTING_READ(SDEIER);
> +	ibx_irq_preinstall(dev);
>  }
>  
>  static void valleyview_irq_preinstall(struct drm_device *dev)
> -- 
> 1.8.1.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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