Re: [PATCH i-g-t v3 11/11] tests/kms_atomic_transition: add in_fences tests

Brian Starkey
2017-01-31T16:52:23Z

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