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

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

[Daniel Vetter]

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
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);
 
 		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

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

[Ville Syrjälä]

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

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

[Daniel Vetter]

On Fri, Mar 7, 2014 at 7:24 PM, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
>
> I'll withold my r-b until we figure out what to do about the
> "no full modeset"case ;)

Atm that case doesn't really exist outside of fastboot=1. And that has
the much larger issue of not updating the watermarks to our own
power-optimized settings. Among a few other residual issues, so I
think we can mop this under the carpet ;-)

I'll respin the patch with the ilk/snb change applied.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

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

[Daniel Vetter]

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
state checking to __cpu_fifo_underrun_reporting_enabled a bit.

v2: Also handle the ilk/snb cpu fifo underrun bits accordingly.
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 | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index bd1f90645697..18b9f600fe25 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,20 +2982,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 +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

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Daniel Vetter]

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
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

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Ville Syrjälä]

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>

> 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

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

[Jani Nikula]

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