[PATCH igt v7 0/4] Assert tests, atomic, multi-blob

[PATCH igt v7 0/4] Assert tests, atomic, multi-blob

[Daniel Stone]

Hi,
Following on from the previous few series, most of which have been
merged ...

Add some self-tests for igt-assert_*() to make sure they do the right
thing, including for fds. This is a bit gross, but does work.

Do a Cocci run through the tree. This doesn't actually pick up a lot
of the changes for some reason, but I have not a clue why.

Add a new subtest to core_prop_blob which creates multiple blobs per
connection, exercising a kernel bug.

Resubmit the atomic tests, with only minor changes this time.

Cheers,
Daniel

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH igt v7 1/4] lib/tests: Add igt_assert_*() self-tests

[Daniel Stone]

Make sure our igt_assert variants are doing something that looks vaguely
like the right thing.

Signed-off-by: Daniel Stone <daniels@collabora.com>
---
 lib/tests/Makefile.sources |   1 +
 lib/tests/igt_simple.c     | 173 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 174 insertions(+)
 create mode 100644 lib/tests/igt_simple.c

diff --git a/lib/tests/Makefile.sources b/lib/tests/Makefile.sources
index 58ae36b..fe5df6e 100644
--- a/lib/tests/Makefile.sources
+++ b/lib/tests/Makefile.sources
@@ -10,6 +10,7 @@ check_PROGRAMS = \
 	igt_timeout \
 	igt_invalid_subtest_name \
 	igt_segfault \
+	igt_simple \
 	$(NULL)
 
 check_SCRIPTS = \
diff --git a/lib/tests/igt_simple.c b/lib/tests/igt_simple.c
new file mode 100644
index 0000000..306b1fb
--- /dev/null
+++ b/lib/tests/igt_simple.c
@@ -0,0 +1,173 @@
+/*
+ * Copyright © 2015 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+#include <assert.h>
+#include <errno.h>
+#include <float.h>
+#include <limits.h>
+#include <math.h>
+#include <signal.h>
+#include <stdint.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+
+#include "igt_core.h"
+
+/*
+ * We need to hide assert from the cocci igt test refactor spatch.
+ *
+ * IMPORTANT: Test infrastructure tests are the only valid places where using
+ * assert is allowed.
+ */
+#define internal_assert assert
+
+char test[] = "test";
+char *argv_run[] = { test };
+void (*test_to_run)(void) = NULL;
+
+/*
+ * A really tedious way of making sure we execute every negative test, and that
+ * they all really fail.
+ */
+#define CHECK_NEG(x) { \
+	igt_subtest_f("XFAIL_simple_%d", __LINE__) { \
+		(*exec_before)++; \
+		x; \
+		raise(SIGBUS); \
+	} \
+	exec_total++; \
+}
+
+static int do_fork(void)
+{
+	int pid, status;
+	int argc;
+
+	switch (pid = fork()) {
+	case -1:
+		internal_assert(0);
+	case 0:
+		argc = 1;
+		igt_simple_init(argc, argv_run);
+		test_to_run();
+		igt_exit();
+	default:
+		while (waitpid(pid, &status, 0) == -1 &&
+		       errno == EINTR)
+			;
+
+		if(WIFSIGNALED(status))
+			return WTERMSIG(status) + 128;
+
+		return WEXITSTATUS(status);
+	}
+}
+
+static void test_cmpint_negative(void)
+{
+	int *exec_before = calloc(1, sizeof(int));
+	int exec_total = 0;
+
+	CHECK_NEG(igt_assert_eq(INT_MIN, INT_MAX));
+
+	CHECK_NEG(igt_assert_eq_u32(0xfffffffeUL, 0xffffffffUL));
+
+	CHECK_NEG(igt_assert_eq_u64(0xfffeffffffffffffULL, 0xffffffffffffffffULL));
+	CHECK_NEG(igt_assert_eq_u64(0xfffffffeffffffffULL, 0xffffffffffffffffULL));
+	CHECK_NEG(igt_assert_eq_u64(0xfffffffffffeffffULL, 0xffffffffffffffffULL));
+
+	CHECK_NEG(igt_assert_eq_double(0.0, DBL_MAX));
+	CHECK_NEG(igt_assert_eq_double(DBL_MAX, nexttoward(DBL_MAX, 0.0)));
+
+	if (*exec_before != exec_total)
+		raise(SIGSEGV);
+}
+
+static void test_cmpint(void)
+{
+	igt_assert_eq(0, 0);
+	igt_assert_eq(INT_MAX, INT_MAX);
+	igt_assert_eq(INT_MAX, INT_MAX);
+	igt_assert_neq(INT_MIN, INT_MAX);
+
+	igt_assert_eq_u32(0, 0);
+	igt_assert_eq_u32(0xffffffffUL, 0xffffffffUL);
+	igt_assert_neq_u32(0xfffffffeUL, 0xffffffffUL);
+
+	igt_assert_eq_u64(0, 0);
+	igt_assert_eq_u64(0xffffffffffffffffULL, 0xffffffffffffffffULL);
+	igt_assert_neq_u64(0xfffffffffffffffeULL, 0xffffffffffffffffULL);
+
+	igt_assert_eq_double(0.0, 0.0);
+	igt_assert_eq_double(DBL_MAX, DBL_MAX);
+	igt_assert_neq_double(0.0, DBL_MAX);
+}
+
+static void test_fd_negative(void)
+{
+	int *exec_before = calloc(1, sizeof(int));
+	int exec_total = 0;
+
+	CHECK_NEG(igt_assert_fd(-1));
+	CHECK_NEG(igt_assert_fd(INT_MIN));
+
+	if (*exec_before != exec_total)
+		raise(SIGSEGV);
+}
+
+static void test_fd(void)
+{
+	igt_assert_fd(0);
+	igt_assert_fd(1);
+	igt_assert_fd(INT_MAX);
+}
+
+igt_main
+{
+	int ret;
+
+	igt_subtest("igt_cmpint")
+		test_cmpint();
+
+	/*
+	 * The awkward subtest dance here is because we really want to use
+	 * subtests in our negative tests, to ensure we actually execute all
+	 * the subtests. But we can't begin a subtest within a subtest, and
+	 * we inherit the state from the parent, so ...
+	 */
+	test_to_run = test_cmpint_negative;
+	ret = do_fork();
+	igt_subtest("igt_cmpint_negative")
+		internal_assert(ret == IGT_EXIT_FAILURE);
+
+	igt_subtest("igt_assert_fd")
+		test_fd();
+
+	test_to_run = test_fd_negative;
+	ret = do_fork();
+	igt_subtest("igt_assert_fd_negative")
+		internal_assert(ret == IGT_EXIT_FAILURE);
+}
-- 
2.5.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH igt v7 1/4] lib/tests: Add igt_assert_*() self-tests

[Thomas Wood]

On 29 October 2015 at 10:31, Daniel Stone <daniels@collabora.com> wrote:
> Make sure our igt_assert variants are doing something that looks vaguely
> like the right thing.
>
> Signed-off-by: Daniel Stone <daniels@collabora.com>
> ---
>  lib/tests/Makefile.sources |   1 +
>  lib/tests/igt_simple.c     | 173 +++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 174 insertions(+)
>  create mode 100644 lib/tests/igt_simple.c

The binary name needs to be added to lib/tests/.gitignore.

Also, "igt_assert" might be a more descriptive name for the test.


>
> diff --git a/lib/tests/Makefile.sources b/lib/tests/Makefile.sources
> index 58ae36b..fe5df6e 100644
> --- a/lib/tests/Makefile.sources
> +++ b/lib/tests/Makefile.sources
> @@ -10,6 +10,7 @@ check_PROGRAMS = \
>         igt_timeout \
>         igt_invalid_subtest_name \
>         igt_segfault \
> +       igt_simple \
>         $(NULL)
>
>  check_SCRIPTS = \
> diff --git a/lib/tests/igt_simple.c b/lib/tests/igt_simple.c
> new file mode 100644
> index 0000000..306b1fb
> --- /dev/null
> +++ b/lib/tests/igt_simple.c
> @@ -0,0 +1,173 @@
> +/*
> + * Copyright © 2015 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
> + */
> +
> +#include <assert.h>
> +#include <errno.h>
> +#include <float.h>
> +#include <limits.h>
> +#include <math.h>
> +#include <signal.h>
> +#include <stdint.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +#include <sys/types.h>
> +#include <sys/wait.h>
> +
> +#include "igt_core.h"
> +
> +/*
> + * We need to hide assert from the cocci igt test refactor spatch.
> + *
> + * IMPORTANT: Test infrastructure tests are the only valid places where using
> + * assert is allowed.
> + */
> +#define internal_assert assert
> +
> +char test[] = "test";
> +char *argv_run[] = { test };
> +void (*test_to_run)(void) = NULL;
> +
> +/*
> + * A really tedious way of making sure we execute every negative test, and that
> + * they all really fail.
> + */
> +#define CHECK_NEG(x) { \
> +       igt_subtest_f("XFAIL_simple_%d", __LINE__) { \
> +               (*exec_before)++; \
> +               x; \
> +               raise(SIGBUS); \
> +       } \
> +       exec_total++; \
> +}
> +
> +static int do_fork(void)
> +{
> +       int pid, status;
> +       int argc;
> +
> +       switch (pid = fork()) {
> +       case -1:
> +               internal_assert(0);
> +       case 0:
> +               argc = 1;
> +               igt_simple_init(argc, argv_run);
> +               test_to_run();
> +               igt_exit();
> +       default:
> +               while (waitpid(pid, &status, 0) == -1 &&
> +                      errno == EINTR)
> +                       ;
> +
> +               if(WIFSIGNALED(status))
> +                       return WTERMSIG(status) + 128;
> +
> +               return WEXITSTATUS(status);
> +       }
> +}
> +
> +static void test_cmpint_negative(void)
> +{
> +       int *exec_before = calloc(1, sizeof(int));
> +       int exec_total = 0;
> +
> +       CHECK_NEG(igt_assert_eq(INT_MIN, INT_MAX));
> +
> +       CHECK_NEG(igt_assert_eq_u32(0xfffffffeUL, 0xffffffffUL));
> +
> +       CHECK_NEG(igt_assert_eq_u64(0xfffeffffffffffffULL, 0xffffffffffffffffULL));
> +       CHECK_NEG(igt_assert_eq_u64(0xfffffffeffffffffULL, 0xffffffffffffffffULL));
> +       CHECK_NEG(igt_assert_eq_u64(0xfffffffffffeffffULL, 0xffffffffffffffffULL));
> +
> +       CHECK_NEG(igt_assert_eq_double(0.0, DBL_MAX));
> +       CHECK_NEG(igt_assert_eq_double(DBL_MAX, nexttoward(DBL_MAX, 0.0)));
> +
> +       if (*exec_before != exec_total)
> +               raise(SIGSEGV);
> +}
> +
> +static void test_cmpint(void)
> +{
> +       igt_assert_eq(0, 0);
> +       igt_assert_eq(INT_MAX, INT_MAX);
> +       igt_assert_eq(INT_MAX, INT_MAX);
> +       igt_assert_neq(INT_MIN, INT_MAX);
> +
> +       igt_assert_eq_u32(0, 0);
> +       igt_assert_eq_u32(0xffffffffUL, 0xffffffffUL);
> +       igt_assert_neq_u32(0xfffffffeUL, 0xffffffffUL);
> +
> +       igt_assert_eq_u64(0, 0);
> +       igt_assert_eq_u64(0xffffffffffffffffULL, 0xffffffffffffffffULL);
> +       igt_assert_neq_u64(0xfffffffffffffffeULL, 0xffffffffffffffffULL);
> +
> +       igt_assert_eq_double(0.0, 0.0);
> +       igt_assert_eq_double(DBL_MAX, DBL_MAX);
> +       igt_assert_neq_double(0.0, DBL_MAX);
> +}
> +
> +static void test_fd_negative(void)
> +{
> +       int *exec_before = calloc(1, sizeof(int));
> +       int exec_total = 0;
> +
> +       CHECK_NEG(igt_assert_fd(-1));
> +       CHECK_NEG(igt_assert_fd(INT_MIN));
> +
> +       if (*exec_before != exec_total)
> +               raise(SIGSEGV);
> +}
> +
> +static void test_fd(void)
> +{
> +       igt_assert_fd(0);
> +       igt_assert_fd(1);
> +       igt_assert_fd(INT_MAX);
> +}
> +
> +igt_main
> +{
> +       int ret;
> +
> +       igt_subtest("igt_cmpint")

The subtest names don't usually have an "igt_" prefix.


> +               test_cmpint();
> +
> +       /*
> +        * The awkward subtest dance here is because we really want to use
> +        * subtests in our negative tests, to ensure we actually execute all
> +        * the subtests. But we can't begin a subtest within a subtest, and
> +        * we inherit the state from the parent, so ...
> +        */
> +       test_to_run = test_cmpint_negative;
> +       ret = do_fork();
> +       igt_subtest("igt_cmpint_negative")
> +               internal_assert(ret == IGT_EXIT_FAILURE);
> +
> +       igt_subtest("igt_assert_fd")
> +               test_fd();
> +
> +       test_to_run = test_fd_negative;
> +       ret = do_fork();
> +       igt_subtest("igt_assert_fd_negative")
> +               internal_assert(ret == IGT_EXIT_FAILURE);
> +}
> --
> 2.5.0
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH igt v7 2/4] tests: Run igt.cocci

[Daniel Stone]

Signed-off-by: Daniel Stone <daniels@collabora.com>
---
 tests/drm_import_export.c        |  2 +-
 tests/gem_bad_reloc.c            |  8 ++----
 tests/gem_concurrent_all.c       |  6 ++--
 tests/gem_ctx_exec.c             |  9 ++----
 tests/gem_ctx_param_basic.c      |  4 +--
 tests/gem_mmap_gtt.c             |  8 ++----
 tests/gem_pin.c                  |  4 +--
 tests/gem_pwrite_pread.c         | 12 ++++----
 tests/gem_reg_read.c             |  2 +-
 tests/gem_tiled_swapping.c       |  4 +--
 tests/gem_tiled_wb.c             |  2 +-
 tests/gem_tiled_wc.c             |  2 +-
 tests/kms_draw_crc.c             |  6 ++--
 tests/kms_frontbuffer_tracking.c | 60 ++++++++++++++++++++--------------------
 tests/kms_panel_fitting.c        |  2 +-
 tests/kms_pipe_b_c_ivb.c         | 30 ++++++++++----------
 tests/kms_plane_scaling.c        |  2 +-
 tests/kms_rotation_crc.c         |  4 +--
 tests/pm_backlight.c             | 20 +++++++-------
 19 files changed, 88 insertions(+), 99 deletions(-)

diff --git a/tests/drm_import_export.c b/tests/drm_import_export.c
index 29c228f..49486ab 100644
--- a/tests/drm_import_export.c
+++ b/tests/drm_import_export.c
@@ -212,7 +212,7 @@ static void test_import_close_race(void)
 		else {
 			pthread_mutex_lock(&t_data.mutex);
 			igt_assert_eq(drm_intel_bo_gem_export_to_prime(bo, &(t_data.prime_fd)), 0);
-			igt_assert(t_data.prime_fd != -1);
+			igt_assert_neq(t_data.prime_fd, -1);
 			pthread_mutex_unlock(&t_data.mutex);
 		}
 
diff --git a/tests/gem_bad_reloc.c b/tests/gem_bad_reloc.c
index 9c285ed..e8701da 100644
--- a/tests/gem_bad_reloc.c
+++ b/tests/gem_bad_reloc.c
@@ -107,9 +107,7 @@ static int negative_reloc(int fd, unsigned flags)
 	execbuf.buffer_count = 2;
 	execbuf.batch_len = 8;
 
-	do_or_die(drmIoctl(fd,
-			   DRM_IOCTL_I915_GEM_EXECBUFFER2,
-			   &execbuf));
+	do_ioctl(fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, &execbuf);
 	gem_close(fd, gem_exec[1].handle);
 
 	igt_info("Found offset %lld for 4k batch\n", (long long)gem_exec[0].offset);
@@ -136,9 +134,7 @@ static int negative_reloc(int fd, unsigned flags)
 
 	execbuf.buffer_count = 1;
 	execbuf.flags = flags & USE_LUT;
-	do_or_die(drmIoctl(fd,
-			   DRM_IOCTL_I915_GEM_EXECBUFFER2,
-			   &execbuf));
+	do_ioctl(fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, &execbuf);
 
 	igt_info("Batch is now at offset %lld\n", (long long)gem_exec[0].offset);
 
diff --git a/tests/gem_concurrent_all.c b/tests/gem_concurrent_all.c
index 1d2d787..3970fc6 100644
--- a/tests/gem_concurrent_all.c
+++ b/tests/gem_concurrent_all.c
@@ -298,8 +298,8 @@ gpu_set_bo(drm_intel_bo *bo, uint32_t val, int width, int height)
 	gem_pwrite.offset = 0;
 	gem_pwrite.size = execbuf.batch_len;
 	gem_pwrite.data_ptr = (uintptr_t)buf;
-	do_or_die(drmIoctl(fd, DRM_IOCTL_I915_GEM_PWRITE, &gem_pwrite));
-	do_or_die(drmIoctl(fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, &execbuf));
+	do_ioctl(fd, DRM_IOCTL_I915_GEM_PWRITE, &gem_pwrite);
+	do_ioctl(fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, &execbuf);
 
 	drmIoctl(fd, DRM_IOCTL_GEM_CLOSE, &create.handle);
 }
@@ -868,7 +868,7 @@ static void bit17_require(void)
 	arg.handle = gem_create(fd, 4096);
 	gem_set_tiling(fd, arg.handle, I915_TILING_X, 512);
 
-	do_or_die(drmIoctl(fd, DRM_IOCTL_I915_GEM_GET_TILING2, &arg));
+	do_ioctl(fd, DRM_IOCTL_I915_GEM_GET_TILING2, &arg);
 	gem_close(fd, arg.handle);
 	igt_require(arg.phys_swizzle_mode == arg.swizzle_mode);
 }
diff --git a/tests/gem_ctx_exec.c b/tests/gem_ctx_exec.c
index f2b7cb6..179e991 100644
--- a/tests/gem_ctx_exec.c
+++ b/tests/gem_ctx_exec.c
@@ -116,8 +116,7 @@ static void big_exec(int fd, uint32_t handle, int ring)
 
 	execbuf.buffer_count = 1;
 	i915_execbuffer2_set_context_id(execbuf, ctx_id1);
-	igt_assert(drmIoctl(fd, DRM_IOCTL_I915_GEM_EXECBUFFER2,
-			    &execbuf) == 0);
+	do_ioctl(fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, &execbuf);
 
 	for (i = 0; i < num_buffers; i++) {
 		uint32_t tmp_handle = gem_create(fd, 4096);
@@ -141,12 +140,10 @@ static void big_exec(int fd, uint32_t handle, int ring)
 	       i - 1, num_buffers);
 
 	/* double check that it works */
-	igt_assert(drmIoctl(fd, DRM_IOCTL_I915_GEM_EXECBUFFER2,
-			    &execbuf) == 0);
+	do_ioctl(fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, &execbuf);
 
 	i915_execbuffer2_set_context_id(execbuf, ctx_id2);
-	igt_assert(drmIoctl(fd, DRM_IOCTL_I915_GEM_EXECBUFFER2,
-			    &execbuf) == 0);
+	do_ioctl(fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, &execbuf);
 	gem_sync(fd, handle);
 }
 
diff --git a/tests/gem_ctx_param_basic.c b/tests/gem_ctx_param_basic.c
index 94245ce..b75800c 100644
--- a/tests/gem_ctx_param_basic.c
+++ b/tests/gem_ctx_param_basic.c
@@ -40,9 +40,9 @@ int32_t ctx;
 #define LOCAL_IOCTL_I915_GEM_CONTEXT_SETPARAM DRM_IOWR (DRM_COMMAND_BASE + LOCAL_I915_GEM_CONTEXT_SETPARAM, struct local_i915_gem_context_param)
 
 #define TEST_SUCCESS(ioc) \
-	igt_assert(drmIoctl(fd, (ioc), &ctx_param) == 0);
+	do_ioctl(fd, (ioc), &ctx_param);
 #define TEST_FAIL(ioc, exp_errno) \
-	igt_assert(drmIoctl(fd, (ioc), &ctx_param) < 0 && errno == exp_errno);
+	do_ioctl_err(fd, (ioc), &ctx_param, exp_errno);
 
 igt_main
 {
diff --git a/tests/gem_mmap_gtt.c b/tests/gem_mmap_gtt.c
index 62ba35f..f586bf1 100644
--- a/tests/gem_mmap_gtt.c
+++ b/tests/gem_mmap_gtt.c
@@ -90,9 +90,7 @@ test_access(int fd)
 
 	/* Check that fd1 can mmap. */
 	mmap_arg.handle = handle;
-	igt_assert(drmIoctl(fd,
-			    DRM_IOCTL_I915_GEM_MMAP_GTT,
-			    &mmap_arg) == 0);
+	do_ioctl(fd, DRM_IOCTL_I915_GEM_MMAP_GTT, &mmap_arg);
 
 	igt_assert(mmap64(0, OBJECT_SIZE, PROT_READ | PROT_WRITE,
 			  MAP_SHARED, fd, mmap_arg.offset));
@@ -122,9 +120,7 @@ test_short(int fd)
 	mmap_arg.handle = gem_create(fd, OBJECT_SIZE);
 	igt_assert(mmap_arg.handle);
 
-	igt_assert(drmIoctl(fd,
-			    DRM_IOCTL_I915_GEM_MMAP_GTT,
-			    &mmap_arg) == 0);
+	do_ioctl(fd, DRM_IOCTL_I915_GEM_MMAP_GTT, &mmap_arg);
 	for (pages = 1; pages <= OBJECT_SIZE / PAGE_SIZE; pages <<= 1) {
 		uint8_t *r, *w;
 
diff --git a/tests/gem_pin.c b/tests/gem_pin.c
index e49d0f7..8da75c5 100644
--- a/tests/gem_pin.c
+++ b/tests/gem_pin.c
@@ -80,7 +80,7 @@ static void exec(int fd, uint32_t handle, uint32_t offset)
 	i915_execbuffer2_set_context_id(execbuf, 0);
 	execbuf.rsvd2 = 0;
 
-	do_or_die(drmIoctl(fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, &execbuf));
+	do_ioctl(fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, &execbuf);
 	igt_assert(gem_exec[0].offset == offset);
 }
 
@@ -172,7 +172,7 @@ static void make_busy(int fd, uint32_t handle)
 
 	gem_write(fd, handle, 0, batch, execbuf.batch_len);
 	for (count = 0; count < 10; count++)
-		do_or_die(drmIoctl(fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, &execbuf));
+		do_ioctl(fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, &execbuf);
 	gem_close(fd, tmp);
 }
 
diff --git a/tests/gem_pwrite_pread.c b/tests/gem_pwrite_pread.c
index ef8890e..fe136d3 100644
--- a/tests/gem_pwrite_pread.c
+++ b/tests/gem_pwrite_pread.c
@@ -100,7 +100,7 @@ static void copy(int fd, uint32_t src, uint32_t dst, void *buf, int len, int loo
 
 	while (loops--) {
 		gem_write(fd, src, 0, buf, len);
-		do_or_die(drmIoctl(fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, &execbuf));
+		do_ioctl(fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, &execbuf);
 		gem_read(fd, dst, 0, buf, len);
 	}
 
@@ -120,7 +120,7 @@ static void as_gtt_mmap(int fd, uint32_t src, uint32_t dst, void *buf, int len,
 			       I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
 		memcpy(src_ptr, buf, len);
 
-		do_or_die(drmIoctl(fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, &execbuf));
+		do_ioctl(fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, &execbuf);
 		gem_set_domain(fd, dst,
 			       I915_GEM_DOMAIN_GTT, 0);
 		memcpy(buf, dst_ptr, len);
@@ -145,7 +145,7 @@ static void as_cpu_mmap(int fd, uint32_t src, uint32_t dst, void *buf, int len,
 			       I915_GEM_DOMAIN_CPU, I915_GEM_DOMAIN_CPU);
 		memcpy(src_ptr, buf, len);
 
-		do_or_die(drmIoctl(fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, &execbuf));
+		do_ioctl(fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, &execbuf);
 		gem_set_domain(fd, dst,
 			       I915_GEM_DOMAIN_CPU, 0);
 		memcpy(buf, dst_ptr, len);
@@ -167,7 +167,7 @@ static void test_copy(int fd, uint32_t src, uint32_t dst, uint32_t *buf, int len
 	gem_write(fd, src, 0, buf, len);
 	memset(buf, 0, len);
 
-	do_or_die(drmIoctl(fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, &execbuf));
+	do_ioctl(fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, &execbuf);
 	gem_read(fd, dst, 0, buf, len);
 
 	gem_close(fd, exec[2].handle);
@@ -189,7 +189,7 @@ static void test_as_gtt_mmap(int fd, uint32_t src, uint32_t dst, int len)
 	for (i = 0; i < len/4; i++)
 		src_ptr[i] = i;
 
-	do_or_die(drmIoctl(fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, &execbuf));
+	do_ioctl(fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, &execbuf);
 	gem_close(fd, exec[2].handle);
 
 	gem_set_domain(fd, dst, I915_GEM_DOMAIN_GTT, 0);
@@ -213,7 +213,7 @@ static void test_as_cpu_mmap(int fd, uint32_t src, uint32_t dst, int len)
 	for (i = 0; i < len/4; i++)
 		src_ptr[i] = i;
 
-	do_or_die(drmIoctl(fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, &execbuf));
+	do_ioctl(fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, &execbuf);
 	gem_close(fd, exec[2].handle);
 
 	gem_set_domain(fd, dst, I915_GEM_DOMAIN_CPU, 0);
diff --git a/tests/gem_reg_read.c b/tests/gem_reg_read.c
index 8fa70d0..79facc1 100644
--- a/tests/gem_reg_read.c
+++ b/tests/gem_reg_read.c
@@ -65,7 +65,7 @@ static bool check_kernel_x86_64(void)
 	struct utsname uts;
 
 	ret = uname(&uts);
-	igt_assert(ret == 0);
+	igt_assert_eq(ret, 0);
 
 	if (!strcmp(uts.machine, "x86_64"))
 		return true;
diff --git a/tests/gem_tiled_swapping.c b/tests/gem_tiled_swapping.c
index f45244b..b5849bc 100644
--- a/tests/gem_tiled_swapping.c
+++ b/tests/gem_tiled_swapping.c
@@ -157,8 +157,8 @@ static void thread_fini(struct thread *t)
 
 static void check_memory_layout(void)
 {
-	if (igt_debugfs_search("i915_swizzle_info", "L-shaped"))
-		igt_skip("L-shaped memory configuration detected\n");
+	igt_skip_on_f(igt_debugfs_search("i915_swizzle_info", "L-shaped"),
+		      "L-shaped memory configuration detected\n");
 
 	igt_debug("normal memory configuration detected, continuing\n");
 }
diff --git a/tests/gem_tiled_wb.c b/tests/gem_tiled_wb.c
index 227ebde..67d54bd 100644
--- a/tests/gem_tiled_wb.c
+++ b/tests/gem_tiled_wb.c
@@ -123,7 +123,7 @@ get_tiling(int fd, uint32_t handle, uint32_t *tiling, uint32_t *swizzle)
 	memset(&arg, 0, sizeof(arg));
 	arg.handle = handle;
 
-	do_or_die(drmIoctl(fd, DRM_IOCTL_I915_GEM_GET_TILING2, &arg));
+	do_ioctl(fd, DRM_IOCTL_I915_GEM_GET_TILING2, &arg);
 	igt_require(arg.phys_swizzle_mode == arg.swizzle_mode);
 
 	*tiling = arg.tiling_mode;
diff --git a/tests/gem_tiled_wc.c b/tests/gem_tiled_wc.c
index a6316cc..65ac385 100644
--- a/tests/gem_tiled_wc.c
+++ b/tests/gem_tiled_wc.c
@@ -118,7 +118,7 @@ get_tiling(int fd, uint32_t handle, uint32_t *tiling, uint32_t *swizzle)
 	memset(&arg, 0, sizeof(arg));
 	arg.handle = handle;
 
-	do_or_die(drmIoctl(fd, DRM_IOCTL_I915_GEM_GET_TILING2, &arg));
+	do_ioctl(fd, DRM_IOCTL_I915_GEM_GET_TILING2, &arg);
 	igt_require(arg.phys_swizzle_mode == arg.swizzle_mode);
 
 	*tiling = arg.tiling_mode;
diff --git a/tests/kms_draw_crc.c b/tests/kms_draw_crc.c
index 10a84fa..dccf535 100644
--- a/tests/kms_draw_crc.c
+++ b/tests/kms_draw_crc.c
@@ -138,7 +138,7 @@ static void get_method_crc(enum igt_draw_method method, uint32_t drm_format,
 
 	rc = drmModeSetCrtc(drm_fd, ms.crtc_id, fb.fb_id, 0, 0,
 			    &ms.connector_id, 1, ms.mode);
-	igt_assert(rc == 0);
+	igt_assert_eq(rc, 0);
 
 	igt_pipe_crc_collect_crc(pipe_crc, crc);
 
@@ -181,7 +181,7 @@ static void get_fill_crc(uint64_t tiling, igt_crc_t *crc)
 
 	rc = drmModeSetCrtc(drm_fd, ms.crtc_id, fb.fb_id, 0, 0,
 			    &ms.connector_id, 1, ms.mode);
-	igt_assert(rc == 0);
+	igt_assert_eq(rc, 0);
 
 	igt_pipe_crc_collect_crc(pipe_crc, crc);
 
@@ -207,7 +207,7 @@ static void fill_fb_subtest(void)
 
 	rc = drmModeSetCrtc(drm_fd, ms.crtc_id, fb.fb_id, 0, 0,
 			    &ms.connector_id, 1, ms.mode);
-	igt_assert(rc == 0);
+	igt_assert_eq(rc, 0);
 
 	igt_pipe_crc_collect_crc(pipe_crc, &base_crc);
 
diff --git a/tests/kms_frontbuffer_tracking.c b/tests/kms_frontbuffer_tracking.c
index 15707b9..38ed662 100644
--- a/tests/kms_frontbuffer_tracking.c
+++ b/tests/kms_frontbuffer_tracking.c
@@ -717,7 +717,7 @@ static void fbc_print_status(void)
 	char buf[128];
 
 	igt_debugfs_read("i915_fbc_status", buf);
-	printf("FBC status:\n%s\n", buf);
+	igt_info("FBC status:\n%s\n", buf);
 }
 
 static bool psr_is_enabled(void)
@@ -734,7 +734,7 @@ static void psr_print_status(void)
 	char buf[256];
 
 	igt_debugfs_read("i915_edp_psr_status", buf);
-	printf("PSR status:\n%s\n", buf);
+	igt_info("PSR status:\n%s\n", buf);
 }
 
 static struct timespec fbc_get_last_action(void)
@@ -988,7 +988,7 @@ static struct rect pat4_get_rect(struct fb_region *fb, int r)
 {
 	struct rect rect;
 
-	igt_assert(r == 0);
+	igt_assert_eq(r, 0);
 
 	rect.x = 0;
 	rect.y = 0;
@@ -1058,16 +1058,16 @@ static void unset_all_crtcs(void)
 	for (i = 0; i < drm.res->count_crtcs; i++) {
 		rc = drmModeSetCrtc(drm.fd, drm.res->crtcs[i], -1, 0, 0, NULL,
 				    0, NULL);
-		igt_assert(rc == 0);
+		igt_assert_eq(rc, 0);
 
 		rc = drmModeSetCursor(drm.fd, drm.res->crtcs[i], 0, 0, 0);
-		igt_assert(rc == 0);
+		igt_assert_eq(rc, 0);
 	}
 
 	for (i = 0; i < drm.plane_res->count_planes; i++) {
 		rc = drmModeSetPlane(drm.fd, drm.plane_res->planes[i], 0, 0, 0,
 				     0, 0, 0, 0, 0, 0, 0, 0);
-		igt_assert(rc == 0);
+		igt_assert_eq(rc, 0);
 	}
 }
 
@@ -1119,7 +1119,7 @@ static void start_busy_thread(struct igt_fb *fb)
 	busy_thread.bpp = fb_get_bpp(fb);
 
 	rc = pthread_create(&busy_thread.thread, NULL, busy_thread_func, NULL);
-	igt_assert(rc == 0);
+	igt_assert_eq(rc, 0);
 }
 
 static void stop_busy_thread(void)
@@ -1172,7 +1172,7 @@ static void init_blue_crc(enum pixel_format format)
 	rc = drmModeSetCrtc(drm.fd, prim_mode_params.crtc_id,
 			    blue.fb_id, 0, 0, &prim_mode_params.connector_id, 1,
 			    prim_mode_params.mode);
-	igt_assert(rc == 0);
+	igt_assert_eq(rc, 0);
 	collect_crcs(&blue_crcs[format].crc);
 
 	print_crc("Blue CRC:  ", &blue_crcs[format].crc);
@@ -1220,7 +1220,7 @@ static void init_crcs(enum pixel_format format,
 				   tmp_fbs[r].fb_id, 0, 0,
 				   &prim_mode_params.connector_id, 1,
 				   prim_mode_params.mode);
-		igt_assert(rc == 0);
+		igt_assert_eq(rc, 0);
 		collect_crcs(&pattern->crcs[format][r]);
 	}
 
@@ -1336,7 +1336,7 @@ static void setup_sink_crc(void)
 	set_mode_for_params(&prim_mode_params);
 
 	sink_crc.fd = igt_debugfs_open("i915_sink_crc_eDP1", O_RDONLY);
-	igt_assert(sink_crc.fd >= 0);
+	igt_assert_lte(0, sink_crc.fd);
 
 	rc = read(sink_crc.fd, crc.data, SINK_CRC_SIZE);
 	errno_ = errno;
@@ -1672,13 +1672,13 @@ static void set_cursor_for_test(const struct test_mode *t,
 	fill_fb_region(&params->cursor, COLOR_PRIM_BG);
 
 	rc = drmModeMoveCursor(drm.fd, params->crtc_id, 0, 0);
-	igt_assert(rc == 0);
+	igt_assert_eq(rc, 0);
 
 	rc = drmModeSetCursor(drm.fd, params->crtc_id,
 			      params->cursor.fb->gem_handle,
 			      params->cursor.w,
 			      params->cursor.h);
-	igt_assert(rc == 0);
+	igt_assert_eq(rc, 0);
 
 	do_assertions(ASSERT_NO_ACTION_CHANGE);
 }
@@ -1695,7 +1695,7 @@ static void set_sprite_for_test(const struct test_mode *t,
 			     params->sprite.w, params->sprite.h,
 			     0, 0, params->sprite.w << 16,
 			     params->sprite.h << 16);
-	igt_assert(rc == 0);
+	igt_assert_eq(rc, 0);
 
 	do_assertions(ASSERT_NO_ACTION_CHANGE);
 }
@@ -2170,7 +2170,7 @@ static void wait_flip_event(void)
 		break;
 	case 1:
 		rc = drmHandleEvent(drm.fd, &evctx);
-		igt_assert(rc == 0);
+		igt_assert_eq(rc, 0);
 		break;
 	default:
 		igt_assert_f(false, "Unexpected poll rc %d\n", rc);
@@ -2187,13 +2187,13 @@ static void page_flip_for_params(struct modeset_params *params,
 	case FLIP_PAGEFLIP:
 		rc = drmModePageFlip(drm.fd, params->crtc_id,
 				     params->fb.fb->fb_id, 0, NULL);
-		igt_assert(rc == 0);
+		igt_assert_eq(rc, 0);
 		break;
 	case FLIP_PAGEFLIP_EVENT:
 		rc = drmModePageFlip(drm.fd, params->crtc_id,
 				     params->fb.fb->fb_id,
 				     DRM_MODE_PAGE_FLIP_EVENT, NULL);
-		igt_assert(rc == 0);
+		igt_assert_eq(rc, 0);
 		wait_flip_event();
 		break;
 	case FLIP_MODESET:
@@ -2349,7 +2349,7 @@ static void move_subtest(const struct test_mode *t)
 		case PLANE_CUR:
 			rc = drmModeMoveCursor(drm.fd, params->crtc_id, rect.x,
 					       rect.y);
-			igt_assert(rc == 0);
+			igt_assert_eq(rc, 0);
 			break;
 		case PLANE_SPR:
 			rc = drmModeSetPlane(drm.fd, params->sprite_id,
@@ -2358,7 +2358,7 @@ static void move_subtest(const struct test_mode *t)
 					     rect.x, rect.y, rect.w,
 					     rect.h, 0, 0, rect.w << 16,
 					     rect.h << 16);
-			igt_assert(rc == 0);
+			igt_assert_eq(rc, 0);
 			break;
 		default:
 			igt_assert(false);
@@ -2410,13 +2410,13 @@ static void onoff_subtest(const struct test_mode *t)
 			case PLANE_CUR:
 				rc = drmModeSetCursor(drm.fd, params->crtc_id,
 						      0, 0, 0);
-				igt_assert(rc == 0);
+				igt_assert_eq(rc, 0);
 				break;
 			case PLANE_SPR:
 				rc = drmModeSetPlane(drm.fd, params->sprite_id,
 						     0, 0, 0, 0, 0, 0, 0, 0, 0,
 						     0, 0);
-				igt_assert(rc == 0);
+				igt_assert_eq(rc, 0);
 				break;
 			default:
 				igt_assert(false);
@@ -2430,7 +2430,7 @@ static void onoff_subtest(const struct test_mode *t)
 						  params->cursor.fb->gem_handle,
 						  params->cursor.w,
 						  params->cursor.h);
-				igt_assert(rc == 0);
+				igt_assert_eq(rc, 0);
 				break;
 			case PLANE_SPR:
 				rc = drmModeSetPlane(drm.fd, params->sprite_id,
@@ -2441,7 +2441,7 @@ static void onoff_subtest(const struct test_mode *t)
 						     0,
 						     params->sprite.w << 16,
 						     params->sprite.h << 16);
-				igt_assert(rc == 0);
+				igt_assert_eq(rc, 0);
 				break;
 			default:
 				igt_assert(false);
@@ -2471,7 +2471,7 @@ static bool prim_plane_disabled(void)
 	igt_assert(found);
 
 	rc = drmSetClientCap(drm.fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 0);
-	igt_assert(rc == 0);
+	igt_assert_eq(rc, 0);
 
 	return disabled;
 }
@@ -2513,7 +2513,7 @@ static void fullscreen_plane_subtest(const struct test_mode *t)
 			     fullscreen_fb.height, 0, 0,
 			     fullscreen_fb.width << 16,
 			     fullscreen_fb.height << 16);
-	igt_assert(rc == 0);
+	igt_assert_eq(rc, 0);
 	update_wanted_crc(t, &pattern->crcs[t->format][0]);
 
 	switch (t->screen) {
@@ -2533,7 +2533,7 @@ static void fullscreen_plane_subtest(const struct test_mode *t)
 
 	rc = drmModeSetPlane(drm.fd, params->sprite_id, 0, 0, 0, 0, 0, 0, 0, 0,
 			     0, 0, 0);
-	igt_assert(rc == 0);
+	igt_assert_eq(rc, 0);
 
 	if (t->screen == SCREEN_PRIM)
 		assertions = ASSERT_LAST_ACTION_CHANGED;
@@ -2817,15 +2817,15 @@ static void try_invalid_strides(void)
 
 	/* Smaller than 512, yet still 64-byte aligned. */
 	rc = __gem_set_tiling(drm.fd, gem_handle, I915_TILING_X, 448);
-	igt_assert(rc == -EINVAL);
+	igt_assert_eq(rc, -EINVAL);
 
 	/* Bigger than 512, but not 64-byte aligned. */
 	rc = __gem_set_tiling(drm.fd, gem_handle, I915_TILING_X, 1022);
-	igt_assert(rc == -EINVAL);
+	igt_assert_eq(rc, -EINVAL);
 
 	/* Just make sure something actually works. */
 	rc = __gem_set_tiling(drm.fd, gem_handle, I915_TILING_X, 1024);
-	igt_assert(rc == 0);
+	igt_assert_eq(rc, 0);
 
 	gem_close(drm.fd, gem_handle);
 }
@@ -2950,11 +2950,11 @@ static int opt_handler(int option, int option_index, void *data)
 		igt_assert(errno == 0);
 		break;
 	case '1':
-		igt_assert(opt.only_pipes == PIPE_COUNT);
+		igt_assert_eq(opt.only_pipes, PIPE_COUNT);
 		opt.only_pipes = PIPE_SINGLE;
 		break;
 	case '2':
-		igt_assert(opt.only_pipes == PIPE_COUNT);
+		igt_assert_eq(opt.only_pipes, PIPE_COUNT);
 		opt.only_pipes = PIPE_DUAL;
 		break;
 	default:
diff --git a/tests/kms_panel_fitting.c b/tests/kms_panel_fitting.c
index 6b44241..f8726e2 100644
--- a/tests/kms_panel_fitting.c
+++ b/tests/kms_panel_fitting.c
@@ -122,7 +122,7 @@ static void prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe,
 				&output->id,
 				1,
 				mode);
-		igt_assert(ret == 0);
+		igt_assert_eq(ret, 0);
 	} else {
 		igt_display_commit2(display, s);
 	}
diff --git a/tests/kms_pipe_b_c_ivb.c b/tests/kms_pipe_b_c_ivb.c
index 9d66bda..bd727f0 100644
--- a/tests/kms_pipe_b_c_ivb.c
+++ b/tests/kms_pipe_b_c_ivb.c
@@ -94,7 +94,7 @@ set_mode_on_pipe(data_t *data, enum pipe pipe, igt_output_t *output)
 				    mode->hdisplay, mode->vdisplay,
 				    DRM_FORMAT_XRGB8888, I915_TILING_NONE,
 				    1.0, 1.0, 1.0, &fb);
-	igt_assert(fb_id >= 0);
+	igt_assert_lte(0, fb_id);
 
 	igt_plane_set_fb(primary, &fb);
 	return igt_display_try_commit2(&data->display, COMMIT_LEGACY);
@@ -150,12 +150,12 @@ test_dpms(data_t *data)
 		 kmstest_pipe_name(PIPE_C), igt_output_name(output2));
 
 	ret = set_big_mode_on_pipe(data, PIPE_B, output1);
-	igt_assert(ret == 0);
+	igt_assert_eq(ret, 0);
 
 	kmstest_set_connector_dpms(data->drm_fd, output1->config.connector, DRM_MODE_DPMS_OFF);
 
 	ret = set_big_mode_on_pipe(data, PIPE_C, output2);
-	igt_assert(ret != 0);
+	igt_assert_neq(ret, 0);
 }
 
 static void
@@ -172,13 +172,13 @@ test_lane_reduction(data_t *data)
 		 kmstest_pipe_name(PIPE_C), igt_output_name(output2));
 
 	ret = set_big_mode_on_pipe(data, PIPE_B, output1);
-	igt_assert(ret == 0);
+	igt_assert_eq(ret, 0);
 
 	ret = set_normal_mode_on_pipe(data, PIPE_B, output1);
-	igt_assert(ret == 0);
+	igt_assert_eq(ret, 0);
 
 	ret = set_normal_mode_on_pipe(data, PIPE_C, output2);
-	igt_assert(ret == 0);
+	igt_assert_eq(ret, 0);
 }
 
 static void
@@ -195,16 +195,16 @@ test_disable_pipe_B(data_t *data)
 		 kmstest_pipe_name(PIPE_C), igt_output_name(output2));
 
 	ret = set_big_mode_on_pipe(data, PIPE_B, output1);
-	igt_assert(ret == 0);
+	igt_assert_eq(ret, 0);
 
 	ret = disable_pipe(data, PIPE_B, output1);
-	igt_assert(ret == 0);
+	igt_assert_eq(ret, 0);
 
 	ret = set_normal_mode_on_pipe(data, PIPE_C, output2);
-	igt_assert(ret == 0);
+	igt_assert_eq(ret, 0);
 
 	ret = set_normal_mode_on_pipe(data, PIPE_B, output1);
-	igt_assert(ret == 0);
+	igt_assert_eq(ret, 0);
 }
 
 static void
@@ -221,13 +221,13 @@ test_from_C_to_B_with_3_lanes(data_t *data)
 		 kmstest_pipe_name(PIPE_C), igt_output_name(output2));
 
 	ret = set_normal_mode_on_pipe(data, PIPE_C, output2);
-	igt_assert(ret == 0);
+	igt_assert_eq(ret, 0);
 
 	ret = disable_pipe(data, PIPE_C, output2);
-	igt_assert(ret == 0);
+	igt_assert_eq(ret, 0);
 
 	ret = set_big_mode_on_pipe(data, PIPE_B, output1);
-	igt_assert(ret == 0);
+	igt_assert_eq(ret, 0);
 }
 
 static void
@@ -244,10 +244,10 @@ test_fail_enable_pipe_C_while_B_has_3_lanes(data_t *data)
 		 kmstest_pipe_name(PIPE_C), igt_output_name(output2));
 
 	ret = set_big_mode_on_pipe(data, PIPE_B, output1);
-	igt_assert(ret == 0);
+	igt_assert_eq(ret, 0);
 
 	ret = set_normal_mode_on_pipe(data, PIPE_C, output2);
-	igt_assert(ret != 0);
+	igt_assert_neq(ret, 0);
 }
 
 static data_t data;
diff --git a/tests/kms_plane_scaling.c b/tests/kms_plane_scaling.c
index 22e0388..74bc6f8 100644
--- a/tests/kms_plane_scaling.c
+++ b/tests/kms_plane_scaling.c
@@ -124,7 +124,7 @@ static void prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe,
 				&output->id,
 				1,
 				mode);
-		igt_assert(ret == 0);
+		igt_assert_eq(ret, 0);
 	} else {
 		igt_display_commit2(display, s);
 	}
diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index 4fb00ab..e7126ad 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -268,9 +268,9 @@ static void test_plane_rotation(data_t *data, enum igt_plane plane_type)
 			igt_plane_set_rotation(plane, data->rotation);
 			ret = igt_display_try_commit2(display, commit);
 			if (data->override_fmt || data->override_tiling) {
-				igt_assert(ret == -EINVAL);
+				igt_assert_eq(ret, -EINVAL);
 			} else {
-				igt_assert(ret == 0);
+				igt_assert_eq(ret, 0);
 				igt_pipe_crc_collect_crc(data->pipe_crc,
 							 &crc_output);
 				igt_assert_crc_equal(&data->ref_crc,
diff --git a/tests/pm_backlight.c b/tests/pm_backlight.c
index bc1d417..d24fb10 100644
--- a/tests/pm_backlight.c
+++ b/tests/pm_backlight.c
@@ -95,12 +95,12 @@ static void test_and_verify(int val)
 {
 	int result;
 
-	igt_assert(backlight_write(val, "brightness") == 0);
-	igt_assert(backlight_read(&result, "brightness") == 0);
+	igt_assert_eq(backlight_write(val, "brightness"), 0);
+	igt_assert_eq(backlight_read(&result, "brightness"), 0);
 	/* Check that the exact value sticks */
-	igt_assert(result == val);
+	igt_assert_eq(result, val);
 
-	igt_assert(backlight_read(&result, "actual_brightness") == 0);
+	igt_assert_eq(backlight_read(&result, "actual_brightness"), 0);
 	/* Some rounding may happen depending on hw. Just check that it's close enough. */
 	igt_assert(result <= val + val * TOLERANCE / 100 && result >= val - val * TOLERANCE / 100);
 }
@@ -118,15 +118,15 @@ static void test_bad_brightness(int max)
 	/* First write some sane value */
 	backlight_write(max / 2, "brightness");
 	/* Writing invalid values should fail and not change the value */
-	igt_assert(backlight_write(-1, "brightness") < 0);
+	igt_assert_lt(backlight_write(-1, "brightness"), 0);
 	backlight_read(&val, "brightness");
-	igt_assert(val == max / 2);
-	igt_assert(backlight_write(max + 1, "brightness") < 0);
+	igt_assert_eq(val, max / 2);
+	igt_assert_lt(backlight_write(max + 1, "brightness"), 0);
 	backlight_read(&val, "brightness");
-	igt_assert(val == max / 2);
-	igt_assert(backlight_write(INT_MAX, "brightness") < 0);
+	igt_assert_eq(val, max / 2);
+	igt_assert_lt(backlight_write(INT_MAX, "brightness"), 0);
 	backlight_read(&val, "brightness");
-	igt_assert(val == max / 2);
+	igt_assert_eq(val, max / 2);
 }
 
 static void test_fade(int max)
-- 
2.5.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH igt v7 3/4] tests/core_prop_blob: Add multiple blobs per connection

[Daniel Stone]

This should hit the bug fixed in:
XXX FIXME INSERT SEANPAUL COMMIT CITE

which was introduced with the initial blob support in:
    commit e2f5d2ea479b9b2619965d43db70939589afe43a
    Author: Daniel Stone <daniels@collabora.com>
    Date:   Fri May 22 13:34:51 2015 +0100

        drm/mode: Add user blob-creation ioctl

        Add an ioctl which allows users to create blob properties from supplied
        data. Currently this only supports modes, creating a drm_display_mode from
        the userspace drm_mode_modeinfo.

        v2: Removed size/type checks.
            Rebased on new patches to allow error propagation from create_blob,
            as well as avoiding double-allocation.

        Signed-off-by: Daniel Stone <daniels@collabora.com>
        Reviewed-by: Maarten Lankhorst <maarten.lankhorst@intel.com>
        Tested-by: Sean Paul <seanpaul@chromium.org>
        Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Signed-off-by: Daniel Stone <daniels@collabora.com>
---
 tests/core_prop_blob.c | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/tests/core_prop_blob.c b/tests/core_prop_blob.c
index df4f3ad..365d728 100644
--- a/tests/core_prop_blob.c
+++ b/tests/core_prop_blob.c
@@ -208,6 +208,43 @@ test_lifetime(int fd)
 }
 
 static void
+test_multiple(int fd)
+{
+	uint32_t prop_ids[5];
+	int fd2;
+	int i;
+
+	fd2 = drm_open_driver(DRIVER_ANY);
+	igt_assert_fd(fd2);
+
+	/* Ensure destroying multiple properties explicitly works as needed. */
+	for (i = 0; i < ARRAY_SIZE(prop_ids); i++) {
+		prop_ids[i] = create_prop(fd2);
+		igt_assert_eq(validate_prop(fd, prop_ids[i]), 0);
+		igt_assert_eq(validate_prop(fd2, prop_ids[i]), 0);
+	}
+	for (i = 0; i < ARRAY_SIZE(prop_ids); i++) {
+		igt_assert_eq(destroy_prop(fd2, prop_ids[i]), 0);
+		igt_assert_eq(validate_prop(fd2, prop_ids[i]), ENOENT);
+	}
+	igt_assert_eq(close(fd2), 0);
+
+	fd2 = drm_open_driver(DRIVER_ANY);
+	igt_assert_fd(fd2);
+
+	/* Ensure that multiple properties get cleaned up on fd close. */
+	for (i = 0; i < ARRAY_SIZE(prop_ids); i++) {
+		prop_ids[i] = create_prop(fd2);
+		igt_assert_eq(validate_prop(fd, prop_ids[i]), 0);
+		igt_assert_eq(validate_prop(fd2, prop_ids[i]), 0);
+	}
+	igt_assert_eq(close(fd2), 0);
+
+	for (i = 0; i < ARRAY_SIZE(prop_ids); i++)
+		igt_assert_eq(validate_prop(fd, prop_ids[i]), ENOENT);
+}
+
+static void
 test_core(int fd)
 {
 	uint32_t prop_id;
@@ -256,6 +293,9 @@ igt_main
 	igt_subtest("blob-prop-lifetime")
 		test_lifetime(fd);
 
+	igt_subtest("blob-multiple")
+		test_multiple(fd);
+
 	igt_fixture
 		close(fd);
 }
-- 
2.5.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH igt v7 4/4] kms_atomic: Add basic atomic modesetting tests

[Daniel Stone]

Add tests for KMS atomic modesetting, to exercise the basic interface
and test failure/corner cases. Should ensure coherency between the
legacy and atomic interfaces.

v2: New patch.
v3: Disable connector checking for now, as it was causing GPU hangs on
    newer kernels.
v4: Rebase.
v5: Use do_ioctl or do_ioctl_err consistently. Use igt_assert_*()
    helper macros rather than igt_assert() directly.
    Move assertions into helper/check functions. Define atomic commit
    helper.
v6: Use do_ioctl_err, and define macros to move errors to
    actual callsite, rather than helper functions.
v7: Fix RELAX_MODE thinko and refresh CRTC state in find_crtc.

Co-authored-by: Micah Fedke <micah.fedke@collabora.com>
Signed-off-by: Daniel Stone <daniels@collabora.com>
---
 configure.ac           |    2 +-
 tests/.gitignore       |    1 +
 tests/Makefile.sources |    1 +
 tests/kms_atomic.c     | 1345 ++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 1348 insertions(+), 1 deletion(-)
 create mode 100644 tests/kms_atomic.c

diff --git a/configure.ac b/configure.ac
index 5f97466..97de58a 100644
--- a/configure.ac
+++ b/configure.ac
@@ -85,7 +85,7 @@ if test "x$GCC" = "xyes"; then
 fi
 AC_SUBST(ASSEMBLER_WARN_CFLAGS)
 
-PKG_CHECK_MODULES(DRM, [libdrm_intel >= 2.4.55 libdrm])
+PKG_CHECK_MODULES(DRM, [libdrm_intel >= 2.4.64 libdrm])
 PKG_CHECK_MODULES(PCIACCESS, [pciaccess >= 0.10])
 PKG_CHECK_MODULES(OVERLAY_XVLIB, [xv x11 xext dri2proto >= 2.6], enable_overlay_xvlib=yes, enable_overlay_xvlib=no)
 PKG_CHECK_MODULES(OVERLAY_XLIB, [cairo-xlib dri2proto >= 2.6], enable_overlay_xlib=yes, enable_overlay_xlib=no)
diff --git a/tests/.gitignore b/tests/.gitignore
index beda511..80af9a7 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -126,6 +126,7 @@ gen3_render_tiledy_blits
 gen7_forcewake_mt
 kms_3d
 kms_addfb_basic
+kms_atomic
 kms_crtc_background_color
 kms_cursor_crc
 kms_draw_crc
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index ac731f9..8fb2de8 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -63,6 +63,7 @@ TESTS_progs_M = \
 	gem_userptr_blits \
 	gem_write_read_ring_switch \
 	kms_addfb_basic \
+	kms_atomic \
 	kms_cursor_crc \
 	kms_draw_crc \
 	kms_fbc_crc \
diff --git a/tests/kms_atomic.c b/tests/kms_atomic.c
new file mode 100644
index 0000000..0f7b896
--- /dev/null
+++ b/tests/kms_atomic.c
@@ -0,0 +1,1345 @@
+/*
+ * Copyright © 2015 Intel Corporation
+ * Copyright © 2014-2015 Collabora, Ltd.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors:
+ *    Micah Fedke <micah.fedke@collabora.co.uk>
+ *    Daniel Stone <daniels@collabora.com>
+ *    Pekka Paalanen <pekka.paalanen@collabora.co.uk>
+ */
+
+/*
+ * Testcase: testing atomic modesetting API
+ */
+
+#include <unistd.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <stdint.h>
+#include <string.h>
+#include <errno.h>
+#include <xf86drmMode.h>
+#include <cairo.h>
+#include "drm.h"
+#include "ioctl_wrappers.h"
+#include "drmtest.h"
+#include "igt.h"
+#include "igt_aux.h"
+
+#ifndef DRM_CLIENT_CAP_ATOMIC
+#define DRM_CLIENT_CAP_ATOMIC 3
+#endif
+
+#ifndef DRM_CAP_CURSOR_WIDTH
+#define DRM_CAP_CURSOR_WIDTH 0x8
+#endif
+
+#ifndef DRM_CAP_CURSOR_HEIGHT
+#define DRM_CAP_CURSOR_HEIGHT 0x9
+#endif
+
+#ifndef DRM_MODE_ATOMIC_TEST_ONLY
+#define DRM_MODE_ATOMIC_TEST_ONLY 0x0100
+#define DRM_MODE_ATOMIC_NONBLOCK 0x0200
+#define DRM_MODE_ATOMIC_ALLOW_MODESET 0x0400
+
+struct drm_mode_atomic {
+	__u32 flags;
+	__u32 count_objs;
+	__u64 objs_ptr;
+	__u64 count_props_ptr;
+	__u64 props_ptr;
+	__u64 prop_values_ptr;
+	__u64 reserved;
+	__u64 user_data;
+};
+#endif
+
+IGT_TEST_DESCRIPTION("Test atomic modesetting API");
+
+enum kms_atomic_check_relax {
+	ATOMIC_RELAX_NONE = 0,
+	CRTC_RELAX_MODE = (1 << 0),
+	PLANE_RELAX_FB = (1 << 1)
+};
+
+/**
+ * KMS plane type enum
+ *
+ * KMS plane types are represented by enums, which do not have stable numeric
+ * values, but must be looked up by their string value each time.
+ *
+ * To make the code more simple, we define a plane_type enum which maps to
+ * each KMS enum value. These values must be looked up through the map, and
+ * cannot be passed directly to KMS functions.
+ */
+enum plane_type {
+	PLANE_TYPE_PRIMARY = 0,
+	PLANE_TYPE_OVERLAY,
+	PLANE_TYPE_CURSOR,
+	NUM_PLANE_TYPE_PROPS
+};
+
+static const char *plane_type_prop_names[NUM_PLANE_TYPE_PROPS] = {
+	"Primary",
+	"Overlay",
+	"Cursor"
+};
+
+enum plane_properties {
+	PLANE_SRC_X = 0,
+	PLANE_SRC_Y,
+	PLANE_SRC_W,
+	PLANE_SRC_H,
+	PLANE_CRTC_X,
+	PLANE_CRTC_Y,
+	PLANE_CRTC_W,
+	PLANE_CRTC_H,
+	PLANE_FB_ID,
+	PLANE_CRTC_ID,
+	PLANE_TYPE,
+	NUM_PLANE_PROPS
+};
+
+static const char *plane_prop_names[NUM_PLANE_PROPS] = {
+	"SRC_X",
+	"SRC_Y",
+	"SRC_W",
+	"SRC_H",
+	"CRTC_X",
+	"CRTC_Y",
+	"CRTC_W",
+	"CRTC_H",
+	"FB_ID",
+	"CRTC_ID",
+	"type"
+};
+
+enum crtc_properties {
+	CRTC_MODE_ID = 0,
+	CRTC_ACTIVE,
+	NUM_CRTC_PROPS
+};
+
+static const char *crtc_prop_names[NUM_CRTC_PROPS] = {
+	"MODE_ID",
+	"ACTIVE"
+};
+
+enum connector_properties {
+	CONNECTOR_CRTC_ID = 0,
+	NUM_CONNECTOR_PROPS
+};
+
+static const char *connector_prop_names[NUM_CONNECTOR_PROPS] = {
+	"CRTC_ID"
+};
+
+#define MAX_PLANES 15
+#define MAX_CRTCS 3
+#define MAX_CONNECTORS 8
+
+struct kms_atomic_blob {
+	uint32_t id; /* 0 if not already allocated */
+	size_t len;
+	void *data;
+};
+
+struct kms_atomic_connector_state {
+	struct kms_atomic_state *state;
+	uint32_t obj;
+	uint32_t crtc_id;
+};
+
+struct kms_atomic_plane_state {
+	struct kms_atomic_state *state;
+	uint32_t obj;
+	enum plane_type type;
+	uint32_t crtc_mask;
+	uint32_t crtc_id; /* 0 to disable */
+	uint32_t fb_id; /* 0 to disable */
+	uint32_t src_x, src_y, src_w, src_h; /* 16.16 fixed-point */
+	uint32_t crtc_x, crtc_y, crtc_w, crtc_h; /* normal integers */
+};
+
+struct kms_atomic_crtc_state {
+	struct kms_atomic_state *state;
+	uint32_t obj;
+	int idx;
+	bool active;
+	struct kms_atomic_blob mode;
+};
+
+struct kms_atomic_state {
+	struct kms_atomic_connector_state connectors[MAX_CONNECTORS];
+	struct kms_atomic_crtc_state crtcs[MAX_CRTCS];
+	struct kms_atomic_plane_state planes[MAX_PLANES];
+	struct kms_atomic_desc *desc;
+};
+
+struct kms_atomic_desc {
+	int fd;
+	uint32_t props_connector[NUM_CONNECTOR_PROPS];
+	uint32_t props_crtc[NUM_CRTC_PROPS];
+	uint32_t props_plane[NUM_PLANE_PROPS];
+	uint64_t props_plane_type[NUM_PLANE_TYPE_PROPS];
+};
+
+static uint32_t blob_duplicate(int fd, uint32_t id_orig)
+{
+	drmModePropertyBlobPtr orig = drmModeGetPropertyBlob(fd, id_orig);
+	uint32_t id_new;
+
+	igt_assert(orig);
+	do_or_die(drmModeCreatePropertyBlob(fd, orig->data, orig->length,
+					    &id_new));
+	drmModeFreePropertyBlob(orig);
+
+	return id_new;
+}
+
+#define crtc_set_prop(req, crtc, prop, value) \
+	igt_assert_lt(0, drmModeAtomicAddProperty(req, crtc->obj, \
+						  crtc->state->desc->props_crtc[prop], \
+						  value));
+
+#define plane_set_prop(req, plane, prop, value) \
+	igt_assert_lt(0, drmModeAtomicAddProperty(req, plane->obj, \
+						  plane->state->desc->props_plane[prop], \
+						  value));
+
+#define do_atomic_commit(fd, req, flags) \
+	do_or_die(drmModeAtomicCommit(fd, req, flags, NULL));
+
+#define do_atomic_commit_err(fd, req, flags, err) { \
+	igt_assert_neq(drmModeAtomicCommit(fd, req, flags, NULL), 0); \
+	igt_assert_eq(errno, err); \
+}
+
+#define crtc_commit_atomic(crtc, plane, req, relax) { \
+	drmModeAtomicSetCursor(req, 0); \
+	crtc_populate_req(crtc, req); \
+	plane_populate_req(plane, req); \
+	do_atomic_commit((crtc)->state->desc->fd, req, 0); \
+	crtc_check_current_state(crtc, plane, relax); \
+	plane_check_current_state(plane, relax); \
+}
+
+#define crtc_commit_atomic_err(crtc, plane, crtc_old, plane_old, req, relax, e) { \
+	drmModeAtomicSetCursor(req, 0); \
+	crtc_populate_req(crtc, req); \
+	plane_populate_req(plane, req); \
+	do_atomic_commit_err((crtc)->state->desc->fd, req, 0, e); \
+	crtc_check_current_state(crtc_old, plane_old, relax); \
+	plane_check_current_state(plane_old, relax); \
+}
+
+#define plane_commit_atomic(plane, req, relax) { \
+	drmModeAtomicSetCursor(req, 0); \
+	plane_populate_req(plane, req); \
+	do_atomic_commit((plane)->state->desc->fd, req, 0); \
+	plane_check_current_state(plane, relax); \
+}
+
+#define plane_commit_atomic_err(plane, plane_old, req, relax, e) { \
+	drmModeAtomicSetCursor(req, 0); \
+	plane_populate_req(plane, req); \
+	do_atomic_commit_err((plane)->state->desc->fd, req, 0, e); \
+	plane_check_current_state(plane_old, relax); \
+}
+
+static void
+connector_get_current_state(struct kms_atomic_connector_state *connector)
+{
+	drmModeObjectPropertiesPtr props;
+	int i;
+
+	props = drmModeObjectGetProperties(connector->state->desc->fd,
+					   connector->obj,
+					   DRM_MODE_OBJECT_CONNECTOR);
+	igt_assert(props);
+
+	for (i = 0; i < props->count_props; i++) {
+		uint32_t *prop_ids = connector->state->desc->props_connector;
+
+		if (props->props[i] == prop_ids[CONNECTOR_CRTC_ID])
+			connector->crtc_id = props->prop_values[i];
+	}
+	drmModeFreeObjectProperties(props);
+}
+
+#if 0
+/* XXX: Checking this repeatedly actually hangs the GPU. I have literally no
+ *      idea why. */
+static void
+connector_check_current_state(struct kms_atomic_connector_state *connector)
+{
+	struct kms_atomic_connector_state connector_kernel;
+	drmModeConnectorPtr legacy;
+	uint32_t crtc_id;
+
+	legacy = drmModeGetConnectorCurrent(connector->state->desc->fd,
+					    connector->obj);
+	igt_assert(legacy);
+
+	if (legacy->encoder_id) {
+		drmModeEncoderPtr legacy_enc;
+
+		legacy_enc = drmModeGetEncoder(connector->state->desc->fd,
+					       legacy->encoder_id);
+		igt_assert(legacy_enc);
+
+		crtc_id = legacy_enc->crtc_id;
+		drmModeFreeEncoder(legacy_enc);
+	} else {
+		crtc_id = 0;
+	}
+
+	igt_assert_eq_u32(crtc_id, connector->crtc_id);
+
+	memcpy(&connector_kernel, connector, sizeof(connector_kernel));
+	connector_get_current_state(&connector_kernel);
+	do_or_die(memcmp(&connector_kernel, connector,
+			 sizeof(connector_kernel)));
+
+	drmModeFreeConnector(legacy);
+}
+#endif
+
+static struct kms_atomic_connector_state *
+find_connector(struct kms_atomic_state *state,
+	       struct kms_atomic_crtc_state *crtc)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(state->connectors); i++) {
+		struct kms_atomic_connector_state *connector =
+			&state->connectors[i];
+
+		if (!connector->obj)
+			continue;
+		if (crtc && connector->crtc_id != crtc->obj)
+			continue;
+
+		return connector;
+	}
+
+	return NULL;
+}
+
+static void plane_populate_req(struct kms_atomic_plane_state *plane,
+			       drmModeAtomicReq *req)
+{
+	plane_set_prop(req, plane, PLANE_CRTC_ID, plane->crtc_id);
+	plane_set_prop(req, plane, PLANE_FB_ID, plane->fb_id);
+	plane_set_prop(req, plane, PLANE_SRC_X, plane->src_x);
+	plane_set_prop(req, plane, PLANE_SRC_Y, plane->src_y);
+	plane_set_prop(req, plane, PLANE_SRC_W, plane->src_w);
+	plane_set_prop(req, plane, PLANE_SRC_H, plane->src_h);
+	plane_set_prop(req, plane, PLANE_CRTC_X, plane->crtc_x);
+	plane_set_prop(req, plane, PLANE_CRTC_Y, plane->crtc_y);
+	plane_set_prop(req, plane, PLANE_CRTC_W, plane->crtc_w);
+	plane_set_prop(req, plane, PLANE_CRTC_H, plane->crtc_h);
+}
+
+static void plane_get_current_state(struct kms_atomic_plane_state *plane)
+{
+	struct kms_atomic_desc *desc = plane->state->desc;
+	drmModeObjectPropertiesPtr props;
+	int i;
+
+	props = drmModeObjectGetProperties(desc->fd, plane->obj,
+					   DRM_MODE_OBJECT_PLANE);
+	igt_assert(props);
+
+	for (i = 0; i < props->count_props; i++) {
+		uint32_t *prop_ids = desc->props_plane;
+
+		if (props->props[i] == prop_ids[PLANE_CRTC_ID])
+			plane->crtc_id = props->prop_values[i];
+		else if (props->props[i] == prop_ids[PLANE_FB_ID])
+			plane->fb_id = props->prop_values[i];
+		else if (props->props[i] == prop_ids[PLANE_CRTC_X])
+			plane->crtc_x = props->prop_values[i];
+		else if (props->props[i] == prop_ids[PLANE_CRTC_Y])
+			plane->crtc_y = props->prop_values[i];
+		else if (props->props[i] == prop_ids[PLANE_CRTC_W])
+			plane->crtc_w = props->prop_values[i];
+		else if (props->props[i] == prop_ids[PLANE_CRTC_H])
+			plane->crtc_h = props->prop_values[i];
+		else if (props->props[i] == prop_ids[PLANE_SRC_X])
+			plane->src_x = props->prop_values[i];
+		else if (props->props[i] == prop_ids[PLANE_SRC_Y])
+			plane->src_y = props->prop_values[i];
+		else if (props->props[i] == prop_ids[PLANE_SRC_W])
+			plane->src_w = props->prop_values[i];
+		else if (props->props[i] == prop_ids[PLANE_SRC_H])
+			plane->src_h = props->prop_values[i];
+		else if (props->props[i] == prop_ids[PLANE_TYPE]) {
+			int j;
+
+			for (j = 0; j < ARRAY_SIZE(desc->props_plane_type); j++) {
+				if (props->prop_values[i] == desc->props_plane_type[j]) {
+					plane->type = j;
+					break;
+				}
+			}
+		}
+	}
+
+	drmModeFreeObjectProperties(props);
+}
+
+static void plane_check_current_state(struct kms_atomic_plane_state *plane,
+				      enum kms_atomic_check_relax relax)
+{
+	drmModePlanePtr legacy;
+	struct kms_atomic_plane_state plane_kernel;
+
+	legacy = drmModeGetPlane(plane->state->desc->fd, plane->obj);
+	igt_assert(legacy);
+
+	igt_assert_eq_u32(legacy->crtc_id, plane->crtc_id);
+
+	if (!(relax & PLANE_RELAX_FB))
+		igt_assert_eq_u32(legacy->fb_id, plane->fb_id);
+
+	memcpy(&plane_kernel, plane, sizeof(plane_kernel));
+	plane_get_current_state(&plane_kernel);
+
+	/* Legacy cursor ioctls create their own, unknowable, internal
+	 * framebuffer which we can't reason about. */
+	if (relax & PLANE_RELAX_FB)
+		plane_kernel.fb_id = plane->fb_id;
+	do_or_die(memcmp(&plane_kernel, plane, sizeof(plane_kernel)));
+
+	drmModeFreePlane(legacy);
+}
+
+static void plane_commit_legacy(struct kms_atomic_plane_state *plane,
+                                enum kms_atomic_check_relax relax)
+{
+	do_or_die(drmModeSetPlane(plane->state->desc->fd, plane->obj,
+				  plane->crtc_id,
+				  plane->fb_id, 0,
+				  plane->crtc_x, plane->crtc_y,
+				  plane->crtc_w, plane->crtc_h,
+				  plane->src_x, plane->src_y,
+				  plane->src_w, plane->src_h));
+	plane_check_current_state(plane, relax);
+}
+
+static struct kms_atomic_plane_state *
+find_plane(struct kms_atomic_state *state, enum plane_type type,
+	   struct kms_atomic_crtc_state *crtc)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(state->planes); i++) {
+		struct kms_atomic_plane_state *plane = &state->planes[i];
+
+		if (!plane->obj)
+			continue;
+		if (type != NUM_PLANE_TYPE_PROPS && plane->type != type)
+			continue;
+		if (crtc && !(plane->crtc_mask & (1 << crtc->idx)))
+			continue;
+
+		plane_get_current_state(plane);
+		return plane;
+	}
+
+	return NULL;
+}
+
+static void crtc_populate_req(struct kms_atomic_crtc_state *crtc,
+			      drmModeAtomicReq *req)
+{
+	crtc_set_prop(req, crtc, CRTC_MODE_ID, crtc->mode.id);
+	crtc_set_prop(req, crtc, CRTC_ACTIVE, crtc->active);
+}
+
+static void crtc_get_current_state(struct kms_atomic_crtc_state *crtc)
+{
+	drmModeObjectPropertiesPtr props;
+	int i;
+
+	props = drmModeObjectGetProperties(crtc->state->desc->fd, crtc->obj,
+					   DRM_MODE_OBJECT_CRTC);
+	igt_assert(props);
+
+	for (i = 0; i < props->count_props; i++) {
+		uint32_t *prop_ids = crtc->state->desc->props_crtc;
+
+		if (props->props[i] == prop_ids[CRTC_MODE_ID]) {
+			drmModePropertyBlobPtr blob;
+
+			crtc->mode.id = props->prop_values[i];
+			if (!crtc->mode.id) {
+				crtc->mode.len = 0;
+				continue;
+			}
+
+			blob = drmModeGetPropertyBlob(crtc->state->desc->fd,
+						      crtc->mode.id);
+			igt_assert(blob);
+			igt_assert_eq_u32(blob->length,
+					  sizeof(struct drm_mode_modeinfo));
+
+			if (!crtc->mode.data ||
+			    memcmp(crtc->mode.data, blob->data, blob->length) != 0)
+				crtc->mode.data = blob->data;
+			crtc->mode.len = blob->length;
+		}
+		else if (props->props[i] == prop_ids[CRTC_ACTIVE]) {
+			crtc->active = props->prop_values[i];
+		}
+	}
+
+	drmModeFreeObjectProperties(props);
+}
+
+static void crtc_check_current_state(struct kms_atomic_crtc_state *crtc,
+				     struct kms_atomic_plane_state *primary,
+				     enum kms_atomic_check_relax relax)
+{
+	struct kms_atomic_crtc_state crtc_kernel;
+	drmModeCrtcPtr legacy;
+
+	legacy = drmModeGetCrtc(crtc->state->desc->fd, crtc->obj);
+	igt_assert(legacy);
+
+	igt_assert_eq_u32(legacy->crtc_id, crtc->obj);
+	igt_assert_eq_u32(legacy->x, primary->src_x >> 16);
+	igt_assert_eq_u32(legacy->y, primary->src_y >> 16);
+
+	if (crtc->active)
+		igt_assert_eq_u32(legacy->buffer_id, primary->fb_id);
+	else
+		igt_assert_eq_u32(legacy->buffer_id, 0);
+
+	if (legacy->mode_valid) {
+		igt_assert_neq(legacy->mode_valid, 0);
+		igt_assert_eq(crtc->mode.len,
+		              sizeof(struct drm_mode_modeinfo));
+		do_or_die(memcmp(&legacy->mode, crtc->mode.data,
+		                 crtc->mode.len));
+		igt_assert_eq(legacy->width, legacy->mode.hdisplay);
+		igt_assert_eq(legacy->height, legacy->mode.vdisplay);
+	} else {
+		igt_assert_eq(legacy->mode_valid, 0);
+	}
+
+	memcpy(&crtc_kernel, crtc, sizeof(crtc_kernel));
+	crtc_get_current_state(&crtc_kernel);
+
+	if (crtc_kernel.mode.id != 0)
+		igt_assert_eq(crtc_kernel.mode.len,
+		              sizeof(struct drm_mode_modeinfo));
+
+	/* Optionally relax the check for MODE_ID: using the legacy SetCrtc
+	 * API can potentially change MODE_ID even if the mode itself remains
+	 * unchanged. */
+	if (((relax & CRTC_RELAX_MODE) &&
+	    (crtc_kernel.mode.id != crtc->mode.id &&
+	     crtc_kernel.mode.id != 0 && crtc->mode.id != 0)) &&
+	    memcmp(crtc_kernel.mode.data, crtc->mode.data,
+		   sizeof(struct drm_mode_modeinfo)) == 0) {
+		crtc_kernel.mode.id = crtc->mode.id;
+		crtc_kernel.mode.data = crtc->mode.data;
+	}
+
+	do_or_die(memcmp(&crtc_kernel, crtc, sizeof(crtc_kernel)));
+
+	drmModeFreeCrtc(legacy);
+}
+
+static void crtc_commit_legacy(struct kms_atomic_crtc_state *crtc,
+			       struct kms_atomic_plane_state *plane,
+			       enum kms_atomic_check_relax relax)
+{
+	drmModeObjectPropertiesPtr props;
+	uint32_t connectors[MAX_CONNECTORS];
+	int num_connectors = 0;
+	int i;
+
+	if (!crtc->active) {
+		do_or_die(drmModeSetCrtc(crtc->state->desc->fd,
+					 crtc->obj, 0, 0, 0, NULL, 0, NULL));
+		return;
+	}
+
+	igt_assert_neq_u32(crtc->mode.id, 0);
+
+	for (i = 0; i < MAX_CONNECTORS; i++) {
+		struct kms_atomic_connector_state *connector =
+			&crtc->state->connectors[i];
+
+		if (connector->crtc_id != crtc->obj)
+			continue;
+
+		connectors[num_connectors++] = connector->obj;
+	}
+
+	do_or_die(drmModeSetCrtc(crtc->state->desc->fd, crtc->obj,
+	                         plane->fb_id,
+				 plane->src_x >> 16, plane->src_y >> 16,
+				 (num_connectors) ? connectors : NULL,
+				 num_connectors,
+				 crtc->mode.data));
+	/* When doing a legacy commit, the core may update MODE_ID to be a new
+	 * blob implicitly created by the legacy request. Hence we backfill
+	 * the value in the state object to ensure they match. */
+	props = drmModeObjectGetProperties(crtc->state->desc->fd, crtc->obj,
+					   DRM_MODE_OBJECT_CRTC);
+	igt_assert(props);
+
+	for (i = 0; i < props->count_props; i++) {
+		if (props->props[i] !=
+		    crtc->state->desc->props_crtc[CRTC_MODE_ID])
+			continue;
+		crtc->mode.id = props->prop_values[i];
+		break;
+	}
+
+	drmModeFreeObjectProperties(props);
+
+	crtc_check_current_state(crtc, plane, relax);
+	plane_check_current_state(plane, relax);
+}
+
+static struct kms_atomic_crtc_state *find_crtc(struct kms_atomic_state *state,
+					       bool must_be_enabled)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(state->crtcs); i++) {
+		struct kms_atomic_crtc_state *crtc = &state->crtcs[i];
+
+		if (!crtc->obj)
+			continue;
+		if (must_be_enabled && !crtc->active)
+			continue;
+
+		crtc_get_current_state(crtc);
+		return crtc;
+	}
+
+	return NULL;
+}
+
+static void fill_obj_props(int fd, uint32_t id, int type, int num_props,
+			   const char **prop_names, uint32_t *prop_ids)
+{
+	drmModeObjectPropertiesPtr props;
+	int i, j;
+
+	props = drmModeObjectGetProperties(fd, id, type);
+	igt_assert(props);
+
+	for (i = 0; i < props->count_props; i++) {
+		drmModePropertyPtr prop =
+			drmModeGetProperty(fd, props->props[i]);
+
+		for (j = 0; j < num_props; j++) {
+			if (strcmp(prop->name, prop_names[j]) != 0)
+				continue;
+			prop_ids[j] = props->props[i];
+			break;
+		}
+
+		drmModeFreeProperty(prop);
+	}
+
+	drmModeFreeObjectProperties(props);
+}
+
+static void fill_obj_prop_map(int fd, uint32_t id, int type, const char *name,
+			      int num_enums, const char **enum_names,
+			      uint64_t *enum_ids)
+{
+	drmModeObjectPropertiesPtr props;
+	int i, j, k;
+
+	props = drmModeObjectGetProperties(fd, id, type);
+	igt_assert(props);
+
+	for (i = 0; i < props->count_props; i++) {
+		drmModePropertyPtr prop =
+			drmModeGetProperty(fd, props->props[i]);
+
+		igt_assert(prop);
+
+		if (strcmp(prop->name, name) != 0) {
+			drmModeFreeProperty(prop);
+			continue;
+		}
+
+		for (j = 0; j < prop->count_enums; j++) {
+			struct drm_mode_property_enum *e = &prop->enums[j];
+
+			for (k = 0; k < num_enums; k++) {
+				if (strcmp(e->name, enum_names[k]) != 0)
+					continue;
+
+				enum_ids[k] = e->value;
+				break;
+			}
+		}
+
+		drmModeFreeProperty(prop);
+	}
+}
+
+static void atomic_setup(struct kms_atomic_state *state)
+{
+	struct kms_atomic_desc *desc = state->desc;
+	drmModeResPtr res;
+	drmModePlaneResPtr res_plane;
+	int i;
+
+	desc->fd = drm_open_driver_master(DRIVER_INTEL);
+	igt_assert_fd(desc->fd);
+
+	do_or_die(drmSetClientCap(desc->fd, DRM_CLIENT_CAP_ATOMIC, 1));
+
+	res = drmModeGetResources(desc->fd);
+	res_plane = drmModeGetPlaneResources(desc->fd);
+	igt_assert(res);
+	igt_assert(res_plane);
+
+	igt_assert_lt(0, res->count_crtcs);
+	igt_assert_lt(0, res_plane->count_planes);
+	igt_assert_lt(0, res->count_connectors);
+
+
+	fill_obj_props(desc->fd, res->crtcs[0],
+		       DRM_MODE_OBJECT_CRTC, NUM_CRTC_PROPS,
+		       crtc_prop_names, desc->props_crtc);
+
+	fill_obj_props(desc->fd, res_plane->planes[0],
+		       DRM_MODE_OBJECT_PLANE, NUM_PLANE_PROPS,
+		       plane_prop_names, desc->props_plane);
+	fill_obj_prop_map(desc->fd, res_plane->planes[0],
+			  DRM_MODE_OBJECT_PLANE, "type",
+			  NUM_PLANE_TYPE_PROPS, plane_type_prop_names,
+			  desc->props_plane_type);
+
+	fill_obj_props(desc->fd, res->connectors[0],
+		       DRM_MODE_OBJECT_CONNECTOR, NUM_CONNECTOR_PROPS,
+		       connector_prop_names, desc->props_connector);
+
+	for (i = 0; i < res->count_crtcs; i++) {
+		struct kms_atomic_crtc_state *crtc = &state->crtcs[i];
+
+		crtc->state = state;
+		crtc->obj = res->crtcs[i];
+		crtc->idx = i;
+		crtc_get_current_state(crtc);
+
+		/* The blob pointed to by MODE_ID could well be transient,
+		 * and lose its last reference as we switch away from it.
+		 * Duplicate the blob here so we have a reference we know we
+		 * own. */
+		if (crtc->mode.id != 0)
+		    crtc->mode.id = blob_duplicate(desc->fd, crtc->mode.id);
+	}
+
+	for (i = 0; i < res_plane->count_planes; i++) {
+		drmModePlanePtr plane =
+			drmModeGetPlane(desc->fd, res_plane->planes[i]);
+		igt_assert(plane);
+
+		state->planes[i].state = state;
+		state->planes[i].obj = res_plane->planes[i];
+		state->planes[i].crtc_mask = plane->possible_crtcs;
+		plane_get_current_state(&state->planes[i]);
+	}
+
+	for (i = 0; i < res->count_connectors; i++) {
+		state->connectors[i].state = state;
+		state->connectors[i].obj = res->connectors[i];
+		connector_get_current_state(&state->connectors[i]);
+	}
+
+	drmModeFreePlaneResources(res_plane);
+	drmModeFreeResources(res);
+}
+
+static uint32_t plane_get_igt_format(struct kms_atomic_plane_state *plane)
+{
+	drmModePlanePtr plane_kms;
+	const uint32_t *igt_formats;
+	uint32_t ret = 0;
+	int num_igt_formats;
+	int i;
+
+	plane_kms = drmModeGetPlane(plane->state->desc->fd, plane->obj);
+	igt_assert(plane_kms);
+
+	igt_get_all_formats(&igt_formats, &num_igt_formats);
+	for (i = 0; i < num_igt_formats; i++) {
+		int j;
+
+		for (j = 0; j < plane_kms->count_formats; j++) {
+			if (plane_kms->formats[j] == igt_formats[i]) {
+				ret = plane_kms->formats[j];
+				break;
+			}
+		}
+	}
+
+	drmModeFreePlane(plane_kms);
+	return ret;
+}
+
+static void plane_overlay(struct kms_atomic_crtc_state *crtc,
+			  struct kms_atomic_plane_state *plane_old)
+{
+	struct drm_mode_modeinfo *mode = crtc->mode.data;
+	struct kms_atomic_plane_state plane = *plane_old;
+	uint32_t format = plane_get_igt_format(&plane);
+	drmModeAtomicReq *req = drmModeAtomicAlloc();
+	struct igt_fb fb;
+	cairo_t *cr;
+
+	igt_require(req);
+	igt_require(format != 0);
+
+	plane.src_x = 0;
+	plane.src_y = 0;
+	plane.src_w = (mode->hdisplay / 2) << 16;
+	plane.src_h = (mode->vdisplay / 2) << 16;
+	plane.crtc_x = mode->hdisplay / 4;
+	plane.crtc_y = mode->vdisplay / 4;
+	plane.crtc_w = mode->hdisplay / 2;
+	plane.crtc_h = mode->vdisplay / 2;
+	plane.crtc_id = crtc->obj;
+	plane.fb_id = igt_create_fb(plane.state->desc->fd,
+				    plane.crtc_w, plane.crtc_h,
+				    format, I915_TILING_NONE, &fb);
+
+	cr = igt_get_cairo_ctx(plane.state->desc->fd, &fb);
+	igt_paint_test_pattern(cr, plane.crtc_w, plane.crtc_h);
+
+	/* Enable the overlay plane using the atomic API, and double-check
+	 * state is what we think it should be. */
+	plane_commit_atomic(&plane, req, ATOMIC_RELAX_NONE);
+
+	/* Disable the plane and check the state matches the old. */
+	plane_commit_atomic(plane_old, req, ATOMIC_RELAX_NONE);
+
+	/* Re-enable the plane through the legacy plane API, and verify through
+	 * atomic. */
+	plane_commit_legacy(&plane, ATOMIC_RELAX_NONE);
+
+	/* Restore the plane to its original settings through the legacy plane
+	 * API, and verify through atomic. */
+	plane_commit_legacy(plane_old, ATOMIC_RELAX_NONE);
+
+	drmModeAtomicFree(req);
+}
+
+static void plane_primary(struct kms_atomic_crtc_state *crtc,
+			  struct kms_atomic_plane_state *plane_old)
+{
+	struct drm_mode_modeinfo *mode = crtc->mode.data;
+	struct kms_atomic_plane_state plane = *plane_old;
+	uint32_t format = plane_get_igt_format(&plane);
+	drmModeAtomicReq *req = drmModeAtomicAlloc();
+	uint32_t connectors[MAX_CONNECTORS] = { 0, };
+	int num_connectors;
+	struct igt_fb fb;
+	cairo_t *cr;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(connectors); i++) {
+		if (crtc->state->connectors[i].crtc_id == crtc->obj)
+			connectors[num_connectors++] =
+				crtc->state->connectors[i].obj;
+	}
+
+	igt_require(format != 0);
+
+	plane.src_x = 0;
+	plane.src_y = 0;
+	plane.src_w = mode->hdisplay << 16;
+	plane.src_h = mode->vdisplay << 16;
+	plane.crtc_x = 0;
+	plane.crtc_y = 0;
+	plane.crtc_w = mode->hdisplay;
+	plane.crtc_h = mode->vdisplay;
+	plane.crtc_id = crtc->obj;
+	plane.fb_id = igt_create_fb(plane.state->desc->fd,
+				    plane.crtc_w, plane.crtc_h,
+				    format, I915_TILING_NONE, &fb);
+
+	cr = igt_get_cairo_ctx(plane.state->desc->fd, &fb);
+	igt_paint_test_pattern(cr, plane.crtc_w, plane.crtc_h);
+
+	/* Flip the primary plane using the atomic API, and double-check
+	 * state is what we think it should be. */
+	crtc_commit_atomic(crtc, &plane, req, ATOMIC_RELAX_NONE);
+
+	/* Restore the primary plane and check the state matches the old. */
+	crtc_commit_atomic(crtc, plane_old, req, ATOMIC_RELAX_NONE);
+
+	/* Re-enable the plane through the legacy CRTC/primary-plane API, and
+	 * verify through atomic. */
+	crtc_commit_legacy(crtc, &plane, CRTC_RELAX_MODE);
+
+	/* Restore the plane to its original settings through the legacy CRTC
+	 * API, and verify through atomic. */
+	crtc_commit_legacy(crtc, plane_old, CRTC_RELAX_MODE);
+
+	/* Finally, restore to the original state. */
+	crtc_commit_atomic(crtc, plane_old, req, ATOMIC_RELAX_NONE);
+
+	drmModeAtomicFree(req);
+}
+
+static void plane_cursor(struct kms_atomic_crtc_state *crtc,
+			 struct kms_atomic_plane_state *plane_old)
+{
+	struct drm_mode_modeinfo *mode = crtc->mode.data;
+	struct kms_atomic_plane_state plane = *plane_old;
+	drmModeAtomicReq *req = drmModeAtomicAlloc();
+	struct igt_fb fb;
+	uint64_t width, height;
+
+	igt_assert(req);
+
+	/* Any kernel new enough for atomic, also has the cursor size caps. */
+	do_or_die(drmGetCap(plane.state->desc->fd,
+	                    DRM_CAP_CURSOR_WIDTH, &width));
+	do_or_die(drmGetCap(plane.state->desc->fd,
+	                    DRM_CAP_CURSOR_HEIGHT, &height));
+
+	plane.src_x = 0;
+	plane.src_y = 0;
+	plane.src_w = width << 16;
+	plane.src_h = height << 16;
+	plane.crtc_x = mode->hdisplay / 2;
+	plane.crtc_y = mode->vdisplay / 2;
+	plane.crtc_w = width;
+	plane.crtc_h = height;
+	plane.crtc_id = crtc->obj;
+	plane.fb_id = igt_create_color_fb(plane.state->desc->fd,
+					  width, height,
+					  DRM_FORMAT_ARGB8888,
+					  LOCAL_DRM_FORMAT_MOD_NONE,
+					  0.0, 0.0, 0.0,
+					  &fb);
+	igt_assert_neq_u32(plane.fb_id, 0);
+
+	/* Flip the cursor plane using the atomic API, and double-check
+	 * state is what we think it should be. */
+	plane_commit_atomic(&plane, req, ATOMIC_RELAX_NONE);
+
+	/* Restore the cursor plane and check the state matches the old. */
+	plane_commit_atomic(plane_old, req, ATOMIC_RELAX_NONE);
+
+	/* Re-enable the plane through the legacy cursor API, and verify
+	 * through atomic. */
+	do_or_die(drmModeMoveCursor(plane.state->desc->fd, plane.crtc_id,
+				    plane.crtc_x, plane.crtc_y));
+	do_or_die(drmModeSetCursor(plane.state->desc->fd, plane.crtc_id,
+				   fb.gem_handle, width, height));
+	plane_check_current_state(&plane, PLANE_RELAX_FB);
+
+	/* Wiggle. */
+	plane.crtc_x -= 16;
+	plane.crtc_y -= 16;
+	do_or_die(drmModeMoveCursor(plane.state->desc->fd, plane.crtc_id,
+				    plane.crtc_x, plane.crtc_y));
+	plane_check_current_state(&plane, PLANE_RELAX_FB);
+
+	/* Restore the plane to its original settings through the legacy cursor
+	 * API, and verify through atomic. */
+	do_or_die(drmModeSetCursor2(plane.state->desc->fd, plane.crtc_id,
+				    0, 0, 0, 0, 0));
+	plane_check_current_state(plane_old, ATOMIC_RELAX_NONE);
+
+	/* Finally, restore to the original state. */
+	plane_commit_atomic(plane_old, req, ATOMIC_RELAX_NONE);
+
+	drmModeAtomicFree(req);
+}
+
+static void plane_invalid_params(struct kms_atomic_crtc_state *crtc,
+				 struct kms_atomic_plane_state *plane_old,
+				 struct kms_atomic_connector_state *conn)
+{
+	struct drm_mode_modeinfo *mode = crtc->mode.data;
+	struct kms_atomic_plane_state plane = *plane_old;
+	uint32_t format = plane_get_igt_format(&plane);
+	drmModeAtomicReq *req = drmModeAtomicAlloc();
+	struct igt_fb fb;
+	cairo_t *cr;
+
+	/* Pass a series of invalid object IDs for the FB ID. */
+	plane.fb_id = plane.obj;
+	plane_commit_atomic_err(&plane, plane_old, req,
+	                        ATOMIC_RELAX_NONE, EINVAL);
+
+	plane.fb_id = crtc->obj;
+	plane_commit_atomic_err(&plane, plane_old, req,
+	                        ATOMIC_RELAX_NONE, EINVAL);
+
+	plane.fb_id = conn->obj;
+	plane_commit_atomic_err(&plane, plane_old, req,
+	                        ATOMIC_RELAX_NONE, EINVAL);
+
+	plane.fb_id = crtc->mode.id;
+	plane_commit_atomic_err(&plane, plane_old, req,
+	                        ATOMIC_RELAX_NONE, EINVAL);
+
+	plane.fb_id = plane_old->fb_id;
+	plane_commit_atomic(&plane, req, ATOMIC_RELAX_NONE);
+
+	/* Pass a series of invalid object IDs for the CRTC ID. */
+	plane.crtc_id = plane.obj;
+	plane_commit_atomic_err(&plane, plane_old, req,
+	                        ATOMIC_RELAX_NONE, EINVAL);
+
+	plane.crtc_id = plane.fb_id;
+	plane_commit_atomic_err(&plane, plane_old, req,
+	                        ATOMIC_RELAX_NONE, EINVAL);
+
+	plane.crtc_id = conn->obj;
+	plane_commit_atomic_err(&plane, plane_old, req,
+	                        ATOMIC_RELAX_NONE, EINVAL);
+
+	plane.crtc_id = crtc->mode.id;
+	plane_commit_atomic_err(&plane, plane_old, req,
+	                        ATOMIC_RELAX_NONE, EINVAL);
+
+	plane.crtc_id = plane_old->crtc_id;
+	plane_commit_atomic(&plane, req, ATOMIC_RELAX_NONE);
+
+	/* Create a framebuffer too small for the plane configuration. */
+	igt_require(format != 0);
+
+	plane.src_x = 0;
+	plane.src_y = 0;
+	plane.src_w = mode->hdisplay << 16;
+	plane.src_h = mode->vdisplay << 16;
+	plane.crtc_x = 0;
+	plane.crtc_y = 0;
+	plane.crtc_w = mode->hdisplay;
+	plane.crtc_h = mode->vdisplay;
+	plane.crtc_id = crtc->obj;
+	plane.fb_id = igt_create_fb(plane.state->desc->fd,
+				    plane.crtc_w - 1, plane.crtc_h - 1,
+				    format, I915_TILING_NONE, &fb);
+
+	cr = igt_get_cairo_ctx(plane.state->desc->fd, &fb);
+	igt_paint_test_pattern(cr, plane.crtc_w - 1, plane.crtc_h - 1);
+
+	plane_commit_atomic_err(&plane, plane_old, req,
+	                        ATOMIC_RELAX_NONE, ENOSPC);
+
+	/* Restore the primary plane and check the state matches the old. */
+	plane_commit_atomic(plane_old, req, ATOMIC_RELAX_NONE);
+
+	drmModeAtomicFree(req);
+}
+
+static void crtc_invalid_params(struct kms_atomic_crtc_state *crtc_old,
+				struct kms_atomic_plane_state *plane,
+				struct kms_atomic_connector_state *conn)
+{
+	struct kms_atomic_crtc_state crtc = *crtc_old;
+	drmModeAtomicReq *req = drmModeAtomicAlloc();
+
+	igt_assert(req);
+
+	/* Pass a series of invalid object IDs for the mode ID. */
+	crtc.mode.id = plane->obj;
+	crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req,
+	                       ATOMIC_RELAX_NONE, EINVAL);
+
+	crtc.mode.id = crtc.obj;
+	crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req,
+	                       ATOMIC_RELAX_NONE, EINVAL);
+
+	crtc.mode.id = conn->obj;
+	crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req,
+	                       ATOMIC_RELAX_NONE, EINVAL);
+
+	crtc.mode.id = plane->fb_id;
+	crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req,
+	                       ATOMIC_RELAX_NONE, EINVAL);
+
+	crtc.mode.id = crtc_old->mode.id;
+	crtc_commit_atomic(&crtc, plane, req, ATOMIC_RELAX_NONE);
+
+	/* Create a blob which is the wrong size to be a valid mode. */
+	do_or_die(drmModeCreatePropertyBlob(crtc.state->desc->fd,
+					    crtc.mode.data,
+					    sizeof(struct drm_mode_modeinfo) - 1,
+					    &crtc.mode.id));
+	crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req,
+	                       ATOMIC_RELAX_NONE, EINVAL);
+
+
+	do_or_die(drmModeCreatePropertyBlob(crtc.state->desc->fd,
+					    crtc.mode.data,
+					    sizeof(struct drm_mode_modeinfo) + 1,
+					    &crtc.mode.id));
+	crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req,
+	                       ATOMIC_RELAX_NONE, EINVAL);
+
+	/* Restore the CRTC and check the state matches the old. */
+	crtc_commit_atomic(crtc_old, plane, req, ATOMIC_RELAX_NONE);
+
+	drmModeAtomicFree(req);
+}
+
+/* Abuse the atomic ioctl directly in order to test various invalid conditions,
+ * which the libdrm wrapper won't allow us to create. */
+static void atomic_invalid_params(struct kms_atomic_crtc_state *crtc,
+				  struct kms_atomic_plane_state *plane,
+				  struct kms_atomic_connector_state *connector)
+{
+	struct kms_atomic_desc *desc = crtc->state->desc;
+	struct drm_mode_atomic ioc;
+	uint32_t obj_raw[16]; /* array of objects (sized by count_objs) */
+	uint32_t num_props_raw[16]; /* array of num props per obj (ditto) */
+	uint32_t props_raw[256]; /* array of props (sum of count_props) */
+	uint64_t values_raw[256]; /* array of values for properties (ditto) */
+	int i;
+
+	memset(&ioc, 0, sizeof(ioc));
+
+	/* An empty request should do nothing. */
+	do_ioctl(desc->fd, DRM_IOCTL_MODE_ATOMIC, &ioc);
+
+	for (i = 0; i < ARRAY_SIZE(obj_raw); i++)
+		obj_raw[i] = 0;
+	for (i = 0; i < ARRAY_SIZE(num_props_raw); i++)
+		num_props_raw[i] = 0;
+	for (i = 0; i < ARRAY_SIZE(props_raw); i++)
+		props_raw[i] = 0;
+	for (i = 0; i < ARRAY_SIZE(values_raw); i++)
+		values_raw[i] = 0;
+
+	ioc.objs_ptr = (uintptr_t) obj_raw;
+	ioc.count_props_ptr = (uintptr_t) num_props_raw;
+	ioc.props_ptr = (uintptr_t) props_raw;
+	ioc.prop_values_ptr = (uintptr_t) values_raw;
+
+	/* Valid pointers, but still should copy nothing. */
+	do_ioctl(desc->fd, DRM_IOCTL_MODE_ATOMIC, &ioc);
+
+	/* Nonsense flags. */
+	ioc.flags = 0xdeadbeef;
+	do_ioctl_err(desc->fd, DRM_IOCTL_MODE_ATOMIC, &ioc, EINVAL);
+
+	/* Specifically forbidden combination. */
+	ioc.flags = DRM_MODE_ATOMIC_TEST_ONLY | DRM_MODE_PAGE_FLIP_EVENT;
+	do_ioctl_err(desc->fd, DRM_IOCTL_MODE_ATOMIC, &ioc, EINVAL);
+
+	ioc.flags = 0;
+	/* Safety check that flags is reset properly. */
+	do_ioctl(desc->fd, DRM_IOCTL_MODE_ATOMIC, &ioc);
+
+	/* Reserved/MBZ. */
+	ioc.reserved = 1;
+	do_ioctl_err(desc->fd, DRM_IOCTL_MODE_ATOMIC, &ioc, EINVAL);
+	ioc.reserved = 0;
+	do_ioctl(desc->fd, DRM_IOCTL_MODE_ATOMIC, &ioc);
+
+	/* Zero is not a valid object ID. */
+	ioc.count_objs = ARRAY_SIZE(obj_raw);
+	do_ioctl_err(desc->fd, DRM_IOCTL_MODE_ATOMIC, &ioc, ENOENT);
+
+	/* Invalid object type (not a thing we can set properties on). */
+	ioc.count_objs = 1;
+	obj_raw[0] = crtc->mode.id;
+	do_ioctl_err(desc->fd, DRM_IOCTL_MODE_ATOMIC, &ioc, ENOENT);
+	obj_raw[0] = plane->fb_id;
+	do_ioctl_err(desc->fd, DRM_IOCTL_MODE_ATOMIC, &ioc, ENOENT);
+
+	/* Filled object but with no properties; no-op. */
+	for (i = 0; i < ARRAY_SIZE(obj_raw); i++)
+		obj_raw[i] = crtc->obj;
+	do_ioctl(desc->fd, DRM_IOCTL_MODE_ATOMIC, &ioc);
+
+	/* Pass in all sorts of things other than the property ID. */
+	num_props_raw[0] = 1;
+	do_ioctl_err(desc->fd, DRM_IOCTL_MODE_ATOMIC, &ioc, ENOENT);
+	props_raw[0] = crtc->obj;
+	do_ioctl_err(desc->fd, DRM_IOCTL_MODE_ATOMIC, &ioc, ENOENT);
+	props_raw[0] = plane->obj;
+	do_ioctl_err(desc->fd, DRM_IOCTL_MODE_ATOMIC, &ioc, ENOENT);
+	props_raw[0] = connector->obj;
+	do_ioctl_err(desc->fd, DRM_IOCTL_MODE_ATOMIC, &ioc, ENOENT);
+	props_raw[0] = crtc->mode.id;
+	do_ioctl_err(desc->fd, DRM_IOCTL_MODE_ATOMIC, &ioc, ENOENT);
+
+	/* Valid property, valid value. */
+	for (i = 0; i < ARRAY_SIZE(props_raw); i++) {
+		props_raw[i] = desc->props_crtc[CRTC_MODE_ID];
+		values_raw[i] = crtc->mode.id;
+	}
+	do_ioctl(desc->fd, DRM_IOCTL_MODE_ATOMIC, &ioc);
+
+	/* Setting the same thing multiple times is OK. */
+	for (i = 0; i < ARRAY_SIZE(obj_raw); i++)
+		num_props_raw[i] = ARRAY_SIZE(props_raw) / ARRAY_SIZE(obj_raw);
+	do_ioctl(desc->fd, DRM_IOCTL_MODE_ATOMIC, &ioc);
+	ioc.count_objs = ARRAY_SIZE(obj_raw);
+	do_ioctl(desc->fd, DRM_IOCTL_MODE_ATOMIC, &ioc);
+
+	/* Pass a series of outlandish addresses. */
+	ioc.objs_ptr = 0;
+	do_ioctl_err(desc->fd, DRM_IOCTL_MODE_ATOMIC, &ioc, EFAULT);
+
+	ioc.objs_ptr = (uintptr_t) obj_raw;
+	ioc.count_props_ptr = 0;
+	do_ioctl_err(desc->fd, DRM_IOCTL_MODE_ATOMIC, &ioc, EFAULT);
+
+	ioc.count_props_ptr = (uintptr_t) num_props_raw;
+	ioc.props_ptr = 0;
+	do_ioctl_err(desc->fd, DRM_IOCTL_MODE_ATOMIC, &ioc, EFAULT);
+
+	ioc.props_ptr = (uintptr_t) props_raw;
+	ioc.prop_values_ptr = 0;
+	do_ioctl_err(desc->fd, DRM_IOCTL_MODE_ATOMIC, &ioc, EFAULT);
+
+	ioc.prop_values_ptr = (uintptr_t) values_raw;
+	do_ioctl(desc->fd, DRM_IOCTL_MODE_ATOMIC, &ioc);
+
+	/* Attempt to overflow and/or trip various boundary conditions. */
+	ioc.count_objs = UINT32_MAX / sizeof(uint32_t);
+	do_ioctl_err(desc->fd, DRM_IOCTL_MODE_ATOMIC, &ioc, ENOENT);
+
+	ioc.count_objs = ARRAY_SIZE(obj_raw);
+	ioc.objs_ptr = UINT64_MAX - sizeof(uint32_t);
+	do_ioctl_err(desc->fd, DRM_IOCTL_MODE_ATOMIC, &ioc, EFAULT);
+	ioc.count_objs = 1;
+	ioc.objs_ptr = UINT64_MAX - sizeof(uint32_t);
+	do_ioctl_err(desc->fd, DRM_IOCTL_MODE_ATOMIC, &ioc, EFAULT);
+
+	num_props_raw[0] = UINT32_MAX / sizeof(uint32_t);
+	do_ioctl_err(desc->fd, DRM_IOCTL_MODE_ATOMIC, &ioc, EFAULT);
+	num_props_raw[0] = UINT32_MAX - 1;
+	do_ioctl_err(desc->fd, DRM_IOCTL_MODE_ATOMIC, &ioc, EFAULT);
+
+	for (i = 0; i < ARRAY_SIZE(obj_raw); i++)
+		num_props_raw[i] = (UINT32_MAX / ARRAY_SIZE(obj_raw)) + 1;
+	do_ioctl_err(desc->fd, DRM_IOCTL_MODE_ATOMIC, &ioc, EFAULT);
+	for (i = 0; i < ARRAY_SIZE(obj_raw); i++)
+		num_props_raw[i] = ARRAY_SIZE(props_raw) / ARRAY_SIZE(obj_raw);
+	do_ioctl_err(desc->fd, DRM_IOCTL_MODE_ATOMIC, &ioc, EFAULT);
+}
+
+igt_main
+{
+	struct kms_atomic_desc desc;
+	struct kms_atomic_state current;
+
+	memset(&desc, 0, sizeof(desc));
+	memset(&current, 0, sizeof(current));
+
+	current.desc = &desc;
+
+	igt_fixture
+		atomic_setup(&current);
+
+	igt_subtest("plane_overlay_legacy") {
+		struct kms_atomic_state scratch = current;
+		struct kms_atomic_crtc_state *crtc = find_crtc(&scratch, true);
+		struct kms_atomic_plane_state *plane =
+			find_plane(&scratch, PLANE_TYPE_OVERLAY, crtc);
+
+		igt_require(crtc);
+		igt_require(plane);
+		plane_overlay(crtc, plane);
+	}
+
+	igt_subtest("plane_primary_legacy") {
+		struct kms_atomic_state scratch = current;
+		struct kms_atomic_crtc_state *crtc = find_crtc(&scratch, true);
+		struct kms_atomic_plane_state *plane =
+			find_plane(&scratch, PLANE_TYPE_PRIMARY, crtc);
+
+		igt_require(crtc);
+		igt_require(plane);
+		plane_primary(crtc, plane);
+	}
+
+	igt_subtest("plane_cursor_legacy") {
+		struct kms_atomic_state scratch = current;
+		struct kms_atomic_crtc_state *crtc = find_crtc(&scratch, true);
+		struct kms_atomic_plane_state *plane =
+			find_plane(&scratch, PLANE_TYPE_CURSOR, crtc);
+
+		igt_require(crtc);
+		igt_require(plane);
+		plane_cursor(crtc, plane);
+	}
+
+	igt_subtest("plane_invalid_params") {
+		struct kms_atomic_state scratch = current;
+		struct kms_atomic_crtc_state *crtc = find_crtc(&scratch, true);
+		struct kms_atomic_plane_state *plane =
+			find_plane(&current, PLANE_TYPE_PRIMARY, crtc);
+		struct kms_atomic_connector_state *conn =
+			find_connector(&scratch, crtc);
+
+		igt_require(crtc);
+		igt_require(plane);
+		plane_invalid_params(crtc, plane, conn);
+	}
+
+	igt_subtest("crtc_invalid_params") {
+		struct kms_atomic_state scratch = current;
+		struct kms_atomic_crtc_state *crtc = find_crtc(&scratch, true);
+		struct kms_atomic_plane_state *plane =
+			find_plane(&scratch, NUM_PLANE_TYPE_PROPS, crtc);
+		struct kms_atomic_connector_state *conn =
+			find_connector(&scratch, crtc);
+
+		igt_require(crtc);
+		igt_require(plane);
+		igt_require(conn);
+		crtc_invalid_params(crtc, plane, conn);
+	}
+
+	igt_subtest("atomic_invalid_params") {
+		struct kms_atomic_state scratch = current;
+		struct kms_atomic_crtc_state *crtc = &scratch.crtcs[0];
+		struct kms_atomic_plane_state *plane =
+			find_plane(&scratch, NUM_PLANE_TYPE_PROPS, crtc);
+		struct kms_atomic_connector_state *conn =
+			find_connector(&scratch, crtc);
+
+		igt_require(plane);
+		igt_require(conn);
+		atomic_invalid_params(crtc, plane, conn);
+	}
+
+	igt_fixture
+		close(desc.fd);
+}
-- 
2.5.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH igt v7 4/4] kms_atomic: Add basic atomic modesetting tests

[Emil Velikov]

Hi Dan,

Suspecting that these were meant for the intel-gfx list, although I
doubt people will object seeing them here :-)

On 29 October 2015 at 10:35, Daniel Stone <daniels@collabora.com> wrote:
> Add tests for KMS atomic modesetting, to exercise the basic interface
> and test failure/corner cases. Should ensure coherency between the
> legacy and atomic interfaces.
>
> v2: New patch.
> v3: Disable connector checking for now, as it was causing GPU hangs on
>     newer kernels.
> v4: Rebase.
> v5: Use do_ioctl or do_ioctl_err consistently. Use igt_assert_*()
>     helper macros rather than igt_assert() directly.
>     Move assertions into helper/check functions. Define atomic commit
>     helper.
> v6: Use do_ioctl_err, and define macros to move errors to
>     actual callsite, rather than helper functions.
> v7: Fix RELAX_MODE thinko and refresh CRTC state in find_crtc.
>
> Co-authored-by: Micah Fedke <micah.fedke@collabora.com>
> Signed-off-by: Daniel Stone <daniels@collabora.com>
> ---
>  configure.ac           |    2 +-
>  tests/.gitignore       |    1 +
>  tests/Makefile.sources |    1 +
>  tests/kms_atomic.c     | 1345 ++++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 1348 insertions(+), 1 deletion(-)
>  create mode 100644 tests/kms_atomic.c
>
> diff --git a/configure.ac b/configure.ac
> index 5f97466..97de58a 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -85,7 +85,7 @@ if test "x$GCC" = "xyes"; then
>  fi
>  AC_SUBST(ASSEMBLER_WARN_CFLAGS)
>
> -PKG_CHECK_MODULES(DRM, [libdrm_intel >= 2.4.55 libdrm])
> +PKG_CHECK_MODULES(DRM, [libdrm_intel >= 2.4.64 libdrm])
Afaics you're not using anything intel specific here. I'd suspect one
wants to bump (add) the libdrm version, this way igt can remove a lot
of the local duplication.

[snip]
> --- /dev/null
> +++ b/tests/kms_atomic.c
[snip]
> +#ifndef DRM_CLIENT_CAP_ATOMIC
> +#define DRM_CLIENT_CAP_ATOMIC 3
> +#endif
> +
> +#ifndef DRM_CAP_CURSOR_WIDTH
> +#define DRM_CAP_CURSOR_WIDTH 0x8
> +#endif
> +
> +#ifndef DRM_CAP_CURSOR_HEIGHT
> +#define DRM_CAP_CURSOR_HEIGHT 0x9
> +#endif
> +
> +#ifndef DRM_MODE_ATOMIC_TEST_ONLY
> +#define DRM_MODE_ATOMIC_TEST_ONLY 0x0100
> +#define DRM_MODE_ATOMIC_NONBLOCK 0x0200
> +#define DRM_MODE_ATOMIC_ALLOW_MODESET 0x0400
> +
> +struct drm_mode_atomic {
> +       __u32 flags;
> +       __u32 count_objs;
> +       __u64 objs_ptr;
> +       __u64 count_props_ptr;
> +       __u64 props_ptr;
> +       __u64 prop_values_ptr;
> +       __u64 reserved;
> +       __u64 user_data;
> +};
> +#endif
> +
... like the above but we still need the cursor bits :-(

[snip]
> +#define MAX_PLANES 15
> +#define MAX_CRTCS 3
> +#define MAX_CONNECTORS 8
> +
Worth adding a note where these MAX values come from - intel specific,
educated guess, etc ?

[snip]
> +#if 0
> +/* XXX: Checking this repeatedly actually hangs the GPU. I have literally no
> + *      idea why. */
> +static void
> +connector_check_current_state(struct kms_atomic_connector_state *connector)
Perhaps add a reference to the bugzilla ticket/discussion thread ?

As it goes - just my 2c, fwiw.

Cheers,
Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH igt v7 4/4] kms_atomic: Add basic atomic modesetting tests

[Daniel Vetter]

On Thu, Oct 29, 2015 at 11:33:48AM +0000, Emil Velikov wrote:
> Hi Dan,
> 
> Suspecting that these were meant for the intel-gfx list, although I
> doubt people will object seeing them here :-)
> 
> On 29 October 2015 at 10:35, Daniel Stone <daniels@collabora.com> wrote:
> > Add tests for KMS atomic modesetting, to exercise the basic interface
> > and test failure/corner cases. Should ensure coherency between the
> > legacy and atomic interfaces.
> >
> > v2: New patch.
> > v3: Disable connector checking for now, as it was causing GPU hangs on
> >     newer kernels.
> > v4: Rebase.
> > v5: Use do_ioctl or do_ioctl_err consistently. Use igt_assert_*()
> >     helper macros rather than igt_assert() directly.
> >     Move assertions into helper/check functions. Define atomic commit
> >     helper.
> > v6: Use do_ioctl_err, and define macros to move errors to
> >     actual callsite, rather than helper functions.
> > v7: Fix RELAX_MODE thinko and refresh CRTC state in find_crtc.
> >
> > Co-authored-by: Micah Fedke <micah.fedke@collabora.com>
> > Signed-off-by: Daniel Stone <daniels@collabora.com>
> > ---
> >  configure.ac           |    2 +-
> >  tests/.gitignore       |    1 +
> >  tests/Makefile.sources |    1 +
> >  tests/kms_atomic.c     | 1345 ++++++++++++++++++++++++++++++++++++++++++++++++
> >  4 files changed, 1348 insertions(+), 1 deletion(-)
> >  create mode 100644 tests/kms_atomic.c
> >
> > diff --git a/configure.ac b/configure.ac
> > index 5f97466..97de58a 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -85,7 +85,7 @@ if test "x$GCC" = "xyes"; then
> >  fi
> >  AC_SUBST(ASSEMBLER_WARN_CFLAGS)
> >
> > -PKG_CHECK_MODULES(DRM, [libdrm_intel >= 2.4.55 libdrm])
> > +PKG_CHECK_MODULES(DRM, [libdrm_intel >= 2.4.64 libdrm])
> Afaics you're not using anything intel specific here. I'd suspect one
> wants to bump (add) the libdrm version, this way igt can remove a lot
> of the local duplication.
> 
> [snip]
> > --- /dev/null
> > +++ b/tests/kms_atomic.c
> [snip]
> > +#ifndef DRM_CLIENT_CAP_ATOMIC
> > +#define DRM_CLIENT_CAP_ATOMIC 3
> > +#endif
> > +
> > +#ifndef DRM_CAP_CURSOR_WIDTH
> > +#define DRM_CAP_CURSOR_WIDTH 0x8
> > +#endif
> > +
> > +#ifndef DRM_CAP_CURSOR_HEIGHT
> > +#define DRM_CAP_CURSOR_HEIGHT 0x9
> > +#endif
> > +
> > +#ifndef DRM_MODE_ATOMIC_TEST_ONLY
> > +#define DRM_MODE_ATOMIC_TEST_ONLY 0x0100
> > +#define DRM_MODE_ATOMIC_NONBLOCK 0x0200
> > +#define DRM_MODE_ATOMIC_ALLOW_MODESET 0x0400
> > +
> > +struct drm_mode_atomic {
> > +       __u32 flags;
> > +       __u32 count_objs;
> > +       __u64 objs_ptr;
> > +       __u64 count_props_ptr;
> > +       __u64 props_ptr;
> > +       __u64 prop_values_ptr;
> > +       __u64 reserved;
> > +       __u64 user_data;
> > +};
> > +#endif
> > +
> ... like the above but we still need the cursor bits :-(
> 
> [snip]
> > +#define MAX_PLANES 15
> > +#define MAX_CRTCS 3
> > +#define MAX_CONNECTORS 8
> > +
> Worth adding a note where these MAX values come from - intel specific,
> educated guess, etc ?

They're definitely too low for radeon (has up to 6 crtcs) and iirc other
limits are similarly funky. I guess we could go with kmalloc or just make
the limits something huge like 256 or so.
-Daniel

> 
> [snip]
> > +#if 0
> > +/* XXX: Checking this repeatedly actually hangs the GPU. I have literally no
> > + *      idea why. */
> > +static void
> > +connector_check_current_state(struct kms_atomic_connector_state *connector)
> Perhaps add a reference to the bugzilla ticket/discussion thread ?
> 
> As it goes - just my 2c, fwiw.
> 
> Cheers,
> Emil
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH igt v7 0/4] Assert tests, atomic, multi-blob

[Daniel Stone]

Hi,
Following on from the previous few series, most of which have been
merged ...

Add some self-tests for igt-assert_*() to make sure they do the right
thing, including for fds. This is a bit gross, but does work.

Do a Cocci run through the tree. This doesn't actually pick up a lot
of the changes for some reason, but I have not a clue why.

Add a new subtest to core_prop_blob which creates multiple blobs per
connection, exercising a kernel bug.

Resubmit the atomic tests, with only minor changes this time.

Cheers,
Daniel

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH igt v7 1/4] lib/tests: Add igt_assert_*() self-tests

[Daniel Stone]

Make sure our igt_assert variants are doing something that looks vaguely
like the right thing.

Signed-off-by: Daniel Stone <daniels@collabora.com>
---
 lib/tests/Makefile.sources |   1 +
 lib/tests/igt_simple.c     | 173 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 174 insertions(+)
 create mode 100644 lib/tests/igt_simple.c

diff --git a/lib/tests/Makefile.sources b/lib/tests/Makefile.sources
index 58ae36b..fe5df6e 100644
--- a/lib/tests/Makefile.sources
+++ b/lib/tests/Makefile.sources
@@ -10,6 +10,7 @@ check_PROGRAMS = \
 	igt_timeout \
 	igt_invalid_subtest_name \
 	igt_segfault \
+	igt_simple \
 	$(NULL)
 
 check_SCRIPTS = \
diff --git a/lib/tests/igt_simple.c b/lib/tests/igt_simple.c
new file mode 100644
index 0000000..306b1fb
--- /dev/null
+++ b/lib/tests/igt_simple.c
@@ -0,0 +1,173 @@
+/*
+ * Copyright © 2015 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+#include <assert.h>
+#include <errno.h>
+#include <float.h>
+#include <limits.h>
+#include <math.h>
+#include <signal.h>
+#include <stdint.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+
+#include "igt_core.h"
+
+/*
+ * We need to hide assert from the cocci igt test refactor spatch.
+ *
+ * IMPORTANT: Test infrastructure tests are the only valid places where using
+ * assert is allowed.
+ */
+#define internal_assert assert
+
+char test[] = "test";
+char *argv_run[] = { test };
+void (*test_to_run)(void) = NULL;
+
+/*
+ * A really tedious way of making sure we execute every negative test, and that
+ * they all really fail.
+ */
+#define CHECK_NEG(x) { \
+	igt_subtest_f("XFAIL_simple_%d", __LINE__) { \
+		(*exec_before)++; \
+		x; \
+		raise(SIGBUS); \
+	} \
+	exec_total++; \
+}
+
+static int do_fork(void)
+{
+	int pid, status;
+	int argc;
+
+	switch (pid = fork()) {
+	case -1:
+		internal_assert(0);
+	case 0:
+		argc = 1;
+		igt_simple_init(argc, argv_run);
+		test_to_run();
+		igt_exit();
+	default:
+		while (waitpid(pid, &status, 0) == -1 &&
+		       errno == EINTR)
+			;
+
+		if(WIFSIGNALED(status))
+			return WTERMSIG(status) + 128;
+
+		return WEXITSTATUS(status);
+	}
+}
+
+static void test_cmpint_negative(void)
+{
+	int *exec_before = calloc(1, sizeof(int));
+	int exec_total = 0;
+
+	CHECK_NEG(igt_assert_eq(INT_MIN, INT_MAX));
+
+	CHECK_NEG(igt_assert_eq_u32(0xfffffffeUL, 0xffffffffUL));
+
+	CHECK_NEG(igt_assert_eq_u64(0xfffeffffffffffffULL, 0xffffffffffffffffULL));
+	CHECK_NEG(igt_assert_eq_u64(0xfffffffeffffffffULL, 0xffffffffffffffffULL));
+	CHECK_NEG(igt_assert_eq_u64(0xfffffffffffeffffULL, 0xffffffffffffffffULL));
+
+	CHECK_NEG(igt_assert_eq_double(0.0, DBL_MAX));
+	CHECK_NEG(igt_assert_eq_double(DBL_MAX, nexttoward(DBL_MAX, 0.0)));
+
+	if (*exec_before != exec_total)
+		raise(SIGSEGV);
+}
+
+static void test_cmpint(void)
+{
+	igt_assert_eq(0, 0);
+	igt_assert_eq(INT_MAX, INT_MAX);
+	igt_assert_eq(INT_MAX, INT_MAX);
+	igt_assert_neq(INT_MIN, INT_MAX);
+
+	igt_assert_eq_u32(0, 0);
+	igt_assert_eq_u32(0xffffffffUL, 0xffffffffUL);
+	igt_assert_neq_u32(0xfffffffeUL, 0xffffffffUL);
+
+	igt_assert_eq_u64(0, 0);
+	igt_assert_eq_u64(0xffffffffffffffffULL, 0xffffffffffffffffULL);
+	igt_assert_neq_u64(0xfffffffffffffffeULL, 0xffffffffffffffffULL);
+
+	igt_assert_eq_double(0.0, 0.0);
+	igt_assert_eq_double(DBL_MAX, DBL_MAX);
+	igt_assert_neq_double(0.0, DBL_MAX);
+}
+
+static void test_fd_negative(void)
+{
+	int *exec_before = calloc(1, sizeof(int));
+	int exec_total = 0;
+
+	CHECK_NEG(igt_assert_fd(-1));
+	CHECK_NEG(igt_assert_fd(INT_MIN));
+
+	if (*exec_before != exec_total)
+		raise(SIGSEGV);
+}
+
+static void test_fd(void)
+{
+	igt_assert_fd(0);
+	igt_assert_fd(1);
+	igt_assert_fd(INT_MAX);
+}
+
+igt_main
+{
+	int ret;
+
+	igt_subtest("igt_cmpint")
+		test_cmpint();
+
+	/*
+	 * The awkward subtest dance here is because we really want to use
+	 * subtests in our negative tests, to ensure we actually execute all
+	 * the subtests. But we can't begin a subtest within a subtest, and
+	 * we inherit the state from the parent, so ...
+	 */
+	test_to_run = test_cmpint_negative;
+	ret = do_fork();
+	igt_subtest("igt_cmpint_negative")
+		internal_assert(ret == IGT_EXIT_FAILURE);
+
+	igt_subtest("igt_assert_fd")
+		test_fd();
+
+	test_to_run = test_fd_negative;
+	ret = do_fork();
+	igt_subtest("igt_assert_fd_negative")
+		internal_assert(ret == IGT_EXIT_FAILURE);
+}
-- 
2.5.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel