Re: [igt-dev] [PATCH i-g-t v6 6/9] igt/lib: Add wrapper to check if gtt mapping is available

[Antonio Argenziano <antonio.argenziano@intel.com>]

On 14/06/19 02:47, Chris Wilson wrote:
> Quoting Antonio Argenziano (2019-06-13 22:54:45)
>> Add wrapper to get mmap_gtt version number and another to check if
>> gtt mapping is at all available.
>>
>> v2:
>>          - Check errno only after failed syscall. (Chris)
>>
>> 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>
>> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>> ---
>>   lib/i915/gem_mman.c | 40 +++++++++++++++++++++++++++++++++++++++-
>>   lib/i915/gem_mman.h | 11 +++++++++++
>>   2 files changed, 50 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/i915/gem_mman.c b/lib/i915/gem_mman.c
>> index 0631bf40..2b3947de 100644
>> --- a/lib/i915/gem_mman.c
>> +++ b/lib/i915/gem_mman.c
>> @@ -40,6 +40,41 @@
>>   #define VG(x) do {} while (0)
>>   #endif
>>   
>> +/**
>> + * gem_mmap__gtt_version:
>> + * @fd: open i915 drm file descriptor
>> + *
>> + * This wraps I915_PARAM_MMAP_GTT_VERSION. It will return the supported feature
>> + * set for gtt mapping. Since the mappable aperture in not always present, this
>> + * function will return '-1' in case there is none.
>> + */
>> +static int gem_mmap__gtt_version(int fd)
>> +{
>> +       static int gtt_version = ~0;
> 
> Ahem.

Still there!?! I forgot to squash with the next patch.

> 
>> +       if (gtt_version == ~0) {
>> +                       struct drm_i915_getparam gp;
>> +
>> +                       gtt_version = 0;
>> +
>> +                       memset(&gp, 0, sizeof(gp));
>> +                       gp.param = I915_PARAM_MMAP_GTT_VERSION;
>> +                       gp.value = &gtt_version;
>> +
>> +                       if(ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp) && errno == -ENODEV)
>> +                               gtt_version = -1; /* No mappable aperture */
> 
> Hahaha, no?
> 
> MMAP_GTT == MMAP_OFFSET so when we extend MMAP_GTT ioctl with
> the MMAP_OFFSET struct, we bump the MMAP_GTT_VERSION

Wasn't my impression from the lmem patches I saw. Then, could we get an 
IOCTL for aperture existence or, even better, something for the actual 
aperture size that returns 0 so we could have a

bool gem_has_mappable_aperture() {
	return get_mappable_aperture_size() > 0;
}

which would make more sense.

Antonio

> -Chris
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev