At Wed, 14 Mar 2012 14:09:19 -0400,
Adam Jackson wrote:
>
> On Wed, 2012-03-14 at 17:44 +0100, Takashi Iwai wrote:
> > At Wed, 14 Mar 2012 10:45:06 -0400,
> > Adam Jackson wrote:
> > > There may or may not be a bit for this in the VBT in the BIOS. But the
> > > more reliably correct thing I suspect would be to just look at the
> > > preferred mode for the panel and assume it's dual-link LVDS if the pixel
> > > clock is >112MHz, since that's the crossover frequency.
> >
> > Coincidently, we hit the same issue with a HP laptop, and wondered how
> > is the best way to fix. I hoped BIOS could handle better,
> > i.e. setting the power bits no matter whether the lid is opened or
> > not. But it doesn't set unless the lid is once opened.
> >
> > (Interestingly, the bits remain even if you close the lid again before
> > booting. Just opening once seems triggering the probing of LVDS
> > panel in BIOS and let it setting the right values.)
> >
> > FWIW, when I check ironalke_crtc_mode_set(), the clock of the HD+
> > LVDS mode (1600x900) is 107800 (refclk 120000), while a similar
> > machine with a HD panel (1366x768) shows 76300. So, I'm not sure
> > whether 112MHz could be a right threshold.
>
> Hm, fair point.
>
> Would be interesting to compare the VBTs between the two boots. The
> lvds_fp_timing struct might actually have the data we want here
> regardless of how the machine booted.
It seems that BIOS doesn't change VTBs at boot no matter whether the
lid is opened or not. It just skips the LVDS initialization in it.
I dumbed the VBIOS data in both lid-open and close cases, and they are
identical.
Then I strated looking at VBT entries, and compared the outputs of two
similar machines with different resolutions. Then I found out that a
different value is set in lvds_fp_timing.lvds_reg_val for the default
panel mode indeed. (This isn't set in all entries but only in the
native one.)
The patch below is to add a check of this LVDS reg value in VBT.
It worked on a couple of HP laptops here. Can anyone check whether it
works (and doesn't regress) on other machines, preferably from other
vendors?
Ideally it would be safer if we can verify the initial LVDS value in
VBT more strictly...
thanks,
Takashi
---
From: Takashi Iwai <tiwai at suse.de>
Subject: [PATCH] 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 <tiwai at suse.de>
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/intel_bios.c | 22 ++++++++++++++++++++++
drivers/gpu/drm/i915/intel_display.c | 26 ++++++++++++++++++++------
3 files changed, 43 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..e04f06d 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -173,6 +173,25 @@ 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)
+{
+ 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);
+ if (timing->lvds_reg_val == -1)
+ return 0; /* just to be sure */
+ 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 +262,9 @@ 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);
}
/* 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..2a61dab 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)
+{
+ /* 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) ==
+ LVDS_CLKB_POWER_UP)
+ return true;
+ return false;
+}
+
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)) {
/* LVDS dual channel */
if (refclk == 100000)
limit = &intel_limits_ironlake_dual_lvds_100m;
@@ -393,8 +409,7 @@ static const intel_limit_t *intel_g4x_limit(struct drm_crtc *crtc)
const intel_limit_t *limit;
if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS)) {
- if ((I915_READ(LVDS) & LVDS_CLKB_POWER_MASK) ==
- LVDS_CLKB_POWER_UP)
+ if (is_dual_link_lvds(dev_priv))
/* LVDS with dual channel */
limit = &intel_limits_g4x_dual_channel_lvds;
else
@@ -531,8 +546,7 @@ intel_find_best_PLL(const intel_limit_t *limit, struct drm_crtc *crtc,
* reliably set up different single/dual channel state, if we
* even can.
*/
- if ((I915_READ(LVDS) & LVDS_CLKB_POWER_MASK) ==
- LVDS_CLKB_POWER_UP)
+ if (is_dual_link_lvds(dev_priv))
clock.p2 = limit->p2.p2_fast;
else
clock.p2 = limit->p2.p2_slow;
--
1.7.9.2
