dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [RFC 1/4] time: export getnstime_raw_and_real for DRM
  2012-10-05 13:36 [RFC 0/4] drm: add raw monotonic timestamp support Imre Deak
@ 2012-10-05 13:36 ` Imre Deak
  2012-10-05 16:14   ` Kristian Høgsberg
  0 siblings, 1 reply; 7+ messages in thread
From: Imre Deak @ 2012-10-05 13:36 UTC (permalink / raw)
  To: Daniel Vetter, Chris Wilson, Kristian Høgsberg; +Cc: intel-gfx, dri-devel

Needed by the upcoming DRM raw monotonic timestamp support.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 kernel/time/timekeeping.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index d3b91e7..073d262 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -365,7 +365,7 @@ void ktime_get_ts(struct timespec *ts)
 }
 EXPORT_SYMBOL_GPL(ktime_get_ts);
 
-#ifdef CONFIG_NTP_PPS
+#if defined(CONFIG_NTP_PPS) || defined(CONFIG_DRM) || defined(CONFIG_DRM_MODULE)
 
 /**
  * getnstime_raw_and_real - get day and raw monotonic time in timespec format
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [RFC 1/4] time: export getnstime_raw_and_real for DRM
  2012-10-05 13:36 ` [RFC 1/4] time: export getnstime_raw_and_real for DRM Imre Deak
@ 2012-10-05 16:14   ` Kristian Høgsberg
  2012-10-09 10:25     ` Imre Deak
  0 siblings, 1 reply; 7+ messages in thread
From: Kristian Høgsberg @ 2012-10-05 16:14 UTC (permalink / raw)
  To: Imre Deak; +Cc: Daniel Vetter, intel-gfx, dri-devel

On Fri, Oct 5, 2012 at 9:36 AM, Imre Deak <imre.deak@intel.com> wrote:
> Needed by the upcoming DRM raw monotonic timestamp support.

I just had a quick look at driver/input/evdev.c, since evdev devices
did a similar change recently to allow evdev timestamp from the
monotonic clock.  They're using a different time API:

	time_mono = ktime_get();
        time_real = ktime_sub(time_mono, ktime_get_monotonic_offset());

and

        event->time = ktime_to_timeval(client->clkid == CLOCK_MONOTONIC ?
			                mono : real);

I'm not really up-to-date on kernel time APIs, but I wonder if that
may be better?  At least, I suspect we wouldn't need changes outside
drm if we use this API.

Kristian


> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  kernel/time/timekeeping.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
> index d3b91e7..073d262 100644
> --- a/kernel/time/timekeeping.c
> +++ b/kernel/time/timekeeping.c
> @@ -365,7 +365,7 @@ void ktime_get_ts(struct timespec *ts)
>  }
>  EXPORT_SYMBOL_GPL(ktime_get_ts);
>
> -#ifdef CONFIG_NTP_PPS
> +#if defined(CONFIG_NTP_PPS) || defined(CONFIG_DRM) || defined(CONFIG_DRM_MODULE)
>
>  /**
>   * getnstime_raw_and_real - get day and raw monotonic time in timespec format
> --
> 1.7.9.5
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC 1/4] time: export getnstime_raw_and_real for DRM
       [not found] <mailman.20114.1349454139.16659.intel-gfx@lists.freedesktop.org>
@ 2012-10-06  1:41 ` Mario Kleiner
  2012-10-08 11:35   ` Imre Deak
  0 siblings, 1 reply; 7+ messages in thread
From: Mario Kleiner @ 2012-10-06  1:41 UTC (permalink / raw)
  Cc: Daniel Vetter, dri-devel, michel, mario.kleiner, intel-gfx


> Subject: Re: [RFC 4/4] drm: add support for raw monotonic vblank
> 	timestamps
> Message-ID: <1349446447.17758.73.camel@thor.local>
> Content-Type: text/plain; charset="ISO-8859-1"
>
> 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).

Hi

The OML_sync_control spec requires monotonic timestamps, so yes, it was 
either a mistake or it wasn't possible at that time.

If an app developer follows the spec, he would consider the current 
behaviour broken -- or should be able to handle monotonic timestamps.

>>
>> 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?
>

Psychtoolbox as my example of a very timing sensitive app: Currently 
uses wall time as a reference for historical reasons. We tell/expect 
users not to do anything in the middle of a work session that could 
cause a sudden jump in wall time, but then ntp time adjustments are 
usually in very small steps, so not a problem, and big adjustments 
should only happen if users do something they'll certainly not do during 
a timing-sensitve session, like manually set the system clock, 
reboot/suspend/resume the machine etc. In practice, this works well 
enough over the typical duration of such sessions (minutes to a few 
hours) and is sometimes needed if multiple machines need to be 
time-synced over the local network.

But then Psychtoolbox checks each timestamp it gets from somewhere 
"outside" (OML_sync_control / INTEL_swap_events / ALSA audio timestamps, 
network receive timestamps, evdev, x11, ...) if it is in gettimeofday() 
aka CLOCK_REALTIME aka wall time or in CLOCK_MONOTONIC time and just 
remaps to whatever its reference clock is.

There's no way around this than to have no fixed expectations, but to 
remap stuff on the fly, because different parts of the Linux universe 
have decided on different time bases, or even switched somewhere from 
one kernel version to the next in the last years, e.g., ALSA, which at 
some time switched from wall clock to CLOCK_MONOTONIC. Sometimes 
clock_gettime() wasn't available at all in older setups, so there only 
was gettimeofday(). Or toolkits like GStreamer use different timebases 
dependent on OS and sometimes even on plugins.

I would expect that other timing sensitive apps have to have ways to 
handle this in similar ways.

Wrt. to the drm vblank/pageflip timestamps, the userspace extensions 
which expose these (INTEL_swap_events, OML_sync_control) don't allow 
apps to select which timebase to use, they define monotonic time as what 
is returned, so i don't know how a userspace app could actually ask the 
DRM for one or the other format? So i guess just switching to 
CLOCK_MONOTONIC shouldn't be that bad.

Kristian, i assume Wayland will also return presentation timestamps in 
the format and microsecond precision of the DRM, right?

On 05.10.12 18:22, intel-gfx-request@lists.freedesktop.org wrote:
> Message: 7 Date: Fri, 5 Oct 2012 12:14:29 -0400 From: Kristian H?gsberg

...
> I just had a quick look at driver/input/evdev.c, since evdev devices
> did a similar change recently to allow evdev timestamp from the
> monotonic clock.  They're using a different time API:
>
> 	time_mono = ktime_get();
>          time_real = ktime_sub(time_mono, ktime_get_monotonic_offset());
>
> and
>
>          event->time = ktime_to_timeval(client->clkid == CLOCK_MONOTONIC ?
> 			                mono : real);
>
> I'm not really up-to-date on kernel time APIs, but I wonder if that
> may be better?  At least, I suspect we wouldn't need changes outside
> drm if we use this API.
>
> Kristian

Userspace apps only have access to what gettimeofday() and 
clock_gettime() for CLOCK_REALTIME (== gettimeofday() afaik) and 
CLOCK_MONOTONIC return, so whatever is returned should be in 
CLOCK_MONOTONIC format, otherwise there will be lots of tears and dead 
kittens. I think what evdev does makes a lot of sense, but i'm also not 
up-to-date about the various layers of timing apis.

-mario

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC 1/4] time: export getnstime_raw_and_real for DRM
  2012-10-06  1:41 ` [RFC 1/4] time: export getnstime_raw_and_real for DRM Mario Kleiner
@ 2012-10-08 11:35   ` Imre Deak
  2012-10-08 23:00     ` Mario Kleiner
  0 siblings, 1 reply; 7+ messages in thread
From: Imre Deak @ 2012-10-08 11:35 UTC (permalink / raw)
  To: Mario Kleiner; +Cc: Daniel Vetter, intel-gfx, michel, dri-devel

On Sat, 2012-10-06 at 03:41 +0200, Mario Kleiner wrote:
> [...]
> 
> But then Psychtoolbox checks each timestamp it gets from somewhere 
> "outside" (OML_sync_control / INTEL_swap_events / ALSA audio timestamps, 
> network receive timestamps, evdev, x11, ...) if it is in gettimeofday() 
> aka CLOCK_REALTIME aka wall time or in CLOCK_MONOTONIC time and just 
> remaps to whatever its reference clock is.
> 
> There's no way around this than to have no fixed expectations, but to 
> remap stuff on the fly, because different parts of the Linux universe 
> have decided on different time bases, or even switched somewhere from 
> one kernel version to the next in the last years, e.g., ALSA, which at 
> some time switched from wall clock to CLOCK_MONOTONIC. Sometimes 
> clock_gettime() wasn't available at all in older setups, so there only 
> was gettimeofday(). Or toolkits like GStreamer use different timebases 
> dependent on OS and sometimes even on plugins.
> 
> I would expect that other timing sensitive apps have to have ways to 
> handle this in similar ways.

I think the question is, can we be sure? I don't think there is any
guarantee that random application X will not be confused by an
unconditional switch to monotonic timestamps.

> Wrt. to the drm vblank/pageflip timestamps, the userspace extensions 
> which expose these (INTEL_swap_events, OML_sync_control) don't allow 
> apps to select which timebase to use, they define monotonic time as what 
> is returned, so i don't know how a userspace app could actually ask the 
> DRM for one or the other format? So i guess just switching to 
> CLOCK_MONOTONIC shouldn't be that bad.

An application could just use the kernel DRM interface directly. I admit
this is not very likely but this is what should determine the rules by
which we change the ABI. 

> Kristian, i assume Wayland will also return presentation timestamps in 
> the format and microsecond precision of the DRM, right?
> 
> On 05.10.12 18:22, intel-gfx-request@lists.freedesktop.org wrote:
> > Message: 7 Date: Fri, 5 Oct 2012 12:14:29 -0400 From: Kristian H?gsberg
> 
> ...
> > I just had a quick look at driver/input/evdev.c, since evdev devices
> > did a similar change recently to allow evdev timestamp from the
> > monotonic clock.  They're using a different time API:
> >
> > 	time_mono = ktime_get();
> >          time_real = ktime_sub(time_mono, ktime_get_monotonic_offset());
> >
> > and
> >
> >          event->time = ktime_to_timeval(client->clkid == CLOCK_MONOTONIC ?
> > 			                mono : real);
> >
> > I'm not really up-to-date on kernel time APIs, but I wonder if that
> > may be better?  At least, I suspect we wouldn't need changes outside
> > drm if we use this API.
> >
> > Kristian
> 
> Userspace apps only have access to what gettimeofday() and 
> clock_gettime() for CLOCK_REALTIME (== gettimeofday() afaik) and 
> CLOCK_MONOTONIC return, so whatever is returned should be in 
> CLOCK_MONOTONIC format, otherwise there will be lots of tears and dead 
> kittens. I think what evdev does makes a lot of sense, but i'm also not 
> up-to-date about the various layers of timing apis.

Yes, this should be the case, regardless of which kernel interface we
decide to use.

--Imre

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC 1/4] time: export getnstime_raw_and_real for DRM
  2012-10-08 11:35   ` Imre Deak
@ 2012-10-08 23:00     ` Mario Kleiner
  2012-10-23 11:50       ` Imre Deak
  0 siblings, 1 reply; 7+ messages in thread
From: Mario Kleiner @ 2012-10-08 23:00 UTC (permalink / raw)
  To: Imre Deak; +Cc: Daniel Vetter, intel-gfx, michel, dri-devel

On 08.10.12 13:35, Imre Deak wrote:
> On Sat, 2012-10-06 at 03:41 +0200, Mario Kleiner wrote:
>> [...]
>>
>> But then Psychtoolbox checks each timestamp it gets from somewhere
>> "outside" (OML_sync_control / INTEL_swap_events / ALSA audio timestamps,
>> network receive timestamps, evdev, x11, ...) if it is in gettimeofday()
>> aka CLOCK_REALTIME aka wall time or in CLOCK_MONOTONIC time and just
>> remaps to whatever its reference clock is.
>>
>> There's no way around this than to have no fixed expectations, but to
>> remap stuff on the fly, because different parts of the Linux universe
>> have decided on different time bases, or even switched somewhere from
>> one kernel version to the next in the last years, e.g., ALSA, which at
>> some time switched from wall clock to CLOCK_MONOTONIC. Sometimes
>> clock_gettime() wasn't available at all in older setups, so there only
>> was gettimeofday(). Or toolkits like GStreamer use different timebases
>> dependent on OS and sometimes even on plugins.
>>
>> I would expect that other timing sensitive apps have to have ways to
>> handle this in similar ways.
>
> I think the question is, can we be sure? I don't think there is any
> guarantee that random application X will not be confused by an
> unconditional switch to monotonic timestamps.
>
>> Wrt. to the drm vblank/pageflip timestamps, the userspace extensions
>> which expose these (INTEL_swap_events, OML_sync_control) don't allow
>> apps to select which timebase to use, they define monotonic time as what
>> is returned, so i don't know how a userspace app could actually ask the
>> DRM for one or the other format? So i guess just switching to
>> CLOCK_MONOTONIC shouldn't be that bad.
>
> An application could just use the kernel DRM interface directly. I admit
> this is not very likely but this is what should determine the rules by
> which we change the ABI.
>

Ok, that's a good point, agreed.
-mario

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC 1/4] time: export getnstime_raw_and_real for DRM
  2012-10-05 16:14   ` Kristian Høgsberg
@ 2012-10-09 10:25     ` Imre Deak
  0 siblings, 0 replies; 7+ messages in thread
From: Imre Deak @ 2012-10-09 10:25 UTC (permalink / raw)
  To: Kristian Høgsberg; +Cc: Daniel Vetter, intel-gfx, dri-devel

On Fri, 2012-10-05 at 12:14 -0400, Kristian Høgsberg wrote:
> On Fri, Oct 5, 2012 at 9:36 AM, Imre Deak <imre.deak@intel.com> wrote:
> > Needed by the upcoming DRM raw monotonic timestamp support.
> 
> I just had a quick look at driver/input/evdev.c, since evdev devices
> did a similar change recently to allow evdev timestamp from the
> monotonic clock.  They're using a different time API:
> 
> 	time_mono = ktime_get();
>         time_real = ktime_sub(time_mono, ktime_get_monotonic_offset());
> 
> and
> 
>         event->time = ktime_to_timeval(client->clkid == CLOCK_MONOTONIC ?
> 			                mono : real);
> 
> I'm not really up-to-date on kernel time APIs, but I wonder if that
> may be better?  At least, I suspect we wouldn't need changes outside
> drm if we use this API.


Thanks, I will use this instead of getnstime_raw_and_real.

The reason is as discussed on #intel-gfx that this provides a monotonic
vs. raw monotonic time and that as you say it won't require a change in
the time keeping code.

--Imre

> 
> Kristian
> 
> 
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  kernel/time/timekeeping.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
> > index d3b91e7..073d262 100644
> > --- a/kernel/time/timekeeping.c
> > +++ b/kernel/time/timekeeping.c
> > @@ -365,7 +365,7 @@ void ktime_get_ts(struct timespec *ts)
> >  }
> >  EXPORT_SYMBOL_GPL(ktime_get_ts);
> >
> > -#ifdef CONFIG_NTP_PPS
> > +#if defined(CONFIG_NTP_PPS) || defined(CONFIG_DRM) || defined(CONFIG_DRM_MODULE)
> >
> >  /**
> >   * getnstime_raw_and_real - get day and raw monotonic time in timespec format
> > --
> > 1.7.9.5
> >


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC 1/4] time: export getnstime_raw_and_real for DRM
  2012-10-08 23:00     ` Mario Kleiner
@ 2012-10-23 11:50       ` Imre Deak
  0 siblings, 0 replies; 7+ messages in thread
From: Imre Deak @ 2012-10-23 11:50 UTC (permalink / raw)
  To: Mario Kleiner; +Cc: Daniel Vetter, intel-gfx, michel, dri-devel

On Tue, 2012-10-09 at 01:00 +0200, Mario Kleiner wrote:
> On 08.10.12 13:35, Imre Deak wrote:
> > On Sat, 2012-10-06 at 03:41 +0200, Mario Kleiner wrote:
> >> [...]
> >>
> >> But then Psychtoolbox checks each timestamp it gets from somewhere
> >> "outside" (OML_sync_control / INTEL_swap_events / ALSA audio timestamps,
> >> network receive timestamps, evdev, x11, ...) if it is in gettimeofday()
> >> aka CLOCK_REALTIME aka wall time or in CLOCK_MONOTONIC time and just
> >> remaps to whatever its reference clock is.
> >>
> >> There's no way around this than to have no fixed expectations, but to
> >> remap stuff on the fly, because different parts of the Linux universe
> >> have decided on different time bases, or even switched somewhere from
> >> one kernel version to the next in the last years, e.g., ALSA, which at
> >> some time switched from wall clock to CLOCK_MONOTONIC. Sometimes
> >> clock_gettime() wasn't available at all in older setups, so there only
> >> was gettimeofday(). Or toolkits like GStreamer use different timebases
> >> dependent on OS and sometimes even on plugins.
> >>
> >> I would expect that other timing sensitive apps have to have ways to
> >> handle this in similar ways.
> >
> > I think the question is, can we be sure? I don't think there is any
> > guarantee that random application X will not be confused by an
> > unconditional switch to monotonic timestamps.
> >
> >> Wrt. to the drm vblank/pageflip timestamps, the userspace extensions
> >> which expose these (INTEL_swap_events, OML_sync_control) don't allow
> >> apps to select which timebase to use, they define monotonic time as what
> >> is returned, so i don't know how a userspace app could actually ask the
> >> DRM for one or the other format? So i guess just switching to
> >> CLOCK_MONOTONIC shouldn't be that bad.
> >
> > An application could just use the kernel DRM interface directly. I admit
> > this is not very likely but this is what should determine the rules by
> > which we change the ABI.
> >
> Ok, that's a good point, agreed.

Still pondering about this, since I wouldn't want to complicate things
unnecessarily by storing both timestamps.

Based on the feedback you guys gave, I now also think we could simply
switch over to monotonic timestamps and have a module flag to fall back
to the old behavior. This is what ALSA does and what V4L2 people are
planning to do as well. As you pointed out the mainstream users of this
interface already expect a monotonic timestamp and I couldn't find
either a good reason to support both formats at the same time.

--Imre

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2012-10-23 11:50 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <mailman.20114.1349454139.16659.intel-gfx@lists.freedesktop.org>
2012-10-06  1:41 ` [RFC 1/4] time: export getnstime_raw_and_real for DRM Mario Kleiner
2012-10-08 11:35   ` Imre Deak
2012-10-08 23:00     ` Mario Kleiner
2012-10-23 11:50       ` Imre Deak
2012-10-05 13:36 [RFC 0/4] drm: add raw monotonic timestamp support Imre Deak
2012-10-05 13:36 ` [RFC 1/4] time: export getnstime_raw_and_real for DRM Imre Deak
2012-10-05 16:14   ` Kristian Høgsberg
2012-10-09 10:25     ` Imre Deak

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).