Hi Thomas,
As I mention, struct mgag200_pll_values should store the PLL values. But the individual compute and set functions for each hardware revision mess this up completely. Sometimes they use 'function values' sometimes they use 'register values'. If you'd try to debug a failed modeset operation and have to look at the PLL, the stored values would be meaningless, because there's simply no logic behind it.
So this is the reason for this chage - and then it makes perferct sense to do it. Without this explanation is was to my eay useless chrunch, but this explanation makes sense.
The purpose of this patch is to make all compute functions store function values in the struct and make all update function compute the register values internally.
Do you think the change would be easier to understand if I change the original _set_plls() functions *before* splitting them into compute and update steps?
Na, this would still be very difficult to track down. The only way I would trust myself doing a proper review would be do code it myself and compare the final result. Alas, I am not going to do this.
I will take a look again when you post the next revision, and unless I stumble on something I can ack the code as in I have at least looked at all the code changes. That should be enough to have it committed and the time will tell if there is some fall-out.
Sam