diff mbox

[5/7] drm/i915: WARN on unclaimed registers

Message ID 1359147462-3902-6-git-send-email-przanoni@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Paulo Zanoni Jan. 25, 2013, 8:57 p.m. UTC
From: Paulo Zanoni <paulo.r.zanoni@intel.com>

While debugging these "unclaimed register" problems I concluded that
having a backtrace is way much more useful than having the register
address, since in a lot of cases the register address print on the
message is not the register we're looking for.

We must fix all the "unclaimed register" problems, so if dmesg gets
too polluted it means we're too bugged.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Ben Widawsky Jan. 26, 2013, 1:11 a.m. UTC | #1
On Fri, 25 Jan 2013 18:57:40 -0200
Paulo Zanoni <przanoni@gmail.com> wrote:

> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> While debugging these "unclaimed register" problems I concluded that
> having a backtrace is way much more useful than having the register
> address, since in a lot of cases the register address print on the
> message is not the register we're looking for.
> 
> We must fix all the "unclaimed register" problems, so if dmesg gets
> too polluted it means we're too bugged.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

As I mentioned internally, I'd still prefer
if (WARN_ON(I915_READ_NOTRACE(FPGA_DBG) & FPGA_DBG_RM_NOCLAIM))

because I don't think the message is particularly useful with a
backtrace in hand.

Also on second thought since the internal review, it's probably not
super useful to have the WARN in the UNCLAIMED_REG_CLEAR, because as
you've mentioned before, it's usually from snd-hda or something like
that. So I'd probably get rid of that unless someone else sees a super
cool usage for it.


With or without painting my color:
Reviewed-by: Ben Widawsky <ben@bwidawsk.net>

> ---
>  drivers/gpu/drm/i915/i915_drv.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 422dfc6..bc0eb88 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1227,14 +1227,14 @@ ilk_dummy_write(struct drm_i915_private *dev_priv)
>  #define UNCLAIMED_REG_CLEAR(dev_priv, reg, op) \
>  	if (IS_HASWELL(dev_priv->dev) && \
>  	    (I915_READ_NOTRACE(FPGA_DBG) & FPGA_DBG_RM_NOCLAIM)) { \
> -		DRM_ERROR("Unclaimed register before %x (%c)\n", reg, op); \
> +		WARN(1, "Unclaimed register before %x (%c)\n", reg, op); \
>  		I915_WRITE_NOTRACE(FPGA_DBG, FPGA_DBG_RM_NOCLAIM); \
>  	}
>  
>  #define UNCLAIMED_REG_CHECK(dev_priv, reg, op) \
>  	if (IS_HASWELL(dev_priv->dev) && \
>  	    (I915_READ_NOTRACE(FPGA_DBG) & FPGA_DBG_RM_NOCLAIM)) { \
> -		DRM_ERROR("Unclaimed register %x (%c)\n", reg, op); \
> +		WARN(1, "Unclaimed register %x (%c)\n", reg, op); \
>  		I915_WRITE_NOTRACE(FPGA_DBG, FPGA_DBG_RM_NOCLAIM); \
>  	}
>
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 422dfc6..bc0eb88 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1227,14 +1227,14 @@  ilk_dummy_write(struct drm_i915_private *dev_priv)
 #define UNCLAIMED_REG_CLEAR(dev_priv, reg, op) \
 	if (IS_HASWELL(dev_priv->dev) && \
 	    (I915_READ_NOTRACE(FPGA_DBG) & FPGA_DBG_RM_NOCLAIM)) { \
-		DRM_ERROR("Unclaimed register before %x (%c)\n", reg, op); \
+		WARN(1, "Unclaimed register before %x (%c)\n", reg, op); \
 		I915_WRITE_NOTRACE(FPGA_DBG, FPGA_DBG_RM_NOCLAIM); \
 	}
 
 #define UNCLAIMED_REG_CHECK(dev_priv, reg, op) \
 	if (IS_HASWELL(dev_priv->dev) && \
 	    (I915_READ_NOTRACE(FPGA_DBG) & FPGA_DBG_RM_NOCLAIM)) { \
-		DRM_ERROR("Unclaimed register %x (%c)\n", reg, op); \
+		WARN(1, "Unclaimed register %x (%c)\n", reg, op); \
 		I915_WRITE_NOTRACE(FPGA_DBG, FPGA_DBG_RM_NOCLAIM); \
 	}