[PATCH] drm/i915: tune down unknown unclaime write hsw debug message

[PATCH] drm/i915: tune down unknown unclaime write hsw debug message

[Daniel Vetter]

This could very well be caused by dirt left behind by the BIOS, so
tune it down to the debug level.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=58897
Tested-by: shui yangwei <yangweix.shui@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_drv.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 530db83..08dbcc3 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1258,7 +1258,7 @@ void i915_write##x(struct drm_i915_private *dev_priv, u32 reg, u##x val) { \
 	if (IS_GEN5(dev_priv->dev)) \
 		ilk_dummy_write(dev_priv); \
 	if (IS_HASWELL(dev_priv->dev) && (I915_READ_NOTRACE(GEN7_ERR_INT) & ERR_INT_MMIO_UNCLAIMED)) { \
-		DRM_ERROR("Unknown unclaimed register before writing to %x\n", reg); \
+		DRM_DEBUG("Unknown unclaimed register before writing to %x\n", reg); \
 		I915_WRITE_NOTRACE(GEN7_ERR_INT, ERR_INT_MMIO_UNCLAIMED); \
 	} \
 	if (IS_VALLEYVIEW(dev_priv->dev) && IS_DISPLAYREG(reg)) { \
-- 
1.7.10.4

Re: [PATCH] drm/i915: tune down unknown unclaime write hsw debug message

[Chris Wilson]

On Tue,  8 Jan 2013 09:33:57 +0100, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> This could very well be caused by dirt left behind by the BIOS, so
> tune it down to the debug level.

If the issue is solely the one described, then it should be silence when
we takeover, just like all the other error/debug status registers.

DRM_DEBUG is a little too noisy for this, maybe we should look at
trace_print or one of the other dynamic debugs for this class of info?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] drm/i915: tune down unknown unclaime write hsw debug message

[Daniel Vetter]

On Tue, Jan 8, 2013 at 10:32 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Tue,  8 Jan 2013 09:33:57 +0100, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>> This could very well be caused by dirt left behind by the BIOS, so
>> tune it down to the debug level.
>
> If the issue is solely the one described, then it should be silence when
> we takeover, just like all the other error/debug status registers.

Good point, I'll check whether pimping gt_reset is good enough.

> DRM_DEBUG is a little too noisy for this, maybe we should look at
> trace_print or one of the other dynamic debugs for this class of info?

The downside of trace_printk is that it doesn't pop up in dmesg. If
all that reg checking starts to hurt, we might want to exclude the GT
range from it - iirc this seemed most useful for display enabling.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH] drm/i915: tune down unknown unclaime write hsw debug message

[Ben Widawsky]

On Tue, 8 Jan 2013 11:07:03 +0100
Daniel Vetter <daniel.vetter@ffwll.ch> wrote:

> On Tue, Jan 8, 2013 at 10:32 AM, Chris Wilson
> <chris@chris-wilson.co.uk> wrote:
> > On Tue,  8 Jan 2013 09:33:57 +0100, Daniel Vetter
> > <daniel.vetter@ffwll.ch> wrote:
> >> This could very well be caused by dirt left behind by the BIOS, so
> >> tune it down to the debug level.
> >
> > If the issue is solely the one described, then it should be silence
> > when we takeover, just like all the other error/debug status
> > registers.
> 
> Good point, I'll check whether pimping gt_reset is good enough.

I agree with Chris FWIW. Changing this for the sake of the one instance
during takeover isn't good enough.

> 
> > DRM_DEBUG is a little too noisy for this, maybe we should look at
> > trace_print or one of the other dynamic debugs for this class of
> > info?
> 
> The downside of trace_printk is that it doesn't pop up in dmesg. If
> all that reg checking starts to hurt, we might want to exclude the GT
> range from it - iirc this seemed most useful for display enabling.
> -Daniel

It was useful for display enabling because that's where the biggest
delta was for IVB->HSW. I don't think that it will be the case for all
platforms. The reason I found it interesting initially is it's also
capable of catching when we try to write to powered down things.

Re: [PATCH] drm/i915: tune down unknown unclaime write hsw debug message

[Paulo Zanoni]

Hi

2013/1/8 Daniel Vetter <daniel.vetter@ffwll.ch>:
> This could very well be caused by dirt left behind by the BIOS, so
> tune it down to the debug level.

This message can also be triggered by
"I915_READ(REGISTER_THAT_DOES_NOT_EXIST)". Still, I also suspect that
the very first message we see is due to something left behind by the
BIOS.

During the very first I915_WRITE that we do in our driver we are
hitting one of these messages. My initial thought was "let's zero
GEN7_ERR_INT inside ivybridge_irq_postinstall", but then I discovered
the error message is so early in our driver initialization that the
correct place to "zero GEN7_ERR_INT" would be inside intel_irq_init
(because it seems this is before the very first I915_WRITE we do).

It seems to me that it's not good to have all those checks in our
I915_{READ/WRITE} macros, so my plans are:

Step 1 - Fix all the messages we still have (3 small patches, I
already have them locally)
Step 2 - Remove the checking code inside I915_WRITE and handle
GEN7_ERR_INT the same way we treat the other interrupts. With this, we
will lose the ability to check exactly which register is triggering
the problem, but no one is supposed to be triggering that problem
since we fixed everything in step 1. If we ever see the error message
triggered by the interrupt, we will at least be able to bisect or to
re-add the messages in a debugging session.

IMHO, knowing which register causes the problem is very useful to the
developer, but the tradeoff between "knowing the register" and
"polluting i915_write and i915_read" should be considered. It's not
hard to add the messages back for a debugging session (and today,
whenever I have to debug these messages, I also add dump_stack() to my
local code).

Opinions? If no one objects I'll submit patches for this soon.

>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=58897
> Tested-by: shui yangwei <yangweix.shui@intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/i915_drv.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 530db83..08dbcc3 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1258,7 +1258,7 @@ void i915_write##x(struct drm_i915_private *dev_priv, u32 reg, u##x val) { \
>         if (IS_GEN5(dev_priv->dev)) \
>                 ilk_dummy_write(dev_priv); \
>         if (IS_HASWELL(dev_priv->dev) && (I915_READ_NOTRACE(GEN7_ERR_INT) & ERR_INT_MMIO_UNCLAIMED)) { \
> -               DRM_ERROR("Unknown unclaimed register before writing to %x\n", reg); \
> +               DRM_DEBUG("Unknown unclaimed register before writing to %x\n", reg); \
>                 I915_WRITE_NOTRACE(GEN7_ERR_INT, ERR_INT_MMIO_UNCLAIMED); \
>         } \
>         if (IS_VALLEYVIEW(dev_priv->dev) && IS_DISPLAYREG(reg)) { \
> --
> 1.7.10.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni