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
