On 22/02/2018 07:52, Chris Wilson wrote:
We current have a single for_each_engine() iterator which we use to
generate both a set of uABI engines and a set of physical engines.
Determining what uABI ring-id corresponds to an actual HW engine is
tricky, so pull that out to a library function and introduce
for_each_physical_engine() for cases where we want to issue requests
once on each HW ring (avoiding aliasing issues).

v2: Remember can_store_dword for gem_sync
v3: Find more open-coded for_each_physical

Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursu...@intel.com>
---
  lib/igt_gt.c               |  23 +++++++++
  lib/igt_gt.h               |   9 ++++
  tests/amdgpu/amd_prime.c   |   6 +--
  tests/drv_hangman.c        |  20 ++------
  tests/gem_busy.c           |  31 ++++++------
  tests/gem_concurrent_all.c |   2 +-
  tests/gem_ctx_create.c     |   5 +-
  tests/gem_ctx_thrash.c     |  37 +++-----------
  tests/gem_exec_async.c     |  15 +++---
  tests/gem_exec_await.c     |  17 +------
  tests/gem_exec_capture.c   |   2 +-
  tests/gem_exec_create.c    |  17 +------
  tests/gem_exec_fence.c     |  16 ++----
  tests/gem_exec_gttfill.c   |  16 +-----
  tests/gem_exec_latency.c   |  19 +++----
  tests/gem_exec_nop.c       |  32 ++----------
  tests/gem_exec_parallel.c  |  15 +-----
  tests/gem_exec_reloc.c     |   2 +-
  tests/gem_exec_schedule.c  |  31 ++++--------
  tests/gem_exec_store.c     |   2 +-
  tests/gem_exec_suspend.c   |  20 ++------
  tests/gem_exec_whisper.c   |  15 +-----
  tests/gem_ring_sync_loop.c |   2 +-
  tests/gem_spin_batch.c     |   5 +-
  tests/gem_sync.c           | 123 ++++++++-------------------------------------
  25 files changed, 124 insertions(+), 358 deletions(-)

diff --git a/lib/igt_gt.c b/lib/igt_gt.c
index f70fcb92..e630550b 100644
--- a/lib/igt_gt.c
+++ b/lib/igt_gt.c
@@ -660,3 +660,26 @@ bool gem_has_engine(int gem_fd,
                            gem_class_instance_to_eb_flags(gem_fd, class,
                                                           instance));
  }
+
+bool gem_ring_is_physical_engine(int fd, unsigned ring)
+{
+       if (ring == I915_EXEC_DEFAULT)
+               return false;
+
+       /* BSD uses an extra flag to chose between aliasing modes */
+       if ((ring & 63) == I915_EXEC_BSD) {
+               bool explicit_bsd = ring & (3 << 13);
+               bool has_bsd2 = gem_has_bsd2(fd);
+               return explicit_bsd ? has_bsd2 : !has_bsd2;
+       }
+
+       return true;
+}
+
+bool gem_ring_has_physical_engine(int fd, unsigned ring)
+{
+       if (!gem_ring_is_physical_engine(fd, ring))
+               return false;
+
+       return gem_has_ring(fd, ring);
+}
diff --git a/lib/igt_gt.h b/lib/igt_gt.h
index 68592410..4d9d1aa0 100644
--- a/lib/igt_gt.h
+++ b/lib/igt_gt.h
@@ -81,6 +81,15 @@ extern const struct intel_execution_engine {
             e__++) \
                for_if (gem_has_ring(fd__, flags__ = e__->exec_id | e__->flags))
+#define for_each_physical_engine(fd__, flags__) \
+       for (const struct intel_execution_engine *e__ = 
intel_execution_engines;\
+            e__->name; \
+            e__++) \
+               for_if (gem_ring_has_physical_engine(fd__, flags__ = e__->exec_id 
| e__->flags))
+
+bool gem_ring_is_physical_engine(int fd, unsigned int ring);
+bool gem_ring_has_physical_engine(int fd, unsigned int ring);
+
  bool gem_can_store_dword(int fd, unsigned int engine);
extern const struct intel_execution_engine2 {
diff --git a/tests/amdgpu/amd_prime.c b/tests/amdgpu/amd_prime.c
index b2f326b4..bb68ccf3 100644
--- a/tests/amdgpu/amd_prime.c
+++ b/tests/amdgpu/amd_prime.c
@@ -179,12 +179,8 @@ static void i915_to_amd(int i915, int amd, 
amdgpu_device_handle device)
        struct cork c;
nengine = 0;
-       for_each_engine(i915, engine) {
-               if (engine == 0)
-                       continue;
-
+       for_each_physical_engine(i915, engine)
                engines[nengine++] = engine;
-       }
        igt_require(nengine);
memset(obj, 0, sizeof(obj));
diff --git a/tests/drv_hangman.c b/tests/drv_hangman.c
index 40c82257..38cb20c3 100644
--- a/tests/drv_hangman.c
+++ b/tests/drv_hangman.c
@@ -183,8 +183,6 @@ static void test_error_state_capture(unsigned ring_id,
        igt_hang_t hang;
        uint64_t offset;
- igt_require(gem_has_ring(device, ring_id));
-
        clear_error_state();
hang = igt_hang_ctx(device, 0, ring_id, HANG_ALLOW_CAPTURE, &offset);
@@ -255,23 +253,11 @@ igt_main
                if (e->exec_id == 0)
                        continue;
- /*
-                * If the device has 2 BSD rings then due to obtuse aliasing
-                * in the API, we can not determine which ring I915_EXEC_BSD
-                * will map to, and so must skip the test; as the matching name
-                * may be either bsd or bsd2 depending on the kernel/test
-                * ordering.
-                *
-                * Here we are not checking that executing on every ABI engine
-                * results in a detectable hang, but that a hang generated
-                * from a specific HW engine gives an indentifiable result.
-                */
-               if (e->exec_id == I915_EXEC_BSD && e->flags == 0)
-                       continue;
-
-               igt_subtest_f("error-state-capture-%s", e->name)
+               igt_subtest_f("error-state-capture-%s", e->name) {
+                       igt_require(gem_ring_has_physical_engine(device, 
e->exec_id | e->flags));
                        test_error_state_capture(e->exec_id | e->flags,
                                                 e->full_name);
+               }
        }
igt_subtest("hangcheck-unterminated")
diff --git a/tests/gem_busy.c b/tests/gem_busy.c
index c349c291..49cb1c95 100644
--- a/tests/gem_busy.c
+++ b/tests/gem_busy.c
@@ -156,7 +156,7 @@ static void semaphore(int fd, unsigned ring, uint32_t flags)
#define PARALLEL 1
  #define HANG 2
-static void one(int fd, unsigned ring, uint32_t flags, unsigned test_flags)
+static void one(int fd, unsigned ring, unsigned test_flags)
  {
        const int gen = intel_gen(intel_get_drm_devid(fd));
        struct drm_i915_gem_exec_object2 obj[2];
@@ -173,7 +173,7 @@ static void one(int fd, unsigned ring, uint32_t flags, 
unsigned test_flags)
        memset(&execbuf, 0, sizeof(execbuf));
        execbuf.buffers_ptr = to_user_pointer(obj);
        execbuf.buffer_count = 2;
-       execbuf.flags = ring | flags;
+       execbuf.flags = ring;
        if (gen < 6)
                execbuf.flags |= I915_EXEC_SECURE;
@@ -245,20 +245,17 @@ static void one(int fd, unsigned ring, uint32_t flags, unsigned test_flags)
        __gem_busy(fd, obj[BATCH].handle, &read[BATCH], &write[BATCH]);
if (test_flags & PARALLEL) {
-               const struct intel_execution_engine *e;
-
-               for (e = intel_execution_engines; e->name; e++) {
-                       if (e->exec_id == 0 || e->exec_id == ring)
-                               continue;
+               unsigned other;
- if (!gem_has_ring(fd, e->exec_id | e->flags))
+               for_each_physical_engine(fd, other) {
+                       if (other == ring)
                                continue;
- if (!gem_can_store_dword(fd, e->exec_id | e->flags))
+                       if (!gem_can_store_dword(fd, other))
                                continue;
- igt_debug("Testing %s in parallel\n", e->name);
-                       one(fd, e->exec_id, e->flags, 0);
+                       igt_debug("Testing %s in parallel\n", e__->name);
+                       one(fd, other, 0);
                }
        }
@@ -591,10 +588,10 @@ igt_main
                                        continue;
igt_subtest_f("extended-%s", e->name) {
-                                       gem_require_ring(fd, e->exec_id | 
e->flags);
+                                       igt_require(gem_ring_has_physical_engine(fd, 
e->exec_id | e->flags));
                                        igt_require(gem_can_store_dword(fd, 
e->exec_id | e->flags));
                                        gem_quiescent_gpu(fd);
-                                       one(fd, e->exec_id, e->flags, 0);
+                                       one(fd, e->exec_id | e->flags, 0);
                                        gem_quiescent_gpu(fd);
                                }
                        }
@@ -605,11 +602,11 @@ igt_main
                                        continue;
igt_subtest_f("extended-parallel-%s", e->name) {
-                                       gem_require_ring(fd, e->exec_id | 
e->flags);
+                                       igt_require(gem_ring_has_physical_engine(fd, 
e->exec_id | e->flags));
                                        igt_require(gem_can_store_dword(fd, 
e->exec_id | e->flags));
gem_quiescent_gpu(fd);
-                                       one(fd, e->exec_id, e->flags, PARALLEL);
+                                       one(fd, e->exec_id | e->flags, 
PARALLEL);
                                        gem_quiescent_gpu(fd);
                                }
                        }
@@ -670,11 +667,11 @@ igt_main
igt_subtest_f("extended-hang-%s", e->name) {
                                        igt_skip_on_simulation();
-                                       gem_require_ring(fd, e->exec_id | 
e->flags);
+                                       igt_require(gem_ring_has_physical_engine(fd, 
e->exec_id | e->flags));
                                        igt_require(gem_can_store_dword(fd, 
e->exec_id | e->flags));
gem_quiescent_gpu(fd);
-                                       one(fd, e->exec_id, e->flags, HANG);
+                                       one(fd, e->exec_id | e->flags, HANG);
                                        gem_quiescent_gpu(fd);
                                }
                        }
diff --git a/tests/gem_concurrent_all.c b/tests/gem_concurrent_all.c
index 201b491b..3a1097ba 100644
--- a/tests/gem_concurrent_all.c
+++ b/tests/gem_concurrent_all.c
@@ -960,7 +960,7 @@ static igt_hang_t all_hang(void)
        execbuf.buffers_ptr = to_user_pointer(&obj);
        execbuf.buffer_count = 1;
- for_each_engine(fd, engine) {
+       for_each_physical_engine(fd, engine) {
                hang = igt_hang_ring(fd, engine);
execbuf.flags = engine;
diff --git a/tests/gem_ctx_create.c b/tests/gem_ctx_create.c
index 058445b6..1b32d6c3 100644
--- a/tests/gem_ctx_create.c
+++ b/tests/gem_ctx_create.c
@@ -321,11 +321,8 @@ igt_main
                igt_require_gem(fd);
                gem_require_contexts(fd);
- for_each_engine(fd, engine) {
-                       if (engine == 0)
-                               continue;
+               for_each_physical_engine(fd, engine)
                        all_engines[all_nengine++] = engine;
-               }
                igt_require(all_nengine);
if (gem_uses_full_ppgtt(fd)) {
diff --git a/tests/gem_ctx_thrash.c b/tests/gem_ctx_thrash.c
index b5d334f4..23203158 100644
--- a/tests/gem_ctx_thrash.c
+++ b/tests/gem_ctx_thrash.c
@@ -130,22 +130,9 @@ static void single(const char *name, bool all_engines)
num_engines = 0;
        if (all_engines) {
-               for (e = intel_execution_engines; e->name; e++) {
-                       if (e->exec_id == 0)
-                               continue;
-
-                       if (has_engine(fd, e, 0))
-                               continue;
-
-                       if (e->exec_id == I915_EXEC_BSD) {
-                               int is_bsd2 = e->flags != 0;
-                               if (gem_has_bsd2(fd) != is_bsd2)
-                                       continue;
-                       }
-
-                       igt_require(has_engine(fd, e, 1) == -ENOENT);
-
-                       engines[num_engines++] = e->exec_id | e->flags;
+               unsigned engine;
+               for_each_physical_engine(fd, engine) {
+                       engines[num_engines++] = engine;
                        if (num_engines == ARRAY_SIZE(engines))
                                break;
                }
@@ -243,7 +230,7 @@ static void single(const char *name, bool all_engines)
  static void processes(void)
  {
        const struct intel_execution_engine *e;
-       unsigned engines[16];
+       unsigned engines[16], engine;
        int num_engines;
        struct rlimit rlim;
        unsigned num_ctx;
@@ -253,20 +240,8 @@ static void processes(void)
        fd = drm_open_driver(DRIVER_INTEL);
num_engines = 0;
-       for (e = intel_execution_engines; e->name; e++) {
-               if (e->exec_id == 0)
-                       continue;
-
-               if (has_engine(fd, e, 0))
-                       continue;
-
-               if (e->exec_id == I915_EXEC_BSD) {
-                       int is_bsd2 = e->flags != 0;
-                       if (gem_has_bsd2(fd) != is_bsd2)
-                               continue;
-               }
-
-               engines[num_engines++] = e->exec_id | e->flags;
+       for_each_physical_engine(fd, engine) {
+               engines[num_engines++] = engine;
                if (num_engines == ARRAY_SIZE(engines))
                        break;
        }
diff --git a/tests/gem_exec_async.c b/tests/gem_exec_async.c
index 30e9452f..9a06af7e 100644
--- a/tests/gem_exec_async.c
+++ b/tests/gem_exec_async.c
@@ -88,6 +88,7 @@ static void one(int fd, unsigned ring, uint32_t flags)
  #define BATCH 1
        struct drm_i915_gem_relocation_entry reloc;
        struct drm_i915_gem_execbuffer2 execbuf;
+       unsigned int other;
        uint32_t *batch;
        int i;
@@ -142,19 +143,14 @@ static void one(int fd, unsigned ring, uint32_t flags)
        gem_close(fd, obj[BATCH].handle);
i = 0;
-       for (const struct intel_execution_engine *e = intel_execution_engines;
-            e->name; e++) {
-               if (e->exec_id == 0 || e->exec_id == ring)
+       for_each_physical_engine(fd, other) {
+               if (other == ring)
                        continue;
- if (!gem_has_ring(fd, e->exec_id | e->flags))
+               if (!gem_can_store_dword(fd, other))
                        continue;
- if (!gem_can_store_dword(fd, e->exec_id | e->flags))
-                       continue;
-
-               store_dword(fd, e->exec_id | e->flags,
-                           obj[SCRATCH].handle, 4*i, i);
+               store_dword(fd, other, obj[SCRATCH].handle, 4*i, i);
                i++;
        }
@@ -209,6 +205,7 @@ igt_main
                        continue;
igt_subtest_f("concurrent-writes-%s", e->name) {
+                       igt_require(gem_ring_has_physical_engine(fd, e->exec_id | 
e->flags));
                        igt_require(gem_can_store_dword(fd, e->exec_id | 
e->flags));
                        one(fd, e->exec_id, e->flags);
                }
diff --git a/tests/gem_exec_await.c b/tests/gem_exec_await.c
index e19363c4..fccc24d9 100644
--- a/tests/gem_exec_await.c
+++ b/tests/gem_exec_await.c
@@ -44,17 +44,6 @@ static double elapsed(const struct timespec *start, const 
struct timespec *end)
                (end->tv_nsec - start->tv_nsec)*1e-9);
  }
-static bool ignore_engine(int fd, unsigned engine)
-{
-       if (engine == 0)
-               return true;
-
-       if (gem_has_bsd2(fd) && engine == I915_EXEC_BSD)
-               return true;
-
-       return false;
-}
-
  static void xchg_obj(void *array, unsigned i, unsigned j)
  {
        struct drm_i915_gem_exec_object2 *obj = array;
@@ -89,12 +78,8 @@ static void wide(int fd, int ring_size, int timeout, 
unsigned int flags)
        double time;
nengine = 0;
-       for_each_engine(fd, engine) {
-               if (ignore_engine(fd, engine))
-                       continue;
-
+       for_each_physical_engine(fd, engine)
                engines[nengine++] = engine;
-       }
        igt_require(nengine);
exec = calloc(nengine, sizeof(*exec));
diff --git a/tests/gem_exec_capture.c b/tests/gem_exec_capture.c
index aa80d59d..43c443be 100644
--- a/tests/gem_exec_capture.c
+++ b/tests/gem_exec_capture.c
@@ -208,7 +208,7 @@ igt_main
                        continue;
igt_subtest_f("capture-%s", e->name) {
-                       gem_require_ring(fd, e->exec_id | e->flags);
+                       igt_require(gem_ring_has_physical_engine(fd, e->exec_id | 
e->flags));
                        igt_require(gem_can_store_dword(fd, e->exec_id | 
e->flags));
                        capture(fd, dir, e->exec_id | e->flags);
                }
diff --git a/tests/gem_exec_create.c b/tests/gem_exec_create.c
index 28479a9d..54a2429e 100644
--- a/tests/gem_exec_create.c
+++ b/tests/gem_exec_create.c
@@ -54,17 +54,6 @@ static double elapsed(const struct timespec *start, const 
struct timespec *end)
                (end->tv_nsec - start->tv_nsec)*1e-9);
  }
-static bool ignore_engine(int fd, unsigned engine)
-{
-       if (engine == 0)
-               return true;
-
-       if (gem_has_bsd2(fd) && engine == I915_EXEC_BSD)
-               return true;
-
-       return false;
-}
-
  #define LEAK 0x1
static void all(int fd, unsigned flags, int timeout, int ncpus)
@@ -77,12 +66,8 @@ static void all(int fd, unsigned flags, int timeout, int 
ncpus)
        unsigned engine;
nengine = 0;
-       for_each_engine(fd, engine) {
-               if (ignore_engine(fd, engine))
-                       continue;
-
+       for_each_physical_engine(fd, engine)
                engines[nengine++] = engine;
-       }
        igt_require(nengine);
memset(&obj, 0, sizeof(obj));
diff --git a/tests/gem_exec_fence.c b/tests/gem_exec_fence.c
index 312505d6..39ca17ee 100644
--- a/tests/gem_exec_fence.c
+++ b/tests/gem_exec_fence.c
@@ -273,7 +273,7 @@ static void test_fence_await(int fd, unsigned ring, 
unsigned flags)
        igt_assert(fence != -1);
i = 0;
-       for_each_engine(fd, engine) {
+       for_each_physical_engine(fd, engine) {
                if (!gem_can_store_dword(fd, engine))
                        continue;
@@ -521,10 +521,7 @@ static void test_parallel(int fd, unsigned int master)
        obj[BATCH].relocation_count = 1;
/* Queue all secondaries */
-       for_each_engine(fd, engine) {
-               if (engine == 0 || engine == I915_EXEC_BSD)
-                       continue;
-
+       for_each_physical_engine(fd, engine) {
                if (engine == master)
                        continue;
@@ -699,15 +696,8 @@ static void test_long_history(int fd, long ring_size, unsigned flags)
                limit = ring_size / 3;
nengine = 0;
-       for_each_engine(fd, engine) {
-               if (engine == 0)
-                       continue;
-
-               if (engine == I915_EXEC_BSD)
-                       continue;
-
+       for_each_physical_engine(fd, engine)
                engines[nengine++] = engine;
-       }
        igt_require(nengine);
gem_quiescent_gpu(fd);
diff --git a/tests/gem_exec_gttfill.c b/tests/gem_exec_gttfill.c
index 96ed832f..4097e407 100644
--- a/tests/gem_exec_gttfill.c
+++ b/tests/gem_exec_gttfill.c
@@ -28,17 +28,6 @@ IGT_TEST_DESCRIPTION("Fill the GTT with batches.");
#define BATCH_SIZE (4096<<10) -static bool ignore_engine(int fd, unsigned engine)
-{
-       if (engine == 0)
-               return true;
-
-       if (gem_has_bsd2(fd) && engine == I915_EXEC_BSD)
-               return true;
-
-       return false;
-}
-
  static void xchg_u32(void *array, unsigned i, unsigned j)
  {
        uint32_t *u32 = array;
@@ -126,10 +115,7 @@ static void fillgtt(int fd, unsigned ring, int timeout)
nengine = 0;
        if (ring == 0) {
-               for_each_engine(fd, engine) {
-                       if (ignore_engine(fd, engine))
-                               continue;
-
+               for_each_physical_engine(fd, engine) {
                        if (!gem_can_store_dword(fd, engine))
                                continue;
diff --git a/tests/gem_exec_latency.c b/tests/gem_exec_latency.c
index 74044bf4..17ab6e33 100644
--- a/tests/gem_exec_latency.c
+++ b/tests/gem_exec_latency.c
@@ -277,13 +277,13 @@ static void latency_from_ring(int fd,
                              unsigned ring, const char *name,
                              unsigned flags)
  {
-       const struct intel_execution_engine *e;
        const int gen = intel_gen(intel_get_drm_devid(fd));
        const int has_64bit_reloc = gen >= 8;
        struct drm_i915_gem_exec_object2 obj[3];
        struct drm_i915_gem_relocation_entry reloc;
        struct drm_i915_gem_execbuffer2 execbuf;
        const unsigned int repeats = ring_size / 2;
+       unsigned int other;
        uint32_t *map, *results;
        uint32_t ctx[2] = {};
        int i, j;
@@ -329,22 +329,16 @@ static void latency_from_ring(int fd,
        reloc.presumed_offset = obj[1].offset;
        reloc.target_handle = flags & CORK ? 1 : 0;
- for (e = intel_execution_engines; e->name; e++) {
+       for_each_physical_engine(fd, other) {
                igt_spin_t *spin = NULL;
                struct cork c;
- if (e->exec_id == 0)
-                       continue;
-
-               if (!gem_has_ring(fd, e->exec_id | e->flags))
-                       continue;
-
                gem_set_domain(fd, obj[2].handle,
                               I915_GEM_DOMAIN_GTT,
                               I915_GEM_DOMAIN_GTT);
if (flags & PREEMPT)
-                       spin = igt_spin_batch_new(fd, ctx[0], ring, 0);
+                       spin = __igt_spin_batch_new(fd, ctx[0], ring, 0);
if (flags & CORK) {
                        plug(fd, &c);
@@ -382,7 +376,7 @@ static void latency_from_ring(int fd,
                        gem_execbuf(fd, &execbuf);
execbuf.flags &= ~ENGINE_FLAGS;
-                       execbuf.flags |= e->exec_id | e->flags;
+                       execbuf.flags |= other;
execbuf.batch_start_offset = 64 * (j + repeats);
                        reloc.offset =
@@ -415,7 +409,8 @@ static void latency_from_ring(int fd,
                igt_spin_batch_free(fd, spin);
igt_info("%s-%s delay: %.2f\n",
-                        name, e->name, (results[2*repeats-1] - results[0]) / 
(double)repeats);
+                        name, e__->name,
+                        (results[2*repeats-1] - results[0]) / (double)repeats);
        }
munmap(map, 64*1024);
@@ -461,7 +456,7 @@ igt_main
igt_subtest_group {
                                igt_fixture {
-                                       gem_require_ring(device, e->exec_id | 
e->flags);
+                                       
igt_require(gem_ring_has_physical_engine(device, e->exec_id | e->flags));
                                }
igt_subtest_f("%s-dispatch", e->name)
diff --git a/tests/gem_exec_nop.c b/tests/gem_exec_nop.c
index d3e9a3e0..d971ffcb 100644
--- a/tests/gem_exec_nop.c
+++ b/tests/gem_exec_nop.c
@@ -188,17 +188,6 @@ static void headless(int fd, uint32_t handle)
        assert_within_epsilon(n_headless, n_display, 0.1f);
  }
-static bool ignore_engine(int fd, unsigned engine)
-{
-       if (engine == 0)
-               return true;
-
-       if (gem_has_bsd2(fd) && engine == I915_EXEC_BSD)
-               return true;
-
-       return false;
-}
-
  static void parallel(int fd, uint32_t handle, int timeout)
  {
        struct drm_i915_gem_execbuffer2 execbuf;
@@ -212,10 +201,7 @@ static void parallel(int fd, uint32_t handle, int timeout)
sum = 0;
        nengine = 0;
-       for_each_engine(fd, engine) {
-               if (ignore_engine(fd, engine))
-                       continue;
-
+       for_each_physical_engine(fd, engine) {
                engines[nengine] = engine;
                names[nengine] = e__->name;
                nengine++;
@@ -277,10 +263,7 @@ static void series(int fd, uint32_t handle, int timeout)
        const char *name;
nengine = 0;
-       for_each_engine(fd, engine) {
-               if (ignore_engine(fd, engine))
-                       continue;
-
+       for_each_physical_engine(fd, engine) {
                time = nop_on_ring(fd, handle, engine, 1, &count) / count;
                if (time > max) {
                        name = e__->name;
@@ -375,12 +358,9 @@ static void sequential(int fd, uint32_t handle, unsigned 
flags, int timeout)
nengine = 0;
        sum = 0;
-       for_each_engine(fd, n) {
+       for_each_physical_engine(fd, n) {
                unsigned long count;
- if (ignore_engine(fd, n))
-                       continue;
-
                time = nop_on_ring(fd, handle, n, 1, &count) / count;
                sum += time;
                igt_debug("%s: %.3fus\n", e__->name, 1e6*time);
@@ -509,12 +489,8 @@ static void fence_signal(int fd, uint32_t handle,
nengine = 0;
        if (ring_id == -1) {
-               for_each_engine(fd, n) {
-                       if (ignore_engine(fd, n))
-                               continue;
-
+               for_each_physical_engine(fd, n)
                        engines[nengine++] = n;
-               }
        } else {
                gem_require_ring(fd, ring_id);
                engines[nengine++] = ring_id;
diff --git a/tests/gem_exec_parallel.c b/tests/gem_exec_parallel.c
index 11b6e775..fe5ffe8f 100644
--- a/tests/gem_exec_parallel.c
+++ b/tests/gem_exec_parallel.c
@@ -55,17 +55,6 @@ static void check_bo(int fd, uint32_t handle, int pass)
        munmap(map, 4096);
  }
-static bool ignore_engine(int fd, unsigned engine)
-{
-       if (engine == 0)
-               return true;
-
-       if (!gem_can_store_dword(fd, engine))
-               return true;
-
-       return false;
-}
-
  #define CONTEXTS 0x1
  #define FDS 0x2
@@ -180,8 +169,8 @@ static void all(int fd, unsigned engine, unsigned flags) nengine = 0;
        if (engine == -1) {
-               for_each_engine(fd, engine) {
-                       if (!ignore_engine(fd, engine))
+               for_each_physical_engine(fd, engine) {
+                       if (gem_can_store_dword(fd, engine))
                                engines[nengine++] = engine;
                }
        } else {
diff --git a/tests/gem_exec_reloc.c b/tests/gem_exec_reloc.c
index 432a42a9..213de1d7 100644
--- a/tests/gem_exec_reloc.c
+++ b/tests/gem_exec_reloc.c
@@ -258,7 +258,7 @@ static void active(int fd, unsigned engine)
nengine = 0;
        if (engine == -1) {
-               for_each_engine(fd, engine) {
+               for_each_physical_engine(fd, engine) {
                        if (gem_can_store_dword(fd, engine))
                                engines[nengine++] = engine;
                }
diff --git a/tests/gem_exec_schedule.c b/tests/gem_exec_schedule.c
index 5f24df33..dff5abb9 100644
--- a/tests/gem_exec_schedule.c
+++ b/tests/gem_exec_schedule.c
@@ -188,17 +188,6 @@ static void fifo(int fd, unsigned ring)
        munmap(ptr, 4096);
  }
-static bool ignore_engine(int fd, unsigned engine)
-{
-       if (engine == 0)
-               return true;
-
-       if (gem_has_bsd2(fd) && engine == I915_EXEC_BSD)
-               return true;
-
-       return false;
-}
-
  static void smoketest(int fd, unsigned ring, unsigned timeout)
  {
        const int ncpus = sysconf(_SC_NPROCESSORS_ONLN);
@@ -210,12 +199,8 @@ static void smoketest(int fd, unsigned ring, unsigned 
timeout)
nengine = 0;
        if (ring == -1) {
-               for_each_engine(fd, engine) {
-                       if (ignore_engine(fd, engine))
-                               continue;
-
+               for_each_physical_engine(fd, engine)
                        engines[nengine++] = engine;
-               }
        } else {
                engines[nengine++] = ring;
        }
@@ -440,7 +425,7 @@ static void preempt_other(int fd, unsigned ring)
        gem_context_set_priority(fd, ctx[HI], MAX_PRIO);
n = 0;
-       for_each_engine(fd, other) {
+       for_each_physical_engine(fd, other) {
                igt_assert(n < ARRAY_SIZE(spin));
spin[n] = __igt_spin_batch_new(fd, ctx[NOISE], other, 0);
@@ -496,7 +481,7 @@ static void preempt_self(int fd, unsigned ring)
n = 0;
        gem_context_set_priority(fd, ctx[HI], MIN_PRIO);
-       for_each_engine(fd, other) {
+       for_each_physical_engine(fd, other) {
                spin[n] = __igt_spin_batch_new(fd, ctx[NOISE], other, 0);
                store_dword(fd, ctx[HI], other,
                            result, (n + 1)*sizeof(uint32_t), n + 1,
@@ -1030,7 +1015,7 @@ igt_main
                                continue;
igt_subtest_f("fifo-%s", e->name) {
-                               gem_require_ring(fd, e->exec_id | e->flags);
+                               igt_require(gem_ring_has_physical_engine(fd, 
e->exec_id | e->flags));
                                igt_require(gem_can_store_dword(fd, e->exec_id) | 
e->flags);
                                fifo(fd, e->exec_id | e->flags);
                        }
@@ -1047,13 +1032,12 @@ igt_main
                        smoketest(fd, -1, 30);
for (e = intel_execution_engines; e->name; e++) {
-                       /* default exec-id is purely symbolic */
                        if (e->exec_id == 0)
                                continue;
igt_subtest_group {
                                igt_fixture {
-                                       gem_require_ring(fd, e->exec_id | 
e->flags);
+                                       igt_require(gem_ring_has_physical_engine(fd, 
e->exec_id | e->flags));
                                        igt_require(gem_can_store_dword(fd, 
e->exec_id) | e->flags);
                                }
@@ -1130,9 +1114,12 @@ igt_main
                }
for (e = intel_execution_engines; e->name; e++) {
+                       if (e->exec_id == 0)
+                               continue;
+
                        igt_subtest_group {
                                igt_fixture {
-                                       gem_require_ring(fd, e->exec_id | 
e->flags);
+                                       igt_require(gem_ring_has_physical_engine(fd, 
e->exec_id | e->flags));
                                        
igt_require(gem_scheduler_has_preemption(fd));
                                }
diff --git a/tests/gem_exec_store.c b/tests/gem_exec_store.c
index 31a2c096..a7673489 100644
--- a/tests/gem_exec_store.c
+++ b/tests/gem_exec_store.c
@@ -220,7 +220,7 @@ static void store_all(int fd)
nengine = 0;
        intel_detect_and_clear_missed_interrupts(fd);
-       for_each_engine(fd, engine) {
+       for_each_physical_engine(fd, engine) {
                if (!gem_can_store_dword(fd, engine))
                        continue;
diff --git a/tests/gem_exec_suspend.c b/tests/gem_exec_suspend.c
index bbdc6e55..351347cb 100644
--- a/tests/gem_exec_suspend.c
+++ b/tests/gem_exec_suspend.c
@@ -62,25 +62,13 @@ static void check_bo(int fd, uint32_t handle)
        munmap(map, 4096);
  }
-static bool ignore_engine(int fd, unsigned engine)
-{
-       if (engine == 0)
-               return true;
-
-       if (!gem_can_store_dword(fd, engine))
-               return true;
-
-       return false;
-}
-
  static void test_all(int fd, unsigned flags)
  {
        unsigned engine;
- for_each_engine(fd, engine) {
-               if (!ignore_engine(fd, engine))
+       for_each_physical_engine(fd, engine)
+               if (gem_can_store_dword(fd, engine))
                        run_test(fd, engine, flags & ~0xff);
-       }
  }
static bool has_semaphores(int fd)
@@ -118,8 +106,8 @@ static void run_test(int fd, unsigned engine, unsigned 
flags)
                 * GPU is then unlikely to be active!)
                 */
                if (has_semaphores(fd)) {
-                       for_each_engine(fd, engine) {
-                               if (!ignore_engine(fd, engine))
+                       for_each_physical_engine(fd, engine) {
+                               if (gem_can_store_dword(fd, engine))
                                        engines[nengine++] = engine;
                        }
                } else {
diff --git a/tests/gem_exec_whisper.c b/tests/gem_exec_whisper.c
index 5f9fedf5..1f4dad63 100644
--- a/tests/gem_exec_whisper.c
+++ b/tests/gem_exec_whisper.c
@@ -79,17 +79,6 @@ static void verify_reloc(int fd, uint32_t handle,
        }
  }
-static bool ignore_engine(int fd, unsigned engine)
-{
-       if (engine == 0)
-               return true;
-
-       if (!gem_can_store_dword(fd, engine))
-               return true;
-
-       return false;
-}
-
  #define CONTEXTS 0x1
  #define FDS 0x2
  #define INTERRUPTIBLE 0x4
@@ -217,8 +206,8 @@ static void whisper(int fd, unsigned engine, unsigned flags)
nengine = 0;
        if (engine == -1) {
-               for_each_engine(fd, engine) {
-                       if (!ignore_engine(fd, engine))
+               for_each_physical_engine(fd, engine) {
+                       if (gem_can_store_dword(fd, engine))
                                engines[nengine++] = engine;
                }
        } else {
diff --git a/tests/gem_ring_sync_loop.c b/tests/gem_ring_sync_loop.c
index ab5bedb3..118f3638 100644
--- a/tests/gem_ring_sync_loop.c
+++ b/tests/gem_ring_sync_loop.c
@@ -48,7 +48,7 @@ sync_loop(int fd)
        int i;
nengine = 0;
-       for_each_engine(fd, engine)
+       for_each_physical_engine(fd, engine)
                engines[nengine++] = engine;
        igt_require(nengine);
diff --git a/tests/gem_spin_batch.c b/tests/gem_spin_batch.c
index 89631130..026f9830 100644
--- a/tests/gem_spin_batch.c
+++ b/tests/gem_spin_batch.c
@@ -76,10 +76,7 @@ static void spin_on_all_engines(int fd, unsigned int 
timeout_sec)
  {
        unsigned engine;
- for_each_engine(fd, engine) {
-               if (engine == 0)
-                       continue;
-
+       for_each_physical_engine(fd, engine) {
                igt_fork(child, 1) {
                        igt_install_exit_handler(spin_exit_handler);
                        spin(fd, engine, timeout_sec);
diff --git a/tests/gem_sync.c b/tests/gem_sync.c
index d70515ea..f451287a 100644
--- a/tests/gem_sync.c
+++ b/tests/gem_sync.c
@@ -86,23 +86,9 @@ sync_ring(int fd, unsigned ring, int num_children, int 
timeout)
        int num_engines = 0;
if (ring == ~0u) {
-               const struct intel_execution_engine *e;
-
-               for (e = intel_execution_engines; e->name; e++) {
-                       if (e->exec_id == 0)
-                               continue;
-
-                       if (!gem_has_ring(fd, e->exec_id | e->flags))
-                               continue;
-
-                       if (e->exec_id == I915_EXEC_BSD) {
-                               int is_bsd2 = e->flags != 0;
-                               if (gem_has_bsd2(fd) != is_bsd2)
-                                       continue;
-                       }
-
-                       names[num_engines] = e->name;
-                       engines[num_engines++] = e->exec_id | e->flags;
+               for_each_physical_engine(fd, ring) {
+                       names[num_engines] = e__->name;
+                       engines[num_engines++] = ring;
                        if (num_engines == ARRAY_SIZE(engines))
                                break;
                }
@@ -200,26 +186,12 @@ store_ring(int fd, unsigned ring, int num_children, int 
timeout)
        int num_engines = 0;
if (ring == ~0u) {
-               const struct intel_execution_engine *e;
-
-               for (e = intel_execution_engines; e->name; e++) {
-                       if (e->exec_id == 0)
-                               continue;
-
-                       if (!gem_has_ring(fd, e->exec_id | e->flags))
-                               continue;
-
-                       if (!gem_can_store_dword(fd, e->exec_id | e->flags))
+               for_each_physical_engine(fd, ring) {
+                       if (!gem_can_store_dword(fd, ring))
                                continue;
- if (e->exec_id == I915_EXEC_BSD) {
-                               int is_bsd2 = e->flags != 0;
-                               if (gem_has_bsd2(fd) != is_bsd2)
-                                       continue;
-                       }
-
-                       names[num_engines] = e->name;
-                       engines[num_engines++] = e->exec_id | e->flags;
+                       names[num_engines] = e__->name;
+                       engines[num_engines++] = ring;
                        if (num_engines == ARRAY_SIZE(engines))
                                break;
                }
@@ -502,31 +474,17 @@ store_many(int fd, unsigned ring, int timeout)
        intel_detect_and_clear_missed_interrupts(fd);
if (ring == ~0u) {
-               const struct intel_execution_engine *e;
-
-               for (e = intel_execution_engines; e->name; e++) {
-                       if (e->exec_id == 0)
-                               continue;
-
-                       if (!gem_has_ring(fd, e->exec_id | e->flags))
+               for_each_physical_engine(fd, ring) {
+                       if (!gem_can_store_dword(fd, ring))
                                continue;
- if (!gem_can_store_dword(fd, e->exec_id | e->flags))
-                               continue;
-
-                       if (e->exec_id == I915_EXEC_BSD) {
-                               int is_bsd2 = e->flags != 0;
-                               if (gem_has_bsd2(fd) != is_bsd2)
-                                       continue;
-                       }
-
                        igt_fork(child, 1)
                                __store_many(fd,
-                                            e->exec_id | e->flags,
+                                            ring,
                                             timeout,
                                             &shared[n]);
- names[n++] = e->name;
+                       names[n++] = e__->name;
                }
                igt_waitchildren();
        } else {
@@ -547,24 +505,11 @@ store_many(int fd, unsigned ring, int timeout)
  static void
  sync_all(int fd, int num_children, int timeout)
  {
-       const struct intel_execution_engine *e;
-       unsigned engines[16];
+       unsigned engines[16], engine;
        int num_engines = 0;
- for (e = intel_execution_engines; e->name; e++) {
-               if (e->exec_id == 0)
-                       continue;
-
-               if (!gem_has_ring(fd, e->exec_id | e->flags))
-                       continue;
-
-               if (e->exec_id == I915_EXEC_BSD) {
-                       int is_bsd2 = e->flags != 0;
-                       if (gem_has_bsd2(fd) != is_bsd2)
-                               continue;
-               }
-
-               engines[num_engines++] = e->exec_id | e->flags;
+       for_each_physical_engine(fd, engine) {
+               engines[num_engines++] = engine;
                if (num_engines == ARRAY_SIZE(engines))
                        break;
        }
@@ -612,27 +557,15 @@ static void
  store_all(int fd, int num_children, int timeout)
  {
        const int gen = intel_gen(intel_get_drm_devid(fd));
-       const struct intel_execution_engine *e;
        unsigned engines[16];
        int num_engines = 0;
+       unsigned int ring;
- for (e = intel_execution_engines; e->name; e++) {
-               if (e->exec_id == 0)
-                       continue;
-
-               if (!gem_has_ring(fd, e->exec_id | e->flags))
-                       continue;
-
-               if (!gem_can_store_dword(fd, e->exec_id))
+       for_each_physical_engine(fd, ring) {
+               if (!gem_can_store_dword(fd, ring))
                        continue;
- if (e->exec_id == I915_EXEC_BSD) {
-                       int is_bsd2 = e->flags != 0;
-                       if (gem_has_bsd2(fd) != is_bsd2)
-                               continue;
-               }
-
-               engines[num_engines++] = e->exec_id | e->flags;
+               engines[num_engines++] = ring;
                if (num_engines == ARRAY_SIZE(engines))
                        break;
        }
@@ -737,23 +670,9 @@ preempt(int fd, unsigned ring, int num_children, int 
timeout)
        uint32_t ctx[2];
if (ring == ~0u) {
-               const struct intel_execution_engine *e;
-
-               for (e = intel_execution_engines; e->name; e++) {
-                       if (e->exec_id == 0)
-                               continue;
-
-                       if (!gem_has_ring(fd, e->exec_id | e->flags))
-                               continue;
-
-                       if (e->exec_id == I915_EXEC_BSD) {
-                               int is_bsd2 = e->flags != 0;
-                               if (gem_has_bsd2(fd) != is_bsd2)
-                                       continue;
-                       }
-
-                       names[num_engines] = e->name;
-                       engines[num_engines++] = e->exec_id | e->flags;
+               for_each_physical_engine(fd, ring) {
+                       names[num_engines] = e__->name;
+                       engines[num_engines++] = ring;
                        if (num_engines == ARRAY_SIZE(engines))
                                break;
                }


I would prefer if for_each_physical engine took an engine parameter but in the interest of progress:

Reviewed-by: Tvrtko Ursulin <tvrtko.ursu...@intel.com>

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to