| Message ID | 20190531083338.3029-1-janusz.krzysztofik@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [i-g-t,v3] lib/i915/gem_mman: Add a helper for obtaining mappable aperture version | expand |
Quoting Janusz Krzysztofik (2019-05-31 09:33:38) > From: Janusz Krzysztofik <janusz.krzysztofik@intel.com> > > If a test calls a function which depends on availabiblity of a > supported mappable aperture, an error may be reported by the kernel on > unsupported hardware. That may negatively affect results reported by a > test framework even if that test ignores the failure and succeedes. > > This helper wraps an IOCTL call which returns a version number of a > mappable aperture. It may be used by tests which need to adjust their > scope depending on availability of specific version of mappable > aperture. > > Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@intel.com> > Cc: Antonio Argenziano <antonio.argenziano@intel.com> > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> > --- > Changelog: > v2 (internal) -> v3: > - make the code less obsucre, more explicit (Antonio), > - reword the helper documentation and commit message. > > v1 (internal) -> v2 (internal): > - minimize future potential conflicts with > https://patchwork.freedesktop.org/patch/294053/?series=58551&rev=1 > (no progress with than one so not waiting for it any longer): > - convert the helper to a drop-in replacement of the one from the > above mentioned patch, returning mappable aperture version, not > only information on its availability, > - drop any other wrappers, > - document the helper, > - reword commit message. > > lib/i915/gem_mman.c | 22 ++++++++++++++++++++++ > lib/i915/gem_mman.h | 1 + > 2 files changed, 23 insertions(+) > > diff --git a/lib/i915/gem_mman.c b/lib/i915/gem_mman.c > index 3cf9a6bb..3a3f3e5c 100644 > --- a/lib/i915/gem_mman.c > +++ b/lib/i915/gem_mman.c > @@ -40,6 +40,28 @@ > #define VG(x) do {} while (0) > #endif > > +/** > + * gem_mmap__gtt_version: > + * @fd: open i915 drm file descriptor > + * > + * This functions wraps up an IOCTL to obtain mappable aperture version. > + * > + * Returns: mappable aperture version, -1 on failure. > + */ > +int gem_mmap__gtt_version(int fd) > +{ > + int gtt_version, ret; > + struct drm_i915_getparam gp = { > + .param = I915_PARAM_MMAP_GTT_VERSION, > + .value = >t_version, > + }; > + > + ret = ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp); > + if (ret == 0) > + ret = gtt_version; > + return ret; Maybe the actual error returned by the kernel and not glibc would be interesting in the future? -Chris
Quoting Janusz Krzysztofik (2019-05-31 09:33:38)
> From: Janusz Krzysztofik <janusz.krzysztofik@intel.com>
This is nothing to do with the mappable aperture version. This is the
nee MMAP_GTT interface version.
-Chris
Hi Chris, On Friday, May 31, 2019 10:39:47 AM CEST Chris Wilson wrote: > Quoting Janusz Krzysztofik (2019-05-31 09:33:38) > > From: Janusz Krzysztofik <janusz.krzysztofik@intel.com> > > > > If a test calls a function which depends on availabiblity of a > > supported mappable aperture, an error may be reported by the kernel on > > unsupported hardware. That may negatively affect results reported by a > > test framework even if that test ignores the failure and succeedes. > > > > This helper wraps an IOCTL call which returns a version number of a > > mappable aperture. It may be used by tests which need to adjust their > > scope depending on availability of specific version of mappable > > aperture. > > > > Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@intel.com> > > Cc: Antonio Argenziano <antonio.argenziano@intel.com> > > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> > > --- > > Changelog: > > v2 (internal) -> v3: > > - make the code less obsucre, more explicit (Antonio), > > - reword the helper documentation and commit message. > > > > v1 (internal) -> v2 (internal): > > - minimize future potential conflicts with > > https://patchwork.freedesktop.org/patch/294053/?series=58551&rev=1 > > (no progress with than one so not waiting for it any longer): > > - convert the helper to a drop-in replacement of the one from the > > above mentioned patch, returning mappable aperture version, not > > only information on its availability, > > - drop any other wrappers, > > - document the helper, > > - reword commit message. > > > > lib/i915/gem_mman.c | 22 ++++++++++++++++++++++ > > lib/i915/gem_mman.h | 1 + > > 2 files changed, 23 insertions(+) > > > > diff --git a/lib/i915/gem_mman.c b/lib/i915/gem_mman.c > > index 3cf9a6bb..3a3f3e5c 100644 > > --- a/lib/i915/gem_mman.c > > +++ b/lib/i915/gem_mman.c > > @@ -40,6 +40,28 @@ > > #define VG(x) do {} while (0) > > #endif > > > > +/** > > + * gem_mmap__gtt_version: > > + * @fd: open i915 drm file descriptor > > + * > > + * This functions wraps up an IOCTL to obtain mappable aperture version. > > + * > > + * Returns: mappable aperture version, -1 on failure. > > + */ > > +int gem_mmap__gtt_version(int fd) > > +{ > > + int gtt_version, ret; > > + struct drm_i915_getparam gp = { > > + .param = I915_PARAM_MMAP_GTT_VERSION, > > + .value = >t_version, > > + }; > > + > > + ret = ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp); > > + if (ret == 0) > > + ret = gtt_version; > > + return ret; > > Maybe the actual error returned by the kernel and not glibc would be > interesting in the future? errno is not overwritten by the helper so it is available to IGT after it is called and actually reported when a call to the helper is wrapped with igt_require(). Do we need more? Thanks, Janusz > -Chris >
On Friday, May 31, 2019 10:41:36 AM CEST Chris Wilson wrote: > Quoting Janusz Krzysztofik (2019-05-31 09:33:38) > > From: Janusz Krzysztofik <janusz.krzysztofik@intel.com> > > This is nothing to do with the mappable aperture version. This is the > nee MMAP_GTT interface version. > -Chris > Sorry for my ignorance, I'll reword it. Thanks, Janusz
Quoting Janusz Krzysztofik (2019-05-31 09:53:41) > Hi Chris, > > On Friday, May 31, 2019 10:39:47 AM CEST Chris Wilson wrote: > > Quoting Janusz Krzysztofik (2019-05-31 09:33:38) > > > From: Janusz Krzysztofik <janusz.krzysztofik@intel.com> > > > > > > If a test calls a function which depends on availabiblity of a > > > supported mappable aperture, an error may be reported by the kernel on > > > unsupported hardware. That may negatively affect results reported by a > > > test framework even if that test ignores the failure and succeedes. > > > > > > This helper wraps an IOCTL call which returns a version number of a > > > mappable aperture. It may be used by tests which need to adjust their > > > scope depending on availability of specific version of mappable > > > aperture. > > > > > > Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@intel.com> > > > Cc: Antonio Argenziano <antonio.argenziano@intel.com> > > > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> > > > --- > > > Changelog: > > > v2 (internal) -> v3: > > > - make the code less obsucre, more explicit (Antonio), > > > - reword the helper documentation and commit message. > > > > > > v1 (internal) -> v2 (internal): > > > - minimize future potential conflicts with > > > https://patchwork.freedesktop.org/patch/294053/?series=58551&rev=1 > > > (no progress with than one so not waiting for it any longer): > > > - convert the helper to a drop-in replacement of the one from the > > > above mentioned patch, returning mappable aperture version, not > > > only information on its availability, > > > - drop any other wrappers, > > > - document the helper, > > > - reword commit message. > > > > > > lib/i915/gem_mman.c | 22 ++++++++++++++++++++++ > > > lib/i915/gem_mman.h | 1 + > > > 2 files changed, 23 insertions(+) > > > > > > diff --git a/lib/i915/gem_mman.c b/lib/i915/gem_mman.c > > > index 3cf9a6bb..3a3f3e5c 100644 > > > --- a/lib/i915/gem_mman.c > > > +++ b/lib/i915/gem_mman.c > > > @@ -40,6 +40,28 @@ > > > #define VG(x) do {} while (0) > > > #endif > > > > > > +/** > > > + * gem_mmap__gtt_version: > > > + * @fd: open i915 drm file descriptor > > > + * > > > + * This functions wraps up an IOCTL to obtain mappable aperture version. > > > + * > > > + * Returns: mappable aperture version, -1 on failure. > > > + */ > > > +int gem_mmap__gtt_version(int fd) > > > +{ > > > + int gtt_version, ret; > > > + struct drm_i915_getparam gp = { > > > + .param = I915_PARAM_MMAP_GTT_VERSION, > > > + .value = >t_version, > > > + }; > > > + > > > + ret = ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp); > > > + if (ret == 0) > > > + ret = gtt_version; > > > + return ret; > > > > Maybe the actual error returned by the kernel and not glibc would be > > interesting in the future? > > errno is not overwritten by the helper so it is available to IGT after it is > called and actually reported when a call to the helper is wrapped with > igt_require(). Do we need more? Yes, we typically return the error and do not use errno. Imagine if we just replaced ioctl() with syscall() :) -Chris
On Friday, May 31, 2019 10:55:46 AM CEST Chris Wilson wrote: > Quoting Janusz Krzysztofik (2019-05-31 09:53:41) > > Hi Chris, > > > > On Friday, May 31, 2019 10:39:47 AM CEST Chris Wilson wrote: > > > Quoting Janusz Krzysztofik (2019-05-31 09:33:38) > > > > From: Janusz Krzysztofik <janusz.krzysztofik@intel.com> > > > > > > > > If a test calls a function which depends on availabiblity of a > > > > supported mappable aperture, an error may be reported by the kernel on > > > > unsupported hardware. That may negatively affect results reported by a > > > > test framework even if that test ignores the failure and succeedes. > > > > > > > > This helper wraps an IOCTL call which returns a version number of a > > > > mappable aperture. It may be used by tests which need to adjust their > > > > scope depending on availability of specific version of mappable > > > > aperture. > > > > > > > > Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@intel.com> > > > > Cc: Antonio Argenziano <antonio.argenziano@intel.com> > > > > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> > > > > --- > > > > Changelog: > > > > v2 (internal) -> v3: > > > > - make the code less obsucre, more explicit (Antonio), > > > > - reword the helper documentation and commit message. > > > > > > > > v1 (internal) -> v2 (internal): > > > > - minimize future potential conflicts with > > > > https://patchwork.freedesktop.org/patch/294053/?series=58551&rev=1 > > > > (no progress with than one so not waiting for it any longer): > > > > - convert the helper to a drop-in replacement of the one from the > > > > above mentioned patch, returning mappable aperture version, not > > > > only information on its availability, > > > > - drop any other wrappers, > > > > - document the helper, > > > > - reword commit message. > > > > > > > > lib/i915/gem_mman.c | 22 ++++++++++++++++++++++ > > > > lib/i915/gem_mman.h | 1 + > > > > 2 files changed, 23 insertions(+) > > > > > > > > diff --git a/lib/i915/gem_mman.c b/lib/i915/gem_mman.c > > > > index 3cf9a6bb..3a3f3e5c 100644 > > > > --- a/lib/i915/gem_mman.c > > > > +++ b/lib/i915/gem_mman.c > > > > @@ -40,6 +40,28 @@ > > > > #define VG(x) do {} while (0) > > > > #endif > > > > > > > > +/** > > > > + * gem_mmap__gtt_version: > > > > + * @fd: open i915 drm file descriptor > > > > + * > > > > + * This functions wraps up an IOCTL to obtain mappable aperture version. > > > > + * > > > > + * Returns: mappable aperture version, -1 on failure. > > > > + */ > > > > +int gem_mmap__gtt_version(int fd) > > > > +{ > > > > + int gtt_version, ret; > > > > + struct drm_i915_getparam gp = { > > > > + .param = I915_PARAM_MMAP_GTT_VERSION, > > > > + .value = >t_version, > > > > + }; > > > > + > > > > + ret = ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp); > > > > + if (ret == 0) > > > > + ret = gtt_version; > > > > + return ret; > > > > > > Maybe the actual error returned by the kernel and not glibc would be > > > interesting in the future? > > > > errno is not overwritten by the helper so it is available to IGT after it is > > called and actually reported when a call to the helper is wrapped with > > igt_require(). Do we need more? > > Yes, we typically return the error and do not use errno. Imagine if we > just replaced ioctl() with syscall() :) OK. I'll fix it. Thanks, Janusz > -Chris >
diff --git a/lib/i915/gem_mman.c b/lib/i915/gem_mman.c index 3cf9a6bb..3a3f3e5c 100644 --- a/lib/i915/gem_mman.c +++ b/lib/i915/gem_mman.c @@ -40,6 +40,28 @@ #define VG(x) do {} while (0) #endif +/** + * gem_mmap__gtt_version: + * @fd: open i915 drm file descriptor + * + * This functions wraps up an IOCTL to obtain mappable aperture version. + * + * Returns: mappable aperture version, -1 on failure. + */ +int gem_mmap__gtt_version(int fd) +{ + int gtt_version, ret; + struct drm_i915_getparam gp = { + .param = I915_PARAM_MMAP_GTT_VERSION, + .value = >t_version, + }; + + ret = ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp); + if (ret == 0) + ret = gtt_version; + return ret; +} + /** * __gem_mmap__gtt: * @fd: open i915 drm file descriptor diff --git a/lib/i915/gem_mman.h b/lib/i915/gem_mman.h index f7242ed7..ab12e566 100644 --- a/lib/i915/gem_mman.h +++ b/lib/i915/gem_mman.h @@ -25,6 +25,7 @@ #ifndef GEM_MMAN_H #define GEM_MMAN_H +int gem_mmap__gtt_version(int fd); void *gem_mmap__gtt(int fd, uint32_t handle, uint64_t size, unsigned prot); void *gem_mmap__cpu(int fd, uint32_t handle, uint64_t offset, uint64_t size, unsigned prot);