[PATCH i-g-t v2] tests/intel/kms_big_fb: Introducing this restriction to optimize the test execution time on simulation

[Kamil Konieczny]

Hi Pranay,
On 2024-05-16 at 15:58:17 +0530, Pranay Samala wrote:

please make subject shorter, instead of:

[PATCH i-g-t v2] tests/intel/kms_big_fb: Introducing this restriction to optimize the test execution time on simulation

imho better:
[PATCH i-g-t v2] tests/intel/kms_big_fb: Restrict runs on simulation


> This test executes on only one pipe and all the availble planes with
------------------------------------------------- ^^^^^^^^
s/availble/available/

> set of 6 coordinates. Due to this, test takes long duration to execute
> on simulation and gets timeout.
> 
> Restricting the test to execute only on primary plane with one of the
> coordinates for simulation.
> 
> v2:
> - Breaking the loop instead of using variable m to test only one
>   coordinate for simulation (Kamil)
> - Removed the variable m (Kamil)
> - By default the test is running on only one pipe (Kamil)
> - Testing on (0,0) coordinate (Kamil)
> 
> Signed-off-by: Pranay Samala <pranay.samala at intel.com>
> ---
>  tests/intel/kms_big_fb.c | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/intel/kms_big_fb.c b/tests/intel/kms_big_fb.c
> index 1e45d8e42..f1207e21f 100644
> --- a/tests/intel/kms_big_fb.c
> +++ b/tests/intel/kms_big_fb.c
> @@ -455,6 +455,11 @@ static bool test_plane(data_t *data)
>  		{ w, h, },
>  	};
>  
> +	if (igt_run_in_simulation()) {
> +		plane = igt_output_get_plane_type(data->output, DRM_PLANE_TYPE_PRIMARY);
> +		igt_plane_set_fb(plane, NULL);
> +	}
> +

Why do you need this here?

>  	if (!igt_plane_has_format_mod(plane, data->format, data->modifier))
>  		return false;
>  
> @@ -529,6 +534,9 @@ static bool test_plane(data_t *data)
>  
>  		igt_assert_crc_equal(&big_crc, &small_crc);
>  		igt_pipe_crc_stop(data->pipe_crc);
> +
> +		if (igt_run_in_simulation())
> +			break;

It would be better to place this just after for(;;) where you
could also choose for which 'i' make test. It is ok if you
want to choose x,y = 0,0

>  	}
>  
>  	return true;
> @@ -595,10 +603,14 @@ static bool test_pipe(data_t *data)
>  	data->pipe_crc = igt_pipe_crc_new(data->drm_fd, data->pipe,
>  					  IGT_PIPE_CRC_SOURCE_AUTO);
>  
> -	for_each_plane_on_pipe(&data->display, data->pipe, data->plane) {
> +	if (igt_run_in_simulation())
>  		ret = test_plane(data);
> -		if (ret)
> -			break;
> +	else {
> +		for_each_plane_on_pipe(&data->display, data->pipe, data->plane) {
> +			ret = test_plane(data);
> +			if (ret)
> +				break;
> +		}

Could you simplify this? With the help of new var,
for example:

bool run_in_simulation = igt_run_in_simulation();

you could write:
		for_each_plane_on_pipe(&data->display, data->pipe, data->plane) {
			ret = test_plane(data);
			if (ret || run_in_simulation)
				break;
		}

Regards,
Kamil

>  	}
>  
>  	if (data->format == DRM_FORMAT_C8)
> -- 
> 2.34.1
>