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 <ch...@chris-wilson.co.uk>
> Signed-off-by: Michał Winiarski <michal.winiar...@intel.com>
> ---

> +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@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to