* [igt-dev] [PATCH] [PATCH i-g-t][V5]tests/i915/gem_sync.c :Added __for_each_physical_engine to utilize all available
@ 2020-04-08 15:12 Arjun Melkaveri
2020-04-08 16:12 ` [igt-dev] ✗ GitLab.Pipeline: failure for tests/i915/gem_sync.c :Added __for_each_physical_engine to utilize all available (rev6) Patchwork
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Arjun Melkaveri @ 2020-04-08 15:12 UTC (permalink / raw)
To: arjun.melkaveri, igt-dev
Replaced the legacy for_each_engine* defines with the ones
implemented in the gem_engine_topology library.
Used gem_context_clone_with_engines
to make sure that engine index was potentially created
based on a default context with engine map configured.
Added gem_reopen_driver and gem_context_copy_engines
to transfer the engine map from parent fd default
context.
V2:
Added Legacy engine coverage for sync_ring and sync_all.
V3:
Added back ALL_ENGINES. Corrected Test cases that used
gem_reopen_driver in fork. Which was not recommended.
V4:
Removed gem_require_ring and gem_can_store_dword.
V5:
Addressed the review comments.
divided tests into static for ALL_Engine test cases
and dynamic for multiple engine .
Cc: Dec Katarzyna <katarzyna.dec@intel.com>
Cc: Ursulin Tvrtko <tvrtko.ursulin@intel.com>
Signed-off-by: sai gowtham <sai.gowtham.ch@intel.com>
Signed-off-by: Arjun Melkaveri <arjun.melkaveri@intel.com>
---
tests/i915/gem_sync.c | 290 +++++++++++++++++++++++++++---------------
1 file changed, 185 insertions(+), 105 deletions(-)
diff --git a/tests/i915/gem_sync.c b/tests/i915/gem_sync.c
index 2ef55ecc..a93fa63a 100644
--- a/tests/i915/gem_sync.c
+++ b/tests/i915/gem_sync.c
@@ -24,6 +24,9 @@
#include <time.h>
#include <pthread.h>
+#include "igt_debugfs.h"
+#include "igt_dummyload.h"
+#include "igt_gt.h"
#include "igt.h"
#include "igt_sysfs.h"
@@ -37,6 +40,9 @@
#define MIN_PRIO LOCAL_I915_CONTEXT_MIN_USER_PRIORITY
#define ENGINE_MASK (I915_EXEC_RING_MASK | LOCAL_I915_EXEC_BSD_MASK)
+#define RESET_TIMEOUT_MS (2 * MSEC_PER_SEC)
+static unsigned long reset_timeout_ms = RESET_TIMEOUT_MS;
+#define NSEC_PER_MSEC (1000 * 1000ull)
IGT_TEST_DESCRIPTION("Basic check of ring<->ring write synchronisation.");
@@ -78,24 +84,35 @@ out:
return ts.tv_sec + 1e-9*ts.tv_nsec;
}
+static void cleanup(int i915)
+{
+ igt_drop_caches_set(i915,
+ /* cancel everything */
+ DROP_RESET_ACTIVE | DROP_RESET_SEQNO |
+ /* cleanup */
+ DROP_ACTIVE | DROP_RETIRE | DROP_IDLE | DROP_FREED);
+ igt_require_gem(i915);
+}
+
+
static void
sync_ring(int fd, unsigned ring, int num_children, int timeout)
{
+ const struct intel_execution_engine2 *e2;
unsigned engines[16];
const char *names[16];
int num_engines = 0;
if (ring == ALL_ENGINES) {
- for_each_physical_engine(e, fd) {
- names[num_engines] = e->name;
- engines[num_engines++] = eb_ring(e);
+ __for_each_physical_engine(fd, e2) {
+ names[num_engines] = e2->name;
+ engines[num_engines++] = e2->flags;
if (num_engines == ARRAY_SIZE(engines))
break;
}
num_children *= num_engines;
} else {
- gem_require_ring(fd, ring);
names[num_engines] = NULL;
engines[num_engines++] = ring;
}
@@ -139,7 +156,7 @@ sync_ring(int fd, unsigned ring, int num_children, int timeout)
}
static void
-idle_ring(int fd, unsigned ring, int timeout)
+idle_ring(int fd, unsigned int ring, int num_children, int timeout)
{
const uint32_t bbe = MI_BATCH_BUFFER_END;
struct drm_i915_gem_exec_object2 object;
@@ -180,24 +197,23 @@ idle_ring(int fd, unsigned ring, int timeout)
static void
wakeup_ring(int fd, unsigned ring, int timeout, int wlen)
{
+ const struct intel_execution_engine2 *e2;
unsigned engines[16];
const char *names[16];
int num_engines = 0;
if (ring == ALL_ENGINES) {
- for_each_physical_engine(e, fd) {
- if (!gem_can_store_dword(fd, eb_ring(e)))
+ __for_each_physical_engine(fd, e2) {
+ if (!gem_class_can_store_dword(fd, e2->class))
continue;
- names[num_engines] = e->name;
- engines[num_engines++] = eb_ring(e);
+ names[num_engines] = e2->name;
+ engines[num_engines++] = e2->flags;
if (num_engines == ARRAY_SIZE(engines))
break;
}
igt_require(num_engines);
} else {
- gem_require_ring(fd, ring);
- igt_require(gem_can_store_dword(fd, ring));
names[num_engines] = NULL;
engines[num_engines++] = ring;
}
@@ -290,26 +306,26 @@ wakeup_ring(int fd, unsigned ring, int timeout, int wlen)
igt_assert_eq(intel_detect_and_clear_missed_interrupts(fd), 0);
}
-static void active_ring(int fd, unsigned ring, int timeout)
+static void active_ring(int fd, unsigned int ring,
+ int num_children, int timeout)
{
+ const struct intel_execution_engine2 *e2;
unsigned engines[16];
const char *names[16];
int num_engines = 0;
if (ring == ALL_ENGINES) {
- for_each_physical_engine(e, fd) {
- if (!gem_can_store_dword(fd, eb_ring(e)))
+ __for_each_physical_engine(fd, e2) {
+ if (!gem_class_can_store_dword(fd, e2->class))
continue;
- names[num_engines] = e->name;
- engines[num_engines++] = eb_ring(e);
+ names[num_engines] = e2->name;
+ engines[num_engines++] = e2->flags;
if (num_engines == ARRAY_SIZE(engines))
break;
}
igt_require(num_engines);
} else {
- gem_require_ring(fd, ring);
- igt_require(gem_can_store_dword(fd, ring));
names[num_engines] = NULL;
engines[num_engines++] = ring;
}
@@ -359,24 +375,23 @@ static void active_ring(int fd, unsigned ring, int timeout)
static void
active_wakeup_ring(int fd, unsigned ring, int timeout, int wlen)
{
+ const struct intel_execution_engine2 *e2;
unsigned engines[16];
const char *names[16];
int num_engines = 0;
if (ring == ALL_ENGINES) {
- for_each_physical_engine(e, fd) {
- if (!gem_can_store_dword(fd, eb_ring(e)))
+ __for_each_physical_engine(fd, e2) {
+ if (!gem_class_can_store_dword(fd, e2->class))
continue;
- names[num_engines] = e->name;
- engines[num_engines++] = eb_ring(e);
+ names[num_engines] = e2->name;
+ engines[num_engines++] = e2->flags;
if (num_engines == ARRAY_SIZE(engines))
break;
}
igt_require(num_engines);
} else {
- gem_require_ring(fd, ring);
- igt_require(gem_can_store_dword(fd, ring));
names[num_engines] = NULL;
engines[num_engines++] = ring;
}
@@ -493,26 +508,25 @@ active_wakeup_ring(int fd, unsigned ring, int timeout, int wlen)
static void
store_ring(int fd, unsigned ring, int num_children, int timeout)
{
+ const struct intel_execution_engine2 *e2;
const int gen = intel_gen(intel_get_drm_devid(fd));
unsigned engines[16];
const char *names[16];
int num_engines = 0;
if (ring == ALL_ENGINES) {
- for_each_physical_engine(e, fd) {
- if (!gem_can_store_dword(fd, eb_ring(e)))
+ __for_each_physical_engine(fd, e2) {
+ if (!gem_class_can_store_dword(fd, e2->class))
continue;
- names[num_engines] = e->name;
- engines[num_engines++] = eb_ring(e);
+ names[num_engines] = e2->name;
+ engines[num_engines++] = e2->flags;
if (num_engines == ARRAY_SIZE(engines))
break;
}
num_children *= num_engines;
} else {
- gem_require_ring(fd, ring);
- igt_require(gem_can_store_dword(fd, ring));
names[num_engines] = NULL;
engines[num_engines++] = ring;
}
@@ -608,6 +622,7 @@ store_ring(int fd, unsigned ring, int num_children, int timeout)
static void
switch_ring(int fd, unsigned ring, int num_children, int timeout)
{
+ const struct intel_execution_engine2 *e2;
const int gen = intel_gen(intel_get_drm_devid(fd));
unsigned engines[16];
const char *names[16];
@@ -616,20 +631,18 @@ switch_ring(int fd, unsigned ring, int num_children, int timeout)
gem_require_contexts(fd);
if (ring == ALL_ENGINES) {
- for_each_physical_engine(e, fd) {
- if (!gem_can_store_dword(fd, eb_ring(e)))
+ __for_each_physical_engine(fd, e2) {
+ if (!gem_class_can_store_dword(fd, e2->class))
continue;
- names[num_engines] = e->name;
- engines[num_engines++] = eb_ring(e);
+ names[num_engines] = e2->name;
+ engines[num_engines++] = e2->flags;
if (num_engines == ARRAY_SIZE(engines))
break;
}
num_children *= num_engines;
} else {
- gem_require_ring(fd, ring);
- igt_require(gem_can_store_dword(fd, ring));
names[num_engines] = NULL;
engines[num_engines++] = ring;
}
@@ -931,8 +944,9 @@ __store_many(int fd, unsigned ring, int timeout, unsigned long *cycles)
}
static void
-store_many(int fd, unsigned ring, int timeout)
+store_many(int fd, unsigned int ring, int num_children, int timeout)
{
+ const struct intel_execution_engine2 *e2;
unsigned long *shared;
const char *names[16];
int n = 0;
@@ -943,22 +957,20 @@ store_many(int fd, unsigned ring, int timeout)
intel_detect_and_clear_missed_interrupts(fd);
if (ring == ALL_ENGINES) {
- for_each_physical_engine(e, fd) {
- if (!gem_can_store_dword(fd, eb_ring(e)))
+ __for_each_physical_engine(fd, e2) {
+ if (!gem_class_can_store_dword(fd, e2->class))
continue;
igt_fork(child, 1)
__store_many(fd,
- eb_ring(e),
+ e2->flags,
timeout,
&shared[n]);
- names[n++] = e->name;
+ names[n++] = e2->name;
}
igt_waitchildren();
} else {
- gem_require_ring(fd, ring);
- igt_require(gem_can_store_dword(fd, ring));
__store_many(fd, ring, timeout, &shared[n]);
names[n++] = NULL;
}
@@ -1025,15 +1037,16 @@ sync_all(int fd, int num_children, int timeout)
static void
store_all(int fd, int num_children, int timeout)
{
+ const struct intel_execution_engine2 *e;
const int gen = intel_gen(intel_get_drm_devid(fd));
unsigned engines[16];
int num_engines = 0;
- for_each_physical_engine(e, fd) {
- if (!gem_can_store_dword(fd, eb_ring(e)))
+ __for_each_physical_engine(fd, e) {
+ if (!gem_class_can_store_dword(fd, e->class))
continue;
- engines[num_engines++] = eb_ring(e);
+ engines[num_engines++] = e->flags;
if (num_engines == ARRAY_SIZE(engines))
break;
}
@@ -1132,22 +1145,22 @@ store_all(int fd, int num_children, int timeout)
static void
preempt(int fd, unsigned ring, int num_children, int timeout)
{
+ const struct intel_execution_engine2 *e2;
unsigned engines[16];
const char *names[16];
int num_engines = 0;
uint32_t ctx[2];
if (ring == ALL_ENGINES) {
- for_each_physical_engine(e, fd) {
- names[num_engines] = e->name;
- engines[num_engines++] = eb_ring(e);
+ __for_each_physical_engine(fd, e2) {
+ names[num_engines] = e2->name;
+ engines[num_engines++] = e2->flags;
if (num_engines == ARRAY_SIZE(engines))
break;
}
num_children *= num_engines;
} else {
- gem_require_ring(fd, ring);
names[num_engines] = NULL;
engines[num_engines++] = ring;
}
@@ -1207,76 +1220,99 @@ preempt(int fd, unsigned ring, int num_children, int timeout)
gem_context_destroy(fd, ctx[0]);
}
+static void do_test(void (*test)(int i915, unsigned int engine,
+ int num_children, int test_timeout),
+ int i915, unsigned int engine, int num_children,
+ int test_timeout, const char *name)
+{
+#define ATTR "preempt_timeout_ms"
+ int timeout = -1;
+
+ cleanup(i915);
+
+ gem_engine_property_scanf(i915, name, ATTR, "%d", &timeout);
+ if (timeout != -1) {
+ igt_require(gem_engine_property_printf(i915, name,
+ ATTR, "%d", 50) > 0);
+ reset_timeout_ms = 200;
+ }
+
+ test(i915, engine, num_children, test_timeout);
+
+ if (timeout != -1) {
+ gem_engine_property_printf(i915, name, ATTR, "%d", timeout);
+ reset_timeout_ms = RESET_TIMEOUT_MS;
+ }
+
+ gem_quiescent_gpu(i915);
+}
+
igt_main
{
- const struct intel_execution_engine *e;
const int ncpus = sysconf(_SC_NPROCESSORS_ONLN);
int fd = -1;
+ struct {
+ const char *name;
+ void (*func)(int fd, unsigned int engine,
+ int num_children, int timeout);
+ int num_children;
+ int timeout;
+ } *test, AllEngine[] = {
+ { "basic-each", sync_ring, 1, 2},
+ { "basic-store-each", store_ring, 1, 2},
+ { "basic-many-each", store_many, 0, 2},
+ { "switch-each", switch_ring, 1, 20},
+ { "forked-switch-each", switch_ring, ncpus, 20},
+ { "forked-each", sync_ring, ncpus, 20},
+ { "forked-store-each", store_ring, ncpus, 20},
+ { "active-each", active_ring, 0, 20},
+ { "wakeup-each", wakeup_ring, 20, 1},
+ { "active-wakeup-each", active_wakeup_ring, 20, 1},
+ { "double-wakeup-each", wakeup_ring, 20, 2},
+ { NULL, NULL },
+ }, tests[] = {
+ { "default", sync_ring, 1, 20},
+ { "idle", idle_ring, 0, 20},
+ { "active", active_ring, 0, 20},
+ { "wakeup", wakeup_ring, 20, 1},
+ { "active-wakeup", active_wakeup_ring, 20, 1},
+ { "double-wakeup", wakeup_ring, 20, 2},
+ { "store", store_ring, 1, 20},
+ { "switch", switch_ring, 1, 20},
+ { "forked-switch", switch_ring, ncpus, 20},
+ { "many", store_many, 0, 20},
+ { "forked", sync_ring, ncpus, 20},
+ { "forked-store", store_ring, ncpus, 20},
+ { NULL, NULL },
+ };
+
+
igt_fixture {
fd = drm_open_driver(DRIVER_INTEL);
igt_require_gem(fd);
gem_submission_print_method(fd);
gem_scheduler_print_capability(fd);
-
igt_fork_hang_detector(fd);
}
- for (e = intel_execution_engines; e->name; e++) {
- igt_subtest_f("%s", e->name)
- sync_ring(fd, eb_ring(e), 1, 20);
- igt_subtest_f("idle-%s", e->name)
- idle_ring(fd, eb_ring(e), 20);
- igt_subtest_f("active-%s", e->name)
- active_ring(fd, eb_ring(e), 20);
- igt_subtest_f("wakeup-%s", e->name)
- wakeup_ring(fd, eb_ring(e), 20, 1);
- igt_subtest_f("active-wakeup-%s", e->name)
- active_wakeup_ring(fd, eb_ring(e), 20, 1);
- igt_subtest_f("double-wakeup-%s", e->name)
- wakeup_ring(fd, eb_ring(e), 20, 2);
- igt_subtest_f("store-%s", e->name)
- store_ring(fd, eb_ring(e), 1, 20);
- igt_subtest_f("switch-%s", e->name)
- switch_ring(fd, eb_ring(e), 1, 20);
- igt_subtest_f("forked-switch-%s", e->name)
- switch_ring(fd, eb_ring(e), ncpus, 20);
- igt_subtest_f("many-%s", e->name)
- store_many(fd, eb_ring(e), 20);
- igt_subtest_f("forked-%s", e->name)
- sync_ring(fd, eb_ring(e), ncpus, 20);
- igt_subtest_f("forked-store-%s", e->name)
- store_ring(fd, eb_ring(e), ncpus, 20);
+ /* Legacy testing must be first. */
+ igt_subtest_group {
+ for (test = AllEngine; test->name; test++) {
+ igt_subtest_f("%s", test->name) {
+ do_test(test->func,
+ fd, ALL_ENGINES,
+ test->num_children,
+ test->timeout,
+ test->name);
+ }
+ }
}
- igt_subtest("basic-each")
- sync_ring(fd, ALL_ENGINES, 1, 2);
- igt_subtest("basic-store-each")
- store_ring(fd, ALL_ENGINES, 1, 2);
- igt_subtest("basic-many-each")
- store_many(fd, ALL_ENGINES, 2);
- igt_subtest("switch-each")
- switch_ring(fd, ALL_ENGINES, 1, 20);
- igt_subtest("forked-switch-each")
- switch_ring(fd, ALL_ENGINES, ncpus, 20);
- igt_subtest("forked-each")
- sync_ring(fd, ALL_ENGINES, ncpus, 20);
- igt_subtest("forked-store-each")
- store_ring(fd, ALL_ENGINES, ncpus, 20);
- igt_subtest("active-each")
- active_ring(fd, ALL_ENGINES, 20);
- igt_subtest("wakeup-each")
- wakeup_ring(fd, ALL_ENGINES, 20, 1);
- igt_subtest("active-wakeup-each")
- active_wakeup_ring(fd, ALL_ENGINES, 20, 1);
- igt_subtest("double-wakeup-each")
- wakeup_ring(fd, ALL_ENGINES, 20, 2);
-
igt_subtest("basic-all")
sync_all(fd, 1, 2);
igt_subtest("basic-store-all")
store_all(fd, 1, 2);
-
igt_subtest("all")
sync_all(fd, 1, 20);
igt_subtest("store-all")
@@ -1286,7 +1322,49 @@ igt_main
igt_subtest("forked-store-all")
store_all(fd, ncpus, 20);
+ /* legacy of selecting engines. */
+
+ igt_subtest_group {
+ for (test = tests; test->name; test++) {
+ igt_subtest_with_dynamic_f("legacy-engines-%s",
+ test->name) {
+ for_each_physical_engine(e, fd) {
+ igt_dynamic_f("%s", e->name) {
+ do_test(test->func,
+ fd, eb_ring(e),
+ test->num_children,
+ test->timeout,
+ e->full_name);
+ }
+ }
+ }
+ }
+ }
+
+ /* New way of selecting engines. */
+
igt_subtest_group {
+ const struct intel_execution_engine2 *e;
+
+ for (test = tests; test->name; test++) {
+ igt_subtest_with_dynamic_f("engines-%s", test->name) {
+ __for_each_physical_engine(fd, e) {
+ igt_dynamic_f("%s", e->name) {
+ do_test(test->func,
+ fd, e->flags,
+ test->num_children,
+ test->timeout,
+ e->name);
+ }
+ }
+ }
+ }
+ }
+
+
+ igt_subtest_group {
+ const struct intel_execution_engine2 *e;
+
igt_fixture {
gem_require_contexts(fd);
igt_require(gem_scheduler_has_ctx_priority(fd));
@@ -1295,10 +1373,12 @@ igt_main
igt_subtest("preempt-all")
preempt(fd, ALL_ENGINES, 1, 20);
-
- for (e = intel_execution_engines; e->name; e++) {
- igt_subtest_f("preempt-%s", e->name)
- preempt(fd, eb_ring(e), ncpus, 20);
+ igt_subtest_with_dynamic("preempt") {
+ __for_each_physical_engine(fd, e) {
+ /* Requires master for STORE_DWORD on gen4/5 */
+ igt_dynamic_f("%s", e->name)
+ preempt(fd, e->flags, ncpus, 20);
+ }
}
}
--
2.25.1
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [igt-dev] ✗ GitLab.Pipeline: failure for tests/i915/gem_sync.c :Added __for_each_physical_engine to utilize all available (rev6)
2020-04-08 15:12 [igt-dev] [PATCH] [PATCH i-g-t][V5]tests/i915/gem_sync.c :Added __for_each_physical_engine to utilize all available Arjun Melkaveri
@ 2020-04-08 16:12 ` Patchwork
2020-04-08 16:22 ` [igt-dev] ✓ Fi.CI.BAT: success " Patchwork
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2020-04-08 16:12 UTC (permalink / raw)
To: Arjun Melkaveri; +Cc: igt-dev
== Series Details ==
Series: tests/i915/gem_sync.c :Added __for_each_physical_engine to utilize all available (rev6)
URL : https://patchwork.freedesktop.org/series/75536/
State : failure
== Summary ==
ERROR! This series introduces new undocumented tests:
gem_sync@engines-active
gem_sync@engines-active-wakeup
gem_sync@engines-default
gem_sync@engines-double-wakeup
gem_sync@engines-forked
gem_sync@engines-forked-store
gem_sync@engines-forked-switch
gem_sync@engines-idle
gem_sync@engines-many
gem_sync@engines-store
gem_sync@engines-switch
gem_sync@engines-wakeup
gem_sync@legacy-engines-active
gem_sync@legacy-engines-active-wakeup
gem_sync@legacy-engines-default
gem_sync@legacy-engines-double-wakeup
gem_sync@legacy-engines-forked
gem_sync@legacy-engines-forked-store
gem_sync@legacy-engines-forked-switch
gem_sync@legacy-engines-idle
gem_sync@legacy-engines-many
gem_sync@legacy-engines-store
gem_sync@legacy-engines-switch
gem_sync@legacy-engines-wakeup
gem_sync@preempt
Can you document them as per the requirement in the [CONTRIBUTING.md]?
[Documentation] has more details on how to do this.
Here are few examples:
https://gitlab.freedesktop.org/drm/igt-gpu-tools/commit/0316695d03aa46108296b27f3982ec93200c7a6e
https://gitlab.freedesktop.org/drm/igt-gpu-tools/commit/443cc658e1e6b492ee17bf4f4d891029eb7a205d
Thanks in advance!
[CONTRIBUTING.md]: https://gitlab.freedesktop.org/drm/igt-gpu-tools/blob/master/CONTRIBUTING.md#L19
[Documentation]: https://drm.pages.freedesktop.org/igt-gpu-tools/igt-gpu-tools-Core.html#igt-describe
Other than that, pipeline status: SUCCESS.
see https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/pipelines/130037 for the overview.
== Logs ==
For more details see: https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/pipelines/130037
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 6+ messages in thread
* [igt-dev] ✓ Fi.CI.BAT: success for tests/i915/gem_sync.c :Added __for_each_physical_engine to utilize all available (rev6)
2020-04-08 15:12 [igt-dev] [PATCH] [PATCH i-g-t][V5]tests/i915/gem_sync.c :Added __for_each_physical_engine to utilize all available Arjun Melkaveri
2020-04-08 16:12 ` [igt-dev] ✗ GitLab.Pipeline: failure for tests/i915/gem_sync.c :Added __for_each_physical_engine to utilize all available (rev6) Patchwork
@ 2020-04-08 16:22 ` Patchwork
2020-04-09 3:20 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
2020-04-14 12:58 ` [igt-dev] [PATCH] [PATCH i-g-t][V5]tests/i915/gem_sync.c :Added __for_each_physical_engine to utilize all available Tvrtko Ursulin
3 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2020-04-08 16:22 UTC (permalink / raw)
To: Arjun Melkaveri; +Cc: igt-dev
== Series Details ==
Series: tests/i915/gem_sync.c :Added __for_each_physical_engine to utilize all available (rev6)
URL : https://patchwork.freedesktop.org/series/75536/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_8277 -> IGTPW_4433
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4433/index.html
Known issues
------------
Here are the changes found in IGTPW_4433 that come from known issues:
### IGT changes ###
#### Possible fixes ####
* igt@gem_exec_suspend@basic-s4-devices:
- fi-tgl-y: [FAIL][1] ([i915#1158]) -> [PASS][2]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8277/fi-tgl-y/igt@gem_exec_suspend@basic-s4-devices.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4433/fi-tgl-y/igt@gem_exec_suspend@basic-s4-devices.html
[i915#1158]: https://gitlab.freedesktop.org/drm/intel/issues/1158
Participating hosts (52 -> 47)
------------------------------
Additional (1): fi-kbl-7560u
Missing (6): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-byt-clapper fi-bdw-samus
Build changes
-------------
* CI: CI-20190529 -> None
* IGT: IGT_5581 -> IGTPW_4433
CI-20190529: 20190529
CI_DRM_8277: f7d56913e1668f3a269db391189a7888a4b22570 @ git://anongit.freedesktop.org/gfx-ci/linux
IGTPW_4433: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4433/index.html
IGT_5581: ab0620e555119ec55f12ba9ab9e6e9246d407648 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
== Testlist changes ==
+++ 25 lines
--- 91 lines
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4433/index.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 6+ messages in thread
* [igt-dev] ✓ Fi.CI.IGT: success for tests/i915/gem_sync.c :Added __for_each_physical_engine to utilize all available (rev6)
2020-04-08 15:12 [igt-dev] [PATCH] [PATCH i-g-t][V5]tests/i915/gem_sync.c :Added __for_each_physical_engine to utilize all available Arjun Melkaveri
2020-04-08 16:12 ` [igt-dev] ✗ GitLab.Pipeline: failure for tests/i915/gem_sync.c :Added __for_each_physical_engine to utilize all available (rev6) Patchwork
2020-04-08 16:22 ` [igt-dev] ✓ Fi.CI.BAT: success " Patchwork
@ 2020-04-09 3:20 ` Patchwork
2020-04-14 12:58 ` [igt-dev] [PATCH] [PATCH i-g-t][V5]tests/i915/gem_sync.c :Added __for_each_physical_engine to utilize all available Tvrtko Ursulin
3 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2020-04-09 3:20 UTC (permalink / raw)
To: Arjun Melkaveri; +Cc: igt-dev
== Series Details ==
Series: tests/i915/gem_sync.c :Added __for_each_physical_engine to utilize all available (rev6)
URL : https://patchwork.freedesktop.org/series/75536/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_8277_full -> IGTPW_4433_full
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4433/index.html
Known issues
------------
Here are the changes found in IGTPW_4433_full that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_exec_params@invalid-bsd-ring:
- shard-iclb: [PASS][1] -> [SKIP][2] ([fdo#109276])
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8277/shard-iclb2/igt@gem_exec_params@invalid-bsd-ring.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4433/shard-iclb8/igt@gem_exec_params@invalid-bsd-ring.html
* igt@gem_fenced_exec_thrash@no-spare-fences:
- shard-snb: [PASS][3] -> [INCOMPLETE][4] ([i915#82])
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8277/shard-snb4/igt@gem_fenced_exec_thrash@no-spare-fences.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4433/shard-snb5/igt@gem_fenced_exec_thrash@no-spare-fences.html
* igt@gen9_exec_parse@allowed-all:
- shard-glk: [PASS][5] -> [DMESG-WARN][6] ([i915#716])
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8277/shard-glk5/igt@gen9_exec_parse@allowed-all.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4433/shard-glk7/igt@gen9_exec_parse@allowed-all.html
* igt@kms_big_fb@linear-32bpp-rotate-0:
- shard-kbl: [PASS][7] -> [FAIL][8] ([i915#1119] / [i915#93] / [i915#95])
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8277/shard-kbl7/igt@kms_big_fb@linear-32bpp-rotate-0.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4433/shard-kbl4/igt@kms_big_fb@linear-32bpp-rotate-0.html
* igt@kms_cursor_crc@pipe-a-cursor-64x21-offscreen:
- shard-kbl: [PASS][9] -> [FAIL][10] ([i915#54] / [i915#93] / [i915#95]) +1 similar issue
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8277/shard-kbl1/igt@kms_cursor_crc@pipe-a-cursor-64x21-offscreen.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4433/shard-kbl2/igt@kms_cursor_crc@pipe-a-cursor-64x21-offscreen.html
* igt@kms_cursor_legacy@flip-vs-cursor-crc-atomic:
- shard-kbl: [PASS][11] -> [FAIL][12] ([i915#1566] / [i915#93] / [i915#95])
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8277/shard-kbl4/igt@kms_cursor_legacy@flip-vs-cursor-crc-atomic.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4433/shard-kbl6/igt@kms_cursor_legacy@flip-vs-cursor-crc-atomic.html
* igt@kms_draw_crc@draw-method-rgb565-pwrite-untiled:
- shard-glk: [PASS][13] -> [FAIL][14] ([i915#177] / [i915#52] / [i915#54])
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8277/shard-glk8/igt@kms_draw_crc@draw-method-rgb565-pwrite-untiled.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4433/shard-glk4/igt@kms_draw_crc@draw-method-rgb565-pwrite-untiled.html
* igt@kms_draw_crc@draw-method-rgb565-render-untiled:
- shard-glk: [PASS][15] -> [FAIL][16] ([i915#52] / [i915#54]) +3 similar issues
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8277/shard-glk9/igt@kms_draw_crc@draw-method-rgb565-render-untiled.html
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4433/shard-glk4/igt@kms_draw_crc@draw-method-rgb565-render-untiled.html
* igt@kms_flip@2x-flip-vs-expired-vblank-interruptible:
- shard-glk: [PASS][17] -> [FAIL][18] ([i915#79])
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8277/shard-glk6/igt@kms_flip@2x-flip-vs-expired-vblank-interruptible.html
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4433/shard-glk5/igt@kms_flip@2x-flip-vs-expired-vblank-interruptible.html
* igt@kms_flip@2x-flip-vs-suspend-interruptible:
- shard-hsw: [PASS][19] -> [INCOMPLETE][20] ([i915#61])
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8277/shard-hsw5/igt@kms_flip@2x-flip-vs-suspend-interruptible.html
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4433/shard-hsw1/igt@kms_flip@2x-flip-vs-suspend-interruptible.html
* igt@kms_flip@flip-vs-suspend-interruptible:
- shard-apl: [PASS][21] -> [DMESG-WARN][22] ([i915#180]) +1 similar issue
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8277/shard-apl2/igt@kms_flip@flip-vs-suspend-interruptible.html
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4433/shard-apl3/igt@kms_flip@flip-vs-suspend-interruptible.html
* igt@kms_mmap_write_crc@main:
- shard-kbl: [PASS][23] -> [FAIL][24] ([i915#93] / [i915#95])
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8277/shard-kbl1/igt@kms_mmap_write_crc@main.html
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4433/shard-kbl4/igt@kms_mmap_write_crc@main.html
* igt@kms_plane_cursor@pipe-a-viewport-size-256:
- shard-apl: [PASS][25] -> [FAIL][26] ([i915#1559] / [i915#95])
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8277/shard-apl6/igt@kms_plane_cursor@pipe-a-viewport-size-256.html
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4433/shard-apl3/igt@kms_plane_cursor@pipe-a-viewport-size-256.html
- shard-kbl: [PASS][27] -> [FAIL][28] ([i915#1559] / [i915#93] / [i915#95])
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8277/shard-kbl1/igt@kms_plane_cursor@pipe-a-viewport-size-256.html
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4433/shard-kbl7/igt@kms_plane_cursor@pipe-a-viewport-size-256.html
* igt@kms_psr@no_drrs:
- shard-iclb: [PASS][29] -> [FAIL][30] ([i915#173])
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8277/shard-iclb5/igt@kms_psr@no_drrs.html
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4433/shard-iclb1/igt@kms_psr@no_drrs.html
* igt@kms_psr@psr2_primary_mmap_cpu:
- shard-iclb: [PASS][31] -> [SKIP][32] ([fdo#109441]) +4 similar issues
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8277/shard-iclb2/igt@kms_psr@psr2_primary_mmap_cpu.html
[32]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4433/shard-iclb5/igt@kms_psr@psr2_primary_mmap_cpu.html
* igt@kms_setmode@basic:
- shard-apl: [PASS][33] -> [FAIL][34] ([i915#31])
[33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8277/shard-apl3/igt@kms_setmode@basic.html
[34]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4433/shard-apl7/igt@kms_setmode@basic.html
* igt@kms_vblank@pipe-a-ts-continuation-suspend:
- shard-kbl: [PASS][35] -> [DMESG-WARN][36] ([i915#180]) +3 similar issues
[35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8277/shard-kbl1/igt@kms_vblank@pipe-a-ts-continuation-suspend.html
[36]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4433/shard-kbl7/igt@kms_vblank@pipe-a-ts-continuation-suspend.html
#### Possible fixes ####
* igt@i915_pm_rc6_residency@rc6-idle:
- shard-snb: [FAIL][37] ([i915#1066]) -> [PASS][38]
[37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8277/shard-snb5/igt@i915_pm_rc6_residency@rc6-idle.html
[38]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4433/shard-snb7/igt@i915_pm_rc6_residency@rc6-idle.html
* igt@kms_busy@basic-flip-pipe-a:
- shard-kbl: [INCOMPLETE][39] -> [PASS][40]
[39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8277/shard-kbl7/igt@kms_busy@basic-flip-pipe-a.html
[40]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4433/shard-kbl4/igt@kms_busy@basic-flip-pipe-a.html
* igt@kms_cursor_crc@pipe-a-cursor-128x42-onscreen:
- shard-kbl: [FAIL][41] ([i915#54] / [i915#93] / [i915#95]) -> [PASS][42]
[41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8277/shard-kbl2/igt@kms_cursor_crc@pipe-a-cursor-128x42-onscreen.html
[42]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4433/shard-kbl6/igt@kms_cursor_crc@pipe-a-cursor-128x42-onscreen.html
* igt@kms_cursor_crc@pipe-a-cursor-64x64-onscreen:
- shard-kbl: [FAIL][43] ([i915#54]) -> [PASS][44] +1 similar issue
[43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8277/shard-kbl7/igt@kms_cursor_crc@pipe-a-cursor-64x64-onscreen.html
[44]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4433/shard-kbl3/igt@kms_cursor_crc@pipe-a-cursor-64x64-onscreen.html
- shard-apl: [FAIL][45] ([i915#54]) -> [PASS][46] +1 similar issue
[45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8277/shard-apl2/igt@kms_cursor_crc@pipe-a-cursor-64x64-onscreen.html
[46]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4433/shard-apl2/igt@kms_cursor_crc@pipe-a-cursor-64x64-onscreen.html
* igt@kms_cursor_crc@pipe-a-cursor-64x64-sliding:
- shard-apl: [FAIL][47] ([i915#54] / [i915#95]) -> [PASS][48]
[47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8277/shard-apl8/igt@kms_cursor_crc@pipe-a-cursor-64x64-sliding.html
[48]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4433/shard-apl7/igt@kms_cursor_crc@pipe-a-cursor-64x64-sliding.html
* igt@kms_draw_crc@draw-method-rgb565-render-ytiled:
- shard-glk: [FAIL][49] ([i915#52] / [i915#54]) -> [PASS][50] +4 similar issues
[49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8277/shard-glk5/igt@kms_draw_crc@draw-method-rgb565-render-ytiled.html
[50]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4433/shard-glk5/igt@kms_draw_crc@draw-method-rgb565-render-ytiled.html
* igt@kms_draw_crc@draw-method-xrgb8888-mmap-wc-ytiled:
- shard-apl: [FAIL][51] ([i915#52] / [i915#54] / [i915#95]) -> [PASS][52]
[51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8277/shard-apl1/igt@kms_draw_crc@draw-method-xrgb8888-mmap-wc-ytiled.html
[52]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4433/shard-apl7/igt@kms_draw_crc@draw-method-xrgb8888-mmap-wc-ytiled.html
* igt@kms_flip_tiling@flip-changes-tiling-y:
- shard-apl: [FAIL][53] ([i915#95]) -> [PASS][54]
[53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8277/shard-apl6/igt@kms_flip_tiling@flip-changes-tiling-y.html
[54]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4433/shard-apl1/igt@kms_flip_tiling@flip-changes-tiling-y.html
* igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes:
- shard-kbl: [DMESG-WARN][55] ([i915#180]) -> [PASS][56] +2 similar issues
[55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8277/shard-kbl1/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes.html
[56]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4433/shard-kbl3/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes.html
* igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes:
- shard-apl: [DMESG-WARN][57] ([i915#180]) -> [PASS][58] +1 similar issue
[57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8277/shard-apl6/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes.html
[58]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4433/shard-apl8/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes.html
* igt@kms_psr@psr2_cursor_blt:
- shard-iclb: [SKIP][59] ([fdo#109441]) -> [PASS][60]
[59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8277/shard-iclb5/igt@kms_psr@psr2_cursor_blt.html
[60]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4433/shard-iclb2/igt@kms_psr@psr2_cursor_blt.html
* {igt@perf@blocking-parameterized}:
- shard-iclb: [FAIL][61] ([i915#1542]) -> [PASS][62]
[61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8277/shard-iclb7/igt@perf@blocking-parameterized.html
[62]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4433/shard-iclb1/igt@perf@blocking-parameterized.html
#### Warnings ####
* igt@kms_fbcon_fbt@fbc:
- shard-kbl: [FAIL][63] ([i915#64]) -> [FAIL][64] ([i915#93] / [i915#95])
[63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8277/shard-kbl4/igt@kms_fbcon_fbt@fbc.html
[64]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4433/shard-kbl4/igt@kms_fbcon_fbt@fbc.html
- shard-apl: [FAIL][65] ([i915#1525]) -> [FAIL][66] ([i915#95])
[65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8277/shard-apl2/igt@kms_fbcon_fbt@fbc.html
[66]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4433/shard-apl6/igt@kms_fbcon_fbt@fbc.html
* igt@kms_fbcon_fbt@fbc-suspend:
- shard-kbl: [DMESG-FAIL][67] ([i915#180] / [i915#95]) -> [FAIL][68] ([i915#93] / [i915#95])
[67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8277/shard-kbl7/igt@kms_fbcon_fbt@fbc-suspend.html
[68]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4433/shard-kbl3/igt@kms_fbcon_fbt@fbc-suspend.html
* igt@kms_plane_alpha_blend@pipe-c-alpha-opaque-fb:
- shard-kbl: [FAIL][69] ([fdo#108145] / [i915#265] / [i915#93] / [i915#95]) -> [FAIL][70] ([fdo#108145] / [i915#265])
[69]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8277/shard-kbl1/igt@kms_plane_alpha_blend@pipe-c-alpha-opaque-fb.html
[70]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4433/shard-kbl7/igt@kms_plane_alpha_blend@pipe-c-alpha-opaque-fb.html
- shard-apl: [FAIL][71] ([fdo#108145] / [i915#265] / [i915#95]) -> [FAIL][72] ([fdo#108145] / [i915#265])
[71]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8277/shard-apl7/igt@kms_plane_alpha_blend@pipe-c-alpha-opaque-fb.html
[72]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4433/shard-apl8/igt@kms_plane_alpha_blend@pipe-c-alpha-opaque-fb.html
* igt@kms_psr2_su@page_flip:
- shard-iclb: [FAIL][73] ([i915#608]) -> [SKIP][74] ([fdo#109642] / [fdo#111068])
[73]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8277/shard-iclb2/igt@kms_psr2_su@page_flip.html
[74]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4433/shard-iclb1/igt@kms_psr2_su@page_flip.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
[fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
[fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
[fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
[fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
[i915#1066]: https://gitlab.freedesktop.org/drm/intel/issues/1066
[i915#1119]: https://gitlab.freedesktop.org/drm/intel/issues/1119
[i915#1525]: https://gitlab.freedesktop.org/drm/intel/issues/1525
[i915#1542]: https://gitlab.freedesktop.org/drm/intel/issues/1542
[i915#1559]: https://gitlab.freedesktop.org/drm/intel/issues/1559
[i915#1566]: https://gitlab.freedesktop.org/drm/intel/issues/1566
[i915#173]: https://gitlab.freedesktop.org/drm/intel/issues/173
[i915#177]: https://gitlab.freedesktop.org/drm/intel/issues/177
[i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
[i915#265]: https://gitlab.freedesktop.org/drm/intel/issues/265
[i915#31]: https://gitlab.freedesktop.org/drm/intel/issues/31
[i915#52]: https://gitlab.freedesktop.org/drm/intel/issues/52
[i915#54]: https://gitlab.freedesktop.org/drm/intel/issues/54
[i915#608]: https://gitlab.freedesktop.org/drm/intel/issues/608
[i915#61]: https://gitlab.freedesktop.org/drm/intel/issues/61
[i915#64]: https://gitlab.freedesktop.org/drm/intel/issues/64
[i915#716]: https://gitlab.freedesktop.org/drm/intel/issues/716
[i915#79]: https://gitlab.freedesktop.org/drm/intel/issues/79
[i915#82]: https://gitlab.freedesktop.org/drm/intel/issues/82
[i915#93]: https://gitlab.freedesktop.org/drm/intel/issues/93
[i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95
Participating hosts (10 -> 8)
------------------------------
Missing (2): pig-skl-6260u pig-glk-j5005
Build changes
-------------
* CI: CI-20190529 -> None
* IGT: IGT_5581 -> IGTPW_4433
* Piglit: piglit_4509 -> None
CI-20190529: 20190529
CI_DRM_8277: f7d56913e1668f3a269db391189a7888a4b22570 @ git://anongit.freedesktop.org/gfx-ci/linux
IGTPW_4433: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4433/index.html
IGT_5581: ab0620e555119ec55f12ba9ab9e6e9246d407648 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4433/index.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [igt-dev] [PATCH] [PATCH i-g-t][V5]tests/i915/gem_sync.c :Added __for_each_physical_engine to utilize all available
2020-04-08 15:12 [igt-dev] [PATCH] [PATCH i-g-t][V5]tests/i915/gem_sync.c :Added __for_each_physical_engine to utilize all available Arjun Melkaveri
` (2 preceding siblings ...)
2020-04-09 3:20 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
@ 2020-04-14 12:58 ` Tvrtko Ursulin
2020-04-14 14:01 ` Melkaveri, Arjun
3 siblings, 1 reply; 6+ messages in thread
From: Tvrtko Ursulin @ 2020-04-14 12:58 UTC (permalink / raw)
To: Arjun Melkaveri, igt-dev
On 08/04/2020 16:12, Arjun Melkaveri wrote:
> Replaced the legacy for_each_engine* defines with the ones
> implemented in the gem_engine_topology library.
>
> Used gem_context_clone_with_engines
> to make sure that engine index was potentially created
> based on a default context with engine map configured.
>
> Added gem_reopen_driver and gem_context_copy_engines
> to transfer the engine map from parent fd default
> context.
>
> V2:
> Added Legacy engine coverage for sync_ring and sync_all.
>
> V3:
> Added back ALL_ENGINES. Corrected Test cases that used
> gem_reopen_driver in fork. Which was not recommended.
>
> V4:
> Removed gem_require_ring and gem_can_store_dword.
>
> V5:
> Addressed the review comments.
> divided tests into static for ALL_Engine test cases
> and dynamic for multiple engine .
I see Chris has been giving you some feedback but I haven't been
following closely. In case of conflicting advice please double-check
with both. More below.
>
> Cc: Dec Katarzyna <katarzyna.dec@intel.com>
> Cc: Ursulin Tvrtko <tvrtko.ursulin@intel.com>
> Signed-off-by: sai gowtham <sai.gowtham.ch@intel.com>
> Signed-off-by: Arjun Melkaveri <arjun.melkaveri@intel.com>
> ---
> tests/i915/gem_sync.c | 290 +++++++++++++++++++++++++++---------------
> 1 file changed, 185 insertions(+), 105 deletions(-)
>
> diff --git a/tests/i915/gem_sync.c b/tests/i915/gem_sync.c
> index 2ef55ecc..a93fa63a 100644
> --- a/tests/i915/gem_sync.c
> +++ b/tests/i915/gem_sync.c
> @@ -24,6 +24,9 @@
> #include <time.h>
> #include <pthread.h>
>
> +#include "igt_debugfs.h"
> +#include "igt_dummyload.h"
> +#include "igt_gt.h"
> #include "igt.h"
> #include "igt_sysfs.h"
>
> @@ -37,6 +40,9 @@
> #define MIN_PRIO LOCAL_I915_CONTEXT_MIN_USER_PRIORITY
>
> #define ENGINE_MASK (I915_EXEC_RING_MASK | LOCAL_I915_EXEC_BSD_MASK)
> +#define RESET_TIMEOUT_MS (2 * MSEC_PER_SEC)
> +static unsigned long reset_timeout_ms = RESET_TIMEOUT_MS;
> +#define NSEC_PER_MSEC (1000 * 1000ull)
>
> IGT_TEST_DESCRIPTION("Basic check of ring<->ring write synchronisation.");
>
> @@ -78,24 +84,35 @@ out:
> return ts.tv_sec + 1e-9*ts.tv_nsec;
> }
>
> +static void cleanup(int i915)
> +{
> + igt_drop_caches_set(i915,
> + /* cancel everything */
> + DROP_RESET_ACTIVE | DROP_RESET_SEQNO |
> + /* cleanup */
> + DROP_ACTIVE | DROP_RETIRE | DROP_IDLE | DROP_FREED);
> + igt_require_gem(i915);
> +}
> +
> +
> static void
> sync_ring(int fd, unsigned ring, int num_children, int timeout)
> {
> + const struct intel_execution_engine2 *e2;
> unsigned engines[16];
> const char *names[16];
> int num_engines = 0;
>
> if (ring == ALL_ENGINES) {
> - for_each_physical_engine(e, fd) {
> - names[num_engines] = e->name;
> - engines[num_engines++] = eb_ring(e);
> + __for_each_physical_engine(fd, e2) {
> + names[num_engines] = e2->name;
> + engines[num_engines++] = e2->flags;
> if (num_engines == ARRAY_SIZE(engines))
> break;
> }
>
> num_children *= num_engines;
> } else {
> - gem_require_ring(fd, ring);
> names[num_engines] = NULL;
> engines[num_engines++] = ring;
> }
> @@ -139,7 +156,7 @@ sync_ring(int fd, unsigned ring, int num_children, int timeout)
> }
>
> static void
> -idle_ring(int fd, unsigned ring, int timeout)
> +idle_ring(int fd, unsigned int ring, int num_children, int timeout)
> {
> const uint32_t bbe = MI_BATCH_BUFFER_END;
> struct drm_i915_gem_exec_object2 object;
> @@ -180,24 +197,23 @@ idle_ring(int fd, unsigned ring, int timeout)
> static void
> wakeup_ring(int fd, unsigned ring, int timeout, int wlen)
> {
> + const struct intel_execution_engine2 *e2;
> unsigned engines[16];
> const char *names[16];
> int num_engines = 0;
>
> if (ring == ALL_ENGINES) {
> - for_each_physical_engine(e, fd) {
> - if (!gem_can_store_dword(fd, eb_ring(e)))
> + __for_each_physical_engine(fd, e2) {
> + if (!gem_class_can_store_dword(fd, e2->class))
> continue;
>
> - names[num_engines] = e->name;
> - engines[num_engines++] = eb_ring(e);
> + names[num_engines] = e2->name;
> + engines[num_engines++] = e2->flags;
> if (num_engines == ARRAY_SIZE(engines))
> break;
> }
> igt_require(num_engines);
> } else {
> - gem_require_ring(fd, ring);
> - igt_require(gem_can_store_dword(fd, ring));
Why it is okay to remove the can_store_dword check?
> names[num_engines] = NULL;
> engines[num_engines++] = ring;
> }
> @@ -290,26 +306,26 @@ wakeup_ring(int fd, unsigned ring, int timeout, int wlen)
> igt_assert_eq(intel_detect_and_clear_missed_interrupts(fd), 0);
> }
>
> -static void active_ring(int fd, unsigned ring, int timeout)
> +static void active_ring(int fd, unsigned int ring,
> + int num_children, int timeout)
> {
> + const struct intel_execution_engine2 *e2;
> unsigned engines[16];
> const char *names[16];
> int num_engines = 0;
>
> if (ring == ALL_ENGINES) {
> - for_each_physical_engine(e, fd) {
> - if (!gem_can_store_dword(fd, eb_ring(e)))
> + __for_each_physical_engine(fd, e2) {
> + if (!gem_class_can_store_dword(fd, e2->class))
> continue;
>
> - names[num_engines] = e->name;
> - engines[num_engines++] = eb_ring(e);
> + names[num_engines] = e2->name;
> + engines[num_engines++] = e2->flags;
> if (num_engines == ARRAY_SIZE(engines))
> break;
> }
> igt_require(num_engines);
> } else {
> - gem_require_ring(fd, ring);
> - igt_require(gem_can_store_dword(fd, ring));
Same here and more below.
> names[num_engines] = NULL;
> engines[num_engines++] = ring;
> }
> @@ -359,24 +375,23 @@ static void active_ring(int fd, unsigned ring, int timeout)
> static void
> active_wakeup_ring(int fd, unsigned ring, int timeout, int wlen)
> {
> + const struct intel_execution_engine2 *e2;
> unsigned engines[16];
> const char *names[16];
> int num_engines = 0;
>
> if (ring == ALL_ENGINES) {
> - for_each_physical_engine(e, fd) {
> - if (!gem_can_store_dword(fd, eb_ring(e)))
> + __for_each_physical_engine(fd, e2) {
> + if (!gem_class_can_store_dword(fd, e2->class))
> continue;
>
> - names[num_engines] = e->name;
> - engines[num_engines++] = eb_ring(e);
> + names[num_engines] = e2->name;
> + engines[num_engines++] = e2->flags;
> if (num_engines == ARRAY_SIZE(engines))
> break;
> }
> igt_require(num_engines);
> } else {
> - gem_require_ring(fd, ring);
> - igt_require(gem_can_store_dword(fd, ring));
> names[num_engines] = NULL;
> engines[num_engines++] = ring;
> }
> @@ -493,26 +508,25 @@ active_wakeup_ring(int fd, unsigned ring, int timeout, int wlen)
> static void
> store_ring(int fd, unsigned ring, int num_children, int timeout)
> {
> + const struct intel_execution_engine2 *e2;
> const int gen = intel_gen(intel_get_drm_devid(fd));
> unsigned engines[16];
> const char *names[16];
> int num_engines = 0;
>
> if (ring == ALL_ENGINES) {
> - for_each_physical_engine(e, fd) {
> - if (!gem_can_store_dword(fd, eb_ring(e)))
> + __for_each_physical_engine(fd, e2) {
> + if (!gem_class_can_store_dword(fd, e2->class))
> continue;
>
> - names[num_engines] = e->name;
> - engines[num_engines++] = eb_ring(e);
> + names[num_engines] = e2->name;
> + engines[num_engines++] = e2->flags;
> if (num_engines == ARRAY_SIZE(engines))
> break;
> }
>
> num_children *= num_engines;
> } else {
> - gem_require_ring(fd, ring);
> - igt_require(gem_can_store_dword(fd, ring));
> names[num_engines] = NULL;
> engines[num_engines++] = ring;
> }
> @@ -608,6 +622,7 @@ store_ring(int fd, unsigned ring, int num_children, int timeout)
> static void
> switch_ring(int fd, unsigned ring, int num_children, int timeout)
> {
> + const struct intel_execution_engine2 *e2;
> const int gen = intel_gen(intel_get_drm_devid(fd));
> unsigned engines[16];
> const char *names[16];
> @@ -616,20 +631,18 @@ switch_ring(int fd, unsigned ring, int num_children, int timeout)
> gem_require_contexts(fd);
>
> if (ring == ALL_ENGINES) {
> - for_each_physical_engine(e, fd) {
> - if (!gem_can_store_dword(fd, eb_ring(e)))
> + __for_each_physical_engine(fd, e2) {
> + if (!gem_class_can_store_dword(fd, e2->class))
> continue;
>
> - names[num_engines] = e->name;
> - engines[num_engines++] = eb_ring(e);
> + names[num_engines] = e2->name;
> + engines[num_engines++] = e2->flags;
> if (num_engines == ARRAY_SIZE(engines))
> break;
> }
>
> num_children *= num_engines;
> } else {
> - gem_require_ring(fd, ring);
> - igt_require(gem_can_store_dword(fd, ring));
> names[num_engines] = NULL;
> engines[num_engines++] = ring;
> }
> @@ -931,8 +944,9 @@ __store_many(int fd, unsigned ring, int timeout, unsigned long *cycles)
> }
>
> static void
> -store_many(int fd, unsigned ring, int timeout)
> +store_many(int fd, unsigned int ring, int num_children, int timeout)
> {
> + const struct intel_execution_engine2 *e2;
> unsigned long *shared;
> const char *names[16];
> int n = 0;
> @@ -943,22 +957,20 @@ store_many(int fd, unsigned ring, int timeout)
> intel_detect_and_clear_missed_interrupts(fd);
>
> if (ring == ALL_ENGINES) {
> - for_each_physical_engine(e, fd) {
> - if (!gem_can_store_dword(fd, eb_ring(e)))
> + __for_each_physical_engine(fd, e2) {
> + if (!gem_class_can_store_dword(fd, e2->class))
> continue;
>
> igt_fork(child, 1)
> __store_many(fd,
> - eb_ring(e),
> + e2->flags,
> timeout,
> &shared[n]);
>
> - names[n++] = e->name;
> + names[n++] = e2->name;
> }
> igt_waitchildren();
> } else {
> - gem_require_ring(fd, ring);
> - igt_require(gem_can_store_dword(fd, ring));
> __store_many(fd, ring, timeout, &shared[n]);
> names[n++] = NULL;
> }
> @@ -1025,15 +1037,16 @@ sync_all(int fd, int num_children, int timeout)
> static void
> store_all(int fd, int num_children, int timeout)
> {
> + const struct intel_execution_engine2 *e;
> const int gen = intel_gen(intel_get_drm_devid(fd));
> unsigned engines[16];
> int num_engines = 0;
>
> - for_each_physical_engine(e, fd) {
> - if (!gem_can_store_dword(fd, eb_ring(e)))
> + __for_each_physical_engine(fd, e) {
> + if (!gem_class_can_store_dword(fd, e->class))
> continue;
>
> - engines[num_engines++] = eb_ring(e);
> + engines[num_engines++] = e->flags;
> if (num_engines == ARRAY_SIZE(engines))
> break;
> }
> @@ -1132,22 +1145,22 @@ store_all(int fd, int num_children, int timeout)
> static void
> preempt(int fd, unsigned ring, int num_children, int timeout)
> {
> + const struct intel_execution_engine2 *e2;
> unsigned engines[16];
> const char *names[16];
> int num_engines = 0;
> uint32_t ctx[2];
>
> if (ring == ALL_ENGINES) {
> - for_each_physical_engine(e, fd) {
> - names[num_engines] = e->name;
> - engines[num_engines++] = eb_ring(e);
> + __for_each_physical_engine(fd, e2) {
> + names[num_engines] = e2->name;
> + engines[num_engines++] = e2->flags;
> if (num_engines == ARRAY_SIZE(engines))
> break;
> }
>
> num_children *= num_engines;
> } else {
> - gem_require_ring(fd, ring);
> names[num_engines] = NULL;
> engines[num_engines++] = ring;
> }
> @@ -1207,76 +1220,99 @@ preempt(int fd, unsigned ring, int num_children, int timeout)
> gem_context_destroy(fd, ctx[0]);
> }
>
> +static void do_test(void (*test)(int i915, unsigned int engine,
> + int num_children, int test_timeout),
> + int i915, unsigned int engine, int num_children,
> + int test_timeout, const char *name)
> +{
> +#define ATTR "preempt_timeout_ms"
> + int timeout = -1;
> +
> + cleanup(i915);
> +
> + gem_engine_property_scanf(i915, name, ATTR, "%d", &timeout);
> + if (timeout != -1) {
> + igt_require(gem_engine_property_printf(i915, name,
> + ATTR, "%d", 50) > 0);
> + reset_timeout_ms = 200;
> + }
> +
> + test(i915, engine, num_children, test_timeout);
> +
> + if (timeout != -1) {
> + gem_engine_property_printf(i915, name, ATTR, "%d", timeout);
> + reset_timeout_ms = RESET_TIMEOUT_MS;
> + }
> +
> + gem_quiescent_gpu(i915);
> +}
Not sure what's this wrapper - reset_timeout_ms seems unused?
> +
> igt_main
> {
> - const struct intel_execution_engine *e;
> const int ncpus = sysconf(_SC_NPROCESSORS_ONLN);
> int fd = -1;
>
> + struct {
> + const char *name;
> + void (*func)(int fd, unsigned int engine,
> + int num_children, int timeout);
> + int num_children;
> + int timeout;
> + } *test, AllEngine[] = {
If you insist, but don't think we use camel case anywhere else. :I
> + { "basic-each", sync_ring, 1, 2},
> + { "basic-store-each", store_ring, 1, 2},
> + { "basic-many-each", store_many, 0, 2},
> + { "switch-each", switch_ring, 1, 20},
> + { "forked-switch-each", switch_ring, ncpus, 20},
> + { "forked-each", sync_ring, ncpus, 20},
> + { "forked-store-each", store_ring, ncpus, 20},
> + { "active-each", active_ring, 0, 20},
> + { "wakeup-each", wakeup_ring, 20, 1},
> + { "active-wakeup-each", active_wakeup_ring, 20, 1},
> + { "double-wakeup-each", wakeup_ring, 20, 2},
> + { NULL, NULL },
> + }, tests[] = {
> + { "default", sync_ring, 1, 20},
> + { "idle", idle_ring, 0, 20},
> + { "active", active_ring, 0, 20},
> + { "wakeup", wakeup_ring, 20, 1},
> + { "active-wakeup", active_wakeup_ring, 20, 1},
> + { "double-wakeup", wakeup_ring, 20, 2},
> + { "store", store_ring, 1, 20},
> + { "switch", switch_ring, 1, 20},
> + { "forked-switch", switch_ring, ncpus, 20},
> + { "many", store_many, 0, 20},
> + { "forked", sync_ring, ncpus, 20},
> + { "forked-store", store_ring, ncpus, 20},
> + { NULL, NULL },
> + };
> +
> +
> igt_fixture {
> fd = drm_open_driver(DRIVER_INTEL);
> igt_require_gem(fd);
> gem_submission_print_method(fd);
> gem_scheduler_print_capability(fd);
> -
> igt_fork_hang_detector(fd);
> }
>
> - for (e = intel_execution_engines; e->name; e++) {
> - igt_subtest_f("%s", e->name)
> - sync_ring(fd, eb_ring(e), 1, 20);
> - igt_subtest_f("idle-%s", e->name)
> - idle_ring(fd, eb_ring(e), 20);
> - igt_subtest_f("active-%s", e->name)
> - active_ring(fd, eb_ring(e), 20);
> - igt_subtest_f("wakeup-%s", e->name)
> - wakeup_ring(fd, eb_ring(e), 20, 1);
> - igt_subtest_f("active-wakeup-%s", e->name)
> - active_wakeup_ring(fd, eb_ring(e), 20, 1);
> - igt_subtest_f("double-wakeup-%s", e->name)
> - wakeup_ring(fd, eb_ring(e), 20, 2);
> - igt_subtest_f("store-%s", e->name)
> - store_ring(fd, eb_ring(e), 1, 20);
> - igt_subtest_f("switch-%s", e->name)
> - switch_ring(fd, eb_ring(e), 1, 20);
> - igt_subtest_f("forked-switch-%s", e->name)
> - switch_ring(fd, eb_ring(e), ncpus, 20);
> - igt_subtest_f("many-%s", e->name)
> - store_many(fd, eb_ring(e), 20);
> - igt_subtest_f("forked-%s", e->name)
> - sync_ring(fd, eb_ring(e), ncpus, 20);
> - igt_subtest_f("forked-store-%s", e->name)
> - store_ring(fd, eb_ring(e), ncpus, 20);
> + /* Legacy testing must be first. */
> + igt_subtest_group {
> + for (test = AllEngine; test->name; test++) {
> + igt_subtest_f("%s", test->name) {
> + do_test(test->func,
> + fd, ALL_ENGINES,
> + test->num_children,
> + test->timeout,
> + test->name);
> + }
On a quick glance, so I may be missing something, problem here is I
think that the first subtest will convert the default context to engine
map so nothing after the first subtest will be legacy any more.
Therefore I think...
> + }
> }
>
> - igt_subtest("basic-each")
> - sync_ring(fd, ALL_ENGINES, 1, 2);
> - igt_subtest("basic-store-each")
> - store_ring(fd, ALL_ENGINES, 1, 2);
> - igt_subtest("basic-many-each")
> - store_many(fd, ALL_ENGINES, 2);
> - igt_subtest("switch-each")
> - switch_ring(fd, ALL_ENGINES, 1, 20);
> - igt_subtest("forked-switch-each")
> - switch_ring(fd, ALL_ENGINES, ncpus, 20);
> - igt_subtest("forked-each")
> - sync_ring(fd, ALL_ENGINES, ncpus, 20);
> - igt_subtest("forked-store-each")
> - store_ring(fd, ALL_ENGINES, ncpus, 20);
> - igt_subtest("active-each")
> - active_ring(fd, ALL_ENGINES, 20);
> - igt_subtest("wakeup-each")
> - wakeup_ring(fd, ALL_ENGINES, 20, 1);
> - igt_subtest("active-wakeup-each")
> - active_wakeup_ring(fd, ALL_ENGINES, 20, 1);
> - igt_subtest("double-wakeup-each")
> - wakeup_ring(fd, ALL_ENGINES, 20, 2);
> -
> igt_subtest("basic-all")
> sync_all(fd, 1, 2);
> igt_subtest("basic-store-all")
> store_all(fd, 1, 2);
> -
> igt_subtest("all")
> sync_all(fd, 1, 20);
> igt_subtest("store-all")
> @@ -1286,7 +1322,49 @@ igt_main
> igt_subtest("forked-store-all")
> store_all(fd, ncpus, 20);
>
> + /* legacy of selecting engines. */
> +
> + igt_subtest_group {
> + for (test = tests; test->name; test++) {
> + igt_subtest_with_dynamic_f("legacy-engines-%s",
> + test->name) {
> + for_each_physical_engine(e, fd) {
> + igt_dynamic_f("%s", e->name) {
> + do_test(test->func,
> + fd, eb_ring(e),
> + test->num_children,
> + test->timeout,
> + e->full_name);
> + }
> + }
> + }
> + }
> + }
... this block should come first. Then the ALL_ENGINES block is not
legacy engine selection due __for_eacy_physical_engine usage.
> +
> + /* New way of selecting engines. */
> +
> igt_subtest_group {
> + const struct intel_execution_engine2 *e;
> +
> + for (test = tests; test->name; test++) {
> + igt_subtest_with_dynamic_f("engines-%s", test->name) {
I'd be tempted to drop the "engines" from all subtest names since it
feels redundant. There is "each" in tests which do all engines so that
should be enough.
> + __for_each_physical_engine(fd, e) {
> + igt_dynamic_f("%s", e->name) {
> + do_test(test->func,
> + fd, e->flags,
> + test->num_children,
> + test->timeout,
> + e->name);
> + }
> + }
> + }
> + }
> + }
> +
> +
> + igt_subtest_group {
> + const struct intel_execution_engine2 *e;
> +
> igt_fixture {
> gem_require_contexts(fd);
> igt_require(gem_scheduler_has_ctx_priority(fd));
> @@ -1295,10 +1373,12 @@ igt_main
>
> igt_subtest("preempt-all")
> preempt(fd, ALL_ENGINES, 1, 20);
> -
> - for (e = intel_execution_engines; e->name; e++) {
> - igt_subtest_f("preempt-%s", e->name)
> - preempt(fd, eb_ring(e), ncpus, 20);
> + igt_subtest_with_dynamic("preempt") {
> + __for_each_physical_engine(fd, e) {
> + /* Requires master for STORE_DWORD on gen4/5 */
> + igt_dynamic_f("%s", e->name)
> + preempt(fd, e->flags, ncpus, 20);
> + }
> }
> }
>
>
Regards,
Tvrtko
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [igt-dev] [PATCH] [PATCH i-g-t][V5]tests/i915/gem_sync.c :Added __for_each_physical_engine to utilize all available
2020-04-14 12:58 ` [igt-dev] [PATCH] [PATCH i-g-t][V5]tests/i915/gem_sync.c :Added __for_each_physical_engine to utilize all available Tvrtko Ursulin
@ 2020-04-14 14:01 ` Melkaveri, Arjun
0 siblings, 0 replies; 6+ messages in thread
From: Melkaveri, Arjun @ 2020-04-14 14:01 UTC (permalink / raw)
To: Tvrtko Ursulin, igt-dev
Thanks
Arjun M
-----Original Message-----
From: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Sent: Tuesday, April 14, 2020 6:28 PM
To: Melkaveri, Arjun <arjun.melkaveri@intel.com>; igt-dev@lists.freedesktop.org
Subject: Re: [igt-dev] [PATCH] [PATCH i-g-t][V5]tests/i915/gem_sync.c :Added __for_each_physical_engine to utilize all available
On 08/04/2020 16:12, Arjun Melkaveri wrote:
> Replaced the legacy for_each_engine* defines with the ones implemented
> in the gem_engine_topology library.
>
> Used gem_context_clone_with_engines
> to make sure that engine index was potentially created based on a
> default context with engine map configured.
>
> Added gem_reopen_driver and gem_context_copy_engines to transfer the
> engine map from parent fd default context.
>
> V2:
> Added Legacy engine coverage for sync_ring and sync_all.
>
> V3:
> Added back ALL_ENGINES. Corrected Test cases that used
> gem_reopen_driver in fork. Which was not recommended.
>
> V4:
> Removed gem_require_ring and gem_can_store_dword.
>
> V5:
> Addressed the review comments.
> divided tests into static for ALL_Engine test cases
> and dynamic for multiple engine .
I see Chris has been giving you some feedback but I haven't been following closely. In case of conflicting advice please double-check with both. More below.
>
> Cc: Dec Katarzyna <katarzyna.dec@intel.com>
> Cc: Ursulin Tvrtko <tvrtko.ursulin@intel.com>
> Signed-off-by: sai gowtham <sai.gowtham.ch@intel.com>
> Signed-off-by: Arjun Melkaveri <arjun.melkaveri@intel.com>
> ---
> tests/i915/gem_sync.c | 290 +++++++++++++++++++++++++++---------------
> 1 file changed, 185 insertions(+), 105 deletions(-)
>
> diff --git a/tests/i915/gem_sync.c b/tests/i915/gem_sync.c index
> 2ef55ecc..a93fa63a 100644
> --- a/tests/i915/gem_sync.c
> +++ b/tests/i915/gem_sync.c
> @@ -24,6 +24,9 @@
> #include <time.h>
> #include <pthread.h>
>
> +#include "igt_debugfs.h"
> +#include "igt_dummyload.h"
> +#include "igt_gt.h"
> #include "igt.h"
> #include "igt_sysfs.h"
>
> @@ -37,6 +40,9 @@
> #define MIN_PRIO LOCAL_I915_CONTEXT_MIN_USER_PRIORITY
>
> #define ENGINE_MASK (I915_EXEC_RING_MASK |
> LOCAL_I915_EXEC_BSD_MASK)
> +#define RESET_TIMEOUT_MS (2 * MSEC_PER_SEC) static unsigned long
> +reset_timeout_ms = RESET_TIMEOUT_MS; #define NSEC_PER_MSEC (1000 *
> +1000ull)
>
> IGT_TEST_DESCRIPTION("Basic check of ring<->ring write
> synchronisation.");
>
> @@ -78,24 +84,35 @@ out:
> return ts.tv_sec + 1e-9*ts.tv_nsec;
> }
>
> +static void cleanup(int i915)
> +{
> + igt_drop_caches_set(i915,
> + /* cancel everything */
> + DROP_RESET_ACTIVE | DROP_RESET_SEQNO |
> + /* cleanup */
> + DROP_ACTIVE | DROP_RETIRE | DROP_IDLE | DROP_FREED);
> + igt_require_gem(i915);
> +}
> +
> +
> static void
> sync_ring(int fd, unsigned ring, int num_children, int timeout)
> {
> + const struct intel_execution_engine2 *e2;
> unsigned engines[16];
> const char *names[16];
> int num_engines = 0;
>
> if (ring == ALL_ENGINES) {
> - for_each_physical_engine(e, fd) {
> - names[num_engines] = e->name;
> - engines[num_engines++] = eb_ring(e);
> + __for_each_physical_engine(fd, e2) {
> + names[num_engines] = e2->name;
> + engines[num_engines++] = e2->flags;
> if (num_engines == ARRAY_SIZE(engines))
> break;
> }
>
> num_children *= num_engines;
> } else {
> - gem_require_ring(fd, ring);
> names[num_engines] = NULL;
> engines[num_engines++] = ring;
> }
> @@ -139,7 +156,7 @@ sync_ring(int fd, unsigned ring, int num_children, int timeout)
> }
>
> static void
> -idle_ring(int fd, unsigned ring, int timeout)
> +idle_ring(int fd, unsigned int ring, int num_children, int timeout)
> {
> const uint32_t bbe = MI_BATCH_BUFFER_END;
> struct drm_i915_gem_exec_object2 object; @@ -180,24 +197,23 @@
> idle_ring(int fd, unsigned ring, int timeout)
> static void
> wakeup_ring(int fd, unsigned ring, int timeout, int wlen)
> {
> + const struct intel_execution_engine2 *e2;
> unsigned engines[16];
> const char *names[16];
> int num_engines = 0;
>
> if (ring == ALL_ENGINES) {
> - for_each_physical_engine(e, fd) {
> - if (!gem_can_store_dword(fd, eb_ring(e)))
> + __for_each_physical_engine(fd, e2) {
> + if (!gem_class_can_store_dword(fd, e2->class))
> continue;
>
> - names[num_engines] = e->name;
> - engines[num_engines++] = eb_ring(e);
> + names[num_engines] = e2->name;
> + engines[num_engines++] = e2->flags;
> if (num_engines == ARRAY_SIZE(engines))
> break;
> }
> igt_require(num_engines);
> } else {
> - gem_require_ring(fd, ring);
> - igt_require(gem_can_store_dword(fd, ring));
Why it is okay to remove the can_store_dword check?
That’s my mistake, I had to replace gem_can_store_dword with gem_class_can_store_dword and modified here too .Will revert this
> names[num_engines] = NULL;
> engines[num_engines++] = ring;
> }
> @@ -290,26 +306,26 @@ wakeup_ring(int fd, unsigned ring, int timeout, int wlen)
> igt_assert_eq(intel_detect_and_clear_missed_interrupts(fd), 0);
> }
>
> -static void active_ring(int fd, unsigned ring, int timeout)
> +static void active_ring(int fd, unsigned int ring,
> + int num_children, int timeout)
> {
> + const struct intel_execution_engine2 *e2;
> unsigned engines[16];
> const char *names[16];
> int num_engines = 0;
>
> if (ring == ALL_ENGINES) {
> - for_each_physical_engine(e, fd) {
> - if (!gem_can_store_dword(fd, eb_ring(e)))
> + __for_each_physical_engine(fd, e2) {
> + if (!gem_class_can_store_dword(fd, e2->class))
> continue;
>
> - names[num_engines] = e->name;
> - engines[num_engines++] = eb_ring(e);
> + names[num_engines] = e2->name;
> + engines[num_engines++] = e2->flags;
> if (num_engines == ARRAY_SIZE(engines))
> break;
> }
> igt_require(num_engines);
> } else {
> - gem_require_ring(fd, ring);
> - igt_require(gem_can_store_dword(fd, ring));
Same here and more below.
> names[num_engines] = NULL;
> engines[num_engines++] = ring;
> }
> @@ -359,24 +375,23 @@ static void active_ring(int fd, unsigned ring, int timeout)
> static void
> active_wakeup_ring(int fd, unsigned ring, int timeout, int wlen)
> {
> + const struct intel_execution_engine2 *e2;
> unsigned engines[16];
> const char *names[16];
> int num_engines = 0;
>
> if (ring == ALL_ENGINES) {
> - for_each_physical_engine(e, fd) {
> - if (!gem_can_store_dword(fd, eb_ring(e)))
> + __for_each_physical_engine(fd, e2) {
> + if (!gem_class_can_store_dword(fd, e2->class))
> continue;
>
> - names[num_engines] = e->name;
> - engines[num_engines++] = eb_ring(e);
> + names[num_engines] = e2->name;
> + engines[num_engines++] = e2->flags;
> if (num_engines == ARRAY_SIZE(engines))
> break;
> }
> igt_require(num_engines);
> } else {
> - gem_require_ring(fd, ring);
> - igt_require(gem_can_store_dword(fd, ring));
> names[num_engines] = NULL;
> engines[num_engines++] = ring;
> }
> @@ -493,26 +508,25 @@ active_wakeup_ring(int fd, unsigned ring, int timeout, int wlen)
> static void
> store_ring(int fd, unsigned ring, int num_children, int timeout)
> {
> + const struct intel_execution_engine2 *e2;
> const int gen = intel_gen(intel_get_drm_devid(fd));
> unsigned engines[16];
> const char *names[16];
> int num_engines = 0;
>
> if (ring == ALL_ENGINES) {
> - for_each_physical_engine(e, fd) {
> - if (!gem_can_store_dword(fd, eb_ring(e)))
> + __for_each_physical_engine(fd, e2) {
> + if (!gem_class_can_store_dword(fd, e2->class))
> continue;
>
> - names[num_engines] = e->name;
> - engines[num_engines++] = eb_ring(e);
> + names[num_engines] = e2->name;
> + engines[num_engines++] = e2->flags;
> if (num_engines == ARRAY_SIZE(engines))
> break;
> }
>
> num_children *= num_engines;
> } else {
> - gem_require_ring(fd, ring);
> - igt_require(gem_can_store_dword(fd, ring));
> names[num_engines] = NULL;
> engines[num_engines++] = ring;
> }
> @@ -608,6 +622,7 @@ store_ring(int fd, unsigned ring, int num_children, int timeout)
> static void
> switch_ring(int fd, unsigned ring, int num_children, int timeout)
> {
> + const struct intel_execution_engine2 *e2;
> const int gen = intel_gen(intel_get_drm_devid(fd));
> unsigned engines[16];
> const char *names[16];
> @@ -616,20 +631,18 @@ switch_ring(int fd, unsigned ring, int num_children, int timeout)
> gem_require_contexts(fd);
>
> if (ring == ALL_ENGINES) {
> - for_each_physical_engine(e, fd) {
> - if (!gem_can_store_dword(fd, eb_ring(e)))
> + __for_each_physical_engine(fd, e2) {
> + if (!gem_class_can_store_dword(fd, e2->class))
> continue;
>
> - names[num_engines] = e->name;
> - engines[num_engines++] = eb_ring(e);
> + names[num_engines] = e2->name;
> + engines[num_engines++] = e2->flags;
> if (num_engines == ARRAY_SIZE(engines))
> break;
> }
>
> num_children *= num_engines;
> } else {
> - gem_require_ring(fd, ring);
> - igt_require(gem_can_store_dword(fd, ring));
> names[num_engines] = NULL;
> engines[num_engines++] = ring;
> }
> @@ -931,8 +944,9 @@ __store_many(int fd, unsigned ring, int timeout, unsigned long *cycles)
> }
>
> static void
> -store_many(int fd, unsigned ring, int timeout)
> +store_many(int fd, unsigned int ring, int num_children, int timeout)
> {
> + const struct intel_execution_engine2 *e2;
> unsigned long *shared;
> const char *names[16];
> int n = 0;
> @@ -943,22 +957,20 @@ store_many(int fd, unsigned ring, int timeout)
> intel_detect_and_clear_missed_interrupts(fd);
>
> if (ring == ALL_ENGINES) {
> - for_each_physical_engine(e, fd) {
> - if (!gem_can_store_dword(fd, eb_ring(e)))
> + __for_each_physical_engine(fd, e2) {
> + if (!gem_class_can_store_dword(fd, e2->class))
> continue;
>
> igt_fork(child, 1)
> __store_many(fd,
> - eb_ring(e),
> + e2->flags,
> timeout,
> &shared[n]);
>
> - names[n++] = e->name;
> + names[n++] = e2->name;
> }
> igt_waitchildren();
> } else {
> - gem_require_ring(fd, ring);
> - igt_require(gem_can_store_dword(fd, ring));
> __store_many(fd, ring, timeout, &shared[n]);
> names[n++] = NULL;
> }
> @@ -1025,15 +1037,16 @@ sync_all(int fd, int num_children, int timeout)
> static void
> store_all(int fd, int num_children, int timeout)
> {
> + const struct intel_execution_engine2 *e;
> const int gen = intel_gen(intel_get_drm_devid(fd));
> unsigned engines[16];
> int num_engines = 0;
>
> - for_each_physical_engine(e, fd) {
> - if (!gem_can_store_dword(fd, eb_ring(e)))
> + __for_each_physical_engine(fd, e) {
> + if (!gem_class_can_store_dword(fd, e->class))
> continue;
>
> - engines[num_engines++] = eb_ring(e);
> + engines[num_engines++] = e->flags;
> if (num_engines == ARRAY_SIZE(engines))
> break;
> }
> @@ -1132,22 +1145,22 @@ store_all(int fd, int num_children, int timeout)
> static void
> preempt(int fd, unsigned ring, int num_children, int timeout)
> {
> + const struct intel_execution_engine2 *e2;
> unsigned engines[16];
> const char *names[16];
> int num_engines = 0;
> uint32_t ctx[2];
>
> if (ring == ALL_ENGINES) {
> - for_each_physical_engine(e, fd) {
> - names[num_engines] = e->name;
> - engines[num_engines++] = eb_ring(e);
> + __for_each_physical_engine(fd, e2) {
> + names[num_engines] = e2->name;
> + engines[num_engines++] = e2->flags;
> if (num_engines == ARRAY_SIZE(engines))
> break;
> }
>
> num_children *= num_engines;
> } else {
> - gem_require_ring(fd, ring);
> names[num_engines] = NULL;
> engines[num_engines++] = ring;
> }
> @@ -1207,76 +1220,99 @@ preempt(int fd, unsigned ring, int num_children, int timeout)
> gem_context_destroy(fd, ctx[0]);
> }
>
> +static void do_test(void (*test)(int i915, unsigned int engine,
> + int num_children, int test_timeout),
> + int i915, unsigned int engine, int num_children,
> + int test_timeout, const char *name) { #define ATTR
> +"preempt_timeout_ms"
> + int timeout = -1;
> +
> + cleanup(i915);
> +
> + gem_engine_property_scanf(i915, name, ATTR, "%d", &timeout);
> + if (timeout != -1) {
> + igt_require(gem_engine_property_printf(i915, name,
> + ATTR, "%d", 50) > 0);
> + reset_timeout_ms = 200;
> + }
> +
> + test(i915, engine, num_children, test_timeout);
> +
> + if (timeout != -1) {
> + gem_engine_property_printf(i915, name, ATTR, "%d", timeout);
> + reset_timeout_ms = RESET_TIMEOUT_MS;
> + }
> +
> + gem_quiescent_gpu(i915);
> +}
Not sure what's this wrapper - reset_timeout_ms seems unused?
Will recheck this
> +
> igt_main
> {
> - const struct intel_execution_engine *e;
> const int ncpus = sysconf(_SC_NPROCESSORS_ONLN);
> int fd = -1;
>
> + struct {
> + const char *name;
> + void (*func)(int fd, unsigned int engine,
> + int num_children, int timeout);
> + int num_children;
> + int timeout;
> + } *test, AllEngine[] = {
If you insist, but don't think we use camel case anywhere else. :I
Will change this to Allengines
> + { "basic-each", sync_ring, 1, 2},
> + { "basic-store-each", store_ring, 1, 2},
> + { "basic-many-each", store_many, 0, 2},
> + { "switch-each", switch_ring, 1, 20},
> + { "forked-switch-each", switch_ring, ncpus, 20},
> + { "forked-each", sync_ring, ncpus, 20},
> + { "forked-store-each", store_ring, ncpus, 20},
> + { "active-each", active_ring, 0, 20},
> + { "wakeup-each", wakeup_ring, 20, 1},
> + { "active-wakeup-each", active_wakeup_ring, 20, 1},
> + { "double-wakeup-each", wakeup_ring, 20, 2},
> + { NULL, NULL },
> + }, tests[] = {
> + { "default", sync_ring, 1, 20},
> + { "idle", idle_ring, 0, 20},
> + { "active", active_ring, 0, 20},
> + { "wakeup", wakeup_ring, 20, 1},
> + { "active-wakeup", active_wakeup_ring, 20, 1},
> + { "double-wakeup", wakeup_ring, 20, 2},
> + { "store", store_ring, 1, 20},
> + { "switch", switch_ring, 1, 20},
> + { "forked-switch", switch_ring, ncpus, 20},
> + { "many", store_many, 0, 20},
> + { "forked", sync_ring, ncpus, 20},
> + { "forked-store", store_ring, ncpus, 20},
> + { NULL, NULL },
> + };
> +
> +
> igt_fixture {
> fd = drm_open_driver(DRIVER_INTEL);
> igt_require_gem(fd);
> gem_submission_print_method(fd);
> gem_scheduler_print_capability(fd);
> -
> igt_fork_hang_detector(fd);
> }
>
> - for (e = intel_execution_engines; e->name; e++) {
> - igt_subtest_f("%s", e->name)
> - sync_ring(fd, eb_ring(e), 1, 20);
> - igt_subtest_f("idle-%s", e->name)
> - idle_ring(fd, eb_ring(e), 20);
> - igt_subtest_f("active-%s", e->name)
> - active_ring(fd, eb_ring(e), 20);
> - igt_subtest_f("wakeup-%s", e->name)
> - wakeup_ring(fd, eb_ring(e), 20, 1);
> - igt_subtest_f("active-wakeup-%s", e->name)
> - active_wakeup_ring(fd, eb_ring(e), 20, 1);
> - igt_subtest_f("double-wakeup-%s", e->name)
> - wakeup_ring(fd, eb_ring(e), 20, 2);
> - igt_subtest_f("store-%s", e->name)
> - store_ring(fd, eb_ring(e), 1, 20);
> - igt_subtest_f("switch-%s", e->name)
> - switch_ring(fd, eb_ring(e), 1, 20);
> - igt_subtest_f("forked-switch-%s", e->name)
> - switch_ring(fd, eb_ring(e), ncpus, 20);
> - igt_subtest_f("many-%s", e->name)
> - store_many(fd, eb_ring(e), 20);
> - igt_subtest_f("forked-%s", e->name)
> - sync_ring(fd, eb_ring(e), ncpus, 20);
> - igt_subtest_f("forked-store-%s", e->name)
> - store_ring(fd, eb_ring(e), ncpus, 20);
> + /* Legacy testing must be first. */
> + igt_subtest_group {
> + for (test = AllEngine; test->name; test++) {
> + igt_subtest_f("%s", test->name) {
> + do_test(test->func,
> + fd, ALL_ENGINES,
> + test->num_children,
> + test->timeout,
> + test->name);
> + }
On a quick glance, so I may be missing something, problem here is I think that the first subtest will convert the default context to engine map so nothing after the first subtest will be legacy any more.
Therefore I think...
True , will reorder this cases .
> + }
> }
>
> - igt_subtest("basic-each")
> - sync_ring(fd, ALL_ENGINES, 1, 2);
> - igt_subtest("basic-store-each")
> - store_ring(fd, ALL_ENGINES, 1, 2);
> - igt_subtest("basic-many-each")
> - store_many(fd, ALL_ENGINES, 2);
> - igt_subtest("switch-each")
> - switch_ring(fd, ALL_ENGINES, 1, 20);
> - igt_subtest("forked-switch-each")
> - switch_ring(fd, ALL_ENGINES, ncpus, 20);
> - igt_subtest("forked-each")
> - sync_ring(fd, ALL_ENGINES, ncpus, 20);
> - igt_subtest("forked-store-each")
> - store_ring(fd, ALL_ENGINES, ncpus, 20);
> - igt_subtest("active-each")
> - active_ring(fd, ALL_ENGINES, 20);
> - igt_subtest("wakeup-each")
> - wakeup_ring(fd, ALL_ENGINES, 20, 1);
> - igt_subtest("active-wakeup-each")
> - active_wakeup_ring(fd, ALL_ENGINES, 20, 1);
> - igt_subtest("double-wakeup-each")
> - wakeup_ring(fd, ALL_ENGINES, 20, 2);
> -
> igt_subtest("basic-all")
> sync_all(fd, 1, 2);
> igt_subtest("basic-store-all")
> store_all(fd, 1, 2);
> -
> igt_subtest("all")
> sync_all(fd, 1, 20);
> igt_subtest("store-all")
> @@ -1286,7 +1322,49 @@ igt_main
> igt_subtest("forked-store-all")
> store_all(fd, ncpus, 20);
>
> + /* legacy of selecting engines. */
> +
> + igt_subtest_group {
> + for (test = tests; test->name; test++) {
> + igt_subtest_with_dynamic_f("legacy-engines-%s",
> + test->name) {
> + for_each_physical_engine(e, fd) {
> + igt_dynamic_f("%s", e->name) {
> + do_test(test->func,
> + fd, eb_ring(e),
> + test->num_children,
> + test->timeout,
> + e->full_name);
> + }
> + }
> + }
> + }
> + }
... this block should come first. Then the ALL_ENGINES block is not legacy engine selection due __for_eacy_physical_engine usage.
Will change this
> +
> + /* New way of selecting engines. */
> +
> igt_subtest_group {
> + const struct intel_execution_engine2 *e;
> +
> + for (test = tests; test->name; test++) {
> + igt_subtest_with_dynamic_f("engines-%s", test->name) {
I'd be tempted to drop the "engines" from all subtest names since it feels redundant. There is "each" in tests which do all engines so that should be enough.
Will verify test name by removing engines from prefix .
> + __for_each_physical_engine(fd, e) {
> + igt_dynamic_f("%s", e->name) {
> + do_test(test->func,
> + fd, e->flags,
> + test->num_children,
> + test->timeout,
> + e->name);
> + }
> + }
> + }
> + }
> + }
> +
> +
> + igt_subtest_group {
> + const struct intel_execution_engine2 *e;
> +
> igt_fixture {
> gem_require_contexts(fd);
> igt_require(gem_scheduler_has_ctx_priority(fd));
> @@ -1295,10 +1373,12 @@ igt_main
>
> igt_subtest("preempt-all")
> preempt(fd, ALL_ENGINES, 1, 20);
> -
> - for (e = intel_execution_engines; e->name; e++) {
> - igt_subtest_f("preempt-%s", e->name)
> - preempt(fd, eb_ring(e), ncpus, 20);
> + igt_subtest_with_dynamic("preempt") {
> + __for_each_physical_engine(fd, e) {
> + /* Requires master for STORE_DWORD on gen4/5 */
> + igt_dynamic_f("%s", e->name)
> + preempt(fd, e->flags, ncpus, 20);
> + }
> }
> }
>
>
Regards,
Tvrtko
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-04-14 14:01 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-08 15:12 [igt-dev] [PATCH] [PATCH i-g-t][V5]tests/i915/gem_sync.c :Added __for_each_physical_engine to utilize all available Arjun Melkaveri
2020-04-08 16:12 ` [igt-dev] ✗ GitLab.Pipeline: failure for tests/i915/gem_sync.c :Added __for_each_physical_engine to utilize all available (rev6) Patchwork
2020-04-08 16:22 ` [igt-dev] ✓ Fi.CI.BAT: success " Patchwork
2020-04-09 3:20 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
2020-04-14 12:58 ` [igt-dev] [PATCH] [PATCH i-g-t][V5]tests/i915/gem_sync.c :Added __for_each_physical_engine to utilize all available Tvrtko Ursulin
2020-04-14 14:01 ` Melkaveri, Arjun
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.