On Sat, 2012-10-06 at 04:46 +0200, Mario Kleiner wrote:

On 05.10.12 15:37, intel-gfx-request@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 Deakimre.deak@intel.com To: Daniel Vetterdaniel.vetter@ffwll.ch, Chris Wilson chris@chris-wilson.co.uk, Kristian H?gsbergkrh@bitplanet.net Cc:intel-gfx@lists.freedesktop.org,dri-devel@lists.freedesktop.org Subject: [Intel-gfx] [RFC 0/4] drm: add raw monotonic timestamp support Message-ID:1349444222-22274-1-git-send-email-imre.deak@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: [Intel-gfx] [RFC 3/4] drm: use raw time in drm_calc_vbltimestamp_from_scanoutpos Message-ID: 1349444222-22274-4-git-send-email-imre.deak@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@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 for the review, I'll fix both issues.

--Imre