Re: [PATCH] drm/i915: Apply post-sync write for pipe control invalidates

[Ben Widawsky <ben@bwidawsk.net>]

On Fri, 10 Aug 2012 10:18:10 +0100
Chris Wilson <chris@chris-wilson.co.uk> wrote:

> When invalidating the TLBs it is documentated as requiring a post-sync
> write. Failure to do so seems to result in a GPU hang.
> 
> Exposure to this hang on IVB seems to be a result of removing the
> extra stalls required for SNB pipecontrol workarounds:
> 
> commit 6c6cf5aa9c583478b19e23149feaa92d01fb8c2d
> Author: Chris Wilson <chris@chris-wilson.co.uk>
> Date:   Fri Jul 20 18:02:28 2012 +0100
> 
>     drm/i915: Only apply the SNB pipe control w/a to gen6
> 
> Reported-by: yex.tian@intel.com
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=53322
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

This is the moral equivalent of my patch to make the simulator happy
on IVB. Daniel, I'll settle for either patch.
Therefore,
Acked-by: Ben Widawsky <ben@bwidawsk.net>

> ---
>  drivers/gpu/drm/i915/intel_ringbuffer.c |   35
> ++++++++++++++++++------------- 1 file changed, 21 insertions(+), 14
> deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
> b/drivers/gpu/drm/i915/intel_ringbuffer.c index 13318a0..7608bc2
> 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -213,20 +213,27 @@ gen6_render_ring_flush(struct intel_ring_buffer
> *ring,
>  	 * number of bits based on the write domains has little
> performance
>  	 * impact.
>  	 */
> -	flags |= PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH;
> -	flags |= PIPE_CONTROL_TLB_INVALIDATE;
> -	flags |= PIPE_CONTROL_INSTRUCTION_CACHE_INVALIDATE;
> -	flags |= PIPE_CONTROL_TEXTURE_CACHE_INVALIDATE;
> -	flags |= PIPE_CONTROL_DEPTH_CACHE_FLUSH;
> -	flags |= PIPE_CONTROL_VF_CACHE_INVALIDATE;
> -	flags |= PIPE_CONTROL_CONST_CACHE_INVALIDATE;
> -	flags |= PIPE_CONTROL_STATE_CACHE_INVALIDATE;
> -	/*
> -	 * Ensure that any following seqno writes only happen when
> the render
> -	 * cache is indeed flushed (but only if the caller actually
> wants that).
> -	 */
> -	if (flush_domains)
> +	if (flush_domains) {
> +		flags |= PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH;
> +		flags |= PIPE_CONTROL_DEPTH_CACHE_FLUSH;
> +		/*
> +		 * Ensure that any following seqno writes only happen
> +		 * when the render cache is indeed flushed.
> +		 */
>  		flags |= PIPE_CONTROL_CS_STALL;
> +	}
> +	if (invalidate_domains) {
> +		flags |= PIPE_CONTROL_TLB_INVALIDATE;
> +		flags |= PIPE_CONTROL_INSTRUCTION_CACHE_INVALIDATE;
> +		flags |= PIPE_CONTROL_TEXTURE_CACHE_INVALIDATE;
> +		flags |= PIPE_CONTROL_VF_CACHE_INVALIDATE;
> +		flags |= PIPE_CONTROL_CONST_CACHE_INVALIDATE;
> +		flags |= PIPE_CONTROL_STATE_CACHE_INVALIDATE;
> +		/*
> +		 * TLB invalidate requires a post-sync write.
> +		 */
> +		flags |= PIPE_CONTROL_QW_WRITE;
> +	}
>  
>  	ret = intel_ring_begin(ring, 4);
>  	if (ret)
> @@ -234,7 +241,7 @@ gen6_render_ring_flush(struct intel_ring_buffer
> *ring, 
>  	intel_ring_emit(ring, GFX_OP_PIPE_CONTROL(4));
>  	intel_ring_emit(ring, flags);
> -	intel_ring_emit(ring, 0);
> +	intel_ring_emit(ring, (u32)ring->status_page.gfx_addr+2048);
>  	intel_ring_emit(ring, 0);
>  	intel_ring_advance(ring);
>  



-- 
Ben Widawsky, Intel Open Source Technology Center