Re: [PATCH v4 3/4] drm/i915/gt: add selftest to exercise WABB

[Nirmoy Das <nirmoy.das@xxxxxxxxxxxxxxx>]

Hi Andrzej,

On 10/23/2023 10:21 PM, Andrzej Hajda wrote:
> Test re-uses logic form indirect ctx BB selftest.
>
> Co-developed-by: Nirmoy Das <nirmoy.das@xxxxxxxxx>

You can remove above and add my Reviewed-by: Nirmoy Das 
<nirmoy.das@xxxxxxxxx> for this patch.

Co-developed-by should be followed by a signed-off. Same for the 
previous patch.


Regards,

Nirmoy

> Co-developed-by: Jonathan Cavitt <jonathan.cavitt@xxxxxxxxx>
> Signed-off-by: Andrzej Hajda <andrzej.hajda@xxxxxxxxx>
> ---
>   drivers/gpu/drm/i915/gt/selftest_lrc.c | 65 ++++++++++++++++++++++++----------
>   1 file changed, 47 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/selftest_lrc.c b/drivers/gpu/drm/i915/gt/selftest_lrc.c
> index 5f826b6dcf5d6f..e17b8777d21dc9 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c
> @@ -1555,7 +1555,7 @@ static int live_lrc_isolation(void *arg)
>   	return err;
>   }
>   
> -static int indirect_ctx_submit_req(struct intel_context *ce)
> +static int wabb_ctx_submit_req(struct intel_context *ce)
>   {
>   	struct i915_request *rq;
>   	int err = 0;
> @@ -1579,7 +1579,8 @@ static int indirect_ctx_submit_req(struct intel_context *ce)
>   #define CTX_BB_CANARY_INDEX  (CTX_BB_CANARY_OFFSET / sizeof(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)
>   {
>   	*cs++ = MI_STORE_REGISTER_MEM_GEN8 |
>   		MI_SRM_LRM_GLOBAL_GTT |
> @@ -1587,26 +1588,43 @@ emit_indirect_ctx_bb_canary(const struct intel_context *ce, u32 *cs)
>   	*cs++ = i915_mmio_reg_offset(RING_START(0));
>   	*cs++ = i915_ggtt_offset(ce->state) +
>   		context_wa_bb_offset(ce) +
> -		CTX_BB_CANARY_OFFSET;
> +		CTX_BB_CANARY_OFFSET +
> +		(per_ctx ? PAGE_SIZE : 0);
>   	*cs++ = 0;
>   
>   	return cs;
>   }
>   
> +static u32 *
> +emit_indirect_ctx_bb_canary(const struct intel_context *ce, u32 *cs)
> +{
> +	return emit_wabb_ctx_canary(ce, cs, false);
> +}
> +
> +static u32 *
> +emit_per_ctx_bb_canary(const struct intel_context *ce, u32 *cs)
> +{
> +	return emit_wabb_ctx_canary(ce, cs, true);
> +}
> +
>   static void
> -indirect_ctx_bb_setup(struct intel_context *ce)
> +wabb_ctx_setup(struct intel_context *ce, bool per_ctx)
>   {
> -	u32 *cs = context_indirect_bb(ce);
> +	u32 *cs = context_wabb(ce, per_ctx);
>   
>   	cs[CTX_BB_CANARY_INDEX] = 0xdeadf00d;
>   
> -	setup_indirect_ctx_bb(ce, ce->engine, emit_indirect_ctx_bb_canary);
> +	if (per_ctx)
> +		setup_per_ctx_bb(ce, ce->engine, emit_per_ctx_bb_canary);
> +	else
> +		setup_indirect_ctx_bb(ce, ce->engine, emit_indirect_ctx_bb_canary);
>   }
>   
> -static bool check_ring_start(struct intel_context *ce)
> +static bool check_ring_start(struct intel_context *ce, bool per_ctx)
>   {
>   	const u32 * const ctx_bb = (void *)(ce->lrc_reg_state) -
> -		LRC_STATE_OFFSET + context_wa_bb_offset(ce);
> +		LRC_STATE_OFFSET + context_wa_bb_offset(ce) +
> +		(per_ctx ? PAGE_SIZE : 0);
>   
>   	if (ctx_bb[CTX_BB_CANARY_INDEX] == ce->lrc_reg_state[CTX_RING_START])
>   		return true;
> @@ -1618,21 +1636,21 @@ static bool check_ring_start(struct intel_context *ce)
>   	return false;
>   }
>   
> -static int indirect_ctx_bb_check(struct intel_context *ce)
> +static int wabb_ctx_check(struct intel_context *ce, bool per_ctx)
>   {
>   	int err;
>   
> -	err = indirect_ctx_submit_req(ce);
> +	err = wabb_ctx_submit_req(ce);
>   	if (err)
>   		return err;
>   
> -	if (!check_ring_start(ce))
> +	if (!check_ring_start(ce, per_ctx))
>   		return -EINVAL;
>   
>   	return 0;
>   }
>   
> -static int __live_lrc_indirect_ctx_bb(struct intel_engine_cs *engine)
> +static int __lrc_wabb_ctx(struct intel_engine_cs *engine, bool per_ctx)
>   {
>   	struct intel_context *a, *b;
>   	int err;
> @@ -1667,14 +1685,14 @@ static int __live_lrc_indirect_ctx_bb(struct intel_engine_cs *engine)
>   	 * As ring start is restored apriori of starting the indirect ctx bb and
>   	 * as it will be different for each context, it fits to this purpose.
>   	 */
> -	indirect_ctx_bb_setup(a);
> -	indirect_ctx_bb_setup(b);
> +	wabb_ctx_setup(a, per_ctx);
> +	wabb_ctx_setup(b, per_ctx);
>   
> -	err = indirect_ctx_bb_check(a);
> +	err = wabb_ctx_check(a, per_ctx);
>   	if (err)
>   		goto unpin_b;
>   
> -	err = indirect_ctx_bb_check(b);
> +	err = wabb_ctx_check(b, per_ctx);
>   
>   unpin_b:
>   	intel_context_unpin(b);
> @@ -1688,7 +1706,7 @@ static int __live_lrc_indirect_ctx_bb(struct intel_engine_cs *engine)
>   	return err;
>   }
>   
> -static int live_lrc_indirect_ctx_bb(void *arg)
> +static int lrc_wabb_ctx(void *arg, bool per_ctx)
>   {
>   	struct intel_gt *gt = arg;
>   	struct intel_engine_cs *engine;
> @@ -1697,7 +1715,7 @@ static int live_lrc_indirect_ctx_bb(void *arg)
>   
>   	for_each_engine(engine, gt, id) {
>   		intel_engine_pm_get(engine);
> -		err = __live_lrc_indirect_ctx_bb(engine);
> +		err = __lrc_wabb_ctx(engine, per_ctx);
>   		intel_engine_pm_put(engine);
>   
>   		if (igt_flush_test(gt->i915))
> @@ -1710,6 +1728,16 @@ static int live_lrc_indirect_ctx_bb(void *arg)
>   	return err;
>   }
>   
> +static int live_lrc_indirect_ctx_bb(void *arg)
> +{
> +	return lrc_wabb_ctx(arg, false);
> +}
> +
> +static int live_lrc_per_ctx_bb(void *arg)
> +{
> +	return lrc_wabb_ctx(arg, true);
> +}
> +
>   static void garbage_reset(struct intel_engine_cs *engine,
>   			  struct i915_request *rq)
>   {
> @@ -1947,6 +1975,7 @@ int intel_lrc_live_selftests(struct drm_i915_private *i915)
>   		SUBTEST(live_lrc_garbage),
>   		SUBTEST(live_pphwsp_runtime),
>   		SUBTEST(live_lrc_indirect_ctx_bb),
> +		SUBTEST(live_lrc_per_ctx_bb),
>   	};
>   
>   	if (!HAS_LOGICAL_RING_CONTEXTS(i915))