Re: [PATCH 2/4] drm/i915: restrict unclaimed register checking

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Fri, Sep 04, 2015 at 01:38:07PM +0000, Zanoni, Paulo R wrote:
> Em Sex, 2015-09-04 às 09:53 +0300, Mika Kuoppala escreveu:
> > Paulo Zanoni <paulo.r.zanoni@intel.com> writes:
> > 
> > > The unclaimed register bit is only triggered when someone touches 
> > > the
> > > specified register range.
> > > 
> > 
> > I got the impression that we get the unclaimed access also
> > for other ranges, if they are powered down during access?
> 
> The range is a conclusion after a discussion I had with Arthur (CCd) a
> few months ago. Unfortunately I can't find the emails in my archive
> here, so maybe he can comment.
> 
> But basically what I did was: put the machine in runtime suspend, open
> debugfs/i915_forcewake_user (because if we don't grab forcewake we will
> freeze the machine when reading some regs), then run this: 
> http://people.freedesktop.org/~pzanoni/fpga_dbg_range.c .

The "RM" in the name already suggest to me that it's just related to
display regiters.

That's also how the VLV/CHV unclaimed register detection more or less
works. IIRC I did a similar experiment on VLV/CHV and found a few
different ranges where it triggers, but I don't think I did it quite
as thoroughly as your test application. I suppose I should do that
and then dig up my old VLV/CHV patch and rebase it on top of whatever
is the new way of doing things.

> 
> > 
> > -Mika
> > 
> > > For the normal use case (with i915.mmio_debug=0), this commit will
> > > avoid the extra __raw_i915_read32() call for every register outside
> > > the specified range, at the expense of a few additional "if"
> > > statements.
> > > 
> > > v2: Put the register range checking earlier (Chris).
> > > 
> > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_uncore.c | 16 ++++++++++++----
> > >  1 file changed, 12 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_uncore.c 
> > > b/drivers/gpu/drm/i915/intel_uncore.c
> > > index 10c61a6..65e0ea8 100644
> > > --- a/drivers/gpu/drm/i915/intel_uncore.c
> > > +++ b/drivers/gpu/drm/i915/intel_uncore.c
> > > @@ -595,6 +595,8 @@ void assert_forcewakes_inactive(struct 
> > > drm_i915_private *dev_priv)
> > >  	 !FORCEWAKE_GEN9_MEDIA_RANGE_OFFSET(reg) && \
> > >  	 !FORCEWAKE_GEN9_COMMON_RANGE_OFFSET(reg))
> > >  
> > > +#define UNCLAIMED_CHECK_RANGE(reg) REG_RANGE(reg, 0x40000, 
> > > 0xC0000)
> > > +
> > >  static void
> > >  ilk_dummy_write(struct drm_i915_private *dev_priv)
> > >  {
> > > @@ -611,6 +613,9 @@ 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 (!UNCLAIMED_CHECK_RANGE(reg))
> > > +		return;
> > > +
> > >  	if (!i915.mmio_debug)
> > >  		return;
> > >  
> > > @@ -623,10 +628,13 @@ 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)
> > > +hsw_unclaimed_reg_detect(struct drm_i915_private *dev_priv, u32 
> > > reg)
> > >  {
> > >  	static bool mmio_debug_once = true;
> > >  
> > > +	if (!UNCLAIMED_CHECK_RANGE(reg))
> > > +		return;
> > > +
> > >  	if (i915.mmio_debug || !mmio_debug_once)
> > >  		return;
> > >  
> > > @@ -892,7 +900,7 @@ hsw_write##x(struct drm_i915_private *dev_priv, 
> > > off_t reg, u##x val, bool trace)
> > >  		gen6_gt_check_fifodbg(dev_priv); \
> > >  	} \
> > >  	hsw_unclaimed_reg_debug(dev_priv, reg, false, false); \
> > > -	hsw_unclaimed_reg_detect(dev_priv); \
> > > +	hsw_unclaimed_reg_detect(dev_priv, reg); \
> > >  	GEN6_WRITE_FOOTER; \
> > >  }
> > >  
> > > @@ -934,7 +942,7 @@ gen8_write##x(struct drm_i915_private 
> > > *dev_priv, off_t reg, u##x val, bool trace
> > >  		__force_wake_get(dev_priv, FORCEWAKE_RENDER); \
> > >  	__raw_i915_write##x(dev_priv, reg, val); \
> > >  	hsw_unclaimed_reg_debug(dev_priv, reg, false, false); \
> > > -	hsw_unclaimed_reg_detect(dev_priv); \
> > > +	hsw_unclaimed_reg_detect(dev_priv, reg); \
> > >  	GEN6_WRITE_FOOTER; \
> > >  }
> > >  
> > > @@ -1000,7 +1008,7 @@ gen9_write##x(struct drm_i915_private 
> > > *dev_priv, off_t reg, u##x val, \
> > >  		__force_wake_get(dev_priv, fw_engine); \
> > >  	__raw_i915_write##x(dev_priv, reg, val); \
> > >  	hsw_unclaimed_reg_debug(dev_priv, reg, false, false); \
> > > -	hsw_unclaimed_reg_detect(dev_priv); \
> > > +	hsw_unclaimed_reg_detect(dev_priv, reg); \
> > >  	GEN6_WRITE_FOOTER; \
> > >  }
> > >  
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx