On Tue, Feb 12, 2013 at 07:37:08PM +0200, Jani Nikula wrote: > On Tue, 12 Feb 2013, Chris Wilson <chris at chris-wilson.co.uk> wrote: > > On Tue, Feb 12, 2013 at 07:06:59PM +0200, Jani Nikula wrote: > >> Keep all the platform output selection in intel_output_setup(), and don't > >> scatter it around. > > > > I see this as doing the opposite. You are littering an already over > > complicated routine with LVDS specific information. > > I think it's fairly straightforward to follow what's happening there, > and with all the platform dependent stuff in the same place you don't > have to dig down from that complicated routine to look for *other* > places where there are platform dependent ifs and checks. Especially if > we need to add *more* of those deep down. I consider the rest of those to be a mess of repetious code that would have been simpler had the platform specific presence checks been pushed down. The only critical aspect of that function is the ordering and more importantly when to and when not to initialise certain outputs. That I feel is hidden in the forest of ifs. Just my 2 cents. -Chris -- Chris Wilson, Intel Open Source Technology Centre