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...
> + */
> + 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@xxxxxxxxxxxxxxxxxx>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
