On Mon, 09 Feb 2015, Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> wrote: > On Mon, Feb 09, 2015 at 06:00:25PM +0200, Jani Nikula wrote: >> On Thu, 22 Jan 2015, Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> wrote: >> > This looked like an odd regression from >> > >> > commit ec5cc0f9b019af95e4571a9fa162d94294c8d90b >> > Author: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> >> > 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@xxxxxxxxxxxxxxxxxx> >> > Cc: stable@xxxxxxxxxxxxxxx >> > Acked-by: Daniel Vetter <daniel@xxxxxxxx> >> > --- >> > 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. I'll take your and Daniel's word for it. I hope I won't have to regret not asking you to split this into two patches... Pushed to drm-intel-next-fixes, thanks for the patch and ack. BR, Jani. > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx