On 23.10.12 20:53, Imre Deak wrote:
For measuring duration we want to avoid that our start/end timestamps jump, so use monotonic instead of real time for that. Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx> --- drivers/gpu/drm/drm_irq.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index 89b830d..7dc203d 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -576,7 +576,8 @@ int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev, int crtc, unsigned flags, struct drm_crtc *refcrtc) { - struct timeval stime, raw_time; + ktime_t stime, etime, mono_time_offset; + struct timeval tv_etime; struct drm_display_mode *mode; int vbl_status, vtotal, vdisplay; int vpos, hpos, i; @@ -625,13 +626,14 @@ int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev, int crtc, preempt_disable(); /* Get system timestamp before query. */ - do_gettimeofday(&stime); + stime = ktime_get(); /* Get vertical and horizontal scanout pos. vpos, hpos. */ vbl_status = dev->driver->get_scanout_position(dev, crtc, &vpos, &hpos); /* Get system timestamp after query. */ - do_gettimeofday(&raw_time); + etime = ktime_get();
Here is possibly a tiny race: The wall_to_monotonic offset value could change between the ktime_get() - which uses it internally for wallclock -> monotonic clock conversion, and the ktime_get_monotonic_offset() query below, so the later subtraction of mono_time_offset from etime would not cancel out the addition to etime inside ktime_get() and you wouldn't get correct walltime back. There seem to be multiple sources of change to the value, e.g., do_settimeofday(), do_adjtimex() - the admin or ntp changing the system clock. The internal code, e.g., ktime_get() use a seqlock to protect against this race.
There's a function ktime_get_real(void) which directly gives you the wall time you want as ktime_t, but then you'd still need to do the ktime_get() query in the !drm_timestamp_monotonic case to calculate duration_ns below.
Same problem in the 2nd patch for get_drm_timestamp(). Otoh, the time window for the race is small and it can only happen in the non-default case of !drm_timestamp_monotonic, so i don't know if it is worth fixing it?
Other than that: Reviewed-by: mario.kleiner
+ mono_time_offset = ktime_get_monotonic_offset(); preempt_enable(); @@ -642,7 +644,7 @@ int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev, int crtc, return -EIO; } - duration_ns = timeval_to_ns(&raw_time) - timeval_to_ns(&stime); + duration_ns = ktime_to_ns(etime) - ktime_to_ns(stime); /* Accept result with < max_error nsecs timing uncertainty. */ if (duration_ns <= (s64) *max_error) @@ -689,14 +691,18 @@ int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev, int crtc, vbl_status |= 0x8; } + etime = ktime_sub(etime, mono_time_offset); + /* save this only for debugging purposes */ + tv_etime = ktime_to_timeval(etime); /* Subtract time delta from raw timestamp to get final * vblank_time timestamp for end of vblank. */ - *vblank_time = ns_to_timeval(timeval_to_ns(&raw_time) - delta_ns); + etime = ktime_sub_ns(etime, delta_ns); + *vblank_time = ktime_to_timeval(etime); DRM_DEBUG("crtc %d : v %d p(%d,%d)@ %ld.%ld -> %ld.%ld [e %d us, %d rep]\n", crtc, (int)vbl_status, hpos, vpos, - (long)raw_time.tv_sec, (long)raw_time.tv_usec, + (long)tv_etime.tv_sec, (long)tv_etime.tv_usec, (long)vblank_time->tv_sec, (long)vblank_time->tv_usec, (int)duration_ns/1000, i);
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel