[PATCH][DOCUMENTATION BUGFIX] latency in micro-, not nanoseconds

[PATCH][DOCUMENTATION BUGFIX] latency in micro-, not nanoseconds

[Dominik Brodowski]

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

All drivers [davej, can you please verify for powernow-k7?] set the 
transition_latency time in microseconds, even though the core demanded
nanoseconds. Instead of fixing up the drivers, fix up the core...

	Dominik

diff -ruN linux-original/include/linux/cpufreq.h linux/include/linux/cpufreq.h
--- linux-original/include/linux/cpufreq.h	2003-11-04 15:27:38.000000000 +0100
+++ linux/include/linux/cpufreq.h	2003-11-04 16:09:23.547389216 +0100
@@ -57,7 +57,7 @@
 struct cpufreq_cpuinfo {
 	unsigned int		max_freq;
 	unsigned int		min_freq;
-	unsigned int		transition_latency; /* in 10^(-9) s */
+	unsigned int		transition_latency; /* in 10^(-6) s = microseconds */
 };
 
 struct cpufreq_real_policy {

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

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

_______________________________________________
Cpufreq mailing list
Cpufreq@www.linux.org.uk
http://www.linux.org.uk/mailman/listinfo/cpufreq

Re: [PATCH][DOCUMENTATION BUGFIX] latency in micro-, not nanoseconds

[Dave Jones]

On Tue, Nov 04, 2003 at 05:08:16PM +0100, Dominik Brodowski wrote:
 > All drivers [davej, can you please verify for powernow-k7?] set the 
 > transition_latency time in microseconds, even though the core demanded
 > nanoseconds. Instead of fixing up the drivers, fix up the core...

Hmm, I think you've unearthed a bug.

The 'latency' we read from the BIOS is in microseconds, but the hardware
wants it in units of 10ns. We do the necessary maths, and everything is fine.
But we're also storing that result in ..

	policy->cpuinfo.transition_latency = latency;

Which is possibly not the right thing to be doing.
I think we want to be storing the 'pre 10ns munging' value here correct?

		Dave

Re: [PATCH][DOCUMENTATION BUGFIX] latency in micro-, not nanoseconds

[Ducrot Bruno]

On Mon, Nov 10, 2003 at 04:32:09PM +0000, Dave Jones wrote:
> On Tue, Nov 04, 2003 at 05:08:16PM +0100, Dominik Brodowski wrote:
>  > All drivers [davej, can you please verify for powernow-k7?] set the 
>  > transition_latency time in microseconds, even though the core demanded
>  > nanoseconds. Instead of fixing up the drivers, fix up the core...
> 
> Hmm, I think you've unearthed a bug.
> 
> The 'latency' we read from the BIOS is in microseconds, but the hardware
> wants it in units of 10ns. We do the necessary maths, and everything is fine.
> But we're also storing that result in ..
> 
> 	policy->cpuinfo.transition_latency = latency;
> 
> Which is possibly not the right thing to be doing.
> I think we want to be storing the 'pre 10ns munging' value here correct?
> 

Well, maybe we have to double it as well (there is actually 2 steps, the
VID change and the FID change, each having 'latency' 1e(-8) second)...

something like:
	policy->cpuinfo.transition_latency = latency / 50;

perhaps?

-- 
Ducrot Bruno

--  Which is worse:  ignorance or apathy?
--  Don't know.  Don't care.

Re: [PATCH][DOCUMENTATION BUGFIX] latency in micro-, not nanoseconds

[Dave Jones]

On Mon, Nov 10, 2003 at 06:15:33PM +0100, Ducrot Bruno wrote:

 > > The 'latency' we read from the BIOS is in microseconds, but the hardware
 > > wants it in units of 10ns. We do the necessary maths, and everything is fine.
 > > But we're also storing that result in ..
 > > 
 > > 	policy->cpuinfo.transition_latency = latency;
 > > 
 > > Which is possibly not the right thing to be doing.
 > > I think we want to be storing the 'pre 10ns munging' value here correct?
 > > 
 > 
 > Well, maybe we have to double it as well (there is actually 2 steps, the
 > VID change and the FID change, each having 'latency' 1e(-8) second)...
 > 
 > something like:
 > 	policy->cpuinfo.transition_latency = latency / 50;

Actually, I'm wondering why we need this at all anyway..

		Dave

Re: [PATCH][DOCUMENTATION BUGFIX] latency in micro-, not nanoseconds

[Dominik Brodowski]

Hi Dave and others,


On Mon, Nov 10, 2003 at 05:22:46PM +0000, Dave Jones wrote:
> On Mon, Nov 10, 2003 at 06:15:33PM +0100, Ducrot Bruno wrote:
> 
>  > > The 'latency' we read from the BIOS is in microseconds, but the hardware
>  > > wants it in units of 10ns. We do the necessary maths, and everything is fine.
>  > > But we're also storing that result in ..
>  > > 
>  > > 	policy->cpuinfo.transition_latency = latency;
>  > > 
>  > > Which is possibly not the right thing to be doing.
>  > > I think we want to be storing the 'pre 10ns munging' value here correct?
>  > > 
>  > 
>  > Well, maybe we have to double it as well (there is actually 2 steps, the
>  > VID change and the FID change, each having 'latency' 1e(-8) second)...
>  > 
>  > something like:
>  > 	policy->cpuinfo.transition_latency = latency / 50;
> 
> Actually, I'm wondering why we need this at all anyway..

To determine the "cost" of frequency and/or voltage transitions when doing
dynamic switching. If the system is unresponsive for too long a period of 
time, dynamic switching doesn't make sense [at least when some sort of
real-time or "responsiveness" is needed]. Also, the time to needed switch 
back to 100% of processing power should be added to the "overhead" idleness
on future load calculations of yet-to-be-written load-predicting cpufreq
governors.

e.g.
target_frequency = predicted_load + margin + overhead 

with "margin" being some sort of "reserve" of processing power available
instantly,
and "overhead" consisting of "book-keeping" and two times the cost of
a cpufreq transition => switching down now, and switching up when there is
the need to because of a higher demand for cpu processing power.


Also, there should be another field in cpufreq_cpuinfo: transition_interval:
the minimum time which must pass between two calls to ->range [currently
->setpolicy() or] ->target.

transition_latency is the time [in microseconds] the system is unresponsive
to interrupts after a ->target or ->range call is passed to the cpufreq
driver. To ease the computation of this value, the overhead of the cpufreq
driver and the cpufreq core may be ignored here, so that transition_latency
is the difference between the time a hardware command is issued, and the
time the hardware command returns, and necessary following delay() calls 
have passed.

transition_interval is the time [in microseconds] which must have passed
between another call to ->target or ->range is issued. It is the duty of the
cpufreq core [and of the cpufreq governors] to guarantee this.

	Dominik

Re: [PATCH][DOCUMENTATION BUGFIX] latency in micro-, not nanoseconds

[Dave Jones]

On Mon, Nov 10, 2003 at 09:50:29PM +0100, Dominik Brodowski wrote:

 > > Actually, I'm wondering why we need this at all anyway..
 > 
 > To determine the "cost" of frequency and/or voltage transitions when doing
 > dynamic switching. If the system is unresponsive for too long a period of 
 > time, dynamic switching doesn't make sense [at least when some sort of
 > real-time or "responsiveness" is needed]. Also, the time to needed switch 
 > back to 100% of processing power should be added to the "overhead" idleness
 > on future load calculations of yet-to-be-written load-predicting cpufreq
 > governors.

Ok, I can buy that. But it's not being used at all right now, right ?

		Dave

Re: [PATCH][DOCUMENTATION BUGFIX] latency in micro-, not nanoseconds

[Dominik Brodowski]

On Mon, Nov 10, 2003 at 09:16:54PM +0000, Dave Jones wrote:
> On Mon, Nov 10, 2003 at 09:50:29PM +0100, Dominik Brodowski wrote:
> 
>  > > Actually, I'm wondering why we need this at all anyway..
>  > 
>  > To determine the "cost" of frequency and/or voltage transitions when doing
>  > dynamic switching. If the system is unresponsive for too long a period of 
>  > time, dynamic switching doesn't make sense [at least when some sort of
>  > real-time or "responsiveness" is needed]. Also, the time to needed switch 
>  > back to 100% of processing power should be added to the "overhead" idleness
>  > on future load calculations of yet-to-be-written load-predicting cpufreq
>  > governors.
> 
> Ok, I can buy that. But it's not being used at all right now, right ?

It is used in the ondemand governor already. Venkatesh knows of the
documentation bug and of the powernow-k7 bug, though.

BTW, it's not enough for powernow-k7 to divide the BIOS-known latency by 50.
This latency for one VID and one FID transition needs to be multiplied by
the number of p-states minus 1, as that's the actual maximum latency due to
the "one step up/down only" loop, IIRC.

	Dominik

Re: [PATCH][DOCUMENTATION BUGFIX] latency in micro-, not nanoseconds

[Dave Jones]

On Tue, Nov 11, 2003 at 07:54:17PM +0100, Dominik Brodowski wrote:
 > BTW, it's not enough for powernow-k7 to divide the BIOS-known latency by 50.
 > This latency for one VID and one FID transition needs to be multiplied by
 > the number of p-states minus 1, as that's the actual maximum latency due to
 > the "one step up/down only" loop, IIRC.

Yoinks, I thought I had merged that patch (so did you judging from
that last sentence), but it fell through the cracks somehow.
I'll see if I can dig it out if I still have it.

		Dave

Re: [PATCH][DOCUMENTATION BUGFIX] latency in micro-, not nanoseconds

[Ducrot Bruno]

On Tue, Nov 11, 2003 at 10:45:02PM +0000, Dave Jones wrote:
> On Tue, Nov 11, 2003 at 07:54:17PM +0100, Dominik Brodowski wrote:
>  > BTW, it's not enough for powernow-k7 to divide the BIOS-known latency by 50.
>  > This latency for one VID and one FID transition needs to be multiplied by
>  > the number of p-states minus 1, as that's the actual maximum latency due to
>  > the "one step up/down only" loop, IIRC.
> 
> Yoinks, I thought I had merged that patch (so did you judging from
> that last sentence), but it fell through the cracks somehow.
> I'll see if I can dig it out if I still have it.
> 
> 		Dave
> 

I expected that correcting latency and fixing the change_[FV]ID()
will make any patch doing 'one step up/down only' for K7 to be rejected?

Cheers,

-- 
Ducrot Bruno

--  Which is worse:  ignorance or apathy?
--  Don't know.  Don't care.

Re: [PATCH][DOCUMENTATION BUGFIX] latency in micro-, not nanoseconds

[Dave Jones]

On Wed, Nov 12, 2003 at 12:01:34PM +0100, Ducrot Bruno wrote:
 > On Tue, Nov 11, 2003 at 10:45:02PM +0000, Dave Jones wrote:
 > > On Tue, Nov 11, 2003 at 07:54:17PM +0100, Dominik Brodowski wrote:
 > >  > BTW, it's not enough for powernow-k7 to divide the BIOS-known latency by 50.
 > >  > This latency for one VID and one FID transition needs to be multiplied by
 > >  > the number of p-states minus 1, as that's the actual maximum latency due to
 > >  > the "one step up/down only" loop, IIRC.
 > > 
 > > Yoinks, I thought I had merged that patch (so did you judging from
 > > that last sentence), but it fell through the cracks somehow.
 > > I'll see if I can dig it out if I still have it.
 > > 
 > > 		Dave
 > > 
 > 
 > I expected that correcting latency and fixing the change_[FV]ID()
 > will make any patch doing 'one step up/down only' for K7 to be rejected?

Worse yet, I can't put my finger on the mail that had the patch.
I'll dig some more, but I'm wondering if this was lost in the
window between a backup and my IBM deskstars dieing a few months back.

Might have to hack something up from scratch after all..

		Dave

Re: [PATCH][DOCUMENTATION BUGFIX] latency in micro-, not nanoseconds

[Ducrot Bruno]

On Wed, Nov 12, 2003 at 02:17:54PM +0000, Dave Jones wrote:
> On Wed, Nov 12, 2003 at 12:01:34PM +0100, Ducrot Bruno wrote:
>  > On Tue, Nov 11, 2003 at 10:45:02PM +0000, Dave Jones wrote:
>  > > On Tue, Nov 11, 2003 at 07:54:17PM +0100, Dominik Brodowski wrote:
>  > >  > BTW, it's not enough for powernow-k7 to divide the BIOS-known latency by 50.
>  > >  > This latency for one VID and one FID transition needs to be multiplied by
>  > >  > the number of p-states minus 1, as that's the actual maximum latency due to
>  > >  > the "one step up/down only" loop, IIRC.
>  > > 
>  > > Yoinks, I thought I had merged that patch (so did you judging from
>  > > that last sentence), but it fell through the cracks somehow.
>  > > I'll see if I can dig it out if I still have it.
>  > > 
>  > > 		Dave
>  > > 
>  > 
>  > I expected that correcting latency and fixing the change_[FV]ID()
>  > will make any patch doing 'one step up/down only' for K7 to be rejected?
> 
> Worse yet, I can't put my finger on the mail that had the patch.
> I'll dig some more, but I'm wondering if this was lost in the
> window between a backup and my IBM deskstars dieing a few months back.
> 

Well, I should have one or two patch(s) for, if really needed,
but what I wanted to point is that now 'latency' is fixed,
and change_VID() is debugged, my brain-damaged try-to-hang

http://www.poupinou.org/cpufreq/test_all_freqs.c

don't want to crash the laptop now.

Cheers,

-- 
Ducrot Bruno

--  Which is worse:  ignorance or apathy?
--  Don't know.  Don't care.

RE: [PATCH][DOCUMENTATION BUGFIX] latency in micro-, not nanoseconds

[Pallipadi, Venkatesh]

> -----Original Message-----
> From: cpufreq-bounces@www.linux.org.uk 
> [mailto:cpufreq-bounces@www.linux.org.uk] On Behalf Of 
> Dominik Brodowski
> Sent: Tuesday, November 11, 2003 10:54 AM
> To: Dave Jones
> Cc: Ducrot Bruno; cpufreq@www.linux.org.uk
> Subject: Re: [PATCH][DOCUMENTATION BUGFIX] latency in micro-, 
> not nanoseconds
> 
> 
> On Mon, Nov 10, 2003 at 09:16:54PM +0000, Dave Jones wrote:
> > Ok, I can buy that. But it's not being used at all right 
> now, right ?
> 
> It is used in the ondemand governor already. Venkatesh knows of the
> documentation bug and of the powernow-k7 bug, though.


Yes. Current version of ondemand governor is using the transiton 
latency to decide whether this policy can be enabled on this CPU 
or not. As it does frequent sampling (default - 4 times per sec)
which may not work well you have higher transition latencies. 
I am planning to use transition latency to decide on an 
appropriate sampling frequency for that CPU, in next release 
of ondemand governor, so that the governor can support wider 
range of CPUs (although with different sampling frequencies).


Thanks,
-Venkatesh