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

2012-03-16 Thread Rodrigo Vivi
Reviewed-by: Rodrigo Vivi rodrigo.v...@gmail.com

On Thu, Mar 15, 2012 at 11:42 AM, Takashi Iwai ti...@suse.de wrote:
 At Thu, 15 Mar 2012 14:30:35 +0100,
 Takashi Iwai wrote:

 At Thu, 15 Mar 2012 13:25:08 +,
 Chris Wilson wrote:
 
  On Thu, 15 Mar 2012 14:15:54 +0100, Takashi Iwai ti...@suse.de wrote:
   +static bool is_dual_link_lvds(struct drm_i915_private *dev_priv)
   +{
   + /* 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;
   + if ((I915_READ(PCH_LVDS)  LVDS_CLKB_POWER_MASK) ==
 
  This is either PCH_LVDS or LVDS depending on the generation.

 Oh, right.  The revised patch is below.

 One more change.  Now it checks the resolution to be sure.


 Takashi

 ---
 From: Takashi Iwai ti...@suse.de
 Subject: [PATCH v3] drm/i915: Check VBIOS value for determining LVDS dual 
 channel mode, too

 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.

 Signed-off-by: Takashi Iwai ti...@suse.de
 ---
 v1-v2: Fix the register for gen=4
 v2-v3: Check the resolution of the entry to be sure

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

[Intel-gfx] [PATCH 0/2] Two patches relating to ring initialization failure

2012-03-16 Thread Sean Paul
These two patches fix/mitigate an issue I was seeing during suspend/resume. I
would see a blt ring initialization failed error in the logs, followed by a
NULL dereference in gen6_render_ring_flush shortly after. The first patch adds
a BUG_ON to catch the NULL, and a bread crumb. The second patch fixes the init
error by waiting for a little bit before failing the read (I found the register
values were correct after waiting a bit).

Sean Paul (2):
  drm/i915: Add BUG_ON when ring-private is NULL
  drm/i915: Add wait_for in init_ring_common

 drivers/gpu/drm/i915/intel_ringbuffer.c |   16 
 1 files changed, 12 insertions(+), 4 deletions(-)

-- 
1.7.7.3

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


[Intel-gfx] [PATCH 1/2] drm/i915: Add BUG_ON when ring-private is NULL

2012-03-16 Thread Sean Paul
I have experienced a number of NULL pointer dereferences on
suspend/resume where ring-private is NULL. These come from failed
initialization of the ring buffer. Sincce this doesn't seem to be easily
recoverable, this patch adds a BUG_ON and a bread crumb pointer for
people who might encounter this in the future.

Signed-off-by: Sean Paul seanp...@chromium.org
---
 drivers/gpu/drm/i915/intel_ringbuffer.c |   10 +-
 1 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index ca3972f..8357822 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -198,9 +198,17 @@ gen6_render_ring_flush(struct intel_ring_buffer *ring,
 {
u32 flags = 0;
struct pipe_control *pc = ring-private;
-   u32 scratch_addr = pc-gtt_offset + 128;
+   u32 scratch_addr;
int ret;
 
+   /* This condition was being hit when the ring buffer wasn't being
+* properly initialized and subsequently cleaned (hence setting
+* ring-private to NULL).
+*/
+   BUG_ON(pc == NULL);
+
+   scratch_addr = pc-gtt_offset + 128;
+
/* Force SNB workarounds for PIPE_CONTROL flushes */
intel_emit_post_sync_nonzero_flush(ring);
 
-- 
1.7.7.3

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


[Intel-gfx] [PATCH 2/2] drm/i915: Add wait_for in init_ring_common

2012-03-16 Thread Sean Paul
I have seen a number of blt ring initialization failed messages
where the ctl or start registers are not the correct value. Upon further
inspection, if the code just waited a little bit, it would read the
correct value. Adding the wait_for to these reads should eliminate the
issue.

Signed-off-by: Sean Paul seanp...@chromium.org
---
 drivers/gpu/drm/i915/intel_ringbuffer.c |6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 8357822..d95f0f9 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -298,9 +298,9 @@ static int init_ring_common(struct intel_ring_buffer *ring)
| RING_REPORT_64K | RING_VALID);
 
/* If the head is still not zero, the ring is dead */
-   if ((I915_READ_CTL(ring)  RING_VALID) == 0 ||
-   I915_READ_START(ring) != obj-gtt_offset ||
-   (I915_READ_HEAD(ring)  HEAD_ADDR) != 0) {
+   if (wait_for((I915_READ_CTL(ring)  RING_VALID) != 0 
+I915_READ_START(ring) == obj-gtt_offset 
+(I915_READ_HEAD(ring)  HEAD_ADDR) == 0, 50)) {
DRM_ERROR(%s initialization failed 
ctl %08x head %08x tail %08x start %08x\n,
ring-name,
-- 
1.7.7.3

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


Re: [Intel-gfx] [PATCH 2/2] drm/i915: Add wait_for in init_ring_common

2012-03-16 Thread Ben Widawsky
On Fri, Mar 16, 2012 at 12:43:22PM -0400, Sean Paul wrote:
 I have seen a number of blt ring initialization failed messages
 where the ctl or start registers are not the correct value. Upon further
 inspection, if the code just waited a little bit, it would read the
 correct value. Adding the wait_for to these reads should eliminate the
 issue.
 
 Signed-off-by: Sean Paul seanp...@chromium.org
Reviewed-by: Ben Widawsky b...@bwidawsk.net

 ---
  drivers/gpu/drm/i915/intel_ringbuffer.c |6 +++---
  1 files changed, 3 insertions(+), 3 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
 b/drivers/gpu/drm/i915/intel_ringbuffer.c
 index 8357822..d95f0f9 100644
 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
 +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
 @@ -298,9 +298,9 @@ static int init_ring_common(struct intel_ring_buffer 
 *ring)
   | RING_REPORT_64K | RING_VALID);
  
   /* If the head is still not zero, the ring is dead */
 - if ((I915_READ_CTL(ring)  RING_VALID) == 0 ||
 - I915_READ_START(ring) != obj-gtt_offset ||
 - (I915_READ_HEAD(ring)  HEAD_ADDR) != 0) {
 + if (wait_for((I915_READ_CTL(ring)  RING_VALID) != 0 
 +  I915_READ_START(ring) == obj-gtt_offset 
 +  (I915_READ_HEAD(ring)  HEAD_ADDR) == 0, 50)) {
   DRM_ERROR(%s initialization failed 
   ctl %08x head %08x tail %08x start %08x\n,
   ring-name,
 -- 
 1.7.7.3
 
 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


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

2012-03-16 Thread Adam Jackson

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.


Looks really good otherwise, thanks!

Reviewed-by: Adam Jackson a...@redhat.com

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


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

2012-03-16 Thread Takashi Iwai
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.

 Looks really good otherwise, thanks!
 
 Reviewed-by: Adam Jackson a...@redhat.com


Thanks!

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


Re: [Intel-gfx] Graphics switching and LVDS detection

2012-03-16 Thread Oliver Seitz



How could this be implemented in a clean way? Of course forcing LVDS on
should become a parameter,


A patch for this was made some time ago, but it was not accepted in the 
end. Concerns were that using those force-parameters could be advertised 
in internet forums and people use them accidentally without 
understanding the purpose.


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


[Intel-gfx] [PATCH 0/2 v4] drm/i915: LVDS channel mode fixes

2012-03-16 Thread Takashi Iwai
Hi,

this is the revised patch for fixing the wrong LVDS channel mode,
e.g. when booted while the lid is closed.  Also, a new module option
to override the LVDS channel mode is added in the second patch.

v1-v2: Fix the register for gen=4
v2-v3: Check the resolution of the entry to be sure
v3-v4: Optimize the register reference; add a module option

thanks,

Takashi

___
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 rodrigo.v...@gmail.com
Reviewed-by: Adam Jackson a...@redhat.com
Signed-off-by: Takashi Iwai ti...@suse.de
---
 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-gfx] [PATCH 2/2] drm/i915: Add lvds_channel module option

2012-03-16 Thread Takashi Iwai
Add a new module optoin lvds_channel to specify the LVDS channel mode
explicitly instead of probing the LVDS register value set by BIOS.
This will be helpful when VBT is broken or incompatible with the
current code.

Signed-off-by: Takashi Iwai ti...@suse.de
---
 drivers/gpu/drm/i915/i915_drv.c  |6 ++
 drivers/gpu/drm/i915/i915_drv.h  |1 +
 drivers/gpu/drm/i915/intel_display.c |5 +
 3 files changed, 12 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 308f819..eee6804 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -80,6 +80,12 @@ MODULE_PARM_DESC(lvds_downclock,
Use panel (LVDS/eDP) downclocking for power savings 
(default: false));
 
+int i915_lvds_channel __read_mostly;
+module_param_named(lvds_channel, i915_lvds_channel, int, 0400);
+MODULE_PARM_DESC(lvds_channel,
+Specify LVDS channel mode 
+(0=probe BIOS [default], 1=single-channel, 2=dual-channel));
+
 int i915_panel_use_ssc __read_mostly = -1;
 module_param_named(lvds_use_ssc, i915_panel_use_ssc, int, 0600);
 MODULE_PARM_DESC(lvds_use_ssc,
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8c8e488..1af2b53 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1014,6 +1014,7 @@ extern int i915_panel_ignore_lid __read_mostly;
 extern unsigned int i915_powersave __read_mostly;
 extern int i915_semaphores __read_mostly;
 extern unsigned int i915_lvds_downclock __read_mostly;
+extern int i915_lvds_channel __read_mostly;
 extern int i915_panel_use_ssc __read_mostly;
 extern int i915_vbt_sdvo_panel_type __read_mostly;
 extern int i915_enable_rc6 __read_mostly;
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index d7e6b76..87a7fb9 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -360,6 +360,11 @@ static bool is_dual_link_lvds(struct drm_i915_private 
*dev_priv,
  unsigned int reg)
 {
unsigned int val;
+
+   /* use the module option value if specified */
+   if (i915_lvds_channel  0)
+   return i915_lvds_channel == 2;
+
/* 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
-- 
1.7.9.2

___
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 ti...@suse.de 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


Re: [Intel-gfx] [PATCH 2/2] drm/i915: Add lvds_channel module option

2012-03-16 Thread Eugeni Dodonov
On Fri, Mar 16, 2012 at 18:41, Takashi Iwai ti...@suse.de wrote:

 Add a new module optoin lvds_channel to specify the LVDS channel mode
 explicitly instead of probing the LVDS register value set by BIOS.
 This will be helpful when VBT is broken or incompatible with the
 current code.

 Signed-off-by: Takashi Iwai ti...@suse.de
 ---
  drivers/gpu/drm/i915/i915_drv.c  |6 ++
  drivers/gpu/drm/i915/i915_drv.h  |1 +
  drivers/gpu/drm/i915/intel_display.c |5 +
  3 files changed, 12 insertions(+)

 diff --git a/drivers/gpu/drm/i915/i915_drv.c
 b/drivers/gpu/drm/i915/i915_drv.c
 index 308f819..eee6804 100644
 --- a/drivers/gpu/drm/i915/i915_drv.c
 +++ b/drivers/gpu/drm/i915/i915_drv.c
 @@ -80,6 +80,12 @@ MODULE_PARM_DESC(lvds_downclock,
Use panel (LVDS/eDP) downclocking for power savings 
(default: false));

 +int i915_lvds_channel __read_mostly;
 +module_param_named(lvds_channel, i915_lvds_channel, int, 0400);


As a minor bikeshedding, perhaps this variable would make more sense if
called 'i915_lvds_channel_mode' instead?

I am fine with either spelling though, it just looks more semantically
correct to me. But in any case,

Reviewed-by: Eugeni Dodonov eugeni.dodo...@intel.com

-- 
Eugeni Dodonov
http://eugeni.dodonov.net/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx