[Intel-gfx] [PATCH V2] drm/i915/gt: Add workaround 14016712196

[Upadhyay, Tejas]

> -----Original Message-----
> From: Andi Shyti <andi.shyti at linux.intel.com>
> Sent: Wednesday, May 31, 2023 5:45 PM
> To: Hajda, Andrzej <andrzej.hajda at intel.com>
> Cc: Upadhyay, Tejas <tejas.upadhyay at intel.com>; intel-
> gfx at lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH V2] drm/i915/gt: Add workaround
> 14016712196
> 
> Hi Andrzej,
> 
> > > @@ -218,6 +242,16 @@ int gen12_emit_flush_rcs(struct i915_request
> *rq, u32 mode)
> > >   		u32 flags = 0;
> > >   		u32 *cs, count;
> > > +		/* Wa_14016712196 */
> > > +		if (IS_MTL_GRAPHICS_STEP(engine->i915, M, STEP_A0,
> STEP_B0) ||
> > > +		    IS_MTL_GRAPHICS_STEP(engine->i915, P, STEP_A0,
> STEP_B0)) {
> > > +			/* dummy PIPE_CONTROL + depth flush */
> > > +			cs = intel_ring_begin(rq, 6);
> > > +			err = mtl_dummy_pipe_control(rq, cs);
> > > +			if (err)
> > > +				return err;
> > > +		}
> > > +
> >
> > In case gen12_emit_flush_rcs is called with EMIT_BARRIER =
> > (EMIT_INVALIDATE
> > | EMIT_FLUSH) dummy pipe will be inserted twice, is it OK?
> 
> good point... I also meant to ask this, indeed. I assumed that before the pipe
> control we needed the dummy pipe. Is that the case, Tejas?

Yes before any DEPTH flush + pipe control we need dummy pipe control.

Thanks,
Tejas
> 
> > >   		flags |= PIPE_CONTROL_COMMAND_CACHE_INVALIDATE;
> > >   		flags |= PIPE_CONTROL_TLB_INVALIDATE;
> > >   		flags |= PIPE_CONTROL_INSTRUCTION_CACHE_INVALIDATE;
> > > @@ -733,6 +767,13 @@ u32 *gen12_emit_fini_breadcrumb_rcs(struct
> i915_request *rq, u32 *cs)
> > >   		     PIPE_CONTROL_DC_FLUSH_ENABLE |
> > >   		     PIPE_CONTROL_FLUSH_ENABLE);
> > > +	/* Wa_14016712196 */
> > > +	if (IS_MTL_GRAPHICS_STEP(i915, M, STEP_A0, STEP_B0) ||
> > > +	    IS_MTL_GRAPHICS_STEP(i915, P, STEP_A0, STEP_B0))
> > > +		/* dummy PIPE_CONTROL + depth flush */
> > > +		cs = gen12_emit_pipe_control(cs, 0,
> > > +
> PIPE_CONTROL_DEPTH_CACHE_FLUSH, 0);
> >
> > How do we know there is enough space for dummy pipe?
> 
> This should be ensured by intel_ring_begin().
> 
> Andi
> 
> > Regards
> > Andrzej
> >
> > > +
> > >   	if (GRAPHICS_VER(i915) == 12 && GRAPHICS_VER_FULL(i915) <
> IP_VER(12, 50))
> > >   		/* Wa_1409600907 */
> > >   		flags |= PIPE_CONTROL_DEPTH_STALL;