[PATCH i-g-t 2/5] igt/prime_vgem: Skip flip if no display

[PATCH i-g-t 2/5] igt/prime_vgem: Skip flip if no display

[Chris Wilson]

We try flipping a vgem surface onto a  i915 scanout. However, if there
is no display we want to disable the kms interface, including the addfb
ioctl. On such systems the call to kms_addfb will naturally fail and the
test cannot be run.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 tests/prime_vgem.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/tests/prime_vgem.c b/tests/prime_vgem.c
index b821fbb8c..a76d3797b 100644
--- a/tests/prime_vgem.c
+++ b/tests/prime_vgem.c
@@ -762,10 +762,13 @@ static void test_flip(int i915, int vgem, unsigned hang)
 		igt_assert(handle[i]);
 		close(fd);
 
-		do_or_die(__kms_addfb(i915, handle[i],
-				      bo[i].width, bo[i].height, bo[i].pitch,
-				      DRM_FORMAT_XRGB8888, I915_TILING_NONE, NULL,
-				      LOCAL_DRM_MODE_FB_MODIFIERS, &fb_id[i]));
+		/* May skip if i915 has no displays */
+		igt_require(__kms_addfb(i915, handle[i],
+					bo[i].width, bo[i].height, bo[i].pitch,
+					DRM_FORMAT_XRGB8888,
+					I915_TILING_NONE, NULL,
+					LOCAL_DRM_MODE_FB_MODIFIERS,
+					&fb_id[i]) == 0);
 		igt_assert(fb_id[i]);
 	}
 
-- 
2.19.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH i-g-t 1/5] igt/kms_getfb: Check the iface exists before use

[Chris Wilson]

If the driver doesn't support the getfb iface (e.g. because KMS has been
disabled), the ioctls will fail with ENOTSUP. This is expected, so skip
the test as nothing useful can be learnt.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 tests/kms_getfb.c | 40 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 38 insertions(+), 2 deletions(-)

diff --git a/tests/kms_getfb.c b/tests/kms_getfb.c
index 81d796a42..71d65488f 100644
--- a/tests/kms_getfb.c
+++ b/tests/kms_getfb.c
@@ -40,6 +40,40 @@
 #include "drm.h"
 #include "drm_fourcc.h"
 
+static bool has_getfb_iface(int fd)
+{
+	struct drm_mode_fb_cmd arg = { };
+	int err;
+
+	err = 0;
+	if (drmIoctl(fd, DRM_IOCTL_MODE_GETFB, &arg))
+		err = -errno;
+	switch (err) {
+	case -ENOTTY: /* ioctl unrecognised (kernel too old) */
+	case -ENOTSUP: /* driver doesn't support KMS */
+		return false;
+	default:
+		return true;
+	}
+}
+
+static bool has_addfb2_iface(int fd)
+{
+	struct drm_mode_fb_cmd2 arg = { };
+	int err;
+
+	err = 0;
+	if (drmIoctl(fd, DRM_IOCTL_MODE_ADDFB2, &arg) == 0)
+		err = -errno;
+	switch (err) {
+	case -ENOTTY: /* ioctl unrecognised (kernel too old) */
+	case -ENOTSUP: /* driver doesn't support KMS */
+		return false;
+	default:
+		return true;
+	}
+}
+
 static void get_ccs_fb(int fd, struct drm_mode_fb_cmd2 *ret)
 {
 	struct drm_mode_fb_cmd2 add = {
@@ -54,6 +88,7 @@ static void get_ccs_fb(int fd, struct drm_mode_fb_cmd2 *ret)
 	};
 	int size;
 
+	igt_require(has_addfb2_iface(fd));
 	igt_require_intel(fd);
 
 	/* An explanation of the magic numbers can be found in kms_ccs.c. */
@@ -191,15 +226,16 @@ static void test_duplicate_handles(int fd)
 		do_ioctl(fd, DRM_IOCTL_MODE_RMFB, &add.fb_id);
 		gem_close(fd, add.handles[0]);
 	}
-
 }
 
 igt_main
 {
 	int fd;
 
-	igt_fixture
+	igt_fixture {
 		fd = drm_open_driver_master(DRIVER_ANY);
+		igt_require(has_getfb_iface(fd));
+	}
 
 	igt_subtest_group
 		test_handle_input(fd);
-- 
2.19.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH i-g-t 2/5] igt/prime_vgem: Skip flip if no display

[Chris Wilson]

We try flipping a vgem surface onto a  i915 scanout. However, if there
is no display we want to disable the kms interface, including the addfb
ioctl. On such systems the call to kms_addfb will naturally fail and the
test cannot be run.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 tests/prime_vgem.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/tests/prime_vgem.c b/tests/prime_vgem.c
index 5e0c4e289..98005ead2 100644
--- a/tests/prime_vgem.c
+++ b/tests/prime_vgem.c
@@ -766,11 +766,13 @@ static void test_flip(int i915, int vgem, unsigned hang)
 
 		strides[0] = bo[i].pitch;
 
-		do_or_die(__kms_addfb(i915, handle[i],
-				      bo[i].width, bo[i].height,
-				      DRM_FORMAT_XRGB8888, I915_TILING_NONE,
-				      strides, offsets, 1,
-				      LOCAL_DRM_MODE_FB_MODIFIERS, &fb_id[i]));
+		/* May skip if i915 has no displays */
+		igt_require(__kms_addfb(i915, handle[i],
+					bo[i].width, bo[i].height,
+					DRM_FORMAT_XRGB8888, I915_TILING_NONE,
+					strides, offsets, 1,
+					LOCAL_DRM_MODE_FB_MODIFIERS,
+				       	&fb_id[i]));
 		igt_assert(fb_id[i]);
 	}
 
-- 
2.19.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH i-g-t 3/5] lib: Report if kms is enabled on the display

[Chris Wilson]

Some drivers may have disabled KMS or there may simply nothing attached
to the device. In either case KMS is unusable and we may prefer to skip.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 lib/igt_kms.c | 14 ++++++++++++--
 lib/igt_kms.h |  3 ++-
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 4563bfd9d..9710bcae1 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -1843,8 +1843,9 @@ static void igt_fill_display_format_mod(igt_display_t *display);
  * 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
  */
-void igt_display_init(igt_display_t *display, int drm_fd)
+bool igt_display_init(igt_display_t *display, int drm_fd)
 {
 	drmModeRes *resources;
 	drmModePlaneRes *plane_resources;
@@ -1857,7 +1858,8 @@ void igt_display_init(igt_display_t *display, int drm_fd)
 	display->drm_fd = drm_fd;
 
 	resources = drmModeGetResources(display->drm_fd);
-	igt_assert(resources);
+	if (!resources)
+		goto out;
 
 	/*
 	 * We cache the number of pipes, that number is a physical limit of the
@@ -2004,7 +2006,15 @@ void igt_display_init(igt_display_t *display, int drm_fd)
 	/* Set reasonable default values for every object in the display. */
 	igt_display_reset(display);
 
+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));
 }
 
 /**
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index 3862efa28..73624399b 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -378,7 +378,8 @@ struct igt_display {
 	int format_mod_count;
 };
 
-void igt_display_init(igt_display_t *display, int drm_fd);
+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);
 int  igt_display_commit2(igt_display_t *display, enum igt_commit_style s);
-- 
2.19.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH i-g-t 4/5] lib/kms: Skip no-op display updates

[Chris Wilson]

If the display is disabled (e.g. the driver has disabled the KMS
interface) there is nothing to do so avoid failing.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 lib/igt_kms.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 9710bcae1..dedc81344 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -3271,6 +3271,9 @@ 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 */
+
 	igt_display_refresh(display);
 
 	if (s == COMMIT_ATOMIC) {
@@ -3321,6 +3324,9 @@ 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 */
+
 	LOG_INDENT(display, "commit");
 
 	igt_display_refresh(display);
-- 
2.19.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH i-g-t 5/5] igt: Require a display (KMS enabled) for KMS tests

[Chris Wilson]

Simple rule of thumb, if a kms_* test calls igt_display_init() in its
global fixture, skip the entire test if the driver has disabled KMS.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 tests/drm_read.c                  | 2 +-
 tests/kms_atomic.c                | 3 +--
 tests/kms_atomic_interruptible.c  | 4 +---
 tests/kms_atomic_transition.c     | 3 +--
 tests/kms_available_modes_crc.c   | 2 +-
 tests/kms_busy.c                  | 3 +--
 tests/kms_ccs.c                   | 2 +-
 tests/kms_chamelium.c             | 2 +-
 tests/kms_chv_cursor_fail.c       | 2 +-
 tests/kms_color.c                 | 2 +-
 tests/kms_concurrent.c            | 3 +--
 tests/kms_crtc_background_color.c | 2 +-
 tests/kms_cursor_crc.c            | 2 +-
 tests/kms_cursor_legacy.c         | 3 +--
 tests/kms_fence_pin_leak.c        | 2 +-
 tests/kms_flip_event_leak.c       | 2 +-
 tests/kms_flip_tiling.c           | 2 +-
 tests/kms_frontbuffer_tracking.c  | 2 +-
 tests/kms_invalid_dotclock.c      | 2 +-
 tests/kms_legacy_colorkey.c       | 2 +-
 tests/kms_mmap_write_crc.c        | 2 +-
 tests/kms_panel_fitting.c         | 2 +-
 tests/kms_pipe_b_c_ivb.c          | 2 +-
 tests/kms_pipe_crc_basic.c        | 2 +-
 tests/kms_plane.c                 | 2 +-
 tests/kms_plane_lowres.c          | 2 +-
 tests/kms_plane_multiple.c        | 3 +--
 tests/kms_plane_scaling.c         | 2 +-
 tests/kms_properties.c            | 2 +-
 tests/kms_psr.c                   | 2 +-
 tests/kms_pwrite_crc.c            | 2 +-
 tests/kms_rmfb.c                  | 2 +-
 tests/kms_rotation_crc.c          | 2 +-
 tests/kms_universal_plane.c       | 2 +-
 tests/kms_vblank.c                | 2 +-
 35 files changed, 35 insertions(+), 43 deletions(-)

diff --git a/tests/drm_read.c b/tests/drm_read.c
index b6aab7312..309f389f6 100644
--- a/tests/drm_read.c
+++ b/tests/drm_read.c
@@ -182,7 +182,7 @@ igt_main
 		fd = drm_open_driver_master(DRIVER_ANY);
 		kmstest_set_vt_graphics_mode();
 
-		igt_display_init(&display, fd);
+		igt_display_require(&display, fd);
 		igt_display_require_output(&display);
 
 		for_each_pipe_with_valid_output(&display, pipe, output) {
diff --git a/tests/kms_atomic.c b/tests/kms_atomic.c
index ac02baf01..3aad2d9aa 100644
--- a/tests/kms_atomic.c
+++ b/tests/kms_atomic.c
@@ -871,8 +871,7 @@ igt_main
 
 		kmstest_set_vt_graphics_mode();
 
-		igt_display_init(&display, display.drm_fd);
-
+		igt_display_require(&display, display.drm_fd);
 		igt_require(display.is_atomic);
 		igt_display_require_output(&display);
 
diff --git a/tests/kms_atomic_interruptible.c b/tests/kms_atomic_interruptible.c
index 64a005597..8e9d4cb69 100644
--- a/tests/kms_atomic_interruptible.c
+++ b/tests/kms_atomic_interruptible.c
@@ -279,10 +279,8 @@ igt_main
 
 		kmstest_set_vt_graphics_mode();
 
-		igt_display_init(&display, display.drm_fd);
-
+		igt_display_require(&display, display.drm_fd);
 		igt_require(display.is_atomic);
-
 		igt_display_require_output(&display);
 
 		igt_require_sw_sync();
diff --git a/tests/kms_atomic_transition.c b/tests/kms_atomic_transition.c
index 2fbd94bd2..12bafe87e 100644
--- a/tests/kms_atomic_transition.c
+++ b/tests/kms_atomic_transition.c
@@ -858,8 +858,7 @@ igt_main
 
 		kmstest_set_vt_graphics_mode();
 
-		igt_display_init(&display, display.drm_fd);
-
+		igt_display_require(&display, display.drm_fd);
 		igt_require(display.is_atomic);
 
 		igt_display_require_output(&display);
diff --git a/tests/kms_available_modes_crc.c b/tests/kms_available_modes_crc.c
index 09fb2e996..45a47750b 100644
--- a/tests/kms_available_modes_crc.c
+++ b/tests/kms_available_modes_crc.c
@@ -498,7 +498,7 @@ igt_main
 	igt_fixture {
 		data.gfx_fd = drm_open_driver_master(DRIVER_INTEL);
 		kmstest_set_vt_graphics_mode();
-		igt_display_init(&data.display, data.gfx_fd);
+		igt_display_require(&data.display, data.gfx_fd);
 		igt_require_pipe_crc(data.gfx_fd);
 	}
 
diff --git a/tests/kms_busy.c b/tests/kms_busy.c
index abf39828b..d821ec711 100644
--- a/tests/kms_busy.c
+++ b/tests/kms_busy.c
@@ -323,8 +323,7 @@ igt_main
 		gem_require_mmap_wc(fd);
 
 		kmstest_set_vt_graphics_mode();
-		igt_display_init(&display, fd);
-		igt_require(display.n_pipes > 0);
+		igt_display_require(&display, fd);
 	}
 
 	/* XXX Extend to cover atomic rendering tests to all planes + legacy */
diff --git a/tests/kms_ccs.c b/tests/kms_ccs.c
index e03f947ea..950f15ee7 100644
--- a/tests/kms_ccs.c
+++ b/tests/kms_ccs.c
@@ -525,7 +525,7 @@ igt_main
 		kmstest_set_vt_graphics_mode();
 		igt_require_pipe_crc(data.drm_fd);
 
-		igt_display_init(&data.display, data.drm_fd);
+		igt_display_require(&data.display, data.drm_fd);
 	}
 
 	for_each_pipe_static(pipe) {
diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
index 2bc34d077..cf7e3c9aa 100644
--- a/tests/kms_chamelium.c
+++ b/tests/kms_chamelium.c
@@ -752,7 +752,7 @@ igt_main
 		/* So fbcon doesn't try to reprobe things itself */
 		kmstest_set_vt_graphics_mode();
 
-		igt_display_init(&data.display, data.drm_fd);
+		igt_display_require(&data.display, data.drm_fd);
 		igt_require(data.display.is_atomic);
 	}
 
diff --git a/tests/kms_chv_cursor_fail.c b/tests/kms_chv_cursor_fail.c
index 8e50978d0..86629555e 100644
--- a/tests/kms_chv_cursor_fail.c
+++ b/tests/kms_chv_cursor_fail.c
@@ -329,7 +329,7 @@ int main(int argc, char **argv)
 
 		igt_require_pipe_crc(data.drm_fd);
 
-		igt_display_init(&data.display, data.drm_fd);
+		igt_display_require(&data.display, data.drm_fd);
 	}
 
 	for_each_pipe_static(data.pipe) {
diff --git a/tests/kms_color.c b/tests/kms_color.c
index bb106dd00..913c70ca8 100644
--- a/tests/kms_color.c
+++ b/tests/kms_color.c
@@ -1163,7 +1163,7 @@ igt_main
 			data.devid = intel_get_drm_devid(data.drm_fd);
 		kmstest_set_vt_graphics_mode();
 
-		igt_display_init(&data.display, data.drm_fd);
+		igt_display_require(&data.display, data.drm_fd);
 	}
 
 	for_each_pipe_static(pipe)
diff --git a/tests/kms_concurrent.c b/tests/kms_concurrent.c
index 283acf8c8..c17b1fe6f 100644
--- a/tests/kms_concurrent.c
+++ b/tests/kms_concurrent.c
@@ -407,8 +407,7 @@ int main(int argc, char *argv[])
 	igt_fixture {
 		data.drm_fd = drm_open_driver_master(DRIVER_ANY);
 		kmstest_set_vt_graphics_mode();
-		igt_display_init(&data.display, data.drm_fd);
-		igt_require(data.display.n_pipes > 0);
+		igt_display_require(&data.display, data.drm_fd);
 		igt_require(data.display.is_atomic);
 	}
 
diff --git a/tests/kms_crtc_background_color.c b/tests/kms_crtc_background_color.c
index 6407e19ba..3df3401f2 100644
--- a/tests/kms_crtc_background_color.c
+++ b/tests/kms_crtc_background_color.c
@@ -180,7 +180,7 @@ igt_simple_main
 
 	data.gfx_fd = drm_open_driver(DRIVER_INTEL);
 	igt_require_pipe_crc(data.gfx_fd);
-	igt_display_init(&data.display, data.gfx_fd);
+	igt_display_require(&data.display, data.gfx_fd);
 
 	test_crtc_background(&data);
 
diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c
index 99946b1ae..1514e7f2e 100644
--- a/tests/kms_cursor_crc.c
+++ b/tests/kms_cursor_crc.c
@@ -675,7 +675,7 @@ igt_main
 
 		igt_require_pipe_crc(data.drm_fd);
 
-		igt_display_init(&data.display, data.drm_fd);
+		igt_display_require(&data.display, data.drm_fd);
 	}
 
 	data.cursor_max_w = cursor_width;
diff --git a/tests/kms_cursor_legacy.c b/tests/kms_cursor_legacy.c
index 82ceebcb0..79df79a38 100644
--- a/tests/kms_cursor_legacy.c
+++ b/tests/kms_cursor_legacy.c
@@ -1365,8 +1365,7 @@ igt_main
 		display.drm_fd = drm_open_driver_master(DRIVER_ANY);
 		kmstest_set_vt_graphics_mode();
 
-		igt_display_init(&display, display.drm_fd);
-		igt_require(display.n_pipes > 0);
+		igt_display_require(&display, display.drm_fd);
 	}
 
 	igt_subtest_group {
diff --git a/tests/kms_fence_pin_leak.c b/tests/kms_fence_pin_leak.c
index 8bbd5563d..62c52b627 100644
--- a/tests/kms_fence_pin_leak.c
+++ b/tests/kms_fence_pin_leak.c
@@ -210,7 +210,7 @@ igt_simple_main
 	igt_assert(data.bufmgr);
 	drm_intel_bufmgr_gem_enable_reuse(data.bufmgr);
 
-	igt_display_init(&data.display, data.drm_fd);
+	igt_display_require(&data.display, data.drm_fd);
 
 	ctx = drm_intel_gem_context_create(data.bufmgr);
 	igt_require(ctx);
diff --git a/tests/kms_flip_event_leak.c b/tests/kms_flip_event_leak.c
index f1a8abd81..50f880ad3 100644
--- a/tests/kms_flip_event_leak.c
+++ b/tests/kms_flip_event_leak.c
@@ -103,7 +103,7 @@ igt_simple_main
 	data.drm_fd = drm_open_driver_master(DRIVER_ANY);
 	kmstest_set_vt_graphics_mode();
 
-	igt_display_init(&data.display, data.drm_fd);
+	igt_display_require(&data.display, data.drm_fd);
 
 	for_each_pipe_with_valid_output(&data.display, pipe, output) {
 		test(&data, pipe, output);
diff --git a/tests/kms_flip_tiling.c b/tests/kms_flip_tiling.c
index beeb111be..d1e6687f7 100644
--- a/tests/kms_flip_tiling.c
+++ b/tests/kms_flip_tiling.c
@@ -151,7 +151,7 @@ igt_main
 		kmstest_set_vt_graphics_mode();
 
 		igt_require_pipe_crc(data.drm_fd);
-		igt_display_init(&data.display, data.drm_fd);
+		igt_display_require(&data.display, data.drm_fd);
 	}
 
 	/*
diff --git a/tests/kms_frontbuffer_tracking.c b/tests/kms_frontbuffer_tracking.c
index 265c313a3..2f677ee5f 100644
--- a/tests/kms_frontbuffer_tracking.c
+++ b/tests/kms_frontbuffer_tracking.c
@@ -1284,7 +1284,7 @@ static void setup_drm(void)
 	drm.debugfs = igt_debugfs_dir(drm.fd);
 
 	kmstest_set_vt_graphics_mode();
-	igt_display_init(&drm.display, drm.fd);
+	igt_display_require(&drm.display, drm.fd);
 
 	drm.bufmgr = drm_intel_bufmgr_gem_init(drm.fd, 4096);
 	igt_assert(drm.bufmgr);
diff --git a/tests/kms_invalid_dotclock.c b/tests/kms_invalid_dotclock.c
index e6e72f528..e7a80d884 100644
--- a/tests/kms_invalid_dotclock.c
+++ b/tests/kms_invalid_dotclock.c
@@ -133,7 +133,7 @@ igt_simple_main
 
 	igt_enable_connectors();
 	kmstest_set_vt_graphics_mode();
-	igt_display_init(&data.display, data.drm_fd);
+	igt_display_require(&data.display, data.drm_fd);
 	data.res = drmModeGetResources(data.drm_fd);
 	kmstest_unset_all_crtcs(data.drm_fd, data.res);
 
diff --git a/tests/kms_legacy_colorkey.c b/tests/kms_legacy_colorkey.c
index 150520ce6..961aa0a31 100644
--- a/tests/kms_legacy_colorkey.c
+++ b/tests/kms_legacy_colorkey.c
@@ -51,7 +51,7 @@ igt_simple_main
 
 	kmstest_set_vt_graphics_mode();
 
-	igt_display_init(&display, drm_fd);
+	igt_display_require(&display, drm_fd);
 
 	for_each_pipe(&display, p) {
 		for_each_plane_on_pipe(&display, p, plane) {
diff --git a/tests/kms_mmap_write_crc.c b/tests/kms_mmap_write_crc.c
index 279d0f67d..93c96ef4d 100644
--- a/tests/kms_mmap_write_crc.c
+++ b/tests/kms_mmap_write_crc.c
@@ -281,7 +281,7 @@ int main(int argc, char **argv)
 
 		igt_require_pipe_crc(data.drm_fd);
 
-		igt_display_init(&data.display, data.drm_fd);
+		igt_display_require(&data.display, data.drm_fd);
 
 		fork_cpuhog_helper();
 	}
diff --git a/tests/kms_panel_fitting.c b/tests/kms_panel_fitting.c
index 7df8104ed..491e429f4 100644
--- a/tests/kms_panel_fitting.c
+++ b/tests/kms_panel_fitting.c
@@ -241,7 +241,7 @@ igt_main
 		igt_skip_on_simulation();
 
 		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_display_require_output(&data.display);
 	}
 
diff --git a/tests/kms_pipe_b_c_ivb.c b/tests/kms_pipe_b_c_ivb.c
index 64086915e..fe2250cee 100644
--- a/tests/kms_pipe_b_c_ivb.c
+++ b/tests/kms_pipe_b_c_ivb.c
@@ -263,7 +263,7 @@ igt_main
 		igt_skip_on(!IS_IVYBRIDGE(devid));
 
 		kmstest_set_vt_graphics_mode();
-		igt_display_init(&data.display, data.drm_fd);
+		igt_display_require(&data.display, data.drm_fd);
 	}
 
 	igt_subtest("pipe-B-dpms-off-modeset-pipe-C")
diff --git a/tests/kms_pipe_crc_basic.c b/tests/kms_pipe_crc_basic.c
index 852f16972..9c9078e9b 100644
--- a/tests/kms_pipe_crc_basic.c
+++ b/tests/kms_pipe_crc_basic.c
@@ -187,7 +187,7 @@ igt_main
 
 		igt_require_pipe_crc(data.drm_fd);
 
-		igt_display_init(&data.display, data.drm_fd);
+		igt_display_require(&data.display, data.drm_fd);
 		data.debugfs = igt_debugfs_dir(data.drm_fd);
 	}
 
diff --git a/tests/kms_plane.c b/tests/kms_plane.c
index 3999dde8f..5a0bc05b0 100644
--- a/tests/kms_plane.c
+++ b/tests/kms_plane.c
@@ -585,7 +585,7 @@ igt_main
 		kmstest_set_vt_graphics_mode();
 
 		igt_require_pipe_crc(data.drm_fd);
-		igt_display_init(&data.display, data.drm_fd);
+		igt_display_require(&data.display, data.drm_fd);
 	}
 
 	for_each_pipe_static(pipe)
diff --git a/tests/kms_plane_lowres.c b/tests/kms_plane_lowres.c
index 5e83ef563..0824ef8fe 100644
--- a/tests/kms_plane_lowres.c
+++ b/tests/kms_plane_lowres.c
@@ -301,7 +301,7 @@ igt_main
 		kmstest_set_vt_graphics_mode();
 
 		igt_require_pipe_crc(data.drm_fd);
-		igt_display_init(&data.display, data.drm_fd);
+		igt_display_require(&data.display, data.drm_fd);
 		igt_require(data.display.is_atomic);
 	}
 
diff --git a/tests/kms_plane_multiple.c b/tests/kms_plane_multiple.c
index a53be0014..721afe594 100644
--- a/tests/kms_plane_multiple.c
+++ b/tests/kms_plane_multiple.c
@@ -393,9 +393,8 @@ int main(int argc, char *argv[])
 		data.drm_fd = drm_open_driver_master(DRIVER_INTEL);
 		kmstest_set_vt_graphics_mode();
 		igt_require_pipe_crc(data.drm_fd);
-		igt_display_init(&data.display, data.drm_fd);
+		igt_display_require(&data.display, data.drm_fd);
 		igt_require(data.display.is_atomic);
-		igt_require(data.display.n_pipes > 0);
 	}
 
 	for_each_pipe_static(pipe) {
diff --git a/tests/kms_plane_scaling.c b/tests/kms_plane_scaling.c
index 7c64ed14b..773162ec7 100644
--- a/tests/kms_plane_scaling.c
+++ b/tests/kms_plane_scaling.c
@@ -544,7 +544,7 @@ igt_main
 	igt_fixture {
 		data.drm_fd = drm_open_driver_master(DRIVER_INTEL);
 		igt_require_pipe_crc(data.drm_fd);
-		igt_display_init(&data.display, data.drm_fd);
+		igt_display_require(&data.display, data.drm_fd);
 		data.devid = intel_get_drm_devid(data.drm_fd);
 		igt_require(data.display.is_atomic);
 	}
diff --git a/tests/kms_properties.c b/tests/kms_properties.c
index 68c0518fc..9548cf440 100644
--- a/tests/kms_properties.c
+++ b/tests/kms_properties.c
@@ -678,7 +678,7 @@ igt_main
 
 		kmstest_set_vt_graphics_mode();
 
-		igt_display_init(&display, display.drm_fd);
+		igt_display_require(&display, display.drm_fd);
 	}
 
 	igt_subtest("plane-properties-legacy")
diff --git a/tests/kms_psr.c b/tests/kms_psr.c
index fcc04770c..ea0c46261 100644
--- a/tests/kms_psr.c
+++ b/tests/kms_psr.c
@@ -111,7 +111,7 @@ static void setup_output(data_t *data)
 
 static void display_init(data_t *data)
 {
-	igt_display_init(&data->display, data->drm_fd);
+	igt_display_require(&data->display, data->drm_fd);
 	setup_output(data);
 }
 
diff --git a/tests/kms_pwrite_crc.c b/tests/kms_pwrite_crc.c
index ee895db63..ffd72e4ab 100644
--- a/tests/kms_pwrite_crc.c
+++ b/tests/kms_pwrite_crc.c
@@ -185,7 +185,7 @@ igt_simple_main
 
 		igt_require_pipe_crc(data.drm_fd);
 
-		igt_display_init(&data.display, data.drm_fd);
+		igt_display_require(&data.display, data.drm_fd);
 	}
 
 	run_test(&data);
diff --git a/tests/kms_rmfb.c b/tests/kms_rmfb.c
index f3461cc91..b1c81cb80 100644
--- a/tests/kms_rmfb.c
+++ b/tests/kms_rmfb.c
@@ -146,7 +146,7 @@ igt_main
 
 		kmstest_set_vt_graphics_mode();
 
-		igt_display_init(&data.display, data.drm_fd);
+		igt_display_require(&data.display, data.drm_fd);
 		igt_display_require_output(&data.display);
 	}
 
diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index f73c6a399..c233ef651 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -532,7 +532,7 @@ igt_main
 
 		igt_require_pipe_crc(data.gfx_fd);
 
-		igt_display_init(&data.display, data.gfx_fd);
+		igt_display_require(&data.display, data.gfx_fd);
 	}
 
 	for (subtest = subtests; subtest->rot; subtest++) {
diff --git a/tests/kms_universal_plane.c b/tests/kms_universal_plane.c
index 58f329e68..cb5070b1d 100644
--- a/tests/kms_universal_plane.c
+++ b/tests/kms_universal_plane.c
@@ -798,7 +798,7 @@ igt_main
 		kmstest_set_vt_graphics_mode();
 
 		igt_require_pipe_crc(data.drm_fd);
-		igt_display_init(&data.display, data.drm_fd);
+		igt_display_require(&data.display, data.drm_fd);
 	}
 
 	for_each_pipe_static(pipe) {
diff --git a/tests/kms_vblank.c b/tests/kms_vblank.c
index 1c0771c6f..dafadb58f 100644
--- a/tests/kms_vblank.c
+++ b/tests/kms_vblank.c
@@ -507,7 +507,7 @@ igt_main
 	igt_fixture {
 		fd = drm_open_driver_master(DRIVER_ANY);
 		kmstest_set_vt_graphics_mode();
-		igt_display_init(&data.display, fd);
+		igt_display_require(&data.display, fd);
 		igt_display_require_output(&data.display);
 	}
 
-- 
2.19.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt-dev] [PATCH i-g-t 1/5] igt/kms_getfb: Check the iface exists before use

[Antonio Argenziano]

On 28/09/18 03:19, Chris Wilson wrote:
> If the driver doesn't support the getfb iface (e.g. because KMS has been
> disabled), the ioctls will fail with ENOTSUP. This is expected, so skip
> the test as nothing useful can be learnt.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   tests/kms_getfb.c | 40 ++++++++++++++++++++++++++++++++++++++--
>   1 file changed, 38 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/kms_getfb.c b/tests/kms_getfb.c
> index 81d796a42..71d65488f 100644
> --- a/tests/kms_getfb.c
> +++ b/tests/kms_getfb.c
> @@ -40,6 +40,40 @@
>   #include "drm.h"
>   #include "drm_fourcc.h"
>   
> +static bool has_getfb_iface(int fd)
> +{
> +	struct drm_mode_fb_cmd arg = { };
> +	int err;
> +
> +	err = 0;
> +	if (drmIoctl(fd, DRM_IOCTL_MODE_GETFB, &arg))
> +		err = -errno;
> +	switch (err) {
> +	case -ENOTTY: /* ioctl unrecognised (kernel too old) */
> +	case -ENOTSUP: /* driver doesn't support KMS */
> +		return false;
> +	default:
> +		return true;
> +	}
> +}
> +
> +static bool has_addfb2_iface(int fd)
> +{
> +	struct drm_mode_fb_cmd2 arg = { };
> +	int err;
> +
> +	err = 0;
> +	if (drmIoctl(fd, DRM_IOCTL_MODE_ADDFB2, &arg) == 0)
> +		err = -errno;

Shouldn't this^ be != 0?

> +	switch (err) {
> +	case -ENOTTY: /* ioctl unrecognised (kernel too old) */
> +	case -ENOTSUP: /* driver doesn't support KMS */
> +		return false;
> +	default:
> +		return true;

Shouldn't we fail on every errno?

Thanks,
Antonio

> +	}
> +}
> +
>   static void get_ccs_fb(int fd, struct drm_mode_fb_cmd2 *ret)
>   {
>   	struct drm_mode_fb_cmd2 add = {
> @@ -54,6 +88,7 @@ static void get_ccs_fb(int fd, struct drm_mode_fb_cmd2 *ret)
>   	};
>   	int size;
>   
> +	igt_require(has_addfb2_iface(fd));
>   	igt_require_intel(fd);
>   
>   	/* An explanation of the magic numbers can be found in kms_ccs.c. */
> @@ -191,15 +226,16 @@ static void test_duplicate_handles(int fd)
>   		do_ioctl(fd, DRM_IOCTL_MODE_RMFB, &add.fb_id);
>   		gem_close(fd, add.handles[0]);
>   	}
> -
>   }
>   
>   igt_main
>   {
>   	int fd;
>   
> -	igt_fixture
> +	igt_fixture {
>   		fd = drm_open_driver_master(DRIVER_ANY);
> +		igt_require(has_getfb_iface(fd));
> +	}
>   
>   	igt_subtest_group
>   		test_handle_input(fd);
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt-dev] [PATCH i-g-t 1/5] igt/kms_getfb: Check the iface exists before use

[Chris Wilson]

Quoting Antonio Argenziano (2018-10-01 19:36:24)
> 
> 
> On 28/09/18 03:19, Chris Wilson wrote:
> > If the driver doesn't support the getfb iface (e.g. because KMS has been
> > disabled), the ioctls will fail with ENOTSUP. This is expected, so skip
> > the test as nothing useful can be learnt.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> >   tests/kms_getfb.c | 40 ++++++++++++++++++++++++++++++++++++++--
> >   1 file changed, 38 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tests/kms_getfb.c b/tests/kms_getfb.c
> > index 81d796a42..71d65488f 100644
> > --- a/tests/kms_getfb.c
> > +++ b/tests/kms_getfb.c
> > @@ -40,6 +40,40 @@
> >   #include "drm.h"
> >   #include "drm_fourcc.h"
> >   
> > +static bool has_getfb_iface(int fd)
> > +{
> > +     struct drm_mode_fb_cmd arg = { };
> > +     int err;
> > +
> > +     err = 0;
> > +     if (drmIoctl(fd, DRM_IOCTL_MODE_GETFB, &arg))
> > +             err = -errno;
> > +     switch (err) {
> > +     case -ENOTTY: /* ioctl unrecognised (kernel too old) */
> > +     case -ENOTSUP: /* driver doesn't support KMS */
> > +             return false;
> > +     default:
> > +             return true;
> > +     }
> > +}
> > +
> > +static bool has_addfb2_iface(int fd)
> > +{
> > +     struct drm_mode_fb_cmd2 arg = { };
> > +     int err;
> > +
> > +     err = 0;
> > +     if (drmIoctl(fd, DRM_IOCTL_MODE_ADDFB2, &arg) == 0)
> > +             err = -errno;
> 
> Shouldn't this^ be != 0?

Yup.
 
> > +     switch (err) {
> > +     case -ENOTTY: /* ioctl unrecognised (kernel too old) */
> > +     case -ENOTSUP: /* driver doesn't support KMS */
> > +             return false;
> > +     default:
> > +             return true;
> 
> Shouldn't we fail on every errno?

No. We want to be very careful in only fail at this point for errno we
know imply the interface is not supported so that we do not confuse an
illegal addfb2 request and fail later in the actual test.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt-dev] [PATCH i-g-t 1/5] igt/kms_getfb: Check the iface exists before use

[Antonio Argenziano]

On 01/10/18 12:43, Chris Wilson wrote:
> Quoting Antonio Argenziano (2018-10-01 19:36:24)
>>
>>
>> On 28/09/18 03:19, Chris Wilson wrote:
>>> If the driver doesn't support the getfb iface (e.g. because KMS has been
>>> disabled), the ioctls will fail with ENOTSUP. This is expected, so skip
>>> the test as nothing useful can be learnt.
>>>
>>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>>> ---
>>>    tests/kms_getfb.c | 40 ++++++++++++++++++++++++++++++++++++++--
>>>    1 file changed, 38 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/tests/kms_getfb.c b/tests/kms_getfb.c
>>> index 81d796a42..71d65488f 100644
>>> --- a/tests/kms_getfb.c
>>> +++ b/tests/kms_getfb.c
>>> @@ -40,6 +40,40 @@
>>>    #include "drm.h"
>>>    #include "drm_fourcc.h"
>>>    
>>> +static bool has_getfb_iface(int fd)
>>> +{
>>> +     struct drm_mode_fb_cmd arg = { };
>>> +     int err;
>>> +
>>> +     err = 0;
>>> +     if (drmIoctl(fd, DRM_IOCTL_MODE_GETFB, &arg))
>>> +             err = -errno;
>>> +     switch (err) {
>>> +     case -ENOTTY: /* ioctl unrecognised (kernel too old) */
>>> +     case -ENOTSUP: /* driver doesn't support KMS */
>>> +             return false;
>>> +     default:
>>> +             return true;
>>> +     }
>>> +}
>>> +
>>> +static bool has_addfb2_iface(int fd)
>>> +{
>>> +     struct drm_mode_fb_cmd2 arg = { };
>>> +     int err;
>>> +
>>> +     err = 0;
>>> +     if (drmIoctl(fd, DRM_IOCTL_MODE_ADDFB2, &arg) == 0)
>>> +             err = -errno;
>>
>> Shouldn't this^ be != 0?
> 
> Yup.
>   
>>> +     switch (err) {
>>> +     case -ENOTTY: /* ioctl unrecognised (kernel too old) */
>>> +     case -ENOTSUP: /* driver doesn't support KMS */
>>> +             return false;
>>> +     default:
>>> +             return true;
>>
>> Shouldn't we fail on every errno?
> 
> No. We want to be very careful in only fail at this point for errno we
> know imply the interface is not supported so that we do not confuse an
> illegal addfb2 request and fail later in the actual test.

Fair enough.

Acked-by: Antonio Argenziano <antonio.argenziano@intel.com>

for the series.

> -Chris
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt-dev] [PATCH i-g-t 1/5] igt/kms_getfb: Check the iface exists before use

[Joonas Lahtinen]

Quoting Antonio Argenziano (2018-10-01 22:53:46)
> Fair enough.
> 
> Acked-by: Antonio Argenziano <antonio.argenziano@intel.com>
> 
> for the series.

Please, read the following chapters (they're applicable for the patch
tag meanings in IGT, too):

https://www.kernel.org/doc/html/v4.18/process/submitting-patches.html#when-to-use-acked-by-cc-and-co-developed-by
https://www.kernel.org/doc/html/v4.18/process/submitting-patches.html#using-reported-by-tested-by-reviewed-by-suggested-by-and-fixes

If we spend the time to actually review the patches, that should be
documented with a proper Reviewed-by and not a vague Acked-by.

Regards, Joonas
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt-dev] [PATCH i-g-t 1/5] igt/kms_getfb: Check the iface exists before use

[Antonio Argenziano]

On 02/10/18 01:30, Joonas Lahtinen wrote:
> Quoting Antonio Argenziano (2018-10-01 22:53:46)
>> Fair enough.
>>
>> Acked-by: Antonio Argenziano <antonio.argenziano@intel.com>
>>
>> for the series.
> 
> Please, read the following chapters (they're applicable for the patch
> tag meanings in IGT, too):
> 
> https://www.kernel.org/doc/html/v4.18/process/submitting-patches.html#when-to-use-acked-by-cc-and-co-developed-by
> https://www.kernel.org/doc/html/v4.18/process/submitting-patches.html#using-reported-by-tested-by-reviewed-by-suggested-by-and-fixes
> 
> If we spend the time to actually review the patches, that should be
> documented with a proper Reviewed-by and not a vague Acked-by.

KMS is really an area I do not know much about. While I can say the 
patches are looking good on the IGT side, I cannot guarantee they use 
the KMS interface appropriately therefore the 'Acked-by'. After reading 
the documentation you linked I think it fits rather well since the only 
feedback I gave was on a small oversight.

Thanks,
Antonio

> 
> Regards, Joonas
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt-dev] [PATCH i-g-t 1/5] igt/kms_getfb: Check the iface exists before use

[Joonas Lahtinen]

Quoting Antonio Argenziano (2018-10-02 23:27:46)
> 
> 
> On 02/10/18 01:30, Joonas Lahtinen wrote:
> > Quoting Antonio Argenziano (2018-10-01 22:53:46)
> >> Fair enough.
> >>
> >> Acked-by: Antonio Argenziano <antonio.argenziano@intel.com>
> >>
> >> for the series.
> > 
> > Please, read the following chapters (they're applicable for the patch
> > tag meanings in IGT, too):
> > 
> > https://www.kernel.org/doc/html/v4.18/process/submitting-patches.html#when-to-use-acked-by-cc-and-co-developed-by
> > https://www.kernel.org/doc/html/v4.18/process/submitting-patches.html#using-reported-by-tested-by-reviewed-by-suggested-by-and-fixes
> > 
> > If we spend the time to actually review the patches, that should be
> > documented with a proper Reviewed-by and not a vague Acked-by.
> 
> KMS is really an area I do not know much about. While I can say the 
> patches are looking good on the IGT side, I cannot guarantee they use 
> the KMS interface appropriately therefore the 'Acked-by'. After reading 
> the documentation you linked I think it fits rather well since the only 
> feedback I gave was on a small oversight.

Fair enough. For future reference, you may want to comment when giving
your Acked-by, the reason for only limited review and not full R-b.

Regards, Joonas
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt-dev] [PATCH i-g-t 1/5] igt/kms_getfb: Check the iface exists before use

[Daniel Vetter]

On Wed, Oct 03, 2018 at 04:04:20PM +0300, Joonas Lahtinen wrote:
> Quoting Antonio Argenziano (2018-10-02 23:27:46)
> > 
> > 
> > On 02/10/18 01:30, Joonas Lahtinen wrote:
> > > Quoting Antonio Argenziano (2018-10-01 22:53:46)
> > >> Fair enough.
> > >>
> > >> Acked-by: Antonio Argenziano <antonio.argenziano@intel.com>
> > >>
> > >> for the series.
> > > 
> > > Please, read the following chapters (they're applicable for the patch
> > > tag meanings in IGT, too):
> > > 
> > > https://www.kernel.org/doc/html/v4.18/process/submitting-patches.html#when-to-use-acked-by-cc-and-co-developed-by
> > > https://www.kernel.org/doc/html/v4.18/process/submitting-patches.html#using-reported-by-tested-by-reviewed-by-suggested-by-and-fixes
> > > 
> > > If we spend the time to actually review the patches, that should be
> > > documented with a proper Reviewed-by and not a vague Acked-by.
> > 
> > KMS is really an area I do not know much about. While I can say the 
> > patches are looking good on the IGT side, I cannot guarantee they use 
> > the KMS interface appropriately therefore the 'Acked-by'. After reading 
> > the documentation you linked I think it fits rather well since the only 
> > feedback I gave was on a small oversight.
> 
> Fair enough. For future reference, you may want to comment when giving
> your Acked-by, the reason for only limited review and not full R-b.

Fyi I also did review this one, but an older version, and those comments
didn't get addressed. So not the only thing that went slightly sideways
here.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx