Re: [igt-dev] [PATCH i-g-t 2/2] kms_flip: Convert to dynamic CRTC test groups

[Petri Latvala <petri.latvala@intel.com>]

On Sat, Apr 11, 2020 at 12:14:27PM +0100, Chris Wilson wrote:
> Use igt_dynamic_f for the dynamic discovered CRTC sets for each test.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Petri Latvala <petri.latvala@intel.com>
> Cc: Martin Peres <martin.peres@linux.intel.com>
> ---
>  tests/kms_flip.c | 93 ++++++++++++++++++++++++------------------------
>  1 file changed, 47 insertions(+), 46 deletions(-)
> 
> diff --git a/tests/kms_flip.c b/tests/kms_flip.c
> index 4535ecc8b..41d33f75a 100755
> --- a/tests/kms_flip.c
> +++ b/tests/kms_flip.c
> @@ -1185,52 +1185,17 @@ static void calibrate_ts(struct test_output *o, int crtc_idx)
>  	o->vblank_interval = mean;
>  }
>  
> -static void run_test_on_crtc_set(struct test_output *o, int *crtc_idxs,
> -				 int crtc_count, int duration_ms)
> +static void __run_test_on_crtc_set(struct test_output *o, int *crtc_idxs,
> +				   int crtc_count, int duration_ms)
>  {
> -	char test_name[128];
> -	unsigned elapsed;
>  	unsigned bo_size = 0;
> +	bool vblank = true;
> +	unsigned elapsed;
>  	uint64_t tiling;
>  	int i;
> -	bool vblank = true;
> -
> -	switch (crtc_count) {
> -	case RUN_TEST:
> -		connector_find_preferred_mode(o->_connector[0], crtc_idxs[0], o);
> -		if (!o->mode_valid)
> -			return;
> -		snprintf(test_name, sizeof(test_name),
> -			 "%s on pipe %s, connector %s-%d",
> -			 igt_subtest_name(),
> -			 kmstest_pipe_name(o->_pipe[0]),
> -			 kmstest_connector_type_str(o->kconnector[0]->connector_type),
> -			 o->kconnector[0]->connector_type_id);
> -		break;
> -	case RUN_PAIR:
> -		connector_find_compatible_mode(crtc_idxs[0], crtc_idxs[1], o);
> -		if (!o->mode_valid)
> -			return;
> -		snprintf(test_name, sizeof(test_name),
> -			 "%s on pipe %s:%s, connector %s-%d:%s-%d",
> -			 igt_subtest_name(),
> -			 kmstest_pipe_name(o->_pipe[0]),
> -			 kmstest_pipe_name(o->_pipe[1]),
> -			 kmstest_connector_type_str(o->kconnector[0]->connector_type),
> -			 o->kconnector[0]->connector_type_id,
> -			 kmstest_connector_type_str(o->kconnector[1]->connector_type),
> -			 o->kconnector[1]->connector_type_id);
> -		break;
> -	default:
> -		igt_assert(0);
> -	}
> -
> -	igt_assert_eq(o->count, crtc_count);
>  
>  	last_connector = o->kconnector[0];
>  
> -	igt_info("Beginning %s\n", test_name);
> -
>  	if (o->flags & TEST_PAN)
>  		o->fb_width *= 2;
>  
> @@ -1274,7 +1239,6 @@ static void run_test_on_crtc_set(struct test_output *o, int *crtc_idxs,
>  		 */
>  		igt_assert_f(crtc_count > 1 || crtc_idxs[0] < 2,
>  			     "set_mode may only fail on the 3rd pipe or in multiple crtc tests\n");
> -		igt_info("\n%s: SKIPPED\n\n", test_name);
>  		goto out;
>  	}
>  	igt_assert(fb_is_bound(o, o->fb_ids[0]));
> @@ -1322,8 +1286,6 @@ static void run_test_on_crtc_set(struct test_output *o, int *crtc_idxs,
>  	if (o->flags & TEST_VBLANK)
>  		check_final_state(o, &o->vblank_state, elapsed);
>  
> -	igt_info("\n%s: PASSED\n\n", test_name);
> -
>  out:
>  	igt_remove_fb(drm_fd, &o->fb_info[2]);
>  	igt_remove_fb(drm_fd, &o->fb_info[1]);
> @@ -1334,6 +1296,45 @@ out:
>  	free_test_output(o);
>  }
>  
> +static void run_test_on_crtc_set(struct test_output *o, int *crtc_idxs,
> +				 int crtc_count, int duration_ms)
> +{
> +	char test_name[128];
> +
> +	switch (crtc_count) {
> +	case RUN_TEST:
> +		connector_find_preferred_mode(o->_connector[0], crtc_idxs[0], o);
> +		if (!o->mode_valid)
> +			return;
> +		snprintf(test_name, sizeof(test_name),
> +			 "%s::%s-%d",
> +			 kmstest_pipe_name(o->_pipe[0]),
> +			 kmstest_connector_type_str(o->kconnector[0]->connector_type),
> +			 o->kconnector[0]->connector_type_id);
> +		break;
> +	case RUN_PAIR:
> +		connector_find_compatible_mode(crtc_idxs[0], crtc_idxs[1], o);
> +		if (!o->mode_valid)
> +			return;
> +		snprintf(test_name, sizeof(test_name),
> +			 "%s:%s::%s-%d:%s-%d",
> +			 kmstest_pipe_name(o->_pipe[0]),
> +			 kmstest_pipe_name(o->_pipe[1]),
> +			 kmstest_connector_type_str(o->kconnector[0]->connector_type),
> +			 o->kconnector[0]->connector_type_id,
> +			 kmstest_connector_type_str(o->kconnector[1]->connector_type),
> +			 o->kconnector[1]->connector_type_id);


While in principle I have nothing against expanding the alphabet of
valid subtest name, it brings in repercussions that I'm not
comfortable reviewing before at least three cups of coffee and even
then I'll have to require some unit tests in the scary json tests
directory. My runner-comms branch would help immensely for the parsing
of these subtest names in runner but alas ETIME...

Is it too unreadable to name these as

"pipe-%s-%s-%d" for RUN_TEST ("pipe-A-eDP-1")

"pipe-%s-pipe-%s-%s-%d-%s-%d" for RUN_PAIR
("pipe-A-pipe-B-eDP-1-DP-2")

?


-- 
Petri Latvala
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev