On Fri, Aug 04, 2017 at 09:43:41AM +0100, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin <[email protected]> > > 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 <[email protected]> > Cc: Daniel Vetter <[email protected]> > Cc: Chris Wilson <[email protected]> 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 <[email protected]> > --- > 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 [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
