On Fri, Aug 04, 2017 at 09:43:41AM +0100, Tvrtko Ursulin wrote:
> 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.
>
> 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@xxxxxxxxx>
> Cc: Daniel Vetter <daniel.vetter@xxxxxxxx>
> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
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@xxxxxxxx>
> ---
> 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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
