[Intel-gfx] Thinkpad T420 and single/dual channel lvds

Takashi Iwai tiwai at suse.de
Sun Mar 18 21:26:21 CET 2012


At Sun, 18 Mar 2012 19:01:21 +0100,
Andreas Heider wrote:
> 
> Am 18.03.2012 um 18:50 schrieb Daniel Vetter:
> > On Fri, Mar 16, 2012 at 09:29:24PM +0100, Takashi Iwai wrote:
> >> At Fri, 16 Mar 2012 15:55:52 -0400,
> >> Adam Jackson wrote:
> >>> 
> >>> On 3/15/12 10:42 AM, Takashi Iwai wrote:
> >>> 
> >>>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >>>> index f851db7..314af26 100644
> >>>> --- a/drivers/gpu/drm/i915/intel_display.c
> >>>> +++ b/drivers/gpu/drm/i915/intel_display.c
> >>>> @@ -356,6 +356,23 @@ static const intel_limit_t intel_limits_ironlake_display_port = {
> >>>>  	.find_pll = intel_find_pll_ironlake_dp,
> >>>>  };
> >>>> 
> >>>> +static bool is_dual_link_lvds(struct drm_i915_private *dev_priv,
> >>>> +			      unsigned int reg)
> >>>> +{
> >>>> +	/* BIOS should set the proper LVDS register value at boot, but
> >>>> +	 * in reality, it doesn't set the value when the lid is closed;
> >>>> +	 * thus when a machine is booted with the lid closed, the LVDS
> >>>> +	 * reg value can't be trusted.  So we need to check "the value
> >>>> +	 * to be set" in VBT at first.
> >>>> +	 */
> >>>> +	if ((dev_priv->bios_lvds_val & LVDS_CLKB_POWER_MASK) ==
> >>>> +	    LVDS_CLKB_POWER_UP)
> >>>> +		return true;
> >>> 
> >>> Would slightly prefer if this was more like:
> >>> 
> >>>     if (dev_priv->bios_lvds_val)
> >>>             return !!(dev_priv->bios_lvds_val & LVDS_CLKB_POWER_MASK);
> >>> 
> >>> Since that way it eliminates some useless register reads in the normal 
> >>> case even for single-link.  Not going to insist on it though.
> >> 
> >> Sounds reasonable, yes.
> >> 
> >> Also, it'd be good to have a module option to override the lvds
> >> channel setup, e.g. lvds_channel=0 for probing BIOS like this,
> >> lvds_channel=1 and 2 are to set the single and dual-channel mode
> >> forcibly, respectively.  If you guys think it's worth, I'll write an
> >> additional patch after fixing this as suggested.
> > 
> > I think we can wait with adding debug options until this blows up. And if
> > there are indeed broken bios out there, we need a full quirk table anyway.
> > 
> > I'll wait for the new patch with Adam's suggestion for merging into -next.
> > -Daniel
> 
> Hi,
> 
> I tried this patch on a Macbook Pro 6,2 and indeed still need to manually set lvds_channel=2. 

OK, that's expected.  Basically my patch would fix only the case where
the default BIOS works but doesn't work in some exceptional case (e.g.
boot with the closed lid).

> It'd be great if this patch with the override parameter or some other way to fix it would end up in mainline.

I think the module option would be good.  It's needed to give an easy
way for testing.  Then we can add the entries in some quirk tables.


thanks,

Takashi



More information about the Intel-gfx mailing list