<div class="gmail_quote">On Mon, Apr 23, 2012 at 10:59, Eugeni Dodonov <span dir="ltr"><<a href="mailto:eugeni@dodonov.net">eugeni@dodonov.net</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

<div class="gmail_quote"><div class="im">On Fri, Apr 20, 2012 at 13:11, Chris Wilson <span dir="ltr"><<a href="mailto:chris@chris-wilson.co.uk" target="_blank">chris@chris-wilson.co.uk</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">


<div>From: Jesse Barnes <<a href="mailto:jbarnes@virtuousgeek.org" target="_blank">jbarnes@virtuousgeek.org</a>><br>
<br>
PCH PLLs aren't required for outputs on the CPU, so we shouldn't just<br>
treat them as part of the pipe.<br>
<br>
So split the code out and manage PCH PLLs separately, allocating them<br>
when needed or trying to re-use existing PCH PLL setups when the timings<br>
match.<br>
<br>
v2: add num_pch_pll field to dev_priv (Daniel)<br>
    don't NULL the pch_pll pointer in disable or DPMS will fail (Jesse)<br>
    put register offsets in pll struct (Chris)<br>
<br>
v3: Decouple enable/disable of PLLs from get/put.<br>
v4: Track temporary PLL disabling during modeset<br>
</div><div>v5: Tidy PLL initialisation by only checking for num_pch_pll == 0 (Eugeni)<br>
</div>v6: Avoid mishandling allocation failure by embedding the small array of<br>
    PLLs into the device struct<br>
<div><br>
Bugzilla: <a href="https://bugs.freedesktop.org/show_bug.cgi?id=44309" target="_blank">https://bugs.freedesktop.org/show_bug.cgi?id=44309</a><br>
Signed-off-by: Jesse Barnes <<a href="mailto:jbarnes@virtuousgeek.org" target="_blank">jbarnes@virtuousgeek.org</a>> (up to v2)<br>
Signed-off-by: Chris Wilson <<a href="mailto:chris@chris-wilson.co.uk" target="_blank">chris@chris-wilson.co.uk</a>> (v3+)<br></div></blockquote><div><br></div></div><div>As I talked with Jesse and Daniel over irc, I hit some WARN with LPT with those patches, but it is not the fault of the patch itself as it is due to differences with Lynx Point/Haswell we have, for which I still reuse those code paths. In the end, I think we'll end up using a different path for Haswell-onwards for crtc enabling sequence, and those will be gone.</div>


<div><br></div><div>So besides those items, the v6 looks correct to me, and works in my test cases, and I haven't found anything else to bikeshed about so far. So:</div><div>Reviewed-by: Eugeni Dodonov <<a href="mailto:eugeni.dodonov@intel.com" target="_blank">eugeni.dodonov@intel.com</a>></div>

</div></blockquote><div><br></div><div>Also, I think I am feeling brave enough now to add a:</div><div>Tested-by: Eugeni Dodonov <<a href="mailto:eugeni.dodonov@intel.com">eugeni.dodonov@intel.com</a>></div><div><br>

</div><div>after giving it a run on on SNB and IVB here.</div></div><div><br></div>-- <br>Eugeni Dodonov<a href="http://eugeni.dodonov.net/" target="_blank"><br></a><br>