site_archiver@lists.apple.com Delivered-To: darwin-dev@lists.apple.com 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 (Darwin-dev@lists.apple.com) Help/Unsubscribe/Update your Subscription: http://lists.apple.com/mailman/options/darwin-dev/site_archiver%40lists.appl... This email sent to site_archiver@lists.apple.com