tests: introduce igt_require

Since igt_skip has funny control flow we can abuse it and make it work
like a special kind of assert which automatically skips tests if a
requirement fails.

Note that in places where we have a less strict test which should
always succeed (e.g. ioctl works or isn't available) the igt_assert
should be place before the igt_require with the more strict
requirements. Otherwise we'll skip a test instead of properly failing
it.

Convert a few users of igt_skip over to igt_require to showcase its
use.

v2: s/gem_check_/gem_require_/ so that we consistently use "require"
to indicate magic check that can call igt_skip. Imo hiding the
igt_require for feature checks is ok, but for more traditional assert
like use cases an explicit igt_require might be better.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

diff --git a/lib/drmtest.c b/lib/drmtest.c
index e601edd..d0e463e 100644 (file)
--- a/lib/drmtest.c
+++ b/lib/drmtest.c
@@ -393,7 +393,7 @@ struct local_drm_i915_gem_caching {
 #define LOCAL_DRM_IOCTL_I915_GEM_GET_CACHEING \
        DRM_IOWR(DRM_COMMAND_BASE + LOCAL_DRM_I915_GEM_GET_CACHEING, struct local_drm_i915_gem_caching)
 
-void gem_check_caching(int fd)
+void gem_require_caching(int fd)
 {
        struct local_drm_i915_gem_caching arg;
        int ret;
@@ -405,11 +405,7 @@ void gem_check_caching(int fd)
        ret = ioctl(fd, LOCAL_DRM_IOCTL_I915_GEM_SET_CACHEING, &arg);
        gem_close(fd, arg.handle);
 
-       if (ret != 0) {
-               if (!igt_only_list_subtests())
-                       printf("no set_caching support detected\n");
-               igt_skip();
-       }
+       igt_require(ret == 0);
 }
 
 void gem_set_caching(int fd, uint32_t handle, int caching)
@@ -421,10 +417,8 @@ void gem_set_caching(int fd, uint32_t handle, int caching)
        arg.caching = caching;
        ret = ioctl(fd, LOCAL_DRM_IOCTL_I915_GEM_SET_CACHEING, &arg);
 
-       if (ret != 0 && (errno == ENOTTY || errno == EINVAL))
-               igt_skip();
-       else
-               igt_assert(ret == 0);
+       igt_assert(ret == 0 || (errno == ENOTTY || errno == EINVAL));
+       igt_require(ret == 0);
 }
 
 uint32_t gem_get_caching(int fd, uint32_t handle)
@@ -770,6 +764,15 @@ void igt_skip(void)
                exit(77);
 }
 
+void __igt_skip_check(const char *file, const int line,
+                     const char *func, const char *check)
+{
+       printf("Test requirement not met in function %s, file %s:%i:\n"
+              "Test requirement: (%s)\n",
+              func, file, line, check);
+       igt_skip();
+}
+
 void igt_success(void)
 {
        succeeded_one = true;
@@ -847,8 +850,7 @@ void igt_skip_on_simulation(void)
        if (igt_only_list_subtests())
                return;
 
-       if (igt_run_in_simulation())
-               igt_skip();
+       igt_require(!igt_run_in_simulation());
 }
 
 /* other helpers */

diff --git a/lib/drmtest.h b/lib/drmtest.h
index 92fa7a2..d9a11e1 100644 (file)
--- a/lib/drmtest.h
+++ b/lib/drmtest.h
@@ -36,6 +36,7 @@
 
 #include "xf86drm.h"
 #include "xf86drmMode.h"
+#include "i915_drm.h"
 #include "intel_batchbuffer.h"
 #include "intel_chipset.h"
 #include "intel_gpu_tools.h"
@@ -104,41 +105,48 @@ bool __igt_run_subtest(const char *subtest_name);
                                   igt_success())
 bool igt_only_list_subtests(void);
 void igt_skip(void);
+void __igt_skip_check(const char *file, const int line,
+                     const char *func, const char *check);
 void igt_success(void);
 void igt_fail(int exitcode) __attribute__((noreturn));
 void __igt_fail_assert(int exitcode, const char *file,
                       const int line, const char *func, const char *assertion)
        __attribute__((noreturn));
 void igt_exit(void);
+/**
+ * igt_assert - fails (sub-)test if a condition is not met
+ *
+ * Should be used everywhere where a test checks results.
+ */
 #define igt_assert(expr) do { if (!(expr)) __igt_fail_assert(99, __FILE__, __LINE__, __func__, #expr ); } while (0)
+/**
+ * igt_require - skip a (sub-)test if a condition is not met
+ *
+ * This is useful to streamline the skip logic since it allows for a more flat
+ * code control flow.
+ */
+#define igt_require(expr) do { if (!(expr)) __igt_skip_check(__FILE__, __LINE__, __func__, #expr ); } while (0)
 
 /* check functions which auto-skip tests by calling igt_skip() */
-void gem_check_caching(int fd);
-static inline bool gem_check_vebox(int fd)
-{
-       if (gem_has_vebox(fd))
-               return true;
-
-       igt_skip();
-       return false;
-}
-
-static inline bool gem_check_bsd(int fd)
-{
-       if (HAS_BSD_RING(intel_get_drm_devid(fd)))
-               return true;
-
-       igt_skip();
-       return false;
-}
-
-static inline bool gem_check_blt(int fd)
+void gem_require_caching(int fd);
+static inline void gem_require_ring(int fd, int ring_id)
 {
-       if (HAS_BLT_RING(intel_get_drm_devid(fd)))
-               return true;
-
-       igt_skip();
-       return false;
+       switch (ring_id) {
+       case I915_EXEC_RENDER:
+               return;
+       case I915_EXEC_BLT:
+               igt_require(HAS_BLT_RING(intel_get_drm_devid(fd)));
+               return;
+       case I915_EXEC_BSD:
+               igt_require(HAS_BSD_RING(intel_get_drm_devid(fd)));
+               return;
+       case I915_EXEC_VEBOX:
+               igt_require(gem_has_vebox(fd));
+               return;
+       default:
+               assert(0);
+               return;
+       }
 }
 
 /* helpers to automatically reduce test runtime in simulation */

diff --git a/tests/gem_caching.c b/tests/gem_caching.c
index 52931c5..6af76fa 100644 (file)
--- a/tests/gem_caching.c
+++ b/tests/gem_caching.c
@@ -117,7 +117,7 @@ int main(int argc, char **argv)
 
        fd = drm_open_any();
 
-       gem_check_caching(fd);
+       gem_require_caching(fd);
 
        devid = intel_get_drm_devid(fd);
        if (IS_GEN2(devid)) /* chipset only handles cached -> uncached */

diff --git a/tests/gem_cs_tlb.c b/tests/gem_cs_tlb.c
index 2f8037a..d4fbabc 100644 (file)
--- a/tests/gem_cs_tlb.c
+++ b/tests/gem_cs_tlb.c
@@ -101,6 +101,8 @@ static void run_on_ring(int fd, unsigned ring_id, const char *ring_name)
        char buf[100];
        int i;
 
+       gem_require_ring(fd, ring_id);
+
        sprintf(buf, "testing %s cs tlb coherency: ", ring_name);
 
        /* Shut up gcc, too stupid. */
@@ -163,16 +165,13 @@ int main(int argc, char **argv)
                run_on_ring(fd, I915_EXEC_RENDER, "render");
 
        igt_subtest("bsd")
-               if (gem_check_bsd(fd))
-                       run_on_ring(fd, I915_EXEC_BSD, "bsd");
+               run_on_ring(fd, I915_EXEC_BSD, "bsd");
 
        igt_subtest("blt")
-               if (gem_check_blt(fd))
-                       run_on_ring(fd, I915_EXEC_BLT, "blt");
+               run_on_ring(fd, I915_EXEC_BLT, "blt");
 
        igt_subtest("vebox")
-               if (gem_check_vebox(fd))
-                       run_on_ring(fd, LOCAL_I915_EXEC_VEBOX, "vebox");
+               run_on_ring(fd, LOCAL_I915_EXEC_VEBOX, "vebox");
 
        close(fd);
 

diff --git a/tests/gem_dummy_reloc_loop.c b/tests/gem_dummy_reloc_loop.c
index de292c7..bccd791 100644 (file)
--- a/tests/gem_dummy_reloc_loop.c
+++ b/tests/gem_dummy_reloc_loop.c
@@ -166,30 +166,27 @@ int main(int argc, char **argv)
        }
 
        igt_subtest("bsd") {
-               if (gem_check_bsd(fd)) {
-                       sleep(2);
-                       printf("running dummy loop on bsd\n");
-                       dummy_reloc_loop(I915_EXEC_BSD);
-                       printf("dummy loop run on bsd completed\n");
-               }
+               gem_require_ring(fd, I915_EXEC_BSD);
+               sleep(2);
+               printf("running dummy loop on bsd\n");
+               dummy_reloc_loop(I915_EXEC_BSD);
+               printf("dummy loop run on bsd completed\n");
        }
 
        igt_subtest("blt") {
-               if (gem_check_blt(fd)) {
-                       sleep(2);
-                       printf("running dummy loop on blt\n");
-                       dummy_reloc_loop(I915_EXEC_BLT);
-                       printf("dummy loop run on blt completed\n");
-               }
+               gem_require_ring(fd, I915_EXEC_BLT);
+               sleep(2);
+               printf("running dummy loop on blt\n");
+               dummy_reloc_loop(I915_EXEC_BLT);
+               printf("dummy loop run on blt completed\n");
        }
 
        igt_subtest("vebox") {
-               if (gem_check_vebox(fd)) {
-                       sleep(2);
-                       printf("running dummy loop on vebox\n");
-                       dummy_reloc_loop(LOCAL_I915_EXEC_VEBOX);
-                       printf("dummy loop run on vebox completed\n");
-               }
+               gem_require_ring(fd, I915_EXEC_VEBOX);
+               sleep(2);
+               printf("running dummy loop on vebox\n");
+               dummy_reloc_loop(LOCAL_I915_EXEC_VEBOX);
+               printf("dummy loop run on vebox completed\n");
        }
 
        igt_subtest("mixed") {

diff --git a/tests/gem_exec_nop.c b/tests/gem_exec_nop.c
index 4cb157c..47bcf10 100644 (file)
--- a/tests/gem_exec_nop.c
+++ b/tests/gem_exec_nop.c
@@ -92,6 +92,8 @@ static void loop(int fd, uint32_t handle, unsigned ring_id, const char *ring_nam
 {
        int count;
 
+       gem_require_ring(fd, ring_id);
+
        for (count = 1; count <= SLOW_QUICK(1<<17, 1<<4); count <<= 1) {
                struct timeval start, end;
 
@@ -103,8 +105,8 @@ static void loop(int fd, uint32_t handle, unsigned ring_id, const char *ring_nam
                       count, elapsed(&start, &end, count), ring_name);
                fflush(stdout);
        }
-
 }
+
 int main(int argc, char **argv)
 {
        uint32_t batch[2] = {MI_BATCH_BUFFER_END};
@@ -122,16 +124,13 @@ int main(int argc, char **argv)
                loop(fd, handle, I915_EXEC_RENDER, "render");
 
        igt_subtest("bsd")
-               if (gem_check_blt(fd))
-                       loop(fd, handle, I915_EXEC_BSD, "bsd");
+               loop(fd, handle, I915_EXEC_BSD, "bsd");
 
        igt_subtest("blt")
-               if (gem_check_blt(fd))
-                       loop(fd, handle, I915_EXEC_BLT, "blt");
+               loop(fd, handle, I915_EXEC_BLT, "blt");
 
        igt_subtest("vebox")
-               if (gem_check_vebox(fd))
-                       loop(fd, handle, LOCAL_I915_EXEC_VEBOX, "vebox");
+               loop(fd, handle, LOCAL_I915_EXEC_VEBOX, "vebox");
 
        gem_close(fd, handle);
 

diff --git a/tests/gem_reg_read.c b/tests/gem_reg_read.c
index 39ebfd8..67c1d91 100644 (file)
--- a/tests/gem_reg_read.c
+++ b/tests/gem_reg_read.c
@@ -66,16 +66,14 @@ static uint64_t timer_query(int fd)
 int main(int argc, char *argv[])
 {
        struct local_drm_i915_reg_read reg_read;
-       int fd;
+       int fd, ret;
 
        fd = drm_open_any();
 
        reg_read.offset = 0x2358;
-       if (drmIoctl(fd, REG_READ_IOCTL, &reg_read)) {
-               if (errno == EINVAL)
-                       igt_skip();
-               igt_fail(1);
-       }
+       ret = drmIoctl(fd, REG_READ_IOCTL, &reg_read);
+       igt_assert(ret == 0 || errno == EINVAL);
+       igt_require(ret == 0);
 
        reg_read.val = timer_query(fd);
        sleep(1);

diff --git a/tests/gem_write_read_ring_switch.c b/tests/gem_write_read_ring_switch.c
index d8b6702..b9db8f2 100644 (file)
--- a/tests/gem_write_read_ring_switch.c
+++ b/tests/gem_write_read_ring_switch.c
@@ -146,20 +146,6 @@ static void run_test(int ring, const char *testname)
        drm_intel_bo_unreference(target_bo);
 }
 
-static int has_ring(int ring)
-{
-       switch (ring) {
-       case I915_EXEC_RENDER: /* test only makes sense with separate blitter */
-               return gem_check_blt(fd);
-       case I915_EXEC_BSD:
-               return gem_check_bsd(fd);
-       case LOCAL_I915_EXEC_VEBOX:
-               return gem_check_vebox(fd);
-       default:
-               return 0;
-       }
-}
-
 int main(int argc, char **argv)
 {
        static const struct {
@@ -206,8 +192,11 @@ int main(int argc, char **argv)
 
        for (i = 0; i < ARRAY_SIZE(tests); i++) {
                igt_subtest(tests[i].name) {
-                       if (has_ring(tests[i].ring))
-                               run_test(tests[i].ring, tests[i].name);
+                       gem_require_ring(fd, tests[i].ring);
+                       /* Testing render only makes sense with separate blt. */
+                       if (tests[i].ring == I915_EXEC_RENDER)
+                               gem_require_ring(fd, I915_EXEC_BLT);
+                       run_test(tests[i].ring, tests[i].name);
                }
        }