Re: [PATCH 2/4] drm/i915: Verify the engine workarounds stick on application

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 13/04/2019 13:58, Chris Wilson wrote:
Read the engine workarounds back using the GPU after loading the initial
context state to verify that we are setting them correctly, and bail if
it fails.

Aren't the context wa/ ones we expect to see saved in the context? As such, what difference do you expect to see between mmio and srm verification? Should even both be done?


Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
---
  drivers/gpu/drm/i915/i915_gem.c               |   6 +
  drivers/gpu/drm/i915/intel_workarounds.c      | 120 ++++++++++++++++++
  drivers/gpu/drm/i915/intel_workarounds.h      |   2 +
  .../drm/i915/selftests/intel_workarounds.c    |  53 +-------
  4 files changed, 134 insertions(+), 47 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 0a818a60ad31..95ae69753e91 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4717,6 +4717,12 @@ static int __intel_engines_record_defaults(struct drm_i915_private *i915)
  		i915_request_add(rq);
  		if (err)
  			goto err_active;
+
+		if (IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM) &&
+		    intel_engine_verify_workarounds(engine, "load")) {
+			err = -EIO;
+			goto err_active;

I am not sure that wedging is required even if only in debug builds. Is DRM_ERROR not enough to flag failures?

+		}
  	}
/* Flush the default context image to memory, and enable powersaving. */
diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c
index 1c54b5030807..db99f2e676bb 100644
--- a/drivers/gpu/drm/i915/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/intel_workarounds.c
@@ -1259,6 +1259,126 @@ void intel_engine_apply_workarounds(struct intel_engine_cs *engine)
  	wa_list_apply(engine->uncore, &engine->wa_list);
  }
+static struct i915_vma *
+create_scratch(struct i915_address_space *vm, int count)
+{
+	struct drm_i915_gem_object *obj;
+	struct i915_vma *vma;
+	unsigned int size;
+	int err;
+
+	size = round_up(count * 4, PAGE_SIZE);
+	obj = i915_gem_object_create_internal(vm->i915, size);
+	if (IS_ERR(obj))
+		return ERR_CAST(obj);
+
+	i915_gem_object_set_cache_coherency(obj, I915_CACHE_LLC);
+
+	vma = i915_vma_instance(obj, vm, NULL);
+	if (IS_ERR(vma)) {
+		err = PTR_ERR(vma);
+		goto err_obj;
+	}
+
+	err = i915_vma_pin(vma, 0, 0,
+			   i915_vma_is_ggtt(vma) ? PIN_GLOBAL : PIN_USER);
+	if (err)
+		goto err_obj;
+
+	return vma;
+
+err_obj:
+	i915_gem_object_put(obj);
+	return ERR_PTR(err);
+}
+
+static int
+wa_list_srm(struct i915_request *rq,
+	    const struct i915_wa_list *wal,
+	    struct i915_vma *vma)
+{
+	const struct i915_wa *wa;
+	u32 srm, *cs;
+	int i;
+
+	srm = MI_STORE_REGISTER_MEM | MI_SRM_LRM_GLOBAL_GTT;
+	if (INTEL_GEN(rq->i915) >= 8)
+		srm++;
+
+	cs = intel_ring_begin(rq, 4 * wal->count);
+	if (IS_ERR(cs))
+		return PTR_ERR(cs);
+
+	for (i = 0, wa = wal->list; i < wal->count; i++, wa++) {
+		*cs++ = srm;
+		*cs++ = i915_mmio_reg_offset(wa->reg);
+		*cs++ = i915_ggtt_offset(vma) + sizeof(u32) * i;
+		*cs++ = 0;
+	}
+	intel_ring_advance(rq, cs);
+
+	return 0;
+}
+
+static int engine_wa_list_verify(struct intel_engine_cs *engine,
+				 const struct i915_wa_list * const wal,
+				 const char *from)
+{
+	const struct i915_wa *wa;
+	struct i915_request *rq;
+	struct i915_vma *vma;
+	unsigned int i;
+	u32 *results;
+	int err;
+
+	if (!wal->count)
+		return 0;
+
+	vma = create_scratch(&engine->i915->ggtt.vm, wal->count);
+	if (IS_ERR(vma))
+		return PTR_ERR(vma);
+
+	rq = i915_request_alloc(engine, engine->kernel_context->gem_context);
+	if (IS_ERR(rq)) {
+		err = PTR_ERR(rq);
+		goto err_vma;
+	}
+
+	err = wa_list_srm(rq, wal, vma);
+	if (err)
+		goto err_vma;
+
+	i915_request_add(rq);
+	if (i915_request_wait(rq, I915_WAIT_LOCKED, HZ / 5) < 0) {
+		err = -ETIME;
+		goto err_vma;
+	}
+
+	results = i915_gem_object_pin_map(vma->obj, I915_MAP_WB);
+	if (IS_ERR(results)) {
+		err = PTR_ERR(results);
+		goto err_vma;
+	}
+
+	err = 0;
+	for (i = 0, wa = wal->list; i < wal->count; i++, wa++)
+		if (!wa_verify(wa, results[i], wal->name, from))
+			err = -ENXIO;
+
+	i915_gem_object_unpin_map(vma->obj);
+
+err_vma:
+	i915_vma_unpin(vma);
+	i915_vma_put(vma);
+	return err;
+}
+
+int intel_engine_verify_workarounds(struct intel_engine_cs *engine,
+				    const char *from)
+{
+	return engine_wa_list_verify(engine, &engine->wa_list, from);
+}
+
  #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
  #include "selftests/intel_workarounds.c"
  #endif
diff --git a/drivers/gpu/drm/i915/intel_workarounds.h b/drivers/gpu/drm/i915/intel_workarounds.h
index 34eee5ec511e..fdf7ebb90f28 100644
--- a/drivers/gpu/drm/i915/intel_workarounds.h
+++ b/drivers/gpu/drm/i915/intel_workarounds.h
@@ -30,5 +30,7 @@ void intel_engine_apply_whitelist(struct intel_engine_cs *engine);
void intel_engine_init_workarounds(struct intel_engine_cs *engine);
  void intel_engine_apply_workarounds(struct intel_engine_cs *engine);
+int intel_engine_verify_workarounds(struct intel_engine_cs *engine,
+				    const char *from);
#endif
diff --git a/drivers/gpu/drm/i915/selftests/intel_workarounds.c b/drivers/gpu/drm/i915/selftests/intel_workarounds.c
index 567b6f8dae86..a363748a7a4f 100644
--- a/drivers/gpu/drm/i915/selftests/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/selftests/intel_workarounds.c
@@ -340,49 +340,6 @@ static int check_whitelist_across_reset(struct intel_engine_cs *engine,
  	return err;
  }
-static struct i915_vma *create_scratch(struct i915_gem_context *ctx)
-{
-	struct drm_i915_gem_object *obj;
-	struct i915_vma *vma;
-	void *ptr;
-	int err;
-
-	obj = i915_gem_object_create_internal(ctx->i915, PAGE_SIZE);
-	if (IS_ERR(obj))
-		return ERR_CAST(obj);
-
-	i915_gem_object_set_cache_coherency(obj, I915_CACHE_LLC);
-
-	ptr = i915_gem_object_pin_map(obj, I915_MAP_WB);
-	if (IS_ERR(ptr)) {
-		err = PTR_ERR(ptr);
-		goto err_obj;
-	}
-	memset(ptr, 0xc5, PAGE_SIZE);
-	i915_gem_object_flush_map(obj);
-	i915_gem_object_unpin_map(obj);
-
-	vma = i915_vma_instance(obj, &ctx->ppgtt->vm, NULL);
-	if (IS_ERR(vma)) {
-		err = PTR_ERR(vma);
-		goto err_obj;
-	}
-
-	err = i915_vma_pin(vma, 0, 0, PIN_USER);
-	if (err)
-		goto err_obj;
-
-	err = i915_gem_object_set_to_cpu_domain(obj, false);
-	if (err)
-		goto err_obj;
-
-	return vma;
-
-err_obj:
-	i915_gem_object_put(obj);
-	return ERR_PTR(err);
-}
-
  static struct i915_vma *create_batch(struct i915_gem_context *ctx)
  {
  	struct drm_i915_gem_object *obj;
@@ -475,7 +432,7 @@ static int check_dirty_whitelist(struct i915_gem_context *ctx,
  	int err = 0, i, v;
  	u32 *cs, *results;
- scratch = create_scratch(ctx);
+	scratch = create_scratch(&ctx->ppgtt->vm, 2 * ARRAY_SIZE(values) + 1);
  	if (IS_ERR(scratch))
  		return PTR_ERR(scratch);
@@ -752,9 +709,11 @@ 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 &= wa_list_verify(engine->uncore,
-				     &lists->engine[id].wa_list, str);
+	for_each_engine(engine, i915, id) {
+		ok &= engine_wa_list_verify(engine,
+					    &lists->engine[id].wa_list,
+					    str) == 0;
+	}
return ok;
  }


Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux