[Intel-gfx] [PATCH 1/2] drm/i915/selftests: rein in igt_write_huge

[Chris Wilson]

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 at chris-wilson.co.uk>
> Signed-off-by: Matthew Auld <matthew.auld at intel.com>
> Cc: Chris Wilson <chris at 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