| Message ID | 20170803120529.27300-1-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Quoting Daniel Vetter (2017-08-03 13:05:29) > Back when we used cs flips it made sense to go through different > engines, since a buffer busy on an engine that we couldnt' use for > cs flipping ended up in different paths. > > But with atomic we use a worker for all flips, and going through the > combinatorial growth of engines just wastes precious machine time. > More so the more modern the platform is. Then test against all engines simultaneously. > Of course gem tests should still do some diagonal testing across all > engines, but the kms side can afford to be a bit cheaper. kms has a separate waiting interface, it would be prudent to not assume that coverage elsewhere gives coverage of kms. Note also that kms has a number of special paths for domain handling which require care and attention. -Chris
On Thu, Aug 3, 2017 at 2:34 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote: > Quoting Daniel Vetter (2017-08-03 13:05:29) >> Back when we used cs flips it made sense to go through different >> engines, since a buffer busy on an engine that we couldnt' use for >> cs flipping ended up in different paths. >> >> But with atomic we use a worker for all flips, and going through the >> combinatorial growth of engines just wastes precious machine time. >> More so the more modern the platform is. > > Then test against all engines simultaneously. What's the benefit of that? Just making sure that we handle multiple dma_fence on the same obj/fb? Afaics the current test doesn't do that either. >> Of course gem tests should still do some diagonal testing across all >> engines, but the kms side can afford to be a bit cheaper. > > kms has a separate waiting interface, it would be prudent to not assume > that coverage elsewhere gives coverage of kms. Note also that kms has a > number of special paths for domain handling which require care and > attention. Again, sounds like a neat extension of test coverage, but I'm just aiming to make the current coverage more efficient in terms of machine time used. Atm kbl takes 15' longer than hsw, and these kms_busy tests are a big chunk of that (another one is that somehow everything related to modeset just takes a bit longer, the final one is that kbl has more features, so skips less). -Daniel
Quoting Daniel Vetter (2017-08-03 16:17:57) > On Thu, Aug 3, 2017 at 2:34 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote: > > Quoting Daniel Vetter (2017-08-03 13:05:29) > >> Back when we used cs flips it made sense to go through different > >> engines, since a buffer busy on an engine that we couldnt' use for > >> cs flipping ended up in different paths. > >> > >> But with atomic we use a worker for all flips, and going through the > >> combinatorial growth of engines just wastes precious machine time. > >> More so the more modern the platform is. > > > > Then test against all engines simultaneously. > > What's the benefit of that? Just making sure that we handle multiple > dma_fence on the same obj/fb? Afaics the current test doesn't do that > either. We increase the potential source for errors, keep the cursory check that any engine can generate content for a flip, and all without adding any significant time (a couple of extra microseconds to submit, 10us to handle the extra interrupts). > >> Of course gem tests should still do some diagonal testing across all > >> engines, but the kms side can afford to be a bit cheaper. > > > > kms has a separate waiting interface, it would be prudent to not assume > > that coverage elsewhere gives coverage of kms. Note also that kms has a > > number of special paths for domain handling which require care and > > attention. > > Again, sounds like a neat extension of test coverage, but I'm just > aiming to make the current coverage more efficient in terms of machine > time used. Indeed, so use all engines instead of an arbitrary choice. -Chris
Op 03-08-17 om 14:05 schreef Daniel Vetter: > Back when we used cs flips it made sense to go through different > engines, since a buffer busy on an engine that we couldnt' use for > cs flipping ended up in different paths. > > But with atomic we use a worker for all flips, and going through the > combinatorial growth of engines just wastes precious machine time. > More so the more modern the platform is. > > Of course gem tests should still do some diagonal testing across all > engines, but the kms side can afford to be a bit cheaper. > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > --- Acked-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
diff --git a/tests/kms_busy.c b/tests/kms_busy.c index af7335db0d11..436d16e6acb9 100644 --- a/tests/kms_busy.c +++ b/tests/kms_busy.c @@ -309,7 +309,8 @@ static void test_pageflip_modeset_hang(igt_display_t *dpy, igt_main { igt_display_t display = { .drm_fd = -1, .n_pipes = IGT_MAX_PIPES }; - const struct intel_execution_engine *e; + /* we only test on render */ + const struct intel_execution_engine *e = intel_execution_engines[1]; igt_skip_on_simulation(); @@ -333,82 +334,80 @@ igt_main igt_skip_on(n >= display.n_pipes); } - for (e = intel_execution_engines; e->name; e++) { - igt_subtest_f("%sflip-%s-%s", - e->exec_id == 0 ? "basic-" : "", - e->name, kmstest_pipe_name(n)) { + igt_subtest_f("%sflip-%s-%s", + e->exec_id == 0 ? "basic-" : "", + e->name, kmstest_pipe_name(n)) { + igt_require(gem_has_ring(display.drm_fd, + e->exec_id | e->flags)); + + test_flip(&display, e->exec_id | e->flags, n, false); + } + igt_subtest_f("%smodeset-%s-%s", + e->exec_id == 0 ? "basic-" : "", + e->name, kmstest_pipe_name(n)) { + igt_require(gem_has_ring(display.drm_fd, + e->exec_id | e->flags)); + + test_flip(&display, e->exec_id | e->flags, n, true); + } + + igt_subtest_group { + igt_hang_t hang; + + igt_fixture { igt_require(gem_has_ring(display.drm_fd, e->exec_id | e->flags)); - test_flip(&display, e->exec_id | e->flags, n, false); + hang = igt_allow_hang(display.drm_fd, 0, 0); } - igt_subtest_f("%smodeset-%s-%s", - e->exec_id == 0 ? "basic-" : "", + + igt_subtest_f("extended-pageflip-modeset-hang-oldfb-%s-%s", e->name, kmstest_pipe_name(n)) { igt_require(gem_has_ring(display.drm_fd, e->exec_id | e->flags)); - test_flip(&display, e->exec_id | e->flags, n, true); + test_pageflip_modeset_hang(&display, e->exec_id | e->flags, n); } - igt_subtest_group { - igt_hang_t hang; - - igt_fixture { - igt_require(gem_has_ring(display.drm_fd, - e->exec_id | e->flags)); + igt_fixture + igt_require(display.is_atomic); - hang = igt_allow_hang(display.drm_fd, 0, 0); - } + igt_subtest_f("extended-pageflip-hang-oldfb-%s-%s", + e->name, kmstest_pipe_name(n)) + test_hang(&display, e->exec_id | e->flags, n, false, false); - igt_subtest_f("extended-pageflip-modeset-hang-oldfb-%s-%s", - e->name, kmstest_pipe_name(n)) { - igt_require(gem_has_ring(display.drm_fd, - e->exec_id | e->flags)); + igt_subtest_f("extended-pageflip-hang-newfb-%s-%s", + e->name, kmstest_pipe_name(n)) + test_hang(&display, e->exec_id | e->flags, n, false, true); - test_pageflip_modeset_hang(&display, e->exec_id | e->flags, n); - } + igt_subtest_f("extended-modeset-hang-oldfb-%s-%s", + e->name, kmstest_pipe_name(n)) + test_hang(&display, e->exec_id | e->flags, n, true, false); - igt_fixture - igt_require(display.is_atomic); + igt_subtest_f("extended-modeset-hang-newfb-%s-%s", + e->name, kmstest_pipe_name(n)) + test_hang(&display, e->exec_id | e->flags, n, true, true); - igt_subtest_f("extended-pageflip-hang-oldfb-%s-%s", - e->name, kmstest_pipe_name(n)) - test_hang(&display, e->exec_id | e->flags, n, false, false); - - igt_subtest_f("extended-pageflip-hang-newfb-%s-%s", - e->name, kmstest_pipe_name(n)) - test_hang(&display, e->exec_id | e->flags, n, false, true); - - igt_subtest_f("extended-modeset-hang-oldfb-%s-%s", - e->name, kmstest_pipe_name(n)) - test_hang(&display, e->exec_id | e->flags, n, true, false); - - igt_subtest_f("extended-modeset-hang-newfb-%s-%s", - e->name, kmstest_pipe_name(n)) - test_hang(&display, e->exec_id | e->flags, n, true, true); - - igt_subtest_f("extended-modeset-hang-oldfb-with-reset-%s-%s", - e->name, kmstest_pipe_name(n)) { - igt_set_module_param_int("force_reset_modeset_test", 1); + igt_subtest_f("extended-modeset-hang-oldfb-with-reset-%s-%s", + e->name, kmstest_pipe_name(n)) { + igt_set_module_param_int("force_reset_modeset_test", 1); - test_hang(&display, e->exec_id | e->flags, n, true, false); + test_hang(&display, e->exec_id | e->flags, n, true, false); - igt_set_module_param_int("force_reset_modeset_test", 0); - } + igt_set_module_param_int("force_reset_modeset_test", 0); + } - igt_subtest_f("extended-modeset-hang-newfb-with-reset-%s-%s", - e->name, kmstest_pipe_name(n)) { - igt_set_module_param_int("force_reset_modeset_test", 1); + igt_subtest_f("extended-modeset-hang-newfb-with-reset-%s-%s", + e->name, kmstest_pipe_name(n)) { + igt_set_module_param_int("force_reset_modeset_test", 1); - test_hang(&display, e->exec_id | e->flags, n, true, true); + test_hang(&display, e->exec_id | e->flags, n, true, true); - igt_set_module_param_int("force_reset_modeset_test", 0); - } + igt_set_module_param_int("force_reset_modeset_test", 0); + } - igt_fixture { - igt_disallow_hang(display.drm_fd, hang); - } + igt_fixture { + igt_disallow_hang(display.drm_fd, hang); } } }
Back when we used cs flips it made sense to go through different engines, since a buffer busy on an engine that we couldnt' use for cs flipping ended up in different paths. But with atomic we use a worker for all flips, and going through the combinatorial growth of engines just wastes precious machine time. More so the more modern the platform is. Of course gem tests should still do some diagonal testing across all engines, but the kms side can afford to be a bit cheaper. Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> --- tests/kms_busy.c | 111 +++++++++++++++++++++++++++---------------------------- 1 file changed, 55 insertions(+), 56 deletions(-)