[Intel-gfx] [PATCH] drm/i915: Reset the HEAD pointer for the ring after writing START

[Chris Wilson]

On Thu, Aug 07, 2014 at 05:26:20PM +0300, Ville Syrjälä wrote:
> On Thu, Aug 07, 2014 at 03:11:17PM +0100, Chris Wilson wrote:
> > Ville found an old w/a documented for g4x that suggested that we need to
> > reset the HEAD after writing START. This is a useful fixup for some of
> > the g4x ring initialisation woes, but as usual, not all.
> > 
> > References: https://bugs.freedesktop.org/show_bug.cgi?id=76554
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_ringbuffer.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > index 45e3ec927051..0a37ac0d9c0d 100644
> > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > @@ -537,6 +537,15 @@ static int init_ring_common(struct intel_engine_cs *ring)
> >  	 * also enforces ordering), otherwise the hw might lose the new ring
> >  	 * register values. */
> >  	I915_WRITE_START(ring, i915_gem_obj_ggtt_offset(obj));
> > +
> > +	/* WaClearRingBufHeadRegAtInit:ctg,elk */
> > +	if (I915_READ_HEAD(ring)) {
> > +		DRM_DEBUG("%s initialization failed [head=%08x], fudging\n",
> > +			  ring->name, I915_READ_HEAD(ring));
> > +		I915_WRITE_HEAD(ring, 0);
> > +		(void)I915_READ_HEAD(ring);
> > +	}
> 
> Just a clarification that I don't know if this is the right w/a name.
> There's no description in the database, but it would seem to fit the
> symptoms of HEAD going wild on its own.
> 
> The spec says this:
> 
> RING_START:
> "Writing this register also causes the Head Offset to be reset to zero,
> and the Wrap Count to be reset to zero."
> 
> RING_HEAD:
> "Ring Buffer Head Offsets must be properly programmed before ring is
> enable"
> 
> So my theory is that the RING_START write actually corruptsa RING_HEAD
> instead of clearing it on g4x, and thus we need to rewrite it once more
> after writing RING_START, but before we enable the ring.
> 
> As far as the patch goes, I think I'd just do the write unconditionally
> in case the corruption is somehow magical and doesn't appear until the
> ring is started. After all it can't do any harm as the RING_START write
> is already supposed to clear it. Not sure if there should be some posting
> reads between the steps just for a bit of extra paranoia.

The read there is mostly for debugging (it's nice to know when w/a are
being applied after all), but it also helps with ordering. As a
compromise:

 /* WaClearRingBufHeadRegAtInit:ctg,elk */
if (I95_READ_HEAD())
	DRM_DEBUG();
I915_WRITE_HEAD(ring, 0);
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre