[Intel-gfx] [PATCH 2/4] drm/i915: Fix serialisation of pipecontrol write vs semaphore signal

[Ville Syrjälä]

On Thu, Apr 14, 2016 at 05:29:31PM +0300, Ville Syrjälä wrote:
> On Mon, Apr 11, 2016 at 10:06:28AM +0100, Chris Wilson wrote:
> > On Mon, Apr 11, 2016 at 11:34:54AM +0300, Ville Syrjälä wrote:
> > > On Sat, Apr 09, 2016 at 11:14:44AM +0100, Chris Wilson wrote:
> > > > In order for the MI_SEMAPHORE_SIGNAL command to wait until after the
> > > > pipecontrol writing the signal value is complete, we have to pause the
> > > > CS inside the PIPE_CONTROL with the CS_STALL bit.
> > > > 
> > > > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_ringbuffer.c | 3 +--
> > > >  1 file changed, 1 insertion(+), 2 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > > > index 556924ee47f9..62d09cf2ea8f 100644
> > > > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> > > > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > > > @@ -1301,7 +1301,7 @@ static int gen8_rcs_signal(struct drm_i915_gem_request *signaller_req,
> > > >  		intel_ring_emit(signaller, GFX_OP_PIPE_CONTROL(6));
> > > >  		intel_ring_emit(signaller, PIPE_CONTROL_GLOBAL_GTT_IVB |
> > > >  					   PIPE_CONTROL_QW_WRITE |
> > > > -					   PIPE_CONTROL_FLUSH_ENABLE);
> > > > +					   PIPE_CONTROL_CS_STALL);
> > > 
> > > Doesn't this just stall when parsing the pipe control? Shouldn't
> > > we intead make sure the post-sync write is issued before the semaphore
> > > is signalled? (pipe_control /w post-sync write + second pipe control w/
> > > flush enable?)
> > 
> > No, afaik and can determine experimentally. The stall is after the
> > post-sync write. The pipe-control dosn't emit the write until it has
> > done the flush/invalidate and will not complete until the write is
> > commited (in theory, until it is coherent). The CS stall prevents the
> > command parser advancing until the pipecontrol is finished.
> 
> OK, so I did a quick experiemnt here doing something like:
> 
> for i=0-128:
> 	pipe_control w/ qw write data=0xdead0000+i offset=(i%8)*8
> for i=0-8:
> 	pipe_control w/ qw write data=i offset=(i%8)*8
> 	mi_lrm reg=0x2310+(i%8)*4 offset=(i%8)*8
> 
> and then check the regs afterwards.
> 
> If I use CS stall in the second pipe control, it does seem to wait for
> the post-sync operation initiated by itself. So seems you are right
> about that. So the register values I saw were:
>  (0x00002310): 0x00000001
>  (0x00002314): 0x00000002
>  (0x00002318): 0x00000003
>  (0x0000231c): 0x00000004
>  (0x00002320): 0x00000005
>  (0x00002324): 0x00000006
>  (0x00002328): 0x00000007
>  (0x0000232c): 0x00000008

Oh and based on that I guess I can say
Reviewed-by: Ville Syrjälä <ville.syrjala at linux.intel.com>

> 
> I also noticed something about the "pipe control flush" (bit 7). It
> only really seems to wait for already initiated post-sync operations.
> If the pipe control that is going to initiate the post-sync op itself
> is still in the pipeline, bit 7 won't actually wait for it. Or at
> least that's how I would interpret these results:
> 
> bit 7 == 0:
>  (0x00002310): 0xdead0058
>  (0x00002314): 0xdead0059
>  (0x00002318): 0xdead0062
>  (0x0000231c): 0xdead0063
>  (0x00002320): 0xdead006c
>  (0x00002324): 0xdead006d
>  (0x00002328): 0xdead006e
>  (0x0000232c): 0xdead0077
> 
> bit 7 == 1:
>  (0x00002310): 0xdead0078
>  (0x00002314): 0xdead0079
>  (0x00002318): 0xdead007a
>  (0x0000231c): 0xdead007b
>  (0x00002320): 0xdead007c
>  (0x00002324): 0xdead007d
>  (0x00002328): 0xdead007e
>  (0x0000232c): 0xdead007f
> 
> So even with bit 7 == 1 I didn't see the value the previous pipe control
> would write.
> 
> So not sure what good that bit really is then since you always need a
> CS stall to make sure that previous pipe controls have reached the end
> of the pipe, and CS stall anyway seems to wait for the post-sync
> operations, so adding bit 7 to the mix seems redundant.
> 
> -- 
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC