[Intel-gfx] [PATCH i-g-t v2] kms_rotation_crc: 90 degree flip test is not a stress test
Tvrtko Ursulin
tvrtko.ursulin at linux.intel.com
Mon Sep 4 14:36:56 UTC 2017
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 at 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 at intel.com>
>>> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
>>> Cc: Chris Wilson <chris at 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 at 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 at gem_ringfill@vebox-bomb
>>> igt at gem_userptr_blits@stress-mm
>>> igt at gem_userptr_blits@stress-mm-invalidate-close
>>> igt at gem_userptr_blits@stress-mm-invalidate-close-overlap
>>> -igt at kms_rotation_crc@primary-rotation-90-flip-stress
>>> +igt at kms_rotation_crc@primary-rotation-90-flip
>>> igt at pm_rpm@gem-execbuf-stress
>>> igt at pm_rpm@gem-execbuf-stress-extra-wait
>>> igt at 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
>>>
>>
More information about the Intel-gfx
mailing list