Le mercredi 5 juillet 2006 16:49, Holger Macht a écrit :
> [...]
> So I'm doing a second apprach and I've got a different way of solving the
> problem. It's implemented as a HAL addon which summarizes all cpufreq
> capable CPUs into six DBus methods on the
> org.freedesktop.Hal.Device.SystemPowerManagement interface.

I personally like the idea. That's one of the missing pieces on HAL 
powermanagement features and I'd really like to see the feature available.

I'd like to make a few comments on the proposed interface.

> It claims the following DBus interfaces on the
> /org/freedesktop/Hal/devices/computer device:
>
> Set a specific CPU frequency governor
> -------------------------------------
>   Interface: org.freedesktop.Hal.Device.SystemPowerManagement
>   Member   : "SetCPUFreqGovernor"
>   Type     : Method call with return
>
>   Param:
>     string : The governor to be set. This can be any arbitary string. Also
>            governors which are not known can be set
>
>   Return type:
>     int    : 0 on success, 1 on error

Shouldn't the return type be a boolean?
That looks odd to use an int here.

> Set the performance independently from the current governor
> -----------------------------------------------------------
>   Interface: org.freedesktop.Hal.Device.SystemPowerManagement
>   Member   : "SetCPUFreqPerformance"
>   Type     : Method call with return
>
>   Param:
>     int    : Value between 1 and 100. The higher the value the more
>            performance you get. 50 is default and should be sufficient
>            in most cases

Original, but why not after. It's original to have a fine grained control like 
this.

>   Return type:
>     int    : 0 on success, 1 on error or if performance settings are not
>            supported for the current governor

Same as above, a boolean looks better to me.

> Set if niced processes should be consider when calculating CPU load
> --------------------------------------------------------------------
>   Interface: org.freedesktop.Hal.Device.SystemPowerManagement
>   Member   : "SetCPUFreqConsiderNice"
>   Type     : Method call with return
>
>   Param:
>     boolean: True if niced processes should be considered, false otherwise
>
>   Return type:
>     int    : 0 on success, 1 on error or if this setting is not supported
>            with the current governor

I'm not completely sure about the use on this. Could you elaborate a bit more?

> Get the current active governor
> -------------------------------
>   Interface: org.freedesktop.Hal.Device.SystemPowerManagement
>   Member   : "GetCPUFreqGovernor"
>   Type     : Method call with return
>
>   Return type:
>     string : The current active governor
>
> Get the current performance setting
> -----------------------------------
>   Interface: org.freedesktop.Hal.Device.SystemPowerManagement
>   Member   : "GetCPUFreqPerformance"
>   Type     : Method call with return
>
>   Return type:
>     int    : The current performance setting from 1 to 100
>
> Get the current consider nice setting
> -------------------------------------
>   Interface: org.freedesktop.Hal.Device.SystemPowerManagement
>   Member   : "GetCPUFreqConsiderNice"
>   Type     : Method call with return
>
>   Return type:
>     boolean: True if niced processes are considered, false otherwise
>

I think one method is missing, I would expect a method to list the available 
CPU governors.

Now I'll let people more knowledgeable like David also comment on the 
patch. ;-)

Regards.
-- 
Kévin 'ervin' Ottens, http://ervin.ipsquad.net
"Ni le maître sans disciple, Ni le disciple sans maître,
Ne font reculer l'ignorance."

Attachment: pgpoBhFSapCYR.pgp
Description: PGP signature

_______________________________________________
powersave-devel mailing list
[email protected]
http://forge.novell.com/mailman/listinfo/powersave-devel

Reply via email to