Re: [igt-dev] [PATCH i-g-t 2/2] tests/kms_big_fb: Delay the expensive big fb creation until the last moment

Maarten Lankhorst
2019-05-28T10:56:22Z

Op 23-05-2019 om 22:19 schreef Ville Syrjala:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> We already perform the format+mod+rotation test using the small fb, so
> we don't actually need to create the big fb until that test has been
> performed. We do need to calculate the big fb dimensions though so that
> we can calculate the coordinates we're going to use in the test.
>
> $ time kms_big_fb
> - real 0m52.902s
> + real 0m52.616s
> so not so great.
>
> But if I run each subtest separately it starts to add up:
> $ for i in `kms_big_fb --l` ; do kms_big_fb --r $i ; done
> - real 5m32.898s
> + real 4m32.164s
>
> The big difference between those two ways of running the test
> is at least partially due to the test reusing big fb between
> the different rotation subtests for each format+mod combo.
> Running each subtest individually can't get that benefit so
> avoiding needless big fb creation starts to make a difference.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> tests/kms_big_fb.c | 77 ++++++++++++++++++++++++----------------------
> 1 file changed, 41 insertions(+), 36 deletions(-)
>
> diff --git a/tests/kms_big_fb.c b/tests/kms_big_fb.c
> index c3520e225ab9..b8813a333643 100644
> --- a/tests/kms_big_fb.c
> +++ b/tests/kms_big_fb.c
> @@ -43,6 +43,7 @@ typedef struct {
> int width, height;
> igt_rotation_t rotation;
> int max_fb_width, max_fb_height;
> + int big_fb_width, big_fb_height;
> uint64_t ram_size, aper_size, mappable_size;
> igt_render_copyfunc_t render_copy;
> drm_intel_bufmgr *bufmgr;
> @@ -186,13 +187,32 @@ static void max_fb_size(data_t *data, int *width, int *height,
> *width, *height);
> }
>
> +static void prep_fb(data_t *data)
> +{
> + if (data->big_fb.fb_id)
> + return;
> +
> + igt_create_fb(data->drm_fd,
> + data->big_fb_width, data->big_fb_height,
> + data->format, data->modifier,
> + &data->big_fb);
> +
> + generate_pattern(data, &data->big_fb, 640, 480);
> +}
> +
> +static void cleanup_fb(data_t *data)
> +{
> + igt_remove_fb(data->drm_fd, &data->big_fb);
> + data->big_fb.fb_id = 0;
> +}
> +
> static bool test_plane(data_t *data)
> {
> igt_plane_t *plane = data->plane;
> struct igt_fb *small_fb = &data->small_fb;
> struct igt_fb *big_fb = &data->big_fb;
> - int w = big_fb->width - small_fb->width;
> - int h = big_fb->height - small_fb->height;
> + int w = data->big_fb_width - small_fb->width;
> + int h = data->big_fb_height - small_fb->height;
> struct {
> int x, y;
> } coords[] = {
> @@ -235,16 +255,6 @@ static bool test_plane(data_t *data)
> y &= ~1;
> }
>
> - /*
> - * Make a 1:1 copy of the desired part of the big fb
> - * rather than try to render the same pattern (translated
> - * accordinly) again via cairo. Something in cairo's
> - * rendering pipeline introduces slight differences into
> - * the result if we try that, and so the crc will not match.
> - */
> - copy_pattern(data, small_fb, 0, 0, big_fb, x, y,
> - small_fb->width, small_fb->height);
> -
> igt_plane_set_fb(plane, small_fb);
> igt_plane_set_size(plane, data->width, data->height);
>
> @@ -262,6 +272,22 @@ static bool test_plane(data_t *data)
> return false;
> }
>
> + /*
> + * To speed up skips we delay the big fb creation until
> + * the above rotation related check has been performed.
> + */
> + prep_fb(data);
> +
> + /*
> + * Make a 1:1 copy of the desired part of the big fb
> + * rather than try to render the same pattern (translated
> + * accordinly) again via cairo. Something in cairo's
> + * rendering pipeline introduces slight differences into
> + * the result if we try that, and so the crc will not match.
> + */
> + copy_pattern(data, small_fb, 0, 0, big_fb, x, y,
> + small_fb->width, small_fb->height);
> +
> igt_display_commit2(&data->display, data->display.is_atomic ?
> COMMIT_ATOMIC : COMMIT_UNIVERSAL);
>
> @@ -352,6 +378,9 @@ static bool test_pipe(data_t *data)
>
> static void test_scanout(data_t *data)
> {
> + max_fb_size(data, &data->big_fb_width, &data->big_fb_height,
> + data->format, data->modifier);
> +
> for_each_pipe_with_valid_output(&data->display, data->pipe, data->output) {
> if (test_pipe(data))
> return;
> @@ -361,29 +390,6 @@ static void test_scanout(data_t *data)
> igt_skip("unsupported configuration\n");
> }
>
> -static void prep_fb(data_t *data)
> -{
> - int width, height;
> -
> - if (data->big_fb.fb_id)
> - return;
> -
> - max_fb_size(data, &width, &height,
> - data->format, data->modifier);
> -
> - igt_create_fb(data->drm_fd, width, height,
> - data->format, data->modifier,
> - &data->big_fb);
> -
> - generate_pattern(data, &data->big_fb, 640, 480);
> -}
> -
> -static void cleanup_fb(data_t *data)
> -{
> - igt_remove_fb(data->drm_fd, &data->big_fb);
> - data->big_fb.fb_id = 0;
> -}
> -
> static void
> test_size_overflow(data_t *data)
> {
> @@ -650,7 +656,6 @@ igt_main
> formats[j].bpp, rotations[k].angle) {
> igt_require(igt_fb_supported_format(data.format));
> igt_require(igt_display_has_format_mod(&data.display, data.format, data.modifier));
> - prep_fb(&data);
> test_scanout(&data);
> }
> }

For the series:

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>