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

[Maarten Lankhorst]

Op 23-05-2019 om 22:19 schreef Ville Syrjala:
> From: Ville Syrjälä <ville.syrjala at 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 at 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 at linux.intel.com>