On 08/09/2017 15:06, Chris Wilson wrote:
Quoting Tvrtko Ursulin (2017-09-08 12:24:07)
From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
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@xxxxxxxxx>
Cc: Daniel Vetter <daniel.vetter@xxxxxxxx>
Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Katarzyna Dec <katarzyna.dec@xxxxxxxxx>
---
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@xxxxxxxxxxxxxxx.
Thanks.
Regards,
Tvrtko
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
