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.zan...@intel.com>
> Date: Wed Jul 16 17:49:29 2014 -0300
>
> drm/i915: reorganize the unclaimed register detection code
>
> Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
> Cc: Rodrigo Vivi <rodrigo.v...@gmail.com>
> Cc: Paulo Zanoni <paulo.r.zan...@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
