[igt-dev] [RFT v4 4/6] igt/i915: Require GTT mapping to be available when needed.
Antonio Argenziano
antonio.argenziano at intel.com
Wed Mar 27 21:05:52 UTC 2019
On 25/03/19 16:36, Chris Wilson wrote:
> 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.
Don't we get the mmap-offset coverage by default in the wc test cases or
do you mean to use the mmap_offset IOCTL to mmap GTT?
>
>> 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.
Didn't we have a convincing argument against using wc + sync?
Antonio
>
>> 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
>
More information about the igt-dev
mailing list