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

[Tvrtko Ursulin]

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 at 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