[Intel-gfx] [PATCH 33/49] drm/i915: Reduce locking in gen8 IRQ handler

Daniel Vetter daniel at ffwll.ch
Fri Mar 27 07:13:08 PDT 2015


On Fri, Mar 27, 2015 at 11:02:05AM +0000, Chris Wilson wrote:
> Similar in vain in reducing the number of unrequired spinlocks used for
> execlist command submission (where the forcewake is required but
> manually controlled), we know that the IRQ registers are outside of the
> powerwell and so we can access them directly. Since we now have direct
> access exported via I915_READ_FW/I915_WRITE_FW, lets put those to use in
> the irq handlers as well.
> 
> In the process, reorder the execlist submission to happen as early as
> possible.
> 
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>

port/pipe interrupts are in display power domains afaik, so I'd prefer not
to lose pm debug with those. But they're also gated behind the master_ctl
bits, so can we have all of the speedups still without touching those?
Same for the pch, just in case.
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_irq.c | 63 ++++++++++++++++++++---------------------
>  1 file changed, 31 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 8b5e0358c592..da3b76b9ebd9 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1285,56 +1285,56 @@ static irqreturn_t gen8_gt_irq_handler(struct drm_device *dev,
>  	irqreturn_t ret = IRQ_NONE;
>  
>  	if (master_ctl & (GEN8_GT_RCS_IRQ | GEN8_GT_BCS_IRQ)) {
> -		tmp = I915_READ(GEN8_GT_IIR(0));
> +		tmp = I915_READ_FW(GEN8_GT_IIR(0));
>  		if (tmp) {
> -			I915_WRITE(GEN8_GT_IIR(0), 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_RENDER_USER_INTERRUPT)
> -				notify_ring(dev, ring);
>  			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_RENDER_USER_INTERRUPT)
> -				notify_ring(dev, ring);
>  			if (bcs & GT_CONTEXT_SWITCH_INTERRUPT)
>  				intel_lrc_irq_handler(ring);
> +			if (bcs & GT_RENDER_USER_INTERRUPT)
> +				notify_ring(dev, ring);
>  		} else
>  			DRM_ERROR("The master control interrupt lied (GT0)!\n");
>  	}
>  
>  	if (master_ctl & (GEN8_GT_VCS1_IRQ | GEN8_GT_VCS2_IRQ)) {
> -		tmp = I915_READ(GEN8_GT_IIR(1));
> +		tmp = I915_READ_FW(GEN8_GT_IIR(1));
>  		if (tmp) {
> -			I915_WRITE(GEN8_GT_IIR(1), 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_RENDER_USER_INTERRUPT)
> -				notify_ring(dev, ring);
>  			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_RENDER_USER_INTERRUPT)
> -				notify_ring(dev, ring);
>  			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 (GT1)!\n");
>  	}
>  
>  	if (master_ctl & GEN8_GT_PM_IRQ) {
> -		tmp = I915_READ(GEN8_GT_IIR(2));
> +		tmp = I915_READ_FW(GEN8_GT_IIR(2));
>  		if (tmp & dev_priv->pm_rps_events) {
> -			I915_WRITE(GEN8_GT_IIR(2),
> -				   tmp & dev_priv->pm_rps_events);
> +			I915_WRITE_FW(GEN8_GT_IIR(2),
> +				      tmp & dev_priv->pm_rps_events);
>  			ret = IRQ_HANDLED;
>  			gen6_rps_irq_handler(dev_priv, tmp);
>  		} else
> @@ -1342,17 +1342,17 @@ static irqreturn_t gen8_gt_irq_handler(struct drm_device *dev,
>  	}
>  
>  	if (master_ctl & GEN8_GT_VECS_IRQ) {
> -		tmp = I915_READ(GEN8_GT_IIR(3));
> +		tmp = I915_READ_FW(GEN8_GT_IIR(3));
>  		if (tmp) {
> -			I915_WRITE(GEN8_GT_IIR(3), 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_RENDER_USER_INTERRUPT)
> -				notify_ring(dev, ring);
>  			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");
>  	}
> @@ -2178,22 +2178,21 @@ static irqreturn_t gen8_irq_handler(int irq, void *arg)
>  		aux_mask |=  GEN9_AUX_CHANNEL_B | GEN9_AUX_CHANNEL_C |
>  			GEN9_AUX_CHANNEL_D;
>  
> -	master_ctl = I915_READ(GEN8_MASTER_IRQ);
> +	master_ctl = I915_READ_FW(GEN8_MASTER_IRQ);
>  	master_ctl &= ~GEN8_MASTER_IRQ_CONTROL;
>  	if (!master_ctl)
>  		return IRQ_NONE;
>  
> -	I915_WRITE(GEN8_MASTER_IRQ, 0);
> -	POSTING_READ(GEN8_MASTER_IRQ);
> +	I915_WRITE_FW(GEN8_MASTER_IRQ, 0);
>  
>  	/* Find, clear, then process each source of interrupt */
>  
>  	ret = gen8_gt_irq_handler(dev, dev_priv, master_ctl);
>  
>  	if (master_ctl & GEN8_DE_MISC_IRQ) {
> -		tmp = I915_READ(GEN8_DE_MISC_IIR);
> +		tmp = I915_READ_FW(GEN8_DE_MISC_IIR);
>  		if (tmp) {
> -			I915_WRITE(GEN8_DE_MISC_IIR, tmp);
> +			I915_WRITE_FW(GEN8_DE_MISC_IIR, tmp);
>  			ret = IRQ_HANDLED;
>  			if (tmp & GEN8_DE_MISC_GSE)
>  				intel_opregion_asle_intr(dev);
> @@ -2205,9 +2204,9 @@ static irqreturn_t gen8_irq_handler(int irq, void *arg)
>  	}
>  
>  	if (master_ctl & GEN8_DE_PORT_IRQ) {
> -		tmp = I915_READ(GEN8_DE_PORT_IIR);
> +		tmp = I915_READ_FW(GEN8_DE_PORT_IIR);
>  		if (tmp) {
> -			I915_WRITE(GEN8_DE_PORT_IIR, tmp);
> +			I915_WRITE_FW(GEN8_DE_PORT_IIR, tmp);
>  			ret = IRQ_HANDLED;
>  
>  			if (tmp & aux_mask)
> @@ -2225,10 +2224,10 @@ static irqreturn_t gen8_irq_handler(int irq, void *arg)
>  		if (!(master_ctl & GEN8_DE_PIPE_IRQ(pipe)))
>  			continue;
>  
> -		pipe_iir = I915_READ(GEN8_DE_PIPE_IIR(pipe));
> +		pipe_iir = I915_READ_FW(GEN8_DE_PIPE_IIR(pipe));
>  		if (pipe_iir) {
>  			ret = IRQ_HANDLED;
> -			I915_WRITE(GEN8_DE_PIPE_IIR(pipe), pipe_iir);
> +			I915_WRITE_FW(GEN8_DE_PIPE_IIR(pipe), pipe_iir);
>  
>  			if (pipe_iir & GEN8_PIPE_VBLANK &&
>  			    intel_pipe_handle_vblank(dev, pipe))
> @@ -2271,9 +2270,9 @@ static irqreturn_t gen8_irq_handler(int irq, void *arg)
>  		 * scheme also closed the SDE interrupt handling race we've seen
>  		 * on older pch-split platforms. But this needs testing.
>  		 */
> -		u32 pch_iir = I915_READ(SDEIIR);
> +		u32 pch_iir = I915_READ_FW(SDEIIR);
>  		if (pch_iir) {
> -			I915_WRITE(SDEIIR, pch_iir);
> +			I915_WRITE_FW(SDEIIR, pch_iir);
>  			ret = IRQ_HANDLED;
>  			cpt_irq_handler(dev, pch_iir);
>  		} else
> @@ -2281,8 +2280,8 @@ static irqreturn_t gen8_irq_handler(int irq, void *arg)
>  
>  	}
>  
> -	I915_WRITE(GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL);
> -	POSTING_READ(GEN8_MASTER_IRQ);
> +	I915_WRITE_FW(GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL);
> +	POSTING_READ_FW(GEN8_MASTER_IRQ);
>  
>  	return ret;
>  }
> -- 
> 2.1.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch


More information about the Intel-gfx mailing list