Message ID | 20170131015847.28628-12-robert.foss@collabora.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On Mon, Jan 30, 2017 at 08:58:47PM -0500, Robert Foss wrote: >From: Gustavo Padovan <gustavo.padovan@collabora.co.uk> > >Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk> >Signed-off-by: Robert Foss <robert.foss@collabora.com> >--- > lib/igt_kms.c | 3 +++ > tests/kms_atomic_transition.c | 48 ++++++++++++++++++++++++++----------------- > 2 files changed, 32 insertions(+), 19 deletions(-) > >diff --git a/lib/igt_kms.c b/lib/igt_kms.c >index 523f3f57..bc815363 100644 >--- a/lib/igt_kms.c >+++ b/lib/igt_kms.c >@@ -2009,6 +2009,9 @@ igt_atomic_prepare_plane_commit(igt_plane_t *plane, igt_pipe_t *pipe, > if (plane->fence_fd >= 0) { > uint64_t fence_fd = (int64_t) plane->fence_fd; > igt_atomic_populate_plane_req(req, plane, IGT_PLANE_IN_FENCE_FD, fence_fd); >+ >+ /* reset fence_fd to prevent it from being set for the next commit */ >+ plane->fence_fd = -1; Who closes it? > } > > if (plane->fb_changed) { >diff --git a/tests/kms_atomic_transition.c b/tests/kms_atomic_transition.c >index eebb5d66..0876bbb3 100644 >--- a/tests/kms_atomic_transition.c >+++ b/tests/kms_atomic_transition.c >@@ -23,7 +23,9 @@ > > #include "igt.h" > #include "drmtest.h" >+#include "sw_sync.h" > #include <errno.h> >+#include <pthread.h> > #include <stdbool.h> > #include <stdio.h> > #include <string.h> >@@ -362,6 +364,9 @@ run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t *output > unsigned flags = DRM_MODE_PAGE_FLIP_EVENT; > int ret; > >+ if (fencing) >+ prepare_fencing(display, pipe); >+ > if (nonblocking) > flags |= DRM_MODE_ATOMIC_NONBLOCK; > >@@ -404,18 +409,19 @@ run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t *output > wm_setup_plane(display, pipe, iter_max - 1, parms); > > if (fencing) >- igt_pipe_set_out_fence_ptr(&display->pipes[pipe], >- (int64_t *) &out_fence); >+ igt_pipe_request_out_fence(&display->pipes[pipe]); >+ Hopefully this can get rebased away? I'm getting confused about what's actually being added/changed in this commit. > ret = igt_display_try_commit_atomic(display, DRM_MODE_ATOMIC_TEST_ONLY | DRM_MODE_ATOMIC_ALLOW_MODESET, NULL); > >- if (ret != -EINVAL || n_planes < 3) >+ if (ret != -EINVAL || display->pipes[pipe].n_planes < 3) > break; > > ret = 0; > for_each_plane_on_pipe(display, pipe, plane) { > i = plane->index; > >- if (plane->is_primary || plane->is_cursor) >+ if (plane->type == DRM_PLANE_TYPE_PRIMARY || >+ plane->type == DRM_PLANE_TYPE_CURSOR) > continue; This seems spurious? ... A bit more add/remove churn below which can hopefully go away with a rebase. Cheers, -Brian > > if (parms[i].width <= 512) >@@ -436,10 +442,8 @@ run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t *output > > wm_setup_plane(display, pipe, i, parms); > >- if (fencing) >- igt_pipe_set_out_fence_ptr(&display->pipes[pipe], &out_fence); >+ atomic_commit(display, pipe, flags, (void *)(unsigned long)i, fencing); > >- igt_display_commit_atomic(display, flags, (void *)(unsigned long)i); > drmHandleEvent(display->drm_fd, &drm_events); > > if (type == TRANSITION_MODESET_DISABLE) { >@@ -463,19 +467,14 @@ run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t *output > if (type == TRANSITION_MODESET) > igt_output_override_mode(output, &override_mode); > >- if (fencing) >- igt_pipe_set_out_fence_ptr(&display->pipes[pipe], &out_fence); >- >- igt_display_commit_atomic(display, flags, (void *)(unsigned long)j); >+ atomic_commit(display, pipe, flags, (void *)(unsigned long)i, fencing); > drmHandleEvent(display->drm_fd, &drm_events); > > wm_setup_plane(display, pipe, i, parms); > if (type == TRANSITION_MODESET) > igt_output_override_mode(output, NULL); > >- if (fencing) >- igt_pipe_set_out_fence_ptr(&display->pipes[pipe], &out_fence); >- >+ atomic_commit(display, pipe, flags, (void *)(unsigned long)i, fencing); > igt_display_commit_atomic(display, flags, (void *)(unsigned long)i); > drmHandleEvent(display->drm_fd, &drm_events); > } >@@ -483,6 +482,8 @@ run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t *output > } > > cleanup: >+ unprepare_fencing(display, pipe); >+ > igt_output_set_pipe(output, PIPE_NONE); > > for_each_plane_on_pipe(display, pipe, plane) >@@ -617,7 +618,7 @@ static void collect_crcs_mask(igt_pipe_crc_t **pipe_crcs, unsigned mask, igt_crc > } > } > >-static void run_modeset_tests(igt_display_t *display, int howmany, bool nonblocking) >+static void run_modeset_tests(igt_display_t *display, int howmany, bool nonblocking, bool fencing) > { > struct igt_fb fbs[2]; > int i, j; >@@ -664,6 +665,9 @@ static void run_modeset_tests(igt_display_t *display, int howmany, bool nonblock > igt_plane_set_size(plane, mode->hdisplay, mode->vdisplay); > } else > igt_plane_set_fb(plane, NULL); >+ >+ if(fencing) >+ igt_pipe_request_out_fence(&display->pipes[i]); > } > > /* >@@ -751,7 +755,7 @@ cleanup: > > } > >-static void run_modeset_transition(igt_display_t *display, int requested_outputs, bool nonblocking) >+static void run_modeset_transition(igt_display_t *display, int requested_outputs, bool nonblocking, bool fencing) > { > igt_output_t *outputs[I915_MAX_PIPES] = {}; > int num_outputs = 0; >@@ -779,7 +783,7 @@ static void run_modeset_transition(igt_display_t *display, int requested_outputs > "Should have at least %i outputs, found %i\n", > requested_outputs, num_outputs); > >- run_modeset_tests(display, requested_outputs, nonblocking); >+ run_modeset_tests(display, requested_outputs, nonblocking, fencing); > } > > igt_main >@@ -838,10 +842,16 @@ igt_main > > for (i = 1; i <= I915_MAX_PIPES; i++) { > igt_subtest_f("%ix-modeset-transitions", i) >- run_modeset_transition(&display, i, false); >+ run_modeset_transition(&display, i, false, false); > > igt_subtest_f("%ix-modeset-transitions-nonblocking", i) >- run_modeset_transition(&display, i, true); >+ run_modeset_transition(&display, i, true, false); >+ >+ igt_subtest_f("%ix-modeset-transitions-fencing", i) >+ run_modeset_transition(&display, i, false, true); >+ >+ igt_subtest_f("%ix-modeset-transitions-nonblocking-fencing", i) >+ run_modeset_transition(&display, i, true, true); > } > > igt_fixture { >-- >2.11.0.453.g787f75f05 >
On 2017-01-31 11:52 AM, Brian Starkey wrote: > On Mon, Jan 30, 2017 at 08:58:47PM -0500, Robert Foss wrote: >> From: Gustavo Padovan <gustavo.padovan@collabora.co.uk> >> >> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk> >> Signed-off-by: Robert Foss <robert.foss@collabora.com> >> --- >> lib/igt_kms.c | 3 +++ >> tests/kms_atomic_transition.c | 48 >> ++++++++++++++++++++++++++----------------- >> 2 files changed, 32 insertions(+), 19 deletions(-) >> >> diff --git a/lib/igt_kms.c b/lib/igt_kms.c >> index 523f3f57..bc815363 100644 >> --- a/lib/igt_kms.c >> +++ b/lib/igt_kms.c >> @@ -2009,6 +2009,9 @@ igt_atomic_prepare_plane_commit(igt_plane_t >> *plane, igt_pipe_t *pipe, >> if (plane->fence_fd >= 0) { >> uint64_t fence_fd = (int64_t) plane->fence_fd; >> igt_atomic_populate_plane_req(req, plane, >> IGT_PLANE_IN_FENCE_FD, fence_fd); >> + >> + /* reset fence_fd to prevent it from being set for the next >> commit */ >> + plane->fence_fd = -1; > > Who closes it? Fixed in v4. > >> } >> >> if (plane->fb_changed) { >> diff --git a/tests/kms_atomic_transition.c >> b/tests/kms_atomic_transition.c >> index eebb5d66..0876bbb3 100644 >> --- a/tests/kms_atomic_transition.c >> +++ b/tests/kms_atomic_transition.c >> @@ -23,7 +23,9 @@ >> >> #include "igt.h" >> #include "drmtest.h" >> +#include "sw_sync.h" >> #include <errno.h> >> +#include <pthread.h> >> #include <stdbool.h> >> #include <stdio.h> >> #include <string.h> >> @@ -362,6 +364,9 @@ run_transition_test(igt_display_t *display, enum >> pipe pipe, igt_output_t *output >> unsigned flags = DRM_MODE_PAGE_FLIP_EVENT; >> int ret; >> >> + if (fencing) >> + prepare_fencing(display, pipe); >> + >> if (nonblocking) >> flags |= DRM_MODE_ATOMIC_NONBLOCK; >> >> @@ -404,18 +409,19 @@ run_transition_test(igt_display_t *display, enum >> pipe pipe, igt_output_t *output >> wm_setup_plane(display, pipe, iter_max - 1, parms); >> >> if (fencing) >> - igt_pipe_set_out_fence_ptr(&display->pipes[pipe], >> - (int64_t *) &out_fence); >> + igt_pipe_request_out_fence(&display->pipes[pipe]); >> + > > Hopefully this can get rebased away? I'm getting confused about > what's actually being added/changed in this commit. Fixed in v4. The igt_pipe_set_out_fence_ptr calls were really spread about. > >> ret = igt_display_try_commit_atomic(display, >> DRM_MODE_ATOMIC_TEST_ONLY | DRM_MODE_ATOMIC_ALLOW_MODESET, NULL); >> >> - if (ret != -EINVAL || n_planes < 3) >> + if (ret != -EINVAL || display->pipes[pipe].n_planes < 3) >> break; >> >> ret = 0; >> for_each_plane_on_pipe(display, pipe, plane) { >> i = plane->index; >> >> - if (plane->is_primary || plane->is_cursor) >> + if (plane->type == DRM_PLANE_TYPE_PRIMARY || >> + plane->type == DRM_PLANE_TYPE_CURSOR) >> continue; > > This seems spurious? > > ... > > A bit more add/remove churn below which can hopefully go away with a > rebase. > Ack, removing churn in v4. > Cheers, > -Brian >> >> if (parms[i].width <= 512) >> @@ -436,10 +442,8 @@ run_transition_test(igt_display_t *display, enum >> pipe pipe, igt_output_t *output >> >> wm_setup_plane(display, pipe, i, parms); >> >> - if (fencing) >> - igt_pipe_set_out_fence_ptr(&display->pipes[pipe], >> &out_fence); >> + atomic_commit(display, pipe, flags, (void *)(unsigned long)i, >> fencing); >> >> - igt_display_commit_atomic(display, flags, (void *)(unsigned >> long)i); >> drmHandleEvent(display->drm_fd, &drm_events); >> >> if (type == TRANSITION_MODESET_DISABLE) { >> @@ -463,19 +467,14 @@ run_transition_test(igt_display_t *display, enum >> pipe pipe, igt_output_t *output >> if (type == TRANSITION_MODESET) >> igt_output_override_mode(output, &override_mode); >> >> - if (fencing) >> - igt_pipe_set_out_fence_ptr(&display->pipes[pipe], >> &out_fence); >> - >> - igt_display_commit_atomic(display, flags, (void >> *)(unsigned long)j); >> + atomic_commit(display, pipe, flags, (void *)(unsigned >> long)i, fencing); >> drmHandleEvent(display->drm_fd, &drm_events); >> >> wm_setup_plane(display, pipe, i, parms); >> if (type == TRANSITION_MODESET) >> igt_output_override_mode(output, NULL); >> >> - if (fencing) >> - igt_pipe_set_out_fence_ptr(&display->pipes[pipe], >> &out_fence); >> - >> + atomic_commit(display, pipe, flags, (void *)(unsigned >> long)i, fencing); >> igt_display_commit_atomic(display, flags, (void >> *)(unsigned long)i); >> drmHandleEvent(display->drm_fd, &drm_events); >> } >> @@ -483,6 +482,8 @@ run_transition_test(igt_display_t *display, enum >> pipe pipe, igt_output_t *output >> } >> >> cleanup: >> + unprepare_fencing(display, pipe); >> + >> igt_output_set_pipe(output, PIPE_NONE); >> >> for_each_plane_on_pipe(display, pipe, plane) >> @@ -617,7 +618,7 @@ static void collect_crcs_mask(igt_pipe_crc_t >> **pipe_crcs, unsigned mask, igt_crc >> } >> } >> >> -static void run_modeset_tests(igt_display_t *display, int howmany, >> bool nonblocking) >> +static void run_modeset_tests(igt_display_t *display, int howmany, >> bool nonblocking, bool fencing) >> { >> struct igt_fb fbs[2]; >> int i, j; >> @@ -664,6 +665,9 @@ static void run_modeset_tests(igt_display_t >> *display, int howmany, bool nonblock >> igt_plane_set_size(plane, mode->hdisplay, mode->vdisplay); >> } else >> igt_plane_set_fb(plane, NULL); >> + >> + if(fencing) >> + igt_pipe_request_out_fence(&display->pipes[i]); >> } >> >> /* >> @@ -751,7 +755,7 @@ cleanup: >> >> } >> >> -static void run_modeset_transition(igt_display_t *display, int >> requested_outputs, bool nonblocking) >> +static void run_modeset_transition(igt_display_t *display, int >> requested_outputs, bool nonblocking, bool fencing) >> { >> igt_output_t *outputs[I915_MAX_PIPES] = {}; >> int num_outputs = 0; >> @@ -779,7 +783,7 @@ static void run_modeset_transition(igt_display_t >> *display, int requested_outputs >> "Should have at least %i outputs, found %i\n", >> requested_outputs, num_outputs); >> >> - run_modeset_tests(display, requested_outputs, nonblocking); >> + run_modeset_tests(display, requested_outputs, nonblocking, fencing); >> } >> >> igt_main >> @@ -838,10 +842,16 @@ igt_main >> >> for (i = 1; i <= I915_MAX_PIPES; i++) { >> igt_subtest_f("%ix-modeset-transitions", i) >> - run_modeset_transition(&display, i, false); >> + run_modeset_transition(&display, i, false, false); >> >> igt_subtest_f("%ix-modeset-transitions-nonblocking", i) >> - run_modeset_transition(&display, i, true); >> + run_modeset_transition(&display, i, true, false); >> + >> + igt_subtest_f("%ix-modeset-transitions-fencing", i) >> + run_modeset_transition(&display, i, false, true); >> + >> + igt_subtest_f("%ix-modeset-transitions-nonblocking-fencing", i) >> + run_modeset_transition(&display, i, true, true); >> } >> >> igt_fixture { >> -- >> 2.11.0.453.g787f75f05 >>
diff --git a/lib/igt_kms.c b/lib/igt_kms.c index 523f3f57..bc815363 100644 --- a/lib/igt_kms.c +++ b/lib/igt_kms.c @@ -2009,6 +2009,9 @@ igt_atomic_prepare_plane_commit(igt_plane_t *plane, igt_pipe_t *pipe, if (plane->fence_fd >= 0) { uint64_t fence_fd = (int64_t) plane->fence_fd; igt_atomic_populate_plane_req(req, plane, IGT_PLANE_IN_FENCE_FD, fence_fd); + + /* reset fence_fd to prevent it from being set for the next commit */ + plane->fence_fd = -1; } if (plane->fb_changed) { diff --git a/tests/kms_atomic_transition.c b/tests/kms_atomic_transition.c index eebb5d66..0876bbb3 100644 --- a/tests/kms_atomic_transition.c +++ b/tests/kms_atomic_transition.c @@ -23,7 +23,9 @@ #include "igt.h" #include "drmtest.h" +#include "sw_sync.h" #include <errno.h> +#include <pthread.h> #include <stdbool.h> #include <stdio.h> #include <string.h> @@ -362,6 +364,9 @@ run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t *output unsigned flags = DRM_MODE_PAGE_FLIP_EVENT; int ret; + if (fencing) + prepare_fencing(display, pipe); + if (nonblocking) flags |= DRM_MODE_ATOMIC_NONBLOCK; @@ -404,18 +409,19 @@ run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t *output wm_setup_plane(display, pipe, iter_max - 1, parms); if (fencing) - igt_pipe_set_out_fence_ptr(&display->pipes[pipe], - (int64_t *) &out_fence); + igt_pipe_request_out_fence(&display->pipes[pipe]); + ret = igt_display_try_commit_atomic(display, DRM_MODE_ATOMIC_TEST_ONLY | DRM_MODE_ATOMIC_ALLOW_MODESET, NULL); - if (ret != -EINVAL || n_planes < 3) + if (ret != -EINVAL || display->pipes[pipe].n_planes < 3) break; ret = 0; for_each_plane_on_pipe(display, pipe, plane) { i = plane->index; - if (plane->is_primary || plane->is_cursor) + if (plane->type == DRM_PLANE_TYPE_PRIMARY || + plane->type == DRM_PLANE_TYPE_CURSOR) continue; if (parms[i].width <= 512) @@ -436,10 +442,8 @@ run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t *output wm_setup_plane(display, pipe, i, parms); - if (fencing) - igt_pipe_set_out_fence_ptr(&display->pipes[pipe], &out_fence); + atomic_commit(display, pipe, flags, (void *)(unsigned long)i, fencing); - igt_display_commit_atomic(display, flags, (void *)(unsigned long)i); drmHandleEvent(display->drm_fd, &drm_events); if (type == TRANSITION_MODESET_DISABLE) { @@ -463,19 +467,14 @@ run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t *output if (type == TRANSITION_MODESET) igt_output_override_mode(output, &override_mode); - if (fencing) - igt_pipe_set_out_fence_ptr(&display->pipes[pipe], &out_fence); - - igt_display_commit_atomic(display, flags, (void *)(unsigned long)j); + atomic_commit(display, pipe, flags, (void *)(unsigned long)i, fencing); drmHandleEvent(display->drm_fd, &drm_events); wm_setup_plane(display, pipe, i, parms); if (type == TRANSITION_MODESET) igt_output_override_mode(output, NULL); - if (fencing) - igt_pipe_set_out_fence_ptr(&display->pipes[pipe], &out_fence); - + atomic_commit(display, pipe, flags, (void *)(unsigned long)i, fencing); igt_display_commit_atomic(display, flags, (void *)(unsigned long)i); drmHandleEvent(display->drm_fd, &drm_events); } @@ -483,6 +482,8 @@ run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t *output } cleanup: + unprepare_fencing(display, pipe); + igt_output_set_pipe(output, PIPE_NONE); for_each_plane_on_pipe(display, pipe, plane) @@ -617,7 +618,7 @@ static void collect_crcs_mask(igt_pipe_crc_t **pipe_crcs, unsigned mask, igt_crc } } -static void run_modeset_tests(igt_display_t *display, int howmany, bool nonblocking) +static void run_modeset_tests(igt_display_t *display, int howmany, bool nonblocking, bool fencing) { struct igt_fb fbs[2]; int i, j; @@ -664,6 +665,9 @@ static void run_modeset_tests(igt_display_t *display, int howmany, bool nonblock igt_plane_set_size(plane, mode->hdisplay, mode->vdisplay); } else igt_plane_set_fb(plane, NULL); + + if(fencing) + igt_pipe_request_out_fence(&display->pipes[i]); } /* @@ -751,7 +755,7 @@ cleanup: } -static void run_modeset_transition(igt_display_t *display, int requested_outputs, bool nonblocking) +static void run_modeset_transition(igt_display_t *display, int requested_outputs, bool nonblocking, bool fencing) { igt_output_t *outputs[I915_MAX_PIPES] = {}; int num_outputs = 0; @@ -779,7 +783,7 @@ static void run_modeset_transition(igt_display_t *display, int requested_outputs "Should have at least %i outputs, found %i\n", requested_outputs, num_outputs); - run_modeset_tests(display, requested_outputs, nonblocking); + run_modeset_tests(display, requested_outputs, nonblocking, fencing); } igt_main @@ -838,10 +842,16 @@ igt_main for (i = 1; i <= I915_MAX_PIPES; i++) { igt_subtest_f("%ix-modeset-transitions", i) - run_modeset_transition(&display, i, false); + run_modeset_transition(&display, i, false, false); igt_subtest_f("%ix-modeset-transitions-nonblocking", i) - run_modeset_transition(&display, i, true); + run_modeset_transition(&display, i, true, false); + + igt_subtest_f("%ix-modeset-transitions-fencing", i) + run_modeset_transition(&display, i, false, true); + + igt_subtest_f("%ix-modeset-transitions-nonblocking-fencing", i) + run_modeset_transition(&display, i, true, true); } igt_fixture {