Re: [igt-dev] [RFC v2 3/3] tests: gem_gem_query_engines_demo: create test

Tvrtko Ursulin
2018-11-22T12:25:18Z

On 21/11/2018 16:10, Andi Shyti wrote:
> The "gem_query_engines_demo" is a demo test used to show how to
> use the new API in commit 65d2f30d ("lib: implement new engine
> discovery interface").
>
> The new API has uses two new ioctls to interrogate the driver
> about the engines.
>
> The test simply gets the engines and executes a buffer.
>
> Signed-off-by: Andi Shyti <andi.shyti@intel.com>
> ---
> tests/Makefile.sources | 3 ++
> tests/i915/gem_query_engines_demo.c | 56 +++++++++++++++++++++++++++++
> tests/meson.build | 1 +
> 3 files changed, 60 insertions(+)
> create mode 100644 tests/i915/gem_query_engines_demo.c
>
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index 5620c1d6..e91d15f2 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -494,6 +494,9 @@ i915_selftest_SOURCES = i915/selftest.c
> TESTS_progs += i915_suspend
> i915_suspend_SOURCES = i915/suspend.c
>
> +TESTS_progs += gem_query_engines_demo
> +discover_engines_SOURCES = i915/gem_query_engines_demo.c
> +
> TESTS_progs_X = gem_concurrent_all
> gem_concurrent_all_SOURCES = i915/gem_concurrent_all.c
>
> diff --git a/tests/i915/gem_query_engines_demo.c b/tests/i915/gem_query_engines_demo.c
> new file mode 100644
> index 00000000..dc6b031e
> --- /dev/null
> +++ b/tests/i915/gem_query_engines_demo.c
> @@ -0,0 +1,56 @@
> +#include "igt.h"
> +#include <unistd.h>
> +#include <stdlib.h>
> +#include <stdint.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include <fcntl.h>
> +#include <inttypes.h>
> +#include <errno.h>
> +#include <sys/stat.h>
> +#include <sys/ioctl.h>
> +#include <sys/time.h>
> +#include <time.h>
> +#include "drm.h"

Feels like to many includes, evidence of copy & paste? :) And license
header needs to be added.

> +
> +static void execbuf_in_ctx(int fd, uint32_t engine, uint32_t ctx, uint32_t handle)

I think plain unsigned int for engine.

> +{
> + struct drm_i915_gem_exec_object2 exec = {
> + .handle = handle,
> + };
> + struct drm_i915_gem_execbuffer2 execbuf = {
> + .buffers_ptr = to_user_pointer(&exec),
> + .buffer_count = 1,
> + .flags = engine,
> + .rsvd1 = ctx & I915_EXEC_CONTEXT_ID_MASK,
> + };

Maybe add igt_assert(!(engine & I915_EXEC_RING_MASK)).

> +
> + gem_execbuf(fd, &execbuf);
> +}
> +
> +igt_main
> +{
> + int fd;
> + uint32_t ctx_id, e;
> + uint32_t handle;
> + uint32_t bbend = MI_BATCH_BUFFER_END;
> +
> + igt_fixture {
> + fd = drm_open_driver(DRIVER_INTEL);
> + ctx_id = gem_context_create(fd);
> +
> + handle = gem_create(fd, 4096);
> + gem_write(fd, handle, 0, &bbend, sizeof(bbend));
> + }
> +
> + igt_subtest("basic") {
> + for_each_engine_ctx(fd, ctx_id, e)
> + execbuf_in_ctx(fd, e, ctx_id, handle);
> + }
> +
> + igt_fixture {
> + gem_close(fd, handle);
> + gem_context_destroy(fd, ctx_id);
> + close(fd);
> + }
> +}
> diff --git a/tests/meson.build b/tests/meson.build
> index 52f91a22..0c091cf3 100644
> --- a/tests/meson.build
> +++ b/tests/meson.build
> @@ -217,6 +217,7 @@ gem_progs = [
> 'gem_wait',
> 'gem_workarounds',
> 'gem_write_read_ring_switch',
> + 'gem_query_engines_demo',
> ]
>
> gen3_progs = [
>

I'd be tempted to move this whole new test into gem_exec_basic. Either
to replace the current loop or add new noop based subtests. I suspect
Chris will have a stronger opinion on which option of the two.

Regards,

Tvrtko