Re: [PATCH] drm/i915: Don't enable display error interrupts from the start

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

On Fri, Mar 07, 2014 at 06:11:42PM +0100, Daniel Vetter wrote:
> We need to enable interrupt processing before all the modeset
> state is set up. But that means we can fall over when we get a pipe
> underrun. This shouldn't happen as long as the bios works correctly
> but as usual this turns out to be wishful thinking.
> 
> So disable error interrupts at irq install time and rely on the
> re-enabling code in the modeset functions to take care of this.

The only issue I see now is that if we don't do a full modeset, we
never enable the error reporting. Maybe we should just enable the
underrun reporting in intel_modeset_setup_hw_state() for all
active pipes?

> 
> Note that due to the SDE interrupt handling race we must
> uncondtionally enable all interrupt sources in SDEIER, hence no need
> to enable the SERR bit specifically.
> 
> On gmch platforms we don't have an explicit enable/mask bit for fifo
> underruns. Fixing this up would require a bit of software tracking,
> hence is material for a separate patch. To make this possible we need
> to switch all gmch platforms to the new pipestat interrupt handling
> scheme Ville implemented for vlv, and then also add a safe form of sw
         ^^^^^
Imre

> state checking to __cpu_fifo_underrun_reporting_enabled a bit.
> 
> Reported-by: Rob Clark <robdclark@gmail.com>
> Cc: Rob Clark <robdclark@gmail.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/i915_irq.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index bd1f90645697..bd8541147da5 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -2922,7 +2922,7 @@ static void ibx_irq_postinstall(struct drm_device *dev)
>  		mask = SDE_GMBUS | SDE_AUX_MASK | SDE_TRANSB_FIFO_UNDER |
>  		       SDE_TRANSA_FIFO_UNDER | SDE_POISON;
>  	} else {
> -		mask = SDE_GMBUS_CPT | SDE_AUX_MASK_CPT | SDE_ERROR_CPT;
> +		mask = SDE_GMBUS_CPT | SDE_AUX_MASK_CPT;
>  
>  		I915_WRITE(SERR_INT, I915_READ(SERR_INT));
>  	}
> @@ -2982,10 +2982,9 @@ static int ironlake_irq_postinstall(struct drm_device *dev)
>  		display_mask = (DE_MASTER_IRQ_CONTROL | DE_GSE_IVB |
>  				DE_PCH_EVENT_IVB | DE_PLANEC_FLIP_DONE_IVB |
>  				DE_PLANEB_FLIP_DONE_IVB |
> -				DE_PLANEA_FLIP_DONE_IVB | DE_AUX_CHANNEL_A_IVB |
> -				DE_ERR_INT_IVB);
> +				DE_PLANEA_FLIP_DONE_IVB | DE_AUX_CHANNEL_A_IVB);
>  		extra_mask = (DE_PIPEC_VBLANK_IVB | DE_PIPEB_VBLANK_IVB |
> -			      DE_PIPEA_VBLANK_IVB);
> +			      DE_PIPEA_VBLANK_IVB | DE_ERR_INT_IVB);

The ILK branch needs to do the same for DE_PIPEB_FIFO_UNDERRUN and
DE_PIPEA_FIFO_UNDERRUN.

BTW I can reproduce the problem 100% on my ILK and IVB using this:
 setterm -blank 1 -powersave powerdown -powerdown 0
 <wait for the console to blank>
 modprobe i915

With the above extra fix for ILK, this patch seems to cure both ILK and
IVB for me, so:
Tested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

I'll withold my r-b until we figure out what to do about the
"no full modeset"case ;)

>  
>  		I915_WRITE(GEN7_ERR_INT, I915_READ(GEN7_ERR_INT));
>  	} else {
> @@ -3111,9 +3110,9 @@ static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)
>  	struct drm_device *dev = dev_priv->dev;
>  	uint32_t de_pipe_masked = GEN8_PIPE_FLIP_DONE |
>  		GEN8_PIPE_CDCLK_CRC_DONE |
> -		GEN8_PIPE_FIFO_UNDERRUN |
>  		GEN8_DE_PIPE_IRQ_FAULT_ERRORS;
> -	uint32_t de_pipe_enables = de_pipe_masked | GEN8_PIPE_VBLANK;
> +	uint32_t de_pipe_enables = de_pipe_masked | GEN8_PIPE_VBLANK |
> +		GEN8_PIPE_FIFO_UNDERRUN;
>  	int pipe;
>  	dev_priv->de_irq_mask[PIPE_A] = ~de_pipe_masked;
>  	dev_priv->de_irq_mask[PIPE_B] = ~de_pipe_masked;
> -- 
> 1.8.5.2

-- 
Ville Syrjälä
Intel OTC