[Intel-gfx] [PATCH 1/2] drm/i915: reorganize the unclaimed register detection code

[Chris Wilson]

On Tue, Aug 26, 2014 at 09:17:11AM -0300, Paulo Zanoni wrote:
> 2014-08-26 7:22 GMT-03:00 Chris Wilson <chris at chris-wilson.co.uk>:
> > On Wed, Jul 16, 2014 at 05:49:29PM -0300, Paulo Zanoni wrote:
> >>  static void
> >> -hsw_unclaimed_reg_check(struct drm_i915_private *dev_priv, u32 reg)
> >> +hsw_unclaimed_reg_detect(struct drm_i915_private *dev_priv)
> >>  {
> >> +     if (i915.mmio_debug)
> >> +             return;
> >> +
> >>       if (__raw_i915_read32(dev_priv, FPGA_DBG) & FPGA_DBG_RM_NOCLAIM) {
> >> -             DRM_ERROR("Unclaimed write to %x\n", reg);
> >> +             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);
> >>       }
> >
> > What was the point here? You still add an extra read to every register
> > write
> 
> Well, we previously had 2 extra reads instead of 1, so with this patch
> we're in a better position :)
> 
> 
> > and then repeat the request to enable mmio_debug ad infinitum.
> 
> Yeah, this could be avoided. OTOH, on most cases it's not gonna happen
> frequently enough to annoy the user.

How do you think I came across this?

> > And
> > you still check for illegal writes in the irq handler.
> 
> That just happens on HSW, not BDW+.

Even on hsw, it should be killed. Checking inside the irq handler is
just insane. Just move it to one of the periodic checks since we can't
get any more information than an error occurred, and ask to be re-run
with mmio_debug (and then shut up) - heck you could even automatically
enable it for a one-shot operation.

> >
> > Just kill this code.
> 
> If we do it, we won't be checking for unclaimed registers on BDW+
> without i915.mmio_debug=1.

Then do as above.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre