Re: [igt-dev] [PATCH i-g-t] tests/kms_plane_multiple: Before going testing check how many planes are allowed

["Souza, Jose" <jose.souza@intel.com>]

[-- Attachment #1.1: Type: text/plain, Size: 3153 bytes --]

On Tue, 2019-04-02 at 17:42 +0300, Juha-Pekka Heikkila wrote:
> Before start testing try out how many planes kernel will allow
> simultaneously to be used.
> 
> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
> ---
>  tests/kms_plane_multiple.c | 32 ++++++++++++++++++++++++++------
>  1 file changed, 26 insertions(+), 6 deletions(-)
> 
> diff --git a/tests/kms_plane_multiple.c b/tests/kms_plane_multiple.c
> index bfaeede..5107d9d 100644
> --- a/tests/kms_plane_multiple.c
> +++ b/tests/kms_plane_multiple.c
> @@ -260,7 +260,9 @@ test_plane_position_with_output(data_t *data,
> enum pipe pipe,
>  {
>  	color_t blue  = { 0.0f, 0.0f, 1.0f };
>  	igt_crc_t crc;
> +	igt_plane_t *plane;
>  	int i;
> +	int err, c = 0;
>  	int iterations = opt.iterations < 1 ? 1 : opt.iterations;
>  	bool loop_forever;
>  	char info[256];
> @@ -274,17 +276,35 @@ test_plane_position_with_output(data_t *data,
> enum pipe pipe,
>  			iterations, iterations > 1 ? "iterations" :
> "iteration");
>  	}
>  
> -	igt_info("Testing connector %s using pipe %s with %d planes %s
> with seed %d\n",
> -		 igt_output_name(output), kmstest_pipe_name(pipe),
> n_planes,
> -		 info, opt.seed);
> -
>  	test_init(data, pipe, n_planes);
>  
>  	test_grab_crc(data, output, pipe, &blue, tiling);
>  
> +	/*
> +	 * Find out how many planes are allowed simultaneously
> +	 */
> +	do {
> +		c++;
> +		prepare_planes(data, pipe, &blue, tiling, c, output);
> +		err = igt_display_try_commit2(&data->display,
> COMMIT_ATOMIC);
> +	} while (!err && c < n_planes);
> +
> +	if(err)
> +		c--;

"do {} while()" does the job but "for()" would be better suitable:

for (planes_allowed = 0; planes_allowed < n_planes; planes_allowed++) {
	int err;

	prepare_planes(data, pipe, &blue, tiling, planes_allowed,
output);
	err = igt_display_try_commit2(&data->display, COMMIT_ATOMIC);
	if (err)
		break;
}


Also you are leaking a bunch o memory here, each call to
prepare_planes() will leak max_planes framebuffers, this could cause
test to fail due no GPU memory available.

Actually this test is already leaking a bunch as test_fini() is only
freeing the first framebuffer, there is other leaks here too like:
data->fb, prepare_planes().x, prepare_planes().y and
prepare_planes().size.

Can you take care of that too?


> +
> +	/*
> +	 * clean up failed state.
> +	 */
> +	for_each_plane_on_pipe(&data->display, pipe, plane)
> +		igt_plane_set_fb(plane, NULL);
> +
> +	igt_info("Testing connector %s using pipe %s with %d planes %s
> with seed %d\n",
> +		 igt_output_name(output), kmstest_pipe_name(pipe), c,
> +		 info, opt.seed);
> +
>  	i = 0;
> -	while (i < iterations || loop_forever) {
> -		prepare_planes(data, pipe, &blue, tiling, n_planes,
> output);
> +	while ((i < iterations || loop_forever) && c > 0) {

We actually want to fail the test if it can not even test one plane, so
add this before this while()

igt_assert_lt(0, planes_allowed);

> +		prepare_planes(data, pipe, &blue, tiling, c, output);
>  
>  		igt_display_commit2(&data->display, COMMIT_ATOMIC);
>  

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 153 bytes --]

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