* [igt-dev] [PATCH i-g-t] tests/i915: Use engine query interface for gem_ctx_isolation/persistence @ 2019-12-03 5:11 Stuart Summers 2019-12-03 6:03 ` [igt-dev] ✗ Fi.CI.BAT: failure for " Patchwork 2019-12-04 9:36 ` [igt-dev] [PATCH i-g-t] " Tvrtko Ursulin 0 siblings, 2 replies; 14+ messages in thread From: Stuart Summers @ 2019-12-03 5:11 UTC (permalink / raw) To: igt-dev Align with gem_exec_basic and other tests using the newer engine query interface into i915 to enumerate active engines. Signed-off-by: Stuart Summers <stuart.summers@intel.com> --- tests/i915/gem_ctx_isolation.c | 4 ++-- tests/i915/gem_ctx_persistence.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/i915/gem_ctx_isolation.c b/tests/i915/gem_ctx_isolation.c index 6aa27133..9435209e 100644 --- a/tests/i915/gem_ctx_isolation.c +++ b/tests/i915/gem_ctx_isolation.c @@ -856,6 +856,7 @@ static unsigned int __has_context_isolation(int fd) igt_main { + struct intel_execution_engine2 *e; unsigned int has_context_isolation = 0; int fd = -1; @@ -876,8 +877,7 @@ igt_main igt_skip_on(gen > LAST_KNOWN_GEN); } - for (const struct intel_execution_engine2 *e = intel_execution_engines2; - e->name; e++) { + __for_each_physical_engine(fd, e) { igt_subtest_group { igt_fixture { igt_require(has_context_isolation & (1 << e->class)); diff --git a/tests/i915/gem_ctx_persistence.c b/tests/i915/gem_ctx_persistence.c index d68431ae..30772159 100644 --- a/tests/i915/gem_ctx_persistence.c +++ b/tests/i915/gem_ctx_persistence.c @@ -727,7 +727,7 @@ igt_main igt_subtest("hangcheck") test_nohangcheck_hostile(i915); - __for_each_static_engine(e) { + __for_each_physical_engine(i915, e) { igt_subtest_group { igt_fixture { gem_require_ring(i915, e->flags); -- 2.21.0.5.gaeb582a983 _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [igt-dev] ✗ Fi.CI.BAT: failure for tests/i915: Use engine query interface for gem_ctx_isolation/persistence 2019-12-03 5:11 [igt-dev] [PATCH i-g-t] tests/i915: Use engine query interface for gem_ctx_isolation/persistence Stuart Summers @ 2019-12-03 6:03 ` Patchwork 2019-12-03 18:31 ` Summers, Stuart 2019-12-04 9:36 ` [igt-dev] [PATCH i-g-t] " Tvrtko Ursulin 1 sibling, 1 reply; 14+ messages in thread From: Patchwork @ 2019-12-03 6:03 UTC (permalink / raw) To: Stuart Summers; +Cc: igt-dev == Series Details == Series: tests/i915: Use engine query interface for gem_ctx_isolation/persistence URL : https://patchwork.freedesktop.org/series/70329/ State : failure == Summary == CI Bug Log - changes from CI_DRM_7467 -> IGTPW_3799 ==================================================== Summary ------- **FAILURE** Serious unknown changes coming with IGTPW_3799 absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in IGTPW_3799, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3799/index.html Possible new issues ------------------- Here are the unknown changes that may have been introduced in IGTPW_3799: ### IGT changes ### #### Possible regressions #### * igt@i915_selftest@live_blt: - fi-hsw-4770r: [PASS][1] -> [DMESG-FAIL][2] [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7467/fi-hsw-4770r/igt@i915_selftest@live_blt.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3799/fi-hsw-4770r/igt@i915_selftest@live_blt.html #### Suppressed #### The following results come from untrusted machines, tests, or statuses. They do not affect the overall result. * igt@i915_module_load@reload-no-display: - {fi-kbl-7560u}: [PASS][3] -> [DMESG-WARN][4] [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7467/fi-kbl-7560u/igt@i915_module_load@reload-no-display.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3799/fi-kbl-7560u/igt@i915_module_load@reload-no-display.html Known issues ------------ Here are the changes found in IGTPW_3799 that come from known issues: ### IGT changes ### #### Issues hit #### * igt@i915_pm_rpm@module-reload: - fi-skl-lmem: [PASS][5] -> [DMESG-WARN][6] ([i915#592]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7467/fi-skl-lmem/igt@i915_pm_rpm@module-reload.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3799/fi-skl-lmem/igt@i915_pm_rpm@module-reload.html * igt@i915_selftest@live_blt: - fi-bwr-2160: [PASS][7] -> [INCOMPLETE][8] ([i915#695]) [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7467/fi-bwr-2160/igt@i915_selftest@live_blt.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3799/fi-bwr-2160/igt@i915_selftest@live_blt.html * igt@kms_chamelium@dp-crc-fast: - fi-icl-u2: [PASS][9] -> [FAIL][10] ([fdo#109635] / [i915#262]) [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7467/fi-icl-u2/igt@kms_chamelium@dp-crc-fast.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3799/fi-icl-u2/igt@kms_chamelium@dp-crc-fast.html #### Possible fixes #### * igt@i915_pm_rpm@module-reload: - fi-skl-6770hq: [DMESG-WARN][11] ([i915#592]) -> [PASS][12] [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7467/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3799/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html * igt@i915_selftest@live_blt: - fi-ivb-3770: [DMESG-FAIL][13] ([i915#563]) -> [PASS][14] [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7467/fi-ivb-3770/igt@i915_selftest@live_blt.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3799/fi-ivb-3770/igt@i915_selftest@live_blt.html - fi-hsw-4770: [DMESG-FAIL][15] ([i915#563]) -> [PASS][16] [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7467/fi-hsw-4770/igt@i915_selftest@live_blt.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3799/fi-hsw-4770/igt@i915_selftest@live_blt.html * igt@i915_selftest@live_gem_contexts: - fi-hsw-peppy: [DMESG-FAIL][17] ([fdo#111692]) -> [PASS][18] [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7467/fi-hsw-peppy/igt@i915_selftest@live_gem_contexts.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3799/fi-hsw-peppy/igt@i915_selftest@live_gem_contexts.html * igt@kms_frontbuffer_tracking@basic: - fi-icl-guc: [FAIL][19] ([i915#49]) -> [PASS][20] [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7467/fi-icl-guc/igt@kms_frontbuffer_tracking@basic.html [20]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3799/fi-icl-guc/igt@kms_frontbuffer_tracking@basic.html #### Warnings #### * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy: - fi-kbl-x1275: [DMESG-WARN][21] ([i915#62] / [i915#92]) -> [DMESG-WARN][22] ([i915#62] / [i915#92] / [i915#95]) +7 similar issues [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7467/fi-kbl-x1275/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html [22]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3799/fi-kbl-x1275/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html * igt@kms_flip@basic-flip-vs-modeset: - fi-kbl-x1275: [DMESG-WARN][23] ([i915#62] / [i915#92] / [i915#95]) -> [DMESG-WARN][24] ([i915#62] / [i915#92]) +8 similar issues [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7467/fi-kbl-x1275/igt@kms_flip@basic-flip-vs-modeset.html [24]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3799/fi-kbl-x1275/igt@kms_flip@basic-flip-vs-modeset.html {name}: This element is suppressed. This means it is ignored when computing the status of the difference (SUCCESS, WARNING, or FAILURE). [fdo#109635]: https://bugs.freedesktop.org/show_bug.cgi?id=109635 [fdo#111692]: https://bugs.freedesktop.org/show_bug.cgi?id=111692 [i915#262]: https://gitlab.freedesktop.org/drm/intel/issues/262 [i915#49]: https://gitlab.freedesktop.org/drm/intel/issues/49 [i915#563]: https://gitlab.freedesktop.org/drm/intel/issues/563 [i915#592]: https://gitlab.freedesktop.org/drm/intel/issues/592 [i915#62]: https://gitlab.freedesktop.org/drm/intel/issues/62 [i915#695]: https://gitlab.freedesktop.org/drm/intel/issues/695 [i915#92]: https://gitlab.freedesktop.org/drm/intel/issues/92 [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95 Participating hosts (50 -> 45) ------------------------------ Missing (5): fi-ilk-m540 fi-hsw-4200u fi-bsw-cyan fi-ctg-p8600 fi-byt-clapper Build changes ------------- * CI: CI-20190529 -> None * IGT: IGT_5321 -> IGTPW_3799 CI-20190529: 20190529 CI_DRM_7467: 14954f24e7251b067b2081aaa09a7da6840da0d5 @ git://anongit.freedesktop.org/gfx-ci/linux IGTPW_3799: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3799/index.html IGT_5321: 9df50aef49e0da4413609d9866b41b82b725f2a0 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3799/index.html _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [igt-dev] ✗ Fi.CI.BAT: failure for tests/i915: Use engine query interface for gem_ctx_isolation/persistence 2019-12-03 6:03 ` [igt-dev] ✗ Fi.CI.BAT: failure for " Patchwork @ 2019-12-03 18:31 ` Summers, Stuart 0 siblings, 0 replies; 14+ messages in thread From: Summers, Stuart @ 2019-12-03 18:31 UTC (permalink / raw) To: igt-dev [-- Attachment #1.1: Type: text/plain, Size: 7507 bytes --] On Tue, 2019-12-03 at 06:03 +0000, Patchwork wrote: > == Series Details == > > Series: tests/i915: Use engine query interface for > gem_ctx_isolation/persistence > URL : https://patchwork.freedesktop.org/series/70329/ > State : failure > > == Summary == > > CI Bug Log - changes from CI_DRM_7467 -> IGTPW_3799 > ==================================================== > > Summary > ------- > > **FAILURE** > > Serious unknown changes coming with IGTPW_3799 absolutely need to > be > verified manually. > > If you think the reported changes have nothing to do with the > changes > introduced in IGTPW_3799, please notify your bug team to allow them > to document this new failure mode, which will reduce false > positives in CI. > > External URL: > https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3799/index.html > > Possible new issues > ------------------- > > Here are the unknown changes that may have been introduced in > IGTPW_3799: > > ### IGT changes ### > > #### Possible regressions #### > > * igt@i915_selftest@live_blt: > - fi-hsw-4770r: [PASS][1] -> [DMESG-FAIL][2] > [1]: > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7467/fi-hsw-4770r/igt@i915_selftest@live_blt.html > [2]: > https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3799/fi-hsw-4770r/igt@i915_selftest@live_blt.html Not sure how this is related to the changes I made. -Stuart > > > #### Suppressed #### > > The following results come from untrusted machines, tests, or > statuses. > They do not affect the overall result. > > * igt@i915_module_load@reload-no-display: > - {fi-kbl-7560u}: [PASS][3] -> [DMESG-WARN][4] > [3]: > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7467/fi-kbl-7560u/igt@i915_module_load@reload-no-display.html > [4]: > https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3799/fi-kbl-7560u/igt@i915_module_load@reload-no-display.html > > > Known issues > ------------ > > Here are the changes found in IGTPW_3799 that come from known > issues: > > ### IGT changes ### > > #### Issues hit #### > > * igt@i915_pm_rpm@module-reload: > - fi-skl-lmem: [PASS][5] -> [DMESG-WARN][6] ([i915#592]) > [5]: > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7467/fi-skl-lmem/igt@i915_pm_rpm@module-reload.html > [6]: > https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3799/fi-skl-lmem/igt@i915_pm_rpm@module-reload.html > > * igt@i915_selftest@live_blt: > - fi-bwr-2160: [PASS][7] -> [INCOMPLETE][8] ([i915#695]) > [7]: > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7467/fi-bwr-2160/igt@i915_selftest@live_blt.html > [8]: > https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3799/fi-bwr-2160/igt@i915_selftest@live_blt.html > > * igt@kms_chamelium@dp-crc-fast: > - fi-icl-u2: [PASS][9] -> [FAIL][10] ([fdo#109635] / > [i915#262]) > [9]: > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7467/fi-icl-u2/igt@kms_chamelium@dp-crc-fast.html > [10]: > https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3799/fi-icl-u2/igt@kms_chamelium@dp-crc-fast.html > > > #### Possible fixes #### > > * igt@i915_pm_rpm@module-reload: > - fi-skl-6770hq: [DMESG-WARN][11] ([i915#592]) -> [PASS][12] > [11]: > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7467/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html > [12]: > https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3799/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html > > * igt@i915_selftest@live_blt: > - fi-ivb-3770: [DMESG-FAIL][13] ([i915#563]) -> [PASS][14] > [13]: > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7467/fi-ivb-3770/igt@i915_selftest@live_blt.html > [14]: > https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3799/fi-ivb-3770/igt@i915_selftest@live_blt.html > - fi-hsw-4770: [DMESG-FAIL][15] ([i915#563]) -> [PASS][16] > [15]: > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7467/fi-hsw-4770/igt@i915_selftest@live_blt.html > [16]: > https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3799/fi-hsw-4770/igt@i915_selftest@live_blt.html > > * igt@i915_selftest@live_gem_contexts: > - fi-hsw-peppy: [DMESG-FAIL][17] ([fdo#111692]) -> > [PASS][18] > [17]: > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7467/fi-hsw-peppy/igt@i915_selftest@live_gem_contexts.html > [18]: > https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3799/fi-hsw-peppy/igt@i915_selftest@live_gem_contexts.html > > * igt@kms_frontbuffer_tracking@basic: > - fi-icl-guc: [FAIL][19] ([i915#49]) -> [PASS][20] > [19]: > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7467/fi-icl-guc/igt@kms_frontbuffer_tracking@basic.html > [20]: > https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3799/fi-icl-guc/igt@kms_frontbuffer_tracking@basic.html > > > #### Warnings #### > > * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy: > - fi-kbl-x1275: [DMESG-WARN][21] ([i915#62] / [i915#92]) -> > [DMESG-WARN][22] ([i915#62] / [i915#92] / [i915#95]) +7 similar > issues > [21]: > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7467/fi-kbl-x1275/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html > [22]: > https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3799/fi-kbl-x1275/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html > > * igt@kms_flip@basic-flip-vs-modeset: > - fi-kbl-x1275: [DMESG-WARN][23] ([i915#62] / [i915#92] / > [i915#95]) -> [DMESG-WARN][24] ([i915#62] / [i915#92]) +8 similar > issues > [23]: > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7467/fi-kbl-x1275/igt@kms_flip@basic-flip-vs-modeset.html > [24]: > https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3799/fi-kbl-x1275/igt@kms_flip@basic-flip-vs-modeset.html > > > {name}: This element is suppressed. This means it is ignored when > computing > the status of the difference (SUCCESS, WARNING, or > FAILURE). > > [fdo#109635]: https://bugs.freedesktop.org/show_bug.cgi?id=109635 > [fdo#111692]: https://bugs.freedesktop.org/show_bug.cgi?id=111692 > [i915#262]: https://gitlab.freedesktop.org/drm/intel/issues/262 > [i915#49]: https://gitlab.freedesktop.org/drm/intel/issues/49 > [i915#563]: https://gitlab.freedesktop.org/drm/intel/issues/563 > [i915#592]: https://gitlab.freedesktop.org/drm/intel/issues/592 > [i915#62]: https://gitlab.freedesktop.org/drm/intel/issues/62 > [i915#695]: https://gitlab.freedesktop.org/drm/intel/issues/695 > [i915#92]: https://gitlab.freedesktop.org/drm/intel/issues/92 > [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95 > > > Participating hosts (50 -> 45) > ------------------------------ > > Missing (5): fi-ilk-m540 fi-hsw-4200u fi-bsw-cyan fi-ctg-p8600 > fi-byt-clapper > > > Build changes > ------------- > > * CI: CI-20190529 -> None > * IGT: IGT_5321 -> IGTPW_3799 > > CI-20190529: 20190529 > CI_DRM_7467: 14954f24e7251b067b2081aaa09a7da6840da0d5 @ > git://anongit.freedesktop.org/gfx-ci/linux > IGTPW_3799: > https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3799/index.html > IGT_5321: 9df50aef49e0da4413609d9866b41b82b725f2a0 @ > git://anongit.freedesktop.org/xorg/app/intel-gpu-tools > > == Logs == > > For more details see: > https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3799/index.html [-- Attachment #1.2: smime.p7s --] [-- Type: application/x-pkcs7-signature, Size: 3270 bytes --] [-- Attachment #2: Type: text/plain, Size: 153 bytes --] _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [igt-dev] [PATCH i-g-t] tests/i915: Use engine query interface for gem_ctx_isolation/persistence 2019-12-03 5:11 [igt-dev] [PATCH i-g-t] tests/i915: Use engine query interface for gem_ctx_isolation/persistence Stuart Summers 2019-12-03 6:03 ` [igt-dev] ✗ Fi.CI.BAT: failure for " Patchwork @ 2019-12-04 9:36 ` Tvrtko Ursulin 2019-12-04 14:53 ` Petri Latvala 1 sibling, 1 reply; 14+ messages in thread From: Tvrtko Ursulin @ 2019-12-04 9:36 UTC (permalink / raw) To: Stuart Summers, igt-dev On 03/12/2019 05:11, Stuart Summers wrote: > Align with gem_exec_basic and other tests using the newer > engine query interface into i915 to enumerate active engines. > > Signed-off-by: Stuart Summers <stuart.summers@intel.com> > --- > tests/i915/gem_ctx_isolation.c | 4 ++-- > tests/i915/gem_ctx_persistence.c | 2 +- > 2 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/tests/i915/gem_ctx_isolation.c b/tests/i915/gem_ctx_isolation.c > index 6aa27133..9435209e 100644 > --- a/tests/i915/gem_ctx_isolation.c > +++ b/tests/i915/gem_ctx_isolation.c > @@ -856,6 +856,7 @@ static unsigned int __has_context_isolation(int fd) > > igt_main > { > + struct intel_execution_engine2 *e; > unsigned int has_context_isolation = 0; > int fd = -1; > > @@ -876,8 +877,7 @@ igt_main > igt_skip_on(gen > LAST_KNOWN_GEN); > } > > - for (const struct intel_execution_engine2 *e = intel_execution_engines2; > - e->name; e++) { > + __for_each_physical_engine(fd, e) { > igt_subtest_group { > igt_fixture { > igt_require(has_context_isolation & (1 << e->class)); > diff --git a/tests/i915/gem_ctx_persistence.c b/tests/i915/gem_ctx_persistence.c > index d68431ae..30772159 100644 > --- a/tests/i915/gem_ctx_persistence.c > +++ b/tests/i915/gem_ctx_persistence.c > @@ -727,7 +727,7 @@ igt_main > igt_subtest("hangcheck") > test_nohangcheck_hostile(i915); > > - __for_each_static_engine(e) { > + __for_each_physical_engine(i915, e) { > igt_subtest_group { > igt_fixture { > gem_require_ring(i915, e->flags); > __for_each_static_engine is correct, at least if you don't want CI folks go look for their pitchforks. :) Same for the first hunk, everything that enumerates subtests needs to be static. Option 2, the preferred one - convert the tests to igt_subtest_with_dynamic and then you can use __for_each_physical_engine. Regards, Tvrtko _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [igt-dev] [PATCH i-g-t] tests/i915: Use engine query interface for gem_ctx_isolation/persistence 2019-12-04 9:36 ` [igt-dev] [PATCH i-g-t] " Tvrtko Ursulin @ 2019-12-04 14:53 ` Petri Latvala 2019-12-04 19:05 ` Summers, Stuart 0 siblings, 1 reply; 14+ messages in thread From: Petri Latvala @ 2019-12-04 14:53 UTC (permalink / raw) To: Tvrtko Ursulin; +Cc: igt-dev On Wed, Dec 04, 2019 at 09:36:11AM +0000, Tvrtko Ursulin wrote: > > On 03/12/2019 05:11, Stuart Summers wrote: > > Align with gem_exec_basic and other tests using the newer > > engine query interface into i915 to enumerate active engines. > > > > Signed-off-by: Stuart Summers <stuart.summers@intel.com> > > --- > > tests/i915/gem_ctx_isolation.c | 4 ++-- > > tests/i915/gem_ctx_persistence.c | 2 +- > > 2 files changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/tests/i915/gem_ctx_isolation.c b/tests/i915/gem_ctx_isolation.c > > index 6aa27133..9435209e 100644 > > --- a/tests/i915/gem_ctx_isolation.c > > +++ b/tests/i915/gem_ctx_isolation.c > > @@ -856,6 +856,7 @@ static unsigned int __has_context_isolation(int fd) > > igt_main > > { > > + struct intel_execution_engine2 *e; > > unsigned int has_context_isolation = 0; > > int fd = -1; > > @@ -876,8 +877,7 @@ igt_main > > igt_skip_on(gen > LAST_KNOWN_GEN); > > } > > - for (const struct intel_execution_engine2 *e = intel_execution_engines2; > > - e->name; e++) { > > + __for_each_physical_engine(fd, e) { > > igt_subtest_group { > > igt_fixture { > > igt_require(has_context_isolation & (1 << e->class)); > > diff --git a/tests/i915/gem_ctx_persistence.c b/tests/i915/gem_ctx_persistence.c > > index d68431ae..30772159 100644 > > --- a/tests/i915/gem_ctx_persistence.c > > +++ b/tests/i915/gem_ctx_persistence.c > > @@ -727,7 +727,7 @@ igt_main > > igt_subtest("hangcheck") > > test_nohangcheck_hostile(i915); > > - __for_each_static_engine(e) { > > + __for_each_physical_engine(i915, e) { > > igt_subtest_group { > > igt_fixture { > > gem_require_ring(i915, e->flags); > > > > __for_each_static_engine is correct, at least if you don't want CI folks go > look for their pitchforks. :) Same for the first hunk, everything that > enumerates subtests needs to be static. > > Option 2, the preferred one - convert the tests to igt_subtest_with_dynamic > and then you can use __for_each_physical_engine. Doesn't __for_each_physical_engine anyway have a hack for being called in that context? Btw, option 2 is https://gitlab.freedesktop.org/drm/igt-gpu-tools/issues/44 Currently blocking that effort is getting https://patchwork.freedesktop.org/series/70286/ into shape. -- Petri Latvala _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [igt-dev] [PATCH i-g-t] tests/i915: Use engine query interface for gem_ctx_isolation/persistence 2019-12-04 14:53 ` Petri Latvala @ 2019-12-04 19:05 ` Summers, Stuart 2019-12-05 9:01 ` Petri Latvala 0 siblings, 1 reply; 14+ messages in thread From: Summers, Stuart @ 2019-12-04 19:05 UTC (permalink / raw) To: Latvala, Petri, tvrtko.ursulin; +Cc: igt-dev [-- Attachment #1.1: Type: text/plain, Size: 2932 bytes --] On Wed, 2019-12-04 at 16:53 +0200, Petri Latvala wrote: > On Wed, Dec 04, 2019 at 09:36:11AM +0000, Tvrtko Ursulin wrote: > > > > On 03/12/2019 05:11, Stuart Summers wrote: > > > Align with gem_exec_basic and other tests using the newer > > > engine query interface into i915 to enumerate active engines. > > > > > > Signed-off-by: Stuart Summers <stuart.summers@intel.com> > > > --- > > > tests/i915/gem_ctx_isolation.c | 4 ++-- > > > tests/i915/gem_ctx_persistence.c | 2 +- > > > 2 files changed, 3 insertions(+), 3 deletions(-) > > > > > > diff --git a/tests/i915/gem_ctx_isolation.c > > > b/tests/i915/gem_ctx_isolation.c > > > index 6aa27133..9435209e 100644 > > > --- a/tests/i915/gem_ctx_isolation.c > > > +++ b/tests/i915/gem_ctx_isolation.c > > > @@ -856,6 +856,7 @@ static unsigned int > > > __has_context_isolation(int fd) > > > igt_main > > > { > > > + struct intel_execution_engine2 *e; > > > unsigned int has_context_isolation = 0; > > > int fd = -1; > > > @@ -876,8 +877,7 @@ igt_main > > > igt_skip_on(gen > LAST_KNOWN_GEN); > > > } > > > - for (const struct intel_execution_engine2 *e = > > > intel_execution_engines2; > > > - e->name; e++) { > > > + __for_each_physical_engine(fd, e) { > > > igt_subtest_group { > > > igt_fixture { > > > igt_require(has_context_isolati > > > on & (1 << e->class)); > > > diff --git a/tests/i915/gem_ctx_persistence.c > > > b/tests/i915/gem_ctx_persistence.c > > > index d68431ae..30772159 100644 > > > --- a/tests/i915/gem_ctx_persistence.c > > > +++ b/tests/i915/gem_ctx_persistence.c > > > @@ -727,7 +727,7 @@ igt_main > > > igt_subtest("hangcheck") > > > test_nohangcheck_hostile(i915); > > > - __for_each_static_engine(e) { > > > + __for_each_physical_engine(i915, e) { > > > igt_subtest_group { > > > igt_fixture { > > > gem_require_ring(i915, e- > > > >flags); > > > > > > > __for_each_static_engine is correct, at least if you don't want CI > > folks go > > look for their pitchforks. :) Same for the first hunk, everything > > that > > enumerates subtests needs to be static. > > > > Option 2, the preferred one - convert the tests to > > igt_subtest_with_dynamic > > and then you can use __for_each_physical_engine. > > Doesn't __for_each_physical_engine anyway have a hack for being > called in that context? > > Btw, option 2 is > https://gitlab.freedesktop.org/drm/igt-gpu-tools/issues/44 > > Currently blocking that effort is getting > https://patchwork.freedesktop.org/series/70286/ into shape. Wait I'm a little confused, sorry for the naivete here. Does this mean these kinds of changes are blocked on the above series? Or are you saying I should go ahead and convert this to the dynamic subtests? Or that we can move forward with the current approach and convert at a later time? Thanks, Stuart > > [-- Attachment #1.2: smime.p7s --] [-- Type: application/x-pkcs7-signature, Size: 3270 bytes --] [-- Attachment #2: Type: text/plain, Size: 153 bytes --] _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [igt-dev] [PATCH i-g-t] tests/i915: Use engine query interface for gem_ctx_isolation/persistence 2019-12-04 19:05 ` Summers, Stuart @ 2019-12-05 9:01 ` Petri Latvala 2019-12-05 9:22 ` Tvrtko Ursulin 0 siblings, 1 reply; 14+ messages in thread From: Petri Latvala @ 2019-12-05 9:01 UTC (permalink / raw) To: Summers, Stuart; +Cc: igt-dev On Wed, Dec 04, 2019 at 07:05:12PM +0000, Summers, Stuart wrote: > On Wed, 2019-12-04 at 16:53 +0200, Petri Latvala wrote: > > On Wed, Dec 04, 2019 at 09:36:11AM +0000, Tvrtko Ursulin wrote: > > > > > > On 03/12/2019 05:11, Stuart Summers wrote: > > > > Align with gem_exec_basic and other tests using the newer > > > > engine query interface into i915 to enumerate active engines. > > > > > > > > Signed-off-by: Stuart Summers <stuart.summers@intel.com> > > > > --- > > > > tests/i915/gem_ctx_isolation.c | 4 ++-- > > > > tests/i915/gem_ctx_persistence.c | 2 +- > > > > 2 files changed, 3 insertions(+), 3 deletions(-) > > > > > > > > diff --git a/tests/i915/gem_ctx_isolation.c > > > > b/tests/i915/gem_ctx_isolation.c > > > > index 6aa27133..9435209e 100644 > > > > --- a/tests/i915/gem_ctx_isolation.c > > > > +++ b/tests/i915/gem_ctx_isolation.c > > > > @@ -856,6 +856,7 @@ static unsigned int > > > > __has_context_isolation(int fd) > > > > igt_main > > > > { > > > > + struct intel_execution_engine2 *e; > > > > unsigned int has_context_isolation = 0; > > > > int fd = -1; > > > > @@ -876,8 +877,7 @@ igt_main > > > > igt_skip_on(gen > LAST_KNOWN_GEN); > > > > } > > > > - for (const struct intel_execution_engine2 *e = > > > > intel_execution_engines2; > > > > - e->name; e++) { > > > > + __for_each_physical_engine(fd, e) { > > > > igt_subtest_group { > > > > igt_fixture { > > > > igt_require(has_context_isolati > > > > on & (1 << e->class)); > > > > diff --git a/tests/i915/gem_ctx_persistence.c > > > > b/tests/i915/gem_ctx_persistence.c > > > > index d68431ae..30772159 100644 > > > > --- a/tests/i915/gem_ctx_persistence.c > > > > +++ b/tests/i915/gem_ctx_persistence.c > > > > @@ -727,7 +727,7 @@ igt_main > > > > igt_subtest("hangcheck") > > > > test_nohangcheck_hostile(i915); > > > > - __for_each_static_engine(e) { > > > > + __for_each_physical_engine(i915, e) { > > > > igt_subtest_group { > > > > igt_fixture { > > > > gem_require_ring(i915, e- > > > > >flags); > > > > > > > > > > __for_each_static_engine is correct, at least if you don't want CI > > > folks go > > > look for their pitchforks. :) Same for the first hunk, everything > > > that > > > enumerates subtests needs to be static. > > > > > > Option 2, the preferred one - convert the tests to > > > igt_subtest_with_dynamic > > > and then you can use __for_each_physical_engine. > > > > Doesn't __for_each_physical_engine anyway have a hack for being > > called in that context? > > > > Btw, option 2 is > > https://gitlab.freedesktop.org/drm/igt-gpu-tools/issues/44 > > > > Currently blocking that effort is getting > > https://patchwork.freedesktop.org/series/70286/ into shape. > > Wait I'm a little confused, sorry for the naivete here. Does this mean > these kinds of changes are blocked on the above series? Or are you > saying I should go ahead and convert this to the dynamic subtests? Or > that we can move forward with the current approach and convert at a > later time? Option 2 is blocked by series 70286, sorry for the confusion. I don't mind slapping your patch in while waiting for that, since it's $RAND days away still to get there, if it fixes a problem you're having now. -- Petri Latvala _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [igt-dev] [PATCH i-g-t] tests/i915: Use engine query interface for gem_ctx_isolation/persistence 2019-12-05 9:01 ` Petri Latvala @ 2019-12-05 9:22 ` Tvrtko Ursulin 2019-12-05 9:47 ` Andi Shyti 0 siblings, 1 reply; 14+ messages in thread From: Tvrtko Ursulin @ 2019-12-05 9:22 UTC (permalink / raw) To: Petri Latvala, Summers, Stuart; +Cc: igt-dev On 05/12/2019 09:01, Petri Latvala wrote: > On Wed, Dec 04, 2019 at 07:05:12PM +0000, Summers, Stuart wrote: >> On Wed, 2019-12-04 at 16:53 +0200, Petri Latvala wrote: >>> On Wed, Dec 04, 2019 at 09:36:11AM +0000, Tvrtko Ursulin wrote: >>>> >>>> On 03/12/2019 05:11, Stuart Summers wrote: >>>>> Align with gem_exec_basic and other tests using the newer >>>>> engine query interface into i915 to enumerate active engines. >>>>> >>>>> Signed-off-by: Stuart Summers <stuart.summers@intel.com> >>>>> --- >>>>> tests/i915/gem_ctx_isolation.c | 4 ++-- >>>>> tests/i915/gem_ctx_persistence.c | 2 +- >>>>> 2 files changed, 3 insertions(+), 3 deletions(-) >>>>> >>>>> diff --git a/tests/i915/gem_ctx_isolation.c >>>>> b/tests/i915/gem_ctx_isolation.c >>>>> index 6aa27133..9435209e 100644 >>>>> --- a/tests/i915/gem_ctx_isolation.c >>>>> +++ b/tests/i915/gem_ctx_isolation.c >>>>> @@ -856,6 +856,7 @@ static unsigned int >>>>> __has_context_isolation(int fd) >>>>> igt_main >>>>> { >>>>> + struct intel_execution_engine2 *e; >>>>> unsigned int has_context_isolation = 0; >>>>> int fd = -1; >>>>> @@ -876,8 +877,7 @@ igt_main >>>>> igt_skip_on(gen > LAST_KNOWN_GEN); >>>>> } >>>>> - for (const struct intel_execution_engine2 *e = >>>>> intel_execution_engines2; >>>>> - e->name; e++) { >>>>> + __for_each_physical_engine(fd, e) { >>>>> igt_subtest_group { >>>>> igt_fixture { >>>>> igt_require(has_context_isolati >>>>> on & (1 << e->class)); >>>>> diff --git a/tests/i915/gem_ctx_persistence.c >>>>> b/tests/i915/gem_ctx_persistence.c >>>>> index d68431ae..30772159 100644 >>>>> --- a/tests/i915/gem_ctx_persistence.c >>>>> +++ b/tests/i915/gem_ctx_persistence.c >>>>> @@ -727,7 +727,7 @@ igt_main >>>>> igt_subtest("hangcheck") >>>>> test_nohangcheck_hostile(i915); >>>>> - __for_each_static_engine(e) { >>>>> + __for_each_physical_engine(i915, e) { >>>>> igt_subtest_group { >>>>> igt_fixture { >>>>> gem_require_ring(i915, e- >>>>>> flags); >>>>> >>>> >>>> __for_each_static_engine is correct, at least if you don't want CI >>>> folks go >>>> look for their pitchforks. :) Same for the first hunk, everything >>>> that >>>> enumerates subtests needs to be static. >>>> >>>> Option 2, the preferred one - convert the tests to >>>> igt_subtest_with_dynamic >>>> and then you can use __for_each_physical_engine. >>> >>> Doesn't __for_each_physical_engine anyway have a hack for being >>> called in that context? >>> >>> Btw, option 2 is >>> https://gitlab.freedesktop.org/drm/igt-gpu-tools/issues/44 >>> >>> Currently blocking that effort is getting >>> https://patchwork.freedesktop.org/series/70286/ into shape. >> >> Wait I'm a little confused, sorry for the naivete here. Does this mean >> these kinds of changes are blocked on the above series? Or are you >> saying I should go ahead and convert this to the dynamic subtests? Or >> that we can move forward with the current approach and convert at a >> later time? > > Option 2 is blocked by series 70286, sorry for the confusion. I don't > mind slapping your patch in while waiting for that, since it's $RAND > days away still to get there, if it fixes a problem you're having now. I think hack for static enumeration if in --list-subtest mode did not work in practice. Andi, do you remember the details? Regards, Tvrtko _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [igt-dev] [PATCH i-g-t] tests/i915: Use engine query interface for gem_ctx_isolation/persistence 2019-12-05 9:22 ` Tvrtko Ursulin @ 2019-12-05 9:47 ` Andi Shyti 2019-12-05 9:54 ` Tvrtko Ursulin 0 siblings, 1 reply; 14+ messages in thread From: Andi Shyti @ 2019-12-05 9:47 UTC (permalink / raw) To: Tvrtko Ursulin; +Cc: igt-dev, Petri Latvala Hi, On Thu, Dec 05, 2019 at 09:22:56AM +0000, Tvrtko Ursulin wrote: > On 05/12/2019 09:01, Petri Latvala wrote: > > On Wed, Dec 04, 2019 at 07:05:12PM +0000, Summers, Stuart wrote: > > > On Wed, 2019-12-04 at 16:53 +0200, Petri Latvala wrote: > > > > On Wed, Dec 04, 2019 at 09:36:11AM +0000, Tvrtko Ursulin wrote: > > > > > > > > > > On 03/12/2019 05:11, Stuart Summers wrote: > > > > > > Align with gem_exec_basic and other tests using the newer > > > > > > engine query interface into i915 to enumerate active engines. > > > > > > > > > > > > Signed-off-by: Stuart Summers <stuart.summers@intel.com> > > > > > > --- > > > > > > tests/i915/gem_ctx_isolation.c | 4 ++-- > > > > > > tests/i915/gem_ctx_persistence.c | 2 +- > > > > > > 2 files changed, 3 insertions(+), 3 deletions(-) > > > > > > > > > > > > diff --git a/tests/i915/gem_ctx_isolation.c > > > > > > b/tests/i915/gem_ctx_isolation.c > > > > > > index 6aa27133..9435209e 100644 > > > > > > --- a/tests/i915/gem_ctx_isolation.c > > > > > > +++ b/tests/i915/gem_ctx_isolation.c > > > > > > @@ -856,6 +856,7 @@ static unsigned int > > > > > > __has_context_isolation(int fd) > > > > > > igt_main > > > > > > { > > > > > > + struct intel_execution_engine2 *e; > > > > > > unsigned int has_context_isolation = 0; > > > > > > int fd = -1; > > > > > > @@ -876,8 +877,7 @@ igt_main > > > > > > igt_skip_on(gen > LAST_KNOWN_GEN); > > > > > > } > > > > > > - for (const struct intel_execution_engine2 *e = > > > > > > intel_execution_engines2; > > > > > > - e->name; e++) { > > > > > > + __for_each_physical_engine(fd, e) { > > > > > > igt_subtest_group { > > > > > > igt_fixture { > > > > > > igt_require(has_context_isolati > > > > > > on & (1 << e->class)); > > > > > > diff --git a/tests/i915/gem_ctx_persistence.c > > > > > > b/tests/i915/gem_ctx_persistence.c > > > > > > index d68431ae..30772159 100644 > > > > > > --- a/tests/i915/gem_ctx_persistence.c > > > > > > +++ b/tests/i915/gem_ctx_persistence.c > > > > > > @@ -727,7 +727,7 @@ igt_main > > > > > > igt_subtest("hangcheck") > > > > > > test_nohangcheck_hostile(i915); > > > > > > - __for_each_static_engine(e) { > > > > > > + __for_each_physical_engine(i915, e) { > > > > > > igt_subtest_group { > > > > > > igt_fixture { > > > > > > gem_require_ring(i915, e- > > > > > > > flags); > > > > > > > > > > > > > > > > __for_each_static_engine is correct, at least if you don't want CI > > > > > folks go > > > > > look for their pitchforks. :) Same for the first hunk, everything > > > > > that > > > > > enumerates subtests needs to be static. > > > > > > > > > > Option 2, the preferred one - convert the tests to > > > > > igt_subtest_with_dynamic > > > > > and then you can use __for_each_physical_engine. > > > > > > > > Doesn't __for_each_physical_engine anyway have a hack for being > > > > called in that context? > > > > > > > > Btw, option 2 is > > > > https://gitlab.freedesktop.org/drm/igt-gpu-tools/issues/44 > > > > > > > > Currently blocking that effort is getting > > > > https://patchwork.freedesktop.org/series/70286/ into shape. > > > > > > Wait I'm a little confused, sorry for the naivete here. Does this mean > > > these kinds of changes are blocked on the above series? Or are you > > > saying I should go ahead and convert this to the dynamic subtests? Or > > > that we can move forward with the current approach and convert at a > > > later time? > > > > Option 2 is blocked by series 70286, sorry for the confusion. I don't > > mind slapping your patch in while waiting for that, since it's $RAND > > days away still to get there, if it fixes a problem you're having now. > > I think hack for static enumeration if in --list-subtest mode did not work > in practice. Andi, do you remember the details? __for_each_physical_engine checks for igt_only_list_subtests(), if so it behaves like __for_each_static_engine(). Was this what you asked? Andi _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [igt-dev] [PATCH i-g-t] tests/i915: Use engine query interface for gem_ctx_isolation/persistence 2019-12-05 9:47 ` Andi Shyti @ 2019-12-05 9:54 ` Tvrtko Ursulin 2019-12-05 10:09 ` Petri Latvala 2019-12-05 10:12 ` Andi Shyti 0 siblings, 2 replies; 14+ messages in thread From: Tvrtko Ursulin @ 2019-12-05 9:54 UTC (permalink / raw) To: Andi Shyti; +Cc: igt-dev, Petri Latvala On 05/12/2019 09:47, Andi Shyti wrote: > Hi, > > On Thu, Dec 05, 2019 at 09:22:56AM +0000, Tvrtko Ursulin wrote: >> On 05/12/2019 09:01, Petri Latvala wrote: >>> On Wed, Dec 04, 2019 at 07:05:12PM +0000, Summers, Stuart wrote: >>>> On Wed, 2019-12-04 at 16:53 +0200, Petri Latvala wrote: >>>>> On Wed, Dec 04, 2019 at 09:36:11AM +0000, Tvrtko Ursulin wrote: >>>>>> >>>>>> On 03/12/2019 05:11, Stuart Summers wrote: >>>>>>> Align with gem_exec_basic and other tests using the newer >>>>>>> engine query interface into i915 to enumerate active engines. >>>>>>> >>>>>>> Signed-off-by: Stuart Summers <stuart.summers@intel.com> >>>>>>> --- >>>>>>> tests/i915/gem_ctx_isolation.c | 4 ++-- >>>>>>> tests/i915/gem_ctx_persistence.c | 2 +- >>>>>>> 2 files changed, 3 insertions(+), 3 deletions(-) >>>>>>> >>>>>>> diff --git a/tests/i915/gem_ctx_isolation.c >>>>>>> b/tests/i915/gem_ctx_isolation.c >>>>>>> index 6aa27133..9435209e 100644 >>>>>>> --- a/tests/i915/gem_ctx_isolation.c >>>>>>> +++ b/tests/i915/gem_ctx_isolation.c >>>>>>> @@ -856,6 +856,7 @@ static unsigned int >>>>>>> __has_context_isolation(int fd) >>>>>>> igt_main >>>>>>> { >>>>>>> + struct intel_execution_engine2 *e; >>>>>>> unsigned int has_context_isolation = 0; >>>>>>> int fd = -1; >>>>>>> @@ -876,8 +877,7 @@ igt_main >>>>>>> igt_skip_on(gen > LAST_KNOWN_GEN); >>>>>>> } >>>>>>> - for (const struct intel_execution_engine2 *e = >>>>>>> intel_execution_engines2; >>>>>>> - e->name; e++) { >>>>>>> + __for_each_physical_engine(fd, e) { >>>>>>> igt_subtest_group { >>>>>>> igt_fixture { >>>>>>> igt_require(has_context_isolati >>>>>>> on & (1 << e->class)); >>>>>>> diff --git a/tests/i915/gem_ctx_persistence.c >>>>>>> b/tests/i915/gem_ctx_persistence.c >>>>>>> index d68431ae..30772159 100644 >>>>>>> --- a/tests/i915/gem_ctx_persistence.c >>>>>>> +++ b/tests/i915/gem_ctx_persistence.c >>>>>>> @@ -727,7 +727,7 @@ igt_main >>>>>>> igt_subtest("hangcheck") >>>>>>> test_nohangcheck_hostile(i915); >>>>>>> - __for_each_static_engine(e) { >>>>>>> + __for_each_physical_engine(i915, e) { >>>>>>> igt_subtest_group { >>>>>>> igt_fixture { >>>>>>> gem_require_ring(i915, e- >>>>>>>> flags); >>>>>>> >>>>>> >>>>>> __for_each_static_engine is correct, at least if you don't want CI >>>>>> folks go >>>>>> look for their pitchforks. :) Same for the first hunk, everything >>>>>> that >>>>>> enumerates subtests needs to be static. >>>>>> >>>>>> Option 2, the preferred one - convert the tests to >>>>>> igt_subtest_with_dynamic >>>>>> and then you can use __for_each_physical_engine. >>>>> >>>>> Doesn't __for_each_physical_engine anyway have a hack for being >>>>> called in that context? >>>>> >>>>> Btw, option 2 is >>>>> https://gitlab.freedesktop.org/drm/igt-gpu-tools/issues/44 >>>>> >>>>> Currently blocking that effort is getting >>>>> https://patchwork.freedesktop.org/series/70286/ into shape. >>>> >>>> Wait I'm a little confused, sorry for the naivete here. Does this mean >>>> these kinds of changes are blocked on the above series? Or are you >>>> saying I should go ahead and convert this to the dynamic subtests? Or >>>> that we can move forward with the current approach and convert at a >>>> later time? >>> >>> Option 2 is blocked by series 70286, sorry for the confusion. I don't >>> mind slapping your patch in while waiting for that, since it's $RAND >>> days away still to get there, if it fixes a problem you're having now. >> >> I think hack for static enumeration if in --list-subtest mode did not work >> in practice. Andi, do you remember the details? > > __for_each_physical_engine checks for igt_only_list_subtests(), > if so it behaves like __for_each_static_engine(). > > Was this what you asked? Yes. Am I mis-remembering that there was a problem with this mandating to keep using __for_each_static_engine directly for subtest enumeration? Maybe I am.. Regards, Tvrtko _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [igt-dev] [PATCH i-g-t] tests/i915: Use engine query interface for gem_ctx_isolation/persistence 2019-12-05 9:54 ` Tvrtko Ursulin @ 2019-12-05 10:09 ` Petri Latvala 2019-12-05 10:23 ` Tvrtko Ursulin 2019-12-05 10:12 ` Andi Shyti 1 sibling, 1 reply; 14+ messages in thread From: Petri Latvala @ 2019-12-05 10:09 UTC (permalink / raw) To: Tvrtko Ursulin; +Cc: igt-dev On Thu, Dec 05, 2019 at 09:54:25AM +0000, Tvrtko Ursulin wrote: > > On 05/12/2019 09:47, Andi Shyti wrote: > > Hi, > > > > On Thu, Dec 05, 2019 at 09:22:56AM +0000, Tvrtko Ursulin wrote: > > > On 05/12/2019 09:01, Petri Latvala wrote: > > > > On Wed, Dec 04, 2019 at 07:05:12PM +0000, Summers, Stuart wrote: > > > > > On Wed, 2019-12-04 at 16:53 +0200, Petri Latvala wrote: > > > > > > On Wed, Dec 04, 2019 at 09:36:11AM +0000, Tvrtko Ursulin wrote: > > > > > > > > > > > > > > On 03/12/2019 05:11, Stuart Summers wrote: > > > > > > > > Align with gem_exec_basic and other tests using the newer > > > > > > > > engine query interface into i915 to enumerate active engines. > > > > > > > > > > > > > > > > Signed-off-by: Stuart Summers <stuart.summers@intel.com> > > > > > > > > --- > > > > > > > > tests/i915/gem_ctx_isolation.c | 4 ++-- > > > > > > > > tests/i915/gem_ctx_persistence.c | 2 +- > > > > > > > > 2 files changed, 3 insertions(+), 3 deletions(-) > > > > > > > > > > > > > > > > diff --git a/tests/i915/gem_ctx_isolation.c > > > > > > > > b/tests/i915/gem_ctx_isolation.c > > > > > > > > index 6aa27133..9435209e 100644 > > > > > > > > --- a/tests/i915/gem_ctx_isolation.c > > > > > > > > +++ b/tests/i915/gem_ctx_isolation.c > > > > > > > > @@ -856,6 +856,7 @@ static unsigned int > > > > > > > > __has_context_isolation(int fd) > > > > > > > > igt_main > > > > > > > > { > > > > > > > > + struct intel_execution_engine2 *e; > > > > > > > > unsigned int has_context_isolation = 0; > > > > > > > > int fd = -1; > > > > > > > > @@ -876,8 +877,7 @@ igt_main > > > > > > > > igt_skip_on(gen > LAST_KNOWN_GEN); > > > > > > > > } > > > > > > > > - for (const struct intel_execution_engine2 *e = > > > > > > > > intel_execution_engines2; > > > > > > > > - e->name; e++) { > > > > > > > > + __for_each_physical_engine(fd, e) { > > > > > > > > igt_subtest_group { > > > > > > > > igt_fixture { > > > > > > > > igt_require(has_context_isolati > > > > > > > > on & (1 << e->class)); > > > > > > > > diff --git a/tests/i915/gem_ctx_persistence.c > > > > > > > > b/tests/i915/gem_ctx_persistence.c > > > > > > > > index d68431ae..30772159 100644 > > > > > > > > --- a/tests/i915/gem_ctx_persistence.c > > > > > > > > +++ b/tests/i915/gem_ctx_persistence.c > > > > > > > > @@ -727,7 +727,7 @@ igt_main > > > > > > > > igt_subtest("hangcheck") > > > > > > > > test_nohangcheck_hostile(i915); > > > > > > > > - __for_each_static_engine(e) { > > > > > > > > + __for_each_physical_engine(i915, e) { > > > > > > > > igt_subtest_group { > > > > > > > > igt_fixture { > > > > > > > > gem_require_ring(i915, e- > > > > > > > > > flags); > > > > > > > > > > > > > > > > > > > > > > __for_each_static_engine is correct, at least if you don't want CI > > > > > > > folks go > > > > > > > look for their pitchforks. :) Same for the first hunk, everything > > > > > > > that > > > > > > > enumerates subtests needs to be static. > > > > > > > > > > > > > > Option 2, the preferred one - convert the tests to > > > > > > > igt_subtest_with_dynamic > > > > > > > and then you can use __for_each_physical_engine. > > > > > > > > > > > > Doesn't __for_each_physical_engine anyway have a hack for being > > > > > > called in that context? > > > > > > > > > > > > Btw, option 2 is > > > > > > https://gitlab.freedesktop.org/drm/igt-gpu-tools/issues/44 > > > > > > > > > > > > Currently blocking that effort is getting > > > > > > https://patchwork.freedesktop.org/series/70286/ into shape. > > > > > > > > > > Wait I'm a little confused, sorry for the naivete here. Does this mean > > > > > these kinds of changes are blocked on the above series? Or are you > > > > > saying I should go ahead and convert this to the dynamic subtests? Or > > > > > that we can move forward with the current approach and convert at a > > > > > later time? > > > > > > > > Option 2 is blocked by series 70286, sorry for the confusion. I don't > > > > mind slapping your patch in while waiting for that, since it's $RAND > > > > days away still to get there, if it fixes a problem you're having now. > > > > > > I think hack for static enumeration if in --list-subtest mode did not work > > > in practice. Andi, do you remember the details? > > > > __for_each_physical_engine checks for igt_only_list_subtests(), > > if so it behaves like __for_each_static_engine(). > > > > Was this what you asked? > > Yes. Am I mis-remembering that there was a problem with this mandating to > keep using __for_each_static_engine directly for subtest enumeration? Maybe > I am.. __physical is used in a couple of tests to enumerate subtests already, and there are still issues caused by that. Not in the CI sense, the enumeration works, but can cause havoc when executing the tests. Don't have links available on hand now, sorry. In a nutshell: It's ok for now. I don't like it, but it's used already and it's going away throughout soon. Feel free to use it until said otherwise. -- Petri Latvala _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [igt-dev] [PATCH i-g-t] tests/i915: Use engine query interface for gem_ctx_isolation/persistence 2019-12-05 10:09 ` Petri Latvala @ 2019-12-05 10:23 ` Tvrtko Ursulin 2019-12-05 22:07 ` Summers, Stuart 0 siblings, 1 reply; 14+ messages in thread From: Tvrtko Ursulin @ 2019-12-05 10:23 UTC (permalink / raw) To: Petri Latvala; +Cc: igt-dev On 05/12/2019 10:09, Petri Latvala wrote: > On Thu, Dec 05, 2019 at 09:54:25AM +0000, Tvrtko Ursulin wrote: >> >> On 05/12/2019 09:47, Andi Shyti wrote: >>> Hi, >>> >>> On Thu, Dec 05, 2019 at 09:22:56AM +0000, Tvrtko Ursulin wrote: >>>> On 05/12/2019 09:01, Petri Latvala wrote: >>>>> On Wed, Dec 04, 2019 at 07:05:12PM +0000, Summers, Stuart wrote: >>>>>> On Wed, 2019-12-04 at 16:53 +0200, Petri Latvala wrote: >>>>>>> On Wed, Dec 04, 2019 at 09:36:11AM +0000, Tvrtko Ursulin wrote: >>>>>>>> >>>>>>>> On 03/12/2019 05:11, Stuart Summers wrote: >>>>>>>>> Align with gem_exec_basic and other tests using the newer >>>>>>>>> engine query interface into i915 to enumerate active engines. >>>>>>>>> >>>>>>>>> Signed-off-by: Stuart Summers <stuart.summers@intel.com> >>>>>>>>> --- >>>>>>>>> tests/i915/gem_ctx_isolation.c | 4 ++-- >>>>>>>>> tests/i915/gem_ctx_persistence.c | 2 +- >>>>>>>>> 2 files changed, 3 insertions(+), 3 deletions(-) >>>>>>>>> >>>>>>>>> diff --git a/tests/i915/gem_ctx_isolation.c >>>>>>>>> b/tests/i915/gem_ctx_isolation.c >>>>>>>>> index 6aa27133..9435209e 100644 >>>>>>>>> --- a/tests/i915/gem_ctx_isolation.c >>>>>>>>> +++ b/tests/i915/gem_ctx_isolation.c >>>>>>>>> @@ -856,6 +856,7 @@ static unsigned int >>>>>>>>> __has_context_isolation(int fd) >>>>>>>>> igt_main >>>>>>>>> { >>>>>>>>> + struct intel_execution_engine2 *e; >>>>>>>>> unsigned int has_context_isolation = 0; >>>>>>>>> int fd = -1; >>>>>>>>> @@ -876,8 +877,7 @@ igt_main >>>>>>>>> igt_skip_on(gen > LAST_KNOWN_GEN); >>>>>>>>> } >>>>>>>>> - for (const struct intel_execution_engine2 *e = >>>>>>>>> intel_execution_engines2; >>>>>>>>> - e->name; e++) { >>>>>>>>> + __for_each_physical_engine(fd, e) { >>>>>>>>> igt_subtest_group { >>>>>>>>> igt_fixture { >>>>>>>>> igt_require(has_context_isolati >>>>>>>>> on & (1 << e->class)); >>>>>>>>> diff --git a/tests/i915/gem_ctx_persistence.c >>>>>>>>> b/tests/i915/gem_ctx_persistence.c >>>>>>>>> index d68431ae..30772159 100644 >>>>>>>>> --- a/tests/i915/gem_ctx_persistence.c >>>>>>>>> +++ b/tests/i915/gem_ctx_persistence.c >>>>>>>>> @@ -727,7 +727,7 @@ igt_main >>>>>>>>> igt_subtest("hangcheck") >>>>>>>>> test_nohangcheck_hostile(i915); >>>>>>>>> - __for_each_static_engine(e) { >>>>>>>>> + __for_each_physical_engine(i915, e) { >>>>>>>>> igt_subtest_group { >>>>>>>>> igt_fixture { >>>>>>>>> gem_require_ring(i915, e- >>>>>>>>>> flags); >>>>>>>>> >>>>>>>> >>>>>>>> __for_each_static_engine is correct, at least if you don't want CI >>>>>>>> folks go >>>>>>>> look for their pitchforks. :) Same for the first hunk, everything >>>>>>>> that >>>>>>>> enumerates subtests needs to be static. >>>>>>>> >>>>>>>> Option 2, the preferred one - convert the tests to >>>>>>>> igt_subtest_with_dynamic >>>>>>>> and then you can use __for_each_physical_engine. >>>>>>> >>>>>>> Doesn't __for_each_physical_engine anyway have a hack for being >>>>>>> called in that context? >>>>>>> >>>>>>> Btw, option 2 is >>>>>>> https://gitlab.freedesktop.org/drm/igt-gpu-tools/issues/44 >>>>>>> >>>>>>> Currently blocking that effort is getting >>>>>>> https://patchwork.freedesktop.org/series/70286/ into shape. >>>>>> >>>>>> Wait I'm a little confused, sorry for the naivete here. Does this mean >>>>>> these kinds of changes are blocked on the above series? Or are you >>>>>> saying I should go ahead and convert this to the dynamic subtests? Or >>>>>> that we can move forward with the current approach and convert at a >>>>>> later time? >>>>> >>>>> Option 2 is blocked by series 70286, sorry for the confusion. I don't >>>>> mind slapping your patch in while waiting for that, since it's $RAND >>>>> days away still to get there, if it fixes a problem you're having now. >>>> >>>> I think hack for static enumeration if in --list-subtest mode did not work >>>> in practice. Andi, do you remember the details? >>> >>> __for_each_physical_engine checks for igt_only_list_subtests(), >>> if so it behaves like __for_each_static_engine(). >>> >>> Was this what you asked? >> >> Yes. Am I mis-remembering that there was a problem with this mandating to >> keep using __for_each_static_engine directly for subtest enumeration? Maybe >> I am.. > > __physical is used in a couple of tests to enumerate subtests already, > and there are still issues caused by that. Not in the CI sense, the > enumeration works, but can cause havoc when executing the tests. Don't > have links available on hand now, sorry. > > In a nutshell: It's ok for now. I don't like it, but it's used already > and it's going away throughout soon. Feel free to use it until said > otherwise. Okay, sorry for the confusion: Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Regards, Tvrtko _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [igt-dev] [PATCH i-g-t] tests/i915: Use engine query interface for gem_ctx_isolation/persistence 2019-12-05 10:23 ` Tvrtko Ursulin @ 2019-12-05 22:07 ` Summers, Stuart 0 siblings, 0 replies; 14+ messages in thread From: Summers, Stuart @ 2019-12-05 22:07 UTC (permalink / raw) To: tvrtko.ursulin, Latvala, Petri; +Cc: igt-dev [-- Attachment #1.1: Type: text/plain, Size: 6376 bytes --] On Thu, 2019-12-05 at 10:23 +0000, Tvrtko Ursulin wrote: > On 05/12/2019 10:09, Petri Latvala wrote: > > On Thu, Dec 05, 2019 at 09:54:25AM +0000, Tvrtko Ursulin wrote: > > > > > > On 05/12/2019 09:47, Andi Shyti wrote: > > > > Hi, > > > > > > > > On Thu, Dec 05, 2019 at 09:22:56AM +0000, Tvrtko Ursulin wrote: > > > > > On 05/12/2019 09:01, Petri Latvala wrote: > > > > > > On Wed, Dec 04, 2019 at 07:05:12PM +0000, Summers, Stuart > > > > > > wrote: > > > > > > > On Wed, 2019-12-04 at 16:53 +0200, Petri Latvala wrote: > > > > > > > > On Wed, Dec 04, 2019 at 09:36:11AM +0000, Tvrtko > > > > > > > > Ursulin wrote: > > > > > > > > > > > > > > > > > > On 03/12/2019 05:11, Stuart Summers wrote: > > > > > > > > > > Align with gem_exec_basic and other tests using the > > > > > > > > > > newer > > > > > > > > > > engine query interface into i915 to enumerate > > > > > > > > > > active engines. > > > > > > > > > > > > > > > > > > > > Signed-off-by: Stuart Summers < > > > > > > > > > > stuart.summers@intel.com> > > > > > > > > > > --- > > > > > > > > > > tests/i915/gem_ctx_isolation.c | 4 ++-- > > > > > > > > > > tests/i915/gem_ctx_persistence.c | 2 +- > > > > > > > > > > 2 files changed, 3 insertions(+), 3 > > > > > > > > > > deletions(-) > > > > > > > > > > > > > > > > > > > > diff --git a/tests/i915/gem_ctx_isolation.c > > > > > > > > > > b/tests/i915/gem_ctx_isolation.c > > > > > > > > > > index 6aa27133..9435209e 100644 > > > > > > > > > > --- a/tests/i915/gem_ctx_isolation.c > > > > > > > > > > +++ b/tests/i915/gem_ctx_isolation.c > > > > > > > > > > @@ -856,6 +856,7 @@ static unsigned int > > > > > > > > > > __has_context_isolation(int fd) > > > > > > > > > > igt_main > > > > > > > > > > { > > > > > > > > > > + struct intel_execution_engine2 *e; > > > > > > > > > > unsigned int has_context_isolation = 0; > > > > > > > > > > int fd = -1; > > > > > > > > > > @@ -876,8 +877,7 @@ igt_main > > > > > > > > > > igt_skip_on(gen > > > > > > > > > > > LAST_KNOWN_GEN); > > > > > > > > > > } > > > > > > > > > > - for (const struct intel_execution_engine2 *e = > > > > > > > > > > intel_execution_engines2; > > > > > > > > > > - e->name; e++) { > > > > > > > > > > + __for_each_physical_engine(fd, e) { > > > > > > > > > > igt_subtest_group { > > > > > > > > > > igt_fixture { > > > > > > > > > > igt_require(has > > > > > > > > > > _context_isolati > > > > > > > > > > on & (1 << e->class)); > > > > > > > > > > diff --git a/tests/i915/gem_ctx_persistence.c > > > > > > > > > > b/tests/i915/gem_ctx_persistence.c > > > > > > > > > > index d68431ae..30772159 100644 > > > > > > > > > > --- a/tests/i915/gem_ctx_persistence.c > > > > > > > > > > +++ b/tests/i915/gem_ctx_persistence.c > > > > > > > > > > @@ -727,7 +727,7 @@ igt_main > > > > > > > > > > igt_subtest("hangcheck") > > > > > > > > > > test_nohangcheck_hostile(i915); > > > > > > > > > > - __for_each_static_engine(e) { > > > > > > > > > > + __for_each_physical_engine(i915, e) { > > > > > > > > > > igt_subtest_group { > > > > > > > > > > igt_fixture { > > > > > > > > > > gem_require_rin > > > > > > > > > > g(i915, e- > > > > > > > > > > > flags); > > > > > > > > > > > > > > > > > > __for_each_static_engine is correct, at least if you > > > > > > > > > don't want CI > > > > > > > > > folks go > > > > > > > > > look for their pitchforks. :) Same for the first > > > > > > > > > hunk, everything > > > > > > > > > that > > > > > > > > > enumerates subtests needs to be static. > > > > > > > > > > > > > > > > > > Option 2, the preferred one - convert the tests to > > > > > > > > > igt_subtest_with_dynamic > > > > > > > > > and then you can use __for_each_physical_engine. > > > > > > > > > > > > > > > > Doesn't __for_each_physical_engine anyway have a hack > > > > > > > > for being > > > > > > > > called in that context? > > > > > > > > > > > > > > > > Btw, option 2 is > > > > > > > > https://gitlab.freedesktop.org/drm/igt-gpu-tools/issues/44 > > > > > > > > > > > > > > > > Currently blocking that effort is getting > > > > > > > > https://patchwork.freedesktop.org/series/70286/ into > > > > > > > > shape. > > > > > > > > > > > > > > Wait I'm a little confused, sorry for the naivete here. > > > > > > > Does this mean > > > > > > > these kinds of changes are blocked on the above series? > > > > > > > Or are you > > > > > > > saying I should go ahead and convert this to the dynamic > > > > > > > subtests? Or > > > > > > > that we can move forward with the current approach and > > > > > > > convert at a > > > > > > > later time? > > > > > > > > > > > > Option 2 is blocked by series 70286, sorry for the > > > > > > confusion. I don't > > > > > > mind slapping your patch in while waiting for that, since > > > > > > it's $RAND > > > > > > days away still to get there, if it fixes a problem you're > > > > > > having now. > > > > > > > > > > I think hack for static enumeration if in --list-subtest mode > > > > > did not work > > > > > in practice. Andi, do you remember the details? > > > > > > > > __for_each_physical_engine checks for igt_only_list_subtests(), > > > > if so it behaves like __for_each_static_engine(). > > > > > > > > Was this what you asked? > > > > > > Yes. Am I mis-remembering that there was a problem with this > > > mandating to > > > keep using __for_each_static_engine directly for subtest > > > enumeration? Maybe > > > I am.. > > > > __physical is used in a couple of tests to enumerate subtests > > already, > > and there are still issues caused by that. Not in the CI sense, the > > enumeration works, but can cause havoc when executing the tests. > > Don't > > have links available on hand now, sorry. > > > > In a nutshell: It's ok for now. I don't like it, but it's used > > already > > and it's going away throughout soon. Feel free to use it until said > > otherwise. > > Okay, sorry for the confusion: > > Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Thanks for the review here and the discussion. Not sure I have push rights here yet. Any chance for some help getting this merged? Thanks, Stuart > > Regards, > > Tvrtko > > [-- Attachment #1.2: smime.p7s --] [-- Type: application/x-pkcs7-signature, Size: 3270 bytes --] [-- Attachment #2: Type: text/plain, Size: 153 bytes --] _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [igt-dev] [PATCH i-g-t] tests/i915: Use engine query interface for gem_ctx_isolation/persistence 2019-12-05 9:54 ` Tvrtko Ursulin 2019-12-05 10:09 ` Petri Latvala @ 2019-12-05 10:12 ` Andi Shyti 1 sibling, 0 replies; 14+ messages in thread From: Andi Shyti @ 2019-12-05 10:12 UTC (permalink / raw) To: Tvrtko Ursulin; +Cc: igt-dev, Petri Latvala Hi Tvrtko, > > On Thu, Dec 05, 2019 at 09:22:56AM +0000, Tvrtko Ursulin wrote: > > > On 05/12/2019 09:01, Petri Latvala wrote: > > > > On Wed, Dec 04, 2019 at 07:05:12PM +0000, Summers, Stuart wrote: > > > > > On Wed, 2019-12-04 at 16:53 +0200, Petri Latvala wrote: > > > > > > On Wed, Dec 04, 2019 at 09:36:11AM +0000, Tvrtko Ursulin wrote: > > > > > > > > > > > > > > On 03/12/2019 05:11, Stuart Summers wrote: > > > > > > > > Align with gem_exec_basic and other tests using the newer > > > > > > > > engine query interface into i915 to enumerate active engines. > > > > > > > > > > > > > > > > Signed-off-by: Stuart Summers <stuart.summers@intel.com> > > > > > > > > --- > > > > > > > > tests/i915/gem_ctx_isolation.c | 4 ++-- > > > > > > > > tests/i915/gem_ctx_persistence.c | 2 +- > > > > > > > > 2 files changed, 3 insertions(+), 3 deletions(-) > > > > > > > > > > > > > > > > diff --git a/tests/i915/gem_ctx_isolation.c > > > > > > > > b/tests/i915/gem_ctx_isolation.c > > > > > > > > index 6aa27133..9435209e 100644 > > > > > > > > --- a/tests/i915/gem_ctx_isolation.c > > > > > > > > +++ b/tests/i915/gem_ctx_isolation.c > > > > > > > > @@ -856,6 +856,7 @@ static unsigned int > > > > > > > > __has_context_isolation(int fd) > > > > > > > > igt_main > > > > > > > > { > > > > > > > > + struct intel_execution_engine2 *e; > > > > > > > > unsigned int has_context_isolation = 0; > > > > > > > > int fd = -1; > > > > > > > > @@ -876,8 +877,7 @@ igt_main > > > > > > > > igt_skip_on(gen > LAST_KNOWN_GEN); > > > > > > > > } > > > > > > > > - for (const struct intel_execution_engine2 *e = > > > > > > > > intel_execution_engines2; > > > > > > > > - e->name; e++) { > > > > > > > > + __for_each_physical_engine(fd, e) { > > > > > > > > igt_subtest_group { > > > > > > > > igt_fixture { > > > > > > > > igt_require(has_context_isolati > > > > > > > > on & (1 << e->class)); > > > > > > > > diff --git a/tests/i915/gem_ctx_persistence.c > > > > > > > > b/tests/i915/gem_ctx_persistence.c > > > > > > > > index d68431ae..30772159 100644 > > > > > > > > --- a/tests/i915/gem_ctx_persistence.c > > > > > > > > +++ b/tests/i915/gem_ctx_persistence.c > > > > > > > > @@ -727,7 +727,7 @@ igt_main > > > > > > > > igt_subtest("hangcheck") > > > > > > > > test_nohangcheck_hostile(i915); > > > > > > > > - __for_each_static_engine(e) { > > > > > > > > + __for_each_physical_engine(i915, e) { > > > > > > > > igt_subtest_group { > > > > > > > > igt_fixture { > > > > > > > > gem_require_ring(i915, e- > > > > > > > > > flags); > > > > > > > > > > > > > > > > > > > > > > __for_each_static_engine is correct, at least if you don't want CI > > > > > > > folks go > > > > > > > look for their pitchforks. :) Same for the first hunk, everything > > > > > > > that > > > > > > > enumerates subtests needs to be static. > > > > > > > > > > > > > > Option 2, the preferred one - convert the tests to > > > > > > > igt_subtest_with_dynamic > > > > > > > and then you can use __for_each_physical_engine. > > > > > > > > > > > > Doesn't __for_each_physical_engine anyway have a hack for being > > > > > > called in that context? > > > > > > > > > > > > Btw, option 2 is > > > > > > https://gitlab.freedesktop.org/drm/igt-gpu-tools/issues/44 > > > > > > > > > > > > Currently blocking that effort is getting > > > > > > https://patchwork.freedesktop.org/series/70286/ into shape. > > > > > > > > > > Wait I'm a little confused, sorry for the naivete here. Does this mean > > > > > these kinds of changes are blocked on the above series? Or are you > > > > > saying I should go ahead and convert this to the dynamic subtests? Or > > > > > that we can move forward with the current approach and convert at a > > > > > later time? > > > > > > > > Option 2 is blocked by series 70286, sorry for the confusion. I don't > > > > mind slapping your patch in while waiting for that, since it's $RAND > > > > days away still to get there, if it fixes a problem you're having now. > > > > > > I think hack for static enumeration if in --list-subtest mode did not work > > > in practice. Andi, do you remember the details? > > > > __for_each_physical_engine checks for igt_only_list_subtests(), > > if so it behaves like __for_each_static_engine(). > > > > Was this what you asked? > > Yes. Am I mis-remembering that there was a problem with this mandating to > keep using __for_each_static_engine directly for subtest enumeration? Maybe > I am.. we added __for_each_static_engine() for simplicity and used when the user does want to loop through static engines. But if a user wants static engines only if the API is not supported, then for_each_physical_engine() works, as well. Indeed, the code says: if (get_param()) { /* we check here if the API fails */ Loop through the static list if igt_only_list_subtests otherwise loop through exising engines (as it was done before by executing a batch buffer). } else if (!param.size) { query and map engines } else { get the engines from the previously contructed list } Andi _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2019-12-05 22:07 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-12-03 5:11 [igt-dev] [PATCH i-g-t] tests/i915: Use engine query interface for gem_ctx_isolation/persistence Stuart Summers 2019-12-03 6:03 ` [igt-dev] ✗ Fi.CI.BAT: failure for " Patchwork 2019-12-03 18:31 ` Summers, Stuart 2019-12-04 9:36 ` [igt-dev] [PATCH i-g-t] " Tvrtko Ursulin 2019-12-04 14:53 ` Petri Latvala 2019-12-04 19:05 ` Summers, Stuart 2019-12-05 9:01 ` Petri Latvala 2019-12-05 9:22 ` Tvrtko Ursulin 2019-12-05 9:47 ` Andi Shyti 2019-12-05 9:54 ` Tvrtko Ursulin 2019-12-05 10:09 ` Petri Latvala 2019-12-05 10:23 ` Tvrtko Ursulin 2019-12-05 22:07 ` Summers, Stuart 2019-12-05 10:12 ` Andi Shyti
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.