[PATCH 0/3] Revised patch(es) for CLOCK_MONOTONIC_RAW timestamp

[PATCH 0/3] Revised patch(es) for CLOCK_MONOTONIC_RAW timestamp

[Takashi Iwai]

Hi,

here is a series of revised kernel patches for adding
CLOCK_MONOTONIC_RAW timestamp type.  In addition to Mark's original
patch, the new field is added to sw_params.

The corresponding alsa-lib patches will follow after these.


Takashi

[PATCH 1/3] ALSA: pcm: simplify snd_pcm_tstamp()

[Takashi Iwai]

No functional change.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/core/pcm_native.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index b653ab001fba..2372c49a8e84 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -2540,9 +2540,7 @@ 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;
+	runtime->tstamp_type = arg;
 	return 0;
 }
 		
-- 
2.0.1

[PATCH 2/3] ALSA: Provide a CLOCK_MONOTONIC_RAW timestamp type

[Takashi Iwai]

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.

[dropped tstamp_type assignment code, as it's no longer needed -- tiwai]

Reported-by: Daniel Thompson <daniel.thompson@linaro.org>
Signed-off-by: Mark Brown <broonie@linaro.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 include/sound/pcm.h         | 11 +++++++++--
 include/uapi/sound/asound.h |  3 ++-
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/include/sound/pcm.h b/include/sound/pcm.h
index d854fb31c000..6f3e10ca0e32 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 224948342f14..cbf7dc850a46 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 */
-- 
2.0.1

[PATCH 3/3] ALSA: pcm: Add timestamp type to sw_params

[Takashi Iwai]

For allowing adjusting the timestamp type on the fly, add it to
sw_params.  The existing ioctl is still kept for compatibility.

Along with this, increment the PCM protocol version.

The extension was suggested by Clemens Ladisch.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 include/uapi/sound/asound.h | 6 ++++--
 sound/core/pcm_native.c     | 3 +++
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/include/uapi/sound/asound.h b/include/uapi/sound/asound.h
index cbf7dc850a46..a7e062f91f39 100644
--- a/include/uapi/sound/asound.h
+++ b/include/uapi/sound/asound.h
@@ -139,7 +139,7 @@ struct snd_hwdep_dsp_image {
  *                                                                           *
  *****************************************************************************/
 
-#define SNDRV_PCM_VERSION		SNDRV_PROTOCOL_VERSION(2, 0, 11)
+#define SNDRV_PCM_VERSION		SNDRV_PROTOCOL_VERSION(2, 0, 12)
 
 typedef unsigned long snd_pcm_uframes_t;
 typedef signed long snd_pcm_sframes_t;
@@ -391,7 +391,9 @@ struct snd_pcm_sw_params {
 	snd_pcm_uframes_t silence_threshold;	/* min distance from noise for silence filling */
 	snd_pcm_uframes_t silence_size;		/* silence block size */
 	snd_pcm_uframes_t boundary;		/* pointers wrap point */
-	unsigned char reserved[64];		/* reserved for future */
+	unsigned int tstamp_type;		/* timestamp type */
+	int pads;				/* alignment, reserved */
+	unsigned char reserved[56];		/* reserved for future */
 };
 
 struct snd_pcm_channel_info {
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index 2372c49a8e84..81dedc381efd 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -543,6 +543,8 @@ static int snd_pcm_sw_params(struct snd_pcm_substream *substream,
 
 	if (params->tstamp_mode > SNDRV_PCM_TSTAMP_LAST)
 		return -EINVAL;
+	if (params->tstamp_type > SNDRV_PCM_TSTAMP_TYPE_LAST)
+		return -EINVAL;
 	if (params->avail_min == 0)
 		return -EINVAL;
 	if (params->silence_size >= runtime->boundary) {
@@ -557,6 +559,7 @@ static int snd_pcm_sw_params(struct snd_pcm_substream *substream,
 	err = 0;
 	snd_pcm_stream_lock_irq(substream);
 	runtime->tstamp_mode = params->tstamp_mode;
+	runtime->tstamp_type = params->tstamp_type;
 	runtime->period_step = params->period_step;
 	runtime->control->avail_min = params->avail_min;
 	runtime->start_threshold = params->start_threshold;
-- 
2.0.1

Re: [PATCH 3/3] ALSA: pcm: Add timestamp type to sw_params

[Pierre-Louis Bossart]

On 7/10/14, 8:04 AM, Takashi Iwai wrote:
> For allowing adjusting the timestamp type on the fly, add it to
> sw_params.  The existing ioctl is still kept for compatibility.

I have strong objections to this extension. It will result in a 
discontinuity in the timestamps reported in pcm_status without a clear 
indication of what timestamp is reported (when does this change occur?), 
and it's completely unclear how userspace would handle a step (positive 
or negative) between ntp-adjusted and non-ntp-adjusted times. For apps 
that make use of the audio_time reported with a wall clock this could 
lead to complete nonsense.
I would have no problems if this was a fixed parameter defined once 
before audio streaming starts.

>
> Along with this, increment the PCM protocol version.
>
> The extension was suggested by Clemens Ladisch.
>
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>   include/uapi/sound/asound.h | 6 ++++--
>   sound/core/pcm_native.c     | 3 +++
>   2 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/include/uapi/sound/asound.h b/include/uapi/sound/asound.h
> index cbf7dc850a46..a7e062f91f39 100644
> --- a/include/uapi/sound/asound.h
> +++ b/include/uapi/sound/asound.h
> @@ -139,7 +139,7 @@ struct snd_hwdep_dsp_image {
>    *                                                                           *
>    *****************************************************************************/
>
> -#define SNDRV_PCM_VERSION		SNDRV_PROTOCOL_VERSION(2, 0, 11)
> +#define SNDRV_PCM_VERSION		SNDRV_PROTOCOL_VERSION(2, 0, 12)
>
>   typedef unsigned long snd_pcm_uframes_t;
>   typedef signed long snd_pcm_sframes_t;
> @@ -391,7 +391,9 @@ struct snd_pcm_sw_params {
>   	snd_pcm_uframes_t silence_threshold;	/* min distance from noise for silence filling */
>   	snd_pcm_uframes_t silence_size;		/* silence block size */
>   	snd_pcm_uframes_t boundary;		/* pointers wrap point */
> -	unsigned char reserved[64];		/* reserved for future */
> +	unsigned int tstamp_type;		/* timestamp type */
> +	int pads;				/* alignment, reserved */
> +	unsigned char reserved[56];		/* reserved for future */
>   };
>
>   struct snd_pcm_channel_info {
> diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
> index 2372c49a8e84..81dedc381efd 100644
> --- a/sound/core/pcm_native.c
> +++ b/sound/core/pcm_native.c
> @@ -543,6 +543,8 @@ static int snd_pcm_sw_params(struct snd_pcm_substream *substream,
>
>   	if (params->tstamp_mode > SNDRV_PCM_TSTAMP_LAST)
>   		return -EINVAL;
> +	if (params->tstamp_type > SNDRV_PCM_TSTAMP_TYPE_LAST)
> +		return -EINVAL;
>   	if (params->avail_min == 0)
>   		return -EINVAL;
>   	if (params->silence_size >= runtime->boundary) {
> @@ -557,6 +559,7 @@ static int snd_pcm_sw_params(struct snd_pcm_substream *substream,
>   	err = 0;
>   	snd_pcm_stream_lock_irq(substream);
>   	runtime->tstamp_mode = params->tstamp_mode;
> +	runtime->tstamp_type = params->tstamp_type;
>   	runtime->period_step = params->period_step;
>   	runtime->control->avail_min = params->avail_min;
>   	runtime->start_threshold = params->start_threshold;
>

Re: [PATCH 3/3] ALSA: pcm: Add timestamp type to sw_params

[Takashi Iwai]

At Thu, 10 Jul 2014 10:23:37 -0500,
Pierre-Louis Bossart wrote:
> 
> On 7/10/14, 8:04 AM, Takashi Iwai wrote:
> > For allowing adjusting the timestamp type on the fly, add it to
> > sw_params.  The existing ioctl is still kept for compatibility.
> 
> I have strong objections to this extension. It will result in a 
> discontinuity in the timestamps reported in pcm_status without a clear 
> indication of what timestamp is reported (when does this change occur?), 
> and it's completely unclear how userspace would handle a step (positive 
> or negative) between ntp-adjusted and non-ntp-adjusted times.

Well, I don't understand the logic; it's app itself who changes the
timestamp type.  It must know when it's changed (because app is
changing it), and how to handle (because app chooses the timestamp
type).  And the current type can be queried via sw_params_*_get()
thing.

Of course, as default, there is no change from the current behavior --
it takes CLOCK_MONOTONIC.  So, there should be no breakage by this
change unless you change the application side to use the new call.

> For apps 
> that make use of the audio_time reported with a wall clock this could 
> lead to complete nonsense.
> I would have no problems if this was a fixed parameter defined once 
> before audio streaming starts.

You don't have to change the setup after the stream starts.  It's
usually set before the stream starting.  The point of using sw_params
is that it's independent from the hardware driver, thus it fits better
than hw_params.  The switching after stream isn't the purpose.  It's
just a gratis bonus.


Takashi

> > Along with this, increment the PCM protocol version.
> >
> > The extension was suggested by Clemens Ladisch.
> >
> > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> > ---
> >   include/uapi/sound/asound.h | 6 ++++--
> >   sound/core/pcm_native.c     | 3 +++
> >   2 files changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/uapi/sound/asound.h b/include/uapi/sound/asound.h
> > index cbf7dc850a46..a7e062f91f39 100644
> > --- a/include/uapi/sound/asound.h
> > +++ b/include/uapi/sound/asound.h
> > @@ -139,7 +139,7 @@ struct snd_hwdep_dsp_image {
> >    *                                                                           *
> >    *****************************************************************************/
> >
> > -#define SNDRV_PCM_VERSION		SNDRV_PROTOCOL_VERSION(2, 0, 11)
> > +#define SNDRV_PCM_VERSION		SNDRV_PROTOCOL_VERSION(2, 0, 12)
> >
> >   typedef unsigned long snd_pcm_uframes_t;
> >   typedef signed long snd_pcm_sframes_t;
> > @@ -391,7 +391,9 @@ struct snd_pcm_sw_params {
> >   	snd_pcm_uframes_t silence_threshold;	/* min distance from noise for silence filling */
> >   	snd_pcm_uframes_t silence_size;		/* silence block size */
> >   	snd_pcm_uframes_t boundary;		/* pointers wrap point */
> > -	unsigned char reserved[64];		/* reserved for future */
> > +	unsigned int tstamp_type;		/* timestamp type */
> > +	int pads;				/* alignment, reserved */
> > +	unsigned char reserved[56];		/* reserved for future */
> >   };
> >
> >   struct snd_pcm_channel_info {
> > diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
> > index 2372c49a8e84..81dedc381efd 100644
> > --- a/sound/core/pcm_native.c
> > +++ b/sound/core/pcm_native.c
> > @@ -543,6 +543,8 @@ static int snd_pcm_sw_params(struct snd_pcm_substream *substream,
> >
> >   	if (params->tstamp_mode > SNDRV_PCM_TSTAMP_LAST)
> >   		return -EINVAL;
> > +	if (params->tstamp_type > SNDRV_PCM_TSTAMP_TYPE_LAST)
> > +		return -EINVAL;
> >   	if (params->avail_min == 0)
> >   		return -EINVAL;
> >   	if (params->silence_size >= runtime->boundary) {
> > @@ -557,6 +559,7 @@ static int snd_pcm_sw_params(struct snd_pcm_substream *substream,
> >   	err = 0;
> >   	snd_pcm_stream_lock_irq(substream);
> >   	runtime->tstamp_mode = params->tstamp_mode;
> > +	runtime->tstamp_type = params->tstamp_type;
> >   	runtime->period_step = params->period_step;
> >   	runtime->control->avail_min = params->avail_min;
> >   	runtime->start_threshold = params->start_threshold;
> >
> 

Re: [PATCH 3/3] ALSA: pcm: Add timestamp type to sw_params

[Pierre-Louis Bossart]

On 7/10/14, 10:42 AM, Takashi Iwai wrote:
> At Thu, 10 Jul 2014 10:23:37 -0500,
> Pierre-Louis Bossart wrote:
>>
>> On 7/10/14, 8:04 AM, Takashi Iwai wrote:
>>> For allowing adjusting the timestamp type on the fly, add it to
>>> sw_params.  The existing ioctl is still kept for compatibility.
>>
>> I have strong objections to this extension. It will result in a
>> discontinuity in the timestamps reported in pcm_status without a clear
>> indication of what timestamp is reported (when does this change occur?),
>> and it's completely unclear how userspace would handle a step (positive
>> or negative) between ntp-adjusted and non-ntp-adjusted times.
>
> Well, I don't understand the logic; it's app itself who changes the
> timestamp type.  It must know when it's changed (because app is
> changing it), and how to handle (because app chooses the timestamp
> type).  And the current type can be queried via sw_params_*_get()
> thing.
>
> Of course, as default, there is no change from the current behavior --
> it takes CLOCK_MONOTONIC.  So, there should be no breakage by this
> change unless you change the application side to use the new call.
>
>> For apps
>> that make use of the audio_time reported with a wall clock this could
>> lead to complete nonsense.
>> I would have no problems if this was a fixed parameter defined once
>> before audio streaming starts.
>
> You don't have to change the setup after the stream starts.  It's
> usually set before the stream starting.  The point of using sw_params
> is that it's independent from the hardware driver, thus it fits better
> than hw_params.  The switching after stream isn't the purpose.  It's
> just a gratis bonus.

So we agree that the dynamic switch isn't the intended usage but we 
allow for it anyway... I guess given that we can enable/disable 
timestamps dynamically this follows the same logic, it's just weird to 
have a sw_param whose intended use is really a hw_param, something you 
set once and don't modify later. If we want user-space to do the right 
thing we should at least document the consequences of a dynamic switch.
No further objections.

>
> Takashi
>
>>> Along with this, increment the PCM protocol version.
>>>
>>> The extension was suggested by Clemens Ladisch.
>>>
>>> Signed-off-by: Takashi Iwai <tiwai@suse.de>
>>> ---
>>>    include/uapi/sound/asound.h | 6 ++++--
>>>    sound/core/pcm_native.c     | 3 +++
>>>    2 files changed, 7 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/include/uapi/sound/asound.h b/include/uapi/sound/asound.h
>>> index cbf7dc850a46..a7e062f91f39 100644
>>> --- a/include/uapi/sound/asound.h
>>> +++ b/include/uapi/sound/asound.h
>>> @@ -139,7 +139,7 @@ struct snd_hwdep_dsp_image {
>>>     *                                                                           *
>>>     *****************************************************************************/
>>>
>>> -#define SNDRV_PCM_VERSION		SNDRV_PROTOCOL_VERSION(2, 0, 11)
>>> +#define SNDRV_PCM_VERSION		SNDRV_PROTOCOL_VERSION(2, 0, 12)
>>>
>>>    typedef unsigned long snd_pcm_uframes_t;
>>>    typedef signed long snd_pcm_sframes_t;
>>> @@ -391,7 +391,9 @@ struct snd_pcm_sw_params {
>>>    	snd_pcm_uframes_t silence_threshold;	/* min distance from noise for silence filling */
>>>    	snd_pcm_uframes_t silence_size;		/* silence block size */
>>>    	snd_pcm_uframes_t boundary;		/* pointers wrap point */
>>> -	unsigned char reserved[64];		/* reserved for future */
>>> +	unsigned int tstamp_type;		/* timestamp type */
>>> +	int pads;				/* alignment, reserved */
>>> +	unsigned char reserved[56];		/* reserved for future */
>>>    };
>>>
>>>    struct snd_pcm_channel_info {
>>> diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
>>> index 2372c49a8e84..81dedc381efd 100644
>>> --- a/sound/core/pcm_native.c
>>> +++ b/sound/core/pcm_native.c
>>> @@ -543,6 +543,8 @@ static int snd_pcm_sw_params(struct snd_pcm_substream *substream,
>>>
>>>    	if (params->tstamp_mode > SNDRV_PCM_TSTAMP_LAST)
>>>    		return -EINVAL;
>>> +	if (params->tstamp_type > SNDRV_PCM_TSTAMP_TYPE_LAST)
>>> +		return -EINVAL;
>>>    	if (params->avail_min == 0)
>>>    		return -EINVAL;
>>>    	if (params->silence_size >= runtime->boundary) {
>>> @@ -557,6 +559,7 @@ static int snd_pcm_sw_params(struct snd_pcm_substream *substream,
>>>    	err = 0;
>>>    	snd_pcm_stream_lock_irq(substream);
>>>    	runtime->tstamp_mode = params->tstamp_mode;
>>> +	runtime->tstamp_type = params->tstamp_type;
>>>    	runtime->period_step = params->period_step;
>>>    	runtime->control->avail_min = params->avail_min;
>>>    	runtime->start_threshold = params->start_threshold;
>>>
>>

Re: [PATCH 3/3] ALSA: pcm: Add timestamp type to sw_params

[Clemens Ladisch]

Pierre-Louis Bossart wrote:
> On 7/10/14, 10:42 AM, Takashi Iwai wrote:
>> Pierre-Louis Bossart wrote:
>>> On 7/10/14, 8:04 AM, Takashi Iwai wrote:
>>>> For allowing adjusting the timestamp type on the fly, add it to
>>>> sw_params.  The existing ioctl is still kept for compatibility.
>>>
>>> I have strong objections to this extension. It will result in a
>>> discontinuity in the timestamps reported in pcm_status without a clear
>>> indication of what timestamp is reported (when does this change occur?),
>>> and it's completely unclear how userspace would handle a step (positive
>>> or negative) between ntp-adjusted and non-ntp-adjusted times.
>>
>> Well, I don't understand the logic; it's app itself who changes the
>> timestamp type.  It must know when it's changed (because app is
>> changing it), and how to handle (because app chooses the timestamp
>> type).  And the current type can be queried via sw_params_*_get()
>> thing.
>>
>> Of course, as default, there is no change from the current behavior --
>> it takes CLOCK_MONOTONIC.  So, there should be no breakage by this
>> change unless you change the application side to use the new call.
>>
>>> For apps
>>> that make use of the audio_time reported with a wall clock this could
>>> lead to complete nonsense.
>>> I would have no problems if this was a fixed parameter defined once
>>> before audio streaming starts.
>>
>> You don't have to change the setup after the stream starts.  It's
>> usually set before the stream starting.  The point of using sw_params
>> is that it's independent from the hardware driver, thus it fits better
>> than hw_params.  The switching after stream isn't the purpose.  It's
>> just a gratis bonus.
>
> So we agree that the dynamic switch isn't the intended usage but we
> allow for it anyway... I guess given that we can enable/disable
> timestamps dynamically this follows the same logic, it's just weird to
> have a sw_param whose intended use is really a hw_param, something you
> set once and don't modify later.

The difference between sw_params and hw_params is that the latter are
affected by device constraints (and might have interdependencies).
That sw_params can be changed at any time is just a consequence of that.

> If we want user-space to do the right thing we should at least
> document the consequences of a dynamic switch.

Changing the clock relative to which the timestamp is reported is not
only the desired but the _only_ consequence of this parameter.  (And
changing from/to GETTIMEOFDAY was already possible.)


Regards,
Clemens

Re: [PATCH 0/3] Revised patch(es) for CLOCK_MONOTONIC_RAW timestamp

[Jaroslav Kysela]

Date 10.7.2014 15:04, Takashi Iwai wrote:
> Hi,
> 
> here is a series of revised kernel patches for adding
> CLOCK_MONOTONIC_RAW timestamp type.  In addition to Mark's original
> patch, the new field is added to sw_params.

Acked-by: Jaroslav Kysela <perex@perex.cz>

-- 
Jaroslav Kysela <perex@perex.cz>
Linux Kernel Sound Maintainer
ALSA Project; Red Hat, Inc.

Re: [PATCH 0/3] Revised patch(es) for CLOCK_MONOTONIC_RAW timestamp

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 297 bytes --]

On Thu, Jul 10, 2014 at 03:04:15PM +0200, Takashi Iwai wrote:
> Hi,
> 
> here is a series of revised kernel patches for adding
> CLOCK_MONOTONIC_RAW timestamp type.  In addition to Mark's original
> patch, the new field is added to sw_params.

Reviewed-by: Mark Brown <broonie@linaro.org>

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 0/3] Revised patch(es) for CLOCK_MONOTONIC_RAW timestamp

[Takashi Iwai]

At Thu, 10 Jul 2014 15:04:15 +0200,
Takashi Iwai wrote:
> 
> Hi,
> 
> here is a series of revised kernel patches for adding
> CLOCK_MONOTONIC_RAW timestamp type.  In addition to Mark's original
> patch, the new field is added to sw_params.
> 
> The corresponding alsa-lib patches will follow after these.

FWIW, I merged the patches with acks to topic/monotonic branch of
sound git tree now.  It's merged to for-next branch.

Will update alsa-lib git tree soon later.


Takashi

Re: [PATCH 0/3] Revised patch(es) for CLOCK_MONOTONIC_RAW timestamp

[Takashi Iwai]

At Mon, 14 Jul 2014 18:19:26 +0200,
Takashi Iwai wrote:
> 
> At Thu, 10 Jul 2014 15:04:15 +0200,
> Takashi Iwai wrote:
> > 
> > Hi,
> > 
> > here is a series of revised kernel patches for adding
> > CLOCK_MONOTONIC_RAW timestamp type.  In addition to Mark's original
> > patch, the new field is added to sw_params.
> > 
> > The corresponding alsa-lib patches will follow after these.
> 
> FWIW, I merged the patches with acks to topic/monotonic branch of
> sound git tree now.  It's merged to for-next branch.

And, now I'll revert the last part, the addition of sw_params again.

The addition to sw_params field would break the things when used with
an old alsa-lib, because the old alsa-lib sets the field to zero and
it overrides back to the gettimeofday from monotonic.

Unfortunately, there is no way in the kernel side whether the
user-space calls in the old sw_params or not.  We should have a field
or ioctl to set the protocol version the user-space talks, not only
the kernel talks.

For alsa-lib, we'd just need two new functions that are decoupled from
sw_params, e.g. snd_pcm_{get|set}_timestamp_type().

I'm going to submit new patches later.


Takashi

Re: [PATCH 0/3] Revised patch(es) for CLOCK_MONOTONIC_RAW timestamp

[Takashi Iwai]

At Wed, 16 Jul 2014 17:15:54 +0200,
Takashi Iwai wrote:
> 
> At Mon, 14 Jul 2014 18:19:26 +0200,
> Takashi Iwai wrote:
> > 
> > At Thu, 10 Jul 2014 15:04:15 +0200,
> > Takashi Iwai wrote:
> > > 
> > > Hi,
> > > 
> > > here is a series of revised kernel patches for adding
> > > CLOCK_MONOTONIC_RAW timestamp type.  In addition to Mark's original
> > > patch, the new field is added to sw_params.
> > > 
> > > The corresponding alsa-lib patches will follow after these.
> > 
> > FWIW, I merged the patches with acks to topic/monotonic branch of
> > sound git tree now.  It's merged to for-next branch.
> 
> And, now I'll revert the last part, the addition of sw_params again.
> 
> The addition to sw_params field would break the things when used with
> an old alsa-lib, because the old alsa-lib sets the field to zero and
> it overrides back to the gettimeofday from monotonic.
> 
> Unfortunately, there is no way in the kernel side whether the
> user-space calls in the old sw_params or not.  We should have a field
> or ioctl to set the protocol version the user-space talks, not only
> the kernel talks.
> 
> For alsa-lib, we'd just need two new functions that are decoupled from
> sw_params, e.g. snd_pcm_{get|set}_timestamp_type().

... thinking of this again, the less intrusive and future-proof change
is rather to add a new field indicating the protocol version from the
user-space side.  Then kernel evaluates the new fields only when a
valid proto number is set there.  This will help also any changes in
sw_params in future.  Patches will follow.


Takashi