On 11/21/2016 01:33 PM, Tomeu Vizoso wrote: > On 21 November 2016 at 10:25, Abdiel Janulgue > <abdiel.janulgue@xxxxxxxxxxxxxxx> wrote: >> v7: Adapt to api rename >> v8: Restore sanitycheck wait on the recursive batch and >> avoid using C99 locals (Chris Wilson) >> >> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> >> Cc: Daniel Vetter <daniel.vetter@xxxxxxxx> >> Signed-off-by: Abdiel Janulgue <abdiel.janulgue@xxxxxxxxxxxxxxx> >> --- >> tests/gem_wait.c | 128 ++++--------------------------------------------------- > > Hi, > > I'm seeing failures on i5-5200U that I don't see with this patch > reverted. They all look like this: > > (gem_wait:19238) CRITICAL: Test assertion failure function basic, file > ../../intel-gpu-tools/tests/gem_wait.c:80: > (gem_wait:19238) CRITICAL: Failed assertion: __gem_wait(fd, &wait) == -62 > (gem_wait:19238) CRITICAL: Last errno: 62, Timer expired > (gem_wait:19238) CRITICAL: error: 0 != -62 > Stack trace: > #0 [__igt_fail_assert+0xf1] > #1 [basic+0x214] > #2 [<unknown>+0x214] > Subtest busy-vebox failed. > **** DEBUG **** > (gem_wait:19238) ioctl-wrappers-DEBUG: Test requirement passed: > gem_has_ring(fd, ring) > (gem_wait:19238) igt-dummyload-DEBUG: Test requirement passed: nengine > (gem_wait:19238) CRITICAL: Test assertion failure function basic, file > ../../intel-gpu-tools/tests/gem_wait.c:80: > (gem_wait:19238) CRITICAL: Failed assertion: __gem_wait(fd, &wait) == -62 > (gem_wait:19238) CRITICAL: Last errno: 62, Timer expired > (gem_wait:19238) CRITICAL: error: 0 != -62 > **** END **** > Subtest busy-vebox: FAIL (0.003s) > > CI seems to have gained support for running BAT on IGT changes, but > that doesn't seem to have been wired to patchwork yet. Maybe you could > arrange for a CI run using a branch in your IGT tree? So this series > are tested on a wider array of HW. Thanks for catching this again. Just tested the previous revision on our CI farm last week. Unfortunately, we have limited bandwidth at the moment of sending every update manually. Hopefully when we do gain BAT on IGT change, this will mitigate the problem a bit. Anyway, please test the following patch. That should hopefully fix the wait issue. - Abdiel > > Regards, > > Tomeu > >> 1 file changed, 8 insertions(+), 120 deletions(-) >> >> diff --git a/tests/gem_wait.c b/tests/gem_wait.c >> index b4127de..6332e76 100644 >> --- a/tests/gem_wait.c >> +++ b/tests/gem_wait.c >> @@ -27,18 +27,6 @@ >> >> #include "igt.h" >> >> -#include <signal.h> >> -#include <time.h> >> -#include <sys/syscall.h> >> - >> -#define gettid() syscall(__NR_gettid) >> -#define sigev_notify_thread_id _sigev_un._tid >> - >> -#define LOCAL_I915_EXEC_BSD_SHIFT (13) >> -#define LOCAL_I915_EXEC_BSD_MASK (3 << LOCAL_I915_EXEC_BSD_SHIFT) >> - >> -#define ENGINE_MASK (I915_EXEC_RING_MASK | LOCAL_I915_EXEC_BSD_MASK) >> - >> static int __gem_wait(int fd, struct drm_i915_gem_wait *w) >> { >> int err; >> @@ -75,129 +63,33 @@ static void invalid_buf(int fd) >> igt_assert_eq(__gem_wait(fd, &wait), -ENOENT); >> } >> >> -static uint32_t *batch; >> - >> -static void sigiter(int sig, siginfo_t *info, void *arg) >> -{ >> - *batch = MI_BATCH_BUFFER_END; >> - __sync_synchronize(); >> -} >> - >> -#define MSEC_PER_SEC (1000) >> -#define USEC_PER_SEC (1000 * MSEC_PER_SEC) >> -#define NSEC_PER_SEC (1000 * USEC_PER_SEC) >> - >> #define BUSY 1 >> #define HANG 2 >> static void basic(int fd, unsigned engine, unsigned flags) >> { >> - const int gen = intel_gen(intel_get_drm_devid(fd)); >> - struct drm_i915_gem_exec_object2 obj; >> - struct drm_i915_gem_relocation_entry reloc; >> - struct drm_i915_gem_execbuffer2 execbuf; >> struct drm_i915_gem_wait wait; >> - unsigned engines[16]; >> - unsigned nengine; >> - int i, timeout; >> - >> - nengine = 0; >> - if (engine == -1) { >> - for_each_engine(fd, engine) >> - if (engine) engines[nengine++] = engine; >> - } else { >> - igt_require(gem_has_ring(fd, engine)); >> - engines[nengine++] = engine; >> - } >> - igt_require(nengine); >> - >> - memset(&execbuf, 0, sizeof(execbuf)); >> - execbuf.buffers_ptr = (uintptr_t)&obj; >> - execbuf.buffer_count = 1; >> - >> - memset(&obj, 0, sizeof(obj)); >> - obj.handle = gem_create(fd, 4096); >> - >> - obj.relocs_ptr = (uintptr_t)&reloc; >> - obj.relocation_count = 1; >> - memset(&reloc, 0, sizeof(reloc)); >> - >> - batch = gem_mmap__gtt(fd, obj.handle, 4096, PROT_WRITE); >> - gem_set_domain(fd, obj.handle, >> - I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT); >> - >> - reloc.target_handle = obj.handle; /* recurse */ >> - reloc.presumed_offset = 0; >> - reloc.offset = sizeof(uint32_t); >> - reloc.delta = 0; >> - reloc.read_domains = I915_GEM_DOMAIN_COMMAND; >> - reloc.write_domain = 0; >> - >> - i = 0; >> - batch[i] = MI_BATCH_BUFFER_START; >> - if (gen >= 8) { >> - batch[i] |= 1 << 8 | 1; >> - batch[++i] = 0; >> - batch[++i] = 0; >> - } else if (gen >= 6) { >> - batch[i] |= 1 << 8; >> - batch[++i] = 0; >> - } else { >> - batch[i] |= 2 << 6; >> - batch[++i] = 0; >> - if (gen < 4) { >> - batch[i] |= 1; >> - reloc.delta = 1; >> - } >> - } >> + igt_spin_t *spin; >> + int timeout; >> + int wait_s = (flags == 0) ? NSEC_PER_SEC : 0; >> >> - for (i = 0; i < nengine; i++) { >> - execbuf.flags &= ~ENGINE_MASK; >> - execbuf.flags |= engines[i]; >> - gem_execbuf(fd, &execbuf); >> - } >> + wait_s = ((flags & HANG) == 0) ? wait_s : -1; >> + spin = igt_spin_batch_new(fd, wait_s, engine, 0); >> >> memset(&wait, 0, sizeof(wait)); >> - wait.bo_handle = obj.handle; >> + wait.bo_handle = spin->handle; >> igt_assert_eq(__gem_wait(fd, &wait), -ETIME); >> >> if (flags & BUSY) { >> struct timespec tv; >> >> timeout = 120; >> - if ((flags & HANG) == 0) { >> - *batch = MI_BATCH_BUFFER_END; >> - __sync_synchronize(); >> + if ((flags & HANG) == 0) >> timeout = 1; >> - } >> >> memset(&tv, 0, sizeof(tv)); >> while (__gem_wait(fd, &wait) == -ETIME) >> igt_assert(igt_seconds_elapsed(&tv) < timeout); >> } else { >> - timer_t timer; >> - >> - if ((flags & HANG) == 0) { >> - struct sigevent sev; >> - struct sigaction act; >> - struct itimerspec its; >> - >> - memset(&sev, 0, sizeof(sev)); >> - sev.sigev_notify = SIGEV_SIGNAL | SIGEV_THREAD_ID; >> - sev.sigev_notify_thread_id = gettid(); >> - sev.sigev_signo = SIGRTMIN + 1; >> - igt_assert(timer_create(CLOCK_MONOTONIC, &sev, &timer) == 0); >> - >> - memset(&act, 0, sizeof(act)); >> - act.sa_sigaction = sigiter; >> - act.sa_flags = SA_SIGINFO; >> - igt_assert(sigaction(SIGRTMIN + 1, &act, NULL) == 0); >> - >> - memset(&its, 0, sizeof(its)); >> - its.it_value.tv_nsec = 0; >> - its.it_value.tv_sec = 1; >> - igt_assert(timer_settime(timer, 0, &its, NULL) == 0); >> - } >> - >> wait.timeout_ns = NSEC_PER_SEC / 2; /* 0.5s */ >> igt_assert_eq(__gem_wait(fd, &wait), -ETIME); >> igt_assert_eq_s64(wait.timeout_ns, 0); >> @@ -215,13 +107,9 @@ static void basic(int fd, unsigned engine, unsigned flags) >> wait.timeout_ns = 0; >> igt_assert_eq(__gem_wait(fd, &wait), 0); >> igt_assert(wait.timeout_ns == 0); >> - >> - if ((flags & HANG) == 0) >> - timer_delete(timer); >> } >> >> - gem_close(fd, obj.handle); >> - munmap(batch, 4096); >> + igt_spin_batch_free(fd, spin); >> } >> >> igt_main >> -- >> 2.7.4 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx > _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx