* [PATCH i-g-t v2] kms_rotation_crc: 90 degree flip test is not a stress test
@ 2017-09-08 11:10 Tvrtko Ursulin
2017-09-08 11:24 ` [PATCH i-g-t v3] " Tvrtko Ursulin
` (4 more replies)
0 siblings, 5 replies; 18+ messages in thread
From: Tvrtko Ursulin @ 2017-09-08 11:10 UTC (permalink / raw)
To: Intel-gfx; +Cc: Daniel Vetter
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
To the best of my recollection the page flipping test was added
simply to start exercising page flips with 90/270 rotation.
There is no need to do 60 flips which can take quite some time,
because we do 60 flips against each pipe and each fb geometry.
Also, calling this a stress test is also not matching the
original idea of the test.
Several changes:
1. Remove the stress from the name and reduce the number of
flips to one only.
2. Move the page flip before CRC collection for a more useful
test.
3. Add more flipping tests, for different rotation and sprite
planes.
4. Convert to table driven subtest generation.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Katarzyna Dec <katarzyna.dec@intel.com>
---
tests/intel-ci/extended.testlist | 2 +-
tests/kms_rotation_crc.c | 137 ++++++++++++++++++++++++---------------
2 files changed, 85 insertions(+), 54 deletions(-)
diff --git a/tests/intel-ci/extended.testlist b/tests/intel-ci/extended.testlist
index 17eed013f810..fb71091758c6 100644
--- a/tests/intel-ci/extended.testlist
+++ b/tests/intel-ci/extended.testlist
@@ -82,7 +82,7 @@ igt@gem_ringfill@vebox-bomb
igt@gem_userptr_blits@stress-mm
igt@gem_userptr_blits@stress-mm-invalidate-close
igt@gem_userptr_blits@stress-mm-invalidate-close-overlap
-igt@kms_rotation_crc@primary-rotation-90-flip-stress
+igt@kms_rotation_crc@primary-rotation-90-flip
igt@pm_rpm@gem-execbuf-stress
igt@pm_rpm@gem-execbuf-stress-extra-wait
igt@pm_rpm@gem-execbuf-stress-pc8
diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index 83e37f126f40..20f1adb67769 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -41,7 +41,7 @@ typedef struct {
int pos_y;
uint32_t override_fmt;
uint64_t override_tiling;
- unsigned int flip_stress;
+ unsigned int flips;
} data_t;
static void
@@ -219,7 +219,7 @@ static void prepare_fbs(data_t *data, igt_output_t *output,
igt_create_fb(data->gfx_fd, w, h, pixel_format, tiling, &data->fb);
- if (data->flip_stress) {
+ if (data->flips) {
igt_create_fb(data->gfx_fd, w, h, pixel_format, tiling, &data->fb_flip);
paint_squares(data, IGT_ROTATION_0, &data->fb_flip, 0.92);
}
@@ -351,15 +351,17 @@ static void test_plane_rotation(data_t *data, int plane_type)
ret = igt_display_try_commit2(display, commit);
if (data->override_fmt || data->override_tiling) {
igt_assert_eq(ret, -EINVAL);
- } else {
- igt_assert_eq(ret, 0);
- igt_pipe_crc_collect_crc(data->pipe_crc,
- &crc_output);
- igt_assert_crc_equal(&data->ref_crc,
- &crc_output);
+ continue;
}
- flip_count = data->flip_stress;
+ /* Verify commit was ok. */
+ igt_assert_eq(ret, 0);
+
+ /*
+ * If flips are requested flip away and back before
+ * checking CRC.
+ */
+ flip_count = data->flips;
while (flip_count--) {
ret = drmModePageFlip(data->gfx_fd,
output->config.crtc->crtc_id,
@@ -376,6 +378,9 @@ static void test_plane_rotation(data_t *data, int plane_type)
igt_assert_eq(ret, 0);
wait_for_pageflip(data->gfx_fd);
}
+
+ igt_pipe_crc_collect_crc(data->pipe_crc, &crc_output);
+ igt_assert_crc_equal(&data->ref_crc, &crc_output);
}
valid_tests++;
@@ -569,8 +574,66 @@ err_commit:
igt_assert_eq(ret, 0);
}
+static const char *plane_test_str(unsigned plane)
+{
+ switch (plane) {
+ case DRM_PLANE_TYPE_PRIMARY:
+ return "primary";
+ case DRM_PLANE_TYPE_OVERLAY:
+ return "sprite";
+ case DRM_PLANE_TYPE_CURSOR:
+ return "cursor";
+ default:
+ igt_assert(0);
+ }
+}
+
+static const char *rot_test_str(igt_rotation_t rot)
+{
+ switch (rot) {
+ case IGT_ROTATION_90:
+ return "90";
+ case IGT_ROTATION_180:
+ return "180";
+ case IGT_ROTATION_270:
+ return "270";
+ default:
+ igt_assert(0);
+ }
+}
+
+static const char *flip_test_str(unsigned flips)
+{
+ if (flips > 1)
+ return "-flip-stress";
+ else if (flips)
+ return "-flip";
+ else
+ return "";
+}
+
igt_main
{
+ struct rot_subtest {
+ unsigned plane;
+ igt_rotation_t rot;
+ unsigned flips;
+ } *subtest, subtests[] = {
+ { DRM_PLANE_TYPE_PRIMARY, IGT_ROTATION_90, 0 },
+ { DRM_PLANE_TYPE_PRIMARY, IGT_ROTATION_180, 0 },
+ { DRM_PLANE_TYPE_PRIMARY, IGT_ROTATION_270, 0 },
+ { DRM_PLANE_TYPE_PRIMARY, IGT_ROTATION_90, 1 },
+ { DRM_PLANE_TYPE_PRIMARY, IGT_ROTATION_180, 1 },
+ { DRM_PLANE_TYPE_PRIMARY, IGT_ROTATION_270, 1 },
+ { DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_90, 0 },
+ { DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_180, 0 },
+ { DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_270, 0 },
+ { DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_90, 1 },
+ { DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_180, 1 },
+ { DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_270, 1 },
+ { DRM_PLANE_TYPE_CURSOR, IGT_ROTATION_180, 0 },
+ { 0, 0, 0}
+ };
data_t data = {};
int gen = 0;
@@ -586,43 +649,19 @@ igt_main
igt_display_init(&data.display, data.gfx_fd);
}
- igt_subtest_f("primary-rotation-180") {
- data.rotation = IGT_ROTATION_180;
- test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY);
- }
-
- igt_subtest_f("sprite-rotation-180") {
- data.rotation = IGT_ROTATION_180;
- test_plane_rotation(&data, DRM_PLANE_TYPE_OVERLAY);
- }
-
- igt_subtest_f("cursor-rotation-180") {
- data.rotation = IGT_ROTATION_180;
- test_plane_rotation(&data, DRM_PLANE_TYPE_CURSOR);
- }
-
- igt_subtest_f("primary-rotation-90") {
- igt_require(gen >= 9);
- data.rotation = IGT_ROTATION_90;
- test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY);
- }
- igt_subtest_f("primary-rotation-270") {
- igt_require(gen >= 9);
- data.rotation = IGT_ROTATION_270;
- test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY);
- }
-
- igt_subtest_f("sprite-rotation-90") {
- igt_require(gen >= 9);
- data.rotation = IGT_ROTATION_90;
- test_plane_rotation(&data, DRM_PLANE_TYPE_OVERLAY);
- }
-
- igt_subtest_f("sprite-rotation-270") {
- igt_require(gen >= 9);
- data.rotation = IGT_ROTATION_270;
- test_plane_rotation(&data, DRM_PLANE_TYPE_OVERLAY);
+ for (subtest = subtests; subtest->rot; subtest++) {
+ igt_subtest_f("%s-rotation-%s%s",
+ plane_test_str(subtest->plane),
+ rot_test_str(subtest->rot),
+ flip_test_str(subtest->flips)) {
+ igt_require(!(subtest->rot &
+ (IGT_ROTATION_90 | IGT_ROTATION_270)) ||
+ gen >= 9);
+ data.rotation = subtest->rot;
+ data.flips = subtest->flips;
+ test_plane_rotation(&data, subtest->plane);
+ }
}
igt_subtest_f("sprite-rotation-90-pos-100-0") {
@@ -650,14 +689,6 @@ igt_main
test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY);
}
- igt_subtest_f("primary-rotation-90-flip-stress") {
- igt_require(gen >= 9);
- data.override_tiling = 0;
- data.flip_stress = 60;
- data.rotation = IGT_ROTATION_90;
- test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY);
- }
-
igt_subtest_f("primary-rotation-90-Y-tiled") {
enum pipe pipe;
igt_output_t *output;
--
2.9.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH i-g-t v3] kms_rotation_crc: 90 degree flip test is not a stress test
2017-09-08 11:10 [PATCH i-g-t v2] kms_rotation_crc: 90 degree flip test is not a stress test Tvrtko Ursulin
@ 2017-09-08 11:24 ` Tvrtko Ursulin
2017-09-08 14:06 ` Chris Wilson
2017-09-08 11:45 ` ✓ Fi.CI.BAT: success for kms_rotation_crc: 90 degree flip test is not a stress test (rev4) Patchwork
` (3 subsequent siblings)
4 siblings, 1 reply; 18+ messages in thread
From: Tvrtko Ursulin @ 2017-09-08 11:24 UTC (permalink / raw)
To: Intel-gfx; +Cc: Daniel Vetter
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
To the best of my recollection the page flipping test was added
simply to start exercising page flips with 90/270 rotation.
There is no need to do 60 flips which can take quite some time,
because we do 60 flips against each pipe and each fb geometry.
Also, calling this a stress test is also not matching the
original idea of the test.
v2:
Several changes:
1. Remove the stress from the name and reduce the number of
flips to one only.
2. Move the page flip before CRC collection for a more useful
test.
3. Add more flipping tests, for different rotation and sprite
planes.
4. Convert to table driven subtest generation.
v3: Remove extended.testlist from the patch.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Katarzyna Dec <katarzyna.dec@intel.com>
---
tests/kms_rotation_crc.c | 137 +++++++++++++++++++++++++++++------------------
1 file changed, 84 insertions(+), 53 deletions(-)
diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index 83e37f126f40..20f1adb67769 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -41,7 +41,7 @@ typedef struct {
int pos_y;
uint32_t override_fmt;
uint64_t override_tiling;
- unsigned int flip_stress;
+ unsigned int flips;
} data_t;
static void
@@ -219,7 +219,7 @@ static void prepare_fbs(data_t *data, igt_output_t *output,
igt_create_fb(data->gfx_fd, w, h, pixel_format, tiling, &data->fb);
- if (data->flip_stress) {
+ if (data->flips) {
igt_create_fb(data->gfx_fd, w, h, pixel_format, tiling, &data->fb_flip);
paint_squares(data, IGT_ROTATION_0, &data->fb_flip, 0.92);
}
@@ -351,15 +351,17 @@ static void test_plane_rotation(data_t *data, int plane_type)
ret = igt_display_try_commit2(display, commit);
if (data->override_fmt || data->override_tiling) {
igt_assert_eq(ret, -EINVAL);
- } else {
- igt_assert_eq(ret, 0);
- igt_pipe_crc_collect_crc(data->pipe_crc,
- &crc_output);
- igt_assert_crc_equal(&data->ref_crc,
- &crc_output);
+ continue;
}
- flip_count = data->flip_stress;
+ /* Verify commit was ok. */
+ igt_assert_eq(ret, 0);
+
+ /*
+ * If flips are requested flip away and back before
+ * checking CRC.
+ */
+ flip_count = data->flips;
while (flip_count--) {
ret = drmModePageFlip(data->gfx_fd,
output->config.crtc->crtc_id,
@@ -376,6 +378,9 @@ static void test_plane_rotation(data_t *data, int plane_type)
igt_assert_eq(ret, 0);
wait_for_pageflip(data->gfx_fd);
}
+
+ igt_pipe_crc_collect_crc(data->pipe_crc, &crc_output);
+ igt_assert_crc_equal(&data->ref_crc, &crc_output);
}
valid_tests++;
@@ -569,8 +574,66 @@ err_commit:
igt_assert_eq(ret, 0);
}
+static const char *plane_test_str(unsigned plane)
+{
+ switch (plane) {
+ case DRM_PLANE_TYPE_PRIMARY:
+ return "primary";
+ case DRM_PLANE_TYPE_OVERLAY:
+ return "sprite";
+ case DRM_PLANE_TYPE_CURSOR:
+ return "cursor";
+ default:
+ igt_assert(0);
+ }
+}
+
+static const char *rot_test_str(igt_rotation_t rot)
+{
+ switch (rot) {
+ case IGT_ROTATION_90:
+ return "90";
+ case IGT_ROTATION_180:
+ return "180";
+ case IGT_ROTATION_270:
+ return "270";
+ default:
+ igt_assert(0);
+ }
+}
+
+static const char *flip_test_str(unsigned flips)
+{
+ if (flips > 1)
+ return "-flip-stress";
+ else if (flips)
+ return "-flip";
+ else
+ return "";
+}
+
igt_main
{
+ struct rot_subtest {
+ unsigned plane;
+ igt_rotation_t rot;
+ unsigned flips;
+ } *subtest, subtests[] = {
+ { DRM_PLANE_TYPE_PRIMARY, IGT_ROTATION_90, 0 },
+ { DRM_PLANE_TYPE_PRIMARY, IGT_ROTATION_180, 0 },
+ { DRM_PLANE_TYPE_PRIMARY, IGT_ROTATION_270, 0 },
+ { DRM_PLANE_TYPE_PRIMARY, IGT_ROTATION_90, 1 },
+ { DRM_PLANE_TYPE_PRIMARY, IGT_ROTATION_180, 1 },
+ { DRM_PLANE_TYPE_PRIMARY, IGT_ROTATION_270, 1 },
+ { DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_90, 0 },
+ { DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_180, 0 },
+ { DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_270, 0 },
+ { DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_90, 1 },
+ { DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_180, 1 },
+ { DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_270, 1 },
+ { DRM_PLANE_TYPE_CURSOR, IGT_ROTATION_180, 0 },
+ { 0, 0, 0}
+ };
data_t data = {};
int gen = 0;
@@ -586,43 +649,19 @@ igt_main
igt_display_init(&data.display, data.gfx_fd);
}
- igt_subtest_f("primary-rotation-180") {
- data.rotation = IGT_ROTATION_180;
- test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY);
- }
-
- igt_subtest_f("sprite-rotation-180") {
- data.rotation = IGT_ROTATION_180;
- test_plane_rotation(&data, DRM_PLANE_TYPE_OVERLAY);
- }
-
- igt_subtest_f("cursor-rotation-180") {
- data.rotation = IGT_ROTATION_180;
- test_plane_rotation(&data, DRM_PLANE_TYPE_CURSOR);
- }
-
- igt_subtest_f("primary-rotation-90") {
- igt_require(gen >= 9);
- data.rotation = IGT_ROTATION_90;
- test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY);
- }
- igt_subtest_f("primary-rotation-270") {
- igt_require(gen >= 9);
- data.rotation = IGT_ROTATION_270;
- test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY);
- }
-
- igt_subtest_f("sprite-rotation-90") {
- igt_require(gen >= 9);
- data.rotation = IGT_ROTATION_90;
- test_plane_rotation(&data, DRM_PLANE_TYPE_OVERLAY);
- }
-
- igt_subtest_f("sprite-rotation-270") {
- igt_require(gen >= 9);
- data.rotation = IGT_ROTATION_270;
- test_plane_rotation(&data, DRM_PLANE_TYPE_OVERLAY);
+ for (subtest = subtests; subtest->rot; subtest++) {
+ igt_subtest_f("%s-rotation-%s%s",
+ plane_test_str(subtest->plane),
+ rot_test_str(subtest->rot),
+ flip_test_str(subtest->flips)) {
+ igt_require(!(subtest->rot &
+ (IGT_ROTATION_90 | IGT_ROTATION_270)) ||
+ gen >= 9);
+ data.rotation = subtest->rot;
+ data.flips = subtest->flips;
+ test_plane_rotation(&data, subtest->plane);
+ }
}
igt_subtest_f("sprite-rotation-90-pos-100-0") {
@@ -650,14 +689,6 @@ igt_main
test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY);
}
- igt_subtest_f("primary-rotation-90-flip-stress") {
- igt_require(gen >= 9);
- data.override_tiling = 0;
- data.flip_stress = 60;
- data.rotation = IGT_ROTATION_90;
- test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY);
- }
-
igt_subtest_f("primary-rotation-90-Y-tiled") {
enum pipe pipe;
igt_output_t *output;
--
2.9.5
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 18+ messages in thread
* ✓ Fi.CI.BAT: success for kms_rotation_crc: 90 degree flip test is not a stress test (rev4)
2017-09-08 11:10 [PATCH i-g-t v2] kms_rotation_crc: 90 degree flip test is not a stress test Tvrtko Ursulin
2017-09-08 11:24 ` [PATCH i-g-t v3] " Tvrtko Ursulin
@ 2017-09-08 11:45 ` Patchwork
2017-09-08 13:58 ` ✗ Fi.CI.IGT: failure " Patchwork
` (2 subsequent siblings)
4 siblings, 0 replies; 18+ messages in thread
From: Patchwork @ 2017-09-08 11:45 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: intel-gfx
== Series Details ==
Series: kms_rotation_crc: 90 degree flip test is not a stress test (rev4)
URL : https://patchwork.freedesktop.org/series/28308/
State : success
== Summary ==
IGT patchset tested on top of latest successful build
d5de89eb17938484718be1144f5957f05ab44dc2 lib/dummyload: Use -1 for all engines
with latest DRM-Tip kernel build CI_DRM_3061
16fef66706a3 drm-tip: 2017y-09m-08d-08h-41m-49s UTC integration manifest
Test kms_cursor_legacy:
Subgroup basic-busy-flip-before-cursor-legacy:
pass -> FAIL (fi-snb-2600) fdo#100215
fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215
fi-bdw-5557u total:289 pass:268 dwarn:0 dfail:0 fail:0 skip:21 time:460s
fi-bdw-gvtdvm total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:445s
fi-blb-e6850 total:289 pass:224 dwarn:1 dfail:0 fail:0 skip:64 time:369s
fi-bsw-n3050 total:289 pass:243 dwarn:0 dfail:0 fail:0 skip:46 time:573s
fi-bwr-2160 total:289 pass:184 dwarn:0 dfail:0 fail:0 skip:105 time:255s
fi-bxt-j4205 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:529s
fi-byt-j1900 total:289 pass:254 dwarn:1 dfail:0 fail:0 skip:34 time:528s
fi-byt-n2820 total:289 pass:250 dwarn:1 dfail:0 fail:0 skip:38 time:519s
fi-cfl-s total:289 pass:250 dwarn:4 dfail:0 fail:0 skip:35 time:469s
fi-elk-e7500 total:289 pass:230 dwarn:0 dfail:0 fail:0 skip:59 time:432s
fi-glk-2a total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:614s
fi-hsw-4770 total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:452s
fi-hsw-4770r total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:429s
fi-ilk-650 total:289 pass:229 dwarn:0 dfail:0 fail:0 skip:60 time:427s
fi-ivb-3520m total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:495s
fi-ivb-3770 total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:477s
fi-kbl-7500u total:289 pass:264 dwarn:1 dfail:0 fail:0 skip:24 time:515s
fi-kbl-7560u total:289 pass:270 dwarn:0 dfail:0 fail:0 skip:19 time:602s
fi-kbl-r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:600s
fi-pnv-d510 total:289 pass:223 dwarn:1 dfail:0 fail:0 skip:65 time:529s
fi-skl-6260u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:467s
fi-skl-6700k total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:535s
fi-skl-6770hq total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:516s
fi-skl-gvtdvm total:289 pass:266 dwarn:0 dfail:0 fail:0 skip:23 time:448s
fi-skl-x1585l total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:525s
fi-snb-2520m total:289 pass:251 dwarn:0 dfail:0 fail:0 skip:38 time:559s
fi-snb-2600 total:289 pass:249 dwarn:0 dfail:0 fail:1 skip:39 time:407s
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_166/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* ✗ Fi.CI.IGT: failure for kms_rotation_crc: 90 degree flip test is not a stress test (rev4)
2017-09-08 11:10 [PATCH i-g-t v2] kms_rotation_crc: 90 degree flip test is not a stress test Tvrtko Ursulin
2017-09-08 11:24 ` [PATCH i-g-t v3] " Tvrtko Ursulin
2017-09-08 11:45 ` ✓ Fi.CI.BAT: success for kms_rotation_crc: 90 degree flip test is not a stress test (rev4) Patchwork
@ 2017-09-08 13:58 ` Patchwork
2017-09-11 17:54 ` ✓ Fi.CI.BAT: success for kms_rotation_crc: 90 degree flip test is not a stress test (rev5) Patchwork
2017-09-11 21:59 ` ✗ Fi.CI.IGT: failure " Patchwork
4 siblings, 0 replies; 18+ messages in thread
From: Patchwork @ 2017-09-08 13:58 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: intel-gfx
== Series Details ==
Series: kms_rotation_crc: 90 degree flip test is not a stress test (rev4)
URL : https://patchwork.freedesktop.org/series/28308/
State : failure
== Summary ==
Test kms_flip:
Subgroup wf_vblank-ts-check-interruptible:
fail -> PASS (shard-hsw) fdo#100368
Subgroup wf_vblank-vs-modeset:
pass -> DMESG-WARN (shard-hsw)
Test gem_eio:
Subgroup in-flight:
pass -> FAIL (shard-hsw) fdo#102616
Test gem_exec_params:
Subgroup invalid-flag:
pass -> SKIP (shard-hsw)
Test gem_exec_basic:
Subgroup readonly-default:
pass -> SKIP (shard-hsw)
Test pm_rpm:
Subgroup gem-execbuf-stress:
pass -> FAIL (shard-hsw)
Test gem_partial_pwrite_pread:
Subgroup write:
pass -> SKIP (shard-hsw)
Test perf:
Subgroup polling:
fail -> PASS (shard-hsw) fdo#102252
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#102616 https://bugs.freedesktop.org/show_bug.cgi?id=102616
fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
shard-hsw total:2307 pass:1235 dwarn:1 dfail:0 fail:13 skip:1058 time:9525s
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_166/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH i-g-t v3] kms_rotation_crc: 90 degree flip test is not a stress test
2017-09-08 11:24 ` [PATCH i-g-t v3] " Tvrtko Ursulin
@ 2017-09-08 14:06 ` Chris Wilson
2017-09-08 14:54 ` Tvrtko Ursulin
0 siblings, 1 reply; 18+ messages in thread
From: Chris Wilson @ 2017-09-08 14:06 UTC (permalink / raw)
To: Tvrtko Ursulin, Intel-gfx; +Cc: Daniel Vetter
Quoting Tvrtko Ursulin (2017-09-08 12:24:07)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> To the best of my recollection the page flipping test was added
> simply to start exercising page flips with 90/270 rotation.
>
> There is no need to do 60 flips which can take quite some time,
> because we do 60 flips against each pipe and each fb geometry.
>
> Also, calling this a stress test is also not matching the
> original idea of the test.
Thanks for making it easy for me to follow! Sounds great.
>
> v2:
>
> Several changes:
>
> 1. Remove the stress from the name and reduce the number of
> flips to one only.
>
> 2. Move the page flip before CRC collection for a more useful
> test.
>
> 3. Add more flipping tests, for different rotation and sprite
> planes.
>
> 4. Convert to table driven subtest generation.
>
> v3: Remove extended.testlist from the patch.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Katarzyna Dec <katarzyna.dec@intel.com>
> ---
> tests/kms_rotation_crc.c | 137 +++++++++++++++++++++++++++++------------------
> 1 file changed, 84 insertions(+), 53 deletions(-)
>
> diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
> index 83e37f126f40..20f1adb67769 100644
> --- a/tests/kms_rotation_crc.c
> +++ b/tests/kms_rotation_crc.c
> @@ -41,7 +41,7 @@ typedef struct {
> int pos_y;
> uint32_t override_fmt;
> uint64_t override_tiling;
> - unsigned int flip_stress;
> + unsigned int flips;
> } data_t;
>
> static void
> @@ -219,7 +219,7 @@ static void prepare_fbs(data_t *data, igt_output_t *output,
>
> igt_create_fb(data->gfx_fd, w, h, pixel_format, tiling, &data->fb);
>
> - if (data->flip_stress) {
> + if (data->flips) {
> igt_create_fb(data->gfx_fd, w, h, pixel_format, tiling, &data->fb_flip);
> paint_squares(data, IGT_ROTATION_0, &data->fb_flip, 0.92);
> }
> @@ -351,15 +351,17 @@ static void test_plane_rotation(data_t *data, int plane_type)
> ret = igt_display_try_commit2(display, commit);
> if (data->override_fmt || data->override_tiling) {
> igt_assert_eq(ret, -EINVAL);
> - } else {
> - igt_assert_eq(ret, 0);
> - igt_pipe_crc_collect_crc(data->pipe_crc,
> - &crc_output);
> - igt_assert_crc_equal(&data->ref_crc,
> - &crc_output);
> + continue;
> }
>
> - flip_count = data->flip_stress;
> + /* Verify commit was ok. */
> + igt_assert_eq(ret, 0);
> +
> + /*
> + * If flips are requested flip away and back before
> + * checking CRC.
And back? We only check of the original framebuffer and not the rotated?
Or am I missing the point...
> + */
> + flip_count = data->flips;
> while (flip_count--) {
> ret = drmModePageFlip(data->gfx_fd,
> output->config.crtc->crtc_id,
> @@ -376,6 +378,9 @@ static void test_plane_rotation(data_t *data, int plane_type)
> igt_assert_eq(ret, 0);
> wait_for_pageflip(data->gfx_fd);
> }
> +
> + igt_pipe_crc_collect_crc(data->pipe_crc, &crc_output);
> + igt_assert_crc_equal(&data->ref_crc, &crc_output);
> }
>
> valid_tests++;
> @@ -569,8 +574,66 @@ err_commit:
> igt_assert_eq(ret, 0);
> }
Consolidation looks good, and the above changes make sense, but the
comment makes me wonder if there is another CRC check we could do.
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH i-g-t v3] kms_rotation_crc: 90 degree flip test is not a stress test
2017-09-08 14:06 ` Chris Wilson
@ 2017-09-08 14:54 ` Tvrtko Ursulin
2017-09-08 16:05 ` Chris Wilson
2017-09-11 17:14 ` [PATCH i-g-t v4] " Tvrtko Ursulin
0 siblings, 2 replies; 18+ messages in thread
From: Tvrtko Ursulin @ 2017-09-08 14:54 UTC (permalink / raw)
To: Chris Wilson, Tvrtko Ursulin, Intel-gfx; +Cc: Daniel Vetter
On 08/09/2017 15:06, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2017-09-08 12:24:07)
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> To the best of my recollection the page flipping test was added
>> simply to start exercising page flips with 90/270 rotation.
>>
>> There is no need to do 60 flips which can take quite some time,
>> because we do 60 flips against each pipe and each fb geometry.
>>
>> Also, calling this a stress test is also not matching the
>> original idea of the test.
>
> Thanks for making it easy for me to follow! Sounds great.
>
>>
>> v2:
>>
>> Several changes:
>>
>> 1. Remove the stress from the name and reduce the number of
>> flips to one only.
>>
>> 2. Move the page flip before CRC collection for a more useful
>> test.
>>
>> 3. Add more flipping tests, for different rotation and sprite
>> planes.
>>
>> 4. Convert to table driven subtest generation.
>>
>> v3: Remove extended.testlist from the patch.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Katarzyna Dec <katarzyna.dec@intel.com>
>> ---
>> tests/kms_rotation_crc.c | 137 +++++++++++++++++++++++++++++------------------
>> 1 file changed, 84 insertions(+), 53 deletions(-)
>>
>> diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
>> index 83e37f126f40..20f1adb67769 100644
>> --- a/tests/kms_rotation_crc.c
>> +++ b/tests/kms_rotation_crc.c
>> @@ -41,7 +41,7 @@ typedef struct {
>> int pos_y;
>> uint32_t override_fmt;
>> uint64_t override_tiling;
>> - unsigned int flip_stress;
>> + unsigned int flips;
>> } data_t;
>>
>> static void
>> @@ -219,7 +219,7 @@ static void prepare_fbs(data_t *data, igt_output_t *output,
>>
>> igt_create_fb(data->gfx_fd, w, h, pixel_format, tiling, &data->fb);
>>
>> - if (data->flip_stress) {
>> + if (data->flips) {
>> igt_create_fb(data->gfx_fd, w, h, pixel_format, tiling, &data->fb_flip);
>> paint_squares(data, IGT_ROTATION_0, &data->fb_flip, 0.92);
>> }
>> @@ -351,15 +351,17 @@ static void test_plane_rotation(data_t *data, int plane_type)
>> ret = igt_display_try_commit2(display, commit);
>> if (data->override_fmt || data->override_tiling) {
>> igt_assert_eq(ret, -EINVAL);
>> - } else {
>> - igt_assert_eq(ret, 0);
>> - igt_pipe_crc_collect_crc(data->pipe_crc,
>> - &crc_output);
>> - igt_assert_crc_equal(&data->ref_crc,
>> - &crc_output);
>> + continue;
>> }
>>
>> - flip_count = data->flip_stress;
>> + /* Verify commit was ok. */
>> + igt_assert_eq(ret, 0);
>> +
>> + /*
>> + * If flips are requested flip away and back before
>> + * checking CRC.
>
> And back? We only check of the original framebuffer and not the rotated?
> Or am I missing the point...
Yes, hm, it would be really bad if both flips silently did nothing, but
the last CRC would still match. So yes, still a holey hole there.
(Note we are not changing rotation with the flip here, just flipping
between two equally rotated fbs.)
I can see two options:
a) Stick a crc not equal assert in between the flips.
b) Collect a second CRC and check both.
I will have to asses the runtime impact of all the options. At the
moment test setup time is quite long. Is it CRC collection, or modeset..
>> + */
>> + flip_count = data->flips;
>> while (flip_count--) {
>> ret = drmModePageFlip(data->gfx_fd,
>> output->config.crtc->crtc_id,
>> @@ -376,6 +378,9 @@ static void test_plane_rotation(data_t *data, int plane_type)
>> igt_assert_eq(ret, 0);
>> wait_for_pageflip(data->gfx_fd);
>> }
>> +
>> + igt_pipe_crc_collect_crc(data->pipe_crc, &crc_output);
>> + igt_assert_crc_equal(&data->ref_crc, &crc_output);
>> }
>>
>> valid_tests++;
>> @@ -569,8 +574,66 @@ err_commit:
>> igt_assert_eq(ret, 0);
>> }
>
> Consolidation looks good, and the above changes make sense, but the
> comment makes me wonder if there is another CRC check we could do.
>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.
Thanks.
Regards,
Tvrtko
> -Chris
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH i-g-t v3] kms_rotation_crc: 90 degree flip test is not a stress test
2017-09-08 14:54 ` Tvrtko Ursulin
@ 2017-09-08 16:05 ` Chris Wilson
2017-09-11 17:14 ` [PATCH i-g-t v4] " Tvrtko Ursulin
1 sibling, 0 replies; 18+ messages in thread
From: Chris Wilson @ 2017-09-08 16:05 UTC (permalink / raw)
To: Tvrtko Ursulin, Tvrtko Ursulin, Intel-gfx; +Cc: Daniel Vetter
Quoting Tvrtko Ursulin (2017-09-08 15:54:18)
>
> On 08/09/2017 15:06, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2017-09-08 12:24:07)
> >> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >> + /*
> >> + * If flips are requested flip away and back before
> >> + * checking CRC.
> >
> > And back? We only check of the original framebuffer and not the rotated?
> > Or am I missing the point...
>
> Yes, hm, it would be really bad if both flips silently did nothing, but
> the last CRC would still match. So yes, still a holey hole there.
>
> (Note we are not changing rotation with the flip here, just flipping
> between two equally rotated fbs.)
Although with pageflips mapped through to a nonblocking modeset, there
shouldn't be any problems now with flipping to a new rotation, stride,
format, etc. I guess they are all checked somewhere, but I also guess
that somewhere is here ;)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH i-g-t v4] kms_rotation_crc: 90 degree flip test is not a stress test
2017-09-08 14:54 ` Tvrtko Ursulin
2017-09-08 16:05 ` Chris Wilson
@ 2017-09-11 17:14 ` Tvrtko Ursulin
2017-09-12 8:49 ` Katarzyna Dec
1 sibling, 1 reply; 18+ messages in thread
From: Tvrtko Ursulin @ 2017-09-11 17:14 UTC (permalink / raw)
To: Intel-gfx; +Cc: Daniel Vetter
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
To the best of my recollection the page flipping test was added
simply to start exercising page flips with 90/270 rotation.
There is no need to do 60 flips which can take quite some time,
because we do 60 flips against each pipe and each fb geometry.
Also, calling this a stress test is also not matching the
original idea of the test.
v2:
Several changes:
1. Remove the stress from the name and reduce the number of
flips to one only.
2. Move the page flip before CRC collection for a more useful
test.
3. Add more flipping tests, for different rotation and sprite
planes.
4. Convert to table driven subtest generation.
v3: Remove extended.testlist from the patch.
v4:
Collect a flip fb crc and check it after flip. This way we test
not only the flip succeeded but the right image is displayed.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Katarzyna Dec <katarzyna.dec@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> (v3)
---
tests/kms_rotation_crc.c | 194 +++++++++++++++++++++++++++++------------------
1 file changed, 122 insertions(+), 72 deletions(-)
diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index 83e37f126f40..21e264addc09 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -35,13 +35,14 @@ typedef struct {
struct igt_fb fb_modeset;
struct igt_fb fb_flip;
igt_crc_t ref_crc;
+ igt_crc_t flip_crc;
igt_pipe_crc_t *pipe_crc;
igt_rotation_t rotation;
int pos_x;
int pos_y;
uint32_t override_fmt;
uint64_t override_tiling;
- unsigned int flip_stress;
+ bool flips;
} data_t;
static void
@@ -163,6 +164,7 @@ static void prepare_fbs(data_t *data, igt_output_t *output,
unsigned int w, h, ref_w, ref_h, min_w, min_h;
uint64_t tiling = data->override_tiling ?: LOCAL_DRM_FORMAT_MOD_NONE;
uint32_t pixel_format = data->override_fmt ?: DRM_FORMAT_XRGB8888;
+ const float flip_opacity = 0.75;
if (data->fb.fb_id) {
igt_plane_set_fb(plane, NULL);
@@ -219,12 +221,28 @@ static void prepare_fbs(data_t *data, igt_output_t *output,
igt_create_fb(data->gfx_fd, w, h, pixel_format, tiling, &data->fb);
- if (data->flip_stress) {
- igt_create_fb(data->gfx_fd, w, h, pixel_format, tiling, &data->fb_flip);
- paint_squares(data, IGT_ROTATION_0, &data->fb_flip, 0.92);
+ igt_plane_set_rotation(plane, IGT_ROTATION_0);
+
+ /*
+ * Create a reference software rotated flip framebuffer.
+ */
+ if (data->flips) {
+ igt_create_fb(data->gfx_fd, ref_w, ref_h, pixel_format, tiling,
+ &data->fb_flip);
+ paint_squares(data, data->rotation, &data->fb_flip,
+ flip_opacity);
+ igt_plane_set_fb(plane, &data->fb_flip);
+ if (plane->type != DRM_PLANE_TYPE_CURSOR)
+ igt_plane_set_position(plane, data->pos_x, data->pos_y);
+ igt_display_commit2(display,
+ display->is_atomic ?
+ COMMIT_ATOMIC : COMMIT_UNIVERSAL);
+ igt_pipe_crc_collect_crc(data->pipe_crc, &data->flip_crc);
}
- /* Step 1: create a reference CRC for a software-rotated fb */
+ /*
+ * Create a reference CRC for a software-rotated fb.
+ */
igt_create_fb(data->gfx_fd, ref_w, ref_h, pixel_format,
data->override_tiling ?: LOCAL_DRM_FORMAT_MOD_NONE, &data->fb_reference);
paint_squares(data, data->rotation, &data->fb_reference, 1.0);
@@ -232,20 +250,30 @@ static void prepare_fbs(data_t *data, igt_output_t *output,
igt_plane_set_fb(plane, &data->fb_reference);
if (plane->type != DRM_PLANE_TYPE_CURSOR)
igt_plane_set_position(plane, data->pos_x, data->pos_y);
- igt_plane_set_rotation(plane, IGT_ROTATION_0);
igt_display_commit2(display, display->is_atomic ? COMMIT_ATOMIC : COMMIT_UNIVERSAL);
igt_pipe_crc_collect_crc(data->pipe_crc, &data->ref_crc);
/*
- * Step 2: prepare the plane with an non-rotated fb let the hw
- * rotate it.
+ * Prepare the plane with an non-rotated fb let the hw rotate it.
*/
paint_squares(data, IGT_ROTATION_0, &data->fb, 1.0);
igt_plane_set_fb(plane, &data->fb);
if (plane->type != DRM_PLANE_TYPE_CURSOR)
igt_plane_set_position(plane, data->pos_x, data->pos_y);
+
+ /*
+ * Prepare the non-rotated flip fb.
+ */
+ if (data->flips) {
+ igt_remove_fb(data->gfx_fd, &data->fb_flip);
+ igt_create_fb(data->gfx_fd, w, h, pixel_format, tiling,
+ &data->fb_flip);
+ paint_squares(data, IGT_ROTATION_0, &data->fb_flip,
+ flip_opacity);
+ }
+
}
static void cleanup_crtc(data_t *data, igt_output_t *output, igt_plane_t *plane)
@@ -300,9 +328,8 @@ static void test_plane_rotation(data_t *data, int plane_type)
igt_output_t *output;
enum pipe pipe;
int valid_tests = 0;
- igt_crc_t crc_output, crc_unrotated;
+ igt_crc_t crc_output;
enum igt_commit_style commit = COMMIT_LEGACY;
- unsigned int flip_count;
int ret;
if (plane_type == DRM_PLANE_TYPE_PRIMARY || plane_type == DRM_PLANE_TYPE_CURSOR)
@@ -341,9 +368,6 @@ static void test_plane_rotation(data_t *data, int plane_type)
igt_display_commit2(display, commit);
- /* collect unrotated CRC */
- igt_pipe_crc_collect_crc(data->pipe_crc, &crc_unrotated);
-
igt_plane_set_rotation(plane, data->rotation);
if (data->rotation == IGT_ROTATION_90 || data->rotation == IGT_ROTATION_270)
igt_plane_set_size(plane, data->fb.height, data->fb.width);
@@ -351,16 +375,21 @@ static void test_plane_rotation(data_t *data, int plane_type)
ret = igt_display_try_commit2(display, commit);
if (data->override_fmt || data->override_tiling) {
igt_assert_eq(ret, -EINVAL);
- } else {
- igt_assert_eq(ret, 0);
- igt_pipe_crc_collect_crc(data->pipe_crc,
- &crc_output);
- igt_assert_crc_equal(&data->ref_crc,
- &crc_output);
+ continue;
}
- flip_count = data->flip_stress;
- while (flip_count--) {
+ /* Verify commit was ok. */
+ igt_assert_eq(ret, 0);
+
+ /* Check CRC */
+ igt_pipe_crc_collect_crc(data->pipe_crc, &crc_output);
+ igt_assert_crc_equal(&data->ref_crc, &crc_output);
+
+ /*
+ * If flips are requested flip to a different fb and
+ * check CRC against that one as well.
+ */
+ if (data->flips) {
ret = drmModePageFlip(data->gfx_fd,
output->config.crtc->crtc_id,
data->fb_flip.fb_id,
@@ -368,13 +397,10 @@ static void test_plane_rotation(data_t *data, int plane_type)
NULL);
igt_assert_eq(ret, 0);
wait_for_pageflip(data->gfx_fd);
- ret = drmModePageFlip(data->gfx_fd,
- output->config.crtc->crtc_id,
- data->fb.fb_id,
- DRM_MODE_PAGE_FLIP_EVENT,
- NULL);
- igt_assert_eq(ret, 0);
- wait_for_pageflip(data->gfx_fd);
+ igt_pipe_crc_collect_crc(data->pipe_crc,
+ &crc_output);
+ igt_assert_crc_equal(&data->flip_crc,
+ &crc_output);
}
}
@@ -569,8 +595,64 @@ err_commit:
igt_assert_eq(ret, 0);
}
+static const char *plane_test_str(unsigned plane)
+{
+ switch (plane) {
+ case DRM_PLANE_TYPE_PRIMARY:
+ return "primary";
+ case DRM_PLANE_TYPE_OVERLAY:
+ return "sprite";
+ case DRM_PLANE_TYPE_CURSOR:
+ return "cursor";
+ default:
+ igt_assert(0);
+ }
+}
+
+static const char *rot_test_str(igt_rotation_t rot)
+{
+ switch (rot) {
+ case IGT_ROTATION_90:
+ return "90";
+ case IGT_ROTATION_180:
+ return "180";
+ case IGT_ROTATION_270:
+ return "270";
+ default:
+ igt_assert(0);
+ }
+}
+
+static const char *flip_test_str(unsigned flips)
+{
+ if (flips)
+ return "-flip";
+ else
+ return "";
+}
+
igt_main
{
+ struct rot_subtest {
+ unsigned plane;
+ igt_rotation_t rot;
+ unsigned flips;
+ } *subtest, subtests[] = {
+ { DRM_PLANE_TYPE_PRIMARY, IGT_ROTATION_90, 0 },
+ { DRM_PLANE_TYPE_PRIMARY, IGT_ROTATION_180, 0 },
+ { DRM_PLANE_TYPE_PRIMARY, IGT_ROTATION_270, 0 },
+ { DRM_PLANE_TYPE_PRIMARY, IGT_ROTATION_90, 1 },
+ { DRM_PLANE_TYPE_PRIMARY, IGT_ROTATION_180, 1 },
+ { DRM_PLANE_TYPE_PRIMARY, IGT_ROTATION_270, 1 },
+ { DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_90, 0 },
+ { DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_180, 0 },
+ { DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_270, 0 },
+ { DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_90, 1 },
+ { DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_180, 1 },
+ { DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_270, 1 },
+ { DRM_PLANE_TYPE_CURSOR, IGT_ROTATION_180, 0 },
+ { 0, 0, 0}
+ };
data_t data = {};
int gen = 0;
@@ -586,43 +668,19 @@ igt_main
igt_display_init(&data.display, data.gfx_fd);
}
- igt_subtest_f("primary-rotation-180") {
- data.rotation = IGT_ROTATION_180;
- test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY);
- }
-
- igt_subtest_f("sprite-rotation-180") {
- data.rotation = IGT_ROTATION_180;
- test_plane_rotation(&data, DRM_PLANE_TYPE_OVERLAY);
- }
-
- igt_subtest_f("cursor-rotation-180") {
- data.rotation = IGT_ROTATION_180;
- test_plane_rotation(&data, DRM_PLANE_TYPE_CURSOR);
- }
-
- igt_subtest_f("primary-rotation-90") {
- igt_require(gen >= 9);
- data.rotation = IGT_ROTATION_90;
- test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY);
- }
- igt_subtest_f("primary-rotation-270") {
- igt_require(gen >= 9);
- data.rotation = IGT_ROTATION_270;
- test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY);
- }
-
- igt_subtest_f("sprite-rotation-90") {
- igt_require(gen >= 9);
- data.rotation = IGT_ROTATION_90;
- test_plane_rotation(&data, DRM_PLANE_TYPE_OVERLAY);
- }
-
- igt_subtest_f("sprite-rotation-270") {
- igt_require(gen >= 9);
- data.rotation = IGT_ROTATION_270;
- test_plane_rotation(&data, DRM_PLANE_TYPE_OVERLAY);
+ for (subtest = subtests; subtest->rot; subtest++) {
+ igt_subtest_f("%s-rotation-%s%s",
+ plane_test_str(subtest->plane),
+ rot_test_str(subtest->rot),
+ flip_test_str(subtest->flips)) {
+ igt_require(!(subtest->rot &
+ (IGT_ROTATION_90 | IGT_ROTATION_270)) ||
+ gen >= 9);
+ data.rotation = subtest->rot;
+ data.flips = subtest->flips;
+ test_plane_rotation(&data, subtest->plane);
+ }
}
igt_subtest_f("sprite-rotation-90-pos-100-0") {
@@ -650,14 +708,6 @@ igt_main
test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY);
}
- igt_subtest_f("primary-rotation-90-flip-stress") {
- igt_require(gen >= 9);
- data.override_tiling = 0;
- data.flip_stress = 60;
- data.rotation = IGT_ROTATION_90;
- test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY);
- }
-
igt_subtest_f("primary-rotation-90-Y-tiled") {
enum pipe pipe;
igt_output_t *output;
--
2.9.5
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 18+ messages in thread
* ✓ Fi.CI.BAT: success for kms_rotation_crc: 90 degree flip test is not a stress test (rev5)
2017-09-08 11:10 [PATCH i-g-t v2] kms_rotation_crc: 90 degree flip test is not a stress test Tvrtko Ursulin
` (2 preceding siblings ...)
2017-09-08 13:58 ` ✗ Fi.CI.IGT: failure " Patchwork
@ 2017-09-11 17:54 ` Patchwork
2017-09-11 21:59 ` ✗ Fi.CI.IGT: failure " Patchwork
4 siblings, 0 replies; 18+ messages in thread
From: Patchwork @ 2017-09-11 17:54 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: intel-gfx
== Series Details ==
Series: kms_rotation_crc: 90 degree flip test is not a stress test (rev5)
URL : https://patchwork.freedesktop.org/series/28308/
State : success
== Summary ==
IGT patchset tested on top of latest successful build
f9e0154630766b63617c64255a68e5129e233a4b igt/gem_evict_(alignment,everything): Limit to low 4G
with latest DRM-Tip kernel build CI_DRM_3072
31d271f5a5d0 drm-tip: 2017y-09m-11d-17h-13m-48s UTC integration manifest
fi-bdw-5557u total:289 pass:268 dwarn:0 dfail:0 fail:0 skip:21 time:449s
fi-bdw-gvtdvm total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:455s
fi-blb-e6850 total:289 pass:224 dwarn:1 dfail:0 fail:0 skip:64 time:379s
fi-bsw-n3050 total:289 pass:243 dwarn:0 dfail:0 fail:0 skip:46 time:542s
fi-bwr-2160 total:289 pass:184 dwarn:0 dfail:0 fail:0 skip:105 time:271s
fi-bxt-j4205 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:511s
fi-byt-j1900 total:289 pass:254 dwarn:1 dfail:0 fail:0 skip:34 time:510s
fi-byt-n2820 total:289 pass:250 dwarn:1 dfail:0 fail:0 skip:38 time:510s
fi-cfl-s total:289 pass:250 dwarn:4 dfail:0 fail:0 skip:35 time:450s
fi-elk-e7500 total:289 pass:230 dwarn:0 dfail:0 fail:0 skip:59 time:462s
fi-glk-2a total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:598s
fi-hsw-4770 total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:430s
fi-hsw-4770r total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:405s
fi-ilk-650 total:289 pass:229 dwarn:0 dfail:0 fail:0 skip:60 time:443s
fi-ivb-3520m total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:498s
fi-ivb-3770 total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:476s
fi-kbl-7500u total:289 pass:263 dwarn:1 dfail:0 fail:1 skip:24 time:482s
fi-kbl-7560u total:289 pass:270 dwarn:0 dfail:0 fail:0 skip:19 time:581s
fi-kbl-r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:591s
fi-pnv-d510 total:289 pass:223 dwarn:1 dfail:0 fail:0 skip:65 time:554s
fi-skl-6260u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:462s
fi-skl-6700k total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:522s
fi-skl-6770hq total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:509s
fi-skl-gvtdvm total:289 pass:266 dwarn:0 dfail:0 fail:0 skip:23 time:468s
fi-skl-x1585l total:289 pass:268 dwarn:0 dfail:0 fail:0 skip:21 time:493s
fi-snb-2520m total:289 pass:251 dwarn:0 dfail:0 fail:0 skip:38 time:582s
fi-snb-2600 total:289 pass:249 dwarn:0 dfail:0 fail:1 skip:39 time:434s
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_171/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* ✗ Fi.CI.IGT: failure for kms_rotation_crc: 90 degree flip test is not a stress test (rev5)
2017-09-08 11:10 [PATCH i-g-t v2] kms_rotation_crc: 90 degree flip test is not a stress test Tvrtko Ursulin
` (3 preceding siblings ...)
2017-09-11 17:54 ` ✓ Fi.CI.BAT: success for kms_rotation_crc: 90 degree flip test is not a stress test (rev5) Patchwork
@ 2017-09-11 21:59 ` Patchwork
4 siblings, 0 replies; 18+ messages in thread
From: Patchwork @ 2017-09-11 21:59 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: intel-gfx
== Series Details ==
Series: kms_rotation_crc: 90 degree flip test is not a stress test (rev5)
URL : https://patchwork.freedesktop.org/series/28308/
State : failure
== Summary ==
Test gem_exec_capture:
Subgroup capture-render:
pass -> INCOMPLETE (shard-hsw)
Test gem_eio:
Subgroup in-flight:
pass -> FAIL (shard-hsw) fdo#102616
fdo#102616 https://bugs.freedesktop.org/show_bug.cgi?id=102616
shard-hsw total:2276 pass:1217 dwarn:0 dfail:0 fail:15 skip:1043 time:9165s
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_171/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH i-g-t v4] kms_rotation_crc: 90 degree flip test is not a stress test
2017-09-11 17:14 ` [PATCH i-g-t v4] " Tvrtko Ursulin
@ 2017-09-12 8:49 ` Katarzyna Dec
0 siblings, 0 replies; 18+ messages in thread
From: Katarzyna Dec @ 2017-09-12 8:49 UTC (permalink / raw)
To: Tvrtko Ursulin, Intel-gfx; +Cc: Daniel Vetter
On Mon, Sep 11, 2017 at 06:14:57PM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> To the best of my recollection the page flipping test was added
> simply to start exercising page flips with 90/270 rotation.
>
> There is no need to do 60 flips which can take quite some time,
> because we do 60 flips against each pipe and each fb geometry.
>
> Also, calling this a stress test is also not matching the
> original idea of the test.
>
> v2:
>
> Several changes:
>
> 1. Remove the stress from the name and reduce the number of
> flips to one only.
>
> 2. Move the page flip before CRC collection for a more useful
> test.
>
> 3. Add more flipping tests, for different rotation and sprite
> planes.
>
> 4. Convert to table driven subtest generation.
>
> v3: Remove extended.testlist from the patch.
>
> v4:
>
> Collect a flip fb crc and check it after flip. This way we test
> not only the flip succeeded but the right image is displayed.
For now it looks like sprite-rotation-X-flip tests are failing
by this assert.
Otherwise:
Reviewed-by: Katarzyna Dec <katarzyna.dec@intel.com>
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Katarzyna Dec <katarzyna.dec@intel.com>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> (v3)
> ---
> tests/kms_rotation_crc.c | 194 +++++++++++++++++++++++++++++------------------
> 1 file changed, 122 insertions(+), 72 deletions(-)
>
> diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
> index 83e37f126f40..21e264addc09 100644
> --- a/tests/kms_rotation_crc.c
> +++ b/tests/kms_rotation_crc.c
> @@ -35,13 +35,14 @@ typedef struct {
> struct igt_fb fb_modeset;
> struct igt_fb fb_flip;
> igt_crc_t ref_crc;
> + igt_crc_t flip_crc;
> igt_pipe_crc_t *pipe_crc;
> igt_rotation_t rotation;
> int pos_x;
> int pos_y;
> uint32_t override_fmt;
> uint64_t override_tiling;
> - unsigned int flip_stress;
> + bool flips;
> } data_t;
>
> static void
> @@ -163,6 +164,7 @@ static void prepare_fbs(data_t *data, igt_output_t *output,
> unsigned int w, h, ref_w, ref_h, min_w, min_h;
> uint64_t tiling = data->override_tiling ?: LOCAL_DRM_FORMAT_MOD_NONE;
> uint32_t pixel_format = data->override_fmt ?: DRM_FORMAT_XRGB8888;
> + const float flip_opacity = 0.75;
>
> if (data->fb.fb_id) {
> igt_plane_set_fb(plane, NULL);
> @@ -219,12 +221,28 @@ static void prepare_fbs(data_t *data, igt_output_t *output,
>
> igt_create_fb(data->gfx_fd, w, h, pixel_format, tiling, &data->fb);
>
> - if (data->flip_stress) {
> - igt_create_fb(data->gfx_fd, w, h, pixel_format, tiling, &data->fb_flip);
> - paint_squares(data, IGT_ROTATION_0, &data->fb_flip, 0.92);
> + igt_plane_set_rotation(plane, IGT_ROTATION_0);
> +
> + /*
> + * Create a reference software rotated flip framebuffer.
> + */
> + if (data->flips) {
> + igt_create_fb(data->gfx_fd, ref_w, ref_h, pixel_format, tiling,
> + &data->fb_flip);
> + paint_squares(data, data->rotation, &data->fb_flip,
> + flip_opacity);
> + igt_plane_set_fb(plane, &data->fb_flip);
> + if (plane->type != DRM_PLANE_TYPE_CURSOR)
> + igt_plane_set_position(plane, data->pos_x, data->pos_y);
> + igt_display_commit2(display,
> + display->is_atomic ?
> + COMMIT_ATOMIC : COMMIT_UNIVERSAL);
> + igt_pipe_crc_collect_crc(data->pipe_crc, &data->flip_crc);
> }
>
> - /* Step 1: create a reference CRC for a software-rotated fb */
> + /*
> + * Create a reference CRC for a software-rotated fb.
> + */
> igt_create_fb(data->gfx_fd, ref_w, ref_h, pixel_format,
> data->override_tiling ?: LOCAL_DRM_FORMAT_MOD_NONE, &data->fb_reference);
> paint_squares(data, data->rotation, &data->fb_reference, 1.0);
> @@ -232,20 +250,30 @@ static void prepare_fbs(data_t *data, igt_output_t *output,
> igt_plane_set_fb(plane, &data->fb_reference);
> if (plane->type != DRM_PLANE_TYPE_CURSOR)
> igt_plane_set_position(plane, data->pos_x, data->pos_y);
> - igt_plane_set_rotation(plane, IGT_ROTATION_0);
> igt_display_commit2(display, display->is_atomic ? COMMIT_ATOMIC : COMMIT_UNIVERSAL);
>
> igt_pipe_crc_collect_crc(data->pipe_crc, &data->ref_crc);
>
> /*
> - * Step 2: prepare the plane with an non-rotated fb let the hw
> - * rotate it.
> + * Prepare the plane with an non-rotated fb let the hw rotate it.
> */
> paint_squares(data, IGT_ROTATION_0, &data->fb, 1.0);
> igt_plane_set_fb(plane, &data->fb);
>
> if (plane->type != DRM_PLANE_TYPE_CURSOR)
> igt_plane_set_position(plane, data->pos_x, data->pos_y);
> +
> + /*
> + * Prepare the non-rotated flip fb.
> + */
> + if (data->flips) {
> + igt_remove_fb(data->gfx_fd, &data->fb_flip);
> + igt_create_fb(data->gfx_fd, w, h, pixel_format, tiling,
> + &data->fb_flip);
> + paint_squares(data, IGT_ROTATION_0, &data->fb_flip,
> + flip_opacity);
> + }
> +
> }
>
> static void cleanup_crtc(data_t *data, igt_output_t *output, igt_plane_t *plane)
> @@ -300,9 +328,8 @@ static void test_plane_rotation(data_t *data, int plane_type)
> igt_output_t *output;
> enum pipe pipe;
> int valid_tests = 0;
> - igt_crc_t crc_output, crc_unrotated;
> + igt_crc_t crc_output;
> enum igt_commit_style commit = COMMIT_LEGACY;
> - unsigned int flip_count;
> int ret;
>
> if (plane_type == DRM_PLANE_TYPE_PRIMARY || plane_type == DRM_PLANE_TYPE_CURSOR)
> @@ -341,9 +368,6 @@ static void test_plane_rotation(data_t *data, int plane_type)
>
> igt_display_commit2(display, commit);
>
> - /* collect unrotated CRC */
> - igt_pipe_crc_collect_crc(data->pipe_crc, &crc_unrotated);
> -
> igt_plane_set_rotation(plane, data->rotation);
> if (data->rotation == IGT_ROTATION_90 || data->rotation == IGT_ROTATION_270)
> igt_plane_set_size(plane, data->fb.height, data->fb.width);
> @@ -351,16 +375,21 @@ static void test_plane_rotation(data_t *data, int plane_type)
> ret = igt_display_try_commit2(display, commit);
> if (data->override_fmt || data->override_tiling) {
> igt_assert_eq(ret, -EINVAL);
> - } else {
> - igt_assert_eq(ret, 0);
> - igt_pipe_crc_collect_crc(data->pipe_crc,
> - &crc_output);
> - igt_assert_crc_equal(&data->ref_crc,
> - &crc_output);
> + continue;
> }
>
> - flip_count = data->flip_stress;
> - while (flip_count--) {
> + /* Verify commit was ok. */
> + igt_assert_eq(ret, 0);
> +
> + /* Check CRC */
> + igt_pipe_crc_collect_crc(data->pipe_crc, &crc_output);
> + igt_assert_crc_equal(&data->ref_crc, &crc_output);
> +
> + /*
> + * If flips are requested flip to a different fb and
> + * check CRC against that one as well.
> + */
> + if (data->flips) {
> ret = drmModePageFlip(data->gfx_fd,
> output->config.crtc->crtc_id,
> data->fb_flip.fb_id,
> @@ -368,13 +397,10 @@ static void test_plane_rotation(data_t *data, int plane_type)
> NULL);
> igt_assert_eq(ret, 0);
> wait_for_pageflip(data->gfx_fd);
> - ret = drmModePageFlip(data->gfx_fd,
> - output->config.crtc->crtc_id,
> - data->fb.fb_id,
> - DRM_MODE_PAGE_FLIP_EVENT,
> - NULL);
> - igt_assert_eq(ret, 0);
> - wait_for_pageflip(data->gfx_fd);
> + igt_pipe_crc_collect_crc(data->pipe_crc,
> + &crc_output);
> + igt_assert_crc_equal(&data->flip_crc,
> + &crc_output);
> }
> }
>
> @@ -569,8 +595,64 @@ err_commit:
> igt_assert_eq(ret, 0);
> }
>
> +static const char *plane_test_str(unsigned plane)
> +{
> + switch (plane) {
> + case DRM_PLANE_TYPE_PRIMARY:
> + return "primary";
> + case DRM_PLANE_TYPE_OVERLAY:
> + return "sprite";
> + case DRM_PLANE_TYPE_CURSOR:
> + return "cursor";
> + default:
> + igt_assert(0);
> + }
> +}
> +
> +static const char *rot_test_str(igt_rotation_t rot)
> +{
> + switch (rot) {
> + case IGT_ROTATION_90:
> + return "90";
> + case IGT_ROTATION_180:
> + return "180";
> + case IGT_ROTATION_270:
> + return "270";
> + default:
> + igt_assert(0);
> + }
> +}
> +
> +static const char *flip_test_str(unsigned flips)
> +{
> + if (flips)
> + return "-flip";
> + else
> + return "";
> +}
> +
> igt_main
> {
> + struct rot_subtest {
> + unsigned plane;
> + igt_rotation_t rot;
> + unsigned flips;
> + } *subtest, subtests[] = {
> + { DRM_PLANE_TYPE_PRIMARY, IGT_ROTATION_90, 0 },
> + { DRM_PLANE_TYPE_PRIMARY, IGT_ROTATION_180, 0 },
> + { DRM_PLANE_TYPE_PRIMARY, IGT_ROTATION_270, 0 },
> + { DRM_PLANE_TYPE_PRIMARY, IGT_ROTATION_90, 1 },
> + { DRM_PLANE_TYPE_PRIMARY, IGT_ROTATION_180, 1 },
> + { DRM_PLANE_TYPE_PRIMARY, IGT_ROTATION_270, 1 },
> + { DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_90, 0 },
> + { DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_180, 0 },
> + { DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_270, 0 },
> + { DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_90, 1 },
> + { DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_180, 1 },
> + { DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_270, 1 },
> + { DRM_PLANE_TYPE_CURSOR, IGT_ROTATION_180, 0 },
> + { 0, 0, 0}
> + };
> data_t data = {};
> int gen = 0;
>
> @@ -586,43 +668,19 @@ igt_main
>
> igt_display_init(&data.display, data.gfx_fd);
> }
> - igt_subtest_f("primary-rotation-180") {
> - data.rotation = IGT_ROTATION_180;
> - test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY);
> - }
> -
> - igt_subtest_f("sprite-rotation-180") {
> - data.rotation = IGT_ROTATION_180;
> - test_plane_rotation(&data, DRM_PLANE_TYPE_OVERLAY);
> - }
> -
> - igt_subtest_f("cursor-rotation-180") {
> - data.rotation = IGT_ROTATION_180;
> - test_plane_rotation(&data, DRM_PLANE_TYPE_CURSOR);
> - }
> -
> - igt_subtest_f("primary-rotation-90") {
> - igt_require(gen >= 9);
> - data.rotation = IGT_ROTATION_90;
> - test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY);
> - }
>
> - igt_subtest_f("primary-rotation-270") {
> - igt_require(gen >= 9);
> - data.rotation = IGT_ROTATION_270;
> - test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY);
> - }
> -
> - igt_subtest_f("sprite-rotation-90") {
> - igt_require(gen >= 9);
> - data.rotation = IGT_ROTATION_90;
> - test_plane_rotation(&data, DRM_PLANE_TYPE_OVERLAY);
> - }
> -
> - igt_subtest_f("sprite-rotation-270") {
> - igt_require(gen >= 9);
> - data.rotation = IGT_ROTATION_270;
> - test_plane_rotation(&data, DRM_PLANE_TYPE_OVERLAY);
> + for (subtest = subtests; subtest->rot; subtest++) {
> + igt_subtest_f("%s-rotation-%s%s",
> + plane_test_str(subtest->plane),
> + rot_test_str(subtest->rot),
> + flip_test_str(subtest->flips)) {
> + igt_require(!(subtest->rot &
> + (IGT_ROTATION_90 | IGT_ROTATION_270)) ||
> + gen >= 9);
> + data.rotation = subtest->rot;
> + data.flips = subtest->flips;
> + test_plane_rotation(&data, subtest->plane);
> + }
> }
>
> igt_subtest_f("sprite-rotation-90-pos-100-0") {
> @@ -650,14 +708,6 @@ igt_main
> test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY);
> }
>
> - igt_subtest_f("primary-rotation-90-flip-stress") {
> - igt_require(gen >= 9);
> - data.override_tiling = 0;
> - data.flip_stress = 60;
> - data.rotation = IGT_ROTATION_90;
> - test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY);
> - }
> -
> igt_subtest_f("primary-rotation-90-Y-tiled") {
> enum pipe pipe;
> igt_output_t *output;
> --
> 2.9.5
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH i-g-t v2] kms_rotation_crc: 90 degree flip test is not a stress test
[not found] ` <804db97c-0746-3796-01c0-8d0b390528f5@linux.intel.com>
@ 2017-09-07 15:07 ` Katarzyna Dec
0 siblings, 0 replies; 18+ messages in thread
From: Katarzyna Dec @ 2017-09-07 15:07 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: intel-gfx
On Thu, Sep 07, 2017 at 03:20:19PM +0100, Tvrtko Ursulin wrote:
>
> Hi,
>
> On 07/09/2017 15:13, Katarzyna Dec wrote:
> > On Mon, Sep 04, 2017 at 03:27:26PM +0100, Tvrtko Ursulin wrote:
> > >
> > > On 07/08/2017 16:53, Daniel Vetter wrote:
> > > > On Fri, Aug 04, 2017 at 09:43:41AM +0100, Tvrtko Ursulin wrote:
> > > > > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > > > >
> > > > > To the best of my recollection the page flipping test was added
> > > > > simply to start exercising page flips with 90/270 rotation.
> > > > >
> > > > > There is no need to do 60 flips which can take quite some time,
> > > > > because we do 60 flips against each pipe and each fb geometry.
> > > > >
> > > > > Also, calling this a stress test is also not matching the
> > > > > original idea of the test.
> > > > >
> > > > > Several changes:
> > > > >
> >
> > > > > 1. Remove the stress from the name and reduce the number of
> > > > > flips to one only.
> > > > >
> > > > > 2. Move the page flip before CRC collection for a more useful
> > > > > test.
> > > > >
> > > > > 3. Add more flipping tests, for different rotation and sprite
> > > > > planes.
> > > >
> > > > I assume you didn't make the test overall slower with this?
> > > >
> > > > > 4. Convert to table driven subtest generation.
> > > > >
> > > > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > > > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > >
> > > > Didn't do a full review, but sounds all reasonable. And I assume you've
> > > > tested this at least locally (the igt patchwork CI instance doesn't do
> > > > full runs ... yet).
> > >
> > > Yes I've ran it on SKL. It adds more subtests so the runtime is longer, but
> > > it also finds new bugs.
> > >
> > > Ideally someone with display-fu picks this up, fixes the bug(s) this exposes
> > > (or tells me the test is wrong), so we are not merging known failing tests?
> > > Or even that is OK?
> > >
> > > Tvrtko
> > >
> > > > Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > > > ---
> > > > > tests/intel-ci/extended.testlist | 2 +-
> > > > > tests/kms_rotation_crc.c | 137 ++++++++++++++++++++++++---------------
> > > > > 2 files changed, 85 insertions(+), 54 deletions(-)
> > > > >
> > > > > diff --git a/tests/intel-ci/extended.testlist b/tests/intel-ci/extended.testlist
> > > > > index 17eed013f810..fb71091758c6 100644
> > > > > --- a/tests/intel-ci/extended.testlist
> > > > > +++ b/tests/intel-ci/extended.testlist
> > > > > @@ -82,7 +82,7 @@ igt@gem_ringfill@vebox-bomb
> > > > > igt@gem_userptr_blits@stress-mm
> > > > > igt@gem_userptr_blits@stress-mm-invalidate-close
> > > > > igt@gem_userptr_blits@stress-mm-invalidate-close-overlap
> > > > > -igt@kms_rotation_crc@primary-rotation-90-flip-stress
> > > > > +igt@kms_rotation_crc@primary-rotation-90-flip
> > > > > igt@pm_rpm@gem-execbuf-stress
> > > > > igt@pm_rpm@gem-execbuf-stress-extra-wait
> > > > > igt@pm_rpm@gem-execbuf-stress-pc8
> > > > > diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
> > > > > index 83e37f126f40..20f1adb67769 100644
> > > > > --- a/tests/kms_rotation_crc.c
> > > > > +++ b/tests/kms_rotation_crc.c
> > > > > @@ -41,7 +41,7 @@ typedef struct {
> > > > > int pos_y;
> > > > > uint32_t override_fmt;
> > > > > uint64_t override_tiling;
> > > > > - unsigned int flip_stress;
> > > > > + unsigned int flips;
> > > > > } data_t;
> > > > > static void
> > > > > @@ -219,7 +219,7 @@ static void prepare_fbs(data_t *data, igt_output_t *output,
> > > > > igt_create_fb(data->gfx_fd, w, h, pixel_format, tiling, &data->fb);
> > > > > - if (data->flip_stress) {
> > > > > + if (data->flips) {
> > > > > igt_create_fb(data->gfx_fd, w, h, pixel_format, tiling, &data->fb_flip);
> > > > > paint_squares(data, IGT_ROTATION_0, &data->fb_flip, 0.92);
> > > > > }
> > > > > @@ -351,15 +351,17 @@ static void test_plane_rotation(data_t *data, int plane_type)
> > > > > ret = igt_display_try_commit2(display, commit);
> > > > > if (data->override_fmt || data->override_tiling) {
> > > > > igt_assert_eq(ret, -EINVAL);
> > Why do we need below changes? (just trying to understand during review)
>
> The move of the flip from after CRC collection to before? I think it makes
> the subtests which flip test more useful so it actually verifies not only
> the flip ioctl succeeded but the expected image is on screen.
>
Thanks for the answer :)
> > > > > - } else {
> > > > > - igt_assert_eq(ret, 0);
> > > > > - igt_pipe_crc_collect_crc(data->pipe_crc,
> > > > > - &crc_output);
> > > > > - igt_assert_crc_equal(&data->ref_crc,
> > > > > - &crc_output);
> > > > > + continue;
> > > > > }
> > > > > - flip_count = data->flip_stress;
> > > > > + /* Verify commit was ok. */
> > > > > + igt_assert_eq(ret, 0);
> > > > > +
> > > > > + /*
> > > > > + * If flips are requested flip away and back before
> > > > > + * checking CRC.
> > > > > + */
> > > > > + flip_count = data->flips;
> > > > > while (flip_count--) {
> > > > > ret = drmModePageFlip(data->gfx_fd,
> > > > > output->config.crtc->crtc_id,
> > > > > @@ -376,6 +378,9 @@ static void test_plane_rotation(data_t *data, int plane_type)
> > > > > igt_assert_eq(ret, 0);
> > > > > wait_for_pageflip(data->gfx_fd);
> > > > > }
> > > > > +
> > > > > + igt_pipe_crc_collect_crc(data->pipe_crc, &crc_output);
> > > > > + igt_assert_crc_equal(&data->ref_crc, &crc_output);
> > > > > }
> > > > > valid_tests++;
> > > > > @@ -569,8 +574,66 @@ err_commit:
> > > > > igt_assert_eq(ret, 0);
> > > > > }
> > I ran all test on drm-tip kernel and flip subtests are failing.
> > Is it a bug or because of changes?
> > I ran on NUC with KBL.
>
> All flip subtests or just the sprite plane ones? I tested on SKL and there
> primary plane works, but the sprite plane seems broken.
>
I run it on drm-tip kernel on NUC with KBL. Only flip tests were faling.
Other passed.
> Regards,
>
> Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH i-g-t v2] kms_rotation_crc: 90 degree flip test is not a stress test
2017-09-04 14:43 ` Daniel Vetter
@ 2017-09-04 14:56 ` Tvrtko Ursulin
0 siblings, 0 replies; 18+ messages in thread
From: Tvrtko Ursulin @ 2017-09-04 14:56 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Daniel Vetter, Intel-gfx
On 04/09/2017 15:43, Daniel Vetter wrote:
> On Mon, Sep 04, 2017 at 03:36:56PM +0100, Tvrtko Ursulin wrote:
>>
>> On 04/09/2017 15:27, Tvrtko Ursulin wrote:
>>> On 07/08/2017 16:53, Daniel Vetter wrote:
>>>> On Fri, Aug 04, 2017 at 09:43:41AM +0100, Tvrtko Ursulin wrote:
>>>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>>
>>>>> To the best of my recollection the page flipping test was added
>>>>> simply to start exercising page flips with 90/270 rotation.
>>>>>
>>>>> There is no need to do 60 flips which can take quite some time,
>>>>> because we do 60 flips against each pipe and each fb geometry.
>>>>>
>>>>> Also, calling this a stress test is also not matching the
>>>>> original idea of the test.
>>>>>
>>>>> Several changes:
>>>>>
>>>>> 1. Remove the stress from the name and reduce the number of
>>>>> flips to one only.
>>>>>
>>>>> 2. Move the page flip before CRC collection for a more useful
>>>>> test.
>>>>>
>>>>> 3. Add more flipping tests, for different rotation and sprite
>>>>> planes.
>>>>
>>>> I assume you didn't make the test overall slower with this?
>>>>
>>>>> 4. Convert to table driven subtest generation.
>>>>>
>>>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>>>>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>>>>
>>>> Didn't do a full review, but sounds all reasonable. And I assume you've
>>>> tested this at least locally (the igt patchwork CI instance doesn't do
>>>> full runs ... yet).
>>>
>>> Yes I've ran it on SKL. It adds more subtests so the runtime is longer,
>>> but it also finds new bugs.
>>
>> Runtime is longer compared to v1 of this patch BTW, I wasn't clear. It is
>> roughly on par with the situation without this patch.
>>
>> But v2 adds a lot more subtests, three of which fail (flip on a rotated
>> sprite plane). So there is value in it I think even like that.
>
> Failing new subtests is ok imo, but pls do give a heads-up to CI folks
> before merging (but I think as long as it's failing stable, it shouldn't
> cause noise). Anything that takes out the machine or driver isn't ok, and
> needs to be fixed first.
>
> So sounds all good to me to go ahead.
I thought we said full review rules on IGT, no? So I won't be merging
this until someone can look at it in more detail.
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH i-g-t v2] kms_rotation_crc: 90 degree flip test is not a stress test
2017-09-04 14:36 ` Tvrtko Ursulin
@ 2017-09-04 14:43 ` Daniel Vetter
2017-09-04 14:56 ` Tvrtko Ursulin
0 siblings, 1 reply; 18+ messages in thread
From: Daniel Vetter @ 2017-09-04 14:43 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: Daniel Vetter, Intel-gfx
On Mon, Sep 04, 2017 at 03:36:56PM +0100, Tvrtko Ursulin wrote:
>
> On 04/09/2017 15:27, Tvrtko Ursulin wrote:
> > On 07/08/2017 16:53, Daniel Vetter wrote:
> > > On Fri, Aug 04, 2017 at 09:43:41AM +0100, Tvrtko Ursulin wrote:
> > > > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > > >
> > > > To the best of my recollection the page flipping test was added
> > > > simply to start exercising page flips with 90/270 rotation.
> > > >
> > > > There is no need to do 60 flips which can take quite some time,
> > > > because we do 60 flips against each pipe and each fb geometry.
> > > >
> > > > Also, calling this a stress test is also not matching the
> > > > original idea of the test.
> > > >
> > > > Several changes:
> > > >
> > > > 1. Remove the stress from the name and reduce the number of
> > > > flips to one only.
> > > >
> > > > 2. Move the page flip before CRC collection for a more useful
> > > > test.
> > > >
> > > > 3. Add more flipping tests, for different rotation and sprite
> > > > planes.
> > >
> > > I assume you didn't make the test overall slower with this?
> > >
> > > > 4. Convert to table driven subtest generation.
> > > >
> > > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > >
> > > Didn't do a full review, but sounds all reasonable. And I assume you've
> > > tested this at least locally (the igt patchwork CI instance doesn't do
> > > full runs ... yet).
> >
> > Yes I've ran it on SKL. It adds more subtests so the runtime is longer,
> > but it also finds new bugs.
>
> Runtime is longer compared to v1 of this patch BTW, I wasn't clear. It is
> roughly on par with the situation without this patch.
>
> But v2 adds a lot more subtests, three of which fail (flip on a rotated
> sprite plane). So there is value in it I think even like that.
Failing new subtests is ok imo, but pls do give a heads-up to CI folks
before merging (but I think as long as it's failing stable, it shouldn't
cause noise). Anything that takes out the machine or driver isn't ok, and
needs to be fixed first.
So sounds all good to me to go ahead.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH i-g-t v2] kms_rotation_crc: 90 degree flip test is not a stress test
2017-09-04 14:27 ` Tvrtko Ursulin
@ 2017-09-04 14:36 ` Tvrtko Ursulin
2017-09-04 14:43 ` Daniel Vetter
[not found] ` <20170907141307.GA15917@kdec5-desk.ger.corp.intel.com>
1 sibling, 1 reply; 18+ messages in thread
From: Tvrtko Ursulin @ 2017-09-04 14:36 UTC (permalink / raw)
To: Daniel Vetter, Tvrtko Ursulin; +Cc: Daniel Vetter, Intel-gfx
On 04/09/2017 15:27, Tvrtko Ursulin wrote:
> On 07/08/2017 16:53, Daniel Vetter wrote:
>> On Fri, Aug 04, 2017 at 09:43:41AM +0100, Tvrtko Ursulin wrote:
>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>
>>> To the best of my recollection the page flipping test was added
>>> simply to start exercising page flips with 90/270 rotation.
>>>
>>> There is no need to do 60 flips which can take quite some time,
>>> because we do 60 flips against each pipe and each fb geometry.
>>>
>>> Also, calling this a stress test is also not matching the
>>> original idea of the test.
>>>
>>> Several changes:
>>>
>>> 1. Remove the stress from the name and reduce the number of
>>> flips to one only.
>>>
>>> 2. Move the page flip before CRC collection for a more useful
>>> test.
>>>
>>> 3. Add more flipping tests, for different rotation and sprite
>>> planes.
>>
>> I assume you didn't make the test overall slower with this?
>>
>>> 4. Convert to table driven subtest generation.
>>>
>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>>
>> Didn't do a full review, but sounds all reasonable. And I assume you've
>> tested this at least locally (the igt patchwork CI instance doesn't do
>> full runs ... yet).
>
> Yes I've ran it on SKL. It adds more subtests so the runtime is longer,
> but it also finds new bugs.
Runtime is longer compared to v1 of this patch BTW, I wasn't clear. It
is roughly on par with the situation without this patch.
But v2 adds a lot more subtests, three of which fail (flip on a rotated
sprite plane). So there is value in it I think even like that.
Tvrtko
> Ideally someone with display-fu picks this up, fixes the bug(s) this
> exposes (or tells me the test is wrong), so we are not merging known
> failing tests? Or even that is OK?
>
> Tvrtko
>
>> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>>> ---
>>> tests/intel-ci/extended.testlist | 2 +-
>>> tests/kms_rotation_crc.c | 137
>>> ++++++++++++++++++++++++---------------
>>> 2 files changed, 85 insertions(+), 54 deletions(-)
>>>
>>> diff --git a/tests/intel-ci/extended.testlist
>>> b/tests/intel-ci/extended.testlist
>>> index 17eed013f810..fb71091758c6 100644
>>> --- a/tests/intel-ci/extended.testlist
>>> +++ b/tests/intel-ci/extended.testlist
>>> @@ -82,7 +82,7 @@ igt@gem_ringfill@vebox-bomb
>>> igt@gem_userptr_blits@stress-mm
>>> igt@gem_userptr_blits@stress-mm-invalidate-close
>>> igt@gem_userptr_blits@stress-mm-invalidate-close-overlap
>>> -igt@kms_rotation_crc@primary-rotation-90-flip-stress
>>> +igt@kms_rotation_crc@primary-rotation-90-flip
>>> igt@pm_rpm@gem-execbuf-stress
>>> igt@pm_rpm@gem-execbuf-stress-extra-wait
>>> igt@pm_rpm@gem-execbuf-stress-pc8
>>> diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
>>> index 83e37f126f40..20f1adb67769 100644
>>> --- a/tests/kms_rotation_crc.c
>>> +++ b/tests/kms_rotation_crc.c
>>> @@ -41,7 +41,7 @@ typedef struct {
>>> int pos_y;
>>> uint32_t override_fmt;
>>> uint64_t override_tiling;
>>> - unsigned int flip_stress;
>>> + unsigned int flips;
>>> } data_t;
>>> static void
>>> @@ -219,7 +219,7 @@ static void prepare_fbs(data_t *data,
>>> igt_output_t *output,
>>> igt_create_fb(data->gfx_fd, w, h, pixel_format, tiling,
>>> &data->fb);
>>> - if (data->flip_stress) {
>>> + if (data->flips) {
>>> igt_create_fb(data->gfx_fd, w, h, pixel_format, tiling,
>>> &data->fb_flip);
>>> paint_squares(data, IGT_ROTATION_0, &data->fb_flip, 0.92);
>>> }
>>> @@ -351,15 +351,17 @@ static void test_plane_rotation(data_t *data,
>>> int plane_type)
>>> ret = igt_display_try_commit2(display, commit);
>>> if (data->override_fmt || data->override_tiling) {
>>> igt_assert_eq(ret, -EINVAL);
>>> - } else {
>>> - igt_assert_eq(ret, 0);
>>> - igt_pipe_crc_collect_crc(data->pipe_crc,
>>> - &crc_output);
>>> - igt_assert_crc_equal(&data->ref_crc,
>>> - &crc_output);
>>> + continue;
>>> }
>>> - flip_count = data->flip_stress;
>>> + /* Verify commit was ok. */
>>> + igt_assert_eq(ret, 0);
>>> +
>>> + /*
>>> + * If flips are requested flip away and back before
>>> + * checking CRC.
>>> + */
>>> + flip_count = data->flips;
>>> while (flip_count--) {
>>> ret = drmModePageFlip(data->gfx_fd,
>>> output->config.crtc->crtc_id,
>>> @@ -376,6 +378,9 @@ static void test_plane_rotation(data_t *data, int
>>> plane_type)
>>> igt_assert_eq(ret, 0);
>>> wait_for_pageflip(data->gfx_fd);
>>> }
>>> +
>>> + igt_pipe_crc_collect_crc(data->pipe_crc, &crc_output);
>>> + igt_assert_crc_equal(&data->ref_crc, &crc_output);
>>> }
>>> valid_tests++;
>>> @@ -569,8 +574,66 @@ err_commit:
>>> igt_assert_eq(ret, 0);
>>> }
>>> +static const char *plane_test_str(unsigned plane)
>>> +{
>>> + switch (plane) {
>>> + case DRM_PLANE_TYPE_PRIMARY:
>>> + return "primary";
>>> + case DRM_PLANE_TYPE_OVERLAY:
>>> + return "sprite";
>>> + case DRM_PLANE_TYPE_CURSOR:
>>> + return "cursor";
>>> + default:
>>> + igt_assert(0);
>>> + }
>>> +}
>>> +
>>> +static const char *rot_test_str(igt_rotation_t rot)
>>> +{
>>> + switch (rot) {
>>> + case IGT_ROTATION_90:
>>> + return "90";
>>> + case IGT_ROTATION_180:
>>> + return "180";
>>> + case IGT_ROTATION_270:
>>> + return "270";
>>> + default:
>>> + igt_assert(0);
>>> + }
>>> +}
>>> +
>>> +static const char *flip_test_str(unsigned flips)
>>> +{
>>> + if (flips > 1)
>>> + return "-flip-stress";
>>> + else if (flips)
>>> + return "-flip";
>>> + else
>>> + return "";
>>> +}
>>> +
>>> igt_main
>>> {
>>> + struct rot_subtest {
>>> + unsigned plane;
>>> + igt_rotation_t rot;
>>> + unsigned flips;
>>> + } *subtest, subtests[] = {
>>> + { DRM_PLANE_TYPE_PRIMARY, IGT_ROTATION_90, 0 },
>>> + { DRM_PLANE_TYPE_PRIMARY, IGT_ROTATION_180, 0 },
>>> + { DRM_PLANE_TYPE_PRIMARY, IGT_ROTATION_270, 0 },
>>> + { DRM_PLANE_TYPE_PRIMARY, IGT_ROTATION_90, 1 },
>>> + { DRM_PLANE_TYPE_PRIMARY, IGT_ROTATION_180, 1 },
>>> + { DRM_PLANE_TYPE_PRIMARY, IGT_ROTATION_270, 1 },
>>> + { DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_90, 0 },
>>> + { DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_180, 0 },
>>> + { DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_270, 0 },
>>> + { DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_90, 1 },
>>> + { DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_180, 1 },
>>> + { DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_270, 1 },
>>> + { DRM_PLANE_TYPE_CURSOR, IGT_ROTATION_180, 0 },
>>> + { 0, 0, 0}
>>> + };
>>> data_t data = {};
>>> int gen = 0;
>>> @@ -586,43 +649,19 @@ igt_main
>>> igt_display_init(&data.display, data.gfx_fd);
>>> }
>>> - igt_subtest_f("primary-rotation-180") {
>>> - data.rotation = IGT_ROTATION_180;
>>> - test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY);
>>> - }
>>> -
>>> - igt_subtest_f("sprite-rotation-180") {
>>> - data.rotation = IGT_ROTATION_180;
>>> - test_plane_rotation(&data, DRM_PLANE_TYPE_OVERLAY);
>>> - }
>>> -
>>> - igt_subtest_f("cursor-rotation-180") {
>>> - data.rotation = IGT_ROTATION_180;
>>> - test_plane_rotation(&data, DRM_PLANE_TYPE_CURSOR);
>>> - }
>>> -
>>> - igt_subtest_f("primary-rotation-90") {
>>> - igt_require(gen >= 9);
>>> - data.rotation = IGT_ROTATION_90;
>>> - test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY);
>>> - }
>>> - igt_subtest_f("primary-rotation-270") {
>>> - igt_require(gen >= 9);
>>> - data.rotation = IGT_ROTATION_270;
>>> - test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY);
>>> - }
>>> -
>>> - igt_subtest_f("sprite-rotation-90") {
>>> - igt_require(gen >= 9);
>>> - data.rotation = IGT_ROTATION_90;
>>> - test_plane_rotation(&data, DRM_PLANE_TYPE_OVERLAY);
>>> - }
>>> -
>>> - igt_subtest_f("sprite-rotation-270") {
>>> - igt_require(gen >= 9);
>>> - data.rotation = IGT_ROTATION_270;
>>> - test_plane_rotation(&data, DRM_PLANE_TYPE_OVERLAY);
>>> + for (subtest = subtests; subtest->rot; subtest++) {
>>> + igt_subtest_f("%s-rotation-%s%s",
>>> + plane_test_str(subtest->plane),
>>> + rot_test_str(subtest->rot),
>>> + flip_test_str(subtest->flips)) {
>>> + igt_require(!(subtest->rot &
>>> + (IGT_ROTATION_90 | IGT_ROTATION_270)) ||
>>> + gen >= 9);
>>> + data.rotation = subtest->rot;
>>> + data.flips = subtest->flips;
>>> + test_plane_rotation(&data, subtest->plane);
>>> + }
>>> }
>>> igt_subtest_f("sprite-rotation-90-pos-100-0") {
>>> @@ -650,14 +689,6 @@ igt_main
>>> test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY);
>>> }
>>> - igt_subtest_f("primary-rotation-90-flip-stress") {
>>> - igt_require(gen >= 9);
>>> - data.override_tiling = 0;
>>> - data.flip_stress = 60;
>>> - data.rotation = IGT_ROTATION_90;
>>> - test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY);
>>> - }
>>> -
>>> igt_subtest_f("primary-rotation-90-Y-tiled") {
>>> enum pipe pipe;
>>> igt_output_t *output;
>>> --
>>> 2.9.4
>>>
>>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH i-g-t v2] kms_rotation_crc: 90 degree flip test is not a stress test
2017-08-07 15:53 ` Daniel Vetter
@ 2017-09-04 14:27 ` Tvrtko Ursulin
2017-09-04 14:36 ` Tvrtko Ursulin
[not found] ` <20170907141307.GA15917@kdec5-desk.ger.corp.intel.com>
0 siblings, 2 replies; 18+ messages in thread
From: Tvrtko Ursulin @ 2017-09-04 14:27 UTC (permalink / raw)
To: Daniel Vetter, Tvrtko Ursulin; +Cc: Daniel Vetter, Intel-gfx
On 07/08/2017 16:53, Daniel Vetter wrote:
> On Fri, Aug 04, 2017 at 09:43:41AM +0100, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> To the best of my recollection the page flipping test was added
>> simply to start exercising page flips with 90/270 rotation.
>>
>> There is no need to do 60 flips which can take quite some time,
>> because we do 60 flips against each pipe and each fb geometry.
>>
>> Also, calling this a stress test is also not matching the
>> original idea of the test.
>>
>> Several changes:
>>
>> 1. Remove the stress from the name and reduce the number of
>> flips to one only.
>>
>> 2. Move the page flip before CRC collection for a more useful
>> test.
>>
>> 3. Add more flipping tests, for different rotation and sprite
>> planes.
>
> I assume you didn't make the test overall slower with this?
>
>> 4. Convert to table driven subtest generation.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>
> Didn't do a full review, but sounds all reasonable. And I assume you've
> tested this at least locally (the igt patchwork CI instance doesn't do
> full runs ... yet).
Yes I've ran it on SKL. It adds more subtests so the runtime is longer,
but it also finds new bugs.
Ideally someone with display-fu picks this up, fixes the bug(s) this
exposes (or tells me the test is wrong), so we are not merging known
failing tests? Or even that is OK?
Tvrtko
> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>> ---
>> tests/intel-ci/extended.testlist | 2 +-
>> tests/kms_rotation_crc.c | 137 ++++++++++++++++++++++++---------------
>> 2 files changed, 85 insertions(+), 54 deletions(-)
>>
>> diff --git a/tests/intel-ci/extended.testlist b/tests/intel-ci/extended.testlist
>> index 17eed013f810..fb71091758c6 100644
>> --- a/tests/intel-ci/extended.testlist
>> +++ b/tests/intel-ci/extended.testlist
>> @@ -82,7 +82,7 @@ igt@gem_ringfill@vebox-bomb
>> igt@gem_userptr_blits@stress-mm
>> igt@gem_userptr_blits@stress-mm-invalidate-close
>> igt@gem_userptr_blits@stress-mm-invalidate-close-overlap
>> -igt@kms_rotation_crc@primary-rotation-90-flip-stress
>> +igt@kms_rotation_crc@primary-rotation-90-flip
>> igt@pm_rpm@gem-execbuf-stress
>> igt@pm_rpm@gem-execbuf-stress-extra-wait
>> igt@pm_rpm@gem-execbuf-stress-pc8
>> diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
>> index 83e37f126f40..20f1adb67769 100644
>> --- a/tests/kms_rotation_crc.c
>> +++ b/tests/kms_rotation_crc.c
>> @@ -41,7 +41,7 @@ typedef struct {
>> int pos_y;
>> uint32_t override_fmt;
>> uint64_t override_tiling;
>> - unsigned int flip_stress;
>> + unsigned int flips;
>> } data_t;
>>
>> static void
>> @@ -219,7 +219,7 @@ static void prepare_fbs(data_t *data, igt_output_t *output,
>>
>> igt_create_fb(data->gfx_fd, w, h, pixel_format, tiling, &data->fb);
>>
>> - if (data->flip_stress) {
>> + if (data->flips) {
>> igt_create_fb(data->gfx_fd, w, h, pixel_format, tiling, &data->fb_flip);
>> paint_squares(data, IGT_ROTATION_0, &data->fb_flip, 0.92);
>> }
>> @@ -351,15 +351,17 @@ static void test_plane_rotation(data_t *data, int plane_type)
>> ret = igt_display_try_commit2(display, commit);
>> if (data->override_fmt || data->override_tiling) {
>> igt_assert_eq(ret, -EINVAL);
>> - } else {
>> - igt_assert_eq(ret, 0);
>> - igt_pipe_crc_collect_crc(data->pipe_crc,
>> - &crc_output);
>> - igt_assert_crc_equal(&data->ref_crc,
>> - &crc_output);
>> + continue;
>> }
>>
>> - flip_count = data->flip_stress;
>> + /* Verify commit was ok. */
>> + igt_assert_eq(ret, 0);
>> +
>> + /*
>> + * If flips are requested flip away and back before
>> + * checking CRC.
>> + */
>> + flip_count = data->flips;
>> while (flip_count--) {
>> ret = drmModePageFlip(data->gfx_fd,
>> output->config.crtc->crtc_id,
>> @@ -376,6 +378,9 @@ static void test_plane_rotation(data_t *data, int plane_type)
>> igt_assert_eq(ret, 0);
>> wait_for_pageflip(data->gfx_fd);
>> }
>> +
>> + igt_pipe_crc_collect_crc(data->pipe_crc, &crc_output);
>> + igt_assert_crc_equal(&data->ref_crc, &crc_output);
>> }
>>
>> valid_tests++;
>> @@ -569,8 +574,66 @@ err_commit:
>> igt_assert_eq(ret, 0);
>> }
>>
>> +static const char *plane_test_str(unsigned plane)
>> +{
>> + switch (plane) {
>> + case DRM_PLANE_TYPE_PRIMARY:
>> + return "primary";
>> + case DRM_PLANE_TYPE_OVERLAY:
>> + return "sprite";
>> + case DRM_PLANE_TYPE_CURSOR:
>> + return "cursor";
>> + default:
>> + igt_assert(0);
>> + }
>> +}
>> +
>> +static const char *rot_test_str(igt_rotation_t rot)
>> +{
>> + switch (rot) {
>> + case IGT_ROTATION_90:
>> + return "90";
>> + case IGT_ROTATION_180:
>> + return "180";
>> + case IGT_ROTATION_270:
>> + return "270";
>> + default:
>> + igt_assert(0);
>> + }
>> +}
>> +
>> +static const char *flip_test_str(unsigned flips)
>> +{
>> + if (flips > 1)
>> + return "-flip-stress";
>> + else if (flips)
>> + return "-flip";
>> + else
>> + return "";
>> +}
>> +
>> igt_main
>> {
>> + struct rot_subtest {
>> + unsigned plane;
>> + igt_rotation_t rot;
>> + unsigned flips;
>> + } *subtest, subtests[] = {
>> + { DRM_PLANE_TYPE_PRIMARY, IGT_ROTATION_90, 0 },
>> + { DRM_PLANE_TYPE_PRIMARY, IGT_ROTATION_180, 0 },
>> + { DRM_PLANE_TYPE_PRIMARY, IGT_ROTATION_270, 0 },
>> + { DRM_PLANE_TYPE_PRIMARY, IGT_ROTATION_90, 1 },
>> + { DRM_PLANE_TYPE_PRIMARY, IGT_ROTATION_180, 1 },
>> + { DRM_PLANE_TYPE_PRIMARY, IGT_ROTATION_270, 1 },
>> + { DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_90, 0 },
>> + { DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_180, 0 },
>> + { DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_270, 0 },
>> + { DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_90, 1 },
>> + { DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_180, 1 },
>> + { DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_270, 1 },
>> + { DRM_PLANE_TYPE_CURSOR, IGT_ROTATION_180, 0 },
>> + { 0, 0, 0}
>> + };
>> data_t data = {};
>> int gen = 0;
>>
>> @@ -586,43 +649,19 @@ igt_main
>>
>> igt_display_init(&data.display, data.gfx_fd);
>> }
>> - igt_subtest_f("primary-rotation-180") {
>> - data.rotation = IGT_ROTATION_180;
>> - test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY);
>> - }
>> -
>> - igt_subtest_f("sprite-rotation-180") {
>> - data.rotation = IGT_ROTATION_180;
>> - test_plane_rotation(&data, DRM_PLANE_TYPE_OVERLAY);
>> - }
>> -
>> - igt_subtest_f("cursor-rotation-180") {
>> - data.rotation = IGT_ROTATION_180;
>> - test_plane_rotation(&data, DRM_PLANE_TYPE_CURSOR);
>> - }
>> -
>> - igt_subtest_f("primary-rotation-90") {
>> - igt_require(gen >= 9);
>> - data.rotation = IGT_ROTATION_90;
>> - test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY);
>> - }
>>
>> - igt_subtest_f("primary-rotation-270") {
>> - igt_require(gen >= 9);
>> - data.rotation = IGT_ROTATION_270;
>> - test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY);
>> - }
>> -
>> - igt_subtest_f("sprite-rotation-90") {
>> - igt_require(gen >= 9);
>> - data.rotation = IGT_ROTATION_90;
>> - test_plane_rotation(&data, DRM_PLANE_TYPE_OVERLAY);
>> - }
>> -
>> - igt_subtest_f("sprite-rotation-270") {
>> - igt_require(gen >= 9);
>> - data.rotation = IGT_ROTATION_270;
>> - test_plane_rotation(&data, DRM_PLANE_TYPE_OVERLAY);
>> + for (subtest = subtests; subtest->rot; subtest++) {
>> + igt_subtest_f("%s-rotation-%s%s",
>> + plane_test_str(subtest->plane),
>> + rot_test_str(subtest->rot),
>> + flip_test_str(subtest->flips)) {
>> + igt_require(!(subtest->rot &
>> + (IGT_ROTATION_90 | IGT_ROTATION_270)) ||
>> + gen >= 9);
>> + data.rotation = subtest->rot;
>> + data.flips = subtest->flips;
>> + test_plane_rotation(&data, subtest->plane);
>> + }
>> }
>>
>> igt_subtest_f("sprite-rotation-90-pos-100-0") {
>> @@ -650,14 +689,6 @@ igt_main
>> test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY);
>> }
>>
>> - igt_subtest_f("primary-rotation-90-flip-stress") {
>> - igt_require(gen >= 9);
>> - data.override_tiling = 0;
>> - data.flip_stress = 60;
>> - data.rotation = IGT_ROTATION_90;
>> - test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY);
>> - }
>> -
>> igt_subtest_f("primary-rotation-90-Y-tiled") {
>> enum pipe pipe;
>> igt_output_t *output;
>> --
>> 2.9.4
>>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH i-g-t v2] kms_rotation_crc: 90 degree flip test is not a stress test
2017-08-04 8:43 ` [PATCH i-g-t v2] " Tvrtko Ursulin
@ 2017-08-07 15:53 ` Daniel Vetter
2017-09-04 14:27 ` Tvrtko Ursulin
0 siblings, 1 reply; 18+ messages in thread
From: Daniel Vetter @ 2017-08-07 15:53 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: Daniel Vetter, Intel-gfx
On Fri, Aug 04, 2017 at 09:43:41AM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> To the best of my recollection the page flipping test was added
> simply to start exercising page flips with 90/270 rotation.
>
> There is no need to do 60 flips which can take quite some time,
> because we do 60 flips against each pipe and each fb geometry.
>
> Also, calling this a stress test is also not matching the
> original idea of the test.
>
> Several changes:
>
> 1. Remove the stress from the name and reduce the number of
> flips to one only.
>
> 2. Move the page flip before CRC collection for a more useful
> test.
>
> 3. Add more flipping tests, for different rotation and sprite
> planes.
I assume you didn't make the test overall slower with this?
> 4. Convert to table driven subtest generation.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
Didn't do a full review, but sounds all reasonable. And I assume you've
tested this at least locally (the igt patchwork CI instance doesn't do
full runs ... yet).
Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> tests/intel-ci/extended.testlist | 2 +-
> tests/kms_rotation_crc.c | 137 ++++++++++++++++++++++++---------------
> 2 files changed, 85 insertions(+), 54 deletions(-)
>
> diff --git a/tests/intel-ci/extended.testlist b/tests/intel-ci/extended.testlist
> index 17eed013f810..fb71091758c6 100644
> --- a/tests/intel-ci/extended.testlist
> +++ b/tests/intel-ci/extended.testlist
> @@ -82,7 +82,7 @@ igt@gem_ringfill@vebox-bomb
> igt@gem_userptr_blits@stress-mm
> igt@gem_userptr_blits@stress-mm-invalidate-close
> igt@gem_userptr_blits@stress-mm-invalidate-close-overlap
> -igt@kms_rotation_crc@primary-rotation-90-flip-stress
> +igt@kms_rotation_crc@primary-rotation-90-flip
> igt@pm_rpm@gem-execbuf-stress
> igt@pm_rpm@gem-execbuf-stress-extra-wait
> igt@pm_rpm@gem-execbuf-stress-pc8
> diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
> index 83e37f126f40..20f1adb67769 100644
> --- a/tests/kms_rotation_crc.c
> +++ b/tests/kms_rotation_crc.c
> @@ -41,7 +41,7 @@ typedef struct {
> int pos_y;
> uint32_t override_fmt;
> uint64_t override_tiling;
> - unsigned int flip_stress;
> + unsigned int flips;
> } data_t;
>
> static void
> @@ -219,7 +219,7 @@ static void prepare_fbs(data_t *data, igt_output_t *output,
>
> igt_create_fb(data->gfx_fd, w, h, pixel_format, tiling, &data->fb);
>
> - if (data->flip_stress) {
> + if (data->flips) {
> igt_create_fb(data->gfx_fd, w, h, pixel_format, tiling, &data->fb_flip);
> paint_squares(data, IGT_ROTATION_0, &data->fb_flip, 0.92);
> }
> @@ -351,15 +351,17 @@ static void test_plane_rotation(data_t *data, int plane_type)
> ret = igt_display_try_commit2(display, commit);
> if (data->override_fmt || data->override_tiling) {
> igt_assert_eq(ret, -EINVAL);
> - } else {
> - igt_assert_eq(ret, 0);
> - igt_pipe_crc_collect_crc(data->pipe_crc,
> - &crc_output);
> - igt_assert_crc_equal(&data->ref_crc,
> - &crc_output);
> + continue;
> }
>
> - flip_count = data->flip_stress;
> + /* Verify commit was ok. */
> + igt_assert_eq(ret, 0);
> +
> + /*
> + * If flips are requested flip away and back before
> + * checking CRC.
> + */
> + flip_count = data->flips;
> while (flip_count--) {
> ret = drmModePageFlip(data->gfx_fd,
> output->config.crtc->crtc_id,
> @@ -376,6 +378,9 @@ static void test_plane_rotation(data_t *data, int plane_type)
> igt_assert_eq(ret, 0);
> wait_for_pageflip(data->gfx_fd);
> }
> +
> + igt_pipe_crc_collect_crc(data->pipe_crc, &crc_output);
> + igt_assert_crc_equal(&data->ref_crc, &crc_output);
> }
>
> valid_tests++;
> @@ -569,8 +574,66 @@ err_commit:
> igt_assert_eq(ret, 0);
> }
>
> +static const char *plane_test_str(unsigned plane)
> +{
> + switch (plane) {
> + case DRM_PLANE_TYPE_PRIMARY:
> + return "primary";
> + case DRM_PLANE_TYPE_OVERLAY:
> + return "sprite";
> + case DRM_PLANE_TYPE_CURSOR:
> + return "cursor";
> + default:
> + igt_assert(0);
> + }
> +}
> +
> +static const char *rot_test_str(igt_rotation_t rot)
> +{
> + switch (rot) {
> + case IGT_ROTATION_90:
> + return "90";
> + case IGT_ROTATION_180:
> + return "180";
> + case IGT_ROTATION_270:
> + return "270";
> + default:
> + igt_assert(0);
> + }
> +}
> +
> +static const char *flip_test_str(unsigned flips)
> +{
> + if (flips > 1)
> + return "-flip-stress";
> + else if (flips)
> + return "-flip";
> + else
> + return "";
> +}
> +
> igt_main
> {
> + struct rot_subtest {
> + unsigned plane;
> + igt_rotation_t rot;
> + unsigned flips;
> + } *subtest, subtests[] = {
> + { DRM_PLANE_TYPE_PRIMARY, IGT_ROTATION_90, 0 },
> + { DRM_PLANE_TYPE_PRIMARY, IGT_ROTATION_180, 0 },
> + { DRM_PLANE_TYPE_PRIMARY, IGT_ROTATION_270, 0 },
> + { DRM_PLANE_TYPE_PRIMARY, IGT_ROTATION_90, 1 },
> + { DRM_PLANE_TYPE_PRIMARY, IGT_ROTATION_180, 1 },
> + { DRM_PLANE_TYPE_PRIMARY, IGT_ROTATION_270, 1 },
> + { DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_90, 0 },
> + { DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_180, 0 },
> + { DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_270, 0 },
> + { DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_90, 1 },
> + { DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_180, 1 },
> + { DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_270, 1 },
> + { DRM_PLANE_TYPE_CURSOR, IGT_ROTATION_180, 0 },
> + { 0, 0, 0}
> + };
> data_t data = {};
> int gen = 0;
>
> @@ -586,43 +649,19 @@ igt_main
>
> igt_display_init(&data.display, data.gfx_fd);
> }
> - igt_subtest_f("primary-rotation-180") {
> - data.rotation = IGT_ROTATION_180;
> - test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY);
> - }
> -
> - igt_subtest_f("sprite-rotation-180") {
> - data.rotation = IGT_ROTATION_180;
> - test_plane_rotation(&data, DRM_PLANE_TYPE_OVERLAY);
> - }
> -
> - igt_subtest_f("cursor-rotation-180") {
> - data.rotation = IGT_ROTATION_180;
> - test_plane_rotation(&data, DRM_PLANE_TYPE_CURSOR);
> - }
> -
> - igt_subtest_f("primary-rotation-90") {
> - igt_require(gen >= 9);
> - data.rotation = IGT_ROTATION_90;
> - test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY);
> - }
>
> - igt_subtest_f("primary-rotation-270") {
> - igt_require(gen >= 9);
> - data.rotation = IGT_ROTATION_270;
> - test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY);
> - }
> -
> - igt_subtest_f("sprite-rotation-90") {
> - igt_require(gen >= 9);
> - data.rotation = IGT_ROTATION_90;
> - test_plane_rotation(&data, DRM_PLANE_TYPE_OVERLAY);
> - }
> -
> - igt_subtest_f("sprite-rotation-270") {
> - igt_require(gen >= 9);
> - data.rotation = IGT_ROTATION_270;
> - test_plane_rotation(&data, DRM_PLANE_TYPE_OVERLAY);
> + for (subtest = subtests; subtest->rot; subtest++) {
> + igt_subtest_f("%s-rotation-%s%s",
> + plane_test_str(subtest->plane),
> + rot_test_str(subtest->rot),
> + flip_test_str(subtest->flips)) {
> + igt_require(!(subtest->rot &
> + (IGT_ROTATION_90 | IGT_ROTATION_270)) ||
> + gen >= 9);
> + data.rotation = subtest->rot;
> + data.flips = subtest->flips;
> + test_plane_rotation(&data, subtest->plane);
> + }
> }
>
> igt_subtest_f("sprite-rotation-90-pos-100-0") {
> @@ -650,14 +689,6 @@ igt_main
> test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY);
> }
>
> - igt_subtest_f("primary-rotation-90-flip-stress") {
> - igt_require(gen >= 9);
> - data.override_tiling = 0;
> - data.flip_stress = 60;
> - data.rotation = IGT_ROTATION_90;
> - test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY);
> - }
> -
> igt_subtest_f("primary-rotation-90-Y-tiled") {
> enum pipe pipe;
> igt_output_t *output;
> --
> 2.9.4
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH i-g-t v2] kms_rotation_crc: 90 degree flip test is not a stress test
2017-08-03 12:42 [PATCH i-g-t] kms_rotation_crc: 90 degree flip test is not a stress test Tvrtko Ursulin
@ 2017-08-04 8:43 ` Tvrtko Ursulin
2017-08-07 15:53 ` Daniel Vetter
0 siblings, 1 reply; 18+ messages in thread
From: Tvrtko Ursulin @ 2017-08-04 8:43 UTC (permalink / raw)
To: Intel-gfx; +Cc: Daniel Vetter
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
To the best of my recollection the page flipping test was added
simply to start exercising page flips with 90/270 rotation.
There is no need to do 60 flips which can take quite some time,
because we do 60 flips against each pipe and each fb geometry.
Also, calling this a stress test is also not matching the
original idea of the test.
Several changes:
1. Remove the stress from the name and reduce the number of
flips to one only.
2. Move the page flip before CRC collection for a more useful
test.
3. Add more flipping tests, for different rotation and sprite
planes.
4. Convert to table driven subtest generation.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
tests/intel-ci/extended.testlist | 2 +-
tests/kms_rotation_crc.c | 137 ++++++++++++++++++++++++---------------
2 files changed, 85 insertions(+), 54 deletions(-)
diff --git a/tests/intel-ci/extended.testlist b/tests/intel-ci/extended.testlist
index 17eed013f810..fb71091758c6 100644
--- a/tests/intel-ci/extended.testlist
+++ b/tests/intel-ci/extended.testlist
@@ -82,7 +82,7 @@ igt@gem_ringfill@vebox-bomb
igt@gem_userptr_blits@stress-mm
igt@gem_userptr_blits@stress-mm-invalidate-close
igt@gem_userptr_blits@stress-mm-invalidate-close-overlap
-igt@kms_rotation_crc@primary-rotation-90-flip-stress
+igt@kms_rotation_crc@primary-rotation-90-flip
igt@pm_rpm@gem-execbuf-stress
igt@pm_rpm@gem-execbuf-stress-extra-wait
igt@pm_rpm@gem-execbuf-stress-pc8
diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index 83e37f126f40..20f1adb67769 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -41,7 +41,7 @@ typedef struct {
int pos_y;
uint32_t override_fmt;
uint64_t override_tiling;
- unsigned int flip_stress;
+ unsigned int flips;
} data_t;
static void
@@ -219,7 +219,7 @@ static void prepare_fbs(data_t *data, igt_output_t *output,
igt_create_fb(data->gfx_fd, w, h, pixel_format, tiling, &data->fb);
- if (data->flip_stress) {
+ if (data->flips) {
igt_create_fb(data->gfx_fd, w, h, pixel_format, tiling, &data->fb_flip);
paint_squares(data, IGT_ROTATION_0, &data->fb_flip, 0.92);
}
@@ -351,15 +351,17 @@ static void test_plane_rotation(data_t *data, int plane_type)
ret = igt_display_try_commit2(display, commit);
if (data->override_fmt || data->override_tiling) {
igt_assert_eq(ret, -EINVAL);
- } else {
- igt_assert_eq(ret, 0);
- igt_pipe_crc_collect_crc(data->pipe_crc,
- &crc_output);
- igt_assert_crc_equal(&data->ref_crc,
- &crc_output);
+ continue;
}
- flip_count = data->flip_stress;
+ /* Verify commit was ok. */
+ igt_assert_eq(ret, 0);
+
+ /*
+ * If flips are requested flip away and back before
+ * checking CRC.
+ */
+ flip_count = data->flips;
while (flip_count--) {
ret = drmModePageFlip(data->gfx_fd,
output->config.crtc->crtc_id,
@@ -376,6 +378,9 @@ static void test_plane_rotation(data_t *data, int plane_type)
igt_assert_eq(ret, 0);
wait_for_pageflip(data->gfx_fd);
}
+
+ igt_pipe_crc_collect_crc(data->pipe_crc, &crc_output);
+ igt_assert_crc_equal(&data->ref_crc, &crc_output);
}
valid_tests++;
@@ -569,8 +574,66 @@ err_commit:
igt_assert_eq(ret, 0);
}
+static const char *plane_test_str(unsigned plane)
+{
+ switch (plane) {
+ case DRM_PLANE_TYPE_PRIMARY:
+ return "primary";
+ case DRM_PLANE_TYPE_OVERLAY:
+ return "sprite";
+ case DRM_PLANE_TYPE_CURSOR:
+ return "cursor";
+ default:
+ igt_assert(0);
+ }
+}
+
+static const char *rot_test_str(igt_rotation_t rot)
+{
+ switch (rot) {
+ case IGT_ROTATION_90:
+ return "90";
+ case IGT_ROTATION_180:
+ return "180";
+ case IGT_ROTATION_270:
+ return "270";
+ default:
+ igt_assert(0);
+ }
+}
+
+static const char *flip_test_str(unsigned flips)
+{
+ if (flips > 1)
+ return "-flip-stress";
+ else if (flips)
+ return "-flip";
+ else
+ return "";
+}
+
igt_main
{
+ struct rot_subtest {
+ unsigned plane;
+ igt_rotation_t rot;
+ unsigned flips;
+ } *subtest, subtests[] = {
+ { DRM_PLANE_TYPE_PRIMARY, IGT_ROTATION_90, 0 },
+ { DRM_PLANE_TYPE_PRIMARY, IGT_ROTATION_180, 0 },
+ { DRM_PLANE_TYPE_PRIMARY, IGT_ROTATION_270, 0 },
+ { DRM_PLANE_TYPE_PRIMARY, IGT_ROTATION_90, 1 },
+ { DRM_PLANE_TYPE_PRIMARY, IGT_ROTATION_180, 1 },
+ { DRM_PLANE_TYPE_PRIMARY, IGT_ROTATION_270, 1 },
+ { DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_90, 0 },
+ { DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_180, 0 },
+ { DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_270, 0 },
+ { DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_90, 1 },
+ { DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_180, 1 },
+ { DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_270, 1 },
+ { DRM_PLANE_TYPE_CURSOR, IGT_ROTATION_180, 0 },
+ { 0, 0, 0}
+ };
data_t data = {};
int gen = 0;
@@ -586,43 +649,19 @@ igt_main
igt_display_init(&data.display, data.gfx_fd);
}
- igt_subtest_f("primary-rotation-180") {
- data.rotation = IGT_ROTATION_180;
- test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY);
- }
-
- igt_subtest_f("sprite-rotation-180") {
- data.rotation = IGT_ROTATION_180;
- test_plane_rotation(&data, DRM_PLANE_TYPE_OVERLAY);
- }
-
- igt_subtest_f("cursor-rotation-180") {
- data.rotation = IGT_ROTATION_180;
- test_plane_rotation(&data, DRM_PLANE_TYPE_CURSOR);
- }
-
- igt_subtest_f("primary-rotation-90") {
- igt_require(gen >= 9);
- data.rotation = IGT_ROTATION_90;
- test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY);
- }
- igt_subtest_f("primary-rotation-270") {
- igt_require(gen >= 9);
- data.rotation = IGT_ROTATION_270;
- test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY);
- }
-
- igt_subtest_f("sprite-rotation-90") {
- igt_require(gen >= 9);
- data.rotation = IGT_ROTATION_90;
- test_plane_rotation(&data, DRM_PLANE_TYPE_OVERLAY);
- }
-
- igt_subtest_f("sprite-rotation-270") {
- igt_require(gen >= 9);
- data.rotation = IGT_ROTATION_270;
- test_plane_rotation(&data, DRM_PLANE_TYPE_OVERLAY);
+ for (subtest = subtests; subtest->rot; subtest++) {
+ igt_subtest_f("%s-rotation-%s%s",
+ plane_test_str(subtest->plane),
+ rot_test_str(subtest->rot),
+ flip_test_str(subtest->flips)) {
+ igt_require(!(subtest->rot &
+ (IGT_ROTATION_90 | IGT_ROTATION_270)) ||
+ gen >= 9);
+ data.rotation = subtest->rot;
+ data.flips = subtest->flips;
+ test_plane_rotation(&data, subtest->plane);
+ }
}
igt_subtest_f("sprite-rotation-90-pos-100-0") {
@@ -650,14 +689,6 @@ igt_main
test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY);
}
- igt_subtest_f("primary-rotation-90-flip-stress") {
- igt_require(gen >= 9);
- data.override_tiling = 0;
- data.flip_stress = 60;
- data.rotation = IGT_ROTATION_90;
- test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY);
- }
-
igt_subtest_f("primary-rotation-90-Y-tiled") {
enum pipe pipe;
igt_output_t *output;
--
2.9.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 18+ messages in thread
end of thread, other threads:[~2017-09-12 8:49 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-08 11:10 [PATCH i-g-t v2] kms_rotation_crc: 90 degree flip test is not a stress test Tvrtko Ursulin
2017-09-08 11:24 ` [PATCH i-g-t v3] " Tvrtko Ursulin
2017-09-08 14:06 ` Chris Wilson
2017-09-08 14:54 ` Tvrtko Ursulin
2017-09-08 16:05 ` Chris Wilson
2017-09-11 17:14 ` [PATCH i-g-t v4] " Tvrtko Ursulin
2017-09-12 8:49 ` Katarzyna Dec
2017-09-08 11:45 ` ✓ Fi.CI.BAT: success for kms_rotation_crc: 90 degree flip test is not a stress test (rev4) Patchwork
2017-09-08 13:58 ` ✗ Fi.CI.IGT: failure " Patchwork
2017-09-11 17:54 ` ✓ Fi.CI.BAT: success for kms_rotation_crc: 90 degree flip test is not a stress test (rev5) Patchwork
2017-09-11 21:59 ` ✗ Fi.CI.IGT: failure " Patchwork
-- strict thread matches above, loose matches on Subject: below --
2017-08-03 12:42 [PATCH i-g-t] kms_rotation_crc: 90 degree flip test is not a stress test Tvrtko Ursulin
2017-08-04 8:43 ` [PATCH i-g-t v2] " Tvrtko Ursulin
2017-08-07 15:53 ` Daniel Vetter
2017-09-04 14:27 ` Tvrtko Ursulin
2017-09-04 14:36 ` Tvrtko Ursulin
2017-09-04 14:43 ` Daniel Vetter
2017-09-04 14:56 ` Tvrtko Ursulin
[not found] ` <20170907141307.GA15917@kdec5-desk.ger.corp.intel.com>
[not found] ` <804db97c-0746-3796-01c0-8d0b390528f5@linux.intel.com>
2017-09-07 15:07 ` Katarzyna Dec
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.