From mboxrd@z Thu Jan 1 00:00:00 1970 From: Keith Packard Subject: Re: [PATCH 1/2] drm/i915: Check VBIOS value for determining LVDS dual channel mode, too Date: Fri, 16 Mar 2012 16:18:03 -0700 Message-ID: <861uosdtjo.fsf@sumi.keithp.com> References: <1331934073-24463-1-git-send-email-tiwai@suse.de> <1331934073-24463-2-git-send-email-tiwai@suse.de> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from keithp.com (home.keithp.com [63.227.221.253]) by gabe.freedesktop.org (Postfix) with ESMTP id 4FB19A0FCD for ; Fri, 16 Mar 2012 16:18:07 -0700 (PDT) In-Reply-To: <1331934073-24463-2-git-send-email-tiwai@suse.de> 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: intel-gfx@lists.freedesktop.org Cc: Takashi Iwai , hcb@chaoticmind.net List-Id: intel-gfx@lists.freedesktop.org <#part sign=pgpmime> On Fri, 16 Mar 2012 22:41:12 +0100, Takashi Iwai wrote: > +/* read the initial LVDS register value for the given panel mode */ > +static unsigned int get_lvds_reg_val(const struct bdb_header *bdb, > + const struct bdb_lvds_lfp_data_ptrs *ptrs, > + int index, > + struct drm_display_mode *mode) To follow the style of intel_bios.c, I think it would make sense to have the function: static const struct lvds_dvo_timing * get_lvds_fp_timing(const struct bdb_lvds_lfp_data *lvds_lfp_data, const struct bdb_lvds_lfp_data_ptrs *lvds_lfp_data_ptrs, int index) then use the results of this in parse_lfp_panel_data, instead of putting the whole computation into this new function. I'd also like to see this code only use the BDB value when the LVDS is disabled at startup time; otherwise, we'll be changing the behavior for all LVDS users, and as BIOS tables are notoriously unreliable, I fear that we'll cause a lot more problems than we solve. -- keith.packard@intel.com