Re: [PATCH] drm/i915: move LVDS support check to output setup

[Jani Nikula <jani.nikula@intel.com>]

On Tue, 12 Feb 2013, Chris Wilson <chris@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.

>> As a useful side effect, do not try to enable LVDS on
>> HSW or VLV.
>
> But you wouldn't with the old arrangement either.

On HSW it would depend on I915_READ(PCH_LVDS) & LVDS_DETECTED, which I
believe it shouldn't, and VBT. On mobile VLV it depends on VBT only. And
I think we know better than the VBT. Unless I'm missing something and
you have to spell it out for me... :/

>> Some checks are done in a slightly different order than before, and on some
>> platforms VGA is now initialized before LVDS.
>
> Is that significant?

I don't think so, but I thought I'd write it down as a note just in case
it turns out to be significant.

> You have not sold me on the benefits of this change.

The main motivation was:
http://mid.gmane.org/CAKMK7uGxFYUZckSbdKqqF15Lgcp9VQgKwCU1-hhTQ0VmrzvY2Q@mail.gmail.com


BR,
Jani.