Re: [PATCH] ALSA: Provide a CLOCK_MONOTONIC_RAW timestamp type

[Takashi Iwai <tiwai@suse.de>]

At Tue,  8 Jul 2014 16:51:49 +0200,
Mark Brown wrote:
> 
> From: Mark Brown <broonie@linaro.org>
> 
> For applications which need to synchronise with external timebases such
> as broadcast TV applications the kernel monotonic time is not optimal as
> it includes adjustments from NTP and so may still include discontinuities
> due to that. A raw monotonic time which does not include any adjustments
> is available in the kernel from getrawmonotonic() so provide userspace with
> a new timestamp type SNDRV_PCM_TSTAMP_TYPE_MONOTONIC_RAW which provides
> timestamps based on this as an option.
> 
> Reported-by: Daniel Thompson <daniel.thompson@linaro.org>
> Signed-off-by: Mark Brown <broonie@linaro.org>
> ---
> 
> Mostly an RFC at this point as I don't have hardware to test this week.

This looks good to me.  Or, thinking of the original use case, should
we just replace ktime_get_ts() with getrawmonotonic()?  The MONOTONIC
mode is really to be monotonic.

While we're at it: there's been long-standing request from V4L guys
for improving the timestamps to sync with video streams.  I suggested
to add a new mode that can be set by the driver.  Worth to consider
it, too?


thanks,

Takashi


>  
>  include/sound/pcm.h         | 11 +++++++++--
>  include/uapi/sound/asound.h |  3 ++-
>  sound/core/pcm_native.c     | 12 +++++++++---
>  3 files changed, 20 insertions(+), 6 deletions(-)
> 
> diff --git a/include/sound/pcm.h b/include/sound/pcm.h
> index d854fb3..6f3e10c 100644
> --- a/include/sound/pcm.h
> +++ b/include/sound/pcm.h
> @@ -931,10 +931,17 @@ void snd_pcm_timer_done(struct snd_pcm_substream *substream);
>  static inline void snd_pcm_gettime(struct snd_pcm_runtime *runtime,
>  				   struct timespec *tv)
>  {
> -	if (runtime->tstamp_type == SNDRV_PCM_TSTAMP_TYPE_MONOTONIC)
> +	switch (runtime->tstamp_type) {
> +	case SNDRV_PCM_TSTAMP_TYPE_MONOTONIC:
>  		ktime_get_ts(tv);
> -	else
> +		break;
> +	case SNDRV_PCM_TSTAMP_TYPE_MONOTONIC_RAW:
> +		getrawmonotonic(tv);
> +		break;
> +	default:
>  		getnstimeofday(tv);
> +		break;
> +	}
>  }
>  
>  /*
> diff --git a/include/uapi/sound/asound.h b/include/uapi/sound/asound.h
> index 2249483..cbf7dc85 100644
> --- a/include/uapi/sound/asound.h
> +++ b/include/uapi/sound/asound.h
> @@ -462,7 +462,8 @@ struct snd_xfern {
>  enum {
>  	SNDRV_PCM_TSTAMP_TYPE_GETTIMEOFDAY = 0,	/* gettimeofday equivalent */
>  	SNDRV_PCM_TSTAMP_TYPE_MONOTONIC,	/* posix_clock_monotonic equivalent */
> -	SNDRV_PCM_TSTAMP_TYPE_LAST = SNDRV_PCM_TSTAMP_TYPE_MONOTONIC,
> +	SNDRV_PCM_TSTAMP_TYPE_MONOTONIC_RAW,    /* monotonic_raw (no NTP) */
> +	SNDRV_PCM_TSTAMP_TYPE_LAST = SNDRV_PCM_TSTAMP_TYPE_MONOTONIC_RAW,
>  };
>  
>  /* channel positions */
> diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
> index b653ab0..fd6d7a7 100644
> --- a/sound/core/pcm_native.c
> +++ b/sound/core/pcm_native.c
> @@ -2540,9 +2540,15 @@ static int snd_pcm_tstamp(struct snd_pcm_substream *substream, int __user *_arg)
>  		return -EFAULT;
>  	if (arg < 0 || arg > SNDRV_PCM_TSTAMP_TYPE_LAST)
>  		return -EINVAL;
> -	runtime->tstamp_type = SNDRV_PCM_TSTAMP_TYPE_GETTIMEOFDAY;
> -	if (arg == SNDRV_PCM_TSTAMP_TYPE_MONOTONIC)
> -		runtime->tstamp_type = SNDRV_PCM_TSTAMP_TYPE_MONOTONIC;
> +	switch (runtime->tstamp_type) {
> +	case SNDRV_PCM_TSTAMP_TYPE_MONOTONIC:
> +	case SNDRV_PCM_TSTAMP_TYPE_MONOTONIC_RAW:
> +		runtime->tstamp_type = runtime->tstamp_type;
> +		break;
> +	default:
> +		runtime->tstamp_type = SNDRV_PCM_TSTAMP_TYPE_GETTIMEOFDAY;
> +		break;
> +	}
>  	return 0;
>  }
>  		
> -- 
> 2.0.0
>