[igt-dev] [PATCH i-g-t] gem_exec_reloc & gem_shrink: Added gem_require_mappable_ggtt to check mappable aperture

[Tvrtko Ursulin]

On 30/11/2020 02:03, Arjun Melkaveri wrote:
> Added gem_require_mappable_ggtt to check mappable aperture.
> This is to avoid any test crash that might happen
> if mappable aperture is not avilable.
> 
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> Signed-off-by: Arjun Melkaveri <arjun.melkaveri at intel.com>
> ---
>   tests/i915/gem_exec_reloc.c | 3 +++
>   tests/i915/gem_shrink.c     | 2 ++
>   2 files changed, 5 insertions(+)
> 
> diff --git a/tests/i915/gem_exec_reloc.c b/tests/i915/gem_exec_reloc.c
> index 8dcb24a6..596b1157 100644
> --- a/tests/i915/gem_exec_reloc.c
> +++ b/tests/i915/gem_exec_reloc.c
> @@ -1217,6 +1217,7 @@ igt_main
>   			if (!(f->flags & NORELOC)) {
>   				igt_subtest_f("%srange%s",
>   					      f->basic ? "basic-" : "", f->name) {
> +					gem_require_mappable_ggtt(fd);

 From the code and commit message it is not immediately obvious to me 
why basic_range needs the aperture, Chris? Answer to that will drive the 
solution.

>   					igt_while_interruptible(f->flags & INTERRUPTIBLE)
>   						basic_range(fd, f->flags);
>   				}
> @@ -1264,6 +1265,7 @@ igt_main
>   	}
>   
>   	igt_subtest_with_dynamic("basic-spin") {
> +

!

>   		__for_each_physical_engine(fd, e) {
>   			igt_dynamic_f("%s", e->name)
>   				active_spin(fd, e->flags);
> @@ -1278,6 +1280,7 @@ igt_main
>   	}
>   
>   	igt_subtest_with_dynamic("basic-many-active") {
> +		gem_require_mappable_ggtt(fd);
>   		__for_each_physical_engine(fd, e) {
>   			igt_dynamic_f("%s", e->name)
>   				many_active(fd, e->flags);

Same for many_active - maybe tests should use ggtt size and not 
aperture, with some tweaks to make size/runtime sane?

> diff --git a/tests/i915/gem_shrink.c b/tests/i915/gem_shrink.c
> index dba62c8f..094975af 100644
> --- a/tests/i915/gem_shrink.c
> +++ b/tests/i915/gem_shrink.c
> @@ -432,6 +432,8 @@ igt_main
>   		fd = drm_open_driver(DRIVER_INTEL);
>   		igt_require_gem(fd);
>   
> +		gem_require_mappable_ggtt(fd);
> +
>   		/*
>   		 * Spawn enough processes to use all memory, but each only
>   		 * uses half the available mappable aperture ~128MiB.
> 

And for this one the same I think. Apart from the subtests using 
mmap-gtt other ones could probably be made work by calculating the 
working set in a different way. Possibly just use the ggtt size and skip 
tests which need aperture if no aperture. Chris would that be acceptable 
or there is a special reason to have N clients with each using an 
aperture sized amount of memory, to total RAM size?

Regards,

Tvrtko