[igt-dev] [PATCH i-g-t v2] tests/kms_plane_multiple: Drain pipe before reading CRC
Maarten Lankhorst
maarten.lankhorst at linux.intel.com
Fri Mar 9 08:40:57 UTC 2018
Op 28-02-18 om 14:44 schreef Mika Kahola:
> In CI runs we every now and then fail to read correct CRC yielding an error
> when comparing reference and grabbed CRC's. Let's first fix the test so that
> we drain the pipe first and then read the correct CRC. While at it, let's
> simplify the test by combining legacy and atomic tests into a one common
> function.
>
> v2: We don't need to drain pipe when we grab first CRC
>
> References: https://bugs.freedesktop.org/show_bug.cgi?id=103166
> Signed-off-by: Mika Kahola <mika.kahola at intel.com>
> ---
> tests/kms_plane_multiple.c | 142 ++++++++++++---------------------------------
> 1 file changed, 38 insertions(+), 104 deletions(-)
>
> diff --git a/tests/kms_plane_multiple.c b/tests/kms_plane_multiple.c
> index 95b7138..ff8ede3 100644
> --- a/tests/kms_plane_multiple.c
> +++ b/tests/kms_plane_multiple.c
> @@ -32,7 +32,6 @@
>
> IGT_TEST_DESCRIPTION("Test atomic mode setting with multiple planes ");
>
> -#define MAX_CRCS 2
> #define SIZE_PLANE 256
> #define SIZE_CURSOR 128
> #define LOOP_FOREVER -1
> @@ -46,6 +45,7 @@ typedef struct {
> typedef struct {
> int drm_fd;
> igt_display_t display;
> + igt_crc_t ref_crc;
> igt_pipe_crc_t *pipe_crc;
> igt_plane_t **plane;
> struct igt_fb *fb;
> @@ -92,20 +92,23 @@ static void test_fini(data_t *data, igt_output_t *output, int n_planes)
> igt_output_set_pipe(output, PIPE_ANY);
>
> igt_pipe_crc_free(data->pipe_crc);
> + data->pipe_crc = NULL;
>
> free(data->plane);
> data->plane = NULL;
> - free(data->fb);
> - data->fb = NULL;
> +
> + igt_remove_fb(data->drm_fd, data->fb);
> +
> + igt_display_reset(&data->display);
> }
>
> static void
> -test_grab_crc(data_t *data, igt_output_t *output, enum pipe pipe, bool atomic,
> - color_t *color, uint64_t tiling, igt_crc_t **crc /* out */)
> +test_grab_crc(data_t *data, igt_output_t *output, enum pipe pipe, int commit,
> + color_t *color, uint64_t tiling)
> {
> drmModeModeInfo *mode;
> igt_plane_t *primary;
> - int ret, n;
> + int ret;
>
> igt_output_set_pipe(output, pipe);
>
> @@ -122,13 +125,16 @@ test_grab_crc(data_t *data, igt_output_t *output, enum pipe pipe, bool atomic,
>
> igt_plane_set_fb(data->plane[primary->index], &data->fb[primary->index]);
>
> - ret = igt_display_try_commit2(&data->display,
> - atomic ? COMMIT_ATOMIC : COMMIT_LEGACY);
> + ret = igt_display_try_commit2(&data->display, commit);
> igt_skip_on(ret != 0);
>
> - igt_pipe_crc_start(data->pipe_crc);
> - n = igt_pipe_crc_get_crcs(data->pipe_crc, 1, crc);
> - igt_assert_eq(n, 1);
> + if (commit == COMMIT_LEGACY) {
> + igt_pipe_crc_collect_crc(data->pipe_crc, &data->ref_crc);
> + } else {
> + igt_pipe_crc_start(data->pipe_crc);
> + igt_pipe_crc_drain(data->pipe_crc);
> + igt_pipe_crc_get_single(data->pipe_crc, &data->ref_crc);
> + }
Why the 2 different paths?
And again, after pipe_crc_start no _drain() is needed, we already have a good CRC.
> }
>
> /*
> @@ -239,17 +245,13 @@ prepare_planes(data_t *data, enum pipe pipe_id, color_t *color,
> }
>
> static void
> -test_atomic_plane_position_with_output(data_t *data, enum pipe pipe,
> - igt_output_t *output, int n_planes,
> - uint64_t tiling)
> +test_plane_position_with_output(data_t *data, enum pipe pipe,
> + igt_output_t *output, int n_planes,
> + uint64_t tiling, int commit)
> {
> - char buf[256];
> - struct drm_event *e = (void *)buf;
> color_t blue = { 0.0f, 0.0f, 1.0f };
> - igt_crc_t *ref = NULL;
> - igt_crc_t *crc = NULL;
> - unsigned int vblank_start, vblank_stop;
> - int i, n, ret;
> + igt_crc_t crc;
> + int i;
> int iterations = opt.iterations < 1 ? 1 : opt.iterations;
> bool loop_forever;
> char info[256];
> @@ -269,89 +271,27 @@ test_atomic_plane_position_with_output(data_t *data, enum pipe pipe,
>
> test_init(data, pipe, n_planes);
>
> - test_grab_crc(data, output, pipe, true, &blue, tiling, &ref);
> + test_grab_crc(data, output, pipe, commit, &blue, tiling);
>
> i = 0;
> while (i < iterations || loop_forever) {
> prepare_planes(data, pipe, &blue, tiling, n_planes, output);
>
> - vblank_start = kmstest_get_vblank(data->display.drm_fd, pipe,
> - DRM_VBLANK_NEXTONMISS);
> + igt_display_commit2(&data->display, commit);
>
> - igt_display_commit_atomic(&data->display,
> - DRM_MODE_PAGE_FLIP_EVENT,
> - &data->display);
> -
> - igt_set_timeout(1, "Stuck on page flip");
> -
> - ret = read(data->display.drm_fd, buf, sizeof(buf));
> - igt_assert(ret >= 0);
> -
> - vblank_stop = kmstest_get_vblank(data->display.drm_fd, pipe, 0);
> - igt_assert_eq(e->type, DRM_EVENT_FLIP_COMPLETE);
> - igt_reset_timeout();
> -
> - n = igt_pipe_crc_get_crcs(data->pipe_crc, vblank_stop - vblank_start, &crc);
> -
> - igt_assert(vblank_stop - vblank_start <= MAX_CRCS);
> - igt_assert_eq(n, vblank_stop - vblank_start);
> -
> - igt_assert_crc_equal(ref, crc);
> -
> - i++;
> - }
> -
> - igt_pipe_crc_stop(data->pipe_crc);
> -
> - test_fini(data, output, n_planes);
> -}
> -
> -static void
> -test_legacy_plane_position_with_output(data_t *data, enum pipe pipe,
> - igt_output_t *output, int n_planes,
> - uint64_t tiling)
> -{
> - color_t blue = { 0.0f, 0.0f, 1.0f };
> - igt_crc_t *ref = NULL;
> - igt_crc_t *crc = NULL;
> - int i, n;
> - int iterations = opt.iterations < 1 ? 1 : opt.iterations;
> - bool loop_forever;
> - char info[256];
> -
> - if (opt.iterations == LOOP_FOREVER) {
> - loop_forever = true;
> - sprintf(info, "forever");
> - } else {
> - loop_forever = false;
> - sprintf(info, "for %d %s",
> - iterations, iterations > 1 ? "iterations" : "iteration");
> - }
> -
> - igt_info("Testing connector %s using pipe %s with %d planes %s with seed %d\n",
> - igt_output_name(output), kmstest_pipe_name(pipe), n_planes,
> - info, opt.seed);
> -
> - test_init(data, pipe, n_planes);
> -
> - test_grab_crc(data, output, pipe, false, &blue, tiling, &ref);
> -
> - i = 0;
> - while (i < iterations || loop_forever) {
> - prepare_planes(data, pipe, &blue, tiling, n_planes, output);
> -
> - igt_display_commit2(&data->display, COMMIT_LEGACY);
> -
> - n = igt_pipe_crc_get_crcs(data->pipe_crc, MAX_CRCS, &crc);
> -
> - igt_assert_eq(n, MAX_CRCS);
> -
> - igt_assert_crc_equal(ref, crc);
> + if (commit == COMMIT_LEGACY) {
> + igt_pipe_crc_collect_crc(data->pipe_crc, &crc);
> + } else {
> + igt_pipe_crc_drain(data->pipe_crc);
> + igt_pipe_crc_get_single(data->pipe_crc, &crc);
> + }
Again why the separate paths?
> + igt_assert_crc_equal(&data->ref_crc, &crc);
>
> i++;
> }
>
> - igt_pipe_crc_stop(data->pipe_crc);
> + if (commit != COMMIT_LEGACY)
> + igt_pipe_crc_stop(data->pipe_crc);
>
> test_fini(data, output, n_planes);
> }
> @@ -378,17 +318,11 @@ test_plane_position(data_t *data, enum pipe pipe, bool atomic, uint64_t tiling)
>
> connected_outs = 0;
> for_each_valid_output_on_pipe(&data->display, pipe, output) {
> - if (atomic)
> - test_atomic_plane_position_with_output(data, pipe,
> - output,
> - n_planes,
> - tiling);
> - else
> - test_legacy_plane_position_with_output(data, pipe,
> - output,
> - n_planes,
> - tiling);
> -
> + test_plane_position_with_output(data, pipe,
> + output,
> + n_planes,
> + tiling,
> + atomic ? COMMIT_ATOMIC : COMMIT_LEGACY);
> connected_outs++;
> }
>
More information about the igt-dev
mailing list