[Intel-gfx] [RFC 4/4] drm: add support for raw monotonic vblank timestamps
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
---
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(-)
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
Re: [Intel-gfx] [RFC 4/4] drm: add support for raw monotonic vblank timestamps
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 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC 4/4] drm: add support for raw monotonic vblank timestamps
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 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC 4/4] drm: add support for raw monotonic vblank timestamps
On Fri, 2012-10-05 at 16:18 -0600, Rob Clark wrote:
> On Fri, Oct 5, 2012 at 7:37 AM, Imre Deak 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);
> > }
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC 4/4] drm: add support for raw monotonic vblank timestamps
On Fri, 2012-10-05 at 18:09 -0600, Rob Clark wrote:
> On Fri, Oct 5, 2012 at 5:41 PM, Imre Deak wrote:
> > On Fri, 2012-10-05 at 16:18 -0600, Rob Clark wrote:
> >> On Fri, Oct 5, 2012 at 7:37 AM, Imre Deak 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
Re: [Intel-gfx] [RFC 4/4] drm: add support for raw monotonic vblank timestamps
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). -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Debian, X and DRI developer ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC 4/4] drm: add support for raw monotonic vblank timestamps
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? -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Debian, X and DRI developer ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC 4/4] drm: add support for raw monotonic vblank timestamps
On Fri, Oct 5, 2012 at 7:37 AM, Imre Deak 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);
> }
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC 4/4] drm: add support for raw monotonic vblank timestamps
On Fri, Oct 5, 2012 at 5:41 PM, Imre Deak wrote:
> On Fri, 2012-10-05 at 16:18 -0600, Rob Clark wrote:
>> On Fri, Oct 5, 2012 at 7:37 AM, Imre Deak 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
Re: [Intel-gfx] [RFC 4/4] drm: add support for raw monotonic vblank timestamps
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
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
