[igt-dev] [PATCH i-g-t v6 7/9] igt/i915: Require GTT mapping to be available when needed.

[Antonio Argenziano]

On 14/06/19 02:54, Chris Wilson wrote:
> Quoting Antonio Argenziano (2019-06-13 22:54:46)
>> diff --git a/tests/i915/gem_exec_schedule.c b/tests/i915/gem_exec_schedule.c
>> index 7b418622..20b3404e 100644
>> --- a/tests/i915/gem_exec_schedule.c
>> +++ b/tests/i915/gem_exec_schedule.c
>> @@ -1660,6 +1660,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);
>>                          }
>>                  }
>> @@ -1761,8 +1762,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);
>> +                               }
> 
> Drop these, I'd rather this test (gem_exec_schedule) outright fail and be
> forced to fix it up (it's a test I very much want to keep) then have it
> skip and overlook.
> 
>> 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);
> 
> Need to incorporate elsewhere. Or be satisfied with the essentials that
> we can allocate large objects. Now how large is a matter of debate, I
> think larger than memory but I haven't yet fixed the kernel.

Not sure what you mean here. Are you saying we should move the test, or 
just have a large object allocation test added to something else?

> 
>> +
>>          test_large_object(fd);
>>   
>>          free(data);
>> diff --git a/tests/i915/gem_madvise.c b/tests/i915/gem_madvise.c
>> index bcaaa22e..ab45aa39 100644
>> --- a/tests/i915/gem_madvise.c
>> +++ b/tests/i915/gem_madvise.c
>> @@ -74,6 +74,8 @@ dontneed_before_mmap(void)
>>                  switch (mode) {
>>                          case GTT:
> 
> Note for earlier patch, align case: to opening indent of switch().
> Also some earlier patches added some LOCALs that are already available
> from i915_drm.h
> 
>>                                  ptr = gem_mmap__gtt(fd, handle, OBJECT_SIZE, PROT_READ | PROT_WRITE);
>> +                               if (!gem_mmap__has_gtt(fd))
>> +                                       continue;
>>                                  break;
>>                          case CPU:
>>                                  ptr = gem_mmap__cpu(fd, handle, 0, OBJECT_SIZE, PROT_READ | PROT_WRITE);
>> @@ -118,6 +120,8 @@ dontneed_after_mmap(void)
>>                  switch (mode) {
>>                          case GTT:
>>                                  ptr = gem_mmap__gtt(fd, handle, OBJECT_SIZE, PROT_READ | PROT_WRITE);
>> +                               if (!gem_mmap__has_gtt(fd))
>> +                                       continue;
>>                                  break;
> 
> Order is important.

Right, I was going for !ptr at some point but then changed my mind.

Antonio

> -Chris
>