[igt-dev] [PATCH i-g-t v2] tests/kms : Added dynamic test cases

[Gupta, Nidhi1]

-----Original Message-----
From: Modem, Bhanuprakash <bhanuprakash.modem at intel.com> 
Sent: Monday, April 18, 2022 4:24 PM
To: Gupta, Nidhi1 <nidhi1.gupta at intel.com>; igt-dev at lists.freedesktop.org
Subject: Re: [PATCH i-g-t v2] tests/kms : Added dynamic test cases

On Mon-18-04-2022 01:23 pm, Nidhi Gupta wrote:
> Modified kms_sequence and kms_concurrent to include dynamic test 
> cases.
> 
> Signed-off-by: Nidhi Gupta <nidhi1.gupta at intel.com>
> ---
>   tests/kms_concurrent.c |  6 ++++--
>   tests/kms_sequence.c   | 13 ++++++++++---
>   2 files changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/tests/kms_concurrent.c b/tests/kms_concurrent.c index 
> dc1e36f2..5536a369 100644
> --- a/tests/kms_concurrent.c
> +++ b/tests/kms_concurrent.c
> @@ -339,9 +339,11 @@ run_tests_for_pipe(data_t *data, enum pipe pipe)
>   
>   	igt_describe("Test atomic mode setting concurrently with multiple planes and screen "
>   		     "resolution.");
> -	igt_subtest_f("pipe-%s", kmstest_pipe_name(pipe))
> +	igt_subtest_with_dynamic_f("pipe-%s", kmstest_pipe_name(pipe)) {
>   		for_each_valid_output_on_pipe(&data->display, pipe, output)
> -			run_test(data, pipe, output);
> +			igt_dynamic_f("%s", igt_output_name(output))
> +				run_test(data, pipe, output);

You need to remove for_each_valid_output_on_pipe() logic from run_test(), since we already doing here the same.
I already added this in IGT cleanup patch, to avoid the confusion I send a new series with 2 patches, one IGT cleanup and other dynamic tests

> +	}
>   }
>   
>   static int opt_handler(int option, int option_index, void *input) 
> diff --git a/tests/kms_sequence.c b/tests/kms_sequence.c index 
> 9c287480..aeabce23 100644
> --- a/tests/kms_sequence.c
> +++ b/tests/kms_sequence.c
> @@ -253,6 +253,7 @@ static void sequence_queue(data_t *data, int fd, int nchildren)
>   igt_main
>   {
>   	int fd;
> +	igt_output_t *output;
>   	data_t data;
>   	const struct {
>   		const char *name;
> @@ -278,6 +279,7 @@ igt_main
>   		fd = drm_open_driver_master(DRIVER_ANY);
>   		kmstest_set_vt_graphics_mode();
>   		igt_display_require(&data.display, fd);
> +		igt_display_require_output(&data.display);
>   	}
>   
>   	for (f = funcs; f->name; f++) {
> @@ -287,9 +289,14 @@ igt_main
>   
>   			igt_describe("This is a test of drmCrtcGetSequence and "
>   				     "drmCrtcQueueSequence");
> -			igt_subtest_f("%s-%s", f->name, m->name) {
> -				data.flags = m->flags;
> -				run_test(&data, fd, f->func);
> +			igt_subtest_with_dynamic_f("%s-%s", f->name, m->name) {
> +				for_each_connected_output(&data.display, output) {
> +					igt_dynamic_f("%s", igt_output_name(output)) {
> +						data.output = output;

We are no where using this "data.output".

Actually, run_test() is iterating for_each_pipe_with_valid_output() to get valid pipe/connector combination.

If you want to create dynamic subtests with pipe/connector config, you need to iterate for_each_pipe_with_valid_output() before calling
igt_dynamic() and remove the same logic from run_test().

- Bhanu

> +						data.flags = m->flags;
> +						run_test(&data, fd, f->func);
> +					}
> +				}
>   			}
>   		}
>   	}