[Intel-gfx] [PATCH v2 1/2] drm/i915: Add WABB blit for Wa_16018031267 / Wa_16018063123

[Cavitt, Jonathan]

-----Original Message-----
From: Andi Shyti <andi.shyti at linux.intel.com> 
Sent: Thursday, August 24, 2023 7:54 AM
To: Cavitt, Jonathan <jonathan.cavitt at intel.com>
Cc: intel-gfx at lists.freedesktop.org; Mistat, Tomasz <tomasz.mistat at intel.com>; Vivi, Rodrigo <rodrigo.vivi at intel.com>; Germano, Gregory F <gregory.f.germano at intel.com>; Roper, Matthew D <matthew.d.roper at intel.com>; Das, Nirmoy <nirmoy.das at intel.com>
Subject: Re: [Intel-gfx] [PATCH v2 1/2] drm/i915: Add WABB blit for Wa_16018031267 / Wa_16018063123
> 
> Hi Jonathan,
> 
> few little things...
> 
> On Wed, Aug 23, 2023 at 11:51:03AM -0700, Jonathan Cavitt wrote:
> > From: Nirmoy Das <nirmoy.das at intel.com>
> > 
> > Apply WABB blit for Wa_16018031267 / Wa_16018063123.
> > Additionally, update the lrc selftest to exercise the new
> > WABB changes.
> > 
> > Signed-off-by: Jonathan Cavitt <jonathan.cavitt at intel.com>
> > Co-developed-by: Nirmoy Das <nirmoy.das at intel.com>
> 
> As the sender of this patch, your SoB should be last and you also
> need to add Nirmoy's SoB above yours.
> 
> (Tags should be added in chronological order)
> 
> [...]
> 
> > +static u32 *
> > +xehp_emit_per_ctx_bb(const struct intel_context *ce, u32 *cs)
> > +{
> > +	/* Wa_16018031267, Wa_16018063123 */
> > +	if (ce->engine->class == COPY_ENGINE_CLASS &&
> > +	    NEEDS_FASTCOLOR_BLT_WABB(ce->engine->i915))
> > +		cs = xehp_emit_fastcolor_blt_wabb(ce, cs);
> 
> I thought the trend was to have things like:
> 
> 	..._needs_wa_16018031267()
> 
> But we don't have a unified system yet
> 
> > +	return cs;
> > +}
> > +
> > +
> 
> two blank lines here
> 
> > +static void
> > +setup_per_ctx_bb(const struct intel_context *ce,
> > +		 const struct intel_engine_cs *engine,
> > +		 u32 *(*emit)(const struct intel_context *, u32 *))
> > +{
> 
> [...]
> 
> >  static u32 *
> > -emit_indirect_ctx_bb_canary(const struct intel_context *ce, u32 *cs)
> > +emit_wabb_ctx_canary(const struct intel_context *ce,
> > +			    u32 *cs, bool per_ctx)
> 
> just a little alignment issue here.
> 
> >  {
> 
> [...]
> 
> Are the failures from CI coming from this series?


Yes.  This series has several failures associated with it, such as a module load failure for
DG2/ATSM, and several failures in the new live_lrc_per_ctx_bb selftest.  I'm not certain
what's causing either set of failures, to be honest, so if you have any guidance on what
might be incorrect in the way I'm setting up the PER_CTX_BB, I'd be willing to try just
about anything.
-Jonathan Cavitt


> 
> Andi
>