From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jani Nikula Subject: Re: [PATCH] drm/i915: move LVDS support check to output setup Date: Tue, 12 Feb 2013 19:37:08 +0200 Message-ID: <877gmdwi0r.fsf@intel.com> References: <1360688819-31968-1-git-send-email-jani.nikula@intel.com> <20130212171749.GB17406@cantiga.alporthouse.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mga03.intel.com (mga03.intel.com [143.182.124.21]) by gabe.freedesktop.org (Postfix) with ESMTP id 44C34E5C60 for ; Tue, 12 Feb 2013 09:37:49 -0800 (PST) In-Reply-To: <20130212171749.GB17406@cantiga.alporthouse.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Chris Wilson Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Tue, 12 Feb 2013, Chris Wilson 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.