[Intel-gfx] [PATCH v2] drm/i915: Insert a command barrier on BLT/BSD cache flushes

[Chris Wilson]

On Mon, Feb 09, 2015 at 06:00:25PM +0200, Jani Nikula wrote:
> On Thu, 22 Jan 2015, Chris Wilson <chris at chris-wilson.co.uk> wrote:
> > This looked like an odd regression from
> >
> > commit ec5cc0f9b019af95e4571a9fa162d94294c8d90b
> > Author: Chris Wilson <chris at chris-wilson.co.uk>
> > Date:   Thu Jun 12 10:28:55 2014 +0100
> >
> >     drm/i915: Restrict GPU boost to the RCS engine
> >
> > but in reality it undercovered a much older coherency bug. The issue that
> > boosting the GPU frequency on the BCS ring was masking was that we could
> > wake the CPU up after completion of a BCS batch and inspect memory prior
> > to the write cache being fully evicted. In order to serialise the
> > breadcrumb interrupt (and so ensure that the CPU's view of memory is
> > coherent) we need to perform a post-sync operation in the MI_FLUSH_DW.
> >
> > v2: Fix all the MI_FLUSH_DW (bsd plus the duplication in execlists).
> >
> > Testcase: gpuX-rcs-gpu-read-after-write
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: stable at vger.kernel.org
> > Acked-by: Daniel Vetter <daniel at ffwll.ch>
> > ---
> >  drivers/gpu/drm/i915/intel_lrc.c        | 20 +++++++++++---------
> >  drivers/gpu/drm/i915/intel_ringbuffer.c | 23 +++++++++++++++++++----
> >  2 files changed, 30 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> > index e405b61cdac5..8e71d8851c9a 100644
> > --- a/drivers/gpu/drm/i915/intel_lrc.c
> > +++ b/drivers/gpu/drm/i915/intel_lrc.c
> > @@ -1237,15 +1237,17 @@ static int gen8_emit_flush(struct intel_ringbuffer *ringbuf,
> >  
> >  	cmd = MI_FLUSH_DW + 1;
> >  
> > -	if (ring == &dev_priv->ring[VCS]) {
> > -		if (invalidate_domains & I915_GEM_GPU_DOMAINS)
> > -			cmd |= MI_INVALIDATE_TLB | MI_INVALIDATE_BSD |
> > -				MI_FLUSH_DW_STORE_INDEX |
> > -				MI_FLUSH_DW_OP_STOREDW;
> > -	} else {
> > -		if (invalidate_domains & I915_GEM_DOMAIN_RENDER)
> > -			cmd |= MI_INVALIDATE_TLB | MI_FLUSH_DW_STORE_INDEX |
> > -				MI_FLUSH_DW_OP_STOREDW;
> > +	/* We always require a command barrier so that subsequent
> > +	 * commands, such as breadcrumb interrupts, are strictly ordered
> > +	 * wrt the contents of the write cache being flushed to memory
> > +	 * (and thus being coherent from the CPU).
> > +	 */
> > +	cmd |= MI_FLUSH_DW_STORE_INDEX | MI_FLUSH_DW_OP_STOREDW;
> > +
> > +	if (invalidate_domains & I915_GEM_GPU_DOMAINS) {
> 
> Why do you change the mask from I915_GEM_DOMAIN_RENDER to
> I915_GEM_GPU_DOMAINS for ring != VCS?

My bad, I didn't notice that execlists was originally broken. The patch
is correct.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre