[PATCH 08/10] drm/i915: print PCH FIFO underrun interrupts

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Feb 08, 2013 at 05:35:19PM -0200, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni at intel.com>
> 
> Also add an "ignore" bit that avoids printing the message in two
> cases:
>   - When the message is in fact expected.
>   - After we get the first message. In tihs case, we expect to get
>     hundreds of consecutive messages, so we just ignore all the
>     subsequent messages until the next crtc_enable.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni at intel.com>

Ah, here's the "block underrun reporting stuff". You need to set up this
infrastructure first, then enable the error interrupts. Otherwise a poor
bloke will hit the irq strom in between these two patches in a bisect and
die from a heart attack when looking at dmesg.

Also, you update the ignore_bla from the irq handler, so this needs proper
locking. And I vote for the bikeshed to be moved into struct intel_crtc,
e.g.

struct intel_crtc {
	...
	
	struct spinlock underrun_lock;
	int underrun_reporting_enabled;
}

Then smash a little helper into every crtc_enable/disable function. Aside:
you need to use the spin_lock_irqsafe/restore functions in the irq handler
(and for safety also in the helpers).  Another curious question: Why do we
need separate ignore bits for the cpu and the pch?
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_drv.h      |    3 +++
>  drivers/gpu/drm/i915/i915_irq.c      |   37 +++++++++++++++++++++++++++++-----
>  drivers/gpu/drm/i915/i915_reg.h      |    7 +++++--
>  drivers/gpu/drm/i915/intel_display.c |   14 ++++++++++++-
>  4 files changed, 53 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 08c5def..e96d75e 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -909,6 +909,9 @@ typedef struct drm_i915_private {
>  	struct work_struct hotplug_work;
>  	bool enable_hotplug_processing;
>  
> +	/* Bit 0: PCH transcoder A and so on. */
> +	u8 ignore_pch_fifo_underrun;
> +
>  	int num_pipe;
>  	int num_pch_pll;
>  
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 703a08a..7497589 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -659,10 +659,17 @@ static void ibx_irq_handler(struct drm_device *dev, u32 pch_iir)
>  	if (pch_iir & (SDE_TRANSB_CRC_ERR | SDE_TRANSA_CRC_ERR))
>  		DRM_DEBUG_DRIVER("PCH transcoder CRC error interrupt\n");
>  
> -	if (pch_iir & SDE_TRANSB_FIFO_UNDER)
> -		DRM_DEBUG_DRIVER("PCH transcoder B underrun interrupt\n");
> -	if (pch_iir & SDE_TRANSA_FIFO_UNDER)
> -		DRM_DEBUG_DRIVER("PCH transcoder A underrun interrupt\n");
> +	if ((pch_iir & SDE_TRANSB_FIFO_UNDER) &&
> +	    !(dev_priv->ignore_pch_fifo_underrun & (1 << TRANSCODER_B))) {
> +		DRM_DEBUG_DRIVER("PCH transcoder B underrun\n");
> +		dev_priv->ignore_pch_fifo_underrun |= (1 << TRANSCODER_B);
> +	}
> +
> +	if ((pch_iir & SDE_TRANSA_FIFO_UNDER) &&
> +	    !(dev_priv->ignore_pch_fifo_underrun & (1 << TRANSCODER_A))) {
> +		DRM_DEBUG_DRIVER("PCH transcoder A underrun\n");
> +		dev_priv->ignore_pch_fifo_underrun |= (1 << TRANSCODER_A);
> +	}
>  }
>  
>  static void err_int_handler(struct drm_device *dev)
> @@ -684,6 +691,24 @@ static void serr_int_handler(struct drm_device *dev)
>  	if (serr_int & SERR_INT_POISON)
>  		DRM_ERROR("PCH poison interrupt\n");
>  
> +	if ((serr_int & SERR_INT_TRANS_A_FIFO_UNDERRUN) &&
> +	    !(dev_priv->ignore_pch_fifo_underrun & (1 << TRANSCODER_A))) {
> +		DRM_ERROR("PCH transcoder A FIFO underrun\n");
> +		dev_priv->ignore_pch_fifo_underrun |= (1 << TRANSCODER_A);
> +	}
> +
> +	if ((serr_int & SERR_INT_TRANS_B_FIFO_UNDERRUN) &&
> +	    !(dev_priv->ignore_pch_fifo_underrun & (1 << TRANSCODER_B))) {
> +		DRM_ERROR("PCH transcoder B FIFO underrun\n");
> +		dev_priv->ignore_pch_fifo_underrun |= (1 << TRANSCODER_B);
> +	}
> +
> +	if ((serr_int & SERR_INT_TRANS_C_FIFO_UNDERRUN) &&
> +	    !(dev_priv->ignore_pch_fifo_underrun & (1 << TRANSCODER_C))) {
> +		DRM_ERROR("PCH transcoder C FIFO underrun\n");
> +		dev_priv->ignore_pch_fifo_underrun |= (1 << TRANSCODER_C);
> +	}
> +
>  	I915_WRITE(SERR_INT, serr_int);
>  }
>  
> @@ -2000,7 +2025,9 @@ static void ibx_irq_postinstall(struct drm_device *dev)
>  		mask = SDE_HOTPLUG_MASK |
>  		       SDE_GMBUS |
>  		       SDE_AUX_MASK |
> -		       SDE_POISON;
> +		       SDE_POISON |
> +		       SDE_TRANSB_FIFO_UNDER |
> +		       SDE_TRANSA_FIFO_UNDER;
>  	} else {
>  		mask = SDE_HOTPLUG_MASK_CPT |
>  		       SDE_GMBUS_CPT |
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index f22e27d..d565bd7 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3554,8 +3554,11 @@
>  #define SDEIIR  0xc4008
>  #define SDEIER  0xc400c
>  
> -#define SERR_INT		0xc4040
> -#define  SERR_INT_POISON	(1 << 31)
> +#define SERR_INT			0xc4040
> +#define  SERR_INT_POISON		(1 << 31)
> +#define  SERR_INT_TRANS_C_FIFO_UNDERRUN	(1 << 6)
> +#define  SERR_INT_TRANS_B_FIFO_UNDERRUN	(1 << 3)
> +#define  SERR_INT_TRANS_A_FIFO_UNDERRUN	(1 << 0)
>  
>  /* digital port hotplug */
>  #define PCH_PORT_HOTPLUG        0xc4030		/* SHOTPLUG_CTL */
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index d75c6a0..67bfb58 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3274,6 +3274,9 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
>  		return;
>  
>  	intel_crtc->active = true;
> +
> +	dev_priv->ignore_pch_fifo_underrun &= ~(1 << pipe);
> +
>  	intel_update_watermarks(dev);
>  
>  	if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS)) {
> @@ -3366,11 +3369,15 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
>  		return;
>  
>  	intel_crtc->active = true;
> -	intel_update_watermarks(dev);
>  
>  	is_pch_port = haswell_crtc_driving_pch(crtc);
>  
>  	if (is_pch_port)
> +		dev_priv->ignore_pch_fifo_underrun &= ~(1 << TRANSCODER_A);
> +
> +	intel_update_watermarks(dev);
> +
> +	if (is_pch_port)
>  		dev_priv->display.fdi_link_train(crtc);
>  
>  	for_each_encoder_on_crtc(dev, crtc, encoder)
> @@ -3453,6 +3460,7 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)
>  	if (dev_priv->cfb_plane == plane)
>  		intel_disable_fbc(dev);
>  
> +	dev_priv->ignore_pch_fifo_underrun |= (1 << pipe);
>  	intel_disable_pipe(dev_priv, pipe);
>  
>  	/* Disable PF */
> @@ -3466,6 +3474,7 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)
>  	ironlake_fdi_disable(crtc);
>  
>  	ironlake_disable_pch_transcoder(dev_priv, pipe);
> +	dev_priv->ignore_pch_fifo_underrun &= ~(1 << pipe);
>  
>  	if (HAS_PCH_CPT(dev)) {
>  		/* disable TRANS_DP_CTL */
> @@ -3535,6 +3544,8 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
>  	if (dev_priv->cfb_plane == plane)
>  		intel_disable_fbc(dev);
>  
> +	if (is_pch_port)
> +		dev_priv->ignore_pch_fifo_underrun |= (1 << TRANSCODER_A);
>  	intel_disable_pipe(dev_priv, pipe);
>  
>  	intel_ddi_disable_transcoder_func(dev_priv, cpu_transcoder);
> @@ -3551,6 +3562,7 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
>  
>  	if (is_pch_port) {
>  		lpt_disable_pch_transcoder(dev_priv);
> +		dev_priv->ignore_pch_fifo_underrun &= ~(1 << TRANSCODER_A);
>  		intel_ddi_fdi_disable(crtc);
>  	}
>  
> -- 
> 1.7.10.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
+41 (0) 79 365 57 48 - http://blog.ffwll.ch


[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux