i915: lvds panel always blank when booted with lid closed

i915: lvds panel always blank when booted with lid closed

[Seth Forshee]

When I boot my Thinkpad T410 in a docking station with the lid closed,
the lvds panel remains blank even when this output is active. This
happens up to and including 3.5-rc2.

I've determined that this happens because lvds isn't being initialized
by the bios when I boot this way, and booting with lvds_channel_mode=2
fixes the issue. I see that there's logic in is_dual_link_lvds()
intended to detect this situation, but it's failing because the T410 has
the LVDS_PIPEB_SELECT bit set. The simple patch below fixes my machine
by masking off this bit when determining whether or not lvds was
initialized by the bios.

I'm not sure though whether or not it's correct to expect that this bit
might be set when lvds hasn't been initialized. The alternative seems to
be quirking this machine as is done for some Macbooks. What is the
correct solution?

Thanks,
Seth


>From 250904ac95cda7630cdd8339724e3c8feceeb586 Mon Sep 17 00:00:00 2001
From: Seth Forshee <seth.forshee@canonical.com>
Date: Tue, 12 Jun 2012 16:52:14 -0500
Subject: [PATCH] drm/i915: ignore LVDS_PIPEB_SELECT when checking for LVDS
 register initialization

The Lenovo Thinkpad T410 has this bit set in the LVDS register when
booted with the lid closed, even though the LVDS hasn't really been
initialized. Ignore this bit so that the VBT value will be used instead.

Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
---
 drivers/gpu/drm/i915/intel_display.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e0aa064..f81f249 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -434,7 +434,7 @@ static bool is_dual_link_lvds(struct drm_i915_private *dev_priv,
 		 * register is uninitialized.
 		 */
 		val = I915_READ(reg);
-		if (!(val & ~LVDS_DETECTED))
+		if (!(val & ~(LVDS_PIPEB_SELECT | LVDS_DETECTED)))
 			val = dev_priv->bios_lvds_val;
 		dev_priv->lvds_val = val;
 	}

Re: i915: lvds panel always blank when booted with lid closed

[Daniel Vetter]

On Wed, Jun 13, 2012 at 09:36:00AM -0500, Seth Forshee wrote:
> When I boot my Thinkpad T410 in a docking station with the lid closed,
> the lvds panel remains blank even when this output is active. This
> happens up to and including 3.5-rc2.
> 
> I've determined that this happens because lvds isn't being initialized
> by the bios when I boot this way, and booting with lvds_channel_mode=2
> fixes the issue. I see that there's logic in is_dual_link_lvds()
> intended to detect this situation, but it's failing because the T410 has
> the LVDS_PIPEB_SELECT bit set. The simple patch below fixes my machine
> by masking off this bit when determining whether or not lvds was
> initialized by the bios.
> 
> I'm not sure though whether or not it's correct to expect that this bit
> might be set when lvds hasn't been initialized. The alternative seems to
> be quirking this machine as is done for some Macbooks. What is the
> correct solution?

Patch looks correct, but can you please change the mask from PIPEB to the
already exsting LVDS_PIPE_MASK? This way it's clearer what's going on.

Thanks, Daniel
> 
> Thanks,
> Seth
> 
> 
> From 250904ac95cda7630cdd8339724e3c8feceeb586 Mon Sep 17 00:00:00 2001
> From: Seth Forshee <seth.forshee@canonical.com>
> Date: Tue, 12 Jun 2012 16:52:14 -0500
> Subject: [PATCH] drm/i915: ignore LVDS_PIPEB_SELECT when checking for LVDS
>  register initialization
> 
> The Lenovo Thinkpad T410 has this bit set in the LVDS register when
> booted with the lid closed, even though the LVDS hasn't really been
> initialized. Ignore this bit so that the VBT value will be used instead.
> 
> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index e0aa064..f81f249 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -434,7 +434,7 @@ static bool is_dual_link_lvds(struct drm_i915_private *dev_priv,
>  		 * register is uninitialized.
>  		 */
>  		val = I915_READ(reg);
> -		if (!(val & ~LVDS_DETECTED))
> +		if (!(val & ~(LVDS_PIPEB_SELECT | LVDS_DETECTED)))
>  			val = dev_priv->bios_lvds_val;
>  		dev_priv->lvds_val = val;
>  	}
> 

-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

[PATCH v2] drm/i915: ignore pipe select bit when checking for LVDS register initialization

[Seth Forshee]

The Lenovo Thinkpad T410 has the LVDS_PIPEB_SELECT bit set in the LVDS
register when booted with the lid closed, even though the LVDS hasn't
really been initialized. Ignore this bit so that the VBT value will be
used instead.

Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
---
 drivers/gpu/drm/i915/intel_display.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e0aa064..ae17526 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -434,7 +434,7 @@ static bool is_dual_link_lvds(struct drm_i915_private *dev_priv,
 		 * register is uninitialized.
 		 */
 		val = I915_READ(reg);
-		if (!(val & ~LVDS_DETECTED))
+		if (!(val & ~(LVDS_PIPE_MASK | LVDS_DETECTED)))
 			val = dev_priv->bios_lvds_val;
 		dev_priv->lvds_val = val;
 	}
-- 
1.7.9.5

Re: [PATCH v2] drm/i915: ignore pipe select bit when checking for LVDS register initialization

[Daniel Vetter]

On Wed, Jun 13, 2012 at 01:46:58PM -0500, Seth Forshee wrote:
> The Lenovo Thinkpad T410 has the LVDS_PIPEB_SELECT bit set in the LVDS
> register when booted with the lid closed, even though the LVDS hasn't
> really been initialized. Ignore this bit so that the VBT value will be
> used instead.
> 
> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
Queued for -next, thanks for the patch. Chris had some reservations about
the sanity of this patch, but given that it works around bios-insanity I'm
gonna just take this chance to stab myself with lvds-machines blowing up
left and right ;-)
-Daniel
> ---
>  drivers/gpu/drm/i915/intel_display.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index e0aa064..ae17526 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -434,7 +434,7 @@ static bool is_dual_link_lvds(struct drm_i915_private *dev_priv,
>  		 * register is uninitialized.
>  		 */
>  		val = I915_READ(reg);
> -		if (!(val & ~LVDS_DETECTED))
> +		if (!(val & ~(LVDS_PIPE_MASK | LVDS_DETECTED)))
>  			val = dev_priv->bios_lvds_val;
>  		dev_priv->lvds_val = val;
>  	}
> -- 
> 1.7.9.5
> 

-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: [PATCH v2] drm/i915: ignore pipe select bit when checking for LVDS register initialization

[Daniel Vetter]

On Wed, Jun 13, 2012 at 01:46:58PM -0500, Seth Forshee wrote:
> The Lenovo Thinkpad T410 has the LVDS_PIPEB_SELECT bit set in the LVDS
> register when booted with the lid closed, even though the LVDS hasn't
> really been initialized. Ignore this bit so that the VBT value will be
> used instead.
> 
> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
Queued for -next, thanks for the patch. Chris had some reservations about
the sanity of this patch, but given that it works around bios-insanity I'm
gonna just take this chance to stab myself with lvds-machines blowing up
left and right ;-)
-Daniel
> ---
>  drivers/gpu/drm/i915/intel_display.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index e0aa064..ae17526 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -434,7 +434,7 @@ static bool is_dual_link_lvds(struct drm_i915_private *dev_priv,
>  		 * register is uninitialized.
>  		 */
>  		val = I915_READ(reg);
> -		if (!(val & ~LVDS_DETECTED))
> +		if (!(val & ~(LVDS_PIPE_MASK | LVDS_DETECTED)))
>  			val = dev_priv->bios_lvds_val;
>  		dev_priv->lvds_val = val;
>  	}
> -- 
> 1.7.9.5
> 

-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: [PATCH v2] drm/i915: ignore pipe select bit when checking for LVDS register initialization

[Seth Forshee]

On Wed, Jun 13, 2012 at 09:46:15PM +0200, Daniel Vetter wrote:
> On Wed, Jun 13, 2012 at 01:46:58PM -0500, Seth Forshee wrote:
> > The Lenovo Thinkpad T410 has the LVDS_PIPEB_SELECT bit set in the LVDS
> > register when booted with the lid closed, even though the LVDS hasn't
> > really been initialized. Ignore this bit so that the VBT value will be
> > used instead.
> > 
> > Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
> Queued for -next, thanks for the patch. Chris had some reservations about
> the sanity of this patch, but given that it works around bios-insanity I'm
> gonna just take this chance to stab myself with lvds-machines blowing up
> left and right ;-)

Let's hope that doesn't happen ;)

I do find myself wondering though whether it might be better to prefer
the value from the VBT whenever there's one available, and only rely on
the actual register value as a fallback, since the bios can't be trusted
to initialize the register. I'm pretty ignorant about all this graphics
stuff though; I assume there's a reason it isn't done this way?

Seth

Re: [PATCH v2] drm/i915: ignore pipe select bit when checking for LVDS register initialization

[Daniel Vetter]

On Wed, Jun 13, 2012 at 03:26:00PM -0500, Seth Forshee wrote:
> On Wed, Jun 13, 2012 at 09:46:15PM +0200, Daniel Vetter wrote:
> > On Wed, Jun 13, 2012 at 01:46:58PM -0500, Seth Forshee wrote:
> > > The Lenovo Thinkpad T410 has the LVDS_PIPEB_SELECT bit set in the LVDS
> > > register when booted with the lid closed, even though the LVDS hasn't
> > > really been initialized. Ignore this bit so that the VBT value will be
> > > used instead.
> > > 
> > > Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
> > Queued for -next, thanks for the patch. Chris had some reservations about
> > the sanity of this patch, but given that it works around bios-insanity I'm
> > gonna just take this chance to stab myself with lvds-machines blowing up
> > left and right ;-)
> 
> Let's hope that doesn't happen ;)
> 
> I do find myself wondering though whether it might be better to prefer
> the value from the VBT whenever there's one available, and only rely on
> the actual register value as a fallback, since the bios can't be trusted
> to initialize the register. I'm pretty ignorant about all this graphics
> stuff though; I assume there's a reason it isn't done this way?

The usual reasoning is that if it's in the register, it's the value that
makes something show up on the screen and hence has a higher change of
being right. Yep, BIOS routinely store total garbage in vbt (or the newer
OpRegion) and somehow fix that up when copying things to the hw :(
Obviously there's also the other case where the hw values aren't set up,
in which case we try to try to fall back to vbt values.
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48