* [igt-dev] [PATCH i-g-t v2] tests/kms_plane_multiple: Drain pipe before reading CRC
@ 2018-02-28 13:44 Mika Kahola
2018-02-28 14:34 ` [igt-dev] ✓ Fi.CI.BAT: success for tests/kms_plane_multiple: Drain pipe before reading CRC (rev2) Patchwork
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Mika Kahola @ 2018-02-28 13:44 UTC (permalink / raw)
To: igt-dev
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@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);
+ }
}
/*
@@ -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);
+ }
+ 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++;
}
--
2.7.4
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [igt-dev] ✓ Fi.CI.BAT: success for tests/kms_plane_multiple: Drain pipe before reading CRC (rev2)
2018-02-28 13:44 [igt-dev] [PATCH i-g-t v2] tests/kms_plane_multiple: Drain pipe before reading CRC Mika Kahola
@ 2018-02-28 14:34 ` Patchwork
2018-02-28 18:47 ` [igt-dev] ✗ Fi.CI.IGT: warning " Patchwork
2018-03-09 8:40 ` [igt-dev] [PATCH i-g-t v2] tests/kms_plane_multiple: Drain pipe before reading CRC Maarten Lankhorst
2 siblings, 0 replies; 7+ messages in thread
From: Patchwork @ 2018-02-28 14:34 UTC (permalink / raw)
To: Mika Kahola; +Cc: igt-dev
== Series Details ==
Series: tests/kms_plane_multiple: Drain pipe before reading CRC (rev2)
URL : https://patchwork.freedesktop.org/series/38851/
State : success
== Summary ==
IGT patchset tested on top of latest successful build
0d47ec161b4eca1b41c5348604aa05b105e5d1cf tests/perf: Fix build warning
with latest DRM-Tip kernel build CI_DRM_3848
446c2101a788 drm-tip: 2018y-02m-28d-13h-18m-03s UTC integration manifest
No testlist changes.
---- Known issues:
Test kms_frontbuffer_tracking:
Subgroup basic:
pass -> FAIL (fi-cnl-y3) fdo#103167
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-b:
incomplete -> PASS (fi-bdw-5557u) fdo#104162
fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
fdo#104162 https://bugs.freedesktop.org/show_bug.cgi?id=104162
fi-bdw-5557u total:288 pass:267 dwarn:0 dfail:0 fail:0 skip:21 time:415s
fi-blb-e6850 total:288 pass:223 dwarn:1 dfail:0 fail:0 skip:64 time:375s
fi-bsw-n3050 total:288 pass:242 dwarn:0 dfail:0 fail:0 skip:46 time:491s
fi-bwr-2160 total:288 pass:183 dwarn:0 dfail:0 fail:0 skip:105 time:288s
fi-bxt-j4205 total:288 pass:259 dwarn:0 dfail:0 fail:0 skip:29 time:481s
fi-byt-j1900 total:288 pass:253 dwarn:0 dfail:0 fail:0 skip:35 time:468s
fi-byt-n2820 total:288 pass:249 dwarn:0 dfail:0 fail:0 skip:39 time:465s
fi-cfl-8700k total:288 pass:260 dwarn:0 dfail:0 fail:0 skip:28 time:395s
fi-cfl-s2 total:288 pass:262 dwarn:0 dfail:0 fail:0 skip:26 time:564s
fi-cnl-y3 total:288 pass:261 dwarn:0 dfail:0 fail:1 skip:26 time:584s
fi-elk-e7500 total:288 pass:229 dwarn:0 dfail:0 fail:0 skip:59 time:409s
fi-gdg-551 total:288 pass:179 dwarn:0 dfail:0 fail:1 skip:108 time:285s
fi-glk-1 total:288 pass:260 dwarn:0 dfail:0 fail:0 skip:28 time:509s
fi-hsw-4770 total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:387s
fi-ilk-650 total:288 pass:228 dwarn:0 dfail:0 fail:0 skip:60 time:414s
fi-ivb-3520m total:288 pass:259 dwarn:0 dfail:0 fail:0 skip:29 time:453s
fi-ivb-3770 total:288 pass:255 dwarn:0 dfail:0 fail:0 skip:33 time:411s
fi-kbl-7500u total:288 pass:263 dwarn:1 dfail:0 fail:0 skip:24 time:451s
fi-kbl-7560u total:288 pass:269 dwarn:0 dfail:0 fail:0 skip:19 time:494s
fi-kbl-7567u total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:450s
fi-kbl-r total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:497s
fi-pnv-d510 total:288 pass:222 dwarn:1 dfail:0 fail:0 skip:65 time:584s
fi-skl-6260u total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:434s
fi-skl-6600u total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:499s
fi-skl-6700hq total:288 pass:262 dwarn:0 dfail:0 fail:0 skip:26 time:519s
fi-skl-6700k2 total:288 pass:264 dwarn:0 dfail:0 fail:0 skip:24 time:491s
fi-skl-6770hq total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:485s
fi-skl-guc total:288 pass:260 dwarn:0 dfail:0 fail:0 skip:28 time:413s
fi-skl-gvtdvm total:288 pass:265 dwarn:0 dfail:0 fail:0 skip:23 time:433s
fi-snb-2520m total:288 pass:248 dwarn:0 dfail:0 fail:0 skip:40 time:523s
fi-snb-2600 total:288 pass:248 dwarn:0 dfail:0 fail:0 skip:40 time:397s
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1023/issues.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 7+ messages in thread
* [igt-dev] ✗ Fi.CI.IGT: warning for tests/kms_plane_multiple: Drain pipe before reading CRC (rev2)
2018-02-28 13:44 [igt-dev] [PATCH i-g-t v2] tests/kms_plane_multiple: Drain pipe before reading CRC Mika Kahola
2018-02-28 14:34 ` [igt-dev] ✓ Fi.CI.BAT: success for tests/kms_plane_multiple: Drain pipe before reading CRC (rev2) Patchwork
@ 2018-02-28 18:47 ` Patchwork
2018-03-09 8:40 ` [igt-dev] [PATCH i-g-t v2] tests/kms_plane_multiple: Drain pipe before reading CRC Maarten Lankhorst
2 siblings, 0 replies; 7+ messages in thread
From: Patchwork @ 2018-02-28 18:47 UTC (permalink / raw)
To: Mika Kahola; +Cc: igt-dev
== Series Details ==
Series: tests/kms_plane_multiple: Drain pipe before reading CRC (rev2)
URL : https://patchwork.freedesktop.org/series/38851/
State : warning
== Summary ==
---- Possible new issues:
Test gem_pwrite:
Subgroup big-gtt-backwards:
pass -> SKIP (shard-apl)
Test kms_chv_cursor_fail:
Subgroup pipe-b-256x256-bottom-edge:
dmesg-warn -> PASS (shard-snb)
Test pm_rc6_residency:
Subgroup rc6-accuracy:
pass -> SKIP (shard-snb)
---- Known issues:
Test gem_eio:
Subgroup in-flight:
incomplete -> PASS (shard-apl) fdo#104945
Test kms_chv_cursor_fail:
Subgroup pipe-b-128x128-right-edge:
dmesg-warn -> PASS (shard-snb) fdo#105185
Test kms_rotation_crc:
Subgroup primary-rotation-180:
pass -> FAIL (shard-snb) fdo#103925
Test kms_sysfs_edid_timing:
warn -> PASS (shard-apl) fdo#100047
Test prime_vgem:
Subgroup basic-fence-flip:
pass -> FAIL (shard-apl) fdo#104008
fdo#104945 https://bugs.freedesktop.org/show_bug.cgi?id=104945
fdo#105185 https://bugs.freedesktop.org/show_bug.cgi?id=105185
fdo#103925 https://bugs.freedesktop.org/show_bug.cgi?id=103925
fdo#100047 https://bugs.freedesktop.org/show_bug.cgi?id=100047
fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008
shard-apl total:3460 pass:1818 dwarn:1 dfail:0 fail:8 skip:1633 time:12206s
shard-hsw total:3460 pass:1767 dwarn:1 dfail:0 fail:1 skip:1690 time:11585s
shard-snb total:3460 pass:1358 dwarn:1 dfail:0 fail:2 skip:2099 time:6675s
Blacklisted hosts:
shard-kbl total:3460 pass:1944 dwarn:1 dfail:0 fail:7 skip:1508 time:9601s
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1023/shards.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [igt-dev] [PATCH i-g-t v2] tests/kms_plane_multiple: Drain pipe before reading CRC
2018-02-28 13:44 [igt-dev] [PATCH i-g-t v2] tests/kms_plane_multiple: Drain pipe before reading CRC Mika Kahola
2018-02-28 14:34 ` [igt-dev] ✓ Fi.CI.BAT: success for tests/kms_plane_multiple: Drain pipe before reading CRC (rev2) Patchwork
2018-02-28 18:47 ` [igt-dev] ✗ Fi.CI.IGT: warning " Patchwork
@ 2018-03-09 8:40 ` Maarten Lankhorst
2018-03-09 11:25 ` Mika Kahola
2 siblings, 1 reply; 7+ messages in thread
From: Maarten Lankhorst @ 2018-03-09 8:40 UTC (permalink / raw)
To: Mika Kahola, igt-dev
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@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++;
> }
>
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [igt-dev] [PATCH i-g-t v2] tests/kms_plane_multiple: Drain pipe before reading CRC
2018-03-09 8:40 ` [igt-dev] [PATCH i-g-t v2] tests/kms_plane_multiple: Drain pipe before reading CRC Maarten Lankhorst
@ 2018-03-09 11:25 ` Mika Kahola
2018-03-09 11:34 ` Maarten Lankhorst
0 siblings, 1 reply; 7+ messages in thread
From: Mika Kahola @ 2018-03-09 11:25 UTC (permalink / raw)
To: Maarten Lankhorst, igt-dev
On Fri, 2018-03-09 at 09:40 +0100, Maarten Lankhorst wrote:
> 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@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?
The *_start *_get_single method didn't work for COMMIT_ATOMIC so I
decided to use legacy way for COMMIT_LEGACY. Maybe we could after all
drop the legacy commits from this test. Originally, this test was all
about atomic tests.
>
> And again, after pipe_crc_start no _drain() is needed, we already
> have a good CRC.
Yeah, this thing is an echo from the past. I remember we already
discussed this when previous patch was under review.
Thanks for the review!
> >
> > }
> >
> > /*
> > @@ -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_NEXTO
> > NMISS);
> > + 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(dat
> > a, pipe,
> > - out
> > put,
> > - n_p
> > lanes,
> > - til
> > ing);
> > - else
> > - test_legacy_plane_position_with_output(dat
> > a, pipe,
> > - out
> > put,
> > - n_p
> > lanes,
> > - til
> > ing);
> > -
> > + test_plane_position_with_output(data, pipe,
> > + output,
> > + n_planes,
> > + tiling,
> > + atomic ?
> > COMMIT_ATOMIC : COMMIT_LEGACY);
> > connected_outs++;
> > }
> >
>
--
Mika Kahola - Intel OTC
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [igt-dev] [PATCH i-g-t v2] tests/kms_plane_multiple: Drain pipe before reading CRC
2018-03-09 11:25 ` Mika Kahola
@ 2018-03-09 11:34 ` Maarten Lankhorst
2018-03-09 12:08 ` Mika Kahola
0 siblings, 1 reply; 7+ messages in thread
From: Maarten Lankhorst @ 2018-03-09 11:34 UTC (permalink / raw)
To: mika.kahola, igt-dev
Op 09-03-18 om 12:25 schreef Mika Kahola:
> On Fri, 2018-03-09 at 09:40 +0100, Maarten Lankhorst wrote:
>> 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@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?
> The *_start *_get_single method didn't work for COMMIT_ATOMIC so I
> decided to use legacy way for COMMIT_LEGACY. Maybe we could after all
> drop the legacy commits from this test. Originally, this test was all
> about atomic tests.
>> And again, after pipe_crc_start no _drain() is needed, we already
>> have a good CRC.
> Yeah, this thing is an echo from the past. I remember we already
> discussed this when previous patch was under review.
>
> Thanks for the review!
Just nuke the legacy paths then. :)
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [igt-dev] [PATCH i-g-t v2] tests/kms_plane_multiple: Drain pipe before reading CRC
2018-03-09 11:34 ` Maarten Lankhorst
@ 2018-03-09 12:08 ` Mika Kahola
0 siblings, 0 replies; 7+ messages in thread
From: Mika Kahola @ 2018-03-09 12:08 UTC (permalink / raw)
To: Maarten Lankhorst, igt-dev
On Fri, 2018-03-09 at 12:34 +0100, Maarten Lankhorst wrote:
> Op 09-03-18 om 12:25 schreef Mika Kahola:
> >
> > On Fri, 2018-03-09 at 09:40 +0100, Maarten Lankhorst wrote:
> > >
> > > 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@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?
> > The *_start *_get_single method didn't work for COMMIT_ATOMIC so I
> > decided to use legacy way for COMMIT_LEGACY. Maybe we could after
> > all
> > drop the legacy commits from this test. Originally, this test was
> > all
> > about atomic tests.
> > >
> > > And again, after pipe_crc_start no _drain() is needed, we already
> > > have a good CRC.
> > Yeah, this thing is an echo from the past. I remember we already
> > discussed this when previous patch was under review.
> >
> > Thanks for the review!
> Just nuke the legacy paths then. :)
Ok. I'll do that :)
--
Mika Kahola - Intel OTC
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-03-09 12:08 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-02-28 13:44 [igt-dev] [PATCH i-g-t v2] tests/kms_plane_multiple: Drain pipe before reading CRC Mika Kahola
2018-02-28 14:34 ` [igt-dev] ✓ Fi.CI.BAT: success for tests/kms_plane_multiple: Drain pipe before reading CRC (rev2) Patchwork
2018-02-28 18:47 ` [igt-dev] ✗ Fi.CI.IGT: warning " Patchwork
2018-03-09 8:40 ` [igt-dev] [PATCH i-g-t v2] tests/kms_plane_multiple: Drain pipe before reading CRC Maarten Lankhorst
2018-03-09 11:25 ` Mika Kahola
2018-03-09 11:34 ` Maarten Lankhorst
2018-03-09 12:08 ` Mika Kahola
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox