On 05.10.12 15:37, intel-gfx-request at lists.freedesktop.org wrote:
>
> Today's Topics:
>
> 1. [RFC 0/4] drm: add raw monotonic timestamp support (Imre Deak)
> 2. [RFC 1/4] time: export getnstime_raw_and_real for DRM (Imre Deak)
> 3. [RFC 2/4] drm: make memset/calloc for _vblank_time more
> robust (Imre Deak)
> 4. [RFC 3/4] drm: use raw time in
> drm_calc_vbltimestamp_from_scanoutpos (Imre Deak)
> 5. [RFC 4/4] drm: add support for raw monotonic vblank
> timestamps (Imre Deak)
>
>
> ----------------------------------------------------------------------
>
> Message: 1
> Date: Fri, 5 Oct 2012 16:36:58 +0300
> From: Imre Deak<imre.deak at intel.com>
> To: Daniel Vetter<daniel.vetter at ffwll.ch>, Chris Wilson
> <chris at chris-wilson.co.uk>, Kristian H?gsberg<krh at bitplanet.net>
> Cc:intel-gfx at lists.freedesktop.org,dri-devel at lists.freedesktop.org
> Subject: [RFC 0/4] drm: add raw monotonic timestamp
> support
> Message-ID:<1349444222-22274-1-git-send-email-imre.deak at intel.com>
>
> This is needed to make applications depending on vblank/page flip
> timestamps independent of time ajdustments.
>
> I've tested these with an updated intel-gpu-test/flip_test and will send
> the update for that once there's no objection about this patchset.
>
I'm mostly fine with this, although the wall time compatibility stuff
may not be useful given that userspace apps, e.g., OpenGL clients, have
no way to actually ask for wall vs. monotonic, and the spec actually
expects them to expect monotonic timestamps.
I also see that an update to nouveau-kms is missing? Afaik the vblank
timestamping on nouveau-kms is still handled by the fallback in the drm,
but pageflip completion uses do_gettimeofday() for the timestamping and
returns a hard-coded zero vblank count all time for pageflip events
(yay!). Lucas Stach and me have written and tested some patches to fix
this over a year ago, but somehow they never made it into the kms
driver, mostly due to bad timing in when stuff was submitted, reviewed
and revised, not for serious technical objections iirc.
Ideally we could give them another try, or at least update nouveaus
pageflip timestamping to avoid really bad breakage for OpenGL clients or
the nouveau-ddx due to inconsistent vblank vs. pageflip timestamps.
I quickly looked over the patches, i think they look mostly good, see
some small comments below.
> Subject: [RFC 3/4] drm: use raw time in
> drm_calc_vbltimestamp_from_scanoutpos
> Message-ID: <1349444222-22274-4-git-send-email-imre.deak at intel.com>
>
> The timestamp is used here for handling the timeout case, so we don't
> want it to be affected by time adjustments.
>
> Signed-off-by: Imre Deak <imre.deak at intel.com>
> ---
> drivers/gpu/drm/drm_irq.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> index 77f6577..5e42981 100644
> --- a/drivers/gpu/drm/drm_irq.c
> +++ b/drivers/gpu/drm/drm_irq.c
> @@ -576,7 +576,7 @@ int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev, int crtc,
> unsigned flags,
> struct drm_crtc *refcrtc)
> {
> - struct timeval stime, raw_time;
> + struct timespec raw_stime, raw_etime, real_etime;
> struct drm_display_mode *mode;
> int vbl_status, vtotal, vdisplay;
> int vpos, hpos, i;
> @@ -625,13 +625,13 @@ int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev, int crtc,
> preempt_disable();
>
> /* Get system timestamp before query. */
> - do_gettimeofday(&stime);
> + getrawmonotonic(&raw_stime);
>
> /* 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);
> + getnstime_raw_and_real(&raw_etime, &real_etime);
>
> preempt_enable();
>
> @@ -642,7 +642,8 @@ 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 = timespec_to_ns(&raw_etime) -
> + timespec_to_ns(&raw_stime);
>
> /* Accept result with < max_error nsecs timing uncertainty. */
> if (duration_ns <= (s64) *max_error)
> @@ -692,11 +693,11 @@ int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev, int crtc,
> /* 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);
> + *vblank_time = ns_to_timeval(timeval_to_ns(&real_time) - delta_ns);
This commit without the followup commit wouldn't compile, because you
changed real_time into real_etime. Your followup commit fixes this, so
squash them into one to avoid compilation problems for bisection?
Then, for the followup patch "[RFC 4/4] drm: add support for raw
monotonic vblank timestamps"
> @@ -693,12 +694,13 @@ int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev, int crtc,
> /* Subtract time delta from raw timestamp to get final
> * vblank_time timestamp for end of vblank.
> */
> - *vblank_time = ns_to_timeval(timeval_to_ns(&real_time) - delta_ns);
> + vblank_time->real = ns_to_timeval(timespec_to_ns(&real_etime) - delta_ns);
> + vblank_time->raw = ns_to_timeval(timespec_to_ns(&raw_etime) - delta_ns);
>
> 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_stime.tv_sec, (long)raw_stime.tv_nsec / 1000,
This shouldn't be raw_stime, but real_etime, so the debug output of
these vars can be compared to the values of vblank_time->real.tv_sec etc.
> - (long)vblank_time->tv_sec, (long)vblank_time->tv_usec,
> + (long)vblank_time->real.tv_sec, (long)vblank_time->real.tv_usec,
> (int)duration_ns/1000, i);
>
>
thanks,
-mario
