[Intel-gfx] [PATCH 1/2] drm/i915/selftests: Add live_context_workarounds

[Chris Wilson]

Quoting Tvrtko Ursulin (2019-05-20 09:28:15)
> From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> 
> Test context workarounds have been correctly applied in a newly created
> context.
> 
> To accomplish this the existing engine_wa_list_verify helper is extended
> to take in a context from which reading of the workaround list will be
> done.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_workarounds.c   | 18 +++++++---
>  .../gpu/drm/i915/gt/selftest_workarounds.c    | 36 ++++++++++++++++++-
>  2 files changed, 49 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> index 43e290306551..4494bc917084 100644
> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> @@ -1338,11 +1338,13 @@ wa_list_srm(struct i915_request *rq,
>         return 0;
>  }
>  
> -static int engine_wa_list_verify(struct intel_engine_cs *engine,
> +static int engine_wa_list_verify(struct i915_gem_context *ctx,
> +                                enum intel_engine_id id,
>                                  const struct i915_wa_list * const wal,
>                                  const char *from)
>  {
>         const struct i915_wa *wa;
> +       struct intel_context *ce;

Pass intel_context around, it's far less hairy in the long run.

>         struct i915_request *rq;
>         struct i915_vma *vma;
>         unsigned int i;
> @@ -1352,11 +1354,16 @@ static int engine_wa_list_verify(struct intel_engine_cs *engine,
>         if (!wal->count)
>                 return 0;
>  
> -       vma = create_scratch(&engine->i915->ggtt.vm, wal->count);
> +       vma = create_scratch(&ctx->i915->ggtt.vm, wal->count);
>         if (IS_ERR(vma))
>                 return PTR_ERR(vma);
>  
> -       rq = i915_request_create(engine->kernel_context);
> +       ce = i915_gem_context_get_engine(ctx, id);
> +       if (IS_ERR(ce))
> +               return PTR_ERR(ce);
> +
> +       rq = intel_context_create_request(ce);

Fwiw, this is igt_request_alloc(), but I'd recommend passing
intel_context.

> +       intel_context_put(ce);
>         if (IS_ERR(rq)) {
>                 err = PTR_ERR(rq);
>                 goto err_vma;
> @@ -1394,7 +1401,10 @@ static int engine_wa_list_verify(struct intel_engine_cs *engine,
>  int intel_engine_verify_workarounds(struct intel_engine_cs *engine,
>                                     const char *from)
>  {
> -       return engine_wa_list_verify(engine, &engine->wa_list, from);
> +       return engine_wa_list_verify(engine->kernel_context->gem_context,
> +                                    engine->id,
> +                                    &engine->wa_list,
> +                                    from);
>  }
>  
>  #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
> diff --git a/drivers/gpu/drm/i915/gt/selftest_workarounds.c b/drivers/gpu/drm/i915/gt/selftest_workarounds.c
> index 9f7680b9984b..ae3f92d55ed8 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_workarounds.c
> @@ -1013,7 +1013,7 @@ static bool verify_gt_engine_wa(struct drm_i915_private *i915,
>         ok &= wa_list_verify(&i915->uncore, &lists->gt_wa_list, str);
>  
>         for_each_engine(engine, i915, id) {
> -               ok &= engine_wa_list_verify(engine,
> +               ok &= engine_wa_list_verify(i915->kernel_context, id,

becomes

	ok &= engine_wa_list_verify(engine->kernel_context,

>                                             &lists->engine[id].wa_list,
>                                             str) == 0;
>         }
> @@ -1142,6 +1142,39 @@ live_engine_reset_gt_engine_workarounds(void *arg)
>         return ret;
>  }
>  
> +static int
> +intel_ctx_verify_workarounds(struct intel_engine_cs *engine, const char *from)
> +{
> +       struct i915_gem_context *ctx =
> +               i915_gem_context_create_kernel(engine->i915, 0);
> +       int ret;
> +
> +       if (IS_ERR(ctx))
> +               return PTR_ERR(ctx);
> +

Looks like the first user for a GEM-less intel_context. Food for
thought.

> +       ret = engine_wa_list_verify(ctx, engine->id, &engine->ctx_wa_list,
> +                                   from);
> +
> +       i915_gem_context_set_closed(ctx);
> +       i915_gem_context_put(ctx);
> +
> +       return ret;
> +}
> +
> +static int
> +live_context_workarounds(void *arg)
> +{
> +       struct drm_i915_private *i915 = arg;
> +       struct intel_engine_cs *engine;
> +       enum intel_engine_id id;
> +       int ret = 0;
> +
> +       for_each_engine(engine, i915, id)

So you might as well create the context in the outer scope, then iterate
over the for_each_gem_engine()

> +               ret |= intel_ctx_verify_workarounds(engine, engine->name);

And then we could even extend this to inject a reset and verify the
context is unaffected.
-Chris