[Intel-gfx] [PATCH 09/10] drm/i915: print IVB/HSW display error interrupts

Jani Nikula jani.nikula at linux.intel.com
Thu Jan 24 14:04:05 CET 2013


On Fri, 18 Jan 2013, Paulo Zanoni <przanoni at gmail.com> wrote:
> From: Paulo Zanoni <paulo.r.zanoni at intel.com>
>
> If we get one of these messages it means we did something wrong, and
> the first step to fix wrong things is to detect them and recognize
> they exist.
>
> For now, leave these messages as DRM_DEBUG_KMS. After we conclude that
> a certain message should not be print since our code is correct, then
> we can promote that specific message to DRM_ERROR.
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
> ---
>  drivers/gpu/drm/i915/i915_irq.c |   56 ++++++++++++++++++++++++++++++++++++++-
>  drivers/gpu/drm/i915/i915_reg.h |   23 ++++++++++++++--
>  2 files changed, 76 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index cc49a6d..2f17b54 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -697,6 +697,54 @@ static void cpt_irq_handler(struct drm_device *dev, u32 pch_iir)
>  					 I915_READ(FDI_RX_IIR(pipe)));
>  }
>  
> +static void ivb_err_int_handler(struct drm_device *dev)
> +{
> +	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;

dev->dev_private being a void *, the cast is unnecessary.

> +	u32 err_int = I915_READ(GEN7_ERR_INT);
> +
> +	if (err_int & ERR_INT_POISON)
> +		DRM_DEBUG_KMS("Poison interrupt\n");
> +	if (err_int & ERR_INT_INVALID_GTT_PTE)
> +		DRM_DEBUG_KMS("Invalid GTT PTE\n");
> +	if (err_int & ERR_INT_INVALID_PTE_DATA)
> +		DRM_DEBUG_KMS("Invalid GTT PTE data\n");
> +	if (err_int & ERR_INT_SPRITE_GTT_FAULT_C)
> +		DRM_DEBUG_KMS("Sprite C GTT fault\n");
> +	if (err_int & ERR_INT_PRIMARY_GTT_FAULT_C)
> +		DRM_DEBUG_KMS("Primary place C GTT fault\n");
> +	if (err_int & ERR_INT_CURSOR_GTT_FAULT_C)
> +		DRM_DEBUG_KMS("Cursor C GTT fault\n");
> +	if (err_int & ERR_INT_SPRITE_GTT_FAULT_B)
> +		DRM_DEBUG_KMS("Sprite B GTT fault\n");
> +	if (err_int & ERR_INT_PRIMARY_GTT_FAULT_B)
> +		DRM_DEBUG_KMS("Primary place B GTT fault\n");
> +	if (err_int & ERR_INT_CURSOR_GTT_FAULT_B)
> +		DRM_DEBUG_KMS("Cursor B GTT fault\n");
> +	if (err_int & ERR_INT_SPRITE_GTT_FAULT_A)
> +		DRM_DEBUG_KMS("Sprite A GTT fault\n");
> +	if (err_int & ERR_INT_PRIMARY_GTT_FAULT_A)
> +		DRM_DEBUG_KMS("Primary place A GTT fault\n");
> +	if (err_int & ERR_INT_CURSOR_GTT_FAULT_A)
> +		DRM_DEBUG_KMS("Cursor A GTT fault\n");
> +	if (err_int & ERR_INT_PIPE_CRR_ERROR_C)

CRR? Should it be CRC? Ditto for the others below.

Also, how about trading some clarity of error messages to less
duplication of info with something like:

#define err_print(status, bit) do { \
	if ((status) & (bit)) \
		DRM_DEBUG_KMS("Error interrupt %s\n", #bit);\
	} while (0)

	err_print(err_int, ERR_INT_POISON);
        err_print(err_int, ERR_INT_INVALID_GTT_PTE);
        err_print(err_int, ERR_INT_INVALID_PTE_DATA);
        ...

#undef err_print

This is just bikeshedding though, up to you.

Reviewed-by: Jani Nikula <jani.nikula at intel.com>


> +		DRM_DEBUG_KMS("Pipe C CRC error\n");
> +	if (err_int & ERR_INT_PIPE_FIFO_UNDERRRUN_C)
> +		DRM_DEBUG_KMS("Pipe C FIFO underrun\n");
> +	if (err_int & ERR_INT_PIPE_CRR_ERROR_B)
> +		DRM_DEBUG_KMS("Pipe B CRC error\n");
> +	if (err_int & ERR_INT_PIPE_FIFO_UNDERRRUN_B)
> +		DRM_DEBUG_KMS("Pipe B FIFO underrun\n");
> +	if (err_int & ERR_INT_PIPE_CRR_ERROR_A)
> +		DRM_DEBUG_KMS("Pipe A CRC error\n");
> +	if (err_int & ERR_INT_PIPE_FIFO_UNDERRRUN_A)
> +		DRM_DEBUG_KMS("Pipe A FIFO underrun\n");
> +
> +	if (IS_HASWELL(dev) && (err_int & ERR_INT_MMIO_UNCLAIMED))
> +		DRM_DEBUG_KMS("MMIO cycle not claimed\n");
> +
> +	I915_WRITE(GEN7_ERR_INT, err_int);
> +}
> +
>  static irqreturn_t ivybridge_irq_handler(int irq, void *arg)
>  {
>  	struct drm_device *dev = (struct drm_device *) arg;
> @@ -720,6 +768,9 @@ static irqreturn_t ivybridge_irq_handler(int irq, void *arg)
>  
>  	de_iir = I915_READ(DEIIR);
>  	if (de_iir) {
> +		if (de_iir & DE_ERR_INT_IVB)
> +			ivb_err_int_handler(dev);
> +
>  		if (de_iir & DE_AUX_CHANNEL_A_IVB)
>  			dp_aux_irq_handler(dev);
>  
> @@ -1964,7 +2015,7 @@ static int ivybridge_irq_postinstall(struct drm_device *dev)
>  		DE_PLANEC_FLIP_DONE_IVB |
>  		DE_PLANEB_FLIP_DONE_IVB |
>  		DE_PLANEA_FLIP_DONE_IVB |
> -		DE_AUX_CHANNEL_A_IVB;
> +		DE_AUX_CHANNEL_A_IVB | DE_ERR_INT_IVB;
>  	u32 render_irqs;
>  	u32 hotplug_mask;
>  	u32 pch_irq_mask;
> @@ -1972,6 +2023,7 @@ static int ivybridge_irq_postinstall(struct drm_device *dev)
>  	dev_priv->irq_mask = ~display_mask;
>  
>  	/* should always can generate irq */
> +	I915_WRITE(GEN7_ERR_INT, I915_READ(GEN7_ERR_INT));
>  	I915_WRITE(DEIIR, I915_READ(DEIIR));
>  	I915_WRITE(DEIMR, dev_priv->irq_mask);
>  	I915_WRITE(DEIER,
> @@ -2135,6 +2187,8 @@ static void ironlake_irq_uninstall(struct drm_device *dev)
>  	I915_WRITE(DEIMR, 0xffffffff);
>  	I915_WRITE(DEIER, 0x0);
>  	I915_WRITE(DEIIR, I915_READ(DEIIR));
> +	if (IS_GEN7(dev))
> +		I915_WRITE(GEN7_ERR_INT, I915_READ(GEN7_ERR_INT));
>  
>  	I915_WRITE(GTIMR, 0xffffffff);
>  	I915_WRITE(GTIER, 0x0);
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index f054554..e8ecdc4 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -511,7 +511,26 @@
>  
>  #define ERROR_GEN6	0x040a0
>  #define GEN7_ERR_INT	0x44040
> -#define   ERR_INT_MMIO_UNCLAIMED (1<<13)
> +#define  ERR_INT_POISON			(1<<31)
> +#define  ERR_INT_INVALID_GTT_PTE	(1<<29)
> +#define  ERR_INT_INVALID_PTE_DATA	(1<<28)
> +#define  ERR_INT_SPRITE_GTT_FAULT_C	(1<<23)
> +#define  ERR_INT_PRIMARY_GTT_FAULT_C	(1<<22)
> +#define  ERR_INT_CURSOR_GTT_FAULT_C	(1<<21)
> +#define  ERR_INT_SPRITE_GTT_FAULT_B	(1<<20)
> +#define  ERR_INT_PRIMARY_GTT_FAULT_B	(1<<19)
> +#define  ERR_INT_CURSOR_GTT_FAULT_B	(1<<18)
> +#define  ERR_INT_SPRITE_GTT_FAULT_A	(1<<17)
> +#define  ERR_INT_PRIMARY_GTT_FAULT_A	(1<<16)
> +#define  ERR_INT_CURSOR_GTT_FAULT_A	(1<<15)
> +#define  ERR_INT_PIPE_CRR_ERROR_C	(1<<7)
> +#define  ERR_INT_PIPE_FIFO_UNDERRRUN_C	(1<<6)
> +#define  ERR_INT_PIPE_CRR_ERROR_B	(1<<4)
> +#define  ERR_INT_PIPE_FIFO_UNDERRRUN_B	(1<<3)
> +#define  ERR_INT_PIPE_CRR_ERROR_A	(1<<1)
> +#define  ERR_INT_PIPE_FIFO_UNDERRRUN_A	(1<<0)
> +/* Haswell only: */
> +#define  ERR_INT_MMIO_UNCLAIMED		(1<<13)
>  
>  /* GM45+ chicken bits -- debug workaround bits that may be required
>   * for various sorts of correct behavior.  The top 16 bits of each are
> @@ -3374,7 +3393,7 @@
>  #define DE_PIPEA_FIFO_UNDERRUN  (1 << 0)
>  
>  /* More Ivybridge lolz */
> -#define DE_ERR_DEBUG_IVB		(1<<30)
> +#define DE_ERR_INT_IVB			(1<<30)
>  #define DE_GSE_IVB			(1<<29)
>  #define DE_PCH_EVENT_IVB		(1<<28)
>  #define DE_DP_A_HOTPLUG_IVB		(1<<27)
> -- 
> 1.7.10.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



More information about the Intel-gfx mailing list