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

[Andi Shyti <andi.shyti@linux.intel.com>]

Hi Jonathan,

few little things...

On Wed, Aug 23, 2023 at 11:51:03AM -0700, Jonathan Cavitt wrote:
> From: Nirmoy Das <nirmoy.das@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@intel.com>
> Co-developed-by: Nirmoy Das <nirmoy.das@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?

Andi