Re: So, what's the status on the recent patches here?

David Singleton
2006-08-19T06:10:02Z

[-- Attachment #1: Type: text/plain, Size: 4055 bytes --]

On 8/14/06, Greg KH <greg@kroah.com> wrote:
> On Mon, Aug 14, 2006 at 07:48:01PM -0400, Dave Jones wrote:
> >
> > This adds a whole bunch of new code, and doesn't seem to make any
> > existing code any simpler (to me at least). From a cpufreq point of view,
> > what does adding this buy us? What problem do we have today that is
> > being solved by all this?

Greg and Dave,

there are two competing patch sets for a new power management
framework. The patch set I sent out simplifies power management,
from both the cpufreq perspective and the embedded world's view of
power management.

I've renamed my patch oppoint so as not confuse it
with the powerop set from Matt Locke (which will probably make
it even more confusing). I've renamed it so it can be seen as an
alternative design approach, not just an alternative implementation
of the same ideas. I've also incorporated suggestions from
Pavel in cleaning up the original patches.

If you'd be willing to take a look at, or try out, the patches
in my patch set you should be able to see how oppoint could simplify
cpufreq code. The first patch is the oppoint-cpufreq.patch and
the second is the oppoint-x86-centrino.patch.

Oppoint could replace large pieces of the cpufreq code
in the kernel, most notably the policy and governor code, which I
believe belongs in user space in the power manager daemon.

You'll notice that the oppoint-cpufreq.patch only touches
two files, cpufreq.c and cpufreq.h. It only creates two new interfaces
to the cpufreq frequency scaling notifier lists to support driver pre
and post scaling routines, already supported in the kernel.

The oppoint-x86-centrino.patch completes the replacement
of cpufreq code by introducing the transition routine to
change frequencies and creates operating points for the
centrino-speedstep processors already supported by Linux.

(although I've recieved a note from Intel that the data I've copied
from the centrino-speedstep cpufreq tables is known to be inaccurate
and unsupported)

This code could replace cpufreq code and simplify it quite a
bit in the process. The kernel drivers that support cpufreq frequency
scaling would not have to be changed. Operating points for the rest
of the processors that support cpufreq would have to be created, but
as you can see it's quite a straight forward transformation from
a cpufreq table to a set of operating points for a processor.

The entire patch set can be found at:

http://source.mvista.com/~dsingleton/2.6.18-rc4/

The patch set consists of:

oppoint-core.patch
oppoint-cpufreq.patch
oppoint-x86-centrino.patch
oppoint-arm-pxa27x.patch

I'll attach oppoint-cpufreq.patch to this email and
send out oppoint-x86-centrino.patch next.

David

> >
> > Every explanation of powerop I've seen so far dives into microdetails,
> > whilst the 10,000ft view has always passed me by other than "this is
> > what we've had in the embedded world".
> >
> > The diagram at http://lists.osdl.org/pipermail/linux-pm/2006-August/003196.html
> > also confuses me. I was under the impression that powerop was adding additional
> > userspace interfaces. If we're not changing how things from a userspace
> > point of view, we're churning a lot of kernel code,.. why?
> >
> > Clue me in here, I'm feeling thick.
>
> You're not alone, I really don't get it either.
>
> But I guess we'll just wait for the next round of unified patches and
> then go from there.
>
> thanks,
>
> greg k-h
> _______________________________________________
> linux-pm mailing list
> linux-pm@lists.osdl.org
> https://lists.osdl.org/mailman/listinfo/linux-pm
>

[-- Attachment #2: oppoint-cpufreq.patch --]
[-- Type: application/octet-stream, Size: 2433 bytes --]

Signed-Off-by: David Singleton <dsingleton@mvista.com>

drivers/cpufreq/cpufreq.c | 36 ++++++++++++++++++++++++++++++++++++
include/linux/cpufreq.h | 2 ++
2 files changed, 38 insertions(+)

Index: linux-2.6.17/drivers/cpufreq/cpufreq.c
===================================================================
--- linux-2.6.17.orig/drivers/cpufreq/cpufreq.c
+++ linux-2.6.17/drivers/cpufreq/cpufreq.c
@@ -226,6 +226,35 @@ static void adjust_jiffies(unsigned long
static inline void adjust_jiffies(unsigned long val, struct cpufreq_freqs *ci) { return; }
#endif

+int cpufreq_prepare_transition(struct oppoint *cur, struct oppoint *new)
+{
+ struct cpufreq_freqs freqs;
+
+ freqs.old = cur->frequency;
+ freqs.new = new->frequency;
+ freqs.cpu = 0;
+ freqs.flags = cpufreq_driver->flags;
+ blocking_notifier_call_chain(&cpufreq_transition_notifier_list,
+ CPUFREQ_PRECHANGE, &freqs);
+ adjust_jiffies(CPUFREQ_PRECHANGE, &freqs);
+ return 0;
+}
+EXPORT_SYMBOL(cpufreq_prepare_transition);
+
+int cpufreq_finish_transition(struct oppoint *cur, struct oppoint *new)
+{
+ struct cpufreq_freqs freqs;
+
+ freqs.old = cur->frequency;
+ freqs.new = new->frequency;
+ freqs.cpu = 0;
+ freqs.flags = cpufreq_driver->flags;
+ adjust_jiffies(CPUFREQ_POSTCHANGE, &freqs);
+ blocking_notifier_call_chain(&cpufreq_transition_notifier_list,
+ CPUFREQ_POSTCHANGE, &freqs);
+ return 0;
+}
+EXPORT_SYMBOL(cpufreq_finish_transition);

/**
* cpufreq_notify_transition - call notifier chain and adjust_jiffies
@@ -920,6 +949,12 @@ static void cpufreq_out_of_sync(unsigned
}

+#ifdef CONFIG_PM
+unsigned int cpufreq_quick_get(unsigned int cpu)
+{
+ return (current_state->frequency * 1000);
+}
+#else
/**
* cpufreq_quick_get - get the CPU frequency (in kHz) frpm policy->cur
* @cpu: CPU number
@@ -941,6 +976,7 @@ unsigned int cpufreq_quick_get(unsigned

return (ret);
}
+#endif
EXPORT_SYMBOL(cpufreq_quick_get);

Index: linux-2.6.17/include/linux/cpufreq.h
===================================================================
--- linux-2.6.17.orig/include/linux/cpufreq.h
+++ linux-2.6.17/include/linux/cpufreq.h
@@ -268,6 +268,8 @@ static inline unsigned int cpufreq_quick
return 0;
}
#endif
+int cpufreq_prepare_transition(struct oppoint *cur, struct oppoint *new);
+int cpufreq_finish_transition(struct oppoint *cur, struct oppoint *new);

/*********************************************************************

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