On Thu, May 22, 2014 at 05:38:13PM -0300, Paulo Zanoni wrote: > 2014-04-24 18:55 GMT-03:00 Daniel Vetter <daniel.vetter@xxxxxxxx>: > > With this all the pch pre-enable work has been moved into the special > > hsw crt encoder functions. > > For the same reasons I gave in the other patches, I'm not convinced > this is an improvement to our code. It looks like we're just breaking > the abstraction exploiting the fact that CRT is the only FDI/PCH > output on Haswell. Now the HSW code will be significantly different > from the ILK/SNB/HSW code, and I really think this can be confusing to > people reading the code. I really think we shouldn't hide things > aren't specific to CRT inside CRT code. See my other mail, imo this does _not_ break the abstraction, but actually fix it. If you look at the crtc/encoder split, the thing that connects these two is the crossbar. So we should put the code into crtc/encoder callbacks so that roughly everything before the crossbar is in crtc callbacks, and everything after it is in encoder callbacks. Of course there will be some small exceptions, e.g. a lot of the ddi port handling (which is after the crossbar) is in crtc code. But that makes imo sense since ddi ports are so nicely unfified between dp, hdmi and fdi. > Since on this series we're basically disagreeing on our opinions on > which coding style is the better, I think we should ask the other > developers to give their opinion too :) I'm not sure more people will help here. But this is an area I'm often struggling on review. E.g. Ben's full ppgtt patches treat the aliasing ppgtt on gen6 in many places like a full ppgtt. Which makes sense since they are both called ppgtt and share a lot of the same low-level code. But from a functional pov the aliasing ppgtt is something completely different from full ppgtt: The first is just a fancy way to write ptes, the 2nd is fundamentally different wrt how buffer objects get mapped into the gpu address space. And I'm pretty sure I've made noises like this back when we've merged the original hsw support ... -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
