latency in micro-, not nanoseconds

PATCH In review Dominik Brodowski 3 participants 11 messages Nov 4, 2003 – Nov 12, 2003 CPU frequency scaling framework

Share & tools

Dominik Brodowski

author

2003-11-04 16:08 UTC #0 lore raw

PATCH 1/1 latency in micro-, not nanoseconds

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

include/linux/cpufreq.h +1 -1

diff -ruN linux-original/include/linux/cpufreq.h linux/include/linux/cpufreq.h
@@ -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 {

_______________________________________________
Cpufreq mailing list
[email protected]
http://www.linux.org.uk/mailman/listinfo/cpufreq

Dave Jones

2003-11-10 16:32 UTC #1 lore raw

Quoted 3 lines · Dominik Brodowski

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

Ducrot Bruno

2003-11-10 17:15 UTC #2 lore raw

Quoted 7 lines · Dave Jones

Quoted 3 lines · Dominik Brodowski
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.

Quoted 9 lines

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?

Signature · 4 lines

Ducrot Bruno

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

Dave Jones

2003-11-10 17:22 UTC #3 lore raw

Quoted 10 lines · Ducrot Bruno

Quoted 9 lines
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?

Quoted 5 lines

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

Dominik Brodowski

author

2003-11-10 20:50 UTC #4 lore raw

In reply to Dave Jones

Hi Dave and others,

Quoted 12 lines · Dave Jones

Quoted 10 lines · Ducrot Bruno
Quoted 9 lines
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?

Quoted 7 lines

Quoted 5 lines
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

Dave Jones

2003-11-10 21:16 UTC #5 lore raw

Quoted 9 lines · Dominik Brodowski

Quoted 1 line
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

Dominik Brodowski

author

2003-11-11 18:54 UTC #6 lore raw

Quoted 12 lines · Dave Jones

Quoted 9 lines · Dominik Brodowski
Quoted 1 line
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.

Quoted 1 line

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

Dave Jones

2003-11-11 22:45 UTC #7 lore raw

Quoted 4 lines · Dominik Brodowski

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

Ducrot Bruno

2003-11-12 11:01 UTC #8 lore raw

Quoted 6 lines · Dave Jones

Quoted 4 lines · Dominik Brodowski
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.

Quoted 6 lines

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,

Signature · 4 lines

Ducrot Bruno

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

Dave Jones

2003-11-12 14:17 UTC #9 lore raw

Quoted 11 lines · Ducrot Bruno

Quoted 10 lines · Dave Jones
Quoted 4 lines · Dominik Brodowski
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.

Quoted 5 lines

Quoted 2 lines
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

Ducrot Bruno

2003-11-12 15:02 UTC #10 lore raw

Quoted 15 lines · Dave Jones

Quoted 14 lines · Ducrot Bruno
Quoted 12 lines · Dave Jones
Quoted 4 lines · Dominik Brodowski
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

Quoted 7 lines

Quoted 2 lines
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,

Signature · 4 lines

Ducrot Bruno

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