[igt-dev] [RFT v4 4/6] igt/i915: Require GTT mapping to be available when needed.

[Chris Wilson]

Quoting Antonio Argenziano (2019-03-25 23:20:41)
> diff --git a/lib/igt_dummyload.c b/lib/igt_dummyload.c
> index 47f6b92b..2e4c0335 100644
> --- a/lib/igt_dummyload.c
> +++ b/lib/igt_dummyload.c
> @@ -117,9 +117,11 @@ emit_recursive_batch(igt_spin_t *spin,
>         obj[BATCH].handle = gem_create(fd, BATCH_SIZE);
>         batch = __gem_mmap__wc(fd, obj[BATCH].handle,
>                                0, BATCH_SIZE, PROT_WRITE);
> -       if (!batch)
> +       if (!batch) {
> +               gem_require_mmap_gtt(fd);
>                 batch = gem_mmap__gtt(fd, obj[BATCH].handle,
>                                         BATCH_SIZE, PROT_WRITE);

batch = __gem_mmap__gtt();
igt_require(batch) ?

Possibly not as informative as gem_require_mmap_gtt() So ~o~

> diff --git a/tests/i915/gem_concurrent_all.c b/tests/i915/gem_concurrent_all.c
> index f719b0a1..e4fc1426 100644
> --- a/tests/i915/gem_concurrent_all.c
> +++ b/tests/i915/gem_concurrent_all.c
> @@ -1422,6 +1422,7 @@ static void cpu_require(void)
>  
>  static void gtt_require(void)
>  {
> +       gem_require_mmap_gtt(fd);
>  }

Needs the combinatorial explosion to exercise accessing via mmap-offset.
If you only update one stress test, update this one.

>  static void bcs_require(void)
> diff --git a/tests/i915/gem_ctx_sseu.c b/tests/i915/gem_ctx_sseu.c
> index 3afa5c15..bf1b50d5 100644
> --- a/tests/i915/gem_ctx_sseu.c
> +++ b/tests/i915/gem_ctx_sseu.c
> @@ -528,8 +528,10 @@ igt_main
>                 igt_subtest("invalid-sseu")
>                         test_invalid_sseu(fd);
>  
> -               igt_subtest("ggtt-args")
> +               igt_subtest("ggtt-args") {
> +                       gem_require_mmap_gtt(fd);
>                         test_ggtt_args(fd);
> +               }
>  
>                 igt_subtest("engines")
>                         test_engines(fd);

> diff --git a/tests/i915/gem_exec_reloc.c b/tests/i915/gem_exec_reloc.c
> index 837f60a6..bb4eec31 100644
> --- a/tests/i915/gem_exec_reloc.c
> +++ b/tests/i915/gem_exec_reloc.c
> @@ -115,6 +115,9 @@ static void from_mmap(int fd, uint64_t size, enum mode mode)
>          */
>         intel_require_memory(2, size, CHECK_RAM);
>  
> +       if ((mode & ~RO) == GTT)
> +               gem_require_mmap_gtt(fd);
> +

We need to stick arguments into mmap-offset buffers. Same is true for
several of the "do we survive userspace being nasty with __user pointers"
tests.

> diff --git a/tests/i915/gem_exec_schedule.c b/tests/i915/gem_exec_schedule.c
> index a9383000..15c8440f 100644
> --- a/tests/i915/gem_exec_schedule.c
> +++ b/tests/i915/gem_exec_schedule.c
> @@ -1236,6 +1236,7 @@ igt_main
>                         igt_subtest_f("independent-%s", e->name) {
>                                 igt_require(gem_ring_has_physical_engine(fd, e->exec_id | e->flags));
>                                 igt_require(gem_can_store_dword(fd, e->exec_id | e->flags));
> +                               gem_require_mmap_gtt(fd);
>                                 independent(fd, e->exec_id | e->flags);
>                         }
>                 }
> @@ -1328,8 +1329,10 @@ igt_main
>                                 igt_subtest_f("wide-%s", e->name)
>                                         wide(fd, e->exec_id | e->flags);
>  
> -                               igt_subtest_f("reorder-wide-%s", e->name)
> +                               igt_subtest_f("reorder-wide-%s", e->name) {
> +                                       gem_require_mmap_gtt(fd);
>                                         reorder_wide(fd, e->exec_id | e->flags);
> +                               }
>  
>                                 igt_subtest_f("smoketest-%s", e->name)
>                                         smoketest(fd, e->exec_id | e->flags, 5);

Hmm. I would rather the basic scheduling tests remained functioning.

> diff --git a/tests/i915/gem_largeobject.c b/tests/i915/gem_largeobject.c
> index 518396fa..a2d47edc 100644
> --- a/tests/i915/gem_largeobject.c
> +++ b/tests/i915/gem_largeobject.c
> @@ -84,6 +84,8 @@ igt_simple_main
>  
>         fd = drm_open_driver(DRIVER_INTEL);
>  
> +       gem_require_mmap_gtt(fd);

Ok, not mmap-offset specific in future, but one of those fun challenges.

> diff --git a/tests/i915/gem_madvise.c b/tests/i915/gem_madvise.c
> index 729a4d33..f4226a84 100644
> --- a/tests/i915/gem_madvise.c
> +++ b/tests/i915/gem_madvise.c
> @@ -61,6 +61,8 @@ dontneed_before_mmap(void)
>         uint32_t handle;
>         char *ptr;
>  
> +       gem_require_mmap_gtt(fd);

Needs mmap-offset integration.

> diff --git a/tests/i915/gem_mmap_offset_exhaustion.c b/tests/i915/gem_mmap_offset_exhaustion.c
> index 8c8e3fa2..86464231 100644
> --- a/tests/i915/gem_mmap_offset_exhaustion.c
> +++ b/tests/i915/gem_mmap_offset_exhaustion.c
> @@ -82,6 +82,8 @@ igt_simple_main
>  
>         fd = drm_open_driver(DRIVER_INTEL);
>  
> +       gem_require_mmap_gtt(fd);

Some might say this became a more important test. Not sure if the test
itself is the best strategy (vs the selftest approach), but it
definitely shouldn't just be skipped for mmap-offset.

> diff --git a/tests/i915/gem_render_copy.c b/tests/i915/gem_render_copy.c
> index 8d62a0f4..5757c547 100644
> --- a/tests/i915/gem_render_copy.c
> +++ b/tests/i915/gem_render_copy.c
> @@ -572,6 +572,8 @@ static void test(data_t *data, uint32_t tiling, uint64_t ccs_modifier)
>         int opt_dump_aub = igt_aub_dump_enabled();
>         int num_src = ARRAY_SIZE(src);
>  
> +       gem_require_mmap_gtt(data->drm_fd);

We should make sure gem_render_copy remains available for debugging the
rendercopy routines, I doubt they are going away.

> diff --git a/tests/i915/gem_userptr_blits.c b/tests/i915/gem_userptr_blits.c
> index 8f8ddf43..9962e539 100644
> --- a/tests/i915/gem_userptr_blits.c
> +++ b/tests/i915/gem_userptr_blits.c
> @@ -554,6 +554,8 @@ static int test_invalid_gtt_mapping(int fd)
>         uint32_t handle;
>         char *gtt, *map;
>  
> +       gem_require_mmap_gtt(fd);

userptr-blits needs mmap-offset enhancements. Principally, if we can
mmap(MAP_FIXED) over top of an active userptr, we need to resolve that
amicably.
-Chris