[igt-dev] [PATCH i-g-t] lib: Move __gem_context_create to common ioctl wrapper library.
Antonio Argenziano
antonio.argenziano at intel.com
Fri Feb 2 18:29:04 UTC 2018
On 02/02/18 10:25, Antonio Argenziano wrote:
> This patch adds a context creation ioctl wrapper that returns the error
> for the caller to consume. Multiple tests that implemented this already,
> have been changed to use the new library function.
>
> v2:
> - Add gem_require_contexts() to check for contexts support (Chris)
>
> v3:
> - Add gem_has_contexts to check for contexts support and change
> gem_require_contexts to skip if contests support is not available.
> (Chris)
>
> v4:
> - Cosmetic changes and use lib function in gem_ctx_create where
> possible. (Michal)
>
> Signed-off-by: Antonio Argenziano <antonio.argenziano at intel.com>
>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Michał Winiarski <michal.winiarski at intel.com>
>
> Reviewed-by: Michał Winiarski <michal.winiarski at intel.com>
Resending after rebasing. Hopefully I haven't missed any new code that
needed changing, another look from someone would be appreciated.
Thanks,
Antonio
> ---
> benchmarks/gem_exec_ctx.c | 6 ++---
> benchmarks/gem_exec_trace.c | 4 +--
> lib/i915/gem_context.c | 61 +++++++++++++++++++++++++++++++++++++--------
> lib/i915/gem_context.h | 5 ++++
> tests/gem_ctx_create.c | 13 +++++-----
> tests/gem_ctx_switch.c | 13 ----------
> tests/gem_eio.c | 13 +---------
> tests/gem_exec_await.c | 14 ++---------
> tests/gem_exec_nop.c | 13 ----------
> tests/gem_exec_parallel.c | 15 +++--------
> tests/gem_exec_reuse.c | 13 ----------
> tests/gem_exec_whisper.c | 13 ----------
> 12 files changed, 74 insertions(+), 109 deletions(-)
>
> diff --git a/benchmarks/gem_exec_ctx.c b/benchmarks/gem_exec_ctx.c
> index 0eac04b0..a1c6e5d7 100644
> --- a/benchmarks/gem_exec_ctx.c
> +++ b/benchmarks/gem_exec_ctx.c
> @@ -64,7 +64,7 @@ static uint32_t batch(int fd)
> return handle;
> }
>
> -static uint32_t __gem_context_create(int fd)
> +static uint32_t __gem_context_create_local(int fd)
> {
> struct drm_i915_gem_context_create create;
>
> @@ -101,7 +101,7 @@ static int loop(unsigned ring,
> execbuf.flags |= LOCAL_I915_EXEC_HANDLE_LUT;
> execbuf.flags |= LOCAL_I915_EXEC_NO_RELOC;
> if (mode != DEFAULT) {
> - execbuf.rsvd1 = __gem_context_create(fd);
> + execbuf.rsvd1 = __gem_context_create_local(fd);
> if (execbuf.rsvd1 == 0)
> return 77;
> }
> @@ -125,7 +125,7 @@ static int loop(unsigned ring,
> uint32_t ctx = 0;
>
> if (mode != DEFAULT && mode != NOP) {
> - execbuf.rsvd1 = __gem_context_create(fd);
> + execbuf.rsvd1 = __gem_context_create_local(fd);
> ctx = gem_context_create(fd);
> }
>
> diff --git a/benchmarks/gem_exec_trace.c b/benchmarks/gem_exec_trace.c
> index 12577649..2724ee92 100644
> --- a/benchmarks/gem_exec_trace.c
> +++ b/benchmarks/gem_exec_trace.c
> @@ -105,7 +105,7 @@ static double elapsed(const struct timespec *start, const struct timespec *end)
> return 1e3*(end->tv_sec - start->tv_sec) + 1e-6*(end->tv_nsec - start->tv_nsec);
> }
>
> -static uint32_t __gem_context_create(int fd)
> +static uint32_t __gem_context_create_local(int fd)
> {
> struct drm_i915_gem_context_create arg = {};
> drmIoctl(fd, DRM_IOCTL_I915_GEM_CONTEXT_CREATE, &arg);
> @@ -216,7 +216,7 @@ static double replay(const char *filename, long nop, long range)
> num_ctx = new_ctx;
> }
>
> - ctx[t->handle] = __gem_context_create(fd);
> + ctx[t->handle] = __gem_context_create_local(fd);
> break;
> }
> case DEL_CTX:
> diff --git a/lib/i915/gem_context.c b/lib/i915/gem_context.c
> index 8f383a97..ad2032e1 100644
> --- a/lib/i915/gem_context.c
> +++ b/lib/i915/gem_context.c
> @@ -43,6 +43,52 @@
> * software features improving submission model (context priority).
> */
>
> +/**
> + * gem_has_contexts:
> + * @fd: open i915 drm file descriptor
> + *
> + * Queries whether context creation is supported or not.
> + *
> + * Returns: Context creation availability.
> + */
> +bool gem_has_contexts(int fd)
> +{
> + uint32_t ctx_id = 0;
> +
> + __gem_context_create(fd, &ctx_id);
> + if (ctx_id)
> + gem_context_destroy(fd, ctx_id);
> +
> + return ctx_id;
> +}
> +
> +/**
> + * gem_require_contexts:
> + * @fd: open i915 drm file descriptor
> + *
> + * This helper will automatically skip the test on platforms where context
> + * support is not available.
> + */
> +void gem_require_contexts(int fd)
> +{
> + igt_require(gem_has_contexts(fd));
> +}
> +
> +int __gem_context_create(int fd, uint32_t *ctx_id)
> +{
> + struct drm_i915_gem_context_create create;
> + int err = 0;
> +
> + memset(&create, 0, sizeof(create));
> + if (igt_ioctl(fd, DRM_IOCTL_I915_GEM_CONTEXT_CREATE, &create) == 0)
> + *ctx_id = create.ctx_id;
> + else
> + err = -errno;
> +
> + errno = 0;
> + return err;
> +}
> +
> /**
> * gem_context_create:
> * @fd: open i915 drm file descriptor
> @@ -55,18 +101,13 @@
> */
> uint32_t gem_context_create(int fd)
> {
> - struct drm_i915_gem_context_create create;
> + uint32_t ctx_id;
> + gem_require_contexts(fd);
>
> - memset(&create, 0, sizeof(create));
> - if (igt_ioctl(fd, DRM_IOCTL_I915_GEM_CONTEXT_CREATE, &create)) {
> - int err = -errno;
> - igt_skip_on(err == -ENODEV || errno == -EINVAL);
> - igt_assert_eq(err, 0);
> - }
> - igt_assert(create.ctx_id != 0);
> - errno = 0;
> + igt_assert_eq(__gem_context_create(fd, &ctx_id), 0);
> + igt_assert(ctx_id != 0);
>
> - return create.ctx_id;
> + return ctx_id;
> }
>
> int __gem_context_destroy(int fd, uint32_t ctx_id)
> diff --git a/lib/i915/gem_context.h b/lib/i915/gem_context.h
> index 96106b71..aef68dda 100644
> --- a/lib/i915/gem_context.h
> +++ b/lib/i915/gem_context.h
> @@ -25,10 +25,15 @@
> #define GEM_CONTEXT_H
>
> uint32_t gem_context_create(int fd);
> +int __gem_context_create(int fd, uint32_t *ctx_id);
> void gem_context_destroy(int fd, uint32_t ctx_id);
> int __gem_context_destroy(int fd, uint32_t ctx_id);
> +
> +bool gem_has_contexts(int fd);
> +void gem_require_contexts(int fd);
> void gem_context_require_bannable(int fd);
> void gem_context_require_param(int fd, uint64_t param);
> +
> void gem_context_get_param(int fd, struct drm_i915_gem_context_param *p);
> void gem_context_set_param(int fd, struct drm_i915_gem_context_param *p);
> int __gem_context_set_param(int fd, struct drm_i915_gem_context_param *p);
> diff --git a/tests/gem_ctx_create.c b/tests/gem_ctx_create.c
> index ae0825a1..6a3a5fe8 100644
> --- a/tests/gem_ctx_create.c
> +++ b/tests/gem_ctx_create.c
> @@ -45,7 +45,7 @@ static unsigned all_nengine;
> static unsigned ppgtt_engines[16];
> static unsigned ppgtt_nengine;
>
> -static int __gem_context_create(int fd, struct drm_i915_gem_context_create *arg)
> +static int __gem_context_create_local(int fd, struct drm_i915_gem_context_create *arg)
> {
> int ret = 0;
> if (drmIoctl(fd, DRM_IOCTL_I915_GEM_CONTEXT_CREATE, arg))
> @@ -241,6 +241,7 @@ static void maximum(int fd, int ncpus, unsigned mode)
> unsigned ctx_size = context_size(fd);
> uint32_t *contexts = NULL;
> unsigned long count = 0;
> + uint32_t ctx_id;
>
> memset(&create, 0, sizeof(create));
> do {
> @@ -255,14 +256,14 @@ static void maximum(int fd, int ncpus, unsigned mode)
>
> err = -ENOMEM;
> if (avail_mem > (count + 1) * ctx_size)
> - err = __gem_context_create(fd, &create);
> + err = __gem_context_create(fd, &ctx_id);
> if (err) {
> igt_info("Created %lu contexts, before failing with '%s' [%d]\n",
> count, strerror(-err), -err);
> break;
> }
>
> - contexts[count++] = create.ctx_id;
> + contexts[count++] = ctx_id;
> } while (1);
> igt_require(count);
>
> @@ -322,7 +323,7 @@ igt_main
> igt_require_gem(fd);
>
> memset(&create, 0, sizeof(create));
> - igt_require(__gem_context_create(fd, &create) == 0);
> + igt_require(__gem_context_create_local(fd, &create) == 0);
> gem_context_destroy(fd, create.ctx_id);
>
> for_each_engine(fd, engine) {
> @@ -347,7 +348,7 @@ igt_main
> memset(&create, 0, sizeof(create));
> create.ctx_id = rand();
> create.pad = 0;
> - igt_assert_eq(__gem_context_create(fd, &create), 0);
> + igt_assert_eq(__gem_context_create_local(fd, &create), 0);
> igt_assert(create.ctx_id != 0);
> gem_context_destroy(fd, create.ctx_id);
> }
> @@ -356,7 +357,7 @@ igt_main
> memset(&create, 0, sizeof(create));
> create.ctx_id = rand();
> create.pad = 1;
> - igt_assert_eq(__gem_context_create(fd, &create), -EINVAL);
> + igt_assert_eq(__gem_context_create_local(fd, &create), -EINVAL);
> }
>
> igt_subtest("maximum-mem")
> diff --git a/tests/gem_ctx_switch.c b/tests/gem_ctx_switch.c
> index fdd67202..285fc1a1 100644
> --- a/tests/gem_ctx_switch.c
> +++ b/tests/gem_ctx_switch.c
> @@ -45,19 +45,6 @@
>
> #define INTERRUPTIBLE 1
>
> -static int __gem_context_create(int fd, uint32_t *ctx_id)
> -{
> - struct drm_i915_gem_context_create arg;
> - int ret = 0;
> -
> - memset(&arg, 0, sizeof(arg));
> - if (drmIoctl(fd, DRM_IOCTL_I915_GEM_CONTEXT_CREATE, &arg))
> - ret = -errno;
> -
> - *ctx_id = arg.ctx_id;
> - return ret;
> -}
> -
> static double elapsed(const struct timespec *start, const struct timespec *end)
> {
> return ((end->tv_sec - start->tv_sec) +
> diff --git a/tests/gem_eio.c b/tests/gem_eio.c
> index 6d5a10f2..430bfac5 100644
> --- a/tests/gem_eio.c
> +++ b/tests/gem_eio.c
> @@ -273,17 +273,6 @@ static void test_inflight_suspend(int fd)
> trigger_reset(fd);
> }
>
> -static uint32_t __gem_context_create(int fd)
> -{
> - struct drm_i915_gem_context_create create;
> -
> - memset(&create, 0, sizeof(create));
> - if (ioctl(fd, DRM_IOCTL_I915_GEM_CONTEXT_CREATE, &create))
> - return 0;
> -
> - return create.ctx_id;
> -}
> -
> static void test_inflight_contexts(int fd)
> {
> struct drm_i915_gem_execbuffer2 execbuf;
> @@ -295,7 +284,7 @@ static void test_inflight_contexts(int fd)
>
> igt_require(gem_has_exec_fence(fd));
>
> - ctx[0] = __gem_context_create(fd);
> + ctx[0] = gem_context_create(fd);
> igt_require(ctx[0]);
> for (unsigned int n = 1; n < ARRAY_SIZE(ctx); n++)
> ctx[n] = gem_context_create(fd);
> diff --git a/tests/gem_exec_await.c b/tests/gem_exec_await.c
> index 28b280ff..133492a7 100644
> --- a/tests/gem_exec_await.c
> +++ b/tests/gem_exec_await.c
> @@ -55,15 +55,6 @@ static bool ignore_engine(int fd, unsigned engine)
> return false;
> }
>
> -static uint32_t __gem_context_create(int fd)
> -{
> - struct drm_i915_gem_context_create arg;
> -
> - memset(&arg, 0, sizeof(arg));
> - drmIoctl(fd, DRM_IOCTL_I915_GEM_CONTEXT_CREATE, &arg);
> - return arg.ctx_id;
> -}
> -
> static void xchg_obj(void *array, unsigned i, unsigned j)
> {
> struct drm_i915_gem_exec_object2 *obj = array;
> @@ -130,8 +121,7 @@ static void wide(int fd, int ring_size, int timeout, unsigned int flags)
> LOCAL_I915_EXEC_HANDLE_LUT);
>
> if (flags & CONTEXTS) {
> - exec[e].execbuf.rsvd1 = __gem_context_create(fd);
> - igt_require(exec[e].execbuf.rsvd1);
> + exec[e].execbuf.rsvd1 = gem_context_create(fd);
> }
>
> exec[e].exec[0].handle = gem_create(fd, 4096);
> @@ -174,7 +164,7 @@ static void wide(int fd, int ring_size, int timeout, unsigned int flags)
>
> if (flags & CONTEXTS) {
> gem_context_destroy(fd, exec[e].execbuf.rsvd1);
> - exec[e].execbuf.rsvd1 = __gem_context_create(fd);
> + exec[e].execbuf.rsvd1 = gem_context_create(fd);
> }
>
> exec[e].reloc.presumed_offset = exec[e].exec[1].offset;
> diff --git a/tests/gem_exec_nop.c b/tests/gem_exec_nop.c
> index 668dcb94..107195a7 100644
> --- a/tests/gem_exec_nop.c
> +++ b/tests/gem_exec_nop.c
> @@ -357,19 +357,6 @@ static void xchg(void *array, unsigned i, unsigned j)
> u[j] = tmp;
> }
>
> -static int __gem_context_create(int fd, uint32_t *ctx_id)
> -{
> - struct drm_i915_gem_context_create arg;
> - int ret = 0;
> -
> - memset(&arg, 0, sizeof(arg));
> - if (drmIoctl(fd, DRM_IOCTL_I915_GEM_CONTEXT_CREATE, &arg))
> - ret = -errno;
> -
> - *ctx_id = arg.ctx_id;
> - return ret;
> -}
> -
> static void sequential(int fd, uint32_t handle, unsigned flags, int timeout)
> {
> const int ncpus = flags & FORKED ? sysconf(_SC_NPROCESSORS_ONLN) : 1;
> diff --git a/tests/gem_exec_parallel.c b/tests/gem_exec_parallel.c
> index 1c53bd64..5c0b027b 100644
> --- a/tests/gem_exec_parallel.c
> +++ b/tests/gem_exec_parallel.c
> @@ -55,20 +55,11 @@ static void check_bo(int fd, uint32_t handle, int pass)
> munmap(map, 4096);
> }
>
> -static uint32_t __gem_context_create(int fd)
> -{
> - struct drm_i915_gem_context_create arg;
> -
> - memset(&arg, 0, sizeof(arg));
> - if (drmIoctl(fd, DRM_IOCTL_I915_GEM_CONTEXT_CREATE, &arg) == 0)
> - gem_context_destroy(fd, arg.ctx_id);
> -
> - return arg.ctx_id;
> -}
> -
> static void gem_require_context(int fd)
> {
> - igt_require(__gem_context_create(fd));
> + uint32_t ctx_id = 0;
> + __gem_context_create(fd, &ctx_id);
> + igt_require(ctx_id);
> }
>
> static bool ignore_engine(int fd, unsigned engine)
> diff --git a/tests/gem_exec_reuse.c b/tests/gem_exec_reuse.c
> index 4e3907cf..11185de1 100644
> --- a/tests/gem_exec_reuse.c
> +++ b/tests/gem_exec_reuse.c
> @@ -56,19 +56,6 @@ static void noop(struct noop *n,
> gem_execbuf(n->fd, &execbuf);
> }
>
> -static int __gem_context_create(int fd, uint32_t *ctx_id)
> -{
> - struct drm_i915_gem_context_create arg;
> - int ret = 0;
> -
> - memset(&arg, 0, sizeof(arg));
> - if (drmIoctl(fd, DRM_IOCTL_I915_GEM_CONTEXT_CREATE, &arg))
> - ret = -errno;
> -
> - *ctx_id = arg.ctx_id;
> - return ret;
> -}
> -
> static int fls(uint64_t x)
> {
> int t;
> diff --git a/tests/gem_exec_whisper.c b/tests/gem_exec_whisper.c
> index 51921ba3..90527b54 100644
> --- a/tests/gem_exec_whisper.c
> +++ b/tests/gem_exec_whisper.c
> @@ -79,19 +79,6 @@ static void verify_reloc(int fd, uint32_t handle,
> }
> }
>
> -static int __gem_context_create(int fd, uint32_t *ctx_id)
> -{
> - struct drm_i915_gem_context_create arg;
> - int ret = 0;
> -
> - memset(&arg, 0, sizeof(arg));
> - if (drmIoctl(fd, DRM_IOCTL_I915_GEM_CONTEXT_CREATE, &arg))
> - ret = -errno;
> -
> - *ctx_id = arg.ctx_id;
> - return ret;
> -}
> -
> static bool ignore_engine(int fd, unsigned engine)
> {
> if (engine == 0)
>
More information about the igt-dev
mailing list