[igt-dev] [PATCH i-g-t] tests/kms_plane_scaling: add invalid parameter tests

[Kamil Konieczny]

Hi Juha-Pekka,

I have few nits, see below.

On 2023-02-09 at 13:51:51 +0200, Juha-Pekka Heikkila wrote:
> Add skeleton for adding invalid parameter tests and add
> two tests which are expected to return -EINVAL or -ERANGE
> 
> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila at gmail.com>
> ---
>  tests/kms_plane_scaling.c | 71 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 71 insertions(+)
> 
> diff --git a/tests/kms_plane_scaling.c b/tests/kms_plane_scaling.c
> index 887a55e63..30a839f15 100644
> --- a/tests/kms_plane_scaling.c
> +++ b/tests/kms_plane_scaling.c
> @@ -819,6 +819,75 @@ static void test_scaler_with_multi_pipe_plane(data_t *d)
>  	igt_assert_eq(ret1 && ret2, 0);
>  }
>  
> +static void invalid_parameter_tests(data_t *d) {
------------------------------------------------- ^
This brace should be at new line.

> +	enum pipe pipe = PIPE_A;
> +	igt_output_t *output;
> +	igt_fb_t fb;
> +	igt_plane_t* plane;
------------------ ^
This should be:
	igt_plane_t *plane;

> +	int rval;
> +
> +	const struct {
> +		const char* testname;
------------------------- ^
Same here.

> +		uint32_t planesize[2];
> +		struct {
> +			enum igt_atomic_plane_properties prop;
> +			uint32_t value;
> +		} params[8];
> +	} paramtests[] = {
> +		{
> +			.testname = "less-than-1-height-src",
> +			.planesize = {256, 8},
> +			.params = {{IGT_PLANE_SRC_H, IGT_FIXED(0, 30) }, {~0}}
> +		},
> +		{
> +			.testname = "less-than-1-width-src",
> +			.planesize = {8, 256},
> +			.params = {{IGT_PLANE_SRC_W, IGT_FIXED(0, 30) }, {~0}}
> +		},
> +	};
> +
> +	igt_fixture {
> +		output = igt_get_single_output_for_pipe(&d->display, pipe);
> +		igt_require(output);
> +
> +		igt_output_set_pipe(output, pipe);
> +		plane = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
> +
> +		igt_create_fb(d->drm_fd, 256, 256,
> +			DRM_FORMAT_XRGB8888,
> +			DRM_FORMAT_MOD_NONE,
> +			&fb);
> +	}
> +
> +	igt_describe("test parameters which should not be accepted");
> +	igt_subtest_with_dynamic("invalid-parameters") {
> +		for (uint32_t i = 0; i < ARRAY_SIZE(paramtests); i++) {
> +			igt_dynamic(paramtests[i].testname) {
> +				igt_plane_set_position(plane, 0, 0);
> +				igt_plane_set_fb(plane, &fb);
> +				igt_plane_set_size(plane,
-------------------------------------------------- ^
If you break line you should align it here.

> +							paramtests[i].planesize[0],
-------------------------------------------------- ^
Align it.

> +							paramtests[i].planesize[1]);
-------------------------------------------------- ^
This is correct align.

> +
> +
> +				for (uint32_t j = 0; paramtests[i].params[j].prop != ~0; j++)
> +					igt_plane_set_prop_value(plane,
---------------------------------------------------------------- ^

> +									paramtests[i].params[j].prop,
---------------------------------------------------------------- ^
Align here.

> +									paramtests[i].params[j].value);
---------------------------------------------------------------- ^
Align here.

> +
> +				rval = 	igt_display_try_commit2(&d->display, COMMIT_ATOMIC);
-- ^
Use tabs, no spaces.

Please use checkpatch.pl from Linux kernel source to check
your patches before sending. It may change in future and
maybe we will have it in our GitLab CI and in patchwork but
for now please do it yourself.

Regards,
Kamil

> +
> +				igt_assert(rval == -EINVAL || rval == -ERANGE);
> +			}
> +		}
> +	}
> +
> +	igt_fixture {
> +		igt_remove_fb(d->drm_fd, &fb);
> +		igt_output_set_pipe(output, PIPE_NONE);
> +	}
> +}
> +
>  static int opt_handler(int opt, int opt_index, void *_data)
>  {
>  	data_t *data = _data;
> @@ -982,6 +1051,8 @@ igt_main_args("", long_opts, help_str, opt_handler, &data)
>  	igt_subtest_f("2x-scaler-multi-pipe")
>  		test_scaler_with_multi_pipe_plane(&data);
>  
> +	invalid_parameter_tests(&data);
> +
>  	igt_fixture {
>  		igt_display_fini(&data.display);
>  		close(data.drm_fd);
> -- 
> 2.39.0
>