[igt-dev] [PATCH i-g-t] tests/kms_plane: Test Refactoring

Modem, Bhanuprakash bhanuprakash.modem at intel.com
Tue Jun 14 06:14:48 UTC 2022


On Mon-13-06-2022 03:47 pm, Karthik B S wrote:
> Add igt_display_reset in test_init().
> Add new function to call all the subtests to avoid code duplication.
> 
> Signed-off-by: Karthik B S <karthik.b.s at intel.com>
> ---
>   tests/kms_plane.c | 117 ++++++++++++++++------------------------------
>   1 file changed, 40 insertions(+), 77 deletions(-)
> 
> diff --git a/tests/kms_plane.c b/tests/kms_plane.c
> index b1be44c3..b9a25762 100644
> --- a/tests/kms_plane.c
> +++ b/tests/kms_plane.c
> @@ -61,6 +61,7 @@ typedef struct {
>   	int num_colors;
>   	uint32_t crop;
>   	bool extended;
> +	unsigned int flags;
>   } data_t;
>   
>   static bool all_pipes;
> @@ -74,7 +75,9 @@ static color_t blue  = { 0.0f, 0.0f, 1.0f };
>    */
>   static void test_init(data_t *data, enum pipe pipe)
>   {
> +	igt_require(data->display.pipes[pipe].n_planes > 0);
>   	data->pipe_crc = igt_pipe_crc_new(data->drm_fd, pipe, INTEL_PIPE_CRC_SOURCE_AUTO);
> +	igt_display_reset(&data->display);
>   }
>   
>   static void test_fini(data_t *data)
> @@ -264,7 +267,7 @@ test_plane_position_with_output(data_t *data,
>   }
>   
>   static void
> -test_plane_position(data_t *data, enum pipe pipe, unsigned int flags)
> +test_plane_position(data_t *data, enum pipe pipe)
>   {
>   	int n_planes = data->display.pipes[pipe].n_planes;
>   	igt_output_t *output;
> @@ -274,12 +277,12 @@ test_plane_position(data_t *data, enum pipe pipe, unsigned int flags)
>   	igt_require(output);
>   
>   	test_init(data, pipe);
> -	test_grab_crc(data, output, pipe, &green, flags, &reference_crc);
> +	test_grab_crc(data, output, pipe, &green, data->flags, &reference_crc);
>   
>   	for (int plane = 1; plane < n_planes; plane++)
>   		test_plane_position_with_output(data, pipe, plane,
>   						output, &reference_crc,
> -						flags);
> +						data->flags);
>   
>   	test_fini(data);
>   }
> @@ -372,7 +375,7 @@ test_plane_panning_with_output(data_t *data,
>   }
>   
>   static void
> -test_plane_panning(data_t *data, enum pipe pipe, unsigned int flags)
> +test_plane_panning(data_t *data, enum pipe pipe)
>   {
>   	igt_output_t *output;
>   	igt_crc_t ref_crc;
> @@ -382,12 +385,12 @@ test_plane_panning(data_t *data, enum pipe pipe, unsigned int flags)
>   
>   	test_init(data, pipe);
>   
> -	if (flags & TEST_PANNING_TOP_LEFT)
> -		test_grab_crc(data, output, pipe, &red, flags, &ref_crc);
> +	if (data->flags & TEST_PANNING_TOP_LEFT)
> +		test_grab_crc(data, output, pipe, &red, data->flags, &ref_crc);
>   	else
> -		test_grab_crc(data, output, pipe, &blue, flags, &ref_crc);
> +		test_grab_crc(data, output, pipe, &blue, data->flags, &ref_crc);
>   
> -	test_plane_panning_with_output(data, pipe, output, &ref_crc, flags);
> +	test_plane_panning_with_output(data, pipe, output, &ref_crc, data->flags);
>   
>   	test_fini(data);
>   }
> @@ -1097,110 +1100,70 @@ static bool is_pipe_limit_reached(int count) {
>   	return count >= CRTC_RESTRICT_CNT && !all_pipes;
>   }
>   
> -static void
> -run_tests_for_pipe_plane(data_t *data)
> +static void run_test(data_t *data, void (*test)(data_t *, enum pipe))
>   {
>   	enum pipe pipe;
>   	int count;
> -	igt_fixture {
> -		igt_require_pipe(&data->display, pipe);
> -		igt_require(data->display.pipes[pipe].n_planes > 0);
> +	count = 0;

Nit: Initialize the variable count at declaration.

> +	for_each_pipe(&data->display, pipe) {
> +		igt_dynamic_f("pipe-%s-planes", kmstest_pipe_name(pipe))
> +			test(data, pipe);
> +
> +		if (is_pipe_limit_reached(++count))
> +			break;
>   	}
> +}
>   
> +static void
> +run_tests_for_pipe_plane(data_t *data)
> +{
>   	igt_describe("verify the pixel formats for given plane and pipe");
> -	igt_subtest_with_dynamic_f("pixel-format") {
> -		count = 0;
> -		for_each_pipe(&data->display, pipe) {
> -			igt_dynamic_f("pipe-%s-planes", kmstest_pipe_name(pipe))
> -				test_pixel_formats(data, pipe);
> -			if (is_pipe_limit_reached(++count))
> -				break;
> -		}
> -	}
> +	igt_subtest_with_dynamic_f("pixel-format")
> +		run_test(data, test_pixel_formats);

I didn't check all tests, but realized that in test_pixel_formats(), we 
need to call test_init() before using igt_output_set_pipe().

Can you please make sure the above point in all subtests?

> +
>   	igt_describe("verify the pixel formats for given plane and pipe with source clamping");
>   	igt_subtest_with_dynamic_f("pixel-format-source-clamping") {
> -		count = 0;
> -		for_each_pipe(&data->display, pipe) {
> -			igt_dynamic_f("pipe-%s-planes", kmstest_pipe_name(pipe)) {
> -				data->crop = 4;
> -				test_pixel_formats(data, pipe);
> -			}
> -			if (is_pipe_limit_reached(++count))
> -				break;
> -		}
> +		data->crop = 4;
> +		run_test(data, test_pixel_formats);
>   	}
>   
>   	data->crop = 0;
>   	igt_describe("verify plane position using two planes to create a fully covered screen");
>   	igt_subtest_with_dynamic_f("plane-position-covered") {
> -		count = 0;
> -		for_each_pipe(&data->display, pipe) {
> -			igt_dynamic_f("pipe-%s-planes", kmstest_pipe_name(pipe))
> -				test_plane_position(data, pipe, 0);
> -			if (is_pipe_limit_reached(++count))
> -				break;
> -		}
> +		data->flags = 0;
> +		run_test(data, test_plane_position);
>   	}
>   
>   	igt_describe("verify plane position using two planes to create a partially covered screen");
>   	igt_subtest_with_dynamic_f("plane-position-hole") {
> -		count = 0;
> -		for_each_pipe(&data->display, pipe) {
> -			igt_dynamic_f("pipe-%s-planes", kmstest_pipe_name(pipe))
> -				test_plane_position(data, pipe,
> -				TEST_POSITION_PARTIALLY_COVERED);
> -			if (is_pipe_limit_reached(++count))
> -				break;
> -		}
> +		data->flags = TEST_POSITION_PARTIALLY_COVERED;

Do we need to clear these flags? Will it effect if we run these subtests 
in reverse order?

Overall, this patch looks good to me.

- Bhanu

> +		run_test(data, test_plane_position);
>   	}
>   
>   	igt_describe("verify plane position using two planes to create a partially covered screen and"
>   		       "check for DPMS");
>   	igt_subtest_with_dynamic_f("plane-position-hole-dpms") {
> -		count = 0;
> -		for_each_pipe(&data->display, pipe) {
> -			igt_dynamic_f("pipe-%s-planes", kmstest_pipe_name(pipe))
> -				test_plane_position(data, pipe,
> -				TEST_POSITION_PARTIALLY_COVERED | TEST_DPMS);
> -			if (is_pipe_limit_reached(++count))
> -				break;
> -		}
> +		data->flags = TEST_POSITION_PARTIALLY_COVERED | TEST_DPMS;
> +		run_test(data, test_plane_position);
>   	}
>   
>   	igt_describe("verify plane panning at top-left position using primary plane");
>   	igt_subtest_with_dynamic_f("plane-panning-top-left") {
> -		count = 0;
> -		for_each_pipe(&data->display, pipe) {
> -			igt_dynamic_f("pipe-%s-planes", kmstest_pipe_name(pipe))
> -				test_plane_panning(data, pipe, TEST_PANNING_TOP_LEFT);
> -			if (is_pipe_limit_reached(++count))
> -				break;
> -		}
> +		data->flags = TEST_PANNING_TOP_LEFT;
> +		run_test(data, test_plane_panning);
>   	}
>   
>   	igt_describe("verify plane panning at bottom-right position using primary plane");
>   	igt_subtest_with_dynamic_f("plane-panning-bottom-right") {
> -		count = 0;
> -		for_each_pipe(&data->display, pipe) {
> -			igt_dynamic_f("pipe-%s-planes", kmstest_pipe_name(pipe))
> -				test_plane_panning(data, pipe, TEST_PANNING_BOTTOM_RIGHT);
> -			if (is_pipe_limit_reached(++count))
> -				break;
> -		}
> +		data->flags = TEST_PANNING_BOTTOM_RIGHT;
> +		run_test(data, test_plane_panning);
>   	}
>   
>   	igt_describe("verify plane panning at bottom-right position using primary plane and executes system"
>   		       "suspend cycles");
>   	igt_subtest_with_dynamic_f("plane-panning-bottom-right-suspend") {
> -		count = 0;
> -		for_each_pipe(&data->display, pipe) {
> -			igt_dynamic_f("pipe-%s-planes", kmstest_pipe_name(pipe))
> -				test_plane_panning(data, pipe,
> -				TEST_PANNING_BOTTOM_RIGHT |
> -				TEST_SUSPEND_RESUME);
> -			if (is_pipe_limit_reached(++count))
> -				break;
> -		}
> +		data->flags = TEST_PANNING_BOTTOM_RIGHT | TEST_SUSPEND_RESUME;
> +		run_test(data, test_plane_panning);
>   	}
>   }
>   



More information about the igt-dev mailing list