Possible issues with PowerMac12_1PlatformPlugin
Possible issues with PowerMac12_1PlatformPlugin
- Subject: Possible issues with PowerMac12_1PlatformPlugin
- From: Benjamin Herrenschmidt <email@hidden>
- Date: Tue, 31 Oct 2006 14:59:40 +1100
Hi folks !
In case anybody on this list is interested in picking this up (I don't
own the machine and have no direct interest in seeing that fixed, I just
felt it might be of some interest to some apple folks so I'm posting it
here).
While looking at the various thermal control loops for PowerMac12,1,
I've found what might be bugs in there. I'm not 100% certain as the code
is fairly intricated but here they are anyway. It's based on
AppleMacRISC4PE-185.0.0 source code which is the latest version of this
module available in the Apple public source repository.
Note that all of these might not be bugs, they might be done on purpose,
though that would be nasty...
* PowerMac12_1_SystemFansCtrlLoop.cpp::calculateNewTarget(), unlike the
usual versions of that code, applies the PID values as an offset to the
value -fetched- back from the target control, it's even commented with a
big fat "IMPORTANT NOTE" in the code.
The problem is that the target control is a
PowerMac12_1VirtualControl.cpp, whose implementation of getTargetValue()
does pretty much nothing but call it's superclass, which will get the
target value from the "target-value" of the dictionary. However, as the
name indicates, PowerMac12_1VirtualControl is a "virtual" control,
which sits between a real HW control and one or many loops, doing all
sort of things irrelevant to this discussion... except for one... when
setting a new target value, it sets it to the target real control... but
never to itself as far as I can see. Thus it will never update the
device-tree value that getTargetValue() fetches.
It never calls super::setTargetValue() that is. And I haven't found
anybody calling sendTargetValue() for it neither. In fact,
PowerMac12_1_SystemFansCtrlLoop::sendNewTarget, _unlike_ other PID
control loops, specifically does _not_ call sendTargetValue() on the
target control.
That means that PowerMac12_1_SystemFansCtrlLoop always apply the PID
value as an offset on top of something that is never updated nor
initialized in the plist as far as I can see.
* The exact same thing happens with PowerMac12_1_CPUFanCtrlLoop in the
sense that it also uses getTargetValue() as a reference and never calls
sendTargetValue() while the target is a virtual control, so the
reference is a never updated (and never initialized in the plist) value,
thus I suspect 0 (I haven't verified, no HW at hand).
* That one might be on purpose, but I find it a bit weird. In
PowerMac12_1VirtualControl::setTargetValue, in the case where multiple
control loops are attached. The code will "remember" for each control
loop the value & reference and tick a "control loop was here" flag.
Then, it checks if all loop have been here, and if yes, calculate a new
real value for the target control and clears all the "was here" flags.
That means that basically, it will only send a new value to the target
after all control loops have set at least a new value once. Which means
that the target control will be updated at the lowest common denominator
of the update rates of all control loops. In practice, according to the
plist, this is actually how it's wired with 2 loops: one is ticking at 1
seconds interval and the other at 5.
I just find this weird... having a loop ticking every secodn (thus
supposed to have fast reactivity) but defeated by the fact that the
target control will only be updated based on the slowest of the loops
(every 5 seconds). Of course, there will still be the metastates to make
sure we don't overtemp, but it looks to me like the code is defeating
the purpose of having one loop tick faster than the other...
That's it for now :) It might also be that the public version of that
code isn't what is actually shipped in 10.4.8 too, since darwin latest
update of that module source was around 10.4.4 I think...
Somebody feel free to file a RADAR for it, or just ignore it, as you
wish.
Cheers,
Ben.
_______________________________________________
Do not post admin requests to the list. They will be ignored.
Darwin-dev mailing list (email@hidden)
Help/Unsubscribe/Update your Subscription:
This email sent to email@hidden