CPUFreq addon in HAL

[David Zeuthen]

On Fri, 2006-07-14 at 20:04 +0200, Holger Macht wrote:
> Well, usually I'm the one arguing for having a possibility to export all
> possible features to the user on the desktop ;-) But...

Hoho, the tables have changed! <-- joke

> This doesn't really fit into the concept of this addon. It would be a
> method that can only exclusively used if the userspace governor is
> set. And that contradics the concept to use common interfaces for all
> governors. And to be honest, we all hope that the userspace governor
> wouldn't be needed anymore someday. However, it would be possible. It
> would need another method SetCPUFreqFrequency and another property
> 'cpufreq_available_frequencies'.  But I really like to avoid that. This
> would also be something which we could add afterwards if it turns out be
> be _really_ needed.

Yeah.

> One question though. What do you exactly mean with "throw an
> exception". Throw it over DBus? Something like
> 
> Return Value:
>        int32 : boolean
>        string: The exception

Nope, basically the return value is "void" (or whatever else if you want
to return a value on success) and on success you simply return without
throwing an exception. If you throw an exception you don't give a return
value. However, the exception is named and also got a detail field.

This works because you only want to throw exceptions when the method
fails. It's pretty much like C++ and/or Java .

> 
> And the exception is something like
> org.freedesktop.Hal.Device.SystemPowerManagement.UnknownGovernor or maybe even
> org.freedesktop.Hal.Device.SystemPowerManagement.Success on success?

Yep. Except you don't throw exceptions on success.

> > What if someone writes
> > into  /sys/devices/system/cpu/cpu0/cpufreq/scaling_governor ? Maybe our
> > answer is just "don't do that", that would be somewhat fine with me...
> 
> When thinking about this from this standpoint, yes, it can change. I
> always think that if you have a process that controls CPUFreq, noone
> should change the settings manually, it's his own fault. I thought a
> little bit more about this and a few problems came to my mind although I
> at first thought we should have HAL properties. Especially when thinking
> about the case that addon-cpufreq is not used to advantage another
> daemon. However, one importance with HAL should be that the date
> abstracted has to be correct and consistent IMHO. And this can't be
> ensured in this case IMO. See below...
> 
> > 
> > 
> > > But again, I don't know which device
> > > object to use for this. Would
> > > org.freedesktop.Hal.Device.SystemPowerManagement be ok?
> > 
> > I think we already use power_management.* namespace for properties. We
> > probably should have used system_power_management to match the D-BUS
> > interface name. I suggest to use
> > 
> >  power_management.current_cpufreq_governor      (string)
> >  power_management.current_cpufreq_performance   (uint64)
> >  power_management.current_cpufreq_consider_nice (bool)
> 
> Same as above applies here. You don't always have these properties, for
> instance when the powersave governor is set or again, if another CPUFreq
> system is used. So I would have to dynamically remove and add properties
> depending on the current governor or if the cpufreq addon is used at
> all. I think that's overkill.

I agree.

> > 
> > > > How about a function to get a list of available governors?
> > > 
> > > Kevin already proposed it and I can add it. But if the other settings are
> > > exported as hal property, wouldn't it make sense to do the same for the
> > > available governors?
> > 
> > Good point, so we'd have
> > 
> >  power_management.available_cpufreq_governors      (strlist)
> 
> When writing this mail, at first I thought that this property has the only
> real good reasons to exist because this shouldn't change underneath us,
> except if modules are loaded or removed. But there's another critical
> point about this. Every distribution has to squeamishly pay attention to
> that the cpufreq modules are loaded before HAL starts up. And if they're
> loaded afterwards, boom, you don't have the correct information exported
> by HAL.
> 
> As a conclusion, I think this all can be done as properties, but it's far
> more clean and easier to have DBus method to get all this information.

OK. Makes sense to me.

> I will look at this addon again and will test again when doing the next,
> hopefully final patch.

Sounds good. Thanks!

Cheers,
David