Re: thermal governor: does it actually work??

[Peter Feuerer]

Hi Boris, Hi Rui,

@Rui, would be great, if you could review the patch and verify, whether
everything I'm telling is the truth, thanks.


Borislav Petkov writes:

> On Sun, Feb 17, 2013 at 04:41:57PM +0100, Peter Feuerer wrote:
> > Don't think so, I think this was already in since 2.6.<something> and
> > I assume with this patch applied, acerhdf works from at least this
> > 2.6.<something> up to new 3.8 and will still work in the future.
>
> Well, it can't be because there wouldn't be breakage otherwise, right?
> I've been running 3.5 on the atom for a long time and it was ok but 3.8
> is showing the issue. So something *has* changed.

I think previous 3.7 were two methods possible, the method with one trippoint,
on which the state gets switched on and off using this code in thermal_sys.c:
1060                                if (temp >= trip_temp)
1061                                        cdev->ops->set_cur_state(cdev, 1);
1062                                else
1063                                        cdev->ops->set_cur_state(cdev, 0);


And the other method with all the different trip points, which is used after 
applying my patch.
The older way has been removed intentionally or not, I don't know. But as
acerhdf was depending on this old way, it stopped working.


> But, as you say above, I guess the two-point scheme of acerhdf doesn't
> have a governor that fits. So maybe the correct solution is to have an
> "on_off" stupid governor which gets used by acerhdf and does simply call
> into acerhdf to switch on the fan to auto above a specified trip point.
>
> It could be a lot of overhead for nothing in the end, though.

I think adding a two-point governor would maybe be the somehow cleaner way...
Rui, what do you think, is there a way, you could provide a two-point governor
with upper temperature for turning on the fan and a lower temperature for
turning it off again?


> > >Maybe the critical and hot types need to go here? I.e., 3 and 4?
> >
> > Yes, crit has to go there.
>
> Right, I'll wait for that version of the patch to test. :)

I added crit to the patch below.