diff mbox

[RFC,4/4] drm: add support for raw monotonic vblank timestamps

Message ID 1349444222-22274-5-git-send-email-imre.deak@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Imre Deak Oct. 5, 2012, 1:37 p.m. UTC
In practice we never want the timestamps for vblank and page flip events
to be affected by time adjustments, so in addition to the gettimeofday
timestamps we used so far add support for raw monotonic timestamps.

For backward compatibility use flags to select between the old and new
timestamp format.

Note that with this change we will save the timestamp in both formats,
for cases where multiple clients are expecting an event notification in
different time formats. In theory we could track the clients and save
the timestamp only in one format if possible, but the overhead of this
tracking would be bigger than just saving both formats always.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/drm_crtc.c                |    2 +
 drivers/gpu/drm/drm_ioctl.c               |    3 ++
 drivers/gpu/drm/drm_irq.c                 |   68 +++++++++++++++++------------
 drivers/gpu/drm/i915/i915_irq.c           |    2 +-
 drivers/gpu/drm/i915/intel_display.c      |   12 ++---
 drivers/gpu/drm/radeon/radeon_display.c   |   10 +++--
 drivers/gpu/drm/radeon/radeon_drv.c       |    2 +-
 drivers/gpu/drm/radeon/radeon_kms.c       |    2 +-
 drivers/gpu/drm/shmobile/shmob_drm_crtc.c |    9 ++--
 include/drm/drm.h                         |    5 ++-
 include/drm/drmP.h                        |   38 +++++++++++++---
 include/drm/drm_mode.h                    |    4 +-
 12 files changed, 104 insertions(+), 53 deletions(-)

Comments

Michel Dänzer Oct. 5, 2012, 1:55 p.m. UTC | #1
On Fre, 2012-10-05 at 16:37 +0300, Imre Deak wrote: 
> In practice we never want the timestamps for vblank and page flip events
> to be affected by time adjustments, so in addition to the gettimeofday
> timestamps we used so far add support for raw monotonic timestamps.
> 
> For backward compatibility use flags to select between the old and new
> timestamp format.
> 
> Note that with this change we will save the timestamp in both formats,
> for cases where multiple clients are expecting an event notification in
> different time formats.

I wonder if all this trouble is really necessary. I honestly can't
imagine any user of this API requiring non-monotonic timestamps and
breaking with monotonic ones. I think it was simply a mistake that we
didn't make them monotonic in the first place (or maybe it wasn't even
possible when this API was first introduced).
Imre Deak Oct. 5, 2012, 1:59 p.m. UTC | #2
On Fri, 2012-10-05 at 15:55 +0200, Michel Dänzer wrote:
> On Fre, 2012-10-05 at 16:37 +0300, Imre Deak wrote: 
> > In practice we never want the timestamps for vblank and page flip events
> > to be affected by time adjustments, so in addition to the gettimeofday
> > timestamps we used so far add support for raw monotonic timestamps.
> > 
> > For backward compatibility use flags to select between the old and new
> > timestamp format.
> > 
> > Note that with this change we will save the timestamp in both formats,
> > for cases where multiple clients are expecting an event notification in
> > different time formats.
> 
> I wonder if all this trouble is really necessary. I honestly can't
> imagine any user of this API requiring non-monotonic timestamps and
> breaking with monotonic ones. I think it was simply a mistake that we
> didn't make them monotonic in the first place (or maybe it wasn't even
> possible when this API was first introduced).

Yea, I'd rather simply switch over to monotonic timestamps too. But that
would break apps that already compare against the wall time for whatever
purpose (for example A/V sync).

--Imre
Michel Dänzer Oct. 5, 2012, 2:14 p.m. UTC | #3
On Fre, 2012-10-05 at 16:59 +0300, Imre Deak wrote: 
> On Fri, 2012-10-05 at 15:55 +0200, Michel Dänzer wrote:
> > On Fre, 2012-10-05 at 16:37 +0300, Imre Deak wrote: 
> > > In practice we never want the timestamps for vblank and page flip events
> > > to be affected by time adjustments, so in addition to the gettimeofday
> > > timestamps we used so far add support for raw monotonic timestamps.
> > > 
> > > For backward compatibility use flags to select between the old and new
> > > timestamp format.
> > > 
> > > Note that with this change we will save the timestamp in both formats,
> > > for cases where multiple clients are expecting an event notification in
> > > different time formats.
> > 
> > I wonder if all this trouble is really necessary. I honestly can't
> > imagine any user of this API requiring non-monotonic timestamps and
> > breaking with monotonic ones. I think it was simply a mistake that we
> > didn't make them monotonic in the first place (or maybe it wasn't even
> > possible when this API was first introduced).
> 
> Yea, I'd rather simply switch over to monotonic timestamps too. But that
> would break apps that already compare against the wall time for whatever
> purpose (for example A/V sync).

Are there actually any such apps in the real world? Do they work when
the wall time jumps?
Imre Deak Oct. 5, 2012, 2:21 p.m. UTC | #4
On Fri, 2012-10-05 at 16:14 +0200, Michel Dänzer wrote:
> On Fre, 2012-10-05 at 16:59 +0300, Imre Deak wrote: 
> > On Fri, 2012-10-05 at 15:55 +0200, Michel Dänzer wrote:
> > > On Fre, 2012-10-05 at 16:37 +0300, Imre Deak wrote: 
> > > > In practice we never want the timestamps for vblank and page flip events
> > > > to be affected by time adjustments, so in addition to the gettimeofday
> > > > timestamps we used so far add support for raw monotonic timestamps.
> > > > 
> > > > For backward compatibility use flags to select between the old and new
> > > > timestamp format.
> > > > 
> > > > Note that with this change we will save the timestamp in both formats,
> > > > for cases where multiple clients are expecting an event notification in
> > > > different time formats.
> > > 
> > > I wonder if all this trouble is really necessary. I honestly can't
> > > imagine any user of this API requiring non-monotonic timestamps and
> > > breaking with monotonic ones. I think it was simply a mistake that we
> > > didn't make them monotonic in the first place (or maybe it wasn't even
> > > possible when this API was first introduced).
> > 
> > Yea, I'd rather simply switch over to monotonic timestamps too. But that
> > would break apps that already compare against the wall time for whatever
> > purpose (for example A/V sync).
> 
> Are there actually any such apps in the real world?

Tbh, I haven't checked, but we can't be sure in any case.

> Do they work when the wall time jumps?

They will have a problem with that yes. But providing them with a
monotonic clock when they expect otherwise will probably make them
unusable, so I think it's better to avoid that.

--Imre
Rob Clark Oct. 5, 2012, 10:18 p.m. UTC | #5
On Fri, Oct 5, 2012 at 7:37 AM, Imre Deak <imre.deak@intel.com> wrote:
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index ab1ef15..056e810 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6247,7 +6247,6 @@ static void do_intel_finish_page_flip(struct drm_device *dev,
>         struct intel_unpin_work *work;
>         struct drm_i915_gem_object *obj;
>         struct drm_pending_vblank_event *e;
> -       struct timeval tvbl;
>         unsigned long flags;
>
>         /* Ignore early vblank irqs */
> @@ -6264,12 +6263,13 @@ static void do_intel_finish_page_flip(struct drm_device *dev,
>         intel_crtc->unpin_work = NULL;
>
>         if (work->event) {
> -               e = work->event;
> -               e->event.sequence = drm_vblank_count_and_time(dev, intel_crtc->pipe, &tvbl);
> -
> -               e->event.tv_sec = tvbl.tv_sec;
> -               e->event.tv_usec = tvbl.tv_usec;
> +               struct drm_vblank_time tvbl;
> +               u32 seq;
>
> +               e = work->event;
> +               seq = drm_vblank_count_and_time(dev, intel_crtc->pipe, &tvbl);
> +               drm_set_event_seq_and_time(&e->event, e->timestamp_raw, seq,
> +                                          &tvbl);
>                 list_add_tail(&e->base.link,
>                               &e->base.file_priv->event_list);
>                 wake_up_interruptible(&e->base.file_priv->event_wait);


btw, I wonder if we could just have a helper like:

int drm_send_page_flip_event(struct drm_device *dev, int crtc,
              struct drm_pending_vblank_event *event);

Since most drivers have pretty much the same code for sending vblank
event after a page flip..

I guess not strictly related to monotonic timestamps, but it has been
a cleanup that I've been meaning to do for a while.. and I guess if
this was done first it wouldn't mean touching each driver (as much) to
add support for monotonic timestamps.

BR,
-R

> diff --git a/drivers/gpu/drm/radeon/radeon_display.c b/drivers/gpu/drm/radeon/radeon_display.c
> index 7ddef8f..55c014a 100644
> --- a/drivers/gpu/drm/radeon/radeon_display.c
> +++ b/drivers/gpu/drm/radeon/radeon_display.c
> @@ -272,7 +272,6 @@ void radeon_crtc_handle_flip(struct radeon_device *rdev, int crtc_id)
>         struct radeon_crtc *radeon_crtc = rdev->mode_info.crtcs[crtc_id];
>         struct radeon_unpin_work *work;
>         struct drm_pending_vblank_event *e;
> -       struct timeval now;
>         unsigned long flags;
>         u32 update_pending;
>         int vpos, hpos;
> @@ -329,10 +328,13 @@ void radeon_crtc_handle_flip(struct radeon_device *rdev, int crtc_id)
>
>         /* wakeup userspace */
>         if (work->event) {
> +               struct drm_vblank_time now;
> +               u32 seq;
> +
>                 e = work->event;
> -               e->event.sequence = drm_vblank_count_and_time(rdev->ddev, crtc_id, &now);
> -               e->event.tv_sec = now.tv_sec;
> -               e->event.tv_usec = now.tv_usec;
> +               seq = drm_vblank_count_and_time(rdev->ddev, crtc_id, &now);
> +               drm_set_event_seq_and_time(&e->event, e->timestamp_raw,
> +                                          seq, &now);
>                 list_add_tail(&e->base.link, &e->base.file_priv->event_list);
>                 wake_up_interruptible(&e->base.file_priv->event_wait);
>         }
Imre Deak Oct. 5, 2012, 11:41 p.m. UTC | #6
On Fri, 2012-10-05 at 16:18 -0600, Rob Clark wrote:
> On Fri, Oct 5, 2012 at 7:37 AM, Imre Deak <imre.deak@intel.com> wrote:
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index ab1ef15..056e810 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -6247,7 +6247,6 @@ static void do_intel_finish_page_flip(struct drm_device *dev,
> >         struct intel_unpin_work *work;
> >         struct drm_i915_gem_object *obj;
> >         struct drm_pending_vblank_event *e;
> > -       struct timeval tvbl;
> >         unsigned long flags;
> >
> >         /* Ignore early vblank irqs */
> > @@ -6264,12 +6263,13 @@ static void do_intel_finish_page_flip(struct drm_device *dev,
> >         intel_crtc->unpin_work = NULL;
> >
> >         if (work->event) {
> > -               e = work->event;
> > -               e->event.sequence = drm_vblank_count_and_time(dev, intel_crtc->pipe, &tvbl);
> > -
> > -               e->event.tv_sec = tvbl.tv_sec;
> > -               e->event.tv_usec = tvbl.tv_usec;
> > +               struct drm_vblank_time tvbl;
> > +               u32 seq;
> >
> > +               e = work->event;
> > +               seq = drm_vblank_count_and_time(dev, intel_crtc->pipe, &tvbl);
> > +               drm_set_event_seq_and_time(&e->event, e->timestamp_raw, seq,
> > +                                          &tvbl);
> >                 list_add_tail(&e->base.link,
> >                               &e->base.file_priv->event_list);
> >                 wake_up_interruptible(&e->base.file_priv->event_wait);
> 
> 
> btw, I wonder if we could just have a helper like:
> 
> int drm_send_page_flip_event(struct drm_device *dev, int crtc,
>               struct drm_pending_vblank_event *event);
> 
> Since most drivers have pretty much the same code for sending vblank
> event after a page flip..
> 
> I guess not strictly related to monotonic timestamps, but it has been
> a cleanup that I've been meaning to do for a while.. and I guess if
> this was done first it wouldn't mean touching each driver (as much) to
> add support for monotonic timestamps.

Good idea, we should do this.

But if we want to reduce the diff from my changes then the proto should
rather be:

int drm_send_page_flip_event(struct drm_device *dev, int crtc,
               struct drm_pending_vblank_event *event,
               int seq, struct timeval *now);

with struct timeval changing to struct drm_vblank_time with my changes.

I can do this if you agree - unless you have something ready.

--Imre

> 
> BR,
> -R
> 
> > diff --git a/drivers/gpu/drm/radeon/radeon_display.c b/drivers/gpu/drm/radeon/radeon_display.c
> > index 7ddef8f..55c014a 100644
> > --- a/drivers/gpu/drm/radeon/radeon_display.c
> > +++ b/drivers/gpu/drm/radeon/radeon_display.c
> > @@ -272,7 +272,6 @@ void radeon_crtc_handle_flip(struct radeon_device *rdev, int crtc_id)
> >         struct radeon_crtc *radeon_crtc = rdev->mode_info.crtcs[crtc_id];
> >         struct radeon_unpin_work *work;
> >         struct drm_pending_vblank_event *e;
> > -       struct timeval now;
> >         unsigned long flags;
> >         u32 update_pending;
> >         int vpos, hpos;
> > @@ -329,10 +328,13 @@ void radeon_crtc_handle_flip(struct radeon_device *rdev, int crtc_id)
> >
> >         /* wakeup userspace */
> >         if (work->event) {
> > +               struct drm_vblank_time now;
> > +               u32 seq;
> > +
> >                 e = work->event;
> > -               e->event.sequence = drm_vblank_count_and_time(rdev->ddev, crtc_id, &now);
> > -               e->event.tv_sec = now.tv_sec;
> > -               e->event.tv_usec = now.tv_usec;
> > +               seq = drm_vblank_count_and_time(rdev->ddev, crtc_id, &now);
> > +               drm_set_event_seq_and_time(&e->event, e->timestamp_raw,
> > +                                          seq, &now);
> >                 list_add_tail(&e->base.link, &e->base.file_priv->event_list);
> >                 wake_up_interruptible(&e->base.file_priv->event_wait);
> >         }
Rob Clark Oct. 6, 2012, 12:09 a.m. UTC | #7
On Fri, Oct 5, 2012 at 5:41 PM, Imre Deak <imre.deak@intel.com> wrote:
> On Fri, 2012-10-05 at 16:18 -0600, Rob Clark wrote:
>> On Fri, Oct 5, 2012 at 7:37 AM, Imre Deak <imre.deak@intel.com> wrote:
>> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> > index ab1ef15..056e810 100644
>> > --- a/drivers/gpu/drm/i915/intel_display.c
>> > +++ b/drivers/gpu/drm/i915/intel_display.c
>> > @@ -6247,7 +6247,6 @@ static void do_intel_finish_page_flip(struct drm_device *dev,
>> >         struct intel_unpin_work *work;
>> >         struct drm_i915_gem_object *obj;
>> >         struct drm_pending_vblank_event *e;
>> > -       struct timeval tvbl;
>> >         unsigned long flags;
>> >
>> >         /* Ignore early vblank irqs */
>> > @@ -6264,12 +6263,13 @@ static void do_intel_finish_page_flip(struct drm_device *dev,
>> >         intel_crtc->unpin_work = NULL;
>> >
>> >         if (work->event) {
>> > -               e = work->event;
>> > -               e->event.sequence = drm_vblank_count_and_time(dev, intel_crtc->pipe, &tvbl);
>> > -
>> > -               e->event.tv_sec = tvbl.tv_sec;
>> > -               e->event.tv_usec = tvbl.tv_usec;
>> > +               struct drm_vblank_time tvbl;
>> > +               u32 seq;
>> >
>> > +               e = work->event;
>> > +               seq = drm_vblank_count_and_time(dev, intel_crtc->pipe, &tvbl);
>> > +               drm_set_event_seq_and_time(&e->event, e->timestamp_raw, seq,
>> > +                                          &tvbl);
>> >                 list_add_tail(&e->base.link,
>> >                               &e->base.file_priv->event_list);
>> >                 wake_up_interruptible(&e->base.file_priv->event_wait);
>>
>>
>> btw, I wonder if we could just have a helper like:
>>
>> int drm_send_page_flip_event(struct drm_device *dev, int crtc,
>>               struct drm_pending_vblank_event *event);
>>
>> Since most drivers have pretty much the same code for sending vblank
>> event after a page flip..
>>
>> I guess not strictly related to monotonic timestamps, but it has been
>> a cleanup that I've been meaning to do for a while.. and I guess if
>> this was done first it wouldn't mean touching each driver (as much) to
>> add support for monotonic timestamps.
>
> Good idea, we should do this.
>
> But if we want to reduce the diff from my changes then the proto should
> rather be:
>
> int drm_send_page_flip_event(struct drm_device *dev, int crtc,
>                struct drm_pending_vblank_event *event,
>                int seq, struct timeval *now);

do we need 'seq' and 'now'?  I was sort of thinking that could all be
internal to the send_page_flip_event() fxn.. ie. like:

---------
void drm_send_page_flip_event(struct drm_device *dev, int pipe,
		struct drm_crtc *crtc, struct drm_pending_vblank_event *event)
{
	struct timeval tnow, tvbl;

	do_gettimeofday(&tnow);

	event->event.sequence = drm_vblank_count_and_time(dev, pipe, &tvbl);

	/* Called before vblank count and timestamps have
	 * been updated for the vblank interval of flip
	 * completion? Need to increment vblank count and
	 * add one videorefresh duration to returned timestamp
	 * to account for this. We assume this happened if we
	 * get called over 0.9 frame durations after the last
	 * timestamped vblank.
	 *
	 * This calculation can not be used with vrefresh rates
	 * below 5Hz (10Hz to be on the safe side) without
	 * promoting to 64 integers.
	 */
	if (10 * (timeval_to_ns(&tnow) - timeval_to_ns(&tvbl)) >
	    9 * crtc->framedur_ns) {
		event->event.sequence++;
		tvbl = ns_to_timeval(timeval_to_ns(&tvbl) +
				     crtc->framedur_ns);
	}

	event->event.tv_sec = tvbl.tv_sec;
	event->event.tv_usec = tvbl.tv_usec;

	list_add_tail(&event->base.link,
		      &event->base.file_priv->event_list);
	wake_up_interruptible(&event->base.file_priv->event_wait);
}
---------

well, this would be the pre-monotonic timestamp version.. and it is a
bit weird to have to pass in both crtc ptr and pipe #.. I don't see
anything obvious in drm core that links pipe # and crtc ptr, although
userspace seems to make this assumption about order of crtc's.  But I
think that if() statement is only in i915 driver.. I assume because
you don't know if this will get called before or after
drm_handle_vblank()?  I guess that shouldn't hurt for other drivers.

then each driver would just have something like:

---------
	if (work->event)
		drm_send_page_flip_event(dev, intel_crtc->pipe, crtc, work->event);
---------


> with struct timeval changing to struct drm_vblank_time with my changes.
>
> I can do this if you agree - unless you have something ready.

I've locally split out this into a helper fxn in omapdrm, but haven't
got around to pushing it to core and updating other drivers.  If you
want I can send a patch, but I guess it is easier to just include
something like this in your patchset.  I'm ok either way.

BR,
-R

> --Imre
>
>>
>> BR,
>> -R
>>
>> > diff --git a/drivers/gpu/drm/radeon/radeon_display.c b/drivers/gpu/drm/radeon/radeon_display.c
>> > index 7ddef8f..55c014a 100644
>> > --- a/drivers/gpu/drm/radeon/radeon_display.c
>> > +++ b/drivers/gpu/drm/radeon/radeon_display.c
>> > @@ -272,7 +272,6 @@ void radeon_crtc_handle_flip(struct radeon_device *rdev, int crtc_id)
>> >         struct radeon_crtc *radeon_crtc = rdev->mode_info.crtcs[crtc_id];
>> >         struct radeon_unpin_work *work;
>> >         struct drm_pending_vblank_event *e;
>> > -       struct timeval now;
>> >         unsigned long flags;
>> >         u32 update_pending;
>> >         int vpos, hpos;
>> > @@ -329,10 +328,13 @@ void radeon_crtc_handle_flip(struct radeon_device *rdev, int crtc_id)
>> >
>> >         /* wakeup userspace */
>> >         if (work->event) {
>> > +               struct drm_vblank_time now;
>> > +               u32 seq;
>> > +
>> >                 e = work->event;
>> > -               e->event.sequence = drm_vblank_count_and_time(rdev->ddev, crtc_id, &now);
>> > -               e->event.tv_sec = now.tv_sec;
>> > -               e->event.tv_usec = now.tv_usec;
>> > +               seq = drm_vblank_count_and_time(rdev->ddev, crtc_id, &now);
>> > +               drm_set_event_seq_and_time(&e->event, e->timestamp_raw,
>> > +                                          seq, &now);
>> >                 list_add_tail(&e->base.link, &e->base.file_priv->event_list);
>> >                 wake_up_interruptible(&e->base.file_priv->event_wait);
>> >         }
>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
Imre Deak Oct. 6, 2012, 12:49 a.m. UTC | #8
On Fri, 2012-10-05 at 18:09 -0600, Rob Clark wrote:
> On Fri, Oct 5, 2012 at 5:41 PM, Imre Deak <imre.deak@intel.com> wrote:
> > On Fri, 2012-10-05 at 16:18 -0600, Rob Clark wrote:
> >> On Fri, Oct 5, 2012 at 7:37 AM, Imre Deak <imre.deak@intel.com> wrote:
> >> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >> > index ab1ef15..056e810 100644
> >> > --- a/drivers/gpu/drm/i915/intel_display.c
> >> > +++ b/drivers/gpu/drm/i915/intel_display.c
> >> > @@ -6247,7 +6247,6 @@ static void do_intel_finish_page_flip(struct drm_device *dev,
> >> >         struct intel_unpin_work *work;
> >> >         struct drm_i915_gem_object *obj;
> >> >         struct drm_pending_vblank_event *e;
> >> > -       struct timeval tvbl;
> >> >         unsigned long flags;
> >> >
> >> >         /* Ignore early vblank irqs */
> >> > @@ -6264,12 +6263,13 @@ static void do_intel_finish_page_flip(struct drm_device *dev,
> >> >         intel_crtc->unpin_work = NULL;
> >> >
> >> >         if (work->event) {
> >> > -               e = work->event;
> >> > -               e->event.sequence = drm_vblank_count_and_time(dev, intel_crtc->pipe, &tvbl);
> >> > -
> >> > -               e->event.tv_sec = tvbl.tv_sec;
> >> > -               e->event.tv_usec = tvbl.tv_usec;
> >> > +               struct drm_vblank_time tvbl;
> >> > +               u32 seq;
> >> >
> >> > +               e = work->event;
> >> > +               seq = drm_vblank_count_and_time(dev, intel_crtc->pipe, &tvbl);
> >> > +               drm_set_event_seq_and_time(&e->event, e->timestamp_raw, seq,
> >> > +                                          &tvbl);
> >> >                 list_add_tail(&e->base.link,
> >> >                               &e->base.file_priv->event_list);
> >> >                 wake_up_interruptible(&e->base.file_priv->event_wait);
> >>
> >>
> >> btw, I wonder if we could just have a helper like:
> >>
> >> int drm_send_page_flip_event(struct drm_device *dev, int crtc,
> >>               struct drm_pending_vblank_event *event);
> >>
> >> Since most drivers have pretty much the same code for sending vblank
> >> event after a page flip..
> >>
> >> I guess not strictly related to monotonic timestamps, but it has been
> >> a cleanup that I've been meaning to do for a while.. and I guess if
> >> this was done first it wouldn't mean touching each driver (as much) to
> >> add support for monotonic timestamps.
> >
> > Good idea, we should do this.
> >
> > But if we want to reduce the diff from my changes then the proto should
> > rather be:
> >
> > int drm_send_page_flip_event(struct drm_device *dev, int crtc,
> >                struct drm_pending_vblank_event *event,
> >                int seq, struct timeval *now);
> 
> do we need 'seq' and 'now'?  I was sort of thinking that could all be
> internal to the send_page_flip_event() fxn.. ie. like:
> 
> ---------
> void drm_send_page_flip_event(struct drm_device *dev, int pipe,
> 		struct drm_crtc *crtc, struct drm_pending_vblank_event *event)
> {
> 	struct timeval tnow, tvbl;
> 
> 	do_gettimeofday(&tnow);
> 
> 	event->event.sequence = drm_vblank_count_and_time(dev, pipe, &tvbl);

Ah, ok, you are right. I originally thought of using this helper in
drm_handle_vblank_events too, but now I realized it's not quite the same
sequence there.

> 
> 	/* Called before vblank count and timestamps have
> 	 * been updated for the vblank interval of flip
> 	 * completion? Need to increment vblank count and
> 	 * add one videorefresh duration to returned timestamp
> 	 * to account for this. We assume this happened if we
> 	 * get called over 0.9 frame durations after the last
> 	 * timestamped vblank.
> 	 *
> 	 * This calculation can not be used with vrefresh rates
> 	 * below 5Hz (10Hz to be on the safe side) without
> 	 * promoting to 64 integers.
> 	 */
> 	if (10 * (timeval_to_ns(&tnow) - timeval_to_ns(&tvbl)) >
> 	    9 * crtc->framedur_ns) {
> 		event->event.sequence++;
> 		tvbl = ns_to_timeval(timeval_to_ns(&tvbl) +
> 				     crtc->framedur_ns);
> 	}

This has been recently removed by danvet's "drm/i915: don't frob the
vblank ts in finish_page_flip", though not yet committed, so we can do
away with it here too.

> 	event->event.tv_sec = tvbl.tv_sec;
> 	event->event.tv_usec = tvbl.tv_usec;
> 
> 	list_add_tail(&event->base.link,
> 		      &event->base.file_priv->event_list);
> 	wake_up_interruptible(&event->base.file_priv->event_wait);
> }


> ---------
> 
> well, this would be the pre-monotonic timestamp version.. and it is a
> bit weird to have to pass in both crtc ptr and pipe #.. I don't see
> anything obvious in drm core that links pipe # and crtc ptr, although
> userspace seems to make this assumption about order of crtc's.  But I
> think that if() statement is only in i915 driver.. I assume because
> you don't know if this will get called before or after
> drm_handle_vblank()?  I guess that shouldn't hurt for other drivers.
> 
> then each driver would just have something like:
> 
> ---------
> 	if (work->event)
> 		drm_send_page_flip_event(dev, intel_crtc->pipe, crtc, work->event);
> ---------
> 
> 
> > with struct timeval changing to struct drm_vblank_time with my changes.
> >
> > I can do this if you agree - unless you have something ready.
> 
> I've locally split out this into a helper fxn in omapdrm, but haven't
> got around to pushing it to core and updating other drivers.  If you
> want I can send a patch, but I guess it is easier to just include
> something like this in your patchset.  I'm ok either way.

I think the easiest is if you post your updated patch since it can be
anyway applied independently. I'll then rebase my changes on top of
that.

Thanks,
Imre
Daniel Vetter Oct. 7, 2012, 8:33 p.m. UTC | #9
On Sat, Oct 06, 2012 at 03:49:16AM +0300, Imre Deak wrote:
> On Fri, 2012-10-05 at 18:09 -0600, Rob Clark wrote:
> > 
> > 	/* Called before vblank count and timestamps have
> > 	 * been updated for the vblank interval of flip
> > 	 * completion? Need to increment vblank count and
> > 	 * add one videorefresh duration to returned timestamp
> > 	 * to account for this. We assume this happened if we
> > 	 * get called over 0.9 frame durations after the last
> > 	 * timestamped vblank.
> > 	 *
> > 	 * This calculation can not be used with vrefresh rates
> > 	 * below 5Hz (10Hz to be on the safe side) without
> > 	 * promoting to 64 integers.
> > 	 */
> > 	if (10 * (timeval_to_ns(&tnow) - timeval_to_ns(&tvbl)) >
> > 	    9 * crtc->framedur_ns) {
> > 		event->event.sequence++;
> > 		tvbl = ns_to_timeval(timeval_to_ns(&tvbl) +
> > 				     crtc->framedur_ns);
> > 	}
> 
> This has been recently removed by danvet's "drm/i915: don't frob the
> vblank ts in finish_page_flip", though not yet committed, so we can do
> away with it here too.

Yeah, I'd prefer if the common code doesn't have such hackes - this bit
here papered over some bugs in our driver code, but only partially
successfully ...
-Daniel
diff mbox

Patch

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index c317f72..e492363 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -3550,6 +3550,8 @@  int drm_mode_page_flip_ioctl(struct drm_device *dev,
 			goto out;
 		}
 
+		e->timestamp_raw = page_flip->flags &
+				   DRM_MODE_PAGE_FLIP_TIMESTAMP_RAW;
 		e->event.base.type = DRM_EVENT_FLIP_COMPLETE;
 		e->event.base.length = sizeof e->event;
 		e->event.user_data = page_flip->user_data;
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 64a62c6..0a30299 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -287,6 +287,9 @@  int drm_getcap(struct drm_device *dev, void *data, struct drm_file *file_priv)
 		req->value |= dev->driver->prime_fd_to_handle ? DRM_PRIME_CAP_IMPORT : 0;
 		req->value |= dev->driver->prime_handle_to_fd ? DRM_PRIME_CAP_EXPORT : 0;
 		break;
+	case DRM_CAP_TIMESTAMP_RAW:
+		req->value = 1;
+		break;
 	default:
 		return -EINVAL;
 	}
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index 5e42981..4879363 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -105,7 +105,7 @@  static void vblank_disable_and_save(struct drm_device *dev, int crtc)
 	u32 vblcount;
 	s64 diff_ns;
 	int vblrc;
-	struct timeval tvblank;
+	struct drm_vblank_time tvblank;
 
 	/* Prevent vblank irq processing while disabling vblank irqs,
 	 * so no updates of timestamps or count can happen after we've
@@ -137,8 +137,8 @@  static void vblank_disable_and_save(struct drm_device *dev, int crtc)
 	 * as updated by last invocation of drm_handle_vblank() in vblank irq.
 	 */
 	vblcount = atomic_read(&dev->_vblank_count[crtc]);
-	diff_ns = timeval_to_ns(&tvblank) -
-		  timeval_to_ns(&vblanktimestamp(dev, crtc, vblcount));
+	diff_ns = timeval_to_ns(&tvblank.raw) -
+		  timeval_to_ns(&vblanktimestamp(dev, crtc, vblcount).raw);
 
 	/* If there is at least 1 msec difference between the last stored
 	 * timestamp and tvblank, then we are currently executing our
@@ -550,7 +550,8 @@  EXPORT_SYMBOL(drm_calc_timestamping_constants);
  * @crtc: Which crtc's vblank timestamp to retrieve.
  * @max_error: Desired maximum allowable error in timestamps (nanosecs).
  *             On return contains true maximum error of timestamp.
- * @vblank_time: Pointer to struct timeval which should receive the timestamp.
+ * @vblank_time: Pointer to struct drm_vblank_time which should receive the
+ *		 timestamp both in raw monotonic and wall-time format.
  * @flags: Flags to pass to driver:
  *         0 = Default.
  *         DRM_CALLED_FROM_VBLIRQ = If function is called from vbl irq handler.
@@ -572,7 +573,7 @@  EXPORT_SYMBOL(drm_calc_timestamping_constants);
  */
 int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev, int crtc,
 					  int *max_error,
-					  struct timeval *vblank_time,
+					  struct drm_vblank_time *vblank_time,
 					  unsigned flags,
 					  struct drm_crtc *refcrtc)
 {
@@ -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,
-		  (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);
 
 	vbl_status = DRM_VBLANKTIME_SCANOUTPOS_METHOD;
@@ -730,8 +732,9 @@  EXPORT_SYMBOL(drm_calc_vbltimestamp_from_scanoutpos);
  * Returns non-zero if timestamp is considered to be very precise.
  */
 u32 drm_get_last_vbltimestamp(struct drm_device *dev, int crtc,
-			      struct timeval *tvblank, unsigned flags)
+			      struct drm_vblank_time *tvblank, unsigned flags)
 {
+	struct timespec ts_raw, ts_real;
 	int ret;
 
 	/* Define requested maximum error on timestamps (nanoseconds). */
@@ -748,7 +751,13 @@  u32 drm_get_last_vbltimestamp(struct drm_device *dev, int crtc,
 	/* GPU high precision timestamp query unsupported or failed.
 	 * Return gettimeofday timestamp as best estimate.
 	 */
-	do_gettimeofday(tvblank);
+	getnstime_raw_and_real(&ts_raw, &ts_real);
+
+	tvblank->raw.tv_sec = ts_raw.tv_sec;
+	tvblank->raw.tv_usec = ts_raw.tv_nsec / 1000;
+
+	tvblank->real.tv_sec = ts_real.tv_sec;
+	tvblank->real.tv_usec = ts_real.tv_nsec / 1000;
 
 	return 0;
 }
@@ -784,7 +793,7 @@  EXPORT_SYMBOL(drm_vblank_count);
  * value.
  */
 u32 drm_vblank_count_and_time(struct drm_device *dev, int crtc,
-			      struct timeval *vblanktime)
+			      struct drm_vblank_time *vblanktime)
 {
 	u32 cur_vblank;
 
@@ -822,7 +831,7 @@  EXPORT_SYMBOL(drm_vblank_count_and_time);
 static void drm_update_vblank_count(struct drm_device *dev, int crtc)
 {
 	u32 cur_vblank, diff, tslot, rc;
-	struct timeval t_vblank;
+	struct drm_vblank_time t_vblank;
 
 	/*
 	 * Interrupts were disabled prior to this call, so deal with counter
@@ -940,7 +949,7 @@  EXPORT_SYMBOL(drm_vblank_put);
 void drm_vblank_off(struct drm_device *dev, int crtc)
 {
 	struct drm_pending_vblank_event *e, *t;
-	struct timeval now;
+	struct drm_vblank_time now;
 	unsigned long irqflags;
 	unsigned int seq;
 
@@ -957,9 +966,8 @@  void drm_vblank_off(struct drm_device *dev, int crtc)
 			  wanted %d, current %d\n",
 			  e->event.sequence, seq);
 
-		e->event.sequence = seq;
-		e->event.tv_sec = now.tv_sec;
-		e->event.tv_usec = now.tv_usec;
+		drm_set_event_seq_and_time(&e->event, e->timestamp_raw, seq,
+					   &now);
 		drm_vblank_put(dev, e->pipe);
 		list_move_tail(&e->base.link, &e->base.file_priv->event_list);
 		wake_up_interruptible(&e->base.file_priv->event_wait);
@@ -1064,7 +1072,7 @@  static int drm_queue_vblank_event(struct drm_device *dev, int pipe,
 				  struct drm_file *file_priv)
 {
 	struct drm_pending_vblank_event *e;
-	struct timeval now;
+	struct drm_vblank_time now;
 	unsigned long flags;
 	unsigned int seq;
 	int ret;
@@ -1076,6 +1084,7 @@  static int drm_queue_vblank_event(struct drm_device *dev, int pipe,
 	}
 
 	e->pipe = pipe;
+	e->timestamp_raw = vblwait->request.type & _DRM_VBLANK_TIMESTAMP_RAW;
 	e->base.pid = current->pid;
 	e->event.base.type = DRM_EVENT_VBLANK;
 	e->event.base.length = sizeof e->event;
@@ -1108,9 +1117,8 @@  static int drm_queue_vblank_event(struct drm_device *dev, int pipe,
 
 	e->event.sequence = vblwait->request.sequence;
 	if ((seq - vblwait->request.sequence) <= (1 << 23)) {
-		e->event.sequence = seq;
-		e->event.tv_sec = now.tv_sec;
-		e->event.tv_usec = now.tv_usec;
+		drm_set_event_seq_and_time(&e->event, e->timestamp_raw, seq,
+					   &now);
 		drm_vblank_put(dev, pipe);
 		list_add_tail(&e->base.link, &e->base.file_priv->event_list);
 		wake_up_interruptible(&e->base.file_priv->event_wait);
@@ -1220,11 +1228,14 @@  int drm_wait_vblank(struct drm_device *dev, void *data,
 		     !dev->irq_enabled));
 
 	if (ret != -EINTR) {
-		struct timeval now;
+		struct drm_vblank_time now;
+		struct timeval *tv;
 
 		vblwait->reply.sequence = drm_vblank_count_and_time(dev, crtc, &now);
-		vblwait->reply.tval_sec = now.tv_sec;
-		vblwait->reply.tval_usec = now.tv_usec;
+		tv = vblwait->request.type & _DRM_VBLANK_TIMESTAMP_RAW ?
+			&now.raw : &now.real;
+		vblwait->reply.tval_sec = tv->tv_sec;
+		vblwait->reply.tval_usec = tv->tv_usec;
 
 		DRM_DEBUG("returning %d to client\n",
 			  vblwait->reply.sequence);
@@ -1240,7 +1251,7 @@  done:
 static void drm_handle_vblank_events(struct drm_device *dev, int crtc)
 {
 	struct drm_pending_vblank_event *e, *t;
-	struct timeval now;
+	struct drm_vblank_time now;
 	unsigned long flags;
 	unsigned int seq;
 
@@ -1257,9 +1268,8 @@  static void drm_handle_vblank_events(struct drm_device *dev, int crtc)
 		DRM_DEBUG("vblank event on %d, current %d\n",
 			  e->event.sequence, seq);
 
-		e->event.sequence = seq;
-		e->event.tv_sec = now.tv_sec;
-		e->event.tv_usec = now.tv_usec;
+		drm_set_event_seq_and_time(&e->event, e->timestamp_raw,
+					   seq, &now);
 		drm_vblank_put(dev, e->pipe);
 		list_move_tail(&e->base.link, &e->base.file_priv->event_list);
 		wake_up_interruptible(&e->base.file_priv->event_wait);
@@ -1284,7 +1294,7 @@  bool drm_handle_vblank(struct drm_device *dev, int crtc)
 {
 	u32 vblcount;
 	s64 diff_ns;
-	struct timeval tvblank;
+	struct drm_vblank_time tvblank;
 	unsigned long irqflags;
 
 	if (!dev->num_crtcs)
@@ -1311,8 +1321,8 @@  bool drm_handle_vblank(struct drm_device *dev, int crtc)
 	drm_get_last_vbltimestamp(dev, crtc, &tvblank, DRM_CALLED_FROM_VBLIRQ);
 
 	/* Compute time difference to timestamp of last vblank */
-	diff_ns = timeval_to_ns(&tvblank) -
-		  timeval_to_ns(&vblanktimestamp(dev, crtc, vblcount));
+	diff_ns = timeval_to_ns(&tvblank.raw) -
+		  timeval_to_ns(&vblanktimestamp(dev, crtc, vblcount).raw);
 
 	/* Update vblank timestamp and count if at least
 	 * DRM_REDUNDANT_VBLIRQ_THRESH_NS nanoseconds
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 2dd5895..e3a75e3 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -243,7 +243,7 @@  static int i915_get_crtc_scanoutpos(struct drm_device *dev, int pipe,
 
 static int i915_get_vblank_timestamp(struct drm_device *dev, int pipe,
 			      int *max_error,
-			      struct timeval *vblank_time,
+			      struct drm_vblank_time *vblank_time,
 			      unsigned flags)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index ab1ef15..056e810 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6247,7 +6247,6 @@  static void do_intel_finish_page_flip(struct drm_device *dev,
 	struct intel_unpin_work *work;
 	struct drm_i915_gem_object *obj;
 	struct drm_pending_vblank_event *e;
-	struct timeval tvbl;
 	unsigned long flags;
 
 	/* Ignore early vblank irqs */
@@ -6264,12 +6263,13 @@  static void do_intel_finish_page_flip(struct drm_device *dev,
 	intel_crtc->unpin_work = NULL;
 
 	if (work->event) {
-		e = work->event;
-		e->event.sequence = drm_vblank_count_and_time(dev, intel_crtc->pipe, &tvbl);
-
-		e->event.tv_sec = tvbl.tv_sec;
-		e->event.tv_usec = tvbl.tv_usec;
+		struct drm_vblank_time tvbl;
+		u32 seq;
 
+		e = work->event;
+		seq = drm_vblank_count_and_time(dev, intel_crtc->pipe, &tvbl);
+		drm_set_event_seq_and_time(&e->event, e->timestamp_raw, seq,
+					   &tvbl);
 		list_add_tail(&e->base.link,
 			      &e->base.file_priv->event_list);
 		wake_up_interruptible(&e->base.file_priv->event_wait);
diff --git a/drivers/gpu/drm/radeon/radeon_display.c b/drivers/gpu/drm/radeon/radeon_display.c
index 7ddef8f..55c014a 100644
--- a/drivers/gpu/drm/radeon/radeon_display.c
+++ b/drivers/gpu/drm/radeon/radeon_display.c
@@ -272,7 +272,6 @@  void radeon_crtc_handle_flip(struct radeon_device *rdev, int crtc_id)
 	struct radeon_crtc *radeon_crtc = rdev->mode_info.crtcs[crtc_id];
 	struct radeon_unpin_work *work;
 	struct drm_pending_vblank_event *e;
-	struct timeval now;
 	unsigned long flags;
 	u32 update_pending;
 	int vpos, hpos;
@@ -329,10 +328,13 @@  void radeon_crtc_handle_flip(struct radeon_device *rdev, int crtc_id)
 
 	/* wakeup userspace */
 	if (work->event) {
+		struct drm_vblank_time now;
+		u32 seq;
+
 		e = work->event;
-		e->event.sequence = drm_vblank_count_and_time(rdev->ddev, crtc_id, &now);
-		e->event.tv_sec = now.tv_sec;
-		e->event.tv_usec = now.tv_usec;
+		seq = drm_vblank_count_and_time(rdev->ddev, crtc_id, &now);
+		drm_set_event_seq_and_time(&e->event, e->timestamp_raw,
+					   seq, &now);
 		list_add_tail(&e->base.link, &e->base.file_priv->event_list);
 		wake_up_interruptible(&e->base.file_priv->event_wait);
 	}
diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c
index 8c593ea..f1825d2 100644
--- a/drivers/gpu/drm/radeon/radeon_drv.c
+++ b/drivers/gpu/drm/radeon/radeon_drv.c
@@ -84,7 +84,7 @@  int radeon_enable_vblank_kms(struct drm_device *dev, int crtc);
 void radeon_disable_vblank_kms(struct drm_device *dev, int crtc);
 int radeon_get_vblank_timestamp_kms(struct drm_device *dev, int crtc,
 				    int *max_error,
-				    struct timeval *vblank_time,
+				    struct drm_vblank_time *vblank_time,
 				    unsigned flags);
 void radeon_driver_irq_preinstall_kms(struct drm_device *dev);
 int radeon_driver_irq_postinstall_kms(struct drm_device *dev);
diff --git a/drivers/gpu/drm/radeon/radeon_kms.c b/drivers/gpu/drm/radeon/radeon_kms.c
index 414b4ac..3229989 100644
--- a/drivers/gpu/drm/radeon/radeon_kms.c
+++ b/drivers/gpu/drm/radeon/radeon_kms.c
@@ -564,7 +564,7 @@  void radeon_disable_vblank_kms(struct drm_device *dev, int crtc)
  */
 int radeon_get_vblank_timestamp_kms(struct drm_device *dev, int crtc,
 				    int *max_error,
-				    struct timeval *vblank_time,
+				    struct drm_vblank_time *vblank_time,
 				    unsigned flags)
 {
 	struct drm_crtc *drmcrtc;
diff --git a/drivers/gpu/drm/shmobile/shmob_drm_crtc.c b/drivers/gpu/drm/shmobile/shmob_drm_crtc.c
index 0e7a930..9eb451f 100644
--- a/drivers/gpu/drm/shmobile/shmob_drm_crtc.c
+++ b/drivers/gpu/drm/shmobile/shmob_drm_crtc.c
@@ -451,7 +451,8 @@  void shmob_drm_crtc_finish_page_flip(struct shmob_drm_crtc *scrtc)
 {
 	struct drm_pending_vblank_event *event;
 	struct drm_device *dev = scrtc->crtc.dev;
-	struct timeval vblanktime;
+	struct drm_vblank_time vblanktime;
+	u32 seq;
 	unsigned long flags;
 
 	spin_lock_irqsave(&dev->event_lock, flags);
@@ -462,9 +463,9 @@  void shmob_drm_crtc_finish_page_flip(struct shmob_drm_crtc *scrtc)
 	if (event == NULL)
 		return;
 
-	event->event.sequence = drm_vblank_count_and_time(dev, 0, &vblanktime);
-	event->event.tv_sec = vblanktime.tv_sec;
-	event->event.tv_usec = vblanktime.tv_usec;
+	seq = drm_vblank_count_and_time(dev, 0, &vblanktime);
+	drm_set_event_seq_and_time(&event->event, event->timestamp_raw,
+				   seq, &vblanktime);
 
 	spin_lock_irqsave(&dev->event_lock, flags);
 	list_add_tail(&event->base.link, &event->base.file_priv->event_list);
diff --git a/include/drm/drm.h b/include/drm/drm.h
index e51035a..3eb425c 100644
--- a/include/drm/drm.h
+++ b/include/drm/drm.h
@@ -465,6 +465,7 @@  enum drm_vblank_seq_type {
 	_DRM_VBLANK_RELATIVE = 0x1,	/**< Wait for given number of vblanks */
 	/* bits 1-6 are reserved for high crtcs */
 	_DRM_VBLANK_HIGH_CRTC_MASK = 0x0000003e,
+	_DRM_VBLANK_TIMESTAMP_RAW = 0x2000000,	/**< return raw monotonic time */
 	_DRM_VBLANK_EVENT = 0x4000000,   /**< Send event instead of blocking */
 	_DRM_VBLANK_FLIP = 0x8000000,   /**< Scheduled buffer swap should flip */
 	_DRM_VBLANK_NEXTONMISS = 0x10000000,	/**< If missed, wait for next vblank */
@@ -475,7 +476,8 @@  enum drm_vblank_seq_type {
 
 #define _DRM_VBLANK_TYPES_MASK (_DRM_VBLANK_ABSOLUTE | _DRM_VBLANK_RELATIVE)
 #define _DRM_VBLANK_FLAGS_MASK (_DRM_VBLANK_EVENT | _DRM_VBLANK_SIGNAL | \
-				_DRM_VBLANK_SECONDARY | _DRM_VBLANK_NEXTONMISS)
+				_DRM_VBLANK_SECONDARY | _DRM_VBLANK_NEXTONMISS | \
+				_DRM_VBLANK_TIMESTAMP_RAW)
 
 struct drm_wait_vblank_request {
 	enum drm_vblank_seq_type type;
@@ -778,6 +780,7 @@  struct drm_event_vblank {
 #define DRM_CAP_DUMB_PREFERRED_DEPTH 0x3
 #define DRM_CAP_DUMB_PREFER_SHADOW 0x4
 #define DRM_CAP_PRIME 0x5
+#define DRM_CAP_TIMESTAMP_RAW 0x6
 
 #define DRM_PRIME_CAP_IMPORT 0x1
 #define DRM_PRIME_CAP_EXPORT 0x2
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index d5f0c16..e2a6599 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -730,6 +730,11 @@  struct drm_bus {
 
 };
 
+struct drm_vblank_time {
+	struct timeval real;
+	struct timeval raw;
+};
+
 /**
  * DRM driver structure. This structure represent the common code for
  * a family of cards. There will one drm_device for each card present
@@ -866,7 +871,7 @@  struct drm_driver {
 	 */
 	int (*get_vblank_timestamp) (struct drm_device *dev, int crtc,
 				     int *max_error,
-				     struct timeval *vblank_time,
+				     struct drm_vblank_time *vblank_time,
 				     unsigned flags);
 
 	/* these have to be filled in */
@@ -1047,6 +1052,7 @@  struct drm_cmdline_mode {
 struct drm_pending_vblank_event {
 	struct drm_pending_event base;
 	int pipe;
+	bool timestamp_raw;
 	struct drm_event_vblank event;
 };
 
@@ -1132,7 +1138,14 @@  struct drm_device {
 
 	wait_queue_head_t *vbl_queue;   /**< VBLANK wait queue */
 	atomic_t *_vblank_count;        /**< number of VBLANK interrupts (driver must alloc the right number of counters) */
-	struct timeval *_vblank_time;   /**< timestamp of current vblank_count (drivers must alloc right number of fields) */
+
+	/*
+	 * timestamp of current vblank_count (drivers must alloc right number
+	 * of fields). We use a raw monotonic timestamp, but for backward
+	 * compatibility with clients depending on the wall time we also store
+	 * the GTOD value.
+	 */
+	struct drm_vblank_time *_vblank_time;
 	spinlock_t vblank_time_lock;    /**< Protects vblank count and time updates during vblank enable/disable */
 	spinlock_t vbl_lock;
 	atomic_t *vblank_refcount;      /* number of users of vblank interruptsper crtc */
@@ -1429,17 +1442,18 @@  extern int drm_wait_vblank(struct drm_device *dev, void *data,
 extern int drm_vblank_wait(struct drm_device *dev, unsigned int *vbl_seq);
 extern u32 drm_vblank_count(struct drm_device *dev, int crtc);
 extern u32 drm_vblank_count_and_time(struct drm_device *dev, int crtc,
-				     struct timeval *vblanktime);
+				     struct drm_vblank_time *vblanktime);
 extern bool drm_handle_vblank(struct drm_device *dev, int crtc);
 extern int drm_vblank_get(struct drm_device *dev, int crtc);
 extern void drm_vblank_put(struct drm_device *dev, int crtc);
 extern void drm_vblank_off(struct drm_device *dev, int crtc);
 extern void drm_vblank_cleanup(struct drm_device *dev);
 extern u32 drm_get_last_vbltimestamp(struct drm_device *dev, int crtc,
-				     struct timeval *tvblank, unsigned flags);
+				     struct drm_vblank_time *tvblank,
+				     unsigned flags);
 extern int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev,
 						 int crtc, int *max_error,
-						 struct timeval *vblank_time,
+						 struct drm_vblank_time *vblank_time,
 						 unsigned flags,
 						 struct drm_crtc *refcrtc);
 extern void drm_calc_timestamping_constants(struct drm_crtc *crtc);
@@ -1771,5 +1785,19 @@  static __inline__ bool drm_can_sleep(void)
 	return true;
 }
 
+static inline void drm_set_event_seq_and_time(struct drm_event_vblank *e,
+					      bool timestamp_raw, u32 seq,
+					      struct drm_vblank_time *time)
+{
+	e->sequence = seq;
+	if (timestamp_raw) {
+		e->tv_sec = time->raw.tv_sec;
+		e->tv_usec = time->raw.tv_usec;
+	} else {
+		e->tv_sec = time->real.tv_sec;
+		e->tv_usec = time->real.tv_usec;
+	}
+}
+
 #endif				/* __KERNEL__ */
 #endif
diff --git a/include/drm/drm_mode.h b/include/drm/drm_mode.h
index 3d6301b..b493011 100644
--- a/include/drm/drm_mode.h
+++ b/include/drm/drm_mode.h
@@ -399,7 +399,9 @@  struct drm_mode_crtc_lut {
 };
 
 #define DRM_MODE_PAGE_FLIP_EVENT 0x01
-#define DRM_MODE_PAGE_FLIP_FLAGS DRM_MODE_PAGE_FLIP_EVENT
+#define DRM_MODE_PAGE_FLIP_TIMESTAMP_RAW 0x02
+#define DRM_MODE_PAGE_FLIP_FLAGS (DRM_MODE_PAGE_FLIP_EVENT | \
+				  DRM_MODE_PAGE_FLIP_TIMESTAMP_RAW)
 
 /*
  * Request a page flip on the specified crtc.