[igt-dev] [PATCH i-g-t 0/2] tests/kms_invalid_mode: Test Cleanup

[igt-dev] [PATCH i-g-t 0/2] tests/kms_invalid_mode: Test Cleanup

[Nidhi Gupta]

Convert all possible subtests to dynamic and sanitize the system
state before starting/exiting the subtest.

Nidhi Gupta (2):
  tests/kms_invalid_mode: Convert tests to dynamic
  tests/kms_invalid_mode: Test Cleanup

 tests/kms_invalid_mode.c | 74 ++++++++++++++--------------------------
 1 file changed, 26 insertions(+), 48 deletions(-)

-- 
2.26.2

[igt-dev] [PATCH i-g-t 1/2] tests/kms_invalid_mode: Convert tests to dynamic

[Nidhi Gupta]

Convert the existing subtests to dynamic subtests at pipe level.

Signed-off-by: Nidhi Gupta <nidhi1.gupta@intel.com>
Reviewed-by: Petri Latvala <petri.latvala@intel.com>
---
 tests/kms_invalid_mode.c | 54 ++++++++++++++++++----------------------
 1 file changed, 24 insertions(+), 30 deletions(-)

diff --git a/tests/kms_invalid_mode.c b/tests/kms_invalid_mode.c
index 630798d8..b79fc78a 100644
--- a/tests/kms_invalid_mode.c
+++ b/tests/kms_invalid_mode.c
@@ -32,6 +32,7 @@ typedef struct _data data_t;
 
 struct _data {
 	int drm_fd;
+	enum pipe pipe;
 	igt_display_t display;
 	igt_output_t *output;
 	drmModeResPtr res;
@@ -177,21 +178,21 @@ adjust_mode_bad_vtotal(data_t *data, drmModeModeInfoPtr mode)
 	return true;
 }
 
-static int
+static void
 test_output(data_t *data)
 {
 	igt_output_t *output = data->output;
 	drmModeModeInfo mode;
 	struct igt_fb fb;
-	int i;
+	int ret;
+	uint32_t crtc_id;
 
 	/*
 	 * FIXME test every mode we have to be more
 	 * sure everything is really getting rejected?
 	 */
 	mode = *igt_output_get_mode(output);
-	if (!data->adjust_mode(data, &mode))
-		return 0;
+	igt_require(data->adjust_mode(data, &mode));
 
 	igt_create_fb(data->drm_fd,
 		      max_t(uint16_t, mode.hdisplay, 64),
@@ -202,32 +203,14 @@ test_output(data_t *data)
 
 	kmstest_unset_all_crtcs(data->drm_fd, data->res);
 
-	for (i = 0; i < data->res->count_crtcs; i++) {
-		int ret;
-
-		igt_info("Checking pipe %c connector %s with mode %s\n",
-			 'A'+i, output->name, mode.name);
+	crtc_id = data->display.pipes[data->pipe].crtc_id;
 
-		ret = drmModeSetCrtc(data->drm_fd, data->res->crtcs[i],
-				     fb.fb_id, 0, 0,
-				     &output->id, 1, &mode);
-		igt_assert_lt(ret, 0);
-	}
+	ret = drmModeSetCrtc(data->drm_fd, crtc_id,
+			     fb.fb_id, 0, 0,
+			     &output->id, 1, &mode);
+	igt_assert_lt(ret, 0);
 
 	igt_remove_fb(data->drm_fd, &fb);
-
-	return 1;
-}
-
-static void test(data_t *data)
-{
-	int valid_connectors = 0;
-
-	for_each_connected_output(&data->display, data->output) {
-		valid_connectors += test_output(data);
-	}
-
-	igt_require_f(valid_connectors, "No suitable connectors found\n");
 }
 
 static int i915_max_dotclock(data_t *data)
@@ -297,6 +280,10 @@ static data_t data;
 
 igt_main
 {
+
+	enum pipe pipe;
+	igt_output_t *output;
+
 	igt_fixture {
 		data.drm_fd = drm_open_driver_master(DRIVER_ANY);
 
@@ -310,10 +297,17 @@ igt_main
 		igt_info("Max dotclock: %d kHz\n", data.max_dotclock);
 	}
 
+	igt_describe("Test all modesets are rejected when the requested mode is invalid");
 	for (int i = 0; i < ARRAY_SIZE(subtests); i++) {
-		igt_subtest(subtests[i].name) {
-			data.adjust_mode = subtests[i].adjust_mode;
-			test(&data);
+		igt_subtest_with_dynamic(subtests[i].name) {
+			for_each_pipe_with_valid_output(&data.display, pipe, output) {
+				igt_dynamic_f("%s-pipe-%s", igt_output_name(output), kmstest_pipe_name(pipe)) {
+					data.output = output;
+					data.pipe = pipe;
+					data.adjust_mode = subtests[i].adjust_mode;
+					test_output(&data);
+				}
+			}
 		}
 	}
 
-- 
2.26.2

Re: [igt-dev] [PATCH i-g-t 1/2] tests/kms_invalid_mode: Convert tests to dynamic

[Modem, Bhanuprakash]

On Mon-27-06-2022 12:37 pm, Nidhi Gupta wrote:
> Convert the existing subtests to dynamic subtests at pipe level.
> 
> Signed-off-by: Nidhi Gupta <nidhi1.gupta@intel.com>
> Reviewed-by: Petri Latvala <petri.latvala@intel.com>
> ---
>   tests/kms_invalid_mode.c | 54 ++++++++++++++++++----------------------
>   1 file changed, 24 insertions(+), 30 deletions(-)
> 
> diff --git a/tests/kms_invalid_mode.c b/tests/kms_invalid_mode.c
> index 630798d8..b79fc78a 100644
> --- a/tests/kms_invalid_mode.c
> +++ b/tests/kms_invalid_mode.c
> @@ -32,6 +32,7 @@ typedef struct _data data_t;
>   
>   struct _data {
>   	int drm_fd;
> +	enum pipe pipe;
>   	igt_display_t display;
>   	igt_output_t *output;
>   	drmModeResPtr res;
> @@ -177,21 +178,21 @@ adjust_mode_bad_vtotal(data_t *data, drmModeModeInfoPtr mode)
>   	return true;
>   }
>   
> -static int
> +static void
>   test_output(data_t *data)
>   {
>   	igt_output_t *output = data->output;
>   	drmModeModeInfo mode;
>   	struct igt_fb fb;
> -	int i;
> +	int ret;
> +	uint32_t crtc_id;
>   
>   	/*
>   	 * FIXME test every mode we have to be more
>   	 * sure everything is really getting rejected?
>   	 */
>   	mode = *igt_output_get_mode(output);
> -	if (!data->adjust_mode(data, &mode))
> -		return 0;
> +	igt_require(data->adjust_mode(data, &mode));
>   
>   	igt_create_fb(data->drm_fd,
>   		      max_t(uint16_t, mode.hdisplay, 64),
> @@ -202,32 +203,14 @@ test_output(data_t *data)
>   
>   	kmstest_unset_all_crtcs(data->drm_fd, data->res);
>   
> -	for (i = 0; i < data->res->count_crtcs; i++) {
> -		int ret;
> -
> -		igt_info("Checking pipe %c connector %s with mode %s\n",
> -			 'A'+i, output->name, mode.name);
> +	crtc_id = data->display.pipes[data->pipe].crtc_id;
>   
> -		ret = drmModeSetCrtc(data->drm_fd, data->res->crtcs[i],
> -				     fb.fb_id, 0, 0,
> -				     &output->id, 1, &mode);
> -		igt_assert_lt(ret, 0);
> -	}
> +	ret = drmModeSetCrtc(data->drm_fd, crtc_id,
> +			     fb.fb_id, 0, 0,
> +			     &output->id, 1, &mode);
> +	igt_assert_lt(ret, 0);
>   
>   	igt_remove_fb(data->drm_fd, &fb);
> -
> -	return 1;
> -}
> -
> -static void test(data_t *data)
> -{
> -	int valid_connectors = 0;
> -
> -	for_each_connected_output(&data->display, data->output) {
> -		valid_connectors += test_output(data);
> -	}
> -
> -	igt_require_f(valid_connectors, "No suitable connectors found\n");
>   }
>   
>   static int i915_max_dotclock(data_t *data)
> @@ -297,6 +280,10 @@ static data_t data;
>   
>   igt_main
>   {
> +
> +	enum pipe pipe;
> +	igt_output_t *output;
> +
>   	igt_fixture {
>   		data.drm_fd = drm_open_driver_master(DRIVER_ANY);
>   
> @@ -310,10 +297,17 @@ igt_main
>   		igt_info("Max dotclock: %d kHz\n", data.max_dotclock);
>   	}
>   
> +	igt_describe("Test all modesets are rejected when the requested mode is invalid");

Can we update like "Make sure that the commit must be rejected when the 
requested mode is invalid." ?

Reviewed-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>

- Bhanu

>   	for (int i = 0; i < ARRAY_SIZE(subtests); i++) {
> -		igt_subtest(subtests[i].name) {
> -			data.adjust_mode = subtests[i].adjust_mode;
> -			test(&data);
> +		igt_subtest_with_dynamic(subtests[i].name) {
> +			for_each_pipe_with_valid_output(&data.display, pipe, output) {
> +				igt_dynamic_f("%s-pipe-%s", igt_output_name(output), kmstest_pipe_name(pipe)) {
> +					data.output = output;
> +					data.pipe = pipe;
> +					data.adjust_mode = subtests[i].adjust_mode;
> +					test_output(&data);
> +				}
> +			}
>   		}
>   	}
>   

[igt-dev] [PATCH i-g-t 2/2] tests/kms_invalid_mode: Test Cleanup

[Nidhi Gupta]

Sanitize the system state before starting the subtest.

Signed-off-by: Nidhi Gupta <nidhi1.gupta@intel.com>
---
 tests/kms_invalid_mode.c | 38 +++++++++++---------------------------
 1 file changed, 11 insertions(+), 27 deletions(-)

diff --git a/tests/kms_invalid_mode.c b/tests/kms_invalid_mode.c
index b79fc78a..0d76adc1 100644
--- a/tests/kms_invalid_mode.c
+++ b/tests/kms_invalid_mode.c
@@ -35,7 +35,6 @@ struct _data {
 	enum pipe pipe;
 	igt_display_t display;
 	igt_output_t *output;
-	drmModeResPtr res;
 	int max_dotclock;
 	bool (*adjust_mode)(data_t *data, drmModeModeInfoPtr mode);
 };
@@ -182,35 +181,23 @@ static void
 test_output(data_t *data)
 {
 	igt_output_t *output = data->output;
-	drmModeModeInfo mode;
-	struct igt_fb fb;
-	int ret;
-	uint32_t crtc_id;
+	drmModeConnector *connector = output->config.connector;
 
+	igt_display_reset(&data->display);
+	igt_output_set_pipe(output, data->pipe);
 	/*
 	 * FIXME test every mode we have to be more
-	 * sure everything is really getting rejected?
+	 e sure everything is really getting rejected?
 	 */
-	mode = *igt_output_get_mode(output);
-	igt_require(data->adjust_mode(data, &mode));
+	igt_sort_connector_modes(connector, sort_drm_modes_by_res_dsc);
 
-	igt_create_fb(data->drm_fd,
-		      max_t(uint16_t, mode.hdisplay, 64),
-		      max_t(uint16_t, mode.vdisplay, 64),
-		      DRM_FORMAT_XRGB8888,
-		      DRM_FORMAT_MOD_LINEAR,
-		      &fb);
-
-	kmstest_unset_all_crtcs(data->drm_fd, data->res);
-
-	crtc_id = data->display.pipes[data->pipe].crtc_id;
-
-	ret = drmModeSetCrtc(data->drm_fd, crtc_id,
-			     fb.fb_id, 0, 0,
-			     &output->id, 1, &mode);
-	igt_assert_lt(ret, 0);
+	for_each_connector_mode(output) {
+		igt_output_override_mode(output, &connector->modes[j__]);
+		igt_require(data->adjust_mode(data, &connector->modes[j__]));
+		igt_display_commit2(&data->display, &data->display.is_atomic ? COMMIT_ATOMIC : COMMIT_LEGACY);
+	}
 
-	igt_remove_fb(data->drm_fd, &fb);
+	igt_output_override_mode(output, NULL);
 }
 
 static int i915_max_dotclock(data_t *data)
@@ -290,8 +277,6 @@ igt_main
 		kmstest_set_vt_graphics_mode();
 
 		igt_display_require(&data.display, data.drm_fd);
-		data.res = drmModeGetResources(data.drm_fd);
-		igt_assert(data.res);
 
 		data.max_dotclock = i915_max_dotclock(&data);
 		igt_info("Max dotclock: %d kHz\n", data.max_dotclock);
@@ -314,6 +299,5 @@ igt_main
 	igt_fixture {
 		igt_display_fini(&data.display);
 		igt_reset_connectors();
-		drmModeFreeResources(data.res);
 	}
 }
-- 
2.26.2

Re: [igt-dev] [PATCH i-g-t 2/2] tests/kms_invalid_mode: Test Cleanup

[Modem, Bhanuprakash]

On Mon-27-06-2022 12:37 pm, Nidhi Gupta wrote:
> Sanitize the system state before starting the subtest.
> 
> Signed-off-by: Nidhi Gupta <nidhi1.gupta@intel.com>
> ---
>   tests/kms_invalid_mode.c | 38 +++++++++++---------------------------
>   1 file changed, 11 insertions(+), 27 deletions(-)
> 
> diff --git a/tests/kms_invalid_mode.c b/tests/kms_invalid_mode.c
> index b79fc78a..0d76adc1 100644
> --- a/tests/kms_invalid_mode.c
> +++ b/tests/kms_invalid_mode.c
> @@ -35,7 +35,6 @@ struct _data {
>   	enum pipe pipe;
>   	igt_display_t display;
>   	igt_output_t *output;
> -	drmModeResPtr res;
>   	int max_dotclock;
>   	bool (*adjust_mode)(data_t *data, drmModeModeInfoPtr mode);
>   };
> @@ -182,35 +181,23 @@ static void
>   test_output(data_t *data)
>   {
>   	igt_output_t *output = data->output;
> -	drmModeModeInfo mode;
> -	struct igt_fb fb;
> -	int ret;
> -	uint32_t crtc_id;
> +	drmModeConnector *connector = output->config.connector;
>   
> +	igt_display_reset(&data->display);
> +	igt_output_set_pipe(output, data->pipe);
>   	/*
>   	 * FIXME test every mode we have to be more
> -	 * sure everything is really getting rejected?
> +	 e sure everything is really getting rejected?

Please drop this comment, since we are trying all connector modes.

>   	 */
> -	mode = *igt_output_get_mode(output);
> -	igt_require(data->adjust_mode(data, &mode));
> +	igt_sort_connector_modes(connector, sort_drm_modes_by_res_dsc);
>   
> -	igt_create_fb(data->drm_fd,
> -		      max_t(uint16_t, mode.hdisplay, 64),
> -		      max_t(uint16_t, mode.vdisplay, 64),
> -		      DRM_FORMAT_XRGB8888,
> -		      DRM_FORMAT_MOD_LINEAR,
> -		      &fb);
> -
> -	kmstest_unset_all_crtcs(data->drm_fd, data->res);
> -
> -	crtc_id = data->display.pipes[data->pipe].crtc_id;
> -
> -	ret = drmModeSetCrtc(data->drm_fd, crtc_id,
> -			     fb.fb_id, 0, 0,
> -			     &output->id, 1, &mode);
> -	igt_assert_lt(ret, 0);
> +	for_each_connector_mode(output) {

What is the impact of CI execution time? If it takes more time, maybe we 
can limit the execution to few (maybe 5) modes.

> +		igt_output_override_mode(output, &connector->modes[j__]);
> +		igt_require(data->adjust_mode(data, &connector->modes[j__]));

For readability, please swap above two statements.

> +		igt_display_commit2(&data->display, &data->display.is_atomic ? COMMIT_ATOMIC : COMMIT_LEGACY);
> +	}
>   
> -	igt_remove_fb(data->drm_fd, &fb);

Why did you drop this?

> +	igt_output_override_mode(output, NULL);

Please unset the crtc.
igt_output_set_pipe(output, NULL);

Apart from these minor changes, overall this patch looks good to me.

- Bhanu

>   }
>   
>   static int i915_max_dotclock(data_t *data)
> @@ -290,8 +277,6 @@ igt_main
>   		kmstest_set_vt_graphics_mode();
>   
>   		igt_display_require(&data.display, data.drm_fd);
> -		data.res = drmModeGetResources(data.drm_fd);
> -		igt_assert(data.res);
>   
>   		data.max_dotclock = i915_max_dotclock(&data);
>   		igt_info("Max dotclock: %d kHz\n", data.max_dotclock);
> @@ -314,6 +299,5 @@ igt_main
>   	igt_fixture {
>   		igt_display_fini(&data.display);
>   		igt_reset_connectors();
> -		drmModeFreeResources(data.res);
>   	}
>   }

[igt-dev] ✗ GitLab.Pipeline: warning for tests/kms_invalid_mode: Test Cleanup (rev3)

[Patchwork]

== Series Details ==

Series: tests/kms_invalid_mode: Test Cleanup (rev3)
URL   : https://patchwork.freedesktop.org/series/105501/
State : warning

== Summary ==

Pipeline status: FAILED.

see https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/pipelines/622730 for the overview.

build:tests-fedora-clang has failed (https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/jobs/24580445):
  ninja: build stopped: subcommand failed.
  ninja: Entering directory `build'
  [1/808] Generating version.h with a custom command.
  [2/740] Linking target tests/kms_cursor_legacy.
  [3/740] Linking target tests/kms_flip.
  [4/740] Linking target tests/kms_getfb.
  [5/740] Compiling C object 'tests/59830eb@@kms_invalid_mode@exe/kms_invalid_mode.c.o'.
  FAILED: tests/59830eb@@kms_invalid_mode@exe/kms_invalid_mode.c.o 
  clang -Itests/59830eb@@kms_invalid_mode@exe -Itests -I../tests -I../include/drm-uapi -I../include/linux-uapi -Ilib -I../lib -I../lib/stubs/syscalls -I. -I../ -I/usr/include/cairo -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/pixman-1 -I/usr/include/freetype2 -I/usr/include/libpng16 -I/usr/include/libdrm -I/usr/include/libdrm/nouveau -I/usr/include/valgrind -Xclang -fcolor-diagnostics -pipe -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wextra -std=gnu11 -O2 -g -D_GNU_SOURCE -include config.h -D_FORTIFY_SOURCE=2 -Wbad-function-cast -Wdeclaration-after-statement -Wformat=2 -Wmissing-declarations -Wmissing-format-attribute -Wmissing-noreturn -Wmissing-prototypes -Wnested-externs -Wold-style-definition -Wpointer-arith -Wredundant-decls -Wshadow -Wstrict-prototypes -Wuninitialized -Wunused -Wno-missing-field-initializers -Wno-pointer-arith -Wno-address-of-packed-member -Wno-sign-compare -Wno-type-limits -Wno-unused-parameter -Wno-unused-result -Werror=address -Werror=array-bounds -Werror=implicit -Werror=init-self -Werror=int-to-pointer-cast -Werror=main -Werror=missing-braces -Werror=nonnull -Werror=pointer-to-int-cast -Werror=return-type -Werror=sequence-point -Werror=trigraphs -Werror=write-strings -fno-builtin-malloc -fno-builtin-calloc -fcommon -pthread -MD -MQ 'tests/59830eb@@kms_invalid_mode@exe/kms_invalid_mode.c.o' -MF 'tests/59830eb@@kms_invalid_mode@exe/kms_invalid_mode.c.o.d' -o 'tests/59830eb@@kms_invalid_mode@exe/kms_invalid_mode.c.o' -c ../tests/kms_invalid_mode.c
  ../tests/kms_invalid_mode.c:197:54: error: address of 'data->display.is_atomic' will always evaluate to 'true' [-Werror,-Wpointer-bool-conversion]
                  igt_display_commit2(&data->display, &data->display.is_atomic ? COMMIT_ATOMIC : COMMIT_LEGACY);
                                                       ~~~~~~~~~~~~~~^~~~~~~~~ ~
  1 error generated.
  ninja: build stopped: subcommand failed.
  section_end:1656314983:step_script
  section_start:1656314983:cleanup_file_variables
  Cleaning up project directory and file based variables
  section_end:1656314984:cleanup_file_variables
  ERROR: Job failed: exit code 1

== Logs ==

For more details see: https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/pipelines/622730

[igt-dev] ✗ Fi.CI.BAT: failure for tests/kms_invalid_mode: Test Cleanup (rev3)

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 5872 bytes --]

== Series Details ==

Series: tests/kms_invalid_mode: Test Cleanup (rev3)
URL   : https://patchwork.freedesktop.org/series/105501/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_11806 -> IGTPW_7390
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with IGTPW_7390 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_7390, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7390/index.html

Participating hosts (40 -> 36)
------------------------------

  Missing    (4): bat-dg2-8 fi-bdw-samus fi-icl-u2 fi-bdw-5557u 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in IGTPW_7390:

### IGT changes ###

#### Possible regressions ####

  * igt@kms_pipe_crc_basic@suspend-read-crc@pipe-b-hdmi-a-1:
    - fi-rkl-11600:       [PASS][1] -> [INCOMPLETE][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11806/fi-rkl-11600/igt@kms_pipe_crc_basic@suspend-read-crc@pipe-b-hdmi-a-1.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7390/fi-rkl-11600/igt@kms_pipe_crc_basic@suspend-read-crc@pipe-b-hdmi-a-1.html

  
Known issues
------------

  Here are the changes found in IGTPW_7390 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live@hangcheck:
    - fi-hsw-g3258:       [PASS][3] -> [INCOMPLETE][4] ([i915#4785])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11806/fi-hsw-g3258/igt@i915_selftest@live@hangcheck.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7390/fi-hsw-g3258/igt@i915_selftest@live@hangcheck.html

  * igt@i915_selftest@live@requests:
    - fi-pnv-d510:        [PASS][5] -> [DMESG-FAIL][6] ([i915#4528])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11806/fi-pnv-d510/igt@i915_selftest@live@requests.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7390/fi-pnv-d510/igt@i915_selftest@live@requests.html

  * igt@kms_chamelium@common-hpd-after-suspend:
    - fi-ivb-3770:        NOTRUN -> [SKIP][7] ([fdo#109271] / [fdo#111827]) +8 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7390/fi-ivb-3770/igt@kms_chamelium@common-hpd-after-suspend.html

  * igt@kms_flip@basic-flip-vs-modeset@a-edp1:
    - fi-tgl-u2:          [PASS][8] -> [DMESG-WARN][9] ([i915#402])
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11806/fi-tgl-u2/igt@kms_flip@basic-flip-vs-modeset@a-edp1.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7390/fi-tgl-u2/igt@kms_flip@basic-flip-vs-modeset@a-edp1.html

  * igt@kms_psr@cursor_plane_move:
    - fi-ivb-3770:        NOTRUN -> [SKIP][10] ([fdo#109271]) +17 similar issues
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7390/fi-ivb-3770/igt@kms_psr@cursor_plane_move.html

  * igt@runner@aborted:
    - fi-pnv-d510:        NOTRUN -> [FAIL][11] ([fdo#109271] / [i915#2403] / [i915#4312])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7390/fi-pnv-d510/igt@runner@aborted.html
    - fi-hsw-g3258:       NOTRUN -> [FAIL][12] ([fdo#109271] / [i915#4312] / [i915#6246])
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7390/fi-hsw-g3258/igt@runner@aborted.html

  
#### Possible fixes ####

  * igt@i915_selftest@live@hangcheck:
    - bat-dg1-5:          [DMESG-FAIL][13] ([i915#4494] / [i915#4957]) -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11806/bat-dg1-5/igt@i915_selftest@live@hangcheck.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7390/bat-dg1-5/igt@i915_selftest@live@hangcheck.html
    - bat-dg1-6:          [DMESG-FAIL][15] ([i915#4494] / [i915#4957]) -> [PASS][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11806/bat-dg1-6/igt@i915_selftest@live@hangcheck.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7390/bat-dg1-6/igt@i915_selftest@live@hangcheck.html

  * igt@kms_flip@basic-flip-vs-dpms@a-edp1:
    - fi-tgl-u2:          [DMESG-WARN][17] ([i915#402]) -> [PASS][18] +1 similar issue
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11806/fi-tgl-u2/igt@kms_flip@basic-flip-vs-dpms@a-edp1.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7390/fi-tgl-u2/igt@kms_flip@basic-flip-vs-dpms@a-edp1.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#2403]: https://gitlab.freedesktop.org/drm/intel/issues/2403
  [i915#3576]: https://gitlab.freedesktop.org/drm/intel/issues/3576
  [i915#402]: https://gitlab.freedesktop.org/drm/intel/issues/402
  [i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312
  [i915#4494]: https://gitlab.freedesktop.org/drm/intel/issues/4494
  [i915#4528]: https://gitlab.freedesktop.org/drm/intel/issues/4528
  [i915#4785]: https://gitlab.freedesktop.org/drm/intel/issues/4785
  [i915#4957]: https://gitlab.freedesktop.org/drm/intel/issues/4957
  [i915#6246]: https://gitlab.freedesktop.org/drm/intel/issues/6246


Build changes
-------------

  * CI: CI-20190529 -> None
  * IGT: IGT_6542 -> IGTPW_7390

  CI-20190529: 20190529
  CI_DRM_11806: cf74a0964a6cd08c439ea7373aec02abd211d882 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_7390: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7390/index.html
  IGT_6542: d38a476ee4b9f9a95d8f452de0d66cc52f7f079b @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7390/index.html

[-- Attachment #2: Type: text/html, Size: 7133 bytes --]

[igt-dev] [PATCH i-g-t 0/2] tests/kms_invalid_mode: Test Cleanup

[Nidhi Gupta]

Convert all possible subtests to dynamic and sanitize the system
state before starting/exiting the subtest

Nidhi Gupta (2):
  tests/kms_invalid_mode: Convert tests to dynamic
  tests/kms_invalid_mode: Test Cleanup

 tests/kms_invalid_mode.c | 73 +++++++++++++++-------------------------
 1 file changed, 28 insertions(+), 45 deletions(-)

-- 
2.26.2

[igt-dev] [PATCH i-g-t 1/2] tests/kms_invalid_mode: Convert tests to dynamic

[Nidhi Gupta]

Convert the existing subtests to dynamic subtests at pipe level.

Signed-off-by: Nidhi Gupta <nidhi1.gupta@intel.com>
Reviewed-by: Petri Latvala <petri.latvala@intel.com>
Reviewed-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
---
 tests/kms_invalid_mode.c | 54 ++++++++++++++++++----------------------
 1 file changed, 24 insertions(+), 30 deletions(-)

diff --git a/tests/kms_invalid_mode.c b/tests/kms_invalid_mode.c
index 630798d8..0287ee02 100644
--- a/tests/kms_invalid_mode.c
+++ b/tests/kms_invalid_mode.c
@@ -32,6 +32,7 @@ typedef struct _data data_t;
 
 struct _data {
 	int drm_fd;
+	enum pipe pipe;
 	igt_display_t display;
 	igt_output_t *output;
 	drmModeResPtr res;
@@ -177,21 +178,21 @@ adjust_mode_bad_vtotal(data_t *data, drmModeModeInfoPtr mode)
 	return true;
 }
 
-static int
+static void
 test_output(data_t *data)
 {
 	igt_output_t *output = data->output;
 	drmModeModeInfo mode;
 	struct igt_fb fb;
-	int i;
+	int ret;
+	uint32_t crtc_id;
 
 	/*
 	 * FIXME test every mode we have to be more
 	 * sure everything is really getting rejected?
 	 */
 	mode = *igt_output_get_mode(output);
-	if (!data->adjust_mode(data, &mode))
-		return 0;
+	igt_require(data->adjust_mode(data, &mode));
 
 	igt_create_fb(data->drm_fd,
 		      max_t(uint16_t, mode.hdisplay, 64),
@@ -202,32 +203,14 @@ test_output(data_t *data)
 
 	kmstest_unset_all_crtcs(data->drm_fd, data->res);
 
-	for (i = 0; i < data->res->count_crtcs; i++) {
-		int ret;
-
-		igt_info("Checking pipe %c connector %s with mode %s\n",
-			 'A'+i, output->name, mode.name);
+	crtc_id = data->display.pipes[data->pipe].crtc_id;
 
-		ret = drmModeSetCrtc(data->drm_fd, data->res->crtcs[i],
-				     fb.fb_id, 0, 0,
-				     &output->id, 1, &mode);
-		igt_assert_lt(ret, 0);
-	}
+	ret = drmModeSetCrtc(data->drm_fd, crtc_id,
+			     fb.fb_id, 0, 0,
+			     &output->id, 1, &mode);
+	igt_assert_lt(ret, 0);
 
 	igt_remove_fb(data->drm_fd, &fb);
-
-	return 1;
-}
-
-static void test(data_t *data)
-{
-	int valid_connectors = 0;
-
-	for_each_connected_output(&data->display, data->output) {
-		valid_connectors += test_output(data);
-	}
-
-	igt_require_f(valid_connectors, "No suitable connectors found\n");
 }
 
 static int i915_max_dotclock(data_t *data)
@@ -297,6 +280,10 @@ static data_t data;
 
 igt_main
 {
+
+	enum pipe pipe;
+	igt_output_t *output;
+
 	igt_fixture {
 		data.drm_fd = drm_open_driver_master(DRIVER_ANY);
 
@@ -310,10 +297,17 @@ igt_main
 		igt_info("Max dotclock: %d kHz\n", data.max_dotclock);
 	}
 
+	igt_describe("Make sure all modesets are rejected when the requested mode is invalid");
 	for (int i = 0; i < ARRAY_SIZE(subtests); i++) {
-		igt_subtest(subtests[i].name) {
-			data.adjust_mode = subtests[i].adjust_mode;
-			test(&data);
+		igt_subtest_with_dynamic(subtests[i].name) {
+			for_each_pipe_with_valid_output(&data.display, pipe, output) {
+				igt_dynamic_f("%s-pipe-%s", igt_output_name(output), kmstest_pipe_name(pipe)) {
+					data.output = output;
+					data.pipe = pipe;
+					data.adjust_mode = subtests[i].adjust_mode;
+					test_output(&data);
+				}
+			}
 		}
 	}
 
-- 
2.26.2

[igt-dev] [PATCH i-g-t 0/2] tests/kms_invalid_mode: Test Cleanup

[Nidhi Gupta]

Convert all possible subtests to dynamic and sanitize the system
state before starting/exiting the subtest.

Nidhi Gupta (2):
  tests/kms_invalid_mode: Convert tests to dynamic
  tests/kms_invalid_mode: Test Cleanup

 tests/kms_invalid_mode.c | 75 ++++++++++++++++------------------------
 1 file changed, 29 insertions(+), 46 deletions(-)

-- 
2.26.2

[igt-dev] [PATCH i-g-t 1/2] tests/kms_invalid_mode: Convert tests to dynamic

[Nidhi Gupta]

Convert the existing subtests to dynamic subtests at pipe level.

Signed-off-by: Nidhi Gupta <nidhi1.gupta@intel.com>
Reviewed-by: Petri Latvala <petri.latvala@intel.com>
Reviewed-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
---
 tests/kms_invalid_mode.c | 54 ++++++++++++++++++----------------------
 1 file changed, 24 insertions(+), 30 deletions(-)

diff --git a/tests/kms_invalid_mode.c b/tests/kms_invalid_mode.c
index 630798d8..0287ee02 100644
--- a/tests/kms_invalid_mode.c
+++ b/tests/kms_invalid_mode.c
@@ -32,6 +32,7 @@ typedef struct _data data_t;
 
 struct _data {
 	int drm_fd;
+	enum pipe pipe;
 	igt_display_t display;
 	igt_output_t *output;
 	drmModeResPtr res;
@@ -177,21 +178,21 @@ adjust_mode_bad_vtotal(data_t *data, drmModeModeInfoPtr mode)
 	return true;
 }
 
-static int
+static void
 test_output(data_t *data)
 {
 	igt_output_t *output = data->output;
 	drmModeModeInfo mode;
 	struct igt_fb fb;
-	int i;
+	int ret;
+	uint32_t crtc_id;
 
 	/*
 	 * FIXME test every mode we have to be more
 	 * sure everything is really getting rejected?
 	 */
 	mode = *igt_output_get_mode(output);
-	if (!data->adjust_mode(data, &mode))
-		return 0;
+	igt_require(data->adjust_mode(data, &mode));
 
 	igt_create_fb(data->drm_fd,
 		      max_t(uint16_t, mode.hdisplay, 64),
@@ -202,32 +203,14 @@ test_output(data_t *data)
 
 	kmstest_unset_all_crtcs(data->drm_fd, data->res);
 
-	for (i = 0; i < data->res->count_crtcs; i++) {
-		int ret;
-
-		igt_info("Checking pipe %c connector %s with mode %s\n",
-			 'A'+i, output->name, mode.name);
+	crtc_id = data->display.pipes[data->pipe].crtc_id;
 
-		ret = drmModeSetCrtc(data->drm_fd, data->res->crtcs[i],
-				     fb.fb_id, 0, 0,
-				     &output->id, 1, &mode);
-		igt_assert_lt(ret, 0);
-	}
+	ret = drmModeSetCrtc(data->drm_fd, crtc_id,
+			     fb.fb_id, 0, 0,
+			     &output->id, 1, &mode);
+	igt_assert_lt(ret, 0);
 
 	igt_remove_fb(data->drm_fd, &fb);
-
-	return 1;
-}
-
-static void test(data_t *data)
-{
-	int valid_connectors = 0;
-
-	for_each_connected_output(&data->display, data->output) {
-		valid_connectors += test_output(data);
-	}
-
-	igt_require_f(valid_connectors, "No suitable connectors found\n");
 }
 
 static int i915_max_dotclock(data_t *data)
@@ -297,6 +280,10 @@ static data_t data;
 
 igt_main
 {
+
+	enum pipe pipe;
+	igt_output_t *output;
+
 	igt_fixture {
 		data.drm_fd = drm_open_driver_master(DRIVER_ANY);
 
@@ -310,10 +297,17 @@ igt_main
 		igt_info("Max dotclock: %d kHz\n", data.max_dotclock);
 	}
 
+	igt_describe("Make sure all modesets are rejected when the requested mode is invalid");
 	for (int i = 0; i < ARRAY_SIZE(subtests); i++) {
-		igt_subtest(subtests[i].name) {
-			data.adjust_mode = subtests[i].adjust_mode;
-			test(&data);
+		igt_subtest_with_dynamic(subtests[i].name) {
+			for_each_pipe_with_valid_output(&data.display, pipe, output) {
+				igt_dynamic_f("%s-pipe-%s", igt_output_name(output), kmstest_pipe_name(pipe)) {
+					data.output = output;
+					data.pipe = pipe;
+					data.adjust_mode = subtests[i].adjust_mode;
+					test_output(&data);
+				}
+			}
 		}
 	}
 
-- 
2.26.2

[igt-dev] [PATCH i-g-t 0/2] tests/kms_invalid_mode: Test Cleanup

[Nidhi Gupta]

Convert all possible subtests to dynamic and sanitize the system
state before starting/exiting the subtest.

Nidhi Gupta (2):
  tests/kms_invalid_mode: Convert tests to dynamic
  tests/kms_invalid_mode: Test Cleanup

 tests/kms_invalid_mode.c | 50 +++++++++++++++-------------------------
 1 file changed, 19 insertions(+), 31 deletions(-)

-- 
2.26.2

[igt-dev] [PATCH i-g-t 1/2] tests/kms_invalid_mode: Convert tests to dynamic

[Nidhi Gupta]

Convert the existing subtests to dynamic subtests at pipe level.

Signed-off-by: Nidhi Gupta <nidhi1.gupta@intel.com>
Reviewed-by: Petri Latvala <petri.latvala@intel.com>
---
 tests/kms_invalid_mode.c | 53 +++++++++++++++++-----------------------
 1 file changed, 23 insertions(+), 30 deletions(-)

diff --git a/tests/kms_invalid_mode.c b/tests/kms_invalid_mode.c
index 630798d8..7e1f683b 100644
--- a/tests/kms_invalid_mode.c
+++ b/tests/kms_invalid_mode.c
@@ -32,6 +32,7 @@ typedef struct _data data_t;
 
 struct _data {
 	int drm_fd;
+	enum pipe pipe;
 	igt_display_t display;
 	igt_output_t *output;
 	drmModeResPtr res;
@@ -177,21 +178,21 @@ adjust_mode_bad_vtotal(data_t *data, drmModeModeInfoPtr mode)
 	return true;
 }
 
-static int
+static void
 test_output(data_t *data)
 {
 	igt_output_t *output = data->output;
 	drmModeModeInfo mode;
 	struct igt_fb fb;
-	int i;
+	int ret;
+	uint32_t crtc_id;
 
 	/*
 	 * FIXME test every mode we have to be more
 	 * sure everything is really getting rejected?
 	 */
 	mode = *igt_output_get_mode(output);
-	if (!data->adjust_mode(data, &mode))
-		return 0;
+	igt_require(data->adjust_mode(data, &mode));
 
 	igt_create_fb(data->drm_fd,
 		      max_t(uint16_t, mode.hdisplay, 64),
@@ -202,32 +203,14 @@ test_output(data_t *data)
 
 	kmstest_unset_all_crtcs(data->drm_fd, data->res);
 
-	for (i = 0; i < data->res->count_crtcs; i++) {
-		int ret;
-
-		igt_info("Checking pipe %c connector %s with mode %s\n",
-			 'A'+i, output->name, mode.name);
+	crtc_id = data->display.pipes[data->pipe].crtc_id;
 
-		ret = drmModeSetCrtc(data->drm_fd, data->res->crtcs[i],
-				     fb.fb_id, 0, 0,
-				     &output->id, 1, &mode);
-		igt_assert_lt(ret, 0);
-	}
+	ret = drmModeSetCrtc(data->drm_fd, crtc_id,
+			     fb.fb_id, 0, 0,
+			     &output->id, 1, &mode);
+	igt_assert_lt(ret, 0);
 
 	igt_remove_fb(data->drm_fd, &fb);
-
-	return 1;
-}
-
-static void test(data_t *data)
-{
-	int valid_connectors = 0;
-
-	for_each_connected_output(&data->display, data->output) {
-		valid_connectors += test_output(data);
-	}
-
-	igt_require_f(valid_connectors, "No suitable connectors found\n");
 }
 
 static int i915_max_dotclock(data_t *data)
@@ -297,6 +280,10 @@ static data_t data;
 
 igt_main
 {
+
+	enum pipe pipe;
+	igt_output_t *output;
+
 	igt_fixture {
 		data.drm_fd = drm_open_driver_master(DRIVER_ANY);
 
@@ -311,9 +298,15 @@ igt_main
 	}
 
 	for (int i = 0; i < ARRAY_SIZE(subtests); i++) {
-		igt_subtest(subtests[i].name) {
-			data.adjust_mode = subtests[i].adjust_mode;
-			test(&data);
+		igt_subtest_with_dynamic(subtests[i].name) {
+			for_each_pipe_with_valid_output(&data.display, pipe, output) {
+				igt_dynamic_f("%s-pipe-%s", igt_output_name(output), kmstest_pipe_name(pipe)) {
+					data.output = output;
+					data.pipe = pipe;
+					data.adjust_mode = subtests[i].adjust_mode;
+					test_output(&data);
+				}
+			}
 		}
 	}
 
-- 
2.26.2

Re: [igt-dev] [PATCH i-g-t 1/2] tests/kms_invalid_mode: Convert tests to dynamic

[Modem, Bhanuprakash]

On Fri-24-06-2022 05:27 am, Nidhi Gupta wrote:
> Convert the existing subtests to dynamic subtests at pipe level.
> 
> Signed-off-by: Nidhi Gupta <nidhi1.gupta@intel.com>
> Reviewed-by: Petri Latvala <petri.latvala@intel.com>
> ---
>   tests/kms_invalid_mode.c | 53 +++++++++++++++++-----------------------
>   1 file changed, 23 insertions(+), 30 deletions(-)
> 
> diff --git a/tests/kms_invalid_mode.c b/tests/kms_invalid_mode.c
> index 630798d8..7e1f683b 100644
> --- a/tests/kms_invalid_mode.c
> +++ b/tests/kms_invalid_mode.c
> @@ -32,6 +32,7 @@ typedef struct _data data_t;
>   
>   struct _data {
>   	int drm_fd;
> +	enum pipe pipe;
>   	igt_display_t display;
>   	igt_output_t *output;
>   	drmModeResPtr res;
> @@ -177,21 +178,21 @@ adjust_mode_bad_vtotal(data_t *data, drmModeModeInfoPtr mode)
>   	return true;
>   }
>   
> -static int
> +static void
>   test_output(data_t *data)
>   {
>   	igt_output_t *output = data->output;
>   	drmModeModeInfo mode;
>   	struct igt_fb fb;
> -	int i;
> +	int ret;
> +	uint32_t crtc_id;
>   
>   	/*
>   	 * FIXME test every mode we have to be more
>   	 * sure everything is really getting rejected?
>   	 */
>   	mode = *igt_output_get_mode(output);
> -	if (!data->adjust_mode(data, &mode))
> -		return 0;
> +	igt_require(data->adjust_mode(data, &mode));
>   
>   	igt_create_fb(data->drm_fd,
>   		      max_t(uint16_t, mode.hdisplay, 64),
> @@ -202,32 +203,14 @@ test_output(data_t *data)
>   
>   	kmstest_unset_all_crtcs(data->drm_fd, data->res);
>   
> -	for (i = 0; i < data->res->count_crtcs; i++) {
> -		int ret;
> -
> -		igt_info("Checking pipe %c connector %s with mode %s\n",
> -			 'A'+i, output->name, mode.name);
> +	crtc_id = data->display.pipes[data->pipe].crtc_id;
>   
> -		ret = drmModeSetCrtc(data->drm_fd, data->res->crtcs[i],
> -				     fb.fb_id, 0, 0,
> -				     &output->id, 1, &mode);
> -		igt_assert_lt(ret, 0);
> -	}
> +	ret = drmModeSetCrtc(data->drm_fd, crtc_id,
> +			     fb.fb_id, 0, 0,
> +			     &output->id, 1, &mode);
> +	igt_assert_lt(ret, 0);
>   
>   	igt_remove_fb(data->drm_fd, &fb);
> -
> -	return 1;
> -}
> -
> -static void test(data_t *data)
> -{
> -	int valid_connectors = 0;
> -
> -	for_each_connected_output(&data->display, data->output) {
> -		valid_connectors += test_output(data);
> -	}
> -
> -	igt_require_f(valid_connectors, "No suitable connectors found\n");
>   }
>   
>   static int i915_max_dotclock(data_t *data)
> @@ -297,6 +280,10 @@ static data_t data;
>   
>   igt_main
>   {
> +
> +	enum pipe pipe;
> +	igt_output_t *output;
> +
>   	igt_fixture {
>   		data.drm_fd = drm_open_driver_master(DRIVER_ANY);
>   
> @@ -311,9 +298,15 @@ igt_main
>   	}
>   
>   	for (int i = 0; i < ARRAY_SIZE(subtests); i++) {
> -		igt_subtest(subtests[i].name) {
> -			data.adjust_mode = subtests[i].adjust_mode;
> -			test(&data);
> +		igt_subtest_with_dynamic(subtests[i].name) {

NAK: Please add test description

- Bhanu

> +			for_each_pipe_with_valid_output(&data.display, pipe, output) {
> +				igt_dynamic_f("%s-pipe-%s", igt_output_name(output), kmstest_pipe_name(pipe)) {
> +					data.output = output;
> +					data.pipe = pipe;
> +					data.adjust_mode = subtests[i].adjust_mode;
> +					test_output(&data);
> +				}
> +			}
>   		}
>   	}
>   

[igt-dev] [PATCH i-g-t 0/2] tests/kms_invalid_mode: Test Cleanup

[Nidhi Gupta]

Convert all possible subtests to dynamic and sanitize the system
state before starting/exiting the subtest.

Nidhi Gupta (2):
  tests/kms_invalid_mode: Convert tests to dynamic
  tests/kms_invalid_mode: Test Cleanup

 tests/kms_invalid_mode.c | 55 ++++++++++++++++++----------------------
 1 file changed, 25 insertions(+), 30 deletions(-)

-- 
2.26.2

[igt-dev] [PATCH i-g-t 1/2] tests/kms_invalid_mode: Convert tests to dynamic

[Nidhi Gupta]

Convert the existing subtests to dynamic subtests at pipe level.

Signed-off-by: Nidhi Gupta <nidhi1.gupta@intel.com>
---
 tests/kms_invalid_mode.c | 53 +++++++++++++++++-----------------------
 1 file changed, 23 insertions(+), 30 deletions(-)

diff --git a/tests/kms_invalid_mode.c b/tests/kms_invalid_mode.c
index 630798d8..7e1f683b 100644
--- a/tests/kms_invalid_mode.c
+++ b/tests/kms_invalid_mode.c
@@ -32,6 +32,7 @@ typedef struct _data data_t;
 
 struct _data {
 	int drm_fd;
+	enum pipe pipe;
 	igt_display_t display;
 	igt_output_t *output;
 	drmModeResPtr res;
@@ -177,21 +178,21 @@ adjust_mode_bad_vtotal(data_t *data, drmModeModeInfoPtr mode)
 	return true;
 }
 
-static int
+static void
 test_output(data_t *data)
 {
 	igt_output_t *output = data->output;
 	drmModeModeInfo mode;
 	struct igt_fb fb;
-	int i;
+	int ret;
+	uint32_t crtc_id;
 
 	/*
 	 * FIXME test every mode we have to be more
 	 * sure everything is really getting rejected?
 	 */
 	mode = *igt_output_get_mode(output);
-	if (!data->adjust_mode(data, &mode))
-		return 0;
+	igt_require(data->adjust_mode(data, &mode));
 
 	igt_create_fb(data->drm_fd,
 		      max_t(uint16_t, mode.hdisplay, 64),
@@ -202,32 +203,14 @@ test_output(data_t *data)
 
 	kmstest_unset_all_crtcs(data->drm_fd, data->res);
 
-	for (i = 0; i < data->res->count_crtcs; i++) {
-		int ret;
-
-		igt_info("Checking pipe %c connector %s with mode %s\n",
-			 'A'+i, output->name, mode.name);
+	crtc_id = data->display.pipes[data->pipe].crtc_id;
 
-		ret = drmModeSetCrtc(data->drm_fd, data->res->crtcs[i],
-				     fb.fb_id, 0, 0,
-				     &output->id, 1, &mode);
-		igt_assert_lt(ret, 0);
-	}
+	ret = drmModeSetCrtc(data->drm_fd, crtc_id,
+			     fb.fb_id, 0, 0,
+			     &output->id, 1, &mode);
+	igt_assert_lt(ret, 0);
 
 	igt_remove_fb(data->drm_fd, &fb);
-
-	return 1;
-}
-
-static void test(data_t *data)
-{
-	int valid_connectors = 0;
-
-	for_each_connected_output(&data->display, data->output) {
-		valid_connectors += test_output(data);
-	}
-
-	igt_require_f(valid_connectors, "No suitable connectors found\n");
 }
 
 static int i915_max_dotclock(data_t *data)
@@ -297,6 +280,10 @@ static data_t data;
 
 igt_main
 {
+
+	enum pipe pipe;
+	igt_output_t *output;
+
 	igt_fixture {
 		data.drm_fd = drm_open_driver_master(DRIVER_ANY);
 
@@ -311,9 +298,15 @@ igt_main
 	}
 
 	for (int i = 0; i < ARRAY_SIZE(subtests); i++) {
-		igt_subtest(subtests[i].name) {
-			data.adjust_mode = subtests[i].adjust_mode;
-			test(&data);
+		igt_subtest_with_dynamic(subtests[i].name) {
+			for_each_pipe_with_valid_output(&data.display, pipe, output) {
+				igt_dynamic_f("%s-pipe-%s", igt_output_name(output), kmstest_pipe_name(pipe)) {
+					data.output = output;
+					data.pipe = pipe;
+					data.adjust_mode = subtests[i].adjust_mode;
+					test_output(&data);
+				}
+			}
 		}
 	}
 
-- 
2.26.2

Re: [igt-dev] [PATCH i-g-t 1/2] tests/kms_invalid_mode: Convert tests to dynamic

[Petri Latvala]

On Wed, Jun 22, 2022 at 09:18:36PM +0530, Nidhi Gupta wrote:
> Convert the existing subtests to dynamic subtests at pipe level.
> 
> Signed-off-by: Nidhi Gupta <nidhi1.gupta@intel.com>

Reviewed-by: Petri Latvala <petri.latvala@intel.com>

> ---
>  tests/kms_invalid_mode.c | 53 +++++++++++++++++-----------------------
>  1 file changed, 23 insertions(+), 30 deletions(-)
> 
> diff --git a/tests/kms_invalid_mode.c b/tests/kms_invalid_mode.c
> index 630798d8..7e1f683b 100644
> --- a/tests/kms_invalid_mode.c
> +++ b/tests/kms_invalid_mode.c
> @@ -32,6 +32,7 @@ typedef struct _data data_t;
>  
>  struct _data {
>  	int drm_fd;
> +	enum pipe pipe;
>  	igt_display_t display;
>  	igt_output_t *output;
>  	drmModeResPtr res;
> @@ -177,21 +178,21 @@ adjust_mode_bad_vtotal(data_t *data, drmModeModeInfoPtr mode)
>  	return true;
>  }
>  
> -static int
> +static void
>  test_output(data_t *data)
>  {
>  	igt_output_t *output = data->output;
>  	drmModeModeInfo mode;
>  	struct igt_fb fb;
> -	int i;
> +	int ret;
> +	uint32_t crtc_id;
>  
>  	/*
>  	 * FIXME test every mode we have to be more
>  	 * sure everything is really getting rejected?
>  	 */
>  	mode = *igt_output_get_mode(output);
> -	if (!data->adjust_mode(data, &mode))
> -		return 0;
> +	igt_require(data->adjust_mode(data, &mode));
>  
>  	igt_create_fb(data->drm_fd,
>  		      max_t(uint16_t, mode.hdisplay, 64),
> @@ -202,32 +203,14 @@ test_output(data_t *data)
>  
>  	kmstest_unset_all_crtcs(data->drm_fd, data->res);
>  
> -	for (i = 0; i < data->res->count_crtcs; i++) {
> -		int ret;
> -
> -		igt_info("Checking pipe %c connector %s with mode %s\n",
> -			 'A'+i, output->name, mode.name);
> +	crtc_id = data->display.pipes[data->pipe].crtc_id;
>  
> -		ret = drmModeSetCrtc(data->drm_fd, data->res->crtcs[i],
> -				     fb.fb_id, 0, 0,
> -				     &output->id, 1, &mode);
> -		igt_assert_lt(ret, 0);
> -	}
> +	ret = drmModeSetCrtc(data->drm_fd, crtc_id,
> +			     fb.fb_id, 0, 0,
> +			     &output->id, 1, &mode);
> +	igt_assert_lt(ret, 0);
>  
>  	igt_remove_fb(data->drm_fd, &fb);
> -
> -	return 1;
> -}
> -
> -static void test(data_t *data)
> -{
> -	int valid_connectors = 0;
> -
> -	for_each_connected_output(&data->display, data->output) {
> -		valid_connectors += test_output(data);
> -	}
> -
> -	igt_require_f(valid_connectors, "No suitable connectors found\n");
>  }
>  
>  static int i915_max_dotclock(data_t *data)
> @@ -297,6 +280,10 @@ static data_t data;
>  
>  igt_main
>  {
> +
> +	enum pipe pipe;
> +	igt_output_t *output;
> +
>  	igt_fixture {
>  		data.drm_fd = drm_open_driver_master(DRIVER_ANY);
>  
> @@ -311,9 +298,15 @@ igt_main
>  	}
>  
>  	for (int i = 0; i < ARRAY_SIZE(subtests); i++) {
> -		igt_subtest(subtests[i].name) {
> -			data.adjust_mode = subtests[i].adjust_mode;
> -			test(&data);
> +		igt_subtest_with_dynamic(subtests[i].name) {
> +			for_each_pipe_with_valid_output(&data.display, pipe, output) {
> +				igt_dynamic_f("%s-pipe-%s", igt_output_name(output), kmstest_pipe_name(pipe)) {
> +					data.output = output;
> +					data.pipe = pipe;
> +					data.adjust_mode = subtests[i].adjust_mode;
> +					test_output(&data);
> +				}
> +			}
>  		}
>  	}
>  
> -- 
> 2.26.2
> 

Re: [igt-dev] [PATCH i-g-t 1/2] tests/kms_invalid_mode: Convert tests to dynamic

[Modem, Bhanuprakash]

On Wed-22-06-2022 09:18 pm, Nidhi Gupta wrote:
> Convert the existing subtests to dynamic subtests at pipe level.
> 
> Signed-off-by: Nidhi Gupta <nidhi1.gupta@intel.com>
> ---
>   tests/kms_invalid_mode.c | 53 +++++++++++++++++-----------------------
>   1 file changed, 23 insertions(+), 30 deletions(-)
> 
> diff --git a/tests/kms_invalid_mode.c b/tests/kms_invalid_mode.c
> index 630798d8..7e1f683b 100644
> --- a/tests/kms_invalid_mode.c
> +++ b/tests/kms_invalid_mode.c
> @@ -32,6 +32,7 @@ typedef struct _data data_t;
>   
>   struct _data {
>   	int drm_fd;
> +	enum pipe pipe;
>   	igt_display_t display;
>   	igt_output_t *output;
>   	drmModeResPtr res;
> @@ -177,21 +178,21 @@ adjust_mode_bad_vtotal(data_t *data, drmModeModeInfoPtr mode)
>   	return true;
>   }
>   
> -static int
> +static void
>   test_output(data_t *data)
>   {
>   	igt_output_t *output = data->output;
>   	drmModeModeInfo mode;
>   	struct igt_fb fb;
> -	int i;
> +	int ret;
> +	uint32_t crtc_id;
>   
>   	/*
>   	 * FIXME test every mode we have to be more
>   	 * sure everything is really getting rejected?
>   	 */
>   	mode = *igt_output_get_mode(output);
> -	if (!data->adjust_mode(data, &mode))
> -		return 0;
> +	igt_require(data->adjust_mode(data, &mode));
>   
>   	igt_create_fb(data->drm_fd,
>   		      max_t(uint16_t, mode.hdisplay, 64),
> @@ -202,32 +203,14 @@ test_output(data_t *data)
>   
>   	kmstest_unset_all_crtcs(data->drm_fd, data->res);
>   
> -	for (i = 0; i < data->res->count_crtcs; i++) {
> -		int ret;
> -
> -		igt_info("Checking pipe %c connector %s with mode %s\n",
> -			 'A'+i, output->name, mode.name);
> +	crtc_id = data->display.pipes[data->pipe].crtc_id;
>   
> -		ret = drmModeSetCrtc(data->drm_fd, data->res->crtcs[i],
> -				     fb.fb_id, 0, 0,
> -				     &output->id, 1, &mode);
> -		igt_assert_lt(ret, 0);
> -	}
> +	ret = drmModeSetCrtc(data->drm_fd, crtc_id,
> +			     fb.fb_id, 0, 0,
> +			     &output->id, 1, &mode);

Instead of using drm calls, is it possible to use igt kms wrappers?

igt_output_set_pipe();
try_commit();

- Bhanu

> +	igt_assert_lt(ret, 0);
>   
>   	igt_remove_fb(data->drm_fd, &fb);
> -
> -	return 1;
> -}
> -
> -static void test(data_t *data)
> -{
> -	int valid_connectors = 0;
> -
> -	for_each_connected_output(&data->display, data->output) {
> -		valid_connectors += test_output(data);
> -	}
> -
> -	igt_require_f(valid_connectors, "No suitable connectors found\n");
>   }
>   
>   static int i915_max_dotclock(data_t *data)
> @@ -297,6 +280,10 @@ static data_t data;
>   
>   igt_main
>   {
> +
> +	enum pipe pipe;
> +	igt_output_t *output;
> +
>   	igt_fixture {
>   		data.drm_fd = drm_open_driver_master(DRIVER_ANY);
>   
> @@ -311,9 +298,15 @@ igt_main
>   	}
>   
>   	for (int i = 0; i < ARRAY_SIZE(subtests); i++) {
> -		igt_subtest(subtests[i].name) {
> -			data.adjust_mode = subtests[i].adjust_mode;
> -			test(&data);
> +		igt_subtest_with_dynamic(subtests[i].name) {
> +			for_each_pipe_with_valid_output(&data.display, pipe, output) {
> +				igt_dynamic_f("%s-pipe-%s", igt_output_name(output), kmstest_pipe_name(pipe)) {
> +					data.output = output;
> +					data.pipe = pipe;
> +					data.adjust_mode = subtests[i].adjust_mode;
> +					test_output(&data);
> +				}
> +			}
>   		}
>   	}
>   

Re: [igt-dev] [PATCH i-g-t 1/2] tests/kms_invalid_mode: Convert tests to dynamic

[Gupta, Nidhi1]

Hi Bhanu,

In many tests we are using drm calls, like drmModePageFlip, drmModeSetCrtc, if I will replace it with igt wrapper it will increase the code lines, should I still change it?

-----Original Message-----
From: Modem, Bhanuprakash <bhanuprakash.modem@intel.com> 
Sent: Thursday, June 23, 2022 8:11 AM
To: Gupta, Nidhi1 <nidhi1.gupta@intel.com>; igt-dev@lists.freedesktop.org
Cc: Latvala, Petri <petri.latvala@intel.com>
Subject: Re: [PATCH i-g-t 1/2] tests/kms_invalid_mode: Convert tests to dynamic

On Wed-22-06-2022 09:18 pm, Nidhi Gupta wrote:
> Convert the existing subtests to dynamic subtests at pipe level.
> 
> Signed-off-by: Nidhi Gupta <nidhi1.gupta@intel.com>
> ---
>   tests/kms_invalid_mode.c | 53 +++++++++++++++++-----------------------
>   1 file changed, 23 insertions(+), 30 deletions(-)
> 
> diff --git a/tests/kms_invalid_mode.c b/tests/kms_invalid_mode.c index 
> 630798d8..7e1f683b 100644
> --- a/tests/kms_invalid_mode.c
> +++ b/tests/kms_invalid_mode.c
> @@ -32,6 +32,7 @@ typedef struct _data data_t;
>   
>   struct _data {
>   	int drm_fd;
> +	enum pipe pipe;
>   	igt_display_t display;
>   	igt_output_t *output;
>   	drmModeResPtr res;
> @@ -177,21 +178,21 @@ adjust_mode_bad_vtotal(data_t *data, drmModeModeInfoPtr mode)
>   	return true;
>   }
>   
> -static int
> +static void
>   test_output(data_t *data)
>   {
>   	igt_output_t *output = data->output;
>   	drmModeModeInfo mode;
>   	struct igt_fb fb;
> -	int i;
> +	int ret;
> +	uint32_t crtc_id;
>   
>   	/*
>   	 * FIXME test every mode we have to be more
>   	 * sure everything is really getting rejected?
>   	 */
>   	mode = *igt_output_get_mode(output);
> -	if (!data->adjust_mode(data, &mode))
> -		return 0;
> +	igt_require(data->adjust_mode(data, &mode));
>   
>   	igt_create_fb(data->drm_fd,
>   		      max_t(uint16_t, mode.hdisplay, 64), @@ -202,32 +203,14 @@ 
> test_output(data_t *data)
>   
>   	kmstest_unset_all_crtcs(data->drm_fd, data->res);
>   
> -	for (i = 0; i < data->res->count_crtcs; i++) {
> -		int ret;
> -
> -		igt_info("Checking pipe %c connector %s with mode %s\n",
> -			 'A'+i, output->name, mode.name);
> +	crtc_id = data->display.pipes[data->pipe].crtc_id;
>   
> -		ret = drmModeSetCrtc(data->drm_fd, data->res->crtcs[i],
> -				     fb.fb_id, 0, 0,
> -				     &output->id, 1, &mode);
> -		igt_assert_lt(ret, 0);
> -	}
> +	ret = drmModeSetCrtc(data->drm_fd, crtc_id,
> +			     fb.fb_id, 0, 0,
> +			     &output->id, 1, &mode);

Instead of using drm calls, is it possible to use igt kms wrappers?

igt_output_set_pipe();
try_commit();

- Bhanu

> +	igt_assert_lt(ret, 0);
>   
>   	igt_remove_fb(data->drm_fd, &fb);
> -
> -	return 1;
> -}
> -
> -static void test(data_t *data)
> -{
> -	int valid_connectors = 0;
> -
> -	for_each_connected_output(&data->display, data->output) {
> -		valid_connectors += test_output(data);
> -	}
> -
> -	igt_require_f(valid_connectors, "No suitable connectors found\n");
>   }
>   
>   static int i915_max_dotclock(data_t *data) @@ -297,6 +280,10 @@ 
> static data_t data;
>   
>   igt_main
>   {
> +
> +	enum pipe pipe;
> +	igt_output_t *output;
> +
>   	igt_fixture {
>   		data.drm_fd = drm_open_driver_master(DRIVER_ANY);
>   
> @@ -311,9 +298,15 @@ igt_main
>   	}
>   
>   	for (int i = 0; i < ARRAY_SIZE(subtests); i++) {
> -		igt_subtest(subtests[i].name) {
> -			data.adjust_mode = subtests[i].adjust_mode;
> -			test(&data);
> +		igt_subtest_with_dynamic(subtests[i].name) {
> +			for_each_pipe_with_valid_output(&data.display, pipe, output) {
> +				igt_dynamic_f("%s-pipe-%s", igt_output_name(output), kmstest_pipe_name(pipe)) {
> +					data.output = output;
> +					data.pipe = pipe;
> +					data.adjust_mode = subtests[i].adjust_mode;
> +					test_output(&data);
> +				}
> +			}
>   		}
>   	}
>   

Re: [igt-dev] [PATCH i-g-t 1/2] tests/kms_invalid_mode: Convert tests to dynamic

[Modem, Bhanuprakash]

On Thu-23-06-2022 11:25 am, Gupta, Nidhi1 wrote:
> Hi Bhanu,
> 
> In many tests we are using drm calls, like drmModePageFlip, drmModeSetCrtc, if I will replace it with igt wrapper it will increase the code lines, should I still change it?

It's fine to increase the number of lines of code. The idea is to use 
IGT wrappers wherever it's possible, since IGT helpers were developed 
purposefully and more handy.

Ofcourse, in some tests it is not possible to use these IGT helpers, ex: 
kms_setmode.

@Petri, any thoughts?

- Bhanu

> 
> -----Original Message-----
> From: Modem, Bhanuprakash <bhanuprakash.modem@intel.com>
> Sent: Thursday, June 23, 2022 8:11 AM
> To: Gupta, Nidhi1 <nidhi1.gupta@intel.com>; igt-dev@lists.freedesktop.org
> Cc: Latvala, Petri <petri.latvala@intel.com>
> Subject: Re: [PATCH i-g-t 1/2] tests/kms_invalid_mode: Convert tests to dynamic
> 
> On Wed-22-06-2022 09:18 pm, Nidhi Gupta wrote:
>> Convert the existing subtests to dynamic subtests at pipe level.
>>
>> Signed-off-by: Nidhi Gupta <nidhi1.gupta@intel.com>
>> ---
>>    tests/kms_invalid_mode.c | 53 +++++++++++++++++-----------------------
>>    1 file changed, 23 insertions(+), 30 deletions(-)
>>
>> diff --git a/tests/kms_invalid_mode.c b/tests/kms_invalid_mode.c index
>> 630798d8..7e1f683b 100644
>> --- a/tests/kms_invalid_mode.c
>> +++ b/tests/kms_invalid_mode.c
>> @@ -32,6 +32,7 @@ typedef struct _data data_t;
>>    
>>    struct _data {
>>    	int drm_fd;
>> +	enum pipe pipe;
>>    	igt_display_t display;
>>    	igt_output_t *output;
>>    	drmModeResPtr res;
>> @@ -177,21 +178,21 @@ adjust_mode_bad_vtotal(data_t *data, drmModeModeInfoPtr mode)
>>    	return true;
>>    }
>>    
>> -static int
>> +static void
>>    test_output(data_t *data)
>>    {
>>    	igt_output_t *output = data->output;
>>    	drmModeModeInfo mode;
>>    	struct igt_fb fb;
>> -	int i;
>> +	int ret;
>> +	uint32_t crtc_id;
>>    
>>    	/*
>>    	 * FIXME test every mode we have to be more
>>    	 * sure everything is really getting rejected?
>>    	 */
>>    	mode = *igt_output_get_mode(output);
>> -	if (!data->adjust_mode(data, &mode))
>> -		return 0;
>> +	igt_require(data->adjust_mode(data, &mode));
>>    
>>    	igt_create_fb(data->drm_fd,
>>    		      max_t(uint16_t, mode.hdisplay, 64), @@ -202,32 +203,14 @@
>> test_output(data_t *data)
>>    
>>    	kmstest_unset_all_crtcs(data->drm_fd, data->res);
>>    
>> -	for (i = 0; i < data->res->count_crtcs; i++) {
>> -		int ret;
>> -
>> -		igt_info("Checking pipe %c connector %s with mode %s\n",
>> -			 'A'+i, output->name, mode.name);
>> +	crtc_id = data->display.pipes[data->pipe].crtc_id;
>>    
>> -		ret = drmModeSetCrtc(data->drm_fd, data->res->crtcs[i],
>> -				     fb.fb_id, 0, 0,
>> -				     &output->id, 1, &mode);
>> -		igt_assert_lt(ret, 0);
>> -	}
>> +	ret = drmModeSetCrtc(data->drm_fd, crtc_id,
>> +			     fb.fb_id, 0, 0,
>> +			     &output->id, 1, &mode);
> 
> Instead of using drm calls, is it possible to use igt kms wrappers?
> 
> igt_output_set_pipe();
> try_commit();
> 
> - Bhanu
> 
>> +	igt_assert_lt(ret, 0);
>>    
>>    	igt_remove_fb(data->drm_fd, &fb);
>> -
>> -	return 1;
>> -}
>> -
>> -static void test(data_t *data)
>> -{
>> -	int valid_connectors = 0;
>> -
>> -	for_each_connected_output(&data->display, data->output) {
>> -		valid_connectors += test_output(data);
>> -	}
>> -
>> -	igt_require_f(valid_connectors, "No suitable connectors found\n");
>>    }
>>    
>>    static int i915_max_dotclock(data_t *data) @@ -297,6 +280,10 @@
>> static data_t data;
>>    
>>    igt_main
>>    {
>> +
>> +	enum pipe pipe;
>> +	igt_output_t *output;
>> +
>>    	igt_fixture {
>>    		data.drm_fd = drm_open_driver_master(DRIVER_ANY);
>>    
>> @@ -311,9 +298,15 @@ igt_main
>>    	}
>>    
>>    	for (int i = 0; i < ARRAY_SIZE(subtests); i++) {
>> -		igt_subtest(subtests[i].name) {
>> -			data.adjust_mode = subtests[i].adjust_mode;
>> -			test(&data);
>> +		igt_subtest_with_dynamic(subtests[i].name) {
>> +			for_each_pipe_with_valid_output(&data.display, pipe, output) {
>> +				igt_dynamic_f("%s-pipe-%s", igt_output_name(output), kmstest_pipe_name(pipe)) {
>> +					data.output = output;
>> +					data.pipe = pipe;
>> +					data.adjust_mode = subtests[i].adjust_mode;
>> +					test_output(&data);
>> +				}
>> +			}
>>    		}
>>    	}
>>    
> 

Re: [igt-dev] [PATCH i-g-t 1/2] tests/kms_invalid_mode: Convert tests to dynamic

[Petri Latvala]

On Thu, Jun 23, 2022 at 11:43:01AM +0530, Modem, Bhanuprakash wrote:
> On Thu-23-06-2022 11:25 am, Gupta, Nidhi1 wrote:
> > Hi Bhanu,
> > 
> > In many tests we are using drm calls, like drmModePageFlip, drmModeSetCrtc, if I will replace it with igt wrapper it will increase the code lines, should I still change it?
> 
> It's fine to increase the number of lines of code. The idea is to use IGT
> wrappers wherever it's possible, since IGT helpers were developed
> purposefully and more handy.
> 
> Ofcourse, in some tests it is not possible to use these IGT helpers, ex:
> kms_setmode.
> 
> @Petri, any thoughts?

When possible, igt wrappers should be preferred, yes.


-- 
Petri Latvala


> 
> - Bhanu
> 
> > 
> > -----Original Message-----
> > From: Modem, Bhanuprakash <bhanuprakash.modem@intel.com>
> > Sent: Thursday, June 23, 2022 8:11 AM
> > To: Gupta, Nidhi1 <nidhi1.gupta@intel.com>; igt-dev@lists.freedesktop.org
> > Cc: Latvala, Petri <petri.latvala@intel.com>
> > Subject: Re: [PATCH i-g-t 1/2] tests/kms_invalid_mode: Convert tests to dynamic
> > 
> > On Wed-22-06-2022 09:18 pm, Nidhi Gupta wrote:
> > > Convert the existing subtests to dynamic subtests at pipe level.
> > > 
> > > Signed-off-by: Nidhi Gupta <nidhi1.gupta@intel.com>
> > > ---
> > >    tests/kms_invalid_mode.c | 53 +++++++++++++++++-----------------------
> > >    1 file changed, 23 insertions(+), 30 deletions(-)
> > > 
> > > diff --git a/tests/kms_invalid_mode.c b/tests/kms_invalid_mode.c index
> > > 630798d8..7e1f683b 100644
> > > --- a/tests/kms_invalid_mode.c
> > > +++ b/tests/kms_invalid_mode.c
> > > @@ -32,6 +32,7 @@ typedef struct _data data_t;
> > >    struct _data {
> > >    	int drm_fd;
> > > +	enum pipe pipe;
> > >    	igt_display_t display;
> > >    	igt_output_t *output;
> > >    	drmModeResPtr res;
> > > @@ -177,21 +178,21 @@ adjust_mode_bad_vtotal(data_t *data, drmModeModeInfoPtr mode)
> > >    	return true;
> > >    }
> > > -static int
> > > +static void
> > >    test_output(data_t *data)
> > >    {
> > >    	igt_output_t *output = data->output;
> > >    	drmModeModeInfo mode;
> > >    	struct igt_fb fb;
> > > -	int i;
> > > +	int ret;
> > > +	uint32_t crtc_id;
> > >    	/*
> > >    	 * FIXME test every mode we have to be more
> > >    	 * sure everything is really getting rejected?
> > >    	 */
> > >    	mode = *igt_output_get_mode(output);
> > > -	if (!data->adjust_mode(data, &mode))
> > > -		return 0;
> > > +	igt_require(data->adjust_mode(data, &mode));
> > >    	igt_create_fb(data->drm_fd,
> > >    		      max_t(uint16_t, mode.hdisplay, 64), @@ -202,32 +203,14 @@
> > > test_output(data_t *data)
> > >    	kmstest_unset_all_crtcs(data->drm_fd, data->res);
> > > -	for (i = 0; i < data->res->count_crtcs; i++) {
> > > -		int ret;
> > > -
> > > -		igt_info("Checking pipe %c connector %s with mode %s\n",
> > > -			 'A'+i, output->name, mode.name);
> > > +	crtc_id = data->display.pipes[data->pipe].crtc_id;
> > > -		ret = drmModeSetCrtc(data->drm_fd, data->res->crtcs[i],
> > > -				     fb.fb_id, 0, 0,
> > > -				     &output->id, 1, &mode);
> > > -		igt_assert_lt(ret, 0);
> > > -	}
> > > +	ret = drmModeSetCrtc(data->drm_fd, crtc_id,
> > > +			     fb.fb_id, 0, 0,
> > > +			     &output->id, 1, &mode);
> > 
> > Instead of using drm calls, is it possible to use igt kms wrappers?
> > 
> > igt_output_set_pipe();
> > try_commit();
> > 
> > - Bhanu
> > 
> > > +	igt_assert_lt(ret, 0);
> > >    	igt_remove_fb(data->drm_fd, &fb);
> > > -
> > > -	return 1;
> > > -}
> > > -
> > > -static void test(data_t *data)
> > > -{
> > > -	int valid_connectors = 0;
> > > -
> > > -	for_each_connected_output(&data->display, data->output) {
> > > -		valid_connectors += test_output(data);
> > > -	}
> > > -
> > > -	igt_require_f(valid_connectors, "No suitable connectors found\n");
> > >    }
> > >    static int i915_max_dotclock(data_t *data) @@ -297,6 +280,10 @@
> > > static data_t data;
> > >    igt_main
> > >    {
> > > +
> > > +	enum pipe pipe;
> > > +	igt_output_t *output;
> > > +
> > >    	igt_fixture {
> > >    		data.drm_fd = drm_open_driver_master(DRIVER_ANY);
> > > @@ -311,9 +298,15 @@ igt_main
> > >    	}
> > >    	for (int i = 0; i < ARRAY_SIZE(subtests); i++) {
> > > -		igt_subtest(subtests[i].name) {
> > > -			data.adjust_mode = subtests[i].adjust_mode;
> > > -			test(&data);
> > > +		igt_subtest_with_dynamic(subtests[i].name) {
> > > +			for_each_pipe_with_valid_output(&data.display, pipe, output) {
> > > +				igt_dynamic_f("%s-pipe-%s", igt_output_name(output), kmstest_pipe_name(pipe)) {
> > > +					data.output = output;
> > > +					data.pipe = pipe;
> > > +					data.adjust_mode = subtests[i].adjust_mode;
> > > +					test_output(&data);
> > > +				}
> > > +			}
> > >    		}
> > >    	}
> > 
> 

Re: [igt-dev] [PATCH i-g-t 1/2] tests/kms_invalid_mode: Convert tests to dynamic

[Modem, Bhanuprakash]

On Thu-23-06-2022 12:57 pm, Petri Latvala wrote:
> On Thu, Jun 23, 2022 at 11:43:01AM +0530, Modem, Bhanuprakash wrote:
>> On Thu-23-06-2022 11:25 am, Gupta, Nidhi1 wrote:
>>> Hi Bhanu,
>>>
>>> In many tests we are using drm calls, like drmModePageFlip, drmModeSetCrtc, if I will replace it with igt wrapper it will increase the code lines, should I still change it?
>>
>> It's fine to increase the number of lines of code. The idea is to use IGT
>> wrappers wherever it's possible, since IGT helpers were developed
>> purposefully and more handy.
>>
>> Ofcourse, in some tests it is not possible to use these IGT helpers, ex:
>> kms_setmode.
>>
>> @Petri, any thoughts?
> 
> When possible, igt wrappers should be preferred, yes.

Thanks Petri,

@Nidhi, please plan to fix as part of cleanup.

> 
>