From 8f5387eb99b640771812d28a0b949aca729bf296 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Tue, 13 Aug 2013 13:20:58 +0200 Subject: [PATCH] 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 --- lib/drmtest.c | 26 +++++++++-------- lib/drmtest.h | 58 ++++++++++++++++++++++---------------- tests/gem_caching.c | 2 +- tests/gem_cs_tlb.c | 11 ++++---- tests/gem_dummy_reloc_loop.c | 33 ++++++++++------------ tests/gem_exec_nop.c | 13 ++++----- tests/gem_reg_read.c | 10 +++---- tests/gem_write_read_ring_switch.c | 21 ++++---------- 8 files changed, 83 insertions(+), 91 deletions(-) diff --git a/lib/drmtest.c b/lib/drmtest.c index e601edd..d0e463e 100644 --- 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 --- 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 --- 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 --- 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 --- 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 --- 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 --- 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, ®_read)) { - if (errno == EINVAL) - igt_skip(); - igt_fail(1); - } + ret = drmIoctl(fd, REG_READ_IOCTL, ®_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 --- 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); } } -- 2.7.4