Thanks for your input on this one. I have a few questions...If they're
out of place, let me know, and I'll shut up. :)

On Sat, 2003-11-01 at 13:09, Dominik Brodowski wrote:
> It seems to me that your governor uses exactly the same algorithm as the one
> proposed by Venkatesh Pallipadi, but maybe with some slightly different
> default values. So, I'd prefer if only one of these two were
> merged into CVS and/or the kernel. And, for that matter, I prefer
> Venkatesh's code at the moment as it's cleaner, supports different CPUs,
> ... - but see below for some details.

I would actually say at this moment that I would probably prefer
Venkatesh's code be merged over my own, for basically the same reasons.
(I'm a student, and very much an amateur coder - a beginner in the
kernel arena. This is very much a learning experience for me. :)

I just took a look through it, and it does seem to use a similar
algorithm, though I don't understand some elements of it. Implementation
seems to be drastically different. I don't understand in what way the
two drivers differ in terms of CPU support. Please elaborate. (I don't
understand how my governor doesn't support different CPUs - I thought it
did.)

I did actually make a few major changes yesterday, the new file is
attached. It now should support SMP, and should now step downwards at
1/5th the speed of stepping up. I will make a pile of additional changes
based on your comments. (Thank you for taking the time to actually read
through my code! I really appreciate it!) 

Since this code won't be merged, should I continue to post my changes
here, or is that inappropriate?  

> That's not entirely correct -- it just has rather strict limits on the
> "transition latency", and several cpfureq drivers don't set the proper
> transition latency but the dummy value "CPUFREQ_ETERNAL" which disables
> dynamic governors [in kernelspace].

That is I noticed that in the code originally. I commented out the
latency check, so I could try the dbs module with the P4-clockmod
driver. This results in a machine lockup, which is why I originally
wrote the "dynamic" module. I assume the lockup is because the dbs
module seems to poll HZ times a second (10 times as often as mine.),
which would overwhelm the p4-clockmod driver. 

> > 2) People seem to prefer userspace tools to govern frequency.
> Well, I surely don't. I prefer kernelspace.

Woo! Strongly agreed!

> > +config CPU_FREQ_DEFAULT_GOV_DYNAMIC
> "Dynamic" governors can't be default governor -- they can fail due to
> transition latency limits, and would leave the cpufreq core in an undefined
> state then. Also, the name "dynamic" is too generic as there likely will be
> different dynamic cpufreq governors available.

I guess it just seems a waste to have to compile in two governors, when
the static governor would only be in use for the short period of time
until a boot script sets up whichever dynamic governor. Point taken
though.

I suppose the name is moot though, since it won't be merged anyway.
Perhaps cpufreq_dynamic_ugly for now. ;)

> There's no valid reason for such a table. A governor should either
> calculate a percentage of operating power, and say whether to go up or down
> if this frequency = percentage * max_frequency isn't available. 

That's why the meat of the table was commented out, and left only
FREQ_MIN and FREQ_MAX, which use policy->min and policy->max. I don't
know how to get the available frequencies from the current driver, so
that's why I used this. (I believe I even made a comment about the
ugliness of this in my code.)

I had originally planned on using the array (not originally a static
const) as a calculated table of available frequencies. Since I couldn't
think of a good way to get a set of available frequencies, I made it as
it is now.

What happens when a governor attempts to set a speed that the driver
can't handle? Is it driver independant? 

> Actually, this is a problem in Venkatesh's code: If the governor says "there's
> too few processing power available, let's increase the speed" it increases
> the current speed by 1 kHz, and says this is a "low limit" which means that
> the new speed must be higher than that. For the common frequency scaling
> implementations out there, which define only few frequency states, this is
> not a problem. For the geode driver, though, which has several thousand
> frequency states available IIRC, this is a big problem: switching from
> lowest to highest speed may take a couple of seconds or even minutes!

Is there a better way to search for available frequencies? Would it be 
worthwhile to set a number of defined steps, say 'max', '2/3 max', '1/3
max', 'min' -> What happens when something like '2/3 max' isn't an
available frequency? 

--
jon anderson