<div class="gmail_quote">On Fri, Apr 13, 2012 at 18:11, Chris Wilson <span dir="ltr"><<a href="mailto:chris@chris-wilson.co.uk">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 class="im">On Fri, 13 Apr 2012 17:08:57 -0300, Eugeni Dodonov <<a href="mailto:eugeni.dodonov@intel.com">eugeni.dodonov@intel.com</a>> wrote:<br>
</div><div class="im">> -     if (IS_HASWELL(dev))<br>
> +     if (IS_HASWELL(dev)) {<br>
>               intel_init_power_wells(dev);<br>
> +             intel_hsw_prepare_ddi_buffers(dev);<br>
</div>Give this a much more generic, more grandiose name and move the<br>
generation specific routines down a level:<br>
  intel_init_ddi() [but make it more verbose and self-documenting!]<br></blockquote><div><br></div><div>Will do.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

When you do that you'll might consider it to actually a part of the<br>
display initialisation and so move it down below intel_init_display().<br>
Remember it is vital for our sanity that we be able to concisely describe<br>
the sequence of steps required to initialise the GPU.<br></blockquote><div><br></div><div>My initial idea here was to re-use the IS_HASWELL check to do all the platform-specific stuff in one place, but with the suggestion above it makes it much easier to move the intel_ddi_init into more logical position.</div>

<div><br></div><div>The specs ask to make sure that the DDI buffers translations are configured correctly before attempting to enable them, so I thought on doing them early in the process, so the further step assume that everything is up and ready to use already. But yes, I believe that it could be done from within intel_init_display as well, and it will be more logical indeed.</div>

<div><br></div><div>In both cases, I agree that it will make the driver code much more self-explainable and easier to maintain. Huge thanks! I'll send a new version with those comments addressed later.</div><div><br>
</div>
</div>-- <br>Eugeni Dodonov<a href="http://eugeni.dodonov.net/" target="_blank"><br></a><br>