On Thu, Jul 16, 2015 at 11:06:59AM +0200, Michał Winiarski wrote: > When reading the timestamp register with single 64b read, we're observing > invalid values on x86_64: > > [f = valid counter value | X = garbage] > > i386: 0x0000000fffffffff > x86_64: 0xffffffffXXXXXXXX > > Test checks if the counter is moving and increasing. > Add a check to see if we can use (reg | 1) flag to get a proper 36b timestamp, > shifting the value on x86_64 if we can't. > > Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > Signed-off-by: Michał Winiarski <michal.winiarski@xxxxxxxxx> > --- > +static bool check_timestamp(int fd) > +{ > + int ret; > + uint64_t val; > + > + ret = read_register(fd, RENDER_RING_TIMESTAMP | 1, &val); > + if (ret != 0 && errno == EINVAL) > + return false; I would have said return ret == 0; would be safer rather than only returning false if it matched the expected error. > + return true; > +} > + > +static uint64_t timer_query(int fd, uint64_t * val) > +{ > + uint64_t offset; > + int ret; > + > + offset = RENDER_RING_TIMESTAMP; > + if (has_proper_timestamp) > + offset |= 1; > + > + ret = read_register(fd, offset, val); > + /* * When reading the timestamp register with single 64b read, we're observing * invalid values on x86_64: * * [f = valid counter value | X = garbage] * * i386: 0x0000000fffffffff * x86_64: 0xffffffffXXXXXXXX * * In the absence of a corrected register read ioctl, attempt * to fix up the return value to be vaguely useful. */ > + if (is_x86_64 && !has_proper_timestamp) > + *val >>= 32; > > - reg_read.val = timer_query(fd); > + return ret; > +} > + > +static void test_timestamp_monotonic(int fd) > +{ > + uint64_t first_val, second_val; > + bool retry = true; > > - igt_assert(ret != 0 && errno == EINVAL); > + igt_fail_on(timer_query(fd, &first_val) != 0); > > +retry: > + sleep(1); > + igt_fail_on(timer_query(fd, &second_val) != 0); > + if (second_val <= first_val && retry) { > + retry = false; > + first_val = second_val; > + goto retry; > + } Hmm, I expected a few more iterations for a monotonic test. Something like: start = gettime(); first_val = timer_query(fd); do { second_val = timer_query(fd); igt_assert_gte(second_val, first_val); first_val = second_val; elapsed = getreltime(&start); } while (elapsed < 5); > + > + igt_assert(second_val > first_val); > +} > + > +igt_main > +{ > + uint64_t val = 0; > + int fd = -1; > + > + fd = drm_open_any(); > + is_x86_64 = check_kernel_x86_64(); > + has_proper_timestamp = check_timestamp(fd); > close(fd); Just do this in the fixture I guess. > + > + igt_fixture { > + fd = drm_open_any(); > + } -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx