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

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

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@chris-wilson.co.uk>
> Cc: Ville Syrjälä <ville.syrjala@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.

> +
>  	I915_WRITE_CTL(ring,
>  			((ringbuf->size - PAGE_SIZE) & RING_NR_PAGES)
>  			| RING_VALID);
> -- 
> 2.1.0.rc1

-- 
Ville Syrjälä
Intel OTC