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

[Jani Nikula <jani.nikula@linux.intel.com>]

On Mon, 10 Mar 2014, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Fri, Mar 07, 2014 at 08:34:46PM +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.
>> 
>> 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
>          ^^^^^
>
> That's still wrong.
>
> Fix that and you can add:
> Tested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Fixed and pushed to -fixes, thanks for the patch and review.

Jani.



>
>> state checking to __cpu_fifo_underrun_reporting_enabled a bit.
>> 
>> v2: Also handle the ilk/snb cpu fifo underrun bits accordingly.
>> Spotted by Ville.
>> 
>> v3: Also handle the south interrupt underrun bits on ibx. Again
>> spotted by Ville.
>> 
>> 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 | 18 ++++++++----------
>>  1 file changed, 8 insertions(+), 10 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
>> index bd1f90645697..bb3bf82174fd 100644
>> --- a/drivers/gpu/drm/i915/i915_irq.c
>> +++ b/drivers/gpu/drm/i915/i915_irq.c
>> @@ -2919,10 +2919,9 @@ static void ibx_irq_postinstall(struct drm_device *dev)
>>  		return;
>>  
>>  	if (HAS_PCH_IBX(dev)) {
>> -		mask = SDE_GMBUS | SDE_AUX_MASK | SDE_TRANSB_FIFO_UNDER |
>> -		       SDE_TRANSA_FIFO_UNDER | SDE_POISON;
>> +		mask = SDE_GMBUS | SDE_AUX_MASK | 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,20 +2981,19 @@ 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);
>>  
>>  		I915_WRITE(GEN7_ERR_INT, I915_READ(GEN7_ERR_INT));
>>  	} else {
>>  		display_mask = (DE_MASTER_IRQ_CONTROL | DE_GSE | DE_PCH_EVENT |
>>  				DE_PLANEA_FLIP_DONE | DE_PLANEB_FLIP_DONE |
>>  				DE_AUX_CHANNEL_A |
>> -				DE_PIPEB_FIFO_UNDERRUN | DE_PIPEA_FIFO_UNDERRUN |
>>  				DE_PIPEB_CRC_DONE | DE_PIPEA_CRC_DONE |
>>  				DE_POISON);
>> -		extra_mask = DE_PIPEA_VBLANK | DE_PIPEB_VBLANK | DE_PCU_EVENT;
>> +		extra_mask = DE_PIPEA_VBLANK | DE_PIPEB_VBLANK | DE_PCU_EVENT |
>> +				DE_PIPEB_FIFO_UNDERRUN | DE_PIPEA_FIFO_UNDERRUN;
>>  	}
>>  
>>  	dev_priv->irq_mask = ~display_mask;
>> @@ -3111,9 +3109,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
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx