Re: [Intel-gfx] [PATCH 2/3] drm/i915: Force the CS stall for invalidate flushes

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

On Fri, Dec 12, 2014 at 09:20:49AM +0000, Chris Wilson wrote:
> On Fri, Dec 12, 2014 at 11:09:15AM +0200, Ville Syrjälä wrote:
> > On Thu, Dec 11, 2014 at 08:17:00AM +0000, Chris Wilson wrote:
> > > In order to act as a full command barrier by itself, we need to tell the
> > > pipecontrol to actually stall the command streamer while the flush runs.
> > > We require the full command barrier before operations like
> > > MI_SET_CONTEXT, which currently rely on a prior invalidate flush.
> > > 
> > > References: https://bugs.freedesktop.org/show_bug.cgi?id=83677
> > > Cc: Simon Farnsworth <simon@farnz.org.uk>
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > Cc: stable@vger.kernel.org
> > > ---
> > >  drivers/gpu/drm/i915/intel_ringbuffer.c | 2 ++
> > >  1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > > index 282279b83ca4..02fb478a2867 100644
> > > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> > > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > > @@ -380,6 +380,8 @@ gen7_render_ring_flush(struct intel_engine_cs *ring,
> > >  		flags |= PIPE_CONTROL_QW_WRITE;
> > >  		flags |= PIPE_CONTROL_GLOBAL_GTT_IVB;
> > >  
> > > +		flags |= PIPE_CONTROL_STALL_AT_SCOREBOARD;
> > > +
> > 
> > Hmm. BSpec says that the render cache won't be flushed when this bit
> > is set. Is that going to cause problems for the gpu_cache_dirty cases
> > where seem to do invalidate+flush with a single PIPE_CONTROL?
> 
> I thought it was DEPTH_STALL that disabled the write flush.

Hmm. Yeah, the previous sentence talks about the depth stall bit. So I
suppose it could still be referring to the depth stall bit when it says
the render cache flush won't be flushed.

> It is
> redundant in the case where the write flush is taking place though and
> you can do:
> 
> /* bspec is not entirely clear when the render target cache flush is
>  * disabled with other stall bits set, so don't set any additional
>  * stalls if we are already using the cache flush.
>  */
> if ((flags & PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH) == 0)
>    flags |= PIPE_CONTROL_STALL_AT_SCOREBOARD;
> -Chris
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre

-- 
Ville Syrjälä
Intel OTC