Re: [PATCH 23/30] drm/i915: Cache GT fifo count for SandyBridge

[Ben Widawsky <ben@bwidawsk.net>]

On Wed, Apr 13, 2011 at 07:21:30PM -0700, Ben Widawsky wrote:
> On Tue, Apr 12, 2011 at 09:31:51PM +0100, Chris Wilson wrote:
> > The read back of the available FIFO entries is vital for system
> > stability, but extremely costly. However, we only need a guide so as to
> > avoid eating into the reserved entries and since we are the only
> > consumer we can cache the read of the count from the last write.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.c |   14 +++++++++-----
> >  drivers/gpu/drm/i915/i915_drv.h |    1 +
> >  2 files changed, 10 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > index c416c1d..1146abd 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -287,12 +287,16 @@ void __gen6_gt_force_wake_put(struct drm_i915_private *dev_priv)
> >  
> >  void __gen6_gt_wait_for_fifo(struct drm_i915_private *dev_priv)
> >  {
> > -	int loop = 500;
> > -	u32 fifo = I915_READ_NOTRACE(GT_FIFO_FREE_ENTRIES);
> > -	while (fifo < 20 && loop--) {
> > -		udelay(10);
> > -		fifo = I915_READ_NOTRACE(GT_FIFO_FREE_ENTRIES);
> > +	if (dev_priv->gt_fifo_count < 20 ) {
> > +		int loop = 500;
> > +		u32 fifo = I915_READ_NOTRACE(GT_FIFO_FREE_ENTRIES);
> > +		while (fifo < 20 && loop--) {
> > +			udelay(10);
> > +			fifo = I915_READ_NOTRACE(GT_FIFO_FREE_ENTRIES);
> > +		}
> > +		dev_priv->gt_fifo_count = fifo;
> >  	}
> > +	dev_priv->gt_fifo_count--;
> >  }
> >  
> >  static int i915_drm_freeze(struct drm_device *dev)
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 2f45228..c837e10 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -268,6 +268,7 @@ typedef struct drm_i915_private {
> >  	int relative_constants_mode;
> >  
> >  	void __iomem *regs;
> > +	u32 gt_fifo_count;
> >  
> >  	struct intel_gmbus {
> >  		struct i2c_adapter adapter;
> 
> I'm sure you noticed that we have seriously problem both here and in the
> put()/get() if the condition doesn't clear up in loop number of times.
> 
> I'd probably add a WARN(!loop, "uh oh"), but the patch is better than
> what is there currently, so I'm okay either way.

Post-decrement, and check fifo, so...
(loop < 0 && !fifo)

> 
> Reviewed-by: Ben Widawsky <ben@bwidawsk.net>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx