Re: [PATCH 41/70] drm/i915: Tidy gen8 IRQ handler

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, Apr 07, 2015 at 04:21:05PM +0100, Chris Wilson wrote:
> Remove some needless variables and parameter passing.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Merged 3 patches up to this one, thanks.
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_irq.c | 113 +++++++++++++++++-----------------------
>  1 file changed, 49 insertions(+), 64 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index c2c80bf490c6..46bcbff89760 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -985,8 +985,7 @@ static void ironlake_rps_change_irq_handler(struct drm_device *dev)
>  	return;
>  }
>  
> -static void notify_ring(struct drm_device *dev,
> -			struct intel_engine_cs *ring)
> +static void notify_ring(struct intel_engine_cs *ring)
>  {
>  	if (!intel_ring_initialized(ring))
>  		return;
> @@ -1248,9 +1247,9 @@ static void ilk_gt_irq_handler(struct drm_device *dev,
>  {
>  	if (gt_iir &
>  	    (GT_RENDER_USER_INTERRUPT | GT_RENDER_PIPECTL_NOTIFY_INTERRUPT))
> -		notify_ring(dev, &dev_priv->ring[RCS]);
> +		notify_ring(&dev_priv->ring[RCS]);
>  	if (gt_iir & ILK_BSD_USER_INTERRUPT)
> -		notify_ring(dev, &dev_priv->ring[VCS]);
> +		notify_ring(&dev_priv->ring[VCS]);
>  }
>  
>  static void snb_gt_irq_handler(struct drm_device *dev,
> @@ -1260,11 +1259,11 @@ static void snb_gt_irq_handler(struct drm_device *dev,
>  
>  	if (gt_iir &
>  	    (GT_RENDER_USER_INTERRUPT | GT_RENDER_PIPECTL_NOTIFY_INTERRUPT))
> -		notify_ring(dev, &dev_priv->ring[RCS]);
> +		notify_ring(&dev_priv->ring[RCS]);
>  	if (gt_iir & GT_BSD_USER_INTERRUPT)
> -		notify_ring(dev, &dev_priv->ring[VCS]);
> +		notify_ring(&dev_priv->ring[VCS]);
>  	if (gt_iir & GT_BLT_USER_INTERRUPT)
> -		notify_ring(dev, &dev_priv->ring[BCS]);
> +		notify_ring(&dev_priv->ring[BCS]);
>  
>  	if (gt_iir & (GT_BLT_CS_ERROR_INTERRUPT |
>  		      GT_BSD_CS_ERROR_INTERRUPT |
> @@ -1275,63 +1274,65 @@ static void snb_gt_irq_handler(struct drm_device *dev,
>  		ivybridge_parity_error_irq_handler(dev, gt_iir);
>  }
>  
> -static irqreturn_t gen8_gt_irq_handler(struct drm_device *dev,
> -				       struct drm_i915_private *dev_priv,
> +static irqreturn_t gen8_gt_irq_handler(struct drm_i915_private *dev_priv,
>  				       u32 master_ctl)
>  {
> -	struct intel_engine_cs *ring;
> -	u32 rcs, bcs, vcs;
> -	uint32_t tmp = 0;
>  	irqreturn_t ret = IRQ_NONE;
>  
>  	if (master_ctl & (GEN8_GT_RCS_IRQ | GEN8_GT_BCS_IRQ)) {
> -		tmp = I915_READ_FW(GEN8_GT_IIR(0));
> +		u32 tmp = I915_READ_FW(GEN8_GT_IIR(0));
>  		if (tmp) {
>  			I915_WRITE_FW(GEN8_GT_IIR(0), tmp);
>  			ret = IRQ_HANDLED;
>  
> -			rcs = tmp >> GEN8_RCS_IRQ_SHIFT;
> -			ring = &dev_priv->ring[RCS];
> -			if (rcs & GT_CONTEXT_SWITCH_INTERRUPT)
> -				intel_lrc_irq_handler(ring);
> -			if (rcs & GT_RENDER_USER_INTERRUPT)
> -				notify_ring(dev, ring);
> -
> -			bcs = tmp >> GEN8_BCS_IRQ_SHIFT;
> -			ring = &dev_priv->ring[BCS];
> -			if (bcs & GT_CONTEXT_SWITCH_INTERRUPT)
> -				intel_lrc_irq_handler(ring);
> -			if (bcs & GT_RENDER_USER_INTERRUPT)
> -				notify_ring(dev, ring);
> +			if (tmp & (GT_CONTEXT_SWITCH_INTERRUPT << GEN8_RCS_IRQ_SHIFT))
> +				intel_lrc_irq_handler(&dev_priv->ring[RCS]);
> +			if (tmp & (GT_RENDER_USER_INTERRUPT << GEN8_RCS_IRQ_SHIFT))
> +				notify_ring(&dev_priv->ring[RCS]);
> +
> +			if (tmp & (GT_CONTEXT_SWITCH_INTERRUPT << GEN8_BCS_IRQ_SHIFT))
> +				intel_lrc_irq_handler(&dev_priv->ring[BCS]);
> +			if (tmp & (GT_RENDER_USER_INTERRUPT << GEN8_BCS_IRQ_SHIFT))
> +				notify_ring(&dev_priv->ring[BCS]);
>  		} else
>  			DRM_ERROR("The master control interrupt lied (GT0)!\n");
>  	}
>  
>  	if (master_ctl & (GEN8_GT_VCS1_IRQ | GEN8_GT_VCS2_IRQ)) {
> -		tmp = I915_READ_FW(GEN8_GT_IIR(1));
> +		u32 tmp = I915_READ_FW(GEN8_GT_IIR(1));
>  		if (tmp) {
>  			I915_WRITE_FW(GEN8_GT_IIR(1), tmp);
>  			ret = IRQ_HANDLED;
>  
> -			vcs = tmp >> GEN8_VCS1_IRQ_SHIFT;
> -			ring = &dev_priv->ring[VCS];
> -			if (vcs & GT_CONTEXT_SWITCH_INTERRUPT)
> -				intel_lrc_irq_handler(ring);
> -			if (vcs & GT_RENDER_USER_INTERRUPT)
> -				notify_ring(dev, ring);
> -
> -			vcs = tmp >> GEN8_VCS2_IRQ_SHIFT;
> -			ring = &dev_priv->ring[VCS2];
> -			if (vcs & GT_CONTEXT_SWITCH_INTERRUPT)
> -				intel_lrc_irq_handler(ring);
> -			if (vcs & GT_RENDER_USER_INTERRUPT)
> -				notify_ring(dev, ring);
> +			if (tmp & (GT_CONTEXT_SWITCH_INTERRUPT << GEN8_VCS1_IRQ_SHIFT))
> +				intel_lrc_irq_handler(&dev_priv->ring[VCS]);
> +			if (tmp & (GT_RENDER_USER_INTERRUPT << GEN8_VCS1_IRQ_SHIFT))
> +				notify_ring(&dev_priv->ring[VCS]);
> +
> +			if (tmp & (GT_CONTEXT_SWITCH_INTERRUPT << GEN8_VCS2_IRQ_SHIFT))
> +				intel_lrc_irq_handler(&dev_priv->ring[VCS2]);
> +			if (tmp & (GT_RENDER_USER_INTERRUPT << GEN8_VCS2_IRQ_SHIFT))
> +				notify_ring(&dev_priv->ring[VCS2]);
>  		} else
>  			DRM_ERROR("The master control interrupt lied (GT1)!\n");
>  	}
>  
> +	if (master_ctl & GEN8_GT_VECS_IRQ) {
> +		u32 tmp = I915_READ_FW(GEN8_GT_IIR(3));
> +		if (tmp) {
> +			I915_WRITE_FW(GEN8_GT_IIR(3), tmp);
> +			ret = IRQ_HANDLED;
> +
> +			if (tmp & (GT_CONTEXT_SWITCH_INTERRUPT << GEN8_VECS_IRQ_SHIFT))
> +				intel_lrc_irq_handler(&dev_priv->ring[VECS]);
> +			if (tmp & (GT_RENDER_USER_INTERRUPT << GEN8_VECS_IRQ_SHIFT))
> +				notify_ring(&dev_priv->ring[VECS]);
> +		} else
> +			DRM_ERROR("The master control interrupt lied (GT3)!\n");
> +	}
> +
>  	if (master_ctl & GEN8_GT_PM_IRQ) {
> -		tmp = I915_READ_FW(GEN8_GT_IIR(2));
> +		u32 tmp = I915_READ_FW(GEN8_GT_IIR(2));
>  		if (tmp & dev_priv->pm_rps_events) {
>  			I915_WRITE_FW(GEN8_GT_IIR(2),
>  				      tmp & dev_priv->pm_rps_events);
> @@ -1341,22 +1342,6 @@ static irqreturn_t gen8_gt_irq_handler(struct drm_device *dev,
>  			DRM_ERROR("The master control interrupt lied (PM)!\n");
>  	}
>  
> -	if (master_ctl & GEN8_GT_VECS_IRQ) {
> -		tmp = I915_READ_FW(GEN8_GT_IIR(3));
> -		if (tmp) {
> -			I915_WRITE_FW(GEN8_GT_IIR(3), tmp);
> -			ret = IRQ_HANDLED;
> -
> -			vcs = tmp >> GEN8_VECS_IRQ_SHIFT;
> -			ring = &dev_priv->ring[VECS];
> -			if (vcs & GT_CONTEXT_SWITCH_INTERRUPT)
> -				intel_lrc_irq_handler(ring);
> -			if (vcs & GT_RENDER_USER_INTERRUPT)
> -				notify_ring(dev, ring);
> -		} else
> -			DRM_ERROR("The master control interrupt lied (GT3)!\n");
> -	}
> -
>  	return ret;
>  }
>  
> @@ -1651,7 +1636,7 @@ static void gen6_rps_irq_handler(struct drm_i915_private *dev_priv, u32 pm_iir)
>  
>  	if (HAS_VEBOX(dev_priv->dev)) {
>  		if (pm_iir & PM_VEBOX_USER_INTERRUPT)
> -			notify_ring(dev_priv->dev, &dev_priv->ring[VECS]);
> +			notify_ring(&dev_priv->ring[VECS]);
>  
>  		if (pm_iir & PM_VEBOX_CS_ERROR_INTERRUPT)
>  			DRM_DEBUG("Command parser error, pm_iir 0x%08x\n", pm_iir);
> @@ -1845,7 +1830,7 @@ static irqreturn_t cherryview_irq_handler(int irq, void *arg)
>  			I915_WRITE(VLV_IIR, iir);
>  		}
>  
> -		gen8_gt_irq_handler(dev, dev_priv, master_ctl);
> +		gen8_gt_irq_handler(dev_priv, master_ctl);
>  
>  		/* Call regardless, as some status bits might not be
>  		 * signalled in iir */
> @@ -2187,7 +2172,7 @@ static irqreturn_t gen8_irq_handler(int irq, void *arg)
>  
>  	/* Find, clear, then process each source of interrupt */
>  
> -	ret = gen8_gt_irq_handler(dev, dev_priv, master_ctl);
> +	ret = gen8_gt_irq_handler(dev_priv, master_ctl);
>  
>  	if (master_ctl & GEN8_DE_MISC_IRQ) {
>  		tmp = I915_READ(GEN8_DE_MISC_IIR);
> @@ -3692,7 +3677,7 @@ static irqreturn_t i8xx_irq_handler(int irq, void *arg)
>  		new_iir = I915_READ16(IIR); /* Flush posted writes */
>  
>  		if (iir & I915_USER_INTERRUPT)
> -			notify_ring(dev, &dev_priv->ring[RCS]);
> +			notify_ring(&dev_priv->ring[RCS]);
>  
>  		for_each_pipe(dev_priv, pipe) {
>  			int plane = pipe;
> @@ -3883,7 +3868,7 @@ static irqreturn_t i915_irq_handler(int irq, void *arg)
>  		new_iir = I915_READ(IIR); /* Flush posted writes */
>  
>  		if (iir & I915_USER_INTERRUPT)
> -			notify_ring(dev, &dev_priv->ring[RCS]);
> +			notify_ring(&dev_priv->ring[RCS]);
>  
>  		for_each_pipe(dev_priv, pipe) {
>  			int plane = pipe;
> @@ -4110,9 +4095,9 @@ static irqreturn_t i965_irq_handler(int irq, void *arg)
>  		new_iir = I915_READ(IIR); /* Flush posted writes */
>  
>  		if (iir & I915_USER_INTERRUPT)
> -			notify_ring(dev, &dev_priv->ring[RCS]);
> +			notify_ring(&dev_priv->ring[RCS]);
>  		if (iir & I915_BSD_USER_INTERRUPT)
> -			notify_ring(dev, &dev_priv->ring[VCS]);
> +			notify_ring(&dev_priv->ring[VCS]);
>  
>  		for_each_pipe(dev_priv, pipe) {
>  			if (pipe_stats[pipe] & PIPE_START_VBLANK_INTERRUPT_STATUS &&
> -- 
> 2.1.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx