Re: [igt-dev] [PATCH v16 7/8] lib: igt_dummyload: use for_each_context_engine()

Tvrtko Ursulin
2019-03-29T12:33:37Z

On 28/03/2019 19:22, Andi Shyti wrote:
> With the new getparam/setparam api, engines are mapped to
> context. Use for_each_context_engine() to loop through existing
> engines.
>
> Signed-off-by: Andi Shyti <andi.shyti@intel.com>
> ---
> lib/igt_dummyload.c | 21 ++++++++-------------
> 1 file changed, 8 insertions(+), 13 deletions(-)
>
> diff --git a/lib/igt_dummyload.c b/lib/igt_dummyload.c
> index 47f6b92b424b..4e94ccc448ba 100644
> --- a/lib/igt_dummyload.c
> +++ b/lib/igt_dummyload.c
> @@ -39,6 +39,7 @@
> #include "ioctl_wrappers.h"
> #include "sw_sync.h"
> #include "igt_vgem.h"
> +#include "i915/gem_engine_topology.h"
> #include "i915/gem_mman.h"
>
> /**
> @@ -86,7 +87,7 @@ emit_recursive_batch(igt_spin_t *spin,
> struct drm_i915_gem_relocation_entry relocs[2], *r;
> struct drm_i915_gem_execbuffer2 *execbuf;
> struct drm_i915_gem_exec_object2 *obj;
> - unsigned int engines[16];
> + unsigned int flags[16];

Bump it to I915_EXEC_RING_MASK + 1 while at it.

> unsigned int nengine;
> int fence_fd = -1;
> uint32_t *batch, *batch_start;
> @@ -94,17 +95,17 @@ emit_recursive_batch(igt_spin_t *spin,
>
> nengine = 0;
> if (opts->engine == ALL_ENGINES) {
> - unsigned int engine;
> + struct intel_execution_engine2 *engine;
>
> - for_each_physical_engine(fd, engine) {
> + for_each_context_engine(fd, opts->ctx, engine) {
> if (opts->flags & IGT_SPIN_POLL_RUN &&
> - !gem_can_store_dword(fd, engine))
> + !gem_engine_can_store_dword(fd, engine))
> continue;
>
> - engines[nengine++] = engine;
> + flags[nengine++] = engine->flags;
> }
> } else {
> - engines[nengine++] = opts->engine;
> + flags[nengine++] = opts->engine;

Single engine case needs the can_store_dword test as well.

You have to ioctl get_param.engines to figure out how to get engine
class. And then just call either gem_class_can_store_dword or
gem_can_store_dword depending on whether there is engine map on the
context or not.

If there is no engine map also do gem_require_ring here (it was lost
from the hunk below). In case of engine map I think you don't need to check.

Regards,

Tvrtko

> }
> igt_require(nengine);
>
> @@ -234,7 +235,7 @@ emit_recursive_batch(igt_spin_t *spin,
>
> for (i = 0; i < nengine; i++) {
> execbuf->flags &= ~ENGINE_MASK;
> - execbuf->flags |= engines[i];
> + execbuf->flags |= flags[i];
>
> gem_execbuf_wr(fd, execbuf);
>
> @@ -308,12 +309,6 @@ igt_spin_batch_factory(int fd, const struct igt_spin_factory *opts)
>
> igt_require_gem(fd);
>
> - if (opts->engine != ALL_ENGINES) {
> - gem_require_ring(fd, opts->engine);
> - if (opts->flags & IGT_SPIN_POLL_RUN)
> - igt_require(gem_can_store_dword(fd, opts->engine));
> - }
> -
> spin = spin_batch_create(fd, opts);
>
> igt_assert(gem_bo_busy(fd, spin->handle));