Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1251170 > unrolled thread

Re: [PATCH v4 1/4] Produce system time from correlated clocksource

Started by"Christopher Hall" <christopher.s.hall@intel.com>
First post2015-10-20 02:20 +0200
Last post2015-10-21 09:50 +0200
Articles 10 — 4 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH v4 1/4] Produce system time from correlated clocksource "Christopher Hall" <christopher.s.hall@intel.com> - 2015-10-20 02:20 +0200
    Re: [PATCH v4 1/4] Produce system time from correlated clocksource John Stultz <john.stultz@linaro.org> - 2015-10-20 02:40 +0200
      Re: [PATCH v4 1/4] Produce system time from correlated clocksource Richard Cochran <richardcochran@gmail.com> - 2015-10-20 11:00 +0200
        Re: [PATCH v4 1/4] Produce system time from correlated clocksource Thomas Gleixner <tglx@linutronix.de> - 2015-10-20 12:50 +0200
          Re: [PATCH v4 1/4] Produce system time from correlated clocksource Richard Cochran <richardcochran@gmail.com> - 2015-10-20 14:00 +0200
            Re: [PATCH v4 1/4] Produce system time from correlated clocksource Richard Cochran <richardcochran@gmail.com> - 2015-10-20 17:00 +0200
              Re: [PATCH v4 1/4] Produce system time from correlated clocksource Thomas Gleixner <tglx@linutronix.de> - 2015-10-20 21:20 +0200
                Re: [PATCH v4 1/4] Produce system time from correlated clocksource Richard Cochran <richardcochran@gmail.com> - 2015-10-20 21:40 +0200
                Re: [PATCH v4 1/4] Produce system time from correlated clocksource John Stultz <john.stultz@linaro.org> - 2015-10-20 22:20 +0200
                  Re: [PATCH v4 1/4] Produce system time from correlated clocksource Thomas Gleixner <tglx@linutronix.de> - 2015-10-21 09:50 +0200

#1251170 — Re: [PATCH v4 1/4] Produce system time from correlated clocksource

From"Christopher Hall" <christopher.s.hall@intel.com>
Date2015-10-20 02:20 +0200
SubjectRe: [PATCH v4 1/4] Produce system time from correlated clocksource
Message-ID<qlsOK-3LO-9@gated-at.bofh.it>
Thomas,

On Thu, 15 Oct 2015 01:15:57 -0700, Thomas Gleixner <tglx@linutronix.de>  
wrote:
>> >
>> > > +#define SHADOW_HISTORY_DEPTH 7
>> >
>> > And that number is 7 because?
>>
>> Due to power of 2 it will be 8 instead. As above the useful history is  
>> 8-2*1
>> ms (1 ms is the minimum jiffy length).  Array size 4 would not be enough
>> history for the DSP which requires 4 ms of history, in the worst case.
>
> And how exactly becomes 7 magically 8?

I'm making the array size a power of two, per your suggestion.

In my view, the candidate array sizes are 4 and 8.  But the DSP driver  
code requires that it be at least 8.  Here is my reasoning:

The number of shadow_timekeeper array elements that contain useful history  
is n-2 where n is the size of the shadow_timekeeper array.  This is true  
because shadow_timekeeper[shadow_index] is a copy of tk_core.timekeeper  
(this isn't history).  The next entry of the shadow_timekeeper array may  
be in-flight and contain invalid information, because update_wall_time()  
makes changes to the next entry of shadow timekeeper outside of the  
sequence lock.  If that occurs, get_correlated_timestamp() would not be  
notified of this change through a change in sequence number.

Combining these two requirements, the candidate array sizes and their  
effective history sizes is:

Array Size   Effective History (in elements)
==========   ===============================
4            2				
8            6
16           14

update_wall_clock() checks that the number of elapsed cycles is >=  
tk->cycle_interval before performing any updates to shadow_timekeeper.   
The minimum cycle count (and minimum time) that is contained in each  
history element is equal to cycle_interval which represents a time period  
of 1/HZ seconds.  The smallest configurable period for cycle_interval is 1  
ms (in cycles).  Each history element, therefore, is guaranteed to contain  
1 ms of history for all kernel configurations.  For correct operation of  
the DSP using the proposed ART->system time API we need at least 4 ms of  
history.  To guarantee this much history is available, the array size  
needs to be 8.

Array Size   Minimum Effective History (in ms)
==========   =================================
4            2				
8            6    <----
16           14

Does this make sense for the choice of array size 8?

> Make it a seperate function which can hand in the information and
> leave the PTP specific sample/conversion function alone.

OK.  The audio driver code will conform to the original correlated  
clocksource API.

FYI, I will be away from work and email until 10/28.  Any responses will  
be delayed until then.

Thanks,
Chris
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]


#1251174

FromJohn Stultz <john.stultz@linaro.org>
Date2015-10-20 02:40 +0200
Message-ID<qlt85-48v-7@gated-at.bofh.it>
In reply to#1251170
On Mon, Oct 19, 2015 at 5:18 PM, Christopher Hall
<christopher.s.hall@intel.com> wrote:
> On Thu, 15 Oct 2015 01:15:57 -0700, Thomas Gleixner <tglx@linutronix.de>
> wrote:
>>>
>>> >
>>> > > +#define SHADOW_HISTORY_DEPTH 7
>>> >
>>> > And that number is 7 because?
>>>
>>> Due to power of 2 it will be 8 instead. As above the useful history is
>>> 8-2*1
>>> ms (1 ms is the minimum jiffy length).  Array size 4 would not be enough
>>> history for the DSP which requires 4 ms of history, in the worst case.
>>
>>
>> And how exactly becomes 7 magically 8?
>
>
> I'm making the array size a power of two, per your suggestion.
>
> In my view, the candidate array sizes are 4 and 8.  But the DSP driver code
> requires that it be at least 8.  Here is my reasoning:
>
> The number of shadow_timekeeper array elements that contain useful history
> is n-2 where n is the size of the shadow_timekeeper array.  This is true
> because shadow_timekeeper[shadow_index] is a copy of tk_core.timekeeper
> (this isn't history).  The next entry of the shadow_timekeeper array may be
> in-flight and contain invalid information, because update_wall_time() makes
> changes to the next entry of shadow timekeeper outside of the sequence lock.
> If that occurs, get_correlated_timestamp() would not be notified of this
> change through a change in sequence number.

Sorry for not commenting here earlier. But I've sort of loosely been
following this thread.

I'm still very very concerned about the complexity of adding any sort
of array of timekeeper structures for historical purposes, and like
Richard, I feel like the rational for all of this has not been made
very clear.

Thomas seems to be a helpful advocate, but I suspect the use case has
been explained to him in detail off list.

If we're only tracking 4ms of history, how does this solution
measurably improve the error over using the timestamps to generate
MONOTONIC_RAW clock deltas (which doesn't require keeping any history)
and using getnstime_raw_and_real to take an anchor point to calculate
the delta from?  Why is adding complexity necessary?

thanks
-john
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1251462

FromRichard Cochran <richardcochran@gmail.com>
Date2015-10-20 11:00 +0200
Message-ID<qlAVY-77N-11@gated-at.bofh.it>
In reply to#1251174
On Mon, Oct 19, 2015 at 05:36:56PM -0700, John Stultz wrote:
> If we're only tracking 4ms of history, how does this solution
> measurably improve the error over using the timestamps to generate
> MONOTONIC_RAW clock deltas (which doesn't require keeping any history)
> and using getnstime_raw_and_real to take an anchor point to calculate
> the delta from?  Why is adding complexity necessary?

This idea is variant of what I suggested in another reply in this
thread.  To my understanding, there is no need at all to keep a
history arbitrarily 4 ms long.  Instead, the DSP driver (or whoever
else may need such a thing) can simply sample the system time at the
rate needed for that particular application.

Thanks,
Richard
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1251578

FromThomas Gleixner <tglx@linutronix.de>
Date2015-10-20 12:50 +0200
Message-ID<qlCEq-1fd-17@gated-at.bofh.it>
In reply to#1251462
On Tue, 20 Oct 2015, Richard Cochran wrote:

> On Mon, Oct 19, 2015 at 05:36:56PM -0700, John Stultz wrote:
> > If we're only tracking 4ms of history, how does this solution
> > measurably improve the error over using the timestamps to generate
> > MONOTONIC_RAW clock deltas (which doesn't require keeping any history)
> > and using getnstime_raw_and_real to take an anchor point to calculate
> > the delta from?  Why is adding complexity necessary?
> 
> This idea is variant of what I suggested in another reply in this
> thread.  To my understanding, there is no need at all to keep a
> history arbitrarily 4 ms long.  Instead, the DSP driver (or whoever
> else may need such a thing) can simply sample the system time at the
> rate needed for that particular application.

That's complete nonsense. The whole point is to have a proper
correlation from ART/audio timestamps to system time. Sampling system
time does not help in any way,

Thanks,

	tglx
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1251616

FromRichard Cochran <richardcochran@gmail.com>
Date2015-10-20 14:00 +0200
Message-ID<qlDK9-2OQ-5@gated-at.bofh.it>
In reply to#1251578
On Tue, Oct 20, 2015 at 12:48:03PM +0200, Thomas Gleixner wrote:
> On Tue, 20 Oct 2015, Richard Cochran wrote:
> 
> > On Mon, Oct 19, 2015 at 05:36:56PM -0700, John Stultz wrote:
> > > If we're only tracking 4ms of history, how does this solution
> > > measurably improve the error over using the timestamps to generate
> > > MONOTONIC_RAW clock deltas (which doesn't require keeping any history)
> > > and using getnstime_raw_and_real to take an anchor point to calculate
> > > the delta from?  Why is adding complexity necessary?
> > 
> > This idea is variant of what I suggested in another reply in this
> > thread.  To my understanding, there is no need at all to keep a
> > history arbitrarily 4 ms long.  Instead, the DSP driver (or whoever
> > else may need such a thing) can simply sample the system time at the
> > rate needed for that particular application.
> 
> That's complete nonsense. The whole point is to have a proper
> correlation from ART/audio timestamps to system time. Sampling system
> time does not help in any way,

You can, in fact, achieve "proper" correlation by sampling.  As John
said, the question is whether the method in the patch set "measurably
improves the error" over using another, simpler method.

Thanks,
Richard

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1251767

FromRichard Cochran <richardcochran@gmail.com>
Date2015-10-20 17:00 +0200
Message-ID<qlGym-6Uz-3@gated-at.bofh.it>
In reply to#1251616
On Tue, Oct 20, 2015 at 01:51:13PM +0200, Richard Cochran wrote:
> You can, in fact, achieve "proper" correlation by sampling.  As John
> said, the question is whether the method in the patch set "measurably
> improves the error" over using another, simpler method.

Here is a short example to put some numbers on the expected error.
Let the driver sample at an interval of 1 ms.  If the system time's
frequency hasn't changed between two samples, A and B, then the driver
may interpolate without introducing any error.

If the frequency is changed between the sample times, then the
interpolated value will have some error.  Because 1 ms is smallest HZ
value, the frequency can change at most once during the sample.  If
the frequency changes near point A or B, then the error is minimal.
The worst case occurs when the frequency is changed half way between A
and B.

Suppose the frequency is changed by 10 PPM, at point C, half way
between A and B.  This change results in a 5 nanosecond time
difference at B (10 PPM over C -> B).  The driver will interpolate
using line A-B with slope increased by 5 PPM, and the worst case
error, found at point C, is then 2.5 nanoseconds.

Thanks,
Richard
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1251978

FromThomas Gleixner <tglx@linutronix.de>
Date2015-10-20 21:20 +0200
Message-ID<qlKBY-4yw-29@gated-at.bofh.it>
In reply to#1251767
On Tue, 20 Oct 2015, Richard Cochran wrote:
> On Tue, Oct 20, 2015 at 01:51:13PM +0200, Richard Cochran wrote:
> > You can, in fact, achieve "proper" correlation by sampling.  As John
> > said, the question is whether the method in the patch set "measurably
> > improves the error" over using another, simpler method.
> 
> Here is a short example to put some numbers on the expected error.
> Let the driver sample at an interval of 1 ms.  If the system time's
> frequency hasn't changed between two samples, A and B, then the driver
> may interpolate without introducing any error.

Darn, we don't want to have that kind of sampling in every driver
which has this kind of problem even if it looks like the simpler
choice for this particular use case. This is going to be something
which next generation chips will have on more than just the audio
interface and we realy want to have a generic solution for this.

Thanks,

	tglx
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1251988

FromRichard Cochran <richardcochran@gmail.com>
Date2015-10-20 21:40 +0200
Message-ID<qlKVk-4Vm-21@gated-at.bofh.it>
In reply to#1251978
On Tue, Oct 20, 2015 at 09:11:21PM +0200, Thomas Gleixner wrote:
> Darn, we don't want to have that kind of sampling in every driver
> which has this kind of problem even if it looks like the simpler
> choice for this particular use case. This is going to be something
> which next generation chips will have on more than just the audio
> interface and we realy want to have a generic solution for this.

Right, having multiple drivers sampling is bad.

Just thinking out loud: how about a service layer that can handle
multiple drivers?  The layer samples at the maximum requested rate,
and buffers the history for the maximum requested backlog.  The
non-max rate users simply get a higher resolution than they need.

A generic solution would handle any history length for old time
stamps, within reason.  I think hard coding 4 ms (or 8 ms or 800 ms)
is clunky.

Thanks,
Richard
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1252002

FromJohn Stultz <john.stultz@linaro.org>
Date2015-10-20 22:20 +0200
Message-ID<qlLy1-5U0-5@gated-at.bofh.it>
In reply to#1251978
On Tue, Oct 20, 2015 at 12:11 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
> On Tue, 20 Oct 2015, Richard Cochran wrote:
>> On Tue, Oct 20, 2015 at 01:51:13PM +0200, Richard Cochran wrote:
>> > You can, in fact, achieve "proper" correlation by sampling.  As John
>> > said, the question is whether the method in the patch set "measurably
>> > improves the error" over using another, simpler method.
>>
>> Here is a short example to put some numbers on the expected error.
>> Let the driver sample at an interval of 1 ms.  If the system time's
>> frequency hasn't changed between two samples, A and B, then the driver
>> may interpolate without introducing any error.
>
> Darn, we don't want to have that kind of sampling in every driver
> which has this kind of problem even if it looks like the simpler
> choice for this particular use case. This is going to be something
> which next generation chips will have on more than just the audio
> interface and we realy want to have a generic solution for this.

I sort of agree with Richard that the timekeeper history approach
doesn't seem like a generic solution here.

And again, you seem to be speaking with a bigger picture in mind that
at least I don't yet share (apologies for being thick headed here).
Being able to have various hardware sharing a time base is quite
useful, and methods for correlating timestamps together are useful.
But I don't yet really understand why its important that we can
translate a hardware timestamp from some time in the past to the
correct system time in the past without error.

thanks
-john
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1252518

FromThomas Gleixner <tglx@linutronix.de>
Date2015-10-21 09:50 +0200
Message-ID<qlWjL-4Qz-1@gated-at.bofh.it>
In reply to#1252002
On Tue, 20 Oct 2015, John Stultz wrote:

> On Tue, Oct 20, 2015 at 12:11 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
> > On Tue, 20 Oct 2015, Richard Cochran wrote:
> >> On Tue, Oct 20, 2015 at 01:51:13PM +0200, Richard Cochran wrote:
> >> > You can, in fact, achieve "proper" correlation by sampling.  As John
> >> > said, the question is whether the method in the patch set "measurably
> >> > improves the error" over using another, simpler method.
> >>
> >> Here is a short example to put some numbers on the expected error.
> >> Let the driver sample at an interval of 1 ms.  If the system time's
> >> frequency hasn't changed between two samples, A and B, then the driver
> >> may interpolate without introducing any error.
> >
> > Darn, we don't want to have that kind of sampling in every driver
> > which has this kind of problem even if it looks like the simpler
> > choice for this particular use case. This is going to be something
> > which next generation chips will have on more than just the audio
> > interface and we realy want to have a generic solution for this.
> 
> I sort of agree with Richard that the timekeeper history approach
> doesn't seem like a generic solution here.
 
I'm not pushing that approach. I just want a generic facility of some
sort to solve that.

> And again, you seem to be speaking with a bigger picture in mind that
> at least I don't yet share (apologies for being thick headed here).
> Being able to have various hardware sharing a time base is quite
> useful, and methods for correlating timestamps together are useful.
> But I don't yet really understand why its important that we can
> translate a hardware timestamp from some time in the past to the
> correct system time in the past without error.

If your device can only provide timestamps from the past, then having
access to the history is important if you want to have precise
correlation.

Thanks,

	tglx
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web