Re: [igt-dev] [RFT v4 6/6] igt/lib: If mappable aperture is missing return 0 size

[Katarzyna Dec <katarzyna.dec@intel.com>]

On Mon, Mar 25, 2019 at 04:20:43PM -0700, Antonio Argenziano wrote:
> So far the aperture size has been read directly from the bar,
> in this patch we return zero if the mappable aperture is not available
> as the value stored in the bar is not accurate. The patch also adds a
> 'require' when a call to gem_mappable_aperture_size() is made so that
> the aperture is guaranteed to exist before checking the size.
> 
> Cc: Katarzyna Dec <katarzyna.dec@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Matthew Auld <matthew.auld@intel.com>
> Signed-off-by: Antonio Argenziano <antonio.argenziano@intel.com>
> ---
>  lib/ioctl_wrappers.c       | 26 ++++++++++++++++++--------
>  lib/ioctl_wrappers.h       |  1 +
>  tests/i915/gem_cpu_reloc.c | 14 ++++++++++----
>  tests/prime_mmap.c         |  7 +++++--
>  4 files changed, 34 insertions(+), 14 deletions(-)
> 
> diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
> index a66eb4bc..c903d05c 100644
> --- a/lib/ioctl_wrappers.c
> +++ b/lib/ioctl_wrappers.c
> @@ -1089,6 +1089,22 @@ uint64_t gem_aperture_size(int fd)
>  	return aperture_size;
>  }
>  
> +static uint64_t __gem_mappable_aperture_size(int fd)
> +{
> +	struct pci_device *pci_dev = igt_device_get_pci_device(fd);
> +	int bar;
> +
> +	if (!gem_mmap__has_gtt(fd))
> +		return 0; /* Aperture not available */
> +
> +	if (intel_gen(pci_dev->device_id) < 3)
> +		bar = 0;
> +	else
> +		bar = 2;
> +
> +	return pci_dev->regions[bar].size;
> +}
> +
>  /**
>   * gem_mappable_aperture_size:
>   *
> @@ -1099,15 +1115,9 @@ uint64_t gem_aperture_size(int fd)
>   */
>  uint64_t gem_mappable_aperture_size(int fd)
>  {
> -	struct pci_device *pci_dev = igt_device_get_pci_device(fd);
> -	int bar;
> -
> -	if (intel_gen(pci_dev->device_id) < 3)
> -		bar = 0;
> -	else
> -		bar = 2;
> +	gem_require_mmap_gtt(fd);
>  
> -	return pci_dev->regions[bar].size;
> +	return __gem_mappable_aperture_size(fd);
>  }
>  
>  /**
> diff --git a/lib/ioctl_wrappers.h b/lib/ioctl_wrappers.h
> index ad93daff..b3b0313e 100644
> --- a/lib/ioctl_wrappers.h
> +++ b/lib/ioctl_wrappers.h
> @@ -38,6 +38,7 @@
>  
>  #include "i915/gem_context.h"
>  #include "i915/gem_scheduler.h"
> +#include "i915/gem_mman.h"
>  
>  /**
>   * igt_ioctl:
> diff --git a/tests/i915/gem_cpu_reloc.c b/tests/i915/gem_cpu_reloc.c
> index 17c2fe11..58fd4470 100644
> --- a/tests/i915/gem_cpu_reloc.c
> +++ b/tests/i915/gem_cpu_reloc.c
> @@ -283,8 +283,11 @@ igt_main
>  		run_test(i915, 1);
>  
>  	igt_subtest("full") {
> -		uint64_t aper_size = gem_mappable_aperture_size(i915);
> -		unsigned long count = aper_size / 4096 + 1;
> +		uint64_t aper_size;
> +		unsigned long count;
> +
> +		aper_size = gem_mappable_aperture_size(i915);
> +		count = aper_size / 4096 + 1;
Is this just a style change? Or I do not see something? Below as well.
It does seem to be reflected in commit msg.
Kasia
>  
>  		intel_require_memory(count, 4096, CHECK_RAM);
>  
> @@ -292,10 +295,13 @@ igt_main
>  	}
>  
>  	igt_subtest("forked") {
> -		uint64_t aper_size = gem_mappable_aperture_size(i915);
> -		unsigned long count = aper_size / 4096 + 1;
> +		uint64_t aper_size;
> +		unsigned long count;
>  		int ncpus = sysconf(_SC_NPROCESSORS_ONLN);
>  
> +		aper_size = gem_mappable_aperture_size(i915);
> +		count = aper_size / 4096 + 1;
> +
>  		intel_require_memory(count, 4096, CHECK_RAM);
>  
>  		igt_fork(child, ncpus)
> diff --git a/tests/prime_mmap.c b/tests/prime_mmap.c
> index f28985da..c844effc 100644
> --- a/tests/prime_mmap.c
> +++ b/tests/prime_mmap.c
> @@ -480,8 +480,11 @@ test_aperture_limit(void)
>  	char *ptr1, *ptr2;
>  	uint32_t handle1, handle2;
>  	/* Two buffers the sum of which > mappable aperture */
> -	uint64_t size1 = (gem_mappable_aperture_size(fd) * 7) / 8;
> -	uint64_t size2 = (gem_mappable_aperture_size(fd) * 3) / 8;
> +	uint64_t size1;
> +	uint64_t size2;
> +
> +	size1 = (gem_mappable_aperture_size(fd) * 7) / 8;
> +	size2 = (gem_mappable_aperture_size(fd) * 3) / 8;
>  
>  	handle1 = gem_create(fd, size1);
>  	fill_bo(handle1, BO_SIZE);
> -- 
> 2.20.1
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev