All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Disable the mmio.debug WARN after it fires
@ 2014-11-24  8:12 Chris Wilson
  2014-11-24  9:45 ` Daniel Vetter
  2014-11-24 16:28 ` [PATCH] drm/i915: Disable the mmio.debug WARN after it shuang.he
  0 siblings, 2 replies; 8+ messages in thread
From: Chris Wilson @ 2014-11-24  8:12 UTC (permalink / raw)
  To: intel-gfx; +Cc: Paulo Zanoni

If we have a single unclaimed register, we will have lots. A WARN for
each one makes the machine unusable and does not aid debugging. Convert
the i915.mmio_debug option to a counter for how many WARNs to fire
before shutting up. Even when i915.mmio_debug was disabled it would
continue to shout an *ERROR* for every interrupt, without any
information at all for debugging.

The massive verbiage was added in
commit 5978118c39c2f72fd8b39ef9c086723542384809
Author: Paulo Zanoni <paulo.r.zanoni@intel.com>
Date:   Wed Jul 16 17:49:29 2014 -0300

    drm/i915: reorganize the unclaimed register detection code

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Rodrigo Vivi <rodrigo.vivi@gmail.com>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h     | 2 +-
 drivers/gpu/drm/i915/i915_params.c  | 5 ++---
 drivers/gpu/drm/i915/intel_uncore.c | 6 ++++--
 3 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 5448ce9d1490..314d8a60d55b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2310,7 +2310,7 @@ struct i915_params {
 	bool disable_display;
 	bool disable_vtd_wa;
 	int use_mmio_flip;
-	bool mmio_debug;
+	int mmio_debug;
 };
 extern struct i915_params i915 __read_mostly;
 
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index c91cb2033cc5..654a492fcfca 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -169,7 +169,6 @@ module_param_named(use_mmio_flip, i915.use_mmio_flip, int, 0600);
 MODULE_PARM_DESC(use_mmio_flip,
 		 "use MMIO flips (-1=never, 0=driver discretion [default], 1=always)");
 
-module_param_named(mmio_debug, i915.mmio_debug, bool, 0600);
+module_param_named(mmio_debug, i915.mmio_debug, int, 0600);
 MODULE_PARM_DESC(mmio_debug,
-	"Enable the MMIO debug code (default: false). This may negatively "
-	"affect performance.");
+	"Enable the MMIO debug code. This may negatively affect performance.");
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index f0230b0e8e11..b830407dc2aa 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -714,25 +714,27 @@ hsw_unclaimed_reg_debug(struct drm_i915_private *dev_priv, u32 reg, bool read,
 	const char *op = read ? "reading" : "writing to";
 	const char *when = before ? "before" : "after";
 
-	if (!i915.mmio_debug)
+	if (i915.mmio_debug <= 0)
 		return;
 
 	if (__raw_i915_read32(dev_priv, FPGA_DBG) & FPGA_DBG_RM_NOCLAIM) {
 		WARN(1, "Unclaimed register detected %s %s register 0x%x\n",
 		     when, op, reg);
 		__raw_i915_write32(dev_priv, FPGA_DBG, FPGA_DBG_RM_NOCLAIM);
+		i915.mmio_debug--;
 	}
 }
 
 static void
 hsw_unclaimed_reg_detect(struct drm_i915_private *dev_priv)
 {
-	if (i915.mmio_debug)
+	if (i915.mmio_debug > 0)
 		return;
 
 	if (__raw_i915_read32(dev_priv, FPGA_DBG) & FPGA_DBG_RM_NOCLAIM) {
 		DRM_ERROR("Unclaimed register detected. Please use the i915.mmio_debug=1 to debug this problem.");
 		__raw_i915_write32(dev_priv, FPGA_DBG, FPGA_DBG_RM_NOCLAIM);
+		i915.mmio_debug--;
 	}
 }
 
-- 
2.1.3

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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/i915: Disable the mmio.debug WARN after it fires
  2014-11-24  8:12 [PATCH] drm/i915: Disable the mmio.debug WARN after it fires Chris Wilson
@ 2014-11-24  9:45 ` Daniel Vetter
  2014-11-24 10:06   ` Chris Wilson
  2014-11-24 16:28 ` [PATCH] drm/i915: Disable the mmio.debug WARN after it shuang.he
  1 sibling, 1 reply; 8+ messages in thread
From: Daniel Vetter @ 2014-11-24  9:45 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx, Paulo Zanoni

On Mon, Nov 24, 2014 at 08:12:57AM +0000, Chris Wilson wrote:
> If we have a single unclaimed register, we will have lots. A WARN for
> each one makes the machine unusable and does not aid debugging. Convert
> the i915.mmio_debug option to a counter for how many WARNs to fire
> before shutting up. Even when i915.mmio_debug was disabled it would
> continue to shout an *ERROR* for every interrupt, without any
> information at all for debugging.
> 
> The massive verbiage was added in
> commit 5978118c39c2f72fd8b39ef9c086723542384809
> Author: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Date:   Wed Jul 16 17:49:29 2014 -0300
> 
>     drm/i915: reorganize the unclaimed register detection code
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h     | 2 +-
>  drivers/gpu/drm/i915/i915_params.c  | 5 ++---
>  drivers/gpu/drm/i915/intel_uncore.c | 6 ++++--
>  3 files changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 5448ce9d1490..314d8a60d55b 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2310,7 +2310,7 @@ struct i915_params {
>  	bool disable_display;
>  	bool disable_vtd_wa;
>  	int use_mmio_flip;
> -	bool mmio_debug;
> +	int mmio_debug;
>  };
>  extern struct i915_params i915 __read_mostly;
>  
> diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
> index c91cb2033cc5..654a492fcfca 100644
> --- a/drivers/gpu/drm/i915/i915_params.c
> +++ b/drivers/gpu/drm/i915/i915_params.c
> @@ -169,7 +169,6 @@ module_param_named(use_mmio_flip, i915.use_mmio_flip, int, 0600);
>  MODULE_PARM_DESC(use_mmio_flip,
>  		 "use MMIO flips (-1=never, 0=driver discretion [default], 1=always)");
>  
> -module_param_named(mmio_debug, i915.mmio_debug, bool, 0600);
> +module_param_named(mmio_debug, i915.mmio_debug, int, 0600);
>  MODULE_PARM_DESC(mmio_debug,
> -	"Enable the MMIO debug code (default: false). This may negatively "
> -	"affect performance.");
> +	"Enable the MMIO debug code. This may negatively affect performance.");
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index f0230b0e8e11..b830407dc2aa 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -714,25 +714,27 @@ hsw_unclaimed_reg_debug(struct drm_i915_private *dev_priv, u32 reg, bool read,
>  	const char *op = read ? "reading" : "writing to";
>  	const char *when = before ? "before" : "after";
>  
> -	if (!i915.mmio_debug)
> +	if (i915.mmio_debug <= 0)
>  		return;
>  
>  	if (__raw_i915_read32(dev_priv, FPGA_DBG) & FPGA_DBG_RM_NOCLAIM) {
>  		WARN(1, "Unclaimed register detected %s %s register 0x%x\n",
>  		     when, op, reg);
>  		__raw_i915_write32(dev_priv, FPGA_DBG, FPGA_DBG_RM_NOCLAIM);
> +		i915.mmio_debug--;
>  	}
>  }
>  
>  static void
>  hsw_unclaimed_reg_detect(struct drm_i915_private *dev_priv)
>  {
> -	if (i915.mmio_debug)
> +	if (i915.mmio_debug > 0)
>  		return;
>  
>  	if (__raw_i915_read32(dev_priv, FPGA_DBG) & FPGA_DBG_RM_NOCLAIM) {
>  		DRM_ERROR("Unclaimed register detected. Please use the i915.mmio_debug=1 to debug this problem.");
>  		__raw_i915_write32(dev_priv, FPGA_DBG, FPGA_DBG_RM_NOCLAIM);
> +		i915.mmio_debug--;

Why the decrement here? And if we make this fancy, maybe we should have a
oneshot set to enable mmio_debug here, i.e.

static mmio_debug_oneshot = 1;

if (unclaimed_mmio_detected) {
	clear();
	mmio_debug = mmio_debug_oneshot--;
}

Or something similar? That way if it's recurring we'll capture it, but
only once.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/i915: Disable the mmio.debug WARN after it fires
  2014-11-24  9:45 ` Daniel Vetter
@ 2014-11-24 10:06   ` Chris Wilson
  2014-11-24 10:24     ` Chris Wilson
  0 siblings, 1 reply; 8+ messages in thread
From: Chris Wilson @ 2014-11-24 10:06 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx, Paulo Zanoni

On Mon, Nov 24, 2014 at 10:45:38AM +0100, Daniel Vetter wrote:
> On Mon, Nov 24, 2014 at 08:12:57AM +0000, Chris Wilson wrote:
> > If we have a single unclaimed register, we will have lots. A WARN for
> > each one makes the machine unusable and does not aid debugging. Convert
> > the i915.mmio_debug option to a counter for how many WARNs to fire
> > before shutting up. Even when i915.mmio_debug was disabled it would
> > continue to shout an *ERROR* for every interrupt, without any
> > information at all for debugging.
> > 
> > The massive verbiage was added in
> > commit 5978118c39c2f72fd8b39ef9c086723542384809
> > Author: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > Date:   Wed Jul 16 17:49:29 2014 -0300
> > 
> >     drm/i915: reorganize the unclaimed register detection code
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h     | 2 +-
> >  drivers/gpu/drm/i915/i915_params.c  | 5 ++---
> >  drivers/gpu/drm/i915/intel_uncore.c | 6 ++++--
> >  3 files changed, 7 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 5448ce9d1490..314d8a60d55b 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -2310,7 +2310,7 @@ struct i915_params {
> >  	bool disable_display;
> >  	bool disable_vtd_wa;
> >  	int use_mmio_flip;
> > -	bool mmio_debug;
> > +	int mmio_debug;
> >  };
> >  extern struct i915_params i915 __read_mostly;
> >  
> > diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
> > index c91cb2033cc5..654a492fcfca 100644
> > --- a/drivers/gpu/drm/i915/i915_params.c
> > +++ b/drivers/gpu/drm/i915/i915_params.c
> > @@ -169,7 +169,6 @@ module_param_named(use_mmio_flip, i915.use_mmio_flip, int, 0600);
> >  MODULE_PARM_DESC(use_mmio_flip,
> >  		 "use MMIO flips (-1=never, 0=driver discretion [default], 1=always)");
> >  
> > -module_param_named(mmio_debug, i915.mmio_debug, bool, 0600);
> > +module_param_named(mmio_debug, i915.mmio_debug, int, 0600);
> >  MODULE_PARM_DESC(mmio_debug,
> > -	"Enable the MMIO debug code (default: false). This may negatively "
> > -	"affect performance.");
> > +	"Enable the MMIO debug code. This may negatively affect performance.");
> > diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> > index f0230b0e8e11..b830407dc2aa 100644
> > --- a/drivers/gpu/drm/i915/intel_uncore.c
> > +++ b/drivers/gpu/drm/i915/intel_uncore.c
> > @@ -714,25 +714,27 @@ hsw_unclaimed_reg_debug(struct drm_i915_private *dev_priv, u32 reg, bool read,
> >  	const char *op = read ? "reading" : "writing to";
> >  	const char *when = before ? "before" : "after";
> >  
> > -	if (!i915.mmio_debug)
> > +	if (i915.mmio_debug <= 0)
> >  		return;
> >  
> >  	if (__raw_i915_read32(dev_priv, FPGA_DBG) & FPGA_DBG_RM_NOCLAIM) {
> >  		WARN(1, "Unclaimed register detected %s %s register 0x%x\n",
> >  		     when, op, reg);
> >  		__raw_i915_write32(dev_priv, FPGA_DBG, FPGA_DBG_RM_NOCLAIM);
> > +		i915.mmio_debug--;
> >  	}
> >  }
> >  
> >  static void
> >  hsw_unclaimed_reg_detect(struct drm_i915_private *dev_priv)
> >  {
> > -	if (i915.mmio_debug)
> > +	if (i915.mmio_debug > 0)
> >  		return;
> >  
> >  	if (__raw_i915_read32(dev_priv, FPGA_DBG) & FPGA_DBG_RM_NOCLAIM) {
> >  		DRM_ERROR("Unclaimed register detected. Please use the i915.mmio_debug=1 to debug this problem.");
> >  		__raw_i915_write32(dev_priv, FPGA_DBG, FPGA_DBG_RM_NOCLAIM);
> > +		i915.mmio_debug--;
> 
> Why the decrement here? And if we make this fancy, maybe we should have a
> oneshot set to enable mmio_debug here, i.e.

I only want to see the ERROR once, not every interrupt :)
 
> static mmio_debug_oneshot = 1;
> 
> if (unclaimed_mmio_detected) {
> 	clear();
> 	mmio_debug = mmio_debug_oneshot--;
> }
> 
> Or something similar? That way if it's recurring we'll capture it, but
> only once.

I suggested something similar to do a one shot enabling of mmio.debug if
we hit the error during interrupts to tumbleweeds. I think that would be
much more useful than our current error.

So

index b830407dc2aa..e7a990c449c4 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -714,7 +714,7 @@ hsw_unclaimed_reg_debug(struct drm_i915_private *dev_priv, u32 reg, bool read,
        const char *op = read ? "reading" : "writing to";
        const char *when = before ? "before" : "after";
 
-       if (i915.mmio_debug <= 0)
+       if (i915.mmio_debug)
                return;
 
        if (__raw_i915_read32(dev_priv, FPGA_DBG) & FPGA_DBG_RM_NOCLAIM) {
@@ -728,13 +728,17 @@ hsw_unclaimed_reg_debug(struct drm_i915_private *dev_priv, u32 reg, bool read,
 static void
 hsw_unclaimed_reg_detect(struct drm_i915_private *dev_priv)
 {
-       if (i915.mmio_debug > 0)
+       static bool mmio_debug_once = true;
+
+       if (i915.mmio_debug || !i915.mmio_debug_once)
                return;
 
        if (__raw_i915_read32(dev_priv, FPGA_DBG) & FPGA_DBG_RM_NOCLAIM) {
-               DRM_ERROR("Unclaimed register detected. Please use the i915.mmio_debug=1 to debug this problem.");
+               DRM_DEBUG("Unclaimed register detected, "
+                         "enabling oneshot unclaimed register detection. "
+                         "Please use the i915.mmio_debug=N for more information.\n");
                __raw_i915_write32(dev_priv, FPGA_DBG, FPGA_DBG_RM_NOCLAIM);
-               i915.mmio_debug--;
+               i915.mmio_debug = i915.mmio_debug_once--;
        }
 }

-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH] drm/i915: Disable the mmio.debug WARN after it fires
  2014-11-24 10:06   ` Chris Wilson
@ 2014-11-24 10:24     ` Chris Wilson
  2014-11-24 10:52       ` Jani Nikula
  0 siblings, 1 reply; 8+ messages in thread
From: Chris Wilson @ 2014-11-24 10:24 UTC (permalink / raw)
  To: intel-gfx; +Cc: Daniel Vetter, Paulo Zanoni

If we have a single unclaimed register, we will have lots. A WARN for
each one makes the machine unusable and does not aid debugging. Convert
the i915.mmio_debug option to a counter for how many WARNs to fire
before shutting up. Even when i915.mmio_debug was disabled it would
continue to shout an *ERROR* for every interrupt, without any
information at all for debugging.

The massive verbiage was added in
commit 5978118c39c2f72fd8b39ef9c086723542384809
Author: Paulo Zanoni <paulo.r.zanoni@intel.com>
Date:   Wed Jul 16 17:49:29 2014 -0300

    drm/i915: reorganize the unclaimed register detection code

v2: Automatically enable invalid mmio reporting for the *next* invalid
access if mmio_debug is disabled by default. This should give us clearer
debug information without polluting the logs too much.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Rodrigo Vivi <rodrigo.vivi@gmail.com>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_drv.h     |  2 +-
 drivers/gpu/drm/i915/i915_params.c  |  6 +++---
 drivers/gpu/drm/i915/intel_uncore.c | 10 ++++++++--
 3 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 5448ce9d1490..314d8a60d55b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2310,7 +2310,7 @@ struct i915_params {
 	bool disable_display;
 	bool disable_vtd_wa;
 	int use_mmio_flip;
-	bool mmio_debug;
+	int mmio_debug;
 };
 extern struct i915_params i915 __read_mostly;
 
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index c91cb2033cc5..9de6e8265a14 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -169,7 +169,7 @@ module_param_named(use_mmio_flip, i915.use_mmio_flip, int, 0600);
 MODULE_PARM_DESC(use_mmio_flip,
 		 "use MMIO flips (-1=never, 0=driver discretion [default], 1=always)");
 
-module_param_named(mmio_debug, i915.mmio_debug, bool, 0600);
+module_param_named(mmio_debug, i915.mmio_debug, int, 0600);
 MODULE_PARM_DESC(mmio_debug,
-	"Enable the MMIO debug code (default: false). This may negatively "
-	"affect performance.");
+	"Enable the MMIO debug code (default: off). "
+	"This may negatively affect performance.");
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index f0230b0e8e11..01ae5aec9e02 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -721,18 +721,24 @@ hsw_unclaimed_reg_debug(struct drm_i915_private *dev_priv, u32 reg, bool read,
 		WARN(1, "Unclaimed register detected %s %s register 0x%x\n",
 		     when, op, reg);
 		__raw_i915_write32(dev_priv, FPGA_DBG, FPGA_DBG_RM_NOCLAIM);
+		i915.mmio_debug--; /* Only report the first N failures */
 	}
 }
 
 static void
 hsw_unclaimed_reg_detect(struct drm_i915_private *dev_priv)
 {
-	if (i915.mmio_debug)
+	static bool mmio_debug_once = true;
+
+	if (i915.mmio_debug || !i915.mmio_debug_once)
 		return;
 
 	if (__raw_i915_read32(dev_priv, FPGA_DBG) & FPGA_DBG_RM_NOCLAIM) {
-		DRM_ERROR("Unclaimed register detected. Please use the i915.mmio_debug=1 to debug this problem.");
+		DRM_DEBUG("Unclaimed register detected, "
+			  "enabling oneshot unclaimed register reporting. "
+			  "Please use the i915.mmio_debug=N for more information.\n");
 		__raw_i915_write32(dev_priv, FPGA_DBG, FPGA_DBG_RM_NOCLAIM);
+		i915.mmio_debug = i915.mmio_debug_once--;
 	}
 }
 
-- 
2.1.3

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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/i915: Disable the mmio.debug WARN after it fires
  2014-11-24 10:24     ` Chris Wilson
@ 2014-11-24 10:52       ` Jani Nikula
  2014-11-24 11:07         ` Chris Wilson
  0 siblings, 1 reply; 8+ messages in thread
From: Jani Nikula @ 2014-11-24 10:52 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx; +Cc: Daniel Vetter, Paulo Zanoni

On Mon, 24 Nov 2014, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> If we have a single unclaimed register, we will have lots. A WARN for
> each one makes the machine unusable and does not aid debugging. Convert
> the i915.mmio_debug option to a counter for how many WARNs to fire
> before shutting up. Even when i915.mmio_debug was disabled it would
> continue to shout an *ERROR* for every interrupt, without any
> information at all for debugging.
>
> The massive verbiage was added in
> commit 5978118c39c2f72fd8b39ef9c086723542384809
> Author: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Date:   Wed Jul 16 17:49:29 2014 -0300
>
>     drm/i915: reorganize the unclaimed register detection code
>
> v2: Automatically enable invalid mmio reporting for the *next* invalid
> access if mmio_debug is disabled by default. This should give us clearer
> debug information without polluting the logs too much.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/i915_drv.h     |  2 +-
>  drivers/gpu/drm/i915/i915_params.c  |  6 +++---
>  drivers/gpu/drm/i915/intel_uncore.c | 10 ++++++++--
>  3 files changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 5448ce9d1490..314d8a60d55b 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2310,7 +2310,7 @@ struct i915_params {
>  	bool disable_display;
>  	bool disable_vtd_wa;
>  	int use_mmio_flip;
> -	bool mmio_debug;
> +	int mmio_debug;
>  };
>  extern struct i915_params i915 __read_mostly;
>  
> diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
> index c91cb2033cc5..9de6e8265a14 100644
> --- a/drivers/gpu/drm/i915/i915_params.c
> +++ b/drivers/gpu/drm/i915/i915_params.c
> @@ -169,7 +169,7 @@ module_param_named(use_mmio_flip, i915.use_mmio_flip, int, 0600);
>  MODULE_PARM_DESC(use_mmio_flip,
>  		 "use MMIO flips (-1=never, 0=driver discretion [default], 1=always)");
>  
> -module_param_named(mmio_debug, i915.mmio_debug, bool, 0600);
> +module_param_named(mmio_debug, i915.mmio_debug, int, 0600);
>  MODULE_PARM_DESC(mmio_debug,
> -	"Enable the MMIO debug code (default: false). This may negatively "
> -	"affect performance.");
> +	"Enable the MMIO debug code (default: off). "
> +	"This may negatively affect performance.");
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index f0230b0e8e11..01ae5aec9e02 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -721,18 +721,24 @@ hsw_unclaimed_reg_debug(struct drm_i915_private *dev_priv, u32 reg, bool read,
>  		WARN(1, "Unclaimed register detected %s %s register 0x%x\n",
>  		     when, op, reg);
>  		__raw_i915_write32(dev_priv, FPGA_DBG, FPGA_DBG_RM_NOCLAIM);
> +		i915.mmio_debug--; /* Only report the first N failures */
>  	}
>  }
>  
>  static void
>  hsw_unclaimed_reg_detect(struct drm_i915_private *dev_priv)
>  {
> -	if (i915.mmio_debug)
> +	static bool mmio_debug_once = true;
> +
> +	if (i915.mmio_debug || !i915.mmio_debug_once)
>  		return;
>  
>  	if (__raw_i915_read32(dev_priv, FPGA_DBG) & FPGA_DBG_RM_NOCLAIM) {
> -		DRM_ERROR("Unclaimed register detected. Please use the i915.mmio_debug=1 to debug this problem.");
> +		DRM_DEBUG("Unclaimed register detected, "
> +			  "enabling oneshot unclaimed register reporting. "
> +			  "Please use the i915.mmio_debug=N for more information.\n");
>  		__raw_i915_write32(dev_priv, FPGA_DBG, FPGA_DBG_RM_NOCLAIM);
> +		i915.mmio_debug = i915.mmio_debug_once--;

I suspect you have uncommitted changes in your work tree.

BR,
Jani.

>  	}
>  }
>  
> -- 
> 2.1.3
>
> _______________________________________________
> 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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/i915: Disable the mmio.debug WARN after it fires
  2014-11-24 10:52       ` Jani Nikula
@ 2014-11-24 11:07         ` Chris Wilson
  2014-11-24 16:23           ` Paulo Zanoni
  0 siblings, 1 reply; 8+ messages in thread
From: Chris Wilson @ 2014-11-24 11:07 UTC (permalink / raw)
  To: Jani Nikula; +Cc: Daniel Vetter, intel-gfx, Paulo Zanoni

On Mon, Nov 24, 2014 at 12:52:23PM +0200, Jani Nikula wrote:
> On Mon, 24 Nov 2014, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > If we have a single unclaimed register, we will have lots. A WARN for
> > each one makes the machine unusable and does not aid debugging. Convert
> > the i915.mmio_debug option to a counter for how many WARNs to fire
> > before shutting up. Even when i915.mmio_debug was disabled it would
> > continue to shout an *ERROR* for every interrupt, without any
> > information at all for debugging.
> >
> > The massive verbiage was added in
> > commit 5978118c39c2f72fd8b39ef9c086723542384809
> > Author: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > Date:   Wed Jul 16 17:49:29 2014 -0300
> >
> >     drm/i915: reorganize the unclaimed register detection code
> >
> > v2: Automatically enable invalid mmio reporting for the *next* invalid
> > access if mmio_debug is disabled by default. This should give us clearer
> > debug information without polluting the logs too much.
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h     |  2 +-
> >  drivers/gpu/drm/i915/i915_params.c  |  6 +++---
> >  drivers/gpu/drm/i915/intel_uncore.c | 10 ++++++++--
> >  3 files changed, 12 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 5448ce9d1490..314d8a60d55b 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -2310,7 +2310,7 @@ struct i915_params {
> >  	bool disable_display;
> >  	bool disable_vtd_wa;
> >  	int use_mmio_flip;
> > -	bool mmio_debug;
> > +	int mmio_debug;
> >  };
> >  extern struct i915_params i915 __read_mostly;
> >  
> > diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
> > index c91cb2033cc5..9de6e8265a14 100644
> > --- a/drivers/gpu/drm/i915/i915_params.c
> > +++ b/drivers/gpu/drm/i915/i915_params.c
> > @@ -169,7 +169,7 @@ module_param_named(use_mmio_flip, i915.use_mmio_flip, int, 0600);
> >  MODULE_PARM_DESC(use_mmio_flip,
> >  		 "use MMIO flips (-1=never, 0=driver discretion [default], 1=always)");
> >  
> > -module_param_named(mmio_debug, i915.mmio_debug, bool, 0600);
> > +module_param_named(mmio_debug, i915.mmio_debug, int, 0600);
> >  MODULE_PARM_DESC(mmio_debug,
> > -	"Enable the MMIO debug code (default: false). This may negatively "
> > -	"affect performance.");
> > +	"Enable the MMIO debug code (default: off). "
> > +	"This may negatively affect performance.");
> > diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> > index f0230b0e8e11..01ae5aec9e02 100644
> > --- a/drivers/gpu/drm/i915/intel_uncore.c
> > +++ b/drivers/gpu/drm/i915/intel_uncore.c
> > @@ -721,18 +721,24 @@ hsw_unclaimed_reg_debug(struct drm_i915_private *dev_priv, u32 reg, bool read,
> >  		WARN(1, "Unclaimed register detected %s %s register 0x%x\n",
> >  		     when, op, reg);
> >  		__raw_i915_write32(dev_priv, FPGA_DBG, FPGA_DBG_RM_NOCLAIM);
> > +		i915.mmio_debug--; /* Only report the first N failures */
> >  	}
> >  }
> >  
> >  static void
> >  hsw_unclaimed_reg_detect(struct drm_i915_private *dev_priv)
> >  {
> > -	if (i915.mmio_debug)
> > +	static bool mmio_debug_once = true;
> > +
> > +	if (i915.mmio_debug || !i915.mmio_debug_once)
> >  		return;
> >  
> >  	if (__raw_i915_read32(dev_priv, FPGA_DBG) & FPGA_DBG_RM_NOCLAIM) {
> > -		DRM_ERROR("Unclaimed register detected. Please use the i915.mmio_debug=1 to debug this problem.");
> > +		DRM_DEBUG("Unclaimed register detected, "
> > +			  "enabling oneshot unclaimed register reporting. "
> > +			  "Please use the i915.mmio_debug=N for more information.\n");
> >  		__raw_i915_write32(dev_priv, FPGA_DBG, FPGA_DBG_RM_NOCLAIM);
> > +		i915.mmio_debug = i915.mmio_debug_once--;
> 
> I suspect you have uncommitted changes in your work tree.

Or that I only use this tree to write gedankenexperiment against -nightly...
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/i915: Disable the mmio.debug WARN after it fires
  2014-11-24 11:07         ` Chris Wilson
@ 2014-11-24 16:23           ` Paulo Zanoni
  0 siblings, 0 replies; 8+ messages in thread
From: Paulo Zanoni @ 2014-11-24 16:23 UTC (permalink / raw)
  To: Chris Wilson, Jani Nikula, Intel Graphics Development,
	Daniel Vetter, Paulo Zanoni

2014-11-24 9:07 GMT-02:00 Chris Wilson <chris@chris-wilson.co.uk>:
> On Mon, Nov 24, 2014 at 12:52:23PM +0200, Jani Nikula wrote:
>> On Mon, 24 Nov 2014, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>> > If we have a single unclaimed register, we will have lots. A WARN for
>> > each one makes the machine unusable and does not aid debugging. Convert
>> > the i915.mmio_debug option to a counter for how many WARNs to fire
>> > before shutting up. Even when i915.mmio_debug was disabled it would
>> > continue to shout an *ERROR* for every interrupt, without any
>> > information at all for debugging.
>> >
>> > The massive verbiage was added in
>> > commit 5978118c39c2f72fd8b39ef9c086723542384809
>> > Author: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> > Date:   Wed Jul 16 17:49:29 2014 -0300
>> >
>> >     drm/i915: reorganize the unclaimed register detection code
>> >
>> > v2: Automatically enable invalid mmio reporting for the *next* invalid
>> > access if mmio_debug is disabled by default. This should give us clearer
>> > debug information without polluting the logs too much.
>> >
>> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>> > Cc: Rodrigo Vivi <rodrigo.vivi@gmail.com>
>> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>> > ---
>> >  drivers/gpu/drm/i915/i915_drv.h     |  2 +-
>> >  drivers/gpu/drm/i915/i915_params.c  |  6 +++---
>> >  drivers/gpu/drm/i915/intel_uncore.c | 10 ++++++++--
>> >  3 files changed, 12 insertions(+), 6 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> > index 5448ce9d1490..314d8a60d55b 100644
>> > --- a/drivers/gpu/drm/i915/i915_drv.h
>> > +++ b/drivers/gpu/drm/i915/i915_drv.h
>> > @@ -2310,7 +2310,7 @@ struct i915_params {
>> >     bool disable_display;
>> >     bool disable_vtd_wa;
>> >     int use_mmio_flip;
>> > -   bool mmio_debug;
>> > +   int mmio_debug;
>> >  };
>> >  extern struct i915_params i915 __read_mostly;
>> >
>> > diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
>> > index c91cb2033cc5..9de6e8265a14 100644
>> > --- a/drivers/gpu/drm/i915/i915_params.c
>> > +++ b/drivers/gpu/drm/i915/i915_params.c
>> > @@ -169,7 +169,7 @@ module_param_named(use_mmio_flip, i915.use_mmio_flip, int, 0600);
>> >  MODULE_PARM_DESC(use_mmio_flip,
>> >              "use MMIO flips (-1=never, 0=driver discretion [default], 1=always)");
>> >
>> > -module_param_named(mmio_debug, i915.mmio_debug, bool, 0600);
>> > +module_param_named(mmio_debug, i915.mmio_debug, int, 0600);
>> >  MODULE_PARM_DESC(mmio_debug,
>> > -   "Enable the MMIO debug code (default: false). This may negatively "
>> > -   "affect performance.");
>> > +   "Enable the MMIO debug code (default: off). "
>> > +   "This may negatively affect performance.");
>> > diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
>> > index f0230b0e8e11..01ae5aec9e02 100644
>> > --- a/drivers/gpu/drm/i915/intel_uncore.c
>> > +++ b/drivers/gpu/drm/i915/intel_uncore.c
>> > @@ -721,18 +721,24 @@ hsw_unclaimed_reg_debug(struct drm_i915_private *dev_priv, u32 reg, bool read,
>> >             WARN(1, "Unclaimed register detected %s %s register 0x%x\n",
>> >                  when, op, reg);
>> >             __raw_i915_write32(dev_priv, FPGA_DBG, FPGA_DBG_RM_NOCLAIM);
>> > +           i915.mmio_debug--; /* Only report the first N failures */
>> >     }
>> >  }
>> >
>> >  static void
>> >  hsw_unclaimed_reg_detect(struct drm_i915_private *dev_priv)
>> >  {
>> > -   if (i915.mmio_debug)
>> > +   static bool mmio_debug_once = true;
>> > +
>> > +   if (i915.mmio_debug || !i915.mmio_debug_once)
>> >             return;
>> >
>> >     if (__raw_i915_read32(dev_priv, FPGA_DBG) & FPGA_DBG_RM_NOCLAIM) {
>> > -           DRM_ERROR("Unclaimed register detected. Please use the i915.mmio_debug=1 to debug this problem.");
>> > +           DRM_DEBUG("Unclaimed register detected, "
>> > +                     "enabling oneshot unclaimed register reporting. "
>> > +                     "Please use the i915.mmio_debug=N for more information.\n");
>> >             __raw_i915_write32(dev_priv, FPGA_DBG, FPGA_DBG_RM_NOCLAIM);
>> > +           i915.mmio_debug = i915.mmio_debug_once--;
>>
>> I suspect you have uncommitted changes in your work tree.
>
> Or that I only use this tree to write gedankenexperiment against -nightly...

As long as there's still a way to disable the one-shot thing (either
by passing i915.mmio_debug=1000 or by i915.mmio_debug_once=0), this
idea/patch and the other WARN_ONCE patch are: Acked-by: Paulo Zanoni
<paulo.r.zanoni@intel.com>. I see there's still some discussion, so
I'm not sure which one will be the final patch...

Most of the cases I investigated didn't cause enough WARNs to make
dmesg useless, so I still see utility on having a way to print more
than the first error. Bonus points if the default option is set to
something like "print only the first 20 errors" :)

> -Chris
>
> --
> Chris Wilson, Intel Open Source Technology Centre
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/i915: Disable the mmio.debug WARN after it
  2014-11-24  8:12 [PATCH] drm/i915: Disable the mmio.debug WARN after it fires Chris Wilson
  2014-11-24  9:45 ` Daniel Vetter
@ 2014-11-24 16:28 ` shuang.he
  1 sibling, 0 replies; 8+ messages in thread
From: shuang.he @ 2014-11-24 16:28 UTC (permalink / raw)
  To: shuang.he, intel-gfx, chris

Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                                  367/367              367/367
ILK                 -2              375/375              373/375
SNB                                  450/450              450/450
IVB                 -3              503/503              500/503
BYT                                  289/289              289/289
HSW                 -3              567/567              564/567
BDW                                  417/417              417/417
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
ILK  igt_gem_reset_stats_close-pending-fork-render      TIMEOUT(5, M37M26)PASS(1, M26)      TIMEOUT(1, M37)
ILK  igt_kms_flip_vblank-vs-hang      TIMEOUT(5, M37M26)PASS(1, M26)      TIMEOUT(1, M37)
IVB  igt_gem_bad_reloc_negative-reloc      NSPT(6, M34M21M4)PASS(1, M21)      NSPT(1, M21)
IVB  igt_gem_bad_reloc_negative-reloc-lut      NSPT(3, M21M34M4)PASS(7, M21M34M4)      NSPT(1, M21)
IVB  igt_kms_cursor_crc_cursor-128x128-sliding      PASS(2, M21)      DMESG_WARN(1, M21)
HSW  igt_gem_bad_reloc_negative-reloc-lut      NSPT(16, M40M20M19)PASS(1, M20)      NSPT(1, M20)
HSW  igt_kms_rotation_crc_primary-rotation      PASS(17, M20M40M19)      DMESG_WARN(1, M20)
HSW  igt_pm_rc6_residency_rc6-accuracy      PASS(17, M20M40M19)      FAIL(1, M20)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2014-11-24 16:28 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-11-24  8:12 [PATCH] drm/i915: Disable the mmio.debug WARN after it fires Chris Wilson
2014-11-24  9:45 ` Daniel Vetter
2014-11-24 10:06   ` Chris Wilson
2014-11-24 10:24     ` Chris Wilson
2014-11-24 10:52       ` Jani Nikula
2014-11-24 11:07         ` Chris Wilson
2014-11-24 16:23           ` Paulo Zanoni
2014-11-24 16:28 ` [PATCH] drm/i915: Disable the mmio.debug WARN after it shuang.he

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.