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

[Chris Wilson <chris@chris-wilson.co.uk>]

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. 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]
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx