Re: [PATCH 2/2] drm/i915/gt: Provide a utility to create a scratch buffer

Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx> · Tue, 15 Dec 2020 13:51:13 -0800

On 12/14/2020 6:15 PM, Chris Wilson wrote:
Primarily used by selftests, but also by runtime debugging of engine
w/a, is a routine to create a temporarily bound buffer for readback.
Almagamate the duplicated routines into one.

Suggested-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
---
  .../drm/i915/gem/selftests/igt_gem_utils.h    |  1 +
  drivers/gpu/drm/i915/gt/intel_gtt.c           | 29 +++++++++++++++
  drivers/gpu/drm/i915/gt/intel_gtt.h           |  3 ++
  drivers/gpu/drm/i915/gt/intel_workarounds.c   | 36 ++-----------------
  drivers/gpu/drm/i915/gt/selftest_execlists.c  | 29 +--------------
  drivers/gpu/drm/i915/gt/selftest_lrc.c        | 24 +------------
  drivers/gpu/drm/i915/gt/selftest_mocs.c       | 29 +--------------
  .../gpu/drm/i915/gt/selftest_workarounds.c    |  6 ++--
  8 files changed, 41 insertions(+), 116 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/selftests/igt_gem_utils.h b/drivers/gpu/drm/i915/gem/selftests/igt_gem_utils.h
index 4221cf84d175..13b241581dbb 100644
--- a/drivers/gpu/drm/i915/gem/selftests/igt_gem_utils.h
+++ b/drivers/gpu/drm/i915/gem/selftests/igt_gem_utils.h
@@ -9,6 +9,7 @@
  
  #include <linux/types.h>
  
+struct i915_address_space;

This doesn't seem to be required in this header.

  struct i915_request;
  struct i915_gem_context;
  struct i915_vma;
diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.c b/drivers/gpu/drm/i915/gt/intel_gtt.c
index 7bfe9072be9a..fbe6c0c5a060 100644
--- a/drivers/gpu/drm/i915/gt/intel_gtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gtt.c
@@ -422,6 +422,35 @@ void setup_private_pat(struct intel_uncore *uncore)
  		bdw_setup_private_ppat(uncore);
  }
  
+struct i915_vma *
+__vm_create_scratch(struct i915_address_space *vm, unsigned long size)
+{
+	struct drm_i915_gem_object *obj;
+	struct i915_vma *vma;
+	int err;
+
+	obj = i915_gem_object_create_internal(vm->i915, PAGE_ALIGN(size));
+	if (IS_ERR(obj))
+		return ERR_CAST(obj);
+
+	i915_gem_object_set_cache_coherency(obj, I915_CACHING_CACHED);
+
+	vma = i915_vma_instance(obj, vm, NULL);
+	if (IS_ERR(vma)) {
+		i915_gem_object_put(obj);
+		return vma;
+	}
+
+	err = i915_vma_pin(vma, 0, 0,
+			   i915_vma_is_ggtt(vma) ? PIN_GLOBAL : PIN_USER);
+	if (err) {
+		i915_gem_object_put(obj);
+		return ERR_PTR(err);
+	}
+
+	return vma;
+}
+
  #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
  #include "selftests/mock_gtt.c"
  #endif
diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.h b/drivers/gpu/drm/i915/gt/intel_gtt.h
index 8a33940a71f3..1cff86073af8 100644
--- a/drivers/gpu/drm/i915/gt/intel_gtt.h
+++ b/drivers/gpu/drm/i915/gt/intel_gtt.h
@@ -573,6 +573,9 @@ int i915_vm_pin_pt_stash(struct i915_address_space *vm,
  void i915_vm_free_pt_stash(struct i915_address_space *vm,
  			   struct i915_vm_pt_stash *stash);
  
+struct i915_vma *
+__vm_create_scratch(struct i915_address_space *vm, unsigned long size);
+
  static inline struct sgt_dma {
  	struct scatterlist *sg;
  	dma_addr_t dma, max;
diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index 52f12a6d66b9..be5b090ae6ae 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -2085,39 +2085,6 @@ 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 * sizeof(u32), 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);
-}
-
  struct mcr_range {
  	u32 start;
  	u32 end;
@@ -2220,7 +2187,8 @@ static int engine_wa_list_verify(struct intel_context *ce,
  	if (!wal->count)
  		return 0;
  
-	vma = create_scratch(&ce->engine->gt->ggtt->vm, wal->count);
+	vma = __vm_create_scratch(&ce->engine->gt->ggtt->vm,
+				  wal->count * sizeof(u32));
  	if (IS_ERR(vma))
  		return PTR_ERR(vma);
  
diff --git a/drivers/gpu/drm/i915/gt/selftest_execlists.c b/drivers/gpu/drm/i915/gt/selftest_execlists.c
index 34c2bb8313eb..c6c0b9918c3e 100644
--- a/drivers/gpu/drm/i915/gt/selftest_execlists.c
+++ b/drivers/gpu/drm/i915/gt/selftest_execlists.c
@@ -25,33 +25,6 @@
  #define NUM_GPR 16
  #define NUM_GPR_DW (NUM_GPR * 2) /* each GPR is 2 dwords */
  
-static struct i915_vma *create_scratch(struct intel_gt *gt)
-{
-	struct drm_i915_gem_object *obj;
-	struct i915_vma *vma;
-	int err;
-
-	obj = i915_gem_object_create_internal(gt->i915, PAGE_SIZE);
-	if (IS_ERR(obj))
-		return ERR_CAST(obj);
-
-	i915_gem_object_set_cache_coherency(obj, I915_CACHING_CACHED);
-
-	vma = i915_vma_instance(obj, &gt->ggtt->vm, NULL);
-	if (IS_ERR(vma)) {
-		i915_gem_object_put(obj);
-		return vma;
-	}
-
-	err = i915_vma_pin(vma, 0, 0, PIN_GLOBAL);
-	if (err) {
-		i915_gem_object_put(obj);
-		return ERR_PTR(err);
-	}
-
-	return vma;
-}
-
  static bool is_active(struct i915_request *rq)
  {
  	if (i915_request_is_active(rq))
@@ -4167,7 +4140,7 @@ static int preserved_virtual_engine(struct intel_gt *gt,
  	int err = 0;
  	u32 *cs;
  
-	scratch = create_scratch(siblings[0]->gt);
+	scratch = __vm_create_scratch(&siblings[0]->gt->ggtt->vm, PAGE_SIZE);
  	if (IS_ERR(scratch))
  		return PTR_ERR(scratch);
  
diff --git a/drivers/gpu/drm/i915/gt/selftest_lrc.c b/drivers/gpu/drm/i915/gt/selftest_lrc.c
index 5774626d3d34..b26982ab7cca 100644
--- a/drivers/gpu/drm/i915/gt/selftest_lrc.c
+++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c
@@ -28,29 +28,7 @@
  
  static struct i915_vma *create_scratch(struct intel_gt *gt)
  {
-	struct drm_i915_gem_object *obj;
-	struct i915_vma *vma;
-	int err;
-
-	obj = i915_gem_object_create_internal(gt->i915, PAGE_SIZE);
-	if (IS_ERR(obj))
-		return ERR_CAST(obj);
-
-	i915_gem_object_set_cache_coherency(obj, I915_CACHING_CACHED);
-
-	vma = i915_vma_instance(obj, &gt->ggtt->vm, NULL);
-	if (IS_ERR(vma)) {
-		i915_gem_object_put(obj);
-		return vma;
-	}
-
-	err = i915_vma_pin(vma, 0, 0, PIN_GLOBAL);
-	if (err) {
-		i915_gem_object_put(obj);
-		return ERR_PTR(err);
-	}
-
-	return vma;
+	return __vm_create_scratch(&gt->ggtt->vm, PAGE_SIZE);
  }
  
  static bool is_active(struct i915_request *rq)
diff --git a/drivers/gpu/drm/i915/gt/selftest_mocs.c b/drivers/gpu/drm/i915/gt/selftest_mocs.c
index 21dcd91cbd62..38d82b94d1a1 100644
--- a/drivers/gpu/drm/i915/gt/selftest_mocs.c
+++ b/drivers/gpu/drm/i915/gt/selftest_mocs.c
@@ -56,33 +56,6 @@ static int request_add_spin(struct i915_request *rq, struct igt_spinner *spin)
  	return err;
  }
  
-static struct i915_vma *create_scratch(struct intel_gt *gt)
-{
-	struct drm_i915_gem_object *obj;
-	struct i915_vma *vma;
-	int err;
-
-	obj = i915_gem_object_create_internal(gt->i915, PAGE_SIZE);
-	if (IS_ERR(obj))
-		return ERR_CAST(obj);
-
-	i915_gem_object_set_cache_coherency(obj, I915_CACHING_CACHED);
-
-	vma = i915_vma_instance(obj, &gt->ggtt->vm, NULL);
-	if (IS_ERR(vma)) {
-		i915_gem_object_put(obj);
-		return vma;
-	}
-
-	err = i915_vma_pin(vma, 0, 0, PIN_GLOBAL);
-	if (err) {
-		i915_gem_object_put(obj);
-		return ERR_PTR(err);
-	}
-
-	return vma;
-}
-
  static int live_mocs_init(struct live_mocs *arg, struct intel_gt *gt)
  {
  	struct drm_i915_mocs_table table;
@@ -101,7 +74,7 @@ static int live_mocs_init(struct live_mocs *arg, struct intel_gt *gt)
  	if (flags & (HAS_GLOBAL_MOCS | HAS_ENGINE_MOCS))
  		arg->mocs = table;
  
-	arg->scratch = create_scratch(gt);
+	arg->scratch = __vm_create_scratch(&gt->ggtt->vm, PAGE_SIZE);
  	if (IS_ERR(arg->scratch))
  		return PTR_ERR(arg->scratch);
  
diff --git a/drivers/gpu/drm/i915/gt/selftest_workarounds.c b/drivers/gpu/drm/i915/gt/selftest_workarounds.c
index 703b77207a47..04f8f5df2270 100644
--- a/drivers/gpu/drm/i915/gt/selftest_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/selftest_workarounds.c
@@ -489,7 +489,7 @@ static int check_dirty_whitelist(struct intel_context *ce)
  	int err = 0, i, v;
  	u32 *cs, *results;
  
-	scratch = create_scratch(ce->vm, 2 * ARRAY_SIZE(values) + 1);
+	scratch = __vm_create_scratch(ce->vm, 2 * ARRAY_SIZE(values) + 1);

The size here needs to be multiplied by sizeof(u32).
With these nits fixed:

Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>

Daniele

  	if (IS_ERR(scratch))
  		return PTR_ERR(scratch);
  
@@ -1028,13 +1028,13 @@ static int live_isolated_whitelist(void *arg)
  		return 0;
  
  	for (i = 0; i < ARRAY_SIZE(client); i++) {
-		client[i].scratch[0] = create_scratch(gt->vm, 1024);
+		client[i].scratch[0] = __vm_create_scratch(gt->vm, 4096);
  		if (IS_ERR(client[i].scratch[0])) {
  			err = PTR_ERR(client[i].scratch[0]);
  			goto err;
  		}
  
-		client[i].scratch[1] = create_scratch(gt->vm, 1024);
+		client[i].scratch[1] = __vm_create_scratch(gt->vm, 4096);
  		if (IS_ERR(client[i].scratch[1])) {
  			err = PTR_ERR(client[i].scratch[1]);
  			i915_vma_unpin_and_release(&client[i].scratch[0], 0);

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






