[Intel-gfx] [PATCH 1/2] drm/i915: Move the GTFIFODBG to the common mmio dbg framework

[Ville Syrjälä]

On Thu, Apr 06, 2017 at 05:05:10PM +0100, Chris Wilson wrote:
> On Thu, Apr 06, 2017 at 06:46:29PM +0300, Ville Syrjälä wrote:
> > On Thu, Apr 06, 2017 at 06:39:42PM +0300, Mika Kuoppala wrote:
> > > +static bool
> > >  check_for_unclaimed_mmio(struct drm_i915_private *dev_priv)
> > >  {
> > > +	bool ret = false;
> > > +
> > >  	if (HAS_FPGA_DBG_UNCLAIMED(dev_priv))
> > > -		return fpga_check_for_unclaimed_mmio(dev_priv);
> > > +		ret |= fpga_check_for_unclaimed_mmio(dev_priv);
> > >  
> > >  	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> > > -		return vlv_check_for_unclaimed_mmio(dev_priv);
> > > +		ret |= vlv_check_for_unclaimed_mmio(dev_priv);
> > >  
> > > -	return false;
> > > +	if (IS_GEN6(dev_priv) || IS_GEN7(dev_priv))
> > > +		ret |= gen6_check_for_fifo_debug(dev_priv);
> > 
> > I'd prefer to keep unclaim vs. wake FIFO separate because the
> > unclaimned stuff is only for display registers. In my plan to
> > split the uncore lock into gt and display locks the unclaimed
> > reg stuff would end up being protected by the display lock rather
> > than the gt lock.
> 
> I don't think it is much of a hindrance, right? We just split it out when
> splitting dpy vs gt.

I suppose. Although if we want to do the FIFO checks for non-GT stuff as
well, then I guess we'd have to risk hitting the FIFO register from
both the display and gt code paths. Not sure if that's safe or not.

> Mika was digging through GTFIFODBG and thought it
> had some bits for a sideband underflow...
> 
> Random thought, would i915->mmio.writeX[reg < 0x40000](i915, reg, val)
> or just push all the decisions to the backend?  I hope gcc would be able
> to automatically store the function pointer i915->mmio.writeX[reg < 0x40000]

I haven't done any real performance analysis on this stuff TBH, apart
from looking at the duration of the atomic commits. I guess separate
function pointers might be nice for code readability at least.

-- 
Ville Syrjälä
Intel OTC