On Thu, Nov 22, 2012 at 03:25:06PM +0200, Imre Deak wrote:
> Since monotonic timestamps are now the preferred time format, change
> timestamps checks to compare against monotonic instead of real time.
> Also add two tests for DRM's compatibility mode where it returns real
> timestamps.
> 
> Signed-off-by: Imre Deak <[email protected]>

A few comments below.
-Daniel

> ---
>  tests/flip_test.c |   49 ++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 48 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/flip_test.c b/tests/flip_test.c
> index d88f81c..258d727 100644
> --- a/tests/flip_test.c
> +++ b/tests/flip_test.c
> @@ -53,6 +53,7 @@
>  #define TEST_VBLANK_BLOCK    (1 << 9)
>  #define TEST_VBLANK_ABSOLUTE (1 << 10)
>  #define TEST_VBLANK_EXPIRED_SEQ      (1 << 11)
> +#define TEST_TIMESTAMP_REAL  (1 << 12)
>  
>  #define EVENT_FLIP           (1 << 0)
>  #define EVENT_VBLANK         (1 << 1)
> @@ -63,6 +64,7 @@ static drm_intel_bufmgr *bufmgr;
>  struct intel_batchbuffer *batch;
>  uint32_t devid;
>  int test_time = 3;
> +static bool monotonic_timestamp;
>  
>  uint32_t *fb_ptr;
>  
> @@ -296,7 +298,19 @@ analog_tv_connector(struct test_output *o)
>  static void event_handler(struct event_state *es, unsigned int frame,
>                         unsigned int sec, unsigned int usec)
>  {
> -     gettimeofday(&es->current_received_ts, NULL);
> +     struct timeval now;
> +
> +     if (monotonic_timestamp) {
> +             struct timespec ts;
> +
> +             clock_gettime(CLOCK_MONOTONIC, &ts);
> +             now.tv_sec = ts.tv_sec;
> +             now.tv_usec = ts.tv_nsec / 1000;
> +     } else {
> +             gettimeofday(&now, NULL);
> +     }
> +     es->current_received_ts = now;
> +
>       es->current_ts.tv_sec = sec;
>       es->current_ts.tv_usec = usec;
>       es->current_seq = frame;
> @@ -899,6 +913,28 @@ static int get_pipe_from_crtc_id(int crtc_id)
>       return pfci.pipe;
>  }
>  
> +static void enable_monotonic_timestamp(bool enable)
> +{
> +     static const char *sysfs_mono_path =
> +             "/sys/module/drm/parameters/timestamp_monotonic";
> +     int mono_ts_enabled;
> +     int scanned;
> +     FILE *f;
> +
> +     f = fopen(sysfs_mono_path, "r+");
> +     assert(f);
> +
> +     scanned = fscanf(f, "%d", &mono_ts_enabled);
> +     assert(scanned == 1 && (mono_ts_enabled == 1 || mono_ts_enabled == 0));
> +
> +     if (mono_ts_enabled != enable)
> +             fprintf(f, "%d", enable);

I think it'd be better to use the drm getcap ioctl here (and default to
disabled if it returns -EINVAL), since that's the officially blessed
interface to figure this out.


> +
> +     fclose(f);
> +
> +     monotonic_timestamp = enable;
> +}
> +
>  static int run_test(int duration, int flags, const char *test_name)
>  {
>       struct test_output o;
> @@ -911,6 +947,8 @@ static int run_test(int duration, int flags, const char 
> *test_name)
>               exit(5);
>       }
>  
> +     enable_monotonic_timestamp(!(flags & TEST_TIMESTAMP_REAL));

I don't follow why we should enable monotonic timestamps for some tests
and not for some others? Shouldn't all tests with TEST_CHECK_TS just use
the same clock the kernel uses?

> +
>       /* Find any connected displays */
>       for (c = 0; c < resources->count_connectors; c++) {
>               for (i = 0; i < resources->count_crtcs; i++) {
> @@ -941,6 +979,8 @@ int main(int argc, char **argv)
>               const char *name;
>       } tests[] = {
>               { 15, TEST_VBLANK | TEST_CHECK_TS, "wf-vblank" },
> +             { 15, TEST_VBLANK | TEST_TIMESTAMP_REAL | TEST_CHECK_TS,
> +                                     "wf-vblank with real timestamps" },
>               { 15, TEST_VBLANK | TEST_VBLANK_BLOCK | TEST_CHECK_TS,
>                                       "blocking wf-vblank" },
>               { 5,  TEST_VBLANK | TEST_VBLANK_ABSOLUTE,
> @@ -955,6 +995,8 @@ int main(int argc, char **argv)
>                                       "delayed wf-vblank vs modeset" },
>  
>               { 15, TEST_FLIP | TEST_CHECK_TS | TEST_EBUSY , "plain flip" },
> +             { 5,  TEST_FLIP | TEST_TIMESTAMP_REAL | TEST_CHECK_TS,
> +                                     "flip with real timestamps" },
>               { 30, TEST_FLIP | TEST_DPMS | TEST_EINVAL, "flip vs dpms" },
>               { 30, TEST_FLIP | TEST_DPMS | TEST_WITH_DUMMY_LOAD, "delayed 
> flip vs dpms" },
>               { 5,  TEST_FLIP | TEST_PAN, "flip vs panning" },
> @@ -973,6 +1015,11 @@ int main(int argc, char **argv)
>       };
>       int i;
>  
> +     if (geteuid() != 0) {
> +             fprintf(stderr, "you must run this as root\n");
> +             exit(EXIT_FAILURE);
> +     }

If you want this, I think we should have a common function to check for
master capability ... Imo you can drop this.

> +
>       drm_fd = drm_open_any();
>  
>       bufmgr = drm_intel_bufmgr_gem_init(drm_fd, 4096);
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> [email protected]
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to