Re: [PATCH 7/7] drm/i915/chv: Add non claimed mmio checking

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

On Tue, Dec 15, 2015 at 04:44:25PM +0200, Ville Syrjälä wrote:
> On Tue, Dec 15, 2015 at 04:25:12PM +0200, Mika Kuoppala wrote:
> > Imre mentioned that chv might also have capability to
> > track unclaimed mmio accesses. And it has, so take it
> > into use.
> 
> VLV too.
> 
> My old patch:
> http://lists.freedesktop.org/archives/intel-gfx/2013-May/027599.html
> 
> > 
> > Cc: Imre Deak <imre.deak@intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h     |  4 ++++
> >  drivers/gpu/drm/i915/intel_uncore.c | 34 ++++++++++++++++++++++++++++++----
> >  2 files changed, 34 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 007ae83..24686ab 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -1711,6 +1711,10 @@ enum skl_disp_power_wells {
> >  #define FPGA_DBG		_MMIO(0x42300)
> >  #define   FPGA_DBG_RM_NOCLAIM	(1<<31)
> >  
> > +#define CLAIM_ER		_MMIO(0x182028)
> > +#define   CLAIM_ER_CLR		(1<<31)
> 
> Looks like you forgot the overflow bit.
> 
> > +#define   CLAIM_ER_CTR_MASK	(0xffff)
> 
> Needless parens.
> 
> > +
> >  #define DERRMR		_MMIO(0x44050)
> >  /* Note that HBLANK events are reserved on bdw+ */
> >  #define   DERRMR_PIPEA_SCANLINE		(1<<0)
> > diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> > index 02bad32..ea8fcd4 100644
> > --- a/drivers/gpu/drm/i915/intel_uncore.c
> > +++ b/drivers/gpu/drm/i915/intel_uncore.c
> > @@ -335,13 +335,10 @@ static void intel_uncore_ellc_detect(struct drm_device *dev)
> >  }
> >  
> >  static bool
> > -check_for_unclaimed_mmio(struct drm_i915_private *dev_priv)
> > +fpga_check_for_unclaimed_mmio(struct drm_i915_private *dev_priv)
> >  {
> >  	u32 dbg;
> >  
> > -	if (!HAS_FPGA_DBG_UNCLAIMED(dev_priv))
> > -		return false;
> > -
> >  	dbg = __raw_i915_read32(dev_priv, FPGA_DBG);
> >  	if (likely(!(dbg & FPGA_DBG_RM_NOCLAIM)))
> >  		return false;
> > @@ -354,6 +351,35 @@ check_for_unclaimed_mmio(struct drm_i915_private *dev_priv)
> >  	return true;
> >  }
> >  
> > +static bool
> > +chv_check_for_unclaimed_mmio(struct drm_i915_private *dev_priv)
> > +{
> > +	u32 cer;
> > +
> > +	cer = __raw_i915_read32(dev_priv, CLAIM_ER);
> > +	if (likely(!(cer & CLAIM_ER_CTR_MASK)))
> > +		return false;
> > +
> > +	__raw_i915_write32(dev_priv, CLAIM_ER, CLAIM_ER_CLR);
> > +
> > +	/* We want to clear it asap, so post */
> > +	__raw_i915_read32(dev_priv, CLAIM_ER);
> 
> __raw_posting_read()
> 
> But not sure why we'd want to do this read really. Seems like pointless
> overhead.
> 
> The other thing is that this only detect unclaimed RM cycles, so display
> registers only. Might be nice to only do the checks when accessing
> display registers so that we don't add overhead to the GT stuff.
> I think the same holds for HSW+ actually.

Oh, after reading some more it doesn't look like you're adding the
checks to the register accesses for chv. So no overhead there.

I think one natural place for some explicit unclaimer reg checks would
be the disp2d power well enable/disable, since a disabled power well
might be the cause for the error. IIRC another could be simply a bogus
register offset.

> 
> > +
> > +	return true;
> > +}
> > +
> > +static bool
> > +check_for_unclaimed_mmio(struct drm_i915_private *dev_priv)
> > +{
> > +	if (HAS_FPGA_DBG_UNCLAIMED(dev_priv))
> > +		return fpga_check_for_unclaimed_mmio(dev_priv);
> > +
> > +	if (IS_CHERRYVIEW(dev_priv))
> > +		return chv_check_for_unclaimed_mmio(dev_priv);
> > +
> > +	return false;
> > +}
> > +
> >  static void __intel_uncore_early_sanitize(struct drm_device *dev,
> >  					  bool restore_forcewake)
> >  {
> > -- 
> > 2.5.0
> 
> -- 
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> 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