[PATCH i-g-t 5/5] tests/kms: Use lib helper to check the pipe/output combo validity

[B, Jeevan]

> -----Original Message-----
> From: igt-dev <igt-dev-bounces at lists.freedesktop.org> On Behalf Of
> Bhanuprakash Modem
> Sent: Monday, April 8, 2024 11:42 AM
> To: igt-dev at lists.freedesktop.org
> Cc: Modem, Bhanuprakash <bhanuprakash.modem at intel.com>
> Subject: [PATCH i-g-t 5/5] tests/kms: Use lib helper to check the pipe/output
> combo validity
> 
> Use IGT lib helper intel_pipe_output_combo_valid() to make sure the selected
> pipe/output/mode combo is valid.
> 
> Signed-off-by: Bhanuprakash Modem <bhanuprakash.modem at intel.com>
> ---
>  tests/kms_dither.c | 24 +++++++------
>  tests/kms_hdr.c    | 90 ++++++++++++++++++++++++++--------------------
>  2 files changed, 65 insertions(+), 49 deletions(-)
> 
> diff --git a/tests/kms_dither.c b/tests/kms_dither.c index af7b7f05c..21b1210b4
> 100644
> --- a/tests/kms_dither.c
> +++ b/tests/kms_dither.c
> @@ -76,13 +76,10 @@ typedef struct {
>  /* Prepare test data. */
>  static void prepare_test(data_t *data, igt_output_t *output, enum pipe p)  {
> -	igt_display_t *display = &data->display;
>  	igt_pipe_t *pipe = &data->display.pipes[p];
> 
>  	igt_assert(pipe);
> 
> -	igt_display_reset(display);
> -
>  	data->primary =
>  		igt_pipe_get_plane_type(pipe, DRM_PLANE_TYPE_PRIMARY);
> 
> @@ -205,6 +202,8 @@ run_dither_test(data_t *data, int fb_bpc, int fb_format,
> int output_bpc)
>  	igt_output_t *output;
>  	igt_display_t *display = &data->display;
> 
> +	igt_display_reset(display);
> +
>  	for_each_connected_output(display, output) {
>  		enum pipe pipe;
> 
> @@ -215,15 +214,20 @@ run_dither_test(data_t *data, int fb_bpc, int
> fb_format, int output_bpc)
>  			continue;
> 
>  		for_each_pipe(display, pipe) {
> -			if (igt_pipe_connector_valid(pipe, output)) {
> -				igt_dynamic_f("pipe-%s-%s",
> -					      kmstest_pipe_name(pipe), output-
> >name)
> -					test_dithering(data, pipe, output,
> fb_bpc,
> -							fb_format,
> output_bpc);
> +			igt_output_set_pipe(output, pipe);
> 
> -				/* One pipe is enough */
> -				break;
> +			if (!intel_pipe_output_combo_valid(display)) {
> +				igt_output_set_pipe(output, PIPE_NONE);
> +				continue;
>  			}
> +
> +			igt_dynamic_f("pipe-%s-%s",
> +					      kmstest_pipe_name(pipe), output-
> >name)
> +				test_dithering(data, pipe, output, fb_bpc,
> +							   fb_format,
> output_bpc);
> +
> +			/* One pipe is enough */
> +			break;
>  		}
>  	}
>  }

Overall patch looks fine, can we split these to 2 patches to simplify. 

> diff --git a/tests/kms_hdr.c b/tests/kms_hdr.c index 99b78f7ef..475463c21
> 100644
> --- a/tests/kms_hdr.c
> +++ b/tests/kms_hdr.c
> @@ -261,6 +261,8 @@ static void test_bpc_switch(data_t *data, uint32_t flags)
>  	igt_display_t *display = &data->display;
>  	igt_output_t *output;
> 
> +	igt_display_reset(display);
> +
>  	for_each_connected_output(display, output) {
>  		enum pipe pipe;
> 
> @@ -271,26 +273,30 @@ static void test_bpc_switch(data_t *data, uint32_t
> flags)
>  			continue;
> 
>  		for_each_pipe(display, pipe) {
> -			if (igt_pipe_connector_valid(pipe, output)) {
> -				prepare_test(data, output, pipe);
> -
> -				if (is_intel_device(data->fd) &&
> -				    !igt_max_bpc_constraint(display, pipe, output,
> 10)) {
> -					test_fini(data);
> -					break;
> -				}
> -
> -				data->mode = igt_output_get_mode(output);
> -				data->w = data->mode->hdisplay;
> -				data->h = data->mode->vdisplay;
> +			igt_output_set_pipe(output, pipe);
> +			if (intel_pipe_output_combo_valid(display)) {
> +				igt_output_set_pipe(output, PIPE_NONE);
> +				continue;
> +			}
> 
> -				igt_dynamic_f("pipe-%s-%s",
> -					      kmstest_pipe_name(pipe), output-
> >name)
> -					test_bpc_switch_on_output(data, pipe,
> output, flags);
> +			prepare_test(data, output, pipe);
> 
> -				/* One pipe is enough */
> +			if (is_intel_device(data->fd) &&
> +			    !igt_max_bpc_constraint(display, pipe, output, 10)) {
> +				test_fini(data);
>  				break;
>  			}
> +
> +			data->mode = igt_output_get_mode(output);
> +			data->w = data->mode->hdisplay;
> +			data->h = data->mode->vdisplay;
> +
> +			igt_dynamic_f("pipe-%s-%s",
> +				      kmstest_pipe_name(pipe), output->name)
> +				test_bpc_switch_on_output(data, pipe, output,
> flags);
> +
> +			/* One pipe is enough */
> +			break;
>  		}
>  	}
>  }
> @@ -614,6 +620,8 @@ static void test_hdr(data_t *data, uint32_t flags)
>  	igt_display_t *display = &data->display;
>  	igt_output_t *output;
> 
> +	igt_display_reset(display);
> +
>  	for_each_connected_output(display, output) {
>  		enum pipe pipe;
> 
> @@ -635,32 +643,36 @@ static void test_hdr(data_t *data, uint32_t flags)
>  			continue;
> 
>  		for_each_pipe(display, pipe) {
> -			if (igt_pipe_connector_valid(pipe, output)) {
> -				prepare_test(data, output, pipe);
> +			igt_output_set_pipe(output, pipe);
> +			if (!intel_pipe_output_combo_valid(display)) {
> +				igt_output_set_pipe(output, PIPE_NONE);
> +				continue;
> +			}
> 
> -				if (is_intel_device(data->fd) &&
> -				    !igt_max_bpc_constraint(display, pipe, output,
> 10)) {
> -					test_fini(data);
> -					break;
> -				}
> -
> -				data->mode = igt_output_get_mode(output);
> -				data->w = data->mode->hdisplay;
> -				data->h = data->mode->vdisplay;
> -
> -				igt_dynamic_f("pipe-%s-%s",
> -					      kmstest_pipe_name(pipe), output-
> >name) {
> -					if (flags & (TEST_NONE | TEST_DPMS |
> TEST_SUSPEND | TEST_INVALID_HDR))
> -						test_static_toggle(data, pipe,
> output, flags);
> -					if (flags & TEST_SWAP)
> -						test_static_swap(data, pipe,
> output);
> -					if (flags &
> TEST_INVALID_METADATA_SIZES)
> -
> 	test_invalid_metadata_sizes(data, output);
> -				}
> -
> -				/* One pipe is enough */
> +			prepare_test(data, output, pipe);
> +
> +			if (is_intel_device(data->fd) &&
> +			    !igt_max_bpc_constraint(display, pipe, output, 10)) {
> +				test_fini(data);
>  				break;
>  			}
> +
> +			data->mode = igt_output_get_mode(output);
> +			data->w = data->mode->hdisplay;
> +			data->h = data->mode->vdisplay;
> +
> +			igt_dynamic_f("pipe-%s-%s",
> +				      kmstest_pipe_name(pipe), output->name) {
> +				if (flags & (TEST_NONE | TEST_DPMS |
> TEST_SUSPEND | TEST_INVALID_HDR))
> +					test_static_toggle(data, pipe, output,
> flags);
> +				if (flags & TEST_SWAP)
> +					test_static_swap(data, pipe, output);
> +				if (flags & TEST_INVALID_METADATA_SIZES)
> +					test_invalid_metadata_sizes(data,
> output);
> +			}
> +
> +			/* One pipe is enough */
> +			break;
>  		}
>  	}
>  }
> --
> 2.43.2