diff mbox

[1/2] drm/i915/selftests: rein in igt_write_huge

Message ID 20171122212140.21348-1-matthew.auld@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Matthew Auld Nov. 22, 2017, 9:21 p.m. UTC 
Rather than repeat the test for each engine, which takes a long time,
let's try alternating between the engines in some randomized
order.

Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/selftests/huge_pages.c | 102 ++++++++++++++++------------
 1 file changed, 57 insertions(+), 45 deletions(-)

Comments

Chris Wilson Nov. 22, 2017, 9:27 p.m. UTC | #1 
Quoting Matthew Auld (2017-11-22 21:21:39)
> Rather than repeat the test for each engine, which takes a long time,
> let's try alternating between the engines in some randomized
> order.
> 
> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/selftests/huge_pages.c | 102 ++++++++++++++++------------
>  1 file changed, 57 insertions(+), 45 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/selftests/huge_pages.c b/drivers/gpu/drm/i915/selftests/huge_pages.c
> index db7a0a1f2960..4809368dfbbc 100644
> --- a/drivers/gpu/drm/i915/selftests/huge_pages.c
> +++ b/drivers/gpu/drm/i915/selftests/huge_pages.c
> @@ -27,6 +27,7 @@
>  #include <linux/prime_numbers.h>
>  
>  #include "mock_drm.h"
> +#include "i915_random.h"
>  
>  static const unsigned int page_sizes[] = {
>         I915_GTT_PAGE_SIZE_2M,
> @@ -1044,7 +1045,10 @@ static int igt_write_huge(struct i915_gem_context *ctx,
>  {
>         struct drm_i915_private *i915 = to_i915(obj->base.dev);
>         struct i915_address_space *vm = ctx->ppgtt ? &ctx->ppgtt->base : &i915->ggtt.base;
> +       static struct intel_engine_cs *engines[I915_NUM_ENGINES];
>         struct intel_engine_cs *engine;
> +       I915_RND_STATE(prng);
> +       IGT_TIMEOUT(end_time);
>         struct i915_vma *vma;
>         unsigned int flags = PIN_USER | PIN_OFFSET_FIXED;
>         unsigned int max_page_size;
> @@ -1052,6 +1056,8 @@ static int igt_write_huge(struct i915_gem_context *ctx,
>         u64 max;
>         u64 num;
>         u64 size;
> +       int *order;
> +       int i, n;
>         int err = 0;
>  
>         GEM_BUG_ON(!i915_gem_object_has_pinned_pages(obj));
> @@ -1067,67 +1073,72 @@ static int igt_write_huge(struct i915_gem_context *ctx,
>         if (IS_ERR(vma))
>                 return PTR_ERR(vma);
>  
> +       n = 0;
>         for_each_engine(engine, i915, id) {
> -               IGT_TIMEOUT(end_time);
> -
>                 if (!intel_engine_can_store_dword(engine)) {
> -                       pr_info("store-dword-imm not supported on engine=%u\n",
> -                               id);
> +                       pr_info("store-dword-imm not supported on engine=%u\n", id);
>                         continue;
>                 }
> +               engines[n++] = engine;
> +       }
>  
> -               /*
> -                * Try various offsets until we timeout -- we want to avoid
> -                * issues hidden by effectively always using offset = 0.
> -                */
> -               for_each_prime_number_from(num, 0, max) {
> -                       u64 offset = num * max_page_size;
> -                       u32 dword;
> +       GEM_BUG_ON(!n);

Think gen2! if (!n) return 0;

> +       order = i915_random_order(n, &prng);

if (!order) return -ENOMEM;

Now, I would be a little more cunning in the construction of the
permutation. Use some_number*n, then engine = order[i] % n;
That way we may test feeding to the same engine a few times in succession,
again exploiting the random pattern.

Overall makes a lot of sense, and does seem to be a good compromise.
-Chris
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/selftests/huge_pages.c b/drivers/gpu/drm/i915/selftests/huge_pages.c
index db7a0a1f2960..4809368dfbbc 100644
--- a/drivers/gpu/drm/i915/selftests/huge_pages.c
+++ b/drivers/gpu/drm/i915/selftests/huge_pages.c
@@ -27,6 +27,7 @@ 
 #include <linux/prime_numbers.h>
 
 #include "mock_drm.h"
+#include "i915_random.h"
 
 static const unsigned int page_sizes[] = {
 	I915_GTT_PAGE_SIZE_2M,
@@ -1044,7 +1045,10 @@  static int igt_write_huge(struct i915_gem_context *ctx,
 {
 	struct drm_i915_private *i915 = to_i915(obj->base.dev);
 	struct i915_address_space *vm = ctx->ppgtt ? &ctx->ppgtt->base : &i915->ggtt.base;
+	static struct intel_engine_cs *engines[I915_NUM_ENGINES];
 	struct intel_engine_cs *engine;
+	I915_RND_STATE(prng);
+	IGT_TIMEOUT(end_time);
 	struct i915_vma *vma;
 	unsigned int flags = PIN_USER | PIN_OFFSET_FIXED;
 	unsigned int max_page_size;
@@ -1052,6 +1056,8 @@  static int igt_write_huge(struct i915_gem_context *ctx,
 	u64 max;
 	u64 num;
 	u64 size;
+	int *order;
+	int i, n;
 	int err = 0;
 
 	GEM_BUG_ON(!i915_gem_object_has_pinned_pages(obj));
@@ -1067,67 +1073,72 @@  static int igt_write_huge(struct i915_gem_context *ctx,
 	if (IS_ERR(vma))
 		return PTR_ERR(vma);
 
+	n = 0;
 	for_each_engine(engine, i915, id) {
-		IGT_TIMEOUT(end_time);
-
 		if (!intel_engine_can_store_dword(engine)) {
-			pr_info("store-dword-imm not supported on engine=%u\n",
-				id);
+			pr_info("store-dword-imm not supported on engine=%u\n", id);
 			continue;
 		}
+		engines[n++] = engine;
+	}
 
-		/*
-		 * Try various offsets until we timeout -- we want to avoid
-		 * issues hidden by effectively always using offset = 0.
-		 */
-		for_each_prime_number_from(num, 0, max) {
-			u64 offset = num * max_page_size;
-			u32 dword;
+	GEM_BUG_ON(!n);
+	order = i915_random_order(n, &prng);
 
-			err = i915_vma_unbind(vma);
-			if (err)
-				goto out_vma_close;
+	/*
+	 * Try various offsets until we timeout -- we want to avoid
+	 * issues hidden by effectively always using offset = 0.
+	 */
+	i = 0;
+	for_each_prime_number_from(num, 0, max) {
+		u64 offset = num * max_page_size;
+		u32 dword;
 
-			err = i915_vma_pin(vma, size, max_page_size, flags | offset);
-			if (err) {
-				/*
-				 * The ggtt may have some pages reserved so
-				 * refrain from erroring out.
-				 */
-				if (err == -ENOSPC && i915_is_ggtt(vm)) {
-					err = 0;
-					continue;
-				}
+		err = i915_vma_unbind(vma);
+		if (err)
+			goto out_vma_close;
 
-				goto out_vma_close;
+		err = i915_vma_pin(vma, size, max_page_size, flags | offset);
+		if (err) {
+			/*
+			 * The ggtt may have some pages reserved so
+			 * refrain from erroring out.
+			 */
+			if (err == -ENOSPC && i915_is_ggtt(vm)) {
+				err = 0;
+				continue;
 			}
 
-			err = igt_check_page_sizes(vma);
-			if (err)
-				goto out_vma_unpin;
+			goto out_vma_close;
+		}
 
-			dword = offset_in_page(num) / 4;
+		err = igt_check_page_sizes(vma);
+		if (err)
+			goto out_vma_unpin;
 
-			err = gpu_write(vma, ctx, engine, dword, num + 1);
-			if (err) {
-				pr_err("gpu-write failed at offset=%llx", offset);
-				goto out_vma_unpin;
-			}
+		dword = offset_in_page(num) / 4;
 
-			err = cpu_check(obj, dword, num + 1);
-			if (err) {
-				pr_err("cpu-check failed at offset=%llx", offset);
-				goto out_vma_unpin;
-			}
+		engine = engines[order[i]];
+		i = (i + 1) % n;
 
-			i915_vma_unpin(vma);
+		err = gpu_write(vma, ctx, engine, dword, num + 1);
+		if (err) {
+			pr_err("gpu-write failed at offset=%llx", offset);
+			goto out_vma_unpin;
+		}
 
-			if (num > 0 &&
-			    igt_timeout(end_time,
-					"%s timed out on engine=%u at offset=%llx, max_page_size=%x\n",
-					__func__, id, offset, max_page_size))
-				break;
+		err = cpu_check(obj, dword, num + 1);
+		if (err) {
+			pr_err("cpu-check failed at offset=%llx", offset);
+			goto out_vma_unpin;
 		}
+
+		i915_vma_unpin(vma);
+
+		if (igt_timeout(end_time,
+				"%s timed out on engine=%u at offset=%llx, max_page_size=%x\n",
+				__func__, engine->id, offset, max_page_size))
+			break;
 	}
 
 out_vma_unpin:
@@ -1135,6 +1146,7 @@  static int igt_write_huge(struct i915_gem_context *ctx,
 		i915_vma_unpin(vma);
 out_vma_close:
 	i915_vma_close(vma);
+	kfree(order);
 
 	return err;
 }