| Message ID | 1353590706-1366-5-git-send-email-imre.deak@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 <imre.deak@intel.com> 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 > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Thu, 2012-11-22 at 14:46 +0100, Daniel Vetter wrote: > 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 <imre.deak@intel.com> > > 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? The idea was to also test the compatibility mode, where we get real timestamps. > > > + > > /* 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. I added this early check since accessing timestamp_monotonic needs it. Otherwise you get some error only later when switching to compatibility mode, which I thought was confusing. > > > + > > drm_fd = drm_open_any(); > > > > bufmgr = drm_intel_bufmgr_gem_init(drm_fd, 4096); > > -- > > 1.7.9.5 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx >
On Thu, Nov 22, 2012 at 2:50 PM, Imre Deak <imre.deak@intel.com> wrote: >> > + 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? > > The idea was to also test the compatibility mode, where we get real > timestamps. Oh, I've missed that you write back the desired value. I don't think we need to test the compat mode if it's not enabled, just test whatever mode is enabled. That way we also check whether the getcap interface works. -Daniel
On Thu, 2012-11-22 at 15:04 +0100, Daniel Vetter wrote: > On Thu, Nov 22, 2012 at 2:50 PM, Imre Deak <imre.deak@intel.com> wrote: > >> > + 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? > > > > The idea was to also test the compatibility mode, where we get real > > timestamps. > > Oh, I've missed that you write back the desired value. I don't think > we need to test the compat mode if it's not enabled, just test > whatever mode is enabled. That way we also check whether the getcap > interface works. Ok, will do so. We have to wait though until DRM_CAP_TIMESTAMP_MONOTONIC is added to libdrm. I've sent already a patch for this. --Imre
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); + + 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)); + /* 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); + } + drm_fd = drm_open_any(); bufmgr = drm_intel_bufmgr_gem_init(drm_fd, 4096);
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 <imre.deak@intel.com> --- tests/flip_test.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 48 insertions(+), 1 deletion(-)