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.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html