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
