Re: [PATCH 1/7] drm/i915: Optimize pipe irq handling on bdw

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Thu, Nov 07, 2013 at 11:05:40AM +0100, Daniel Vetter wrote:
> We have a per-pipe bit in the master irq control register, so use it.
> This allows us to drop the masks for aggregate interrupt bits and be a
> bit more explicit in the code. It also removes one indentation level.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/i915_irq.c | 40 +++++++++++++++++++---------------------
>  drivers/gpu/drm/i915/i915_reg.h |  5 +----
>  2 files changed, 20 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 54338cf72feb..c04fbbf0acf7 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1749,6 +1749,7 @@ static irqreturn_t gen8_irq_handler(int irq, void *arg)
>  	u32 master_ctl;
>  	irqreturn_t ret = IRQ_NONE;
>  	uint32_t tmp = 0;
> +	enum pipe pipe;
>  
>  	atomic_inc(&dev_priv->irq_received);
>  
> @@ -1777,31 +1778,28 @@ static irqreturn_t gen8_irq_handler(int irq, void *arg)
>  		}
>  	}
>  
> -	if (master_ctl & GEN8_DE_IRQS) {
> -		int de_ret = 0;
> -		int pipe;
> -		for_each_pipe(pipe) {
> -			uint32_t pipe_iir;
> -
> -			pipe_iir = I915_READ(GEN8_DE_PIPE_IIR(pipe));
> -			if (pipe_iir & GEN8_PIPE_VBLANK)
> -				drm_handle_vblank(dev, pipe);
> +	for_each_pipe(pipe) {
> +		uint32_t pipe_iir;
>  
> -			if (pipe_iir & GEN8_PIPE_FLIP_DONE) {
> -				intel_prepare_page_flip(dev, pipe);
> -				intel_finish_page_flip_plane(dev, pipe);
> -			}
> +		if (!(master_ctl & GEN8_DE_PIPE_IRQ(pipe)))
> +			continue;
>  
> -			if (pipe_iir & GEN8_DE_PIPE_IRQ_ERRORS)
> -				DRM_ERROR("Errors on pipe %c\n", 'A' + pipe);
> +		pipe_iir = I915_READ(GEN8_DE_PIPE_IIR(pipe));
> +		if (pipe_iir & GEN8_PIPE_VBLANK)
> +			drm_handle_vblank(dev, pipe);
>  
> -			if (pipe_iir) {
> -				de_ret++;
> -				ret = IRQ_HANDLED;
> -				I915_WRITE(GEN8_DE_PIPE_IIR(pipe), pipe_iir);
> -			}
> +		if (pipe_iir & GEN8_PIPE_FLIP_DONE) {
> +			intel_prepare_page_flip(dev, pipe);
> +			intel_finish_page_flip_plane(dev, pipe);
>  		}
> -		if (!de_ret)
> +
> +		if (pipe_iir & GEN8_DE_PIPE_IRQ_ERRORS)
> +			DRM_ERROR("Errors on pipe %c\n", 'A' + pipe);

Could use pipe_name(). Otherwise looks good.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> +
> +		if (pipe_iir) {
> +			ret = IRQ_HANDLED;
> +			I915_WRITE(GEN8_DE_PIPE_IIR(pipe), pipe_iir);
> +		} else
>  			DRM_ERROR("The master control interrupt lied (DE PIPE)!\n");
>  	}
>  
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 2b9e66c0c6cf..f150edaa64ca 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4031,15 +4031,12 @@
>  #define  GEN8_DE_PIPE_C_IRQ		(1<<18)
>  #define  GEN8_DE_PIPE_B_IRQ		(1<<17)
>  #define  GEN8_DE_PIPE_A_IRQ		(1<<16)
> +#define  GEN8_DE_PIPE_IRQ(pipe)		(1<<(16+pipe))
>  #define  GEN8_GT_VECS_IRQ		(1<<6)
>  #define  GEN8_GT_VCS2_IRQ		(1<<3)
>  #define  GEN8_GT_VCS1_IRQ		(1<<2)
>  #define  GEN8_GT_BCS_IRQ		(1<<1)
>  #define  GEN8_GT_RCS_IRQ		(1<<0)
> -/* Lazy definition */
> -#define  GEN8_GT_IRQS			0x000000ff
> -#define  GEN8_DE_IRQS			0x01ff0000
> -#define  GEN8_RSVD_IRQS			0xB700ff00
>  
>  #define GEN8_GT_ISR(which) (0x44300 + (0x10 * (which)))
>  #define GEN8_GT_IMR(which) (0x44304 + (0x10 * (which)))
> -- 
> 1.8.4.rc3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC