Re: Alternative Concept [Was: Re: [RFC] CPUFreq PowerOP integration, Intro 0/3]

[Dominik Brodowski <linux@dominikbrodowski.net>]

Hi,

On Fri, Oct 13, 2006 at 02:43:48AM +0400, Eugeny S. Mints wrote:
> >A) The lowest level: lots of knobs.
> >
> >Somewhere in a "computer system"[2] there are very many "knobs" which may
> >be turned to influence various voltages, clock levels, or operating modes
> >("turbo", "performance" or "powersave", for example).
> >
> >Also, there might be many dependencies on how these "knobs" may be
> >changed.
> >
> >Let's assume the system is in a well-defined, working state right now.
> In terms which we use to describe PowerOP a "kbob" is "power parameter"
> and "operating point" is an entity which corresponds to "well-defined, 
> _working_ [system power] state".
> 
> So, what PowerOP Core does: it just maintains a collection of operating 
> point,
> i.e. collection of known-to-be-working system power states. On many 
> platforms
> (especially embedded) not all combinations of power parameters are valid.  
> Some
> [invalid] combinations of the power parameters may crash or damage the 
> system.

Exactly. And that is what I suggest you may want to do in the level _above_
the knobs. Not all platform need or even want this limitations; some require
it. So let's not force it upon everyone, but let's just use it where it
makes sense?


<operating points table library>
              |
	<knob layer>

> 1) you have a table which tells you that there are some  combinations of 
> power
> parameter values which are
> a) _known-to-be-working_
> and
> b) contains SOME_PLATFORM_POWER_PARAMETER=value1.
> Then you chose one of these operating points and switch to it.
> 
> The table creation is simply registration of operating points with POwerOP 
> Core.

Sort of.

> Now selection and switch. Obviously the functionality of selection between
> operating points based on some algo (which btw varies even not across 
> platforms
> but even across different profiles of the same platform) has nothing to do 
> with
> the code which actually switches operating points. So having such
> functionalities coupled within the ->set() method is just invalid design - 
> they
> have to be separated. That's exactly what PowerOP approach does:

Looking at the code, not really -- there my approach was able to use the
same "separation", or "coupling", which PowerOP uses.


> an upper 
> layer
> can implement selection logic leveraging PowerOP Core interface and then 
> request
> POwerOP Core to switch system to the selected operating point.

That's also possible using my approach.

> 2) table does not exist. There are two options here:
> 
> Either,
> a)an entity which calls ->set() for a particular power parameter IS
> RESPONSIBLE for that resulting combination of power parameter values (once 
> the
> set has been executed) IS valid one
> 
> OR
> b) the system executes complex logic you described under D) 1) (in fact,
> cpufreq policy notifiers) to get a valid combination of power parameter 
> values
> with a predefined value of a certain power parameter.
> 
> Let me illustrate why 2)a) is just particular case in contrast to POwerOP 
> which
> is general case in this situation.
> 
> i) PowerOP Core provides interface to get/set value of a particular power 
> parameter
> 
> ii)  Let's assume we limit the set of operating points for a platform to 
> one point.  This one operating point is always the current operating point. 
> All operations occur on the the current operating point.
> 
> iii)in the assumptions above your  ->set() is nothing else than:
> set(param, value)
> {
>  struct powerop_pwr_param p;
> 
>  p.attr = param;
>  p.value = value;
>  powerop_set_pwr_params(CURRENT_POINT, &p, 1);
>  powerop_set_point(CURRENT_POINT);
> }

Here you turn PowerOP on its head -- you modify one Operating Point runtime?
Isn't that two Operating Points? That sounds strange. Also, it again looks
at it from a top->bottom view, which I dislike...

> That's it. Bottom line is: what you are talking about is NOT an Alternative
> Concept but a particular case instead. While PowerOP design is generic case.

As I said -- the differences might be subtle. But they're important. And I
don't care how anything is called -- I care that both the concept and the
code is good.

Thanks,
	Dominik