[igt-dev] [PATCH i-g-t 1/5] tests/sw_sync: use igt_fork_helper

[igt-dev] [PATCH i-g-t 1/5] tests/sw_sync: use igt_fork_helper

[Daniel Vetter]

I'll need to wrap a bit of magic around all the fork() calls in our
tests. Simplest way to get there is to roll out the existing helpers,
which even saves a bit of boilerplate code.

Cc: Robert Foss <robert.foss@collabora.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 tests/sw_sync.c | 54 +++++++++++++++++--------------------------------
 1 file changed, 18 insertions(+), 36 deletions(-)

diff --git a/tests/sw_sync.c b/tests/sw_sync.c
index 59603763f663..207908ed7e70 100644
--- a/tests/sw_sync.c
+++ b/tests/sw_sync.c
@@ -175,6 +175,7 @@ static void test_sync_busy_fork_unixsocket(void)
 	int timeline;
 	int skip = 0;
 	int sv[2];
+	struct igt_helper_process proc;
 
 
 	timeline = sw_sync_timeline_create();
@@ -185,9 +186,7 @@ static void test_sync_busy_fork_unixsocket(void)
 		goto out;
 	}
 
-	switch (fork()) {
-	case 0:
-	{
+	igt_fork_helper(&proc) {
 		/* Child process */
 		int socket = sv[1];
 		int socket_timeline;
@@ -213,17 +212,8 @@ static void test_sync_busy_fork_unixsocket(void)
 
 		/* Advance timeline from 0 -> 1 */
 		sw_sync_timeline_inc(socket_timeline, 1);
-
-		_Exit(0);
-		break;
-	}
-	case -1:
-	{
-		/* Failed fork */
-		skip = 1;
-		break;
 	}
-	default:
+
 	{
 		/* Parent process */
 		int socket = sv[0];
@@ -252,15 +242,14 @@ static void test_sync_busy_fork_unixsocket(void)
 
 		if (sendmsg(socket, &msg, 0) < 0) {
 		    skip = 1;
-		    goto out;
+		} else {
+			igt_assert_f(sync_fence_wait(fence, 2*1000) == 0,
+				     "Fence not signaled (timeline value 1 fence seqno 1)\n");
 		}
-
-		igt_assert_f(sync_fence_wait(fence, 2*1000) == 0,
-			     "Fence not signaled (timeline value 1 fence seqno 1)\n");
-		break;
-	}
 	}
 
+	igt_stop_helper(&proc);
+
 out:
 	close(fence);
 	close(timeline);
@@ -272,32 +261,25 @@ static void test_sync_busy_fork(void)
 	int fence;
 	int timeline;
 	int skip = 0;
+	struct igt_helper_process proc;
 
 	timeline = sw_sync_timeline_create();
 	fence = sw_sync_timeline_create_fence(timeline, 1);
 
-	switch (fork()) {
-	case 0:
-		/* Child process */
+	igt_fork_helper(&proc) {
 		usleep(1*1000*1000);
 		/* Advance timeline from 0 -> 1 */
 		sw_sync_timeline_inc(timeline, 1);
-		_Exit(0);
-		break;
-	case -1:
-		/* Failed fork */
-		skip = 1;
-		break;
-	default:
-		/* Parent process */
-		igt_assert_f(sync_fence_wait(fence, 0) == -ETIME,
-			     "Fence signaled (it should not have been signalled yet)\n");
-
-		igt_assert_f(sync_fence_wait(fence, 2*1000) == 0,
-			     "Fence not signaled (timeline value 1 fence seqno 1)\n");
-		break;
 	}
 
+	igt_assert_f(sync_fence_wait(fence, 0) == -ETIME,
+		     "Fence signaled (it should not have been signalled yet)\n");
+
+	igt_assert_f(sync_fence_wait(fence, 2*1000) == 0,
+		     "Fence not signaled (timeline value 1 fence seqno 1)\n");
+
+	igt_stop_helper(&proc);
+
 	close(fence);
 	close(timeline);
 	igt_require(!skip);
-- 
2.19.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 2/5] tests/debugfs: use igt_display_require

[Daniel Vetter]

Need to extract into a test subgroup to make sure we only skip the
tests that need display support.

v2: Chris pointed out that "read-all-entries" was the original non-kms
tests, and we don't want to skip that if there's no output. Make a
seperate test for this.

Also, that kind of where libraries magically second-guess what the
test might have wanted when it supplies an invalid request is exactly
why I want to fix the igt_display_init API regression.

Cc: Antonio Argenziano <antonio.argenziano@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 tests/debugfs_test.c | 37 ++++++++++++++++++++++++-------------
 1 file changed, 24 insertions(+), 13 deletions(-)

diff --git a/tests/debugfs_test.c b/tests/debugfs_test.c
index 2e87e4420b15..eb32932ed686 100644
--- a/tests/debugfs_test.c
+++ b/tests/debugfs_test.c
@@ -87,23 +87,14 @@ static void read_and_discard_sysfs_entries(int path_fd, int indent)
 	closedir(dir);
 }
 
-igt_main
+static void kms_tests(int fd, int debugfs)
 {
-	int fd = -1, debugfs;
 	igt_display_t display;
 	struct igt_fb fb[IGT_MAX_PIPES];
 	enum pipe pipe;
 
-	igt_skip_on_simulation();
-
-	igt_fixture {
-		fd = drm_open_driver_master(DRIVER_INTEL);
-		igt_require_gem(fd);
-		debugfs = igt_debugfs_dir(fd);
-
-		kmstest_set_vt_graphics_mode();
-		igt_display_init(&display, fd);
-	}
+	igt_fixture
+		igt_display_require(&display, fd);
 
 	igt_subtest("read_all_entries") {
 		/* try to light all pipes */
@@ -152,6 +143,27 @@ igt_main
 		read_and_discard_sysfs_entries(debugfs, 0);
 	}
 
+	igt_fixture
+		igt_display_fini(&display);
+}
+
+igt_main
+{
+	int fd = -1, debugfs;
+
+	igt_skip_on_simulation();
+
+	igt_fixture {
+		fd = drm_open_driver_master(DRIVER_INTEL);
+		igt_require_gem(fd);
+		debugfs = igt_debugfs_dir(fd);
+
+		kmstest_set_vt_graphics_mode();
+	}
+
+	igt_subtest_group
+		kms_tests(fd, debugfs);
+
 	igt_subtest("emon_crash") {
 		int i;
 		/*
@@ -174,7 +186,6 @@ igt_main
 	}
 
 	igt_fixture {
-		igt_display_fini(&display);
 		close(debugfs);
 		close(fd);
 	}
-- 
2.19.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 3/5] tests: Use igt_display_require

[Daniel Vetter]

Remaining tests that have been overlooked and don't need any
invasive changes to limit the skipping to only the relevant parts.

v2: read-all-entries was supposed to be the original non-KMS test.
Split them up for clarity.

Cc: Antonio Argenziano <antonio.argenziano@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 lib/igt_chamelium.c               | 2 +-
 tests/debugfs_test.c              | 5 ++++-
 tests/kms_atomic_interruptible.c  | 4 ++--
 tests/kms_force_connector_basic.c | 2 +-
 tests/kms_getfb.c                 | 2 +-
 tests/kms_plane_alpha_blend.c     | 2 +-
 tests/perf_pmu.c                  | 2 +-
 tests/pm_backlight.c              | 2 +-
 tests/prime_mmap_kms.c            | 2 +-
 9 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/lib/igt_chamelium.c b/lib/igt_chamelium.c
index a80ead163846..d136fb04c342 100644
--- a/lib/igt_chamelium.c
+++ b/lib/igt_chamelium.c
@@ -1547,7 +1547,7 @@ static void chamelium_exit_handler(int sig)
 /**
  * chamelium_init:
  * @chamelium: The Chamelium instance to use
- * @drm_fd: a display initialized with #igt_display_init
+ * @drm_fd: a display initialized with #igt_display_require
  *
  * Sets up a connection with a chamelium, using the URL specified in the
  * Chamelium configuration. This must be called first before trying to use the
diff --git a/tests/debugfs_test.c b/tests/debugfs_test.c
index eb32932ed686..6a87d90afd0e 100644
--- a/tests/debugfs_test.c
+++ b/tests/debugfs_test.c
@@ -96,7 +96,7 @@ static void kms_tests(int fd, int debugfs)
 	igt_fixture
 		igt_display_require(&display, fd);
 
-	igt_subtest("read_all_entries") {
+	igt_subtest("read_all_entries_display_on") {
 		/* try to light all pipes */
 		for_each_pipe(&display, pipe) {
 			igt_output_t *output;
@@ -161,6 +161,9 @@ igt_main
 		kmstest_set_vt_graphics_mode();
 	}
 
+	igt_subtest("read_all_entries")
+		read_and_discard_sysfs_entries(debugfs, 0);
+
 	igt_subtest_group
 		kms_tests(fd, debugfs);
 
diff --git a/tests/kms_atomic_interruptible.c b/tests/kms_atomic_interruptible.c
index 8e9d4cb69c4d..be688638973f 100644
--- a/tests/kms_atomic_interruptible.c
+++ b/tests/kms_atomic_interruptible.c
@@ -85,8 +85,8 @@ static void run_plane_test(igt_display_t *display, enum pipe pipe, igt_output_t
 
 	/*
 	 * Make sure we start with everything disabled to force a real modeset.
-	 * igt_display_init only sets sw state, and assumes the first test doesn't care
-	 * about hw state.
+	 * igt_display_require only sets sw state, and assumes the first test
+	 * doesn't care about hw state.
 	 */
 	igt_display_commit2(display, COMMIT_ATOMIC);
 
diff --git a/tests/kms_force_connector_basic.c b/tests/kms_force_connector_basic.c
index e9325dec9305..b8246e669939 100644
--- a/tests/kms_force_connector_basic.c
+++ b/tests/kms_force_connector_basic.c
@@ -217,7 +217,7 @@ int main(int argc, char **argv)
 
 		/* attempt to use the display */
 		kmstest_set_vt_graphics_mode();
-		igt_display_init(&display, drm_fd);
+		igt_display_require(&display, drm_fd);
 		igt_display_commit(&display);
 		igt_display_fini(&display);
 
diff --git a/tests/kms_getfb.c b/tests/kms_getfb.c
index 07ffd79c4613..ca0b01c05e5c 100644
--- a/tests/kms_getfb.c
+++ b/tests/kms_getfb.c
@@ -116,7 +116,7 @@ static uint32_t get_any_prop_id(int fd)
 {
 	igt_display_t display;
 
-	igt_display_init(&display, fd);
+	igt_display_require(&display, fd);
 	for (int i = 0; i < display.n_outputs; i++) {
 		igt_output_t *output = &display.outputs[i];
 		if (output->props[IGT_CONNECTOR_DPMS] != 0)
diff --git a/tests/kms_plane_alpha_blend.c b/tests/kms_plane_alpha_blend.c
index 3fab118ae0e1..1d9d8933d7e2 100644
--- a/tests/kms_plane_alpha_blend.c
+++ b/tests/kms_plane_alpha_blend.c
@@ -565,7 +565,7 @@ igt_main
 	igt_fixture {
 		data.gfx_fd = drm_open_driver(DRIVER_ANY);
 		igt_require_pipe_crc(data.gfx_fd);
-		igt_display_init(&data.display, data.gfx_fd);
+		igt_display_require(&data.display, data.gfx_fd);
 		igt_require(data.display.is_atomic);
 	}
 
diff --git a/tests/perf_pmu.c b/tests/perf_pmu.c
index b34bc66ce2c4..21292bf3a2fe 100644
--- a/tests/perf_pmu.c
+++ b/tests/perf_pmu.c
@@ -811,7 +811,7 @@ event_wait(int gem_fd, const struct intel_execution_engine2 *e)
 	igt_skip_on(IS_VALLEYVIEW(devid) || IS_CHERRYVIEW(devid));
 
 	kmstest_set_vt_graphics_mode();
-	igt_display_init(&data.display, gem_fd);
+	igt_display_require(&data.display, gem_fd);
 
 	/**
 	 * We will use the display to render event forwarind so need to
diff --git a/tests/pm_backlight.c b/tests/pm_backlight.c
index 32808cdf6ca4..054300f6e2e1 100644
--- a/tests/pm_backlight.c
+++ b/tests/pm_backlight.c
@@ -214,7 +214,7 @@ igt_main
 		 * try to enable all.
 		 */
 		kmstest_set_vt_graphics_mode();
-		igt_display_init(&display, drm_open_driver(DRIVER_INTEL));
+		igt_display_require(&display, drm_open_driver(DRIVER_INTEL));
 
 		/* should be ../../cardX-$output */
 		igt_assert_lt(12, readlink(BACKLIGHT_PATH "/device", full_name, sizeof(full_name) - 1));
diff --git a/tests/prime_mmap_kms.c b/tests/prime_mmap_kms.c
index faace4afd478..fdc37214d96d 100644
--- a/tests/prime_mmap_kms.c
+++ b/tests/prime_mmap_kms.c
@@ -248,7 +248,7 @@ igt_main
 
 		igt_require_pipe_crc(gpu.drm_fd);
 
-		igt_display_init(&gpu.display, gpu.drm_fd);
+		igt_display_require(&gpu.display, gpu.drm_fd);
 	}
 
 	igt_subtest("buffer-sharing")
-- 
2.19.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 4/5] lib/kms: Drop igt_display_init

[Daniel Vetter]

If you need the high-level functions, then you probably need a
full display. Unexport the non-requiring version, and adjust the
documentation. This also gives us proper docs for the recently
added igt_display_require.

v2: Rebase, kms_content_protection is new.

Cc: Antonio Argenziano <antonio.argenziano@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 lib/igt_kms.c                  | 14 +++++---------
 lib/igt_kms.h                  |  1 -
 tests/kms_content_protection.c |  3 ++-
 3 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index d806ccc1e0b9..7214101e2696 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -1861,16 +1861,17 @@ static void igt_fill_plane_format_mod(igt_display_t *display, igt_plane_t *plane
 static void igt_fill_display_format_mod(igt_display_t *display);
 
 /**
- * igt_display_init:
+ * igt_display_require:
  * @display: a pointer to an #igt_display_t structure
  * @drm_fd: a drm file descriptor
  *
  * Initialize @display and allocate the various resources required. Use
  * #igt_display_fini to release the resources when they are no longer required.
  *
- * Returns: true if the display has outputs and pipes available, false otherwise
+ * This function automatically skips if the kernel driver doesn't support any
+ * CRTC or outputs.
  */
-bool igt_display_init(igt_display_t *display, int drm_fd)
+void igt_display_require(igt_display_t *display, int drm_fd)
 {
 	drmModeRes *resources;
 	drmModePlaneRes *plane_resources;
@@ -2034,12 +2035,7 @@ bool igt_display_init(igt_display_t *display, int drm_fd)
 out:
 	LOG_UNINDENT(display);
 
-	return display->n_pipes && display->n_outputs;
-}
-
-void igt_display_require(igt_display_t *display, int drm_fd)
-{
-	igt_require(igt_display_init(display, drm_fd));
+	igt_require(display->n_pipes && display->n_outputs);
 }
 
 /**
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index e6aff339af60..09395360007f 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -382,7 +382,6 @@ struct igt_display {
 	int format_mod_count;
 };
 
-bool igt_display_init(igt_display_t *display, int drm_fd);
 void igt_display_require(igt_display_t *display, int drm_fd);
 void igt_display_fini(igt_display_t *display);
 void igt_display_reset(igt_display_t *display);
diff --git a/tests/kms_content_protection.c b/tests/kms_content_protection.c
index 801eff66c272..4818e021f9c5 100644
--- a/tests/kms_content_protection.c
+++ b/tests/kms_content_protection.c
@@ -26,6 +26,7 @@
 #include <fcntl.h>
 #include "igt.h"
 #include "igt_sysfs.h"
+#include "igt_kms.h"
 
 IGT_TEST_DESCRIPTION("Test content protection (HDCP)");
 
@@ -290,7 +291,7 @@ igt_main
 
 		data.drm_fd = drm_open_driver(DRIVER_ANY);
 
-		igt_display_init(&data.display, data.drm_fd);
+		igt_display_require(&data.display, data.drm_fd);
 	}
 
 	igt_subtest("legacy")
-- 
2.19.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 5/5] lib/kms: warn if we commit without outputs

[Daniel Vetter]

With the high-level helpers requiring outputs there's not point
in silently ignoring issues anymore. Complain about that if it
ever happens.

This reverts

commit 212b71372bfbb73663d872df31118d6b396ada4f
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Fri Sep 14 21:03:38 2018 +0100

    lib/kms: Skip no-op display updates

which created an in my opinion serious API issue by silently dropping
possible errors on the floor. Instead of silently second guess what
the test might have wanted to do in the absence of display outputs
it's much better to be explicit, and enforce that.

v2: Improve commit message.

Cc: Antonio Argenziano <antonio.argenziano@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 lib/igt_kms.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 7214101e2696..5a35f6bbc177 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -3292,7 +3292,7 @@ static int do_display_commit(igt_display_t *display,
 	enum pipe pipe;
 	LOG_INDENT(display, "commit");
 
-	if (!display->n_pipes || !display->n_outputs)
+	if (igt_warn_on(!display->n_pipes || !display->n_outputs))
 		return 0; /* nothing to do */
 
 	igt_display_refresh(display);
@@ -3345,7 +3345,7 @@ int igt_display_try_commit_atomic(igt_display_t *display, uint32_t flags, void *
 {
 	int ret;
 
-	if (!display->n_pipes || !display->n_outputs)
+	if (igt_warn_on(!display->n_pipes || !display->n_outputs))
 		return 0; /* nothing to do */
 
 	LOG_INDENT(display, "commit");
-- 
2.19.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,1/5] tests/sw_sync: use igt_fork_helper

[Patchwork]

== Series Details ==

Series: series starting with [i-g-t,1/5] tests/sw_sync: use igt_fork_helper
URL   : https://patchwork.freedesktop.org/series/51928/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_5064 -> IGTPW_2027 =

== Summary - WARNING ==

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

  External URL: https://patchwork.freedesktop.org/api/1.0/series/51928/revisions/1/mbox/

== Possible new issues ==

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

  === IGT changes ===

    ==== Warnings ====

    igt@drv_selftest@live_guc:
      fi-apl-guc:         SKIP -> PASS
      fi-skl-iommu:       SKIP -> PASS +1

    igt@kms_flip@basic-flip-vs-wf_vblank:
      fi-glk-j4005:       SKIP -> PASS

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_selftest@live_contexts:
      fi-icl-u:           NOTRUN -> DMESG-FAIL (fdo#108569)

    igt@drv_selftest@live_hangcheck:
      fi-kbl-7560u:       PASS -> INCOMPLETE (fdo#108044)

    igt@kms_frontbuffer_tracking@basic:
      fi-byt-clapper:     PASS -> FAIL (fdo#103167)

    igt@kms_pipe_crc_basic@hang-read-crc-pipe-a:
      fi-byt-clapper:     PASS -> FAIL (fdo#107362, fdo#103191)

    
    ==== Possible fixes ====

    igt@drv_selftest@live_execlists:
      fi-apl-guc:         INCOMPLETE (fdo#106693) -> PASS

    igt@drv_selftest@live_hangcheck:
      fi-skl-iommu:       INCOMPLETE (fdo#108602) -> PASS

    igt@gem_exec_suspend@basic-s3:
      fi-icl-u:           INCOMPLETE (fdo#107713) -> PASS

    igt@kms_flip@basic-flip-vs-modeset:
      fi-glk-j4005:       DMESG-WARN (fdo#106000) -> PASS

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b-frame-sequence:
      fi-byt-clapper:     FAIL (fdo#107362, fdo#103191) -> PASS

    
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#106000 https://bugs.freedesktop.org/show_bug.cgi?id=106000
  fdo#106693 https://bugs.freedesktop.org/show_bug.cgi?id=106693
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
  fdo#107713 https://bugs.freedesktop.org/show_bug.cgi?id=107713
  fdo#108044 https://bugs.freedesktop.org/show_bug.cgi?id=108044
  fdo#108569 https://bugs.freedesktop.org/show_bug.cgi?id=108569
  fdo#108602 https://bugs.freedesktop.org/show_bug.cgi?id=108602


== Participating hosts (47 -> 42) ==

  Missing    (5): fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-bwr-2160 fi-ctg-p8600 


== Build changes ==

    * IGT: IGT_4703 -> IGTPW_2027

  CI_DRM_5064: 9458f0c67b519ddf1c2b4eec9110e826c4e7d2f3 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2027: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2027/
  IGT_4703: f882a542a3eb24e78e51aa6410a3a67c0efb4e97 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools



== Testlist changes ==

+igt@debugfs_test@read_all_entries_display_on

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2027/issues.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 2/5] tests/debugfs: use igt_display_require

[Chris Wilson]

Quoting Daniel Vetter (2018-11-02 09:57:23)
> Need to extract into a test subgroup to make sure we only skip the
> tests that need display support.
> 
> v2: Chris pointed out that "read-all-entries" was the original non-kms
> tests, and we don't want to skip that if there's no output. Make a
> seperate test for this.
> 
> Also, that kind of where libraries magically second-guess what the
> test might have wanted when it supplies an invalid request is exactly
> why I want to fix the igt_display_init API regression.

So strongly disagree about who is second guessing. A degenerate request
is still valid and I think a desirably property of the API.

You are handed a valid request to do nothing, stop complaining about it.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✓ Fi.CI.IGT: success for series starting with [i-g-t,1/5] tests/sw_sync: use igt_fork_helper

[Patchwork]

== Series Details ==

Series: series starting with [i-g-t,1/5] tests/sw_sync: use igt_fork_helper
URL   : https://patchwork.freedesktop.org/series/51928/
State : success

== Summary ==

= CI Bug Log - changes from IGT_4703_full -> IGTPW_2027_full =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/51928/revisions/1/mbox/

== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_exec_reuse@contexts:
      shard-glk:          PASS -> INCOMPLETE (fdo#103359, k.org#198133)

    igt@gem_exec_schedule@pi-ringfull-vebox:
      shard-glk:          NOTRUN -> FAIL (fdo#103158)

    igt@gem_mocs_settings@mocs-reset-dirty-render:
      shard-apl:          PASS -> INCOMPLETE (fdo#103927)

    igt@kms_busy@extended-modeset-hang-newfb-render-a:
      shard-snb:          PASS -> DMESG-WARN (fdo#107956)

    igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-b:
      shard-snb:          NOTRUN -> DMESG-WARN (fdo#107956) +1

    igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-b:
      shard-apl:          PASS -> DMESG-WARN (fdo#107956)

    igt@kms_cursor_crc@cursor-128x128-random:
      shard-apl:          PASS -> FAIL (fdo#103232) +1

    igt@kms_cursor_crc@cursor-128x128-suspend:
      shard-apl:          PASS -> FAIL (fdo#103191, fdo#103232) +1
      shard-kbl:          PASS -> FAIL (fdo#103191, fdo#103232)

    igt@kms_cursor_crc@cursor-128x42-random:
      shard-glk:          PASS -> FAIL (fdo#103232) +3

    igt@kms_cursor_crc@cursor-256x256-dpms:
      shard-kbl:          PASS -> FAIL (fdo#103232)

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-move:
      shard-snb:          PASS -> INCOMPLETE (fdo#105411)

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-onoff:
      shard-apl:          PASS -> FAIL (fdo#103167) +3

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-render:
      shard-kbl:          PASS -> FAIL (fdo#103167) +1

    igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-fullscreen:
      shard-glk:          PASS -> FAIL (fdo#103167) +3

    igt@kms_plane@plane-position-covered-pipe-a-planes:
      shard-kbl:          PASS -> FAIL (fdo#103166)

    igt@kms_plane_alpha_blend@pipe-c-alpha-7efc:
      shard-apl:          PASS -> FAIL (fdo#108145)

    igt@kms_plane_multiple@atomic-pipe-a-tiling-y:
      shard-apl:          PASS -> FAIL (fdo#103166) +1

    igt@kms_universal_plane@universal-plane-pipe-c-functional:
      shard-glk:          PASS -> FAIL (fdo#103166) +2

    igt@kms_vblank@pipe-c-ts-continuation-modeset-hang:
      shard-apl:          PASS -> DMESG-WARN (fdo#103558, fdo#105602) +1

    igt@perf@oa-exponents:
      shard-glk:          PASS -> FAIL (fdo#105483)

    
    ==== Possible fixes ====

    igt@drv_suspend@shrink:
      shard-snb:          INCOMPLETE (fdo#106886, fdo#105411) -> PASS

    igt@gem_exec_faulting_reloc@normal:
      shard-glk:          DMESG-WARN (fdo#106538, fdo#105763) -> PASS +1

    igt@kms_available_modes_crc@available_mode_test_crc:
      shard-apl:          FAIL (fdo#106641) -> PASS

    igt@kms_ccs@pipe-a-crc-sprite-planes-basic:
      shard-glk:          FAIL (fdo#108145) -> PASS

    igt@kms_cursor_crc@cursor-128x42-sliding:
      shard-apl:          FAIL (fdo#103232) -> PASS +2

    igt@kms_cursor_crc@cursor-256x256-dpms:
      shard-glk:          FAIL (fdo#103232) -> PASS +3

    igt@kms_cursor_crc@cursor-256x256-suspend:
      shard-apl:          FAIL (fdo#103191, fdo#103232) -> PASS

    igt@kms_cursor_crc@cursor-64x64-dpms:
      shard-kbl:          FAIL (fdo#103232) -> PASS +1

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-mmap-wc:
      shard-kbl:          FAIL (fdo#103167) -> PASS

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-cpu:
      shard-apl:          FAIL (fdo#103167) -> PASS +1

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-onoff:
      shard-glk:          FAIL (fdo#103167) -> PASS +3

    igt@kms_plane_multiple@atomic-pipe-b-tiling-y:
      shard-glk:          FAIL (fdo#103166) -> PASS +5

    igt@kms_plane_multiple@atomic-pipe-c-tiling-yf:
      shard-apl:          FAIL (fdo#103166) -> PASS +2
      shard-kbl:          FAIL (fdo#103166) -> PASS +3

    igt@kms_setmode@basic:
      shard-kbl:          FAIL (fdo#99912) -> PASS

    igt@kms_vblank@pipe-c-query-idle-hang:
      shard-glk:          INCOMPLETE (fdo#103359, k.org#198133) -> PASS

    
  fdo#103158 https://bugs.freedesktop.org/show_bug.cgi?id=103158
  fdo#103166 https://bugs.freedesktop.org/show_bug.cgi?id=103166
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#103232 https://bugs.freedesktop.org/show_bug.cgi?id=103232
  fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
  fdo#103558 https://bugs.freedesktop.org/show_bug.cgi?id=103558
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
  fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411
  fdo#105483 https://bugs.freedesktop.org/show_bug.cgi?id=105483
  fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602
  fdo#105763 https://bugs.freedesktop.org/show_bug.cgi?id=105763
  fdo#106538 https://bugs.freedesktop.org/show_bug.cgi?id=106538
  fdo#106641 https://bugs.freedesktop.org/show_bug.cgi?id=106641
  fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886
  fdo#107956 https://bugs.freedesktop.org/show_bug.cgi?id=107956
  fdo#108145 https://bugs.freedesktop.org/show_bug.cgi?id=108145
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
  k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133


== Participating hosts (6 -> 5) ==

  Missing    (1): shard-skl 


== Build changes ==

    * IGT: IGT_4703 -> IGTPW_2027
    * Linux: CI_DRM_5060 -> CI_DRM_5064

  CI_DRM_5060: 68732cb54bff70449aa3bc5a500a20b8629f53fa @ git://anongit.freedesktop.org/gfx-ci/linux
  CI_DRM_5064: 9458f0c67b519ddf1c2b4eec9110e826c4e7d2f3 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2027: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2027/
  IGT_4703: f882a542a3eb24e78e51aa6410a3a67c0efb4e97 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2027/shards.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 5/5] lib/kms: warn if we commit without outputs

[Maarten Lankhorst]

Op 02-11-18 om 10:57 schreef Daniel Vetter:
> With the high-level helpers requiring outputs there's not point
> in silently ignoring issues anymore. Complain about that if it
> ever happens.
>
> This reverts
>
> commit 212b71372bfbb73663d872df31118d6b396ada4f
> Author: Chris Wilson <chris@chris-wilson.co.uk>
> Date:   Fri Sep 14 21:03:38 2018 +0100
>
>     lib/kms: Skip no-op display updates
>
> which created an in my opinion serious API issue by silently dropping
> possible errors on the floor. Instead of silently second guess what
> the test might have wanted to do in the absence of display outputs
> it's much better to be explicit, and enforce that.
>
> v2: Improve commit message.
>
> Cc: Antonio Argenziano <antonio.argenziano@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  lib/igt_kms.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 7214101e2696..5a35f6bbc177 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -3292,7 +3292,7 @@ static int do_display_commit(igt_display_t *display,
>  	enum pipe pipe;
>  	LOG_INDENT(display, "commit");
>  
> -	if (!display->n_pipes || !display->n_outputs)
> +	if (igt_warn_on(!display->n_pipes || !display->n_outputs))
>  		return 0; /* nothing to do */
>  
>  	igt_display_refresh(display);
> @@ -3345,7 +3345,7 @@ int igt_display_try_commit_atomic(igt_display_t *display, uint32_t flags, void *
>  {
>  	int ret;
>  
> -	if (!display->n_pipes || !display->n_outputs)
> +	if (igt_warn_on(!display->n_pipes || !display->n_outputs))
>  		return 0; /* nothing to do */
>  
>  	LOG_INDENT(display, "commit");

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 1/5] tests/sw_sync: use igt_fork_helper

[Robert Foss]

Hey Daniel,

This looks good and compiles for me, feel free to add my r-b:
Reviewed-by: Robert Foss <robert.foss@collabora.com>

On 2018-11-02 10:57, Daniel Vetter wrote:
> I'll need to wrap a bit of magic around all the fork() calls in our
> tests. Simplest way to get there is to roll out the existing helpers,
> which even saves a bit of boilerplate code.
> 
> Cc: Robert Foss <robert.foss@collabora.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>   tests/sw_sync.c | 54 +++++++++++++++++--------------------------------
>   1 file changed, 18 insertions(+), 36 deletions(-)
> 
> diff --git a/tests/sw_sync.c b/tests/sw_sync.c
> index 59603763f663..207908ed7e70 100644
> --- a/tests/sw_sync.c
> +++ b/tests/sw_sync.c
> @@ -175,6 +175,7 @@ static void test_sync_busy_fork_unixsocket(void)
>   	int timeline;
>   	int skip = 0;
>   	int sv[2];
> +	struct igt_helper_process proc;
>   
>   
>   	timeline = sw_sync_timeline_create();
> @@ -185,9 +186,7 @@ static void test_sync_busy_fork_unixsocket(void)
>   		goto out;
>   	}
>   
> -	switch (fork()) {
> -	case 0:
> -	{
> +	igt_fork_helper(&proc) {
>   		/* Child process */
>   		int socket = sv[1];
>   		int socket_timeline;
> @@ -213,17 +212,8 @@ static void test_sync_busy_fork_unixsocket(void)
>   
>   		/* Advance timeline from 0 -> 1 */
>   		sw_sync_timeline_inc(socket_timeline, 1);
> -
> -		_Exit(0);
> -		break;
> -	}
> -	case -1:
> -	{
> -		/* Failed fork */
> -		skip = 1;
> -		break;
>   	}
> -	default:
> +
>   	{
>   		/* Parent process */
>   		int socket = sv[0];
> @@ -252,15 +242,14 @@ static void test_sync_busy_fork_unixsocket(void)
>   
>   		if (sendmsg(socket, &msg, 0) < 0) {
>   		    skip = 1;
> -		    goto out;
> +		} else {
> +			igt_assert_f(sync_fence_wait(fence, 2*1000) == 0,
> +				     "Fence not signaled (timeline value 1 fence seqno 1)\n");
>   		}
> -
> -		igt_assert_f(sync_fence_wait(fence, 2*1000) == 0,
> -			     "Fence not signaled (timeline value 1 fence seqno 1)\n");
> -		break;
> -	}
>   	}
>   
> +	igt_stop_helper(&proc);
> +
>   out:
>   	close(fence);
>   	close(timeline);
> @@ -272,32 +261,25 @@ static void test_sync_busy_fork(void)
>   	int fence;
>   	int timeline;
>   	int skip = 0;
> +	struct igt_helper_process proc;
>   
>   	timeline = sw_sync_timeline_create();
>   	fence = sw_sync_timeline_create_fence(timeline, 1);
>   
> -	switch (fork()) {
> -	case 0:
> -		/* Child process */
> +	igt_fork_helper(&proc) {
>   		usleep(1*1000*1000);
>   		/* Advance timeline from 0 -> 1 */
>   		sw_sync_timeline_inc(timeline, 1);
> -		_Exit(0);
> -		break;
> -	case -1:
> -		/* Failed fork */
> -		skip = 1;
> -		break;
> -	default:
> -		/* Parent process */
> -		igt_assert_f(sync_fence_wait(fence, 0) == -ETIME,
> -			     "Fence signaled (it should not have been signalled yet)\n");
> -
> -		igt_assert_f(sync_fence_wait(fence, 2*1000) == 0,
> -			     "Fence not signaled (timeline value 1 fence seqno 1)\n");
> -		break;
>   	}
>   
> +	igt_assert_f(sync_fence_wait(fence, 0) == -ETIME,
> +		     "Fence signaled (it should not have been signalled yet)\n");
> +
> +	igt_assert_f(sync_fence_wait(fence, 2*1000) == 0,
> +		     "Fence not signaled (timeline value 1 fence seqno 1)\n");
> +
> +	igt_stop_helper(&proc);
> +
>   	close(fence);
>   	close(timeline);
>   	igt_require(!skip);
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 2/5] tests/debugfs: use igt_display_require

[Antonio Argenziano]

On 02/11/18 02:57, Daniel Vetter wrote:
> Need to extract into a test subgroup to make sure we only skip the
> tests that need display support.
> 
> v2: Chris pointed out that "read-all-entries" was the original non-kms
> tests, and we don't want to skip that if there's no output. Make a
> seperate test for this.

Is the separate test implemented in this patch? I only see you adding a 
kms_tests() where 'read_all_entries' is  after an igt_display_require().

> 
> Also, that kind of where libraries magically second-guess what the
> test might have wanted when it supplies an invalid request is exactly
> why I want to fix the igt_display_init API regression.
> 
> Cc: Antonio Argenziano <antonio.argenziano@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>   tests/debugfs_test.c | 37 ++++++++++++++++++++++++-------------
>   1 file changed, 24 insertions(+), 13 deletions(-)
> 
> diff --git a/tests/debugfs_test.c b/tests/debugfs_test.c
> index 2e87e4420b15..eb32932ed686 100644
> --- a/tests/debugfs_test.c
> +++ b/tests/debugfs_test.c
> @@ -87,23 +87,14 @@ static void read_and_discard_sysfs_entries(int path_fd, int indent)
>   	closedir(dir);
>   }
>   
> -igt_main
> +static void kms_tests(int fd, int debugfs)
>   {
> -	int fd = -1, debugfs;
>   	igt_display_t display;
>   	struct igt_fb fb[IGT_MAX_PIPES];
>   	enum pipe pipe;
>   
> -	igt_skip_on_simulation();
> -
> -	igt_fixture {
> -		fd = drm_open_driver_master(DRIVER_INTEL);
> -		igt_require_gem(fd);
> -		debugfs = igt_debugfs_dir(fd);
> -
> -		kmstest_set_vt_graphics_mode();
> -		igt_display_init(&display, fd);
> -	}
> +	igt_fixture
> +		igt_display_require(&display, fd);
>   
>   	igt_subtest("read_all_entries") {
>   		/* try to light all pipes */
> @@ -152,6 +143,27 @@ igt_main
>   		read_and_discard_sysfs_entries(debugfs, 0);
>   	}
>   
> +	igt_fixture
> +		igt_display_fini(&display);
> +}
> +
> +igt_main
> +{
> +	int fd = -1, debugfs;
> +
> +	igt_skip_on_simulation();

I think this^ can be removed.

Antonio

> +
> +	igt_fixture {
> +		fd = drm_open_driver_master(DRIVER_INTEL);
> +		igt_require_gem(fd);
> +		debugfs = igt_debugfs_dir(fd);
> +
> +		kmstest_set_vt_graphics_mode();
> +	}
> +
> +	igt_subtest_group
> +		kms_tests(fd, debugfs);
> +
>   	igt_subtest("emon_crash") {
>   		int i;
>   		/*
> @@ -174,7 +186,6 @@ igt_main
>   	}
>   
>   	igt_fixture {
> -		igt_display_fini(&display);
>   		close(debugfs);
>   		close(fd);
>   	}
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 2/5] tests/debugfs: use igt_display_require

[Daniel Vetter]

On Tue, Nov 6, 2018 at 6:18 PM Antonio Argenziano
<antonio.argenziano@intel.com> wrote:
>
>
>
> On 02/11/18 02:57, Daniel Vetter wrote:
> > Need to extract into a test subgroup to make sure we only skip the
> > tests that need display support.
> >
> > v2: Chris pointed out that "read-all-entries" was the original non-kms
> > tests, and we don't want to skip that if there's no output. Make a
> > seperate test for this.
>
> Is the separate test implemented in this patch? I only see you adding a
> kms_tests() where 'read_all_entries' is  after an igt_display_require().

Oops, squashed it accidentally into the wrong patch :-/ One the
missing hunks are in the next one. I'll rebase a bunch and resend.
-Daniel

>
> >
> > Also, that kind of where libraries magically second-guess what the
> > test might have wanted when it supplies an invalid request is exactly
> > why I want to fix the igt_display_init API regression.
> >
> > Cc: Antonio Argenziano <antonio.argenziano@intel.com>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > ---
> >   tests/debugfs_test.c | 37 ++++++++++++++++++++++++-------------
> >   1 file changed, 24 insertions(+), 13 deletions(-)
> >
> > diff --git a/tests/debugfs_test.c b/tests/debugfs_test.c
> > index 2e87e4420b15..eb32932ed686 100644
> > --- a/tests/debugfs_test.c
> > +++ b/tests/debugfs_test.c
> > @@ -87,23 +87,14 @@ static void read_and_discard_sysfs_entries(int path_fd, int indent)
> >       closedir(dir);
> >   }
> >
> > -igt_main
> > +static void kms_tests(int fd, int debugfs)
> >   {
> > -     int fd = -1, debugfs;
> >       igt_display_t display;
> >       struct igt_fb fb[IGT_MAX_PIPES];
> >       enum pipe pipe;
> >
> > -     igt_skip_on_simulation();
> > -
> > -     igt_fixture {
> > -             fd = drm_open_driver_master(DRIVER_INTEL);
> > -             igt_require_gem(fd);
> > -             debugfs = igt_debugfs_dir(fd);
> > -
> > -             kmstest_set_vt_graphics_mode();
> > -             igt_display_init(&display, fd);
> > -     }
> > +     igt_fixture
> > +             igt_display_require(&display, fd);
> >
> >       igt_subtest("read_all_entries") {
> >               /* try to light all pipes */
> > @@ -152,6 +143,27 @@ igt_main
> >               read_and_discard_sysfs_entries(debugfs, 0);
> >       }
> >
> > +     igt_fixture
> > +             igt_display_fini(&display);
> > +}
> > +
> > +igt_main
> > +{
> > +     int fd = -1, debugfs;
> > +
> > +     igt_skip_on_simulation();
>
> I think this^ can be removed.
>
> Antonio
>
> > +
> > +     igt_fixture {
> > +             fd = drm_open_driver_master(DRIVER_INTEL);
> > +             igt_require_gem(fd);
> > +             debugfs = igt_debugfs_dir(fd);
> > +
> > +             kmstest_set_vt_graphics_mode();
> > +     }
> > +
> > +     igt_subtest_group
> > +             kms_tests(fd, debugfs);
> > +
> >       igt_subtest("emon_crash") {
> >               int i;
> >               /*
> > @@ -174,7 +186,6 @@ igt_main
> >       }
> >
> >       igt_fixture {
> > -             igt_display_fini(&display);
> >               close(debugfs);
> >               close(fd);
> >       }
> >



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t] tests/debugfs: use igt_display_require

[Daniel Vetter]

Need to extract into a test subgroup to make sure we only skip the
tests that need display support.

v2: Chris pointed out that "read-all-entries" was the original non-kms
tests, and we don't want to skip that if there's no output. Make a
seperate test for this.

Also, that kind of where libraries magically second-guess what the
test might have wanted when it supplies an invalid request is exactly
why I want to fix the igt_display_init API regression.

v3: Actually squash in the hunk that was supposed to do v2 into this
patch (Antonio).

Cc: Antonio Argenziano <antonio.argenziano@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 tests/debugfs_test.c | 42 ++++++++++++++++++++++++++++--------------
 1 file changed, 28 insertions(+), 14 deletions(-)

diff --git a/tests/debugfs_test.c b/tests/debugfs_test.c
index 2e87e4420b15..6a87d90afd0e 100644
--- a/tests/debugfs_test.c
+++ b/tests/debugfs_test.c
@@ -87,25 +87,16 @@ static void read_and_discard_sysfs_entries(int path_fd, int indent)
 	closedir(dir);
 }
 
-igt_main
+static void kms_tests(int fd, int debugfs)
 {
-	int fd = -1, debugfs;
 	igt_display_t display;
 	struct igt_fb fb[IGT_MAX_PIPES];
 	enum pipe pipe;
 
-	igt_skip_on_simulation();
-
-	igt_fixture {
-		fd = drm_open_driver_master(DRIVER_INTEL);
-		igt_require_gem(fd);
-		debugfs = igt_debugfs_dir(fd);
-
-		kmstest_set_vt_graphics_mode();
-		igt_display_init(&display, fd);
-	}
+	igt_fixture
+		igt_display_require(&display, fd);
 
-	igt_subtest("read_all_entries") {
+	igt_subtest("read_all_entries_display_on") {
 		/* try to light all pipes */
 		for_each_pipe(&display, pipe) {
 			igt_output_t *output;
@@ -152,6 +143,30 @@ igt_main
 		read_and_discard_sysfs_entries(debugfs, 0);
 	}
 
+	igt_fixture
+		igt_display_fini(&display);
+}
+
+igt_main
+{
+	int fd = -1, debugfs;
+
+	igt_skip_on_simulation();
+
+	igt_fixture {
+		fd = drm_open_driver_master(DRIVER_INTEL);
+		igt_require_gem(fd);
+		debugfs = igt_debugfs_dir(fd);
+
+		kmstest_set_vt_graphics_mode();
+	}
+
+	igt_subtest("read_all_entries")
+		read_and_discard_sysfs_entries(debugfs, 0);
+
+	igt_subtest_group
+		kms_tests(fd, debugfs);
+
 	igt_subtest("emon_crash") {
 		int i;
 		/*
@@ -174,7 +189,6 @@ igt_main
 	}
 
 	igt_fixture {
-		igt_display_fini(&display);
 		close(debugfs);
 		close(fd);
 	}
-- 
2.19.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t] tests: Use igt_display_require

[Daniel Vetter]

Remaining tests that have been overlooked and don't need any
invasive changes to limit the skipping to only the relevant parts.

v2: [A rebase gone wrong]

v3: Move the misplaced hunk to the right patch (Antonio).

Cc: Antonio Argenziano <antonio.argenziano@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 lib/igt_chamelium.c               | 2 +-
 tests/kms_atomic_interruptible.c  | 4 ++--
 tests/kms_force_connector_basic.c | 2 +-
 tests/kms_getfb.c                 | 2 +-
 tests/kms_plane_alpha_blend.c     | 2 +-
 tests/perf_pmu.c                  | 2 +-
 tests/pm_backlight.c              | 2 +-
 tests/prime_mmap_kms.c            | 2 +-
 8 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/lib/igt_chamelium.c b/lib/igt_chamelium.c
index a80ead163846..d136fb04c342 100644
--- a/lib/igt_chamelium.c
+++ b/lib/igt_chamelium.c
@@ -1547,7 +1547,7 @@ static void chamelium_exit_handler(int sig)
 /**
  * chamelium_init:
  * @chamelium: The Chamelium instance to use
- * @drm_fd: a display initialized with #igt_display_init
+ * @drm_fd: a display initialized with #igt_display_require
  *
  * Sets up a connection with a chamelium, using the URL specified in the
  * Chamelium configuration. This must be called first before trying to use the
diff --git a/tests/kms_atomic_interruptible.c b/tests/kms_atomic_interruptible.c
index 8e9d4cb69c4d..be688638973f 100644
--- a/tests/kms_atomic_interruptible.c
+++ b/tests/kms_atomic_interruptible.c
@@ -85,8 +85,8 @@ static void run_plane_test(igt_display_t *display, enum pipe pipe, igt_output_t
 
 	/*
 	 * Make sure we start with everything disabled to force a real modeset.
-	 * igt_display_init only sets sw state, and assumes the first test doesn't care
-	 * about hw state.
+	 * igt_display_require only sets sw state, and assumes the first test
+	 * doesn't care about hw state.
 	 */
 	igt_display_commit2(display, COMMIT_ATOMIC);
 
diff --git a/tests/kms_force_connector_basic.c b/tests/kms_force_connector_basic.c
index e9325dec9305..b8246e669939 100644
--- a/tests/kms_force_connector_basic.c
+++ b/tests/kms_force_connector_basic.c
@@ -217,7 +217,7 @@ int main(int argc, char **argv)
 
 		/* attempt to use the display */
 		kmstest_set_vt_graphics_mode();
-		igt_display_init(&display, drm_fd);
+		igt_display_require(&display, drm_fd);
 		igt_display_commit(&display);
 		igt_display_fini(&display);
 
diff --git a/tests/kms_getfb.c b/tests/kms_getfb.c
index 07ffd79c4613..ca0b01c05e5c 100644
--- a/tests/kms_getfb.c
+++ b/tests/kms_getfb.c
@@ -116,7 +116,7 @@ static uint32_t get_any_prop_id(int fd)
 {
 	igt_display_t display;
 
-	igt_display_init(&display, fd);
+	igt_display_require(&display, fd);
 	for (int i = 0; i < display.n_outputs; i++) {
 		igt_output_t *output = &display.outputs[i];
 		if (output->props[IGT_CONNECTOR_DPMS] != 0)
diff --git a/tests/kms_plane_alpha_blend.c b/tests/kms_plane_alpha_blend.c
index 3fab118ae0e1..1d9d8933d7e2 100644
--- a/tests/kms_plane_alpha_blend.c
+++ b/tests/kms_plane_alpha_blend.c
@@ -565,7 +565,7 @@ igt_main
 	igt_fixture {
 		data.gfx_fd = drm_open_driver(DRIVER_ANY);
 		igt_require_pipe_crc(data.gfx_fd);
-		igt_display_init(&data.display, data.gfx_fd);
+		igt_display_require(&data.display, data.gfx_fd);
 		igt_require(data.display.is_atomic);
 	}
 
diff --git a/tests/perf_pmu.c b/tests/perf_pmu.c
index b34bc66ce2c4..21292bf3a2fe 100644
--- a/tests/perf_pmu.c
+++ b/tests/perf_pmu.c
@@ -811,7 +811,7 @@ event_wait(int gem_fd, const struct intel_execution_engine2 *e)
 	igt_skip_on(IS_VALLEYVIEW(devid) || IS_CHERRYVIEW(devid));
 
 	kmstest_set_vt_graphics_mode();
-	igt_display_init(&data.display, gem_fd);
+	igt_display_require(&data.display, gem_fd);
 
 	/**
 	 * We will use the display to render event forwarind so need to
diff --git a/tests/pm_backlight.c b/tests/pm_backlight.c
index 32808cdf6ca4..054300f6e2e1 100644
--- a/tests/pm_backlight.c
+++ b/tests/pm_backlight.c
@@ -214,7 +214,7 @@ igt_main
 		 * try to enable all.
 		 */
 		kmstest_set_vt_graphics_mode();
-		igt_display_init(&display, drm_open_driver(DRIVER_INTEL));
+		igt_display_require(&display, drm_open_driver(DRIVER_INTEL));
 
 		/* should be ../../cardX-$output */
 		igt_assert_lt(12, readlink(BACKLIGHT_PATH "/device", full_name, sizeof(full_name) - 1));
diff --git a/tests/prime_mmap_kms.c b/tests/prime_mmap_kms.c
index faace4afd478..fdc37214d96d 100644
--- a/tests/prime_mmap_kms.c
+++ b/tests/prime_mmap_kms.c
@@ -248,7 +248,7 @@ igt_main
 
 		igt_require_pipe_crc(gpu.drm_fd);
 
-		igt_display_init(&gpu.display, gpu.drm_fd);
+		igt_display_require(&gpu.display, gpu.drm_fd);
 	}
 
 	igt_subtest("buffer-sharing")
-- 
2.19.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,1/5] tests/sw_sync: use igt_fork_helper (rev3)

[Patchwork]

== Series Details ==

Series: series starting with [i-g-t,1/5] tests/sw_sync: use igt_fork_helper (rev3)
URL   : https://patchwork.freedesktop.org/series/51928/
State : success

== Summary ==

= CI Bug Log - changes from IGT_4712 -> IGTPW_2043 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/51928/revisions/3/mbox/

== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_module_reload@basic-reload-inject:
      fi-byt-clapper:     PASS -> WARN (fdo#108688)

    igt@gem_mmap_gtt@basic-read:
      fi-glk-dsi:         PASS -> INCOMPLETE (fdo#103359, k.org#198133)

    igt@kms_chamelium@dp-edid-read:
      fi-kbl-7500u:       PASS -> WARN (fdo#102672)

    igt@kms_frontbuffer_tracking@basic:
      fi-byt-clapper:     PASS -> FAIL (fdo#103167)

    igt@kms_pipe_crc_basic@hang-read-crc-pipe-a:
      fi-byt-clapper:     PASS -> FAIL (fdo#103191, fdo#107362)

    igt@pm_rpm@module-reload:
      fi-byt-clapper:     PASS -> FAIL (fdo#108675)

    
    ==== Possible fixes ====

    igt@gem_ctx_create@basic-files:
      fi-icl-u2:          DMESG-WARN (fdo#107724) -> PASS

    igt@kms_chamelium@common-hpd-after-suspend:
      fi-skl-6700k2:      FAIL (fdo#103841) -> PASS

    igt@kms_pipe_crc_basic@read-crc-pipe-b:
      fi-byt-clapper:     FAIL (fdo#107362) -> PASS

    
  fdo#102672 https://bugs.freedesktop.org/show_bug.cgi?id=102672
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
  fdo#103841 https://bugs.freedesktop.org/show_bug.cgi?id=103841
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
  fdo#107724 https://bugs.freedesktop.org/show_bug.cgi?id=107724
  fdo#108675 https://bugs.freedesktop.org/show_bug.cgi?id=108675
  fdo#108688 https://bugs.freedesktop.org/show_bug.cgi?id=108688
  k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133


== Participating hosts (52 -> 46) ==

  Additional (1): fi-cnl-u 
  Missing    (7): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-icl-u 


== Build changes ==

    * IGT: IGT_4712 -> IGTPW_2043

  CI_DRM_5097: c20dfc4f015dfd41246beb7d72338aa50543a5ef @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2043: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2043/
  IGT_4712: a3ede1b535ac8137f6949c468edd7054453d5dae @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools



== Testlist changes ==

+igt@debugfs_test@read_all_entries_display_on

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2043/issues.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✓ Fi.CI.IGT: success for series starting with [i-g-t,1/5] tests/sw_sync: use igt_fork_helper (rev3)

[Patchwork]

== Series Details ==

Series: series starting with [i-g-t,1/5] tests/sw_sync: use igt_fork_helper (rev3)
URL   : https://patchwork.freedesktop.org/series/51928/
State : success

== Summary ==

= CI Bug Log - changes from IGT_4712_full -> IGTPW_2043_full =

== Summary - WARNING ==

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

  External URL: https://patchwork.freedesktop.org/api/1.0/series/51928/revisions/3/mbox/

== Possible new issues ==

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

  === IGT changes ===

    ==== Warnings ====

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-blt:
      shard-snb:          PASS -> SKIP +1

    igt@tools_test@tools_test:
      shard-hsw:          PASS -> SKIP

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_ccs@pipe-a-crc-sprite-planes-basic:
      shard-glk:          PASS -> FAIL (fdo#108145)

    igt@kms_cursor_crc@cursor-128x128-suspend:
      shard-apl:          PASS -> FAIL (fdo#103232, fdo#103191)
      shard-kbl:          PASS -> FAIL (fdo#103232, fdo#103191)

    igt@kms_cursor_crc@cursor-256x256-random:
      shard-glk:          PASS -> FAIL (fdo#103232) +6

    igt@kms_cursor_crc@cursor-256x85-sliding:
      shard-kbl:          PASS -> FAIL (fdo#103232) +2

    igt@kms_cursor_crc@cursor-64x21-random:
      shard-apl:          PASS -> FAIL (fdo#103232) +9

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-onoff:
      shard-apl:          PASS -> FAIL (fdo#103167) +2

    igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-onoff:
      shard-glk:          PASS -> FAIL (fdo#103167) +11

    igt@kms_plane_multiple@atomic-pipe-a-tiling-y:
      shard-glk:          PASS -> FAIL (fdo#103166) +4

    igt@kms_plane_multiple@atomic-pipe-c-tiling-yf:
      shard-apl:          PASS -> FAIL (fdo#103166) +3

    igt@prime_vgem@basic-fence-flip:
      shard-snb:          PASS -> INCOMPLETE (fdo#105411)

    
    ==== Possible fixes ====

    igt@gem_exec_bad_domains@cpu-domain:
      shard-apl:          DMESG-WARN (fdo#105602, fdo#103558) -> PASS +4

    igt@gem_pwrite_pread@snooped-copy-correctness:
      shard-snb:          INCOMPLETE (fdo#105411) -> PASS

    igt@kms_color@pipe-b-legacy-gamma:
      shard-apl:          FAIL (fdo#104782) -> PASS

    igt@kms_cursor_crc@cursor-256x256-suspend:
      shard-glk:          FAIL (fdo#103232) -> PASS +3
      shard-apl:          FAIL (fdo#103232, fdo#103191) -> PASS

    igt@kms_cursor_crc@cursor-64x64-dpms:
      shard-kbl:          FAIL (fdo#103232) -> PASS +2
      shard-apl:          FAIL (fdo#103232) -> PASS +3

    igt@kms_cursor_legacy@cursor-vs-flip-toggle:
      shard-hsw:          FAIL (fdo#103355) -> PASS

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-render:
      shard-apl:          FAIL (fdo#103167) -> PASS

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-fullscreen:
      shard-glk:          FAIL (fdo#103167) -> PASS +1

    igt@kms_plane_alpha_blend@pipe-b-constant-alpha-max:
      shard-glk:          FAIL (fdo#108145) -> PASS

    igt@kms_plane_multiple@atomic-pipe-a-tiling-x:
      shard-apl:          FAIL (fdo#103166) -> PASS +3

    igt@kms_plane_multiple@atomic-pipe-b-tiling-x:
      shard-glk:          FAIL (fdo#103166) -> PASS +1

    igt@kms_plane_multiple@atomic-pipe-b-tiling-y:
      shard-kbl:          FAIL (fdo#103166) -> PASS

    igt@kms_setmode@basic-clone-single-crtc:
      shard-snb:          DMESG-WARN (fdo#107469) -> PASS +2

    
    ==== Warnings ====

    igt@kms_plane_alpha_blend@pipe-a-constant-alpha-max:
      shard-glk:          FAIL (fdo#108145) -> DMESG-FAIL (fdo#108145, fdo#106538)

    
  fdo#103166 https://bugs.freedesktop.org/show_bug.cgi?id=103166
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#103232 https://bugs.freedesktop.org/show_bug.cgi?id=103232
  fdo#103355 https://bugs.freedesktop.org/show_bug.cgi?id=103355
  fdo#103558 https://bugs.freedesktop.org/show_bug.cgi?id=103558
  fdo#104782 https://bugs.freedesktop.org/show_bug.cgi?id=104782
  fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411
  fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602
  fdo#106538 https://bugs.freedesktop.org/show_bug.cgi?id=106538
  fdo#107469 https://bugs.freedesktop.org/show_bug.cgi?id=107469
  fdo#108145 https://bugs.freedesktop.org/show_bug.cgi?id=108145


== Participating hosts (6 -> 5) ==

  Missing    (1): shard-skl 


== Build changes ==

    * IGT: IGT_4712 -> IGTPW_2043

  CI_DRM_5097: c20dfc4f015dfd41246beb7d72338aa50543a5ef @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2043: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2043/
  IGT_4712: a3ede1b535ac8137f6949c468edd7054453d5dae @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2043/shards.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 5/5] lib/kms: warn if we commit without outputs

[Daniel Vetter]

On Fri, Nov 02, 2018 at 01:11:30PM +0100, Maarten Lankhorst wrote:
> Op 02-11-18 om 10:57 schreef Daniel Vetter:
> > With the high-level helpers requiring outputs there's not point
> > in silently ignoring issues anymore. Complain about that if it
> > ever happens.
> >
> > This reverts
> >
> > commit 212b71372bfbb73663d872df31118d6b396ada4f
> > Author: Chris Wilson <chris@chris-wilson.co.uk>
> > Date:   Fri Sep 14 21:03:38 2018 +0100
> >
> >     lib/kms: Skip no-op display updates
> >
> > which created an in my opinion serious API issue by silently dropping
> > possible errors on the floor. Instead of silently second guess what
> > the test might have wanted to do in the absence of display outputs
> > it's much better to be explicit, and enforce that.
> >
> > v2: Improve commit message.
> >
> > Cc: Antonio Argenziano <antonio.argenziano@intel.com>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > ---
> >  lib/igt_kms.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> > index 7214101e2696..5a35f6bbc177 100644
> > --- a/lib/igt_kms.c
> > +++ b/lib/igt_kms.c
> > @@ -3292,7 +3292,7 @@ static int do_display_commit(igt_display_t *display,
> >  	enum pipe pipe;
> >  	LOG_INDENT(display, "commit");
> >  
> > -	if (!display->n_pipes || !display->n_outputs)
> > +	if (igt_warn_on(!display->n_pipes || !display->n_outputs))
> >  		return 0; /* nothing to do */
> >  
> >  	igt_display_refresh(display);
> > @@ -3345,7 +3345,7 @@ int igt_display_try_commit_atomic(igt_display_t *display, uint32_t flags, void *
> >  {
> >  	int ret;
> >  
> > -	if (!display->n_pipes || !display->n_outputs)
> > +	if (igt_warn_on(!display->n_pipes || !display->n_outputs))
> >  		return 0; /* nothing to do */
> >  
> >  	LOG_INDENT(display, "commit");
> 
> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

Is this for the entire series?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 5/5] lib/kms: warn if we commit without outputs

[Maarten Lankhorst]

Op 02-11-18 om 13:11 schreef Maarten Lankhorst:
> Op 02-11-18 om 10:57 schreef Daniel Vetter:
>> With the high-level helpers requiring outputs there's not point
>> in silently ignoring issues anymore. Complain about that if it
>> ever happens.
>>
>> This reverts
>>
>> commit 212b71372bfbb73663d872df31118d6b396ada4f
>> Author: Chris Wilson <chris@chris-wilson.co.uk>
>> Date:   Fri Sep 14 21:03:38 2018 +0100
>>
>>     lib/kms: Skip no-op display updates
>>
>> which created an in my opinion serious API issue by silently dropping
>> possible errors on the floor. Instead of silently second guess what
>> the test might have wanted to do in the absence of display outputs
>> it's much better to be explicit, and enforce that.
>>
>> v2: Improve commit message.
>>
>> Cc: Antonio Argenziano <antonio.argenziano@intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
>> ---
>>  lib/igt_kms.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
>> index 7214101e2696..5a35f6bbc177 100644
>> --- a/lib/igt_kms.c
>> +++ b/lib/igt_kms.c
>> @@ -3292,7 +3292,7 @@ static int do_display_commit(igt_display_t *display,
>>  	enum pipe pipe;
>>  	LOG_INDENT(display, "commit");
>>  
>> -	if (!display->n_pipes || !display->n_outputs)
>> +	if (igt_warn_on(!display->n_pipes || !display->n_outputs))
>>  		return 0; /* nothing to do */
>>  
>>  	igt_display_refresh(display);
>> @@ -3345,7 +3345,7 @@ int igt_display_try_commit_atomic(igt_display_t *display, uint32_t flags, void *
>>  {
>>  	int ret;
>>  
>> -	if (!display->n_pipes || !display->n_outputs)
>> +	if (igt_warn_on(!display->n_pipes || !display->n_outputs))
>>  		return 0; /* nothing to do */
>>  
>>  	LOG_INDENT(display, "commit");
> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>
For the whole series. :)

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t] lib/kms: warn if we commit without outputs

[Daniel Vetter]

With the high-level helpers requiring outputs there's not point
in silently ignoring issues anymore. Complain about that if it
ever happens.

This reverts

commit 212b71372bfbb73663d872df31118d6b396ada4f
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Fri Sep 14 21:03:38 2018 +0100

    lib/kms: Skip no-op display updates

which created an in my opinion serious API issue by silently dropping
possible errors on the floor. Instead of silently second guess what
the test might have wanted to do in the absence of display outputs
it's much better to be explicit, and enforce that.

v2: Improve commit message.

v3: Switch to an assert and update comments, to make it clear this is
igt_display api abuse (Arek).

Cc: Antonio Argenziano <antonio.argenziano@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> (v2)
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 lib/igt_kms.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 7214101e2696..73cea75d19f0 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -3292,8 +3292,8 @@ static int do_display_commit(igt_display_t *display,
 	enum pipe pipe;
 	LOG_INDENT(display, "commit");
 
-	if (!display->n_pipes || !display->n_outputs)
-		return 0; /* nothing to do */
+	/* someone managed to bypass igt_display_require, catch them */
+	assert(display->n_pipes && display->n_outputs);
 
 	igt_display_refresh(display);
 
@@ -3345,8 +3345,8 @@ int igt_display_try_commit_atomic(igt_display_t *display, uint32_t flags, void *
 {
 	int ret;
 
-	if (!display->n_pipes || !display->n_outputs)
-		return 0; /* nothing to do */
+	/* someone managed to bypass igt_display_require, catch them */
+	assert(display->n_pipes && display->n_outputs);
 
 	LOG_INDENT(display, "commit");
 
-- 
2.19.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✗ Fi.CI.BAT: failure for series starting with [i-g-t,1/5] tests/sw_sync: use igt_fork_helper (rev4)

[Patchwork]

== Series Details ==

Series: series starting with [i-g-t,1/5] tests/sw_sync: use igt_fork_helper (rev4)
URL   : https://patchwork.freedesktop.org/series/51928/
State : failure

== Summary ==

IGT patchset build failed on latest successful build
29ae0925abe1d3a0202059538559468ad947d42d gitlab-ci: Depend docker images jobs on .gitlab-ci.yaml too

[129/757] Linking target lib/tests/igt_timeout.
[130/757] Linking target lib/tests/igt_invalid_subtest_name.
[131/757] Linking target tests/core_get_client_auth.
[132/757] Linking target tests/core_auth.
[133/757] Linking target tests/core_getclient.
[134/757] Linking target tests/core_getstats.
[135/757] Linking target tests/core_getversion.
[136/757] Linking target tests/core_prop_blob.
[137/757] Linking target tests/core_setmaster_vs_auth.
[138/757] Linking target tests/drm_import_export.
[139/757] Linking target tests/debugfs_test.
[140/757] Linking target tests/drm_mm.
[141/757] Linking target tests/drm_read.
[142/757] Linking target tests/kms_3d.
[143/757] Linking target tests/kms_addfb_basic.
[144/757] Linking target tests/kms_atomic.
[145/757] Linking target tests/kms_atomic_interruptible.
[146/757] Linking target tests/kms_available_modes_crc.
[147/757] Linking target tests/kms_atomic_transition.
[148/757] Linking target tests/kms_ccs.
[149/757] Linking target tests/kms_busy.
[150/757] Linking target tests/kms_color.
[151/757] Linking target tests/kms_concurrent.
[152/757] Linking target tests/kms_chv_cursor_fail.
[153/757] Linking target tests/kms_content_protection.
[154/757] Linking target tests/kms_crtc_background_color.
[155/757] Linking target tests/kms_cursor_crc.
[156/757] Linking target tests/kms_cursor_legacy.
[157/757] Linking target tests/kms_draw_crc.
[158/757] Linking target tests/kms_fence_pin_leak.
[159/757] Linking target tests/kms_fbcon_fbt.
[160/757] Linking target tests/kms_flip.
[161/757] Linking target tests/kms_flip_event_leak.
[162/757] Linking target tests/kms_force_connector_basic.
[163/757] Linking target tests/kms_flip_tiling.
[164/757] Linking target tests/kms_getfb.
[165/757] Linking target tests/kms_frontbuffer_tracking.
[166/757] Linking target tests/kms_invalid_dotclock.
[167/757] Linking target tests/kms_hdmi_inject.
[168/757] Compiling C object 'tests/kms_lease@exe/kms_lease.c.o'.
FAILED: tests/kms_lease@exe/kms_lease.c.o 
ccache cc  -Itests/kms_lease@exe -Itests -I../tests -I. -I../ -I../lib/stubs/syscalls -Ilib -I../lib -I../include/drm-uapi -I/usr/include/cairo -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -I/usr/include/pixman-1 -I/usr/include/libpng16 -I/usr/include/freetype2 -I/usr/include/libpng12 -I/opt/igt/include -I/opt/igt/include/libdrm -I/usr/include/x86_64-linux-gnu -I/usr/include -I/usr/include/libdrm -I/usr/include/libdrm/nouveau -I/home/cidrm/kernel_headers/include -fdiagnostics-color=always -pipe -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wextra -std=gnu11 -O0 -g -D_GNU_SOURCE -include config.h -Wbad-function-cast -Wdeclaration-after-statement -Wformat=2 -Wimplicit-fallthrough=0 -Wlogical-op -Wmissing-declarations -Wmissing-format-attribute -Wmissing-noreturn -Wmissing-prototypes -Wnested-externs -Wold-style-definition -Wpointer-arith -Wredundant-decls -Wshadow -Wstrict-prototypes -Wuninitialized -Wunused -Wno-clobbered -Wno-maybe-uninitialized -Wno-missing-field-initializers -Wno-pointer-arith -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 -pthread -MD -MQ 'tests/kms_lease@exe/kms_lease.c.o' -MF 'tests/kms_lease@exe/kms_lease.c.o.d' -o 'tests/kms_lease@exe/kms_lease.c.o' -c ../tests/kms_lease.c
../tests/kms_lease.c: In function ‘simple_lease’:
../tests/kms_lease.c:297:2: error: implicit declaration of function ‘igt_display_init’; did you mean ‘igt_display_fini’? [-Werror=implicit-function-declaration]
  igt_display_init(&lease.display, lease.fd);
  ^~~~~~~~~~~~~~~~
  igt_display_fini
../tests/kms_lease.c:297:2: warning: nested extern declaration of ‘igt_display_init’ [-Wnested-externs]
cc1: some warnings being treated as errors
ninja: build stopped: subcommand failed.

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev