[Intel-gfx] [PATCH i-g-t v3 10/11] tests/kms_atomic_transition: add out_fences tests
Brian Starkey
brian.starkey at arm.com
Tue Jan 31 16:52:00 UTC 2017
On Mon, Jan 30, 2017 at 08:58:46PM -0500, Robert Foss wrote:
>Signed-off-by: Gustavo Padovan <gustavo.padovan at collabora.co.uk>
>Signed-off-by: Robert Foss <robert.foss at collabora.com>
>---
> lib/igt_kms.c | 35 ++++++----
> tests/kms_atomic_transition.c | 148 ++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 169 insertions(+), 14 deletions(-)
>
>diff --git a/lib/igt_kms.c b/lib/igt_kms.c
>index f14496dd..523f3f57 100644
>--- a/lib/igt_kms.c
>+++ b/lib/igt_kms.c
>@@ -53,6 +53,7 @@
> #include "intel_chipset.h"
> #include "igt_debugfs.h"
> #include "igt_sysfs.h"
>+#include "sw_sync.h"
>
> /**
> * SECTION:igt_kms
>@@ -2479,6 +2480,21 @@ static int igt_atomic_commit(igt_display_t *display, uint32_t flags, void *user_
> }
>
> ret = drmModeAtomicCommit(display->drm_fd, req, flags, user_data);
>+ if (!ret) {
>+
>+ for_each_pipe(display, pipe) {
>+ igt_pipe_t *pipe_obj = &display->pipes[pipe];
>+
>+ if (pipe_obj->out_fence != -1)
>+ continue;
Should be "if (pipe_obj->fence == -1)" ? The stuff below seems to be
assuming the fence is valid.
>+
>+ igt_assert(pipe_obj->out_fence >= 0);
>+ ret = sync_fence_wait(pipe_obj->out_fence, 1000);
>+ igt_assert(ret == 0);
>+ close(pipe_obj->out_fence);
>+ }
>+ }
>+
> drmModeAtomicFree(req);
> return ret;
>
>@@ -2972,6 +2988,11 @@ void igt_plane_set_rotation(igt_plane_t *plane, igt_rotation_t rotation)
> plane->rotation_changed = true;
> }
>
>+void igt_pipe_request_out_fence(igt_pipe_t *pipe)
Oh here it is!
>+{
>+ pipe->out_fence_requested = true;
>+}
>+
> void
> igt_pipe_set_degamma_lut(igt_pipe_t *pipe, void *ptr, size_t length)
> {
>@@ -2994,20 +3015,6 @@ igt_pipe_set_gamma_lut(igt_pipe_t *pipe, void *ptr, size_t length)
> }
>
> /**
>- * igt_pipe_set_out_fence_ptr:
>- * @pipe: pipe pointer to which background color to be set
>- * @fence_ptr: out fence pointer
>- *
>- * Sets the out fence pointer that will be passed to the kernel in
>- * the atomic ioctl. When the kernel returns the out fence pointer
>- * will contain the fd number of the out fence created by KMS.
>- */
>-void igt_pipe_set_out_fence_ptr(igt_pipe_t *pipe, int64_t *fence_ptr)
... and there this one goes. This deletion and the function above
need to be squashed into the earlier commit I suppose.
>-{
>- pipe->out_fence_ptr = fence_ptr;
>-}
>-
>-/**
> * igt_crtc_set_background:
> * @pipe: pipe pointer to which background color to be set
> * @background: background color value in BGR 16bpc
>diff --git a/tests/kms_atomic_transition.c b/tests/kms_atomic_transition.c
>index 72429759..eebb5d66 100644
>--- a/tests/kms_atomic_transition.c
>+++ b/tests/kms_atomic_transition.c
>@@ -253,6 +253,93 @@ retry:
> sprite_width, sprite_height, alpha);
> }
>
>+int *timeline;
>+pthread_t *thread;
>+int *seqno;
>+
>+static void prepare_fencing(igt_display_t *display, enum pipe pipe)
>+{
>+ igt_plane_t *plane;
>+ int n_planes;
>+
>+ igt_require_sw_sync();
>+
>+ n_planes = display->pipes[pipe].n_planes;
>+ timeline = calloc(sizeof(*timeline), n_planes);
>+ igt_assert_f(timeline != NULL, "Failed to allocate memory for timelines\n");
>+ thread = calloc(sizeof(*thread), n_planes);
>+ igt_assert_f(thread != NULL, "Failed to allocate memory for thread\n");
>+ seqno = calloc(sizeof(*seqno), n_planes);
>+ igt_assert_f(seqno != NULL, "Failed to allocate memory for seqno\n");
The 6 lines above are space-indented
>+
>+ for_each_plane_on_pipe(display, pipe, plane)
>+ timeline[plane->index] = sw_sync_timeline_create();
>+}
>+
>+static void unprepare_fencing(igt_display_t *display, enum pipe pipe)
>+{
>+ igt_plane_t *plane;
>+
>+ for_each_plane_on_pipe(display, pipe, plane)
>+ close(timeline[plane->index]);
>+
>+ free(timeline);
>+ free(thread);
>+ free(seqno);
>+}
>+
>+static void *fence_inc_thread(void *arg)
>+{
>+ int t = *((int *) arg);
>+
>+ pthread_detach(pthread_self());
>+
>+ usleep(5000);
>+ sw_sync_timeline_inc(t, 1);
>+ return NULL;
>+}
>+
>+static void configure_fencing(igt_display_t *display, enum pipe pipe)
>+{
>+ igt_plane_t *plane;
>+ int i, fd, ret;
>+
>+ for_each_plane_on_pipe(display, pipe, plane) {
>+
>+ if (!plane->fb)
>+ continue;
>+
>+ i = plane->index;
>+
>+ seqno[i]++;
>+ fd = sw_sync_timeline_create_fence(timeline[i], seqno[i]);
>+ igt_plane_set_fence_fd(plane, fd);
>+ ret = pthread_create(&thread[i], NULL, fence_inc_thread, &timeline[i]);
>+ igt_assert_eq(ret, 0);
>+ }
>+}
>+
>+static void clear_fencing(igt_display_t *display, enum pipe pipe)
>+{
>+ igt_plane_t *plane;
>+
>+ for_each_plane_on_pipe(display, pipe, plane)
>+ igt_plane_set_fence_fd(plane, -1);
Someone should close the old fence_fd if it's not -1.
I think igt_plane_set_fence_fd() should dup() the passed in fence, and
igt_display_atomic_commit() should set it to -1 after the commit.
That makes it very obvious, and that way igt_plane_set_fence_fd() can
always safely close plane->fence_fd if it gets called while the
current fence_fd is valid, because it always owns that fd. Callers
have their own copy to take care of.
In any case, I think you're leaking fds here.
>+}
>+
>+static void atomic_commit(igt_display_t *display, enum pipe pipe, unsigned int flags, void *data, bool fencing)
>+{
>+ if (fencing) {
>+ configure_fencing(display, pipe);
>+ igt_pipe_request_out_fence(&display->pipes[pipe]);
>+ }
>+
>+ igt_display_commit_atomic(display, flags, data);
>+
>+ if (fencing)
>+ clear_fencing(display, pipe);
>+}
>+
> /*
> * 1. Set primary plane to a known fb.
> * 2. Make sure getcrtc returns the correct fb id.
>@@ -273,6 +360,7 @@ run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t *output
> struct plane_parms parms[display->pipes[pipe].n_planes];
> bool skip_test = false;
> unsigned flags = DRM_MODE_PAGE_FLIP_EVENT;
>+ int ret;
>
> if (nonblocking)
> flags |= DRM_MODE_ATOMIC_NONBLOCK;
>@@ -307,11 +395,50 @@ run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t *output
>
> setup_parms(display, pipe, mode, &argb_fb, &sprite_fb, parms);
>
>+ /*
>+ * In some configurations the tests may not run to completion with all
>+ * sprite planes lit up at 4k resolution, try decreasing width/size of secondary
>+ * planes to fix this
>+ */
>+ while (1) {
>+ wm_setup_plane(display, pipe, iter_max - 1, parms);
>+
>+ if (fencing)
>+ igt_pipe_set_out_fence_ptr(&display->pipes[pipe],
That function was deleted in this patch.
>+ (int64_t *) &out_fence);
>+ ret = igt_display_try_commit_atomic(display, DRM_MODE_ATOMIC_TEST_ONLY | DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
>+
>+ if (ret != -EINVAL || n_planes < 3)
>+ break;
>+
>+ ret = 0;
>+ for_each_plane_on_pipe(display, pipe, plane) {
>+ i = plane->index;
>+
>+ if (plane->is_primary || plane->is_cursor)
>+ continue;
>+
>+ if (parms[i].width <= 512)
>+ continue;
>+
>+ parms[i].width /= 2;
>+ ret = 1;
>+ igt_info("Reducing sprite %i to %ux%u\n", i - 1, parms[i].width, parms[i].height);
>+ break;
>+ }
>+
>+ if (!ret)
>+ igt_skip("Cannot run tests without proper size sprite planes\n");
>+ }
>+
> for (i = 0; i < iter_max; i++) {
> igt_output_set_pipe(output, pipe);
>
> wm_setup_plane(display, pipe, i, parms);
>
>+ if (fencing)
>+ igt_pipe_set_out_fence_ptr(&display->pipes[pipe], &out_fence);
Same again.
>+
> igt_display_commit_atomic(display, flags, (void *)(unsigned long)i);
> drmHandleEvent(display->drm_fd, &drm_events);
>
>@@ -320,6 +447,9 @@ run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t *output
>
> wm_setup_plane(display, pipe, 0, parms);
>
>+ if (fencing)
>+ igt_pipe_request_out_fence(&display->pipes[pipe]);
>+
> igt_display_commit_atomic(display, flags, (void *)0UL);
>
> drmHandleEvent(display->drm_fd, &drm_events);
>@@ -333,6 +463,9 @@ 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);
etc.
-Brian
>+
> igt_display_commit_atomic(display, flags, (void *)(unsigned long)j);
> drmHandleEvent(display->drm_fd, &drm_events);
>
>@@ -340,6 +473,9 @@ run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t *output
> if (type == TRANSITION_MODESET)
> igt_output_override_mode(output, NULL);
>
>+ if (fencing)
>+ igt_pipe_set_out_fence_ptr(&display->pipes[pipe], &out_fence);
>+
> igt_display_commit_atomic(display, flags, (void *)(unsigned long)i);
> drmHandleEvent(display->drm_fd, &drm_events);
> }
>@@ -676,14 +812,26 @@ igt_main
> for_each_pipe_with_valid_output(&display, pipe, output)
> run_transition_test(&display, pipe, output, TRANSITION_PLANES, false, false);
>
>+ igt_subtest("plane-all-transition-fencing")
>+ for_each_pipe_with_valid_output(&display, pipe, output)
>+ run_transition_test(&display, pipe, output, TRANSITION_PLANES, false, true);
>+
> igt_subtest("plane-all-transition-nonblocking")
> for_each_pipe_with_valid_output(&display, pipe, output)
> run_transition_test(&display, pipe, output, TRANSITION_PLANES, true, false);
>
>+ igt_subtest("plane-all-transition-nonblocking-fencing")
>+ for_each_pipe_with_valid_output(&display, pipe, output)
>+ run_transition_test(&display, pipe, output, TRANSITION_PLANES, true, true);
>+
> igt_subtest("plane-all-modeset-transition")
> for_each_pipe_with_valid_output(&display, pipe, output)
> run_transition_test(&display, pipe, output, TRANSITION_MODESET, false, false);
>
>+ igt_subtest("plane-all-modeset-transition-fencing")
>+ for_each_pipe_with_valid_output(&display, pipe, output)
>+ run_transition_test(&display, pipe, output, TRANSITION_MODESET, false, true);
>+
> igt_subtest("plane-toggle-modeset-transition")
> for_each_pipe_with_valid_output(&display, pipe, output)
> run_transition_test(&display, pipe, output, TRANSITION_MODESET_DISABLE, false, false);
>--
>2.11.0.453.g787f75f05
>
More information about the Intel-gfx
mailing list