Re: [igt-dev] [PATCH i-g-t] tests/kms_rotation_crc: Speed up by not testing every format

[Martin Peres <martin.peres@linux.intel.com>]

On 2020-02-14 18:55, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> As we did with kms_plane_scaling let's not test every pixel
> format for rotation fails either. Instead we test each "class"
> of formats as defined by igt_reduce_format(). The hope being
> that there are no specific bugs in the hw for different
> swizzles of the same underlying base format.
> 
> On KBL:
> $ time ./build/tests/kms_rotation_crc --r sprite-rotation-90
> - real	0m39,851s
> - user	0m1,037s
> - sys	0m12,895s
> + real	0m16,773s
> + user	0m0,357s
> + sys	0m5,475s
> 
> As per the usual recipe we add an --extended cmdline knob to
> force the full coverage.
> 
> Only cuts ~30% of the total kms_rotation_crc runtime though.
> There are other subtests which take over 80s on this particular
> machine, so further work is clearly required to get the testing
> time to sane levels.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

This cuts the execution time of the test from 00:15:07 -> 00:06:53 on
TGL, so this is definitely welcomed!

As for the patch itself, I agree with the approach and I like that you
added the --extended flag just to cover our basis. One day, we should
work on running these extended tests randomly, but today is not the day.

Reviewed-by: Martin Peres <martin.peres@linux.intel.com>

Thanks!

> ---
>  tests/kms_rotation_crc.c | 58 +++++++++++++++++++++++++++++++++++++---
>  1 file changed, 55 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
> index 2a7b10e990e2..fc4c13389953 100644
> --- a/tests/kms_rotation_crc.c
> +++ b/tests/kms_rotation_crc.c
> @@ -23,6 +23,7 @@
>   */
>  
>  #include "igt.h"
> +#include "igt_vec.h"
>  #include <math.h>
>  
>  #define MAX_FENCES 32
> @@ -65,6 +66,7 @@ typedef struct {
>  
>  	struct p_struct *multiplaneoldview;
>  	struct p_point planepos[MAXMULTIPLANESAMOUNT];
> +	bool extended;
>  } data_t;
>  
>  typedef struct {
> @@ -371,6 +373,28 @@ static void test_single_case(data_t *data, enum pipe pipe,
>  	}
>  }
>  
> +static bool test_format(data_t *data,
> +			struct igt_vec *tested_formats,
> +			uint32_t format)
> +{
> +	if (!igt_fb_supported_format(format))
> +		return false;
> +
> +	if (!is_i915_device(data->gfx_fd) ||
> +	    data->extended)
> +		return true;
> +
> +	format = igt_reduce_format(format);
> +
> +	/* only test each format "class" once */
> +	if (igt_vec_index(tested_formats, &format) >= 0)
> +		return false;
> +
> +	igt_vec_push(tested_formats, &format);
> +
> +	return true;
> +}
> +
>  static void test_plane_rotation(data_t *data, int plane_type, bool test_bad_format)
>  {
>  	igt_display_t *display = &data->display;
> @@ -408,15 +432,21 @@ static void test_plane_rotation(data_t *data, int plane_type, bool test_bad_form
>  				continue;
>  
>  			if (!data->override_fmt) {
> +				struct igt_vec tested_formats;
> +
> +				igt_vec_init(&tested_formats, sizeof(uint32_t));
> +
>  				for (j = 0; j < plane->drm_plane->count_formats; j++) {
>  					uint32_t format = plane->drm_plane->formats[j];
>  
> -					if (!igt_fb_supported_format(format))
> +					if (!test_format(data, &tested_formats, format))
>  						continue;
>  
>  					test_single_case(data, pipe, output, plane, i,
>  							 format, test_bad_format);
>  				}
> +
> +				igt_vec_fini(&tested_formats);
>  			} else {
>  				test_single_case(data, pipe, output, plane, i,
>  						 data->override_fmt, test_bad_format);
> @@ -738,7 +768,30 @@ static const char *tiling_test_str(uint64_t tiling)
>  	}
>  }
>  
> -igt_main
> +static int opt_handler(int opt, int opt_index, void *_data)
> +{
> +	data_t *data = _data;
> +
> +	switch (opt) {
> +	case 'e':
> +		data->extended = true;
> +		break;
> +	}
> +
> +	return IGT_OPT_HANDLER_SUCCESS;
> +}
> +
> +static const struct option long_opts[] = {
> +	{ .name = "extended", .has_arg = false, .val = 'e', },
> +	{}
> +};
> +
> +static const char help_str[] =
> +	"  --extended\t\tRun the extended tests\n";
> +
> +static data_t data;
> +
> +igt_main_args("", long_opts, help_str, opt_handler, &data)
>  {
>  	struct rot_subtest {
>  		unsigned plane;
> @@ -771,7 +824,6 @@ igt_main
>  		{ 0, 0 }
>  	};
>  
> -	data_t data = {};
>  	int gen = 0;
>  
>  	igt_fixture {
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev