Re: [Intel-gfx] [PATCH 1/2] drm/i915: Check VBIOS value for determining LVDS dual channel mode, too

2012-03-22 Thread Paulo Zanoni
2012/3/20 Takashi Iwai :
>
> This patch adds a workaround for that problem by checking the initial
> LVDS register value in VBT.
>
> Reviewed-by: Rodrigo Vivi 
> Reviewed-by: Adam Jackson 
> Signed-off-by: Takashi Iwai 

Tested on 4 laptops. Two of them with docking stations so I didn't
need to open the lid, press the power button and then close the lid.

They were all working before the patch, and they keep working after
the patch. The test was basically booting with closed lid, then after
gdm appears on the vga monitor, open the lid and check if lvds still
works.

Tested-By: Paulo Zanoni 

-- 
Paulo Zanoni
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 1/2] drm/i915: Check VBIOS value for determining LVDS dual channel mode, too

2012-03-21 Thread Daniel Vetter
On Tue, Mar 20, 2012 at 01:07:05PM +0100, Takashi Iwai wrote:
> Currently i915 driver checks [PCH_]LVDS register bits to decide
> whether to set up the dual-link or the single-link mode.  This relies
> implicitly on that BIOS initializes the register properly at boot.
> However, BIOS doesn't initialize it always.  When the machine is
> booted with the closed lid, BIOS skips the LVDS reg initialization.
> This ends up in blank output on a machine with a dual-link LVDS when
> you open the lid after the boot.
> 
> This patch adds a workaround for that problem by checking the initial
> LVDS register value in VBT.
> 
> Reviewed-by: Rodrigo Vivi 
> Reviewed-by: Adam Jackson 
> Signed-off-by: Takashi Iwai 

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=37742

Not tested by the reporter, but sounds exactly like the issue this patch
here tries to fix.
-Daniel
-- 
Daniel Vetter
Mail: dan...@ffwll.ch
Mobile: +41 (0)79 365 57 48
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 1/2] drm/i915: Check VBIOS value for determining LVDS dual channel mode, too

2012-03-20 Thread Rodrigo Vivi
Altough Keith's idea is very good I tested here with many systems that
are already working nowadays and it didn't break anything. Including
atom at 945gme, ironlake, sandybridge and ivybridge... including
single and dual channel modes...

Tested-by: Rodrigo Vivi 

On Tue, Mar 20, 2012 at 11:57 AM, Keith Packard  wrote:
> <#part sign=pgpmime>
> On Tue, 20 Mar 2012 13:04:41 +0100, Takashi Iwai  wrote:
>
>> Since checking the lid state is tricky and unreliable, the practical
>> check would be simply reading the first LVDS reg and seeing whether it
>> was initialized or not.  It seems that it reads to 0x02 when booted
>> with the lid close, which is LVDS_DETECTED bit.
>
> Right, lid-detect is not useful, so I suggested using the new code path
> only if the LVDS was *not* actually running at startup time. That should
> avoid almost all common cases that work correctly today.
>
> --
> keith.pack...@intel.com
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
GPG: 0x905BE242 @ wwwkeys.pgp.net
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 1/2] drm/i915: Check VBIOS value for determining LVDS dual channel mode, too

2012-03-20 Thread Keith Packard
<#part sign=pgpmime>
On Tue, 20 Mar 2012 13:04:41 +0100, Takashi Iwai  wrote:

> Since checking the lid state is tricky and unreliable, the practical
> check would be simply reading the first LVDS reg and seeing whether it
> was initialized or not.  It seems that it reads to 0x02 when booted
> with the lid close, which is LVDS_DETECTED bit.

Right, lid-detect is not useful, so I suggested using the new code path
only if the LVDS was *not* actually running at startup time. That should
avoid almost all common cases that work correctly today.

-- 
keith.pack...@intel.com
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH 1/2] drm/i915: Check VBIOS value for determining LVDS dual channel mode, too

2012-03-20 Thread Takashi Iwai
Currently i915 driver checks [PCH_]LVDS register bits to decide
whether to set up the dual-link or the single-link mode.  This relies
implicitly on that BIOS initializes the register properly at boot.
However, BIOS doesn't initialize it always.  When the machine is
booted with the closed lid, BIOS skips the LVDS reg initialization.
This ends up in blank output on a machine with a dual-link LVDS when
you open the lid after the boot.

This patch adds a workaround for that problem by checking the initial
LVDS register value in VBT.

Reviewed-by: Rodrigo Vivi 
Reviewed-by: Adam Jackson 
Signed-off-by: Takashi Iwai 
---
 drivers/gpu/drm/i915/i915_drv.h  |2 ++
 drivers/gpu/drm/i915/intel_bios.c|   36 ++
 drivers/gpu/drm/i915/intel_display.c |   30 ++--
 3 files changed, 62 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 9689ca3..28397b8 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -376,6 +376,8 @@ typedef struct drm_i915_private {
unsigned int lvds_use_ssc:1;
unsigned int display_clock_mode:1;
int lvds_ssc_freq;
+   unsigned int bios_lvds_val; /* initial [PCH_]LVDS reg val in VBIOS */
+   unsigned int lvds_val; /* used for checking LVDS channel mode */
struct {
int rate;
int lanes;
diff --git a/drivers/gpu/drm/i915/intel_bios.c 
b/drivers/gpu/drm/i915/intel_bios.c
index 63880e2..f71cbd0 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -173,6 +173,28 @@ get_lvds_dvo_timing(const struct bdb_lvds_lfp_data 
*lvds_lfp_data,
return (struct lvds_dvo_timing *)(entry + dvo_timing_offset);
 }
 
+/* get lvds_fp_timing entry
+ * this function may return NULL if the corresponding entry is invalid
+ */
+static const struct lvds_fp_timing *
+get_lvds_fp_timing(const struct bdb_header *bdb,
+  const struct bdb_lvds_lfp_data *data,
+  const struct bdb_lvds_lfp_data_ptrs *ptrs,
+  int index)
+{
+   size_t data_ofs = (const u8 *)data - (const u8 *)bdb;
+   u16 data_size = ((const u16 *)data)[-1]; /* stored in header */
+   size_t ofs;
+
+   if (index >= ARRAY_SIZE(ptrs->ptr))
+   return NULL;
+   ofs = ptrs->ptr[index].fp_timing_offset;
+   if (ofs < data_ofs ||
+   ofs + sizeof(struct lvds_fp_timing) > data_ofs + data_size)
+   return NULL;
+   return (const struct lvds_fp_timing *)((const u8 *)bdb + ofs);
+}
+
 /* Try to find integrated panel data */
 static void
 parse_lfp_panel_data(struct drm_i915_private *dev_priv,
@@ -182,6 +204,7 @@ parse_lfp_panel_data(struct drm_i915_private *dev_priv,
const struct bdb_lvds_lfp_data *lvds_lfp_data;
const struct bdb_lvds_lfp_data_ptrs *lvds_lfp_data_ptrs;
const struct lvds_dvo_timing *panel_dvo_timing;
+   const struct lvds_fp_timing *fp_timing;
struct drm_display_mode *panel_fixed_mode;
int i, downclock;
 
@@ -243,6 +266,19 @@ parse_lfp_panel_data(struct drm_i915_private *dev_priv,
  "Normal Clock %dKHz, downclock %dKHz\n",
  panel_fixed_mode->clock, 10*downclock);
}
+
+   fp_timing = get_lvds_fp_timing(bdb, lvds_lfp_data,
+  lvds_lfp_data_ptrs,
+  lvds_options->panel_type);
+   if (fp_timing) {
+   /* check the resolution, just to be sure */
+   if (fp_timing->x_res == panel_fixed_mode->hdisplay &&
+   fp_timing->y_res == panel_fixed_mode->vdisplay) {
+   dev_priv->bios_lvds_val = fp_timing->lvds_reg_val;
+   DRM_DEBUG_KMS("VBT initial LVDS value %x\n",
+ dev_priv->bios_lvds_val);
+   }
+   }
 }
 
 /* Try to find sdvo panel data */
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index f851db7..f5ed808 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -356,6 +356,27 @@ 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)
+{
+   unsigned int val;
+
+   if (dev_priv->lvds_val)
+   val = dev_priv->lvds_val;
+   else {
+   /* BIOS should set the proper LVDS register value at boot, but
+* in reality, it doesn't set the value when the lid is closed;
+* we need to check "the value to be set" in VBT when LVDS
+* register is uninitialized.
+*/
+   val = I915_READ(reg);
+   if (!(val & ~LVDS_DETECTE

Re: [Intel-gfx] [PATCH 1/2] drm/i915: Check VBIOS value for determining LVDS dual channel mode, too

2012-03-20 Thread Takashi Iwai
At Tue, 20 Mar 2012 11:01:22 +0100,
Daniel Vetter wrote:
> 
> On Mon, Mar 19, 2012 at 12:07:36PM +0100, Takashi Iwai wrote:
> > Currently i915 driver checks [PCH_]LVDS register bits to decide
> > whether to set up the dual-link or the single-link mode.  This relies
> > implicitly on that BIOS initializes the register properly at boot.
> > However, BIOS doesn't initialize it always.  When the machine is
> > booted with the closed lid, BIOS skips the LVDS reg initialization.
> > This ends up in blank output on a machine with a dual-link LVDS when
> > you open the lid after the boot.
> > 
> > This patch adds a workaround for that problem by checking the initial
> > LVDS register value in VBT.
> > 
> > Reviewed-by: Rodrigo Vivi 
> > Reviewed-by: Adam Jackson 
> > Signed-off-by: Takashi Iwai 
> 
> If I understand Keith correctly he would like this to only get used when
> the lid is closed and the panel is off. But I don't see that mention in
> you change log nor can I find how it works in the code. Has this been lost
> or am I confused?

Then I just overlooked that part in Keith's comment.

Since checking the lid state is tricky and unreliable, the practical
check would be simply reading the first LVDS reg and seeing whether it
was initialized or not.  It seems that it reads to 0x02 when booted
with the lid close, which is LVDS_DETECTED bit.

I'll resend the patches again with that fix.


thanks,

Takashi
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 1/2] drm/i915: Check VBIOS value for determining LVDS dual channel mode, too

2012-03-20 Thread Daniel Vetter
On Mon, Mar 19, 2012 at 12:07:36PM +0100, Takashi Iwai wrote:
> Currently i915 driver checks [PCH_]LVDS register bits to decide
> whether to set up the dual-link or the single-link mode.  This relies
> implicitly on that BIOS initializes the register properly at boot.
> However, BIOS doesn't initialize it always.  When the machine is
> booted with the closed lid, BIOS skips the LVDS reg initialization.
> This ends up in blank output on a machine with a dual-link LVDS when
> you open the lid after the boot.
> 
> This patch adds a workaround for that problem by checking the initial
> LVDS register value in VBT.
> 
> Reviewed-by: Rodrigo Vivi 
> Reviewed-by: Adam Jackson 
> Signed-off-by: Takashi Iwai 

If I understand Keith correctly he would like this to only get used when
the lid is closed and the panel is off. But I don't see that mention in
you change log nor can I find how it works in the code. Has this been lost
or am I confused?

Thanks, Daniel
-- 
Daniel Vetter
Mail: dan...@ffwll.ch
Mobile: +41 (0)79 365 57 48
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH 1/2] drm/i915: Check VBIOS value for determining LVDS dual channel mode, too

2012-03-19 Thread Takashi Iwai
Currently i915 driver checks [PCH_]LVDS register bits to decide
whether to set up the dual-link or the single-link mode.  This relies
implicitly on that BIOS initializes the register properly at boot.
However, BIOS doesn't initialize it always.  When the machine is
booted with the closed lid, BIOS skips the LVDS reg initialization.
This ends up in blank output on a machine with a dual-link LVDS when
you open the lid after the boot.

This patch adds a workaround for that problem by checking the initial
LVDS register value in VBT.

Reviewed-by: Rodrigo Vivi 
Reviewed-by: Adam Jackson 
Signed-off-by: Takashi Iwai 
---
 drivers/gpu/drm/i915/i915_drv.h  |1 +
 drivers/gpu/drm/i915/intel_bios.c|   36 ++
 drivers/gpu/drm/i915/intel_display.c |   26 ++--
 3 files changed, 57 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 9689ca3..8c8e488 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -376,6 +376,7 @@ typedef struct drm_i915_private {
unsigned int lvds_use_ssc:1;
unsigned int display_clock_mode:1;
int lvds_ssc_freq;
+   unsigned int bios_lvds_val; /* initial [PCH_]LVDS reg val in VBIOS */
struct {
int rate;
int lanes;
diff --git a/drivers/gpu/drm/i915/intel_bios.c 
b/drivers/gpu/drm/i915/intel_bios.c
index 63880e2..f71cbd0 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -173,6 +173,28 @@ get_lvds_dvo_timing(const struct bdb_lvds_lfp_data 
*lvds_lfp_data,
return (struct lvds_dvo_timing *)(entry + dvo_timing_offset);
 }
 
+/* get lvds_fp_timing entry
+ * this function may return NULL if the corresponding entry is invalid
+ */
+static const struct lvds_fp_timing *
+get_lvds_fp_timing(const struct bdb_header *bdb,
+  const struct bdb_lvds_lfp_data *data,
+  const struct bdb_lvds_lfp_data_ptrs *ptrs,
+  int index)
+{
+   size_t data_ofs = (const u8 *)data - (const u8 *)bdb;
+   u16 data_size = ((const u16 *)data)[-1]; /* stored in header */
+   size_t ofs;
+
+   if (index >= ARRAY_SIZE(ptrs->ptr))
+   return NULL;
+   ofs = ptrs->ptr[index].fp_timing_offset;
+   if (ofs < data_ofs ||
+   ofs + sizeof(struct lvds_fp_timing) > data_ofs + data_size)
+   return NULL;
+   return (const struct lvds_fp_timing *)((const u8 *)bdb + ofs);
+}
+
 /* Try to find integrated panel data */
 static void
 parse_lfp_panel_data(struct drm_i915_private *dev_priv,
@@ -182,6 +204,7 @@ parse_lfp_panel_data(struct drm_i915_private *dev_priv,
const struct bdb_lvds_lfp_data *lvds_lfp_data;
const struct bdb_lvds_lfp_data_ptrs *lvds_lfp_data_ptrs;
const struct lvds_dvo_timing *panel_dvo_timing;
+   const struct lvds_fp_timing *fp_timing;
struct drm_display_mode *panel_fixed_mode;
int i, downclock;
 
@@ -243,6 +266,19 @@ parse_lfp_panel_data(struct drm_i915_private *dev_priv,
  "Normal Clock %dKHz, downclock %dKHz\n",
  panel_fixed_mode->clock, 10*downclock);
}
+
+   fp_timing = get_lvds_fp_timing(bdb, lvds_lfp_data,
+  lvds_lfp_data_ptrs,
+  lvds_options->panel_type);
+   if (fp_timing) {
+   /* check the resolution, just to be sure */
+   if (fp_timing->x_res == panel_fixed_mode->hdisplay &&
+   fp_timing->y_res == panel_fixed_mode->vdisplay) {
+   dev_priv->bios_lvds_val = fp_timing->lvds_reg_val;
+   DRM_DEBUG_KMS("VBT initial LVDS value %x\n",
+ dev_priv->bios_lvds_val);
+   }
+   }
 }
 
 /* Try to find sdvo panel data */
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index f851db7..d7e6b76 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)
+{
+   unsigned int val;
+   /* 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)
+   val = dev_priv->bios_lvds_val;
+   else
+   val = I915_READ(reg);
+   return (val & LVDS_CLKB_POWER_MASK) == LVDS_CLKB_POWER_UP;
+}
+
 stat

Re: [Intel-gfx] [PATCH 1/2] drm/i915: Check VBIOS value for determining LVDS dual channel mode, too

2012-03-18 Thread Takashi Iwai
At Sat, 17 Mar 2012 12:01:17 -0700,
Keith Packard wrote:
> 
> <#part sign=pgpmime>
> On Sat, 17 Mar 2012 08:59:56 +0100, Takashi Iwai  wrote:
> 
> > Well, the LVDS reg data isn't in lvds_dvo_timing but in lvds_fp_timing,
> > thus you need to look at a different entry in anyway.
> 
> Right, a parallel function which returns the lvds_fp_timing structure
> instead of pulling the data out of it. Just makes the code look more
> like the existing stuff. And, if we need more data from the
> lvds_fp_timing in the future, we'll have it available directly.

Ah, OK I got your point now.  This makes sense.
I'll rewrite the patch tomorrow.


> > I skipped it to simplify the code, but that'd be safer, indeed.
> 
> Reducing the chances for regressions is my primary concern here; most
> people boot their machines with the lid open, so if we avoid the new
> code in that case, we'll be more likely to not break things.

Yes, that's my biggest concern too.
So I'd really appreciate if anyone else checks whether the patch
doesn't break, at least (and helps sometimes).


thanks,

Takashi

> 
> -- 
> keith.pack...@intel.com
> 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 1/2] drm/i915: Check VBIOS value for determining LVDS dual channel mode, too

2012-03-17 Thread Keith Packard
<#part sign=pgpmime>
On Sat, 17 Mar 2012 08:59:56 +0100, Takashi Iwai  wrote:

> Well, the LVDS reg data isn't in lvds_dvo_timing but in lvds_fp_timing,
> thus you need to look at a different entry in anyway.

Right, a parallel function which returns the lvds_fp_timing structure
instead of pulling the data out of it. Just makes the code look more
like the existing stuff. And, if we need more data from the
lvds_fp_timing in the future, we'll have it available directly.

> I skipped it to simplify the code, but that'd be safer, indeed.

Reducing the chances for regressions is my primary concern here; most
people boot their machines with the lid open, so if we avoid the new
code in that case, we'll be more likely to not break things.

-- 
keith.pack...@intel.com
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 1/2] drm/i915: Check VBIOS value for determining LVDS dual channel mode, too

2012-03-17 Thread Takashi Iwai
At Fri, 16 Mar 2012 16:18:03 -0700,
Keith Packard wrote:
> 
> <#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.

Well, the LVDS reg data isn't in lvds_dvo_timing but in lvds_fp_timing,
thus you need to look at a different entry in anyway.

> 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.

I skipped it to simplify the code, but that'd be safer, indeed.

(Though, the chance of getting a wrong value is fairly small since
 the function verifies whether the resolution of the entry matches
 with the given mode, too.)


thanks,

Takashi
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 1/2] drm/i915: Check VBIOS value for determining LVDS dual channel mode, too

2012-03-16 Thread Keith Packard
<#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.pack...@intel.com
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH 1/2] drm/i915: Check VBIOS value for determining LVDS dual channel mode, too

2012-03-16 Thread Takashi Iwai
Currently i915 driver checks [PCH_]LVDS register bits to decide
whether to set up the dual-link or the single-link mode.  This relies
implicitly on that BIOS initializes the register properly at boot.
However, BIOS doesn't initialize it always.  When the machine is
booted with the closed lid, BIOS skips the LVDS reg initialization.
This ends up in blank output on a machine with a dual-link LVDS when
you open the lid after the boot.

This patch adds a workaround for that problem by checking the initial
LVDS register value in VBT.

Reviewed-by: Rodrigo Vivi 
Reviewed-by: Adam Jackson 
Signed-off-by: Takashi Iwai 
---
 drivers/gpu/drm/i915/i915_drv.h  |1 +
 drivers/gpu/drm/i915/intel_bios.c|   26 ++
 drivers/gpu/drm/i915/intel_display.c |   26 --
 3 files changed, 47 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 9689ca3..8c8e488 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -376,6 +376,7 @@ typedef struct drm_i915_private {
unsigned int lvds_use_ssc:1;
unsigned int display_clock_mode:1;
int lvds_ssc_freq;
+   unsigned int bios_lvds_val; /* initial [PCH_]LVDS reg val in VBIOS */
struct {
int rate;
int lanes;
diff --git a/drivers/gpu/drm/i915/intel_bios.c 
b/drivers/gpu/drm/i915/intel_bios.c
index 63880e2..6b93f92 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -173,6 +173,27 @@ get_lvds_dvo_timing(const struct bdb_lvds_lfp_data 
*lvds_lfp_data,
return (struct lvds_dvo_timing *)(entry + dvo_timing_offset);
 }
 
+/* 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)
+{
+   unsigned int ofs;
+   const struct lvds_fp_timing *timing;
+
+   if (index >= ARRAY_SIZE(ptrs->ptr))
+   return 0;
+   ofs = ptrs->ptr[index].fp_timing_offset;
+   if (ofs + sizeof(*timing) > bdb->bdb_size)
+   return 0;
+   timing = (const struct lvds_fp_timing *)((const u8 *)bdb + ofs);
+   /* check the resolution, just to be sure */
+   if (timing->x_res != mode->hdisplay || timing->y_res != mode->vdisplay)
+   return 0;
+   return timing->lvds_reg_val;
+}
+
 /* Try to find integrated panel data */
 static void
 parse_lfp_panel_data(struct drm_i915_private *dev_priv,
@@ -243,6 +264,11 @@ parse_lfp_panel_data(struct drm_i915_private *dev_priv,
  "Normal Clock %dKHz, downclock %dKHz\n",
  panel_fixed_mode->clock, 10*downclock);
}
+
+   dev_priv->bios_lvds_val = get_lvds_reg_val(bdb, lvds_lfp_data_ptrs,
+  lvds_options->panel_type,
+  panel_fixed_mode);
+   DRM_DEBUG_KMS("VBT initial LVDS value %x\n", dev_priv->bios_lvds_val);
 }
 
 /* Try to find sdvo panel data */
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index f851db7..d7e6b76 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)
+{
+   unsigned int val;
+   /* 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)
+   val = dev_priv->bios_lvds_val;
+   else
+   val = I915_READ(reg);
+   return (val & LVDS_CLKB_POWER_MASK) == LVDS_CLKB_POWER_UP;
+}
+
 static const intel_limit_t *intel_ironlake_limit(struct drm_crtc *crtc,
int refclk)
 {
@@ -364,8 +381,7 @@ static const intel_limit_t *intel_ironlake_limit(struct 
drm_crtc *crtc,
const intel_limit_t *limit;
 
if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS)) {
-   if ((I915_READ(PCH_LVDS) & LVDS_CLKB_POWER_MASK) ==
-   LVDS_CLKB_POWER_UP) {
+   if (is_dual_link_lvds(dev_priv, PCH_LVDS)) {
/* LVDS dual channel */
if (refclk == 10)
limit = &intel_limits_ironlake_dual_lvds_100m;
@@ -393,8 +409,