[PATCH 2/3] [drm/i915] - Implement ability to disabled LVDS fixed mode

[PATCH 2/3] [drm/i915] - Implement ability to disabled LVDS fixed mode

[Mike Isely]

An LVDS connected display device typically has a "fixed" mode to
which its video timings are locked, and that mode is not controllable
by the user - user-specified timings are always ignored.  If the fixed
mode's expected resolution does not match the user-specified frame
buffer's resolution, then the port is programmed to scale the image
(this is how multiple resolutions are handled on an LCD panel and it's
why you get fuzzy LCD resolutions when the chosen resolution doesn't
match the native resolution).

Unfortunately this setup makes it impossible for the user to control
the display's video timings.  This new option implements a new kernel
option, lvds_fixed, which defaults to true.  But when it is set to
false (zero), fixed mode is disabled; all scaling is gone and the
driver will then use the user-specified video timings.  Obviously the
user has to get the timings correct, but this is really not much
different than the bad old days of CRT video modes.

And while we normally don't want to disable fixed mode, the fact of
the matter is that sometimes there's no choice if the driver otherwise
gets the fixed mode "wrong".  This can happen in some embedded systems
where the video BIOS really has no knowledge of the display device.

Warning: With fixed mode disabled, and if there is no other mode data
supplied anywhere (e.g. KMS trying to run a framebuffer for fbcon on
an LVDS console), then the LVDS port will likely simply be disabled.
However that's easily remedied by supplying video mode information on
the kernel command line, for example "video=LVDS-1:800x600-24MR@100"
for a 100Hz 800x600 24bpp resolution.

Signed-off-by: Mike Isely <isely@pobox.com>
---
 drivers/gpu/drm/i915/i915_drv.c   |    3 +++
 drivers/gpu/drm/i915/i915_drv.h   |    1 +
 drivers/gpu/drm/i915/intel_lvds.c |   31 ++++++++++++++++++++++---------
 3 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index c89f71c251acf230db613229eca90d24584b9729..004880aa3a948669b8b4e23d9ad73d132cff81d0 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -46,6 +46,9 @@ module_param_named(fbpercrtc, i915_fbpercrtc, int, 0400);
 unsigned int i915_powersave = 1;
 module_param_named(powersave, i915_powersave, int, 0600);
 
+unsigned int i915_lvds_fixed = 1;
+module_param_named(lvds_fixed, i915_lvds_fixed, int, 0600);
+
 unsigned int i915_lvds_downclock = 0;
 module_param_named(lvds_downclock, i915_lvds_downclock, int, 0400);
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 526cef2972ab9afce1c17f57ef39ce9cc4dc736f..3fa8681459aa596e12e885568e5b48f0c9a60719 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -885,6 +885,7 @@ extern struct drm_ioctl_desc i915_ioctls[];
 extern int i915_max_ioctl;
 extern unsigned int i915_fbpercrtc;
 extern unsigned int i915_powersave;
+extern unsigned int i915_lvds_fixed;
 extern unsigned int i915_lvds_downclock;
 extern unsigned int i915_lvds_24bit;
 
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index fe779b33899af536eb2e76429c9b05c363ce7721..303d60f71ca6dcf4ce7b59fe625ed5377af24bc4 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -157,10 +157,12 @@ static int intel_lvds_mode_valid(struct drm_connector *connector,
 	struct intel_lvds *intel_lvds = intel_attached_lvds(connector);
 	struct drm_display_mode *fixed_mode = intel_lvds->fixed_mode;
 
-	if (mode->hdisplay > fixed_mode->hdisplay)
-		return MODE_PANEL;
-	if (mode->vdisplay > fixed_mode->vdisplay)
-		return MODE_PANEL;
+	if (fixed_mode) {
+		if (mode->hdisplay > fixed_mode->hdisplay)
+			return MODE_PANEL;
+		if (mode->vdisplay > fixed_mode->vdisplay)
+			return MODE_PANEL;
+	}
 
 	return MODE_OK;
 }
@@ -253,7 +255,8 @@ static bool intel_lvds_mode_fixup(struct drm_encoder *encoder,
 	 * with the panel scaling set up to source from the H/VDisplay
 	 * of the original mode.
 	 */
-	intel_fixed_panel_mode(intel_lvds->fixed_mode, adjusted_mode);
+	if (intel_lvds->fixed_mode)
+		intel_fixed_panel_mode(intel_lvds->fixed_mode, adjusted_mode);
 
 	if (HAS_PCH_SPLIT(dev)) {
 		intel_pch_panel_fitting(dev, intel_lvds->fitting_mode,
@@ -488,11 +491,13 @@ static int intel_lvds_get_modes(struct drm_connector *connector)
 	if (intel_lvds->edid)
 		return drm_add_edid_modes(connector, intel_lvds->edid);
 
-	mode = drm_mode_duplicate(dev, intel_lvds->fixed_mode);
-	if (mode == 0)
-		return 0;
+	if (intel_lvds->fixed_mode) {
+		mode = drm_mode_duplicate(dev, intel_lvds->fixed_mode);
+		if (mode == 0)
+			return 0;
 
-	drm_mode_probed_add(connector, mode);
+		drm_mode_probed_add(connector, mode);
+	}
 	return 1;
 }
 
@@ -942,6 +947,14 @@ bool intel_lvds_init(struct drm_device *dev)
 	 *    if closed, act like it's not there for now
 	 */
 
+	if (!i915_lvds_fixed) {
+		DRM_DEBUG_KMS("Skipping any attempt to determine"
+			      " panel fixed mode.\n");
+		goto out;
+	}
+	DRM_DEBUG_KMS("Attempting to determine panel fixed mode.\n");
+
+
 	/*
 	 * Attempt to get the fixed panel mode from DDC.  Assume that the
 	 * preferred mode is the right one.
-- 
1.5.6.5


-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8

Manual control of LVDS pixel formats and dual channel mode

[Chris Wilson]

Mike, I wasn't keen on introducing this option to disable the LVDS fixed
mode. I think we want to allow the user to specify the fixed mode to use
instead, without the intermediate stop-gap solution of completely disabling
the probe. Later we can extend that ability (if possible or still desirable
on reflection) so that we can specify all the LVDS configuration options
in the single string.

But first can you check that I've correctly forward ported your other
patches on top of Eric's refactoring.

Many thanks,
-Chris

[PATCH 1/2] drm/i915: Implement direct support for 24 bit LVDS pixel format

[Chris Wilson]

From: Mike Isely <isely@isely.net>

LVDS digital data can be formatted as 18 bits/pixel or one of two
24 bits/pixel possibilities.  All are incompatible with one another,
so the GPU's LVDS output has to be configured to match the format
expected by the display device.

In many (most) cases this is generally not a problem because the LVDS
device is integral to the processor board (e.g. a laptop) and thus the
video BIOS already sets this up correctly as part of the boot
process.  Then the i915 drm simply works with what has been already
set up.

But there are cases where the LVDS-driven display and the GPU are
discrete components - this can happen in embedded environments where
the processor board is a COTS device with its own BIOS and the display
is added to it later.  In that situation the video BIOS on the
processor board can't know anything about the LVDS display which leads
to problems if the pixel format is not 18 bit (usually 18 bit is the
default).

This patch implements a new kernel option for the i915 kernel module:
"lvds_24bit".  The default value of zero preserves the previous "don't
change anything" behavior.  If it is set to "1" or "2" then 24 bit
format is enabled - the choice between "1" and "2" selects the
particular 24 bit format.  If it is set to "3" then 24 format is
specifically disabled, which should be an extremely rare case but is
included for completeness.

There was a similar patch back in 2008 to support 24 bit LVDS with the
user-mode xorg intel driver, using a new driver option in the
xorg.conf file.  However that patch was a casualty of the move to
kernel mode switching.  This patch implements the same sort of
solution, just now it's in the kernel drm driver for i915 driven GPUs.

Signed-off-by: Mike Isely <isely@pobox.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_drv.c      |    4 ++
 drivers/gpu/drm/i915/i915_drv.h      |    1 +
 drivers/gpu/drm/i915/i915_reg.h      |    8 ++++-
 drivers/gpu/drm/i915/intel_display.c |   52 ++++++++++++++++++++++++++++-----
 4 files changed, 56 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 1146abd..29ee1e3 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -67,6 +67,10 @@ module_param_named(vbt_sdvo_panel_type, i915_vbt_sdvo_panel_type, int, 0600);
 static bool i915_try_reset = true;
 module_param_named(reset, i915_try_reset, bool, 0600);
 
+unsigned int i915_lvds_24bit = 0;
+module_param_named(lvds_24bit, i915_lvds_24bit, int, 0600);
+MODULE_PARM_DESC(lvds_24bit, "LVDS 24 bit pixel format: 0=leave untouched (default), 1=24 bit '2.0' format, 2=24 bit '2.1' format, 3=force older 18 bit format");
+
 static struct drm_driver driver;
 extern int intel_agp_enabled;
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d1fadb8..97153f0 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -981,6 +981,7 @@ extern unsigned int i915_lvds_downclock;
 extern unsigned int i915_panel_use_ssc;
 extern int i915_vbt_sdvo_panel_type;
 extern unsigned int i915_enable_rc6;
+extern unsigned int i915_lvds_24bit;
 
 extern int i915_suspend(struct drm_device *dev, pm_message_t state);
 extern int i915_resume(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 024e01f..99fa52d 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1453,7 +1453,13 @@
 /* LVDS sync polarity flags. Set to invert (i.e. negative) */
 #define   LVDS_VSYNC_POLARITY		(1 << 21)
 #define   LVDS_HSYNC_POLARITY		(1 << 20)
-
+/*
+ * Selects between .0 and .1 formats:
+ *
+ * 0 = 1x18.0, 2x18.0, 1x24.0 or 2x24.0
+ * 1 = 1x24.1 or 2x24.1
+ */
+#define LVDS_DATA_FORMAT_DOT_ONE	(1 << 24)
 /* Enable border for unscaled (or aspect-scaled) display */
 #define   LVDS_BORDER_ENABLE		(1 << 15)
 /*
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 0d316e9..f84dd21 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4225,6 +4225,44 @@ static inline bool intel_panel_use_ssc(struct drm_i915_private *dev_priv)
 	return dev_priv->lvds_use_ssc && i915_panel_use_ssc;
 }
 
+static void intel_lvds_select_pixel_format(u32 *val)
+{
+	/* Control the output pixel format. */
+	switch (i915_lvds_24bit) {
+	default:
+	case 0:
+		/* 0 means don't mess with 18 vs 24 bit LVDS pixel
+		 * format.  Instead we trust whatever the video
+		 * BIOS should have done to set up the panel.
+		 * This is normally the safest choice since most
+		 * LVDS-connected panels are integral to the
+		 * system and thus the video BIOS knows how to set
+		 * it up appropriately. */
+		break;
+	case 1:
+		/* Enable 24 bit pixel mode using the "2.0" format */
+		*val |= LVDS_A3_POWER_UP;
+		*val &= ~LVDS_DATA_FORMAT_DOT_ONE;
+		break;
+	case 2:
+		/* Enable 24 bit pixel mode using the "2.1"
+		 * format; this choice is equivalent to the
+		 * LVDS24BitMode option in the old pre-KMS user
+		 * space driver. */
+		*val |= LVDS_A3_POWER_UP;
+		*val |= LVDS_DATA_FORMAT_DOT_ONE;
+		break;
+	case 3:
+		/* Enable 18 bit pixel mode - this should be a
+		   very rare case since this is usually the
+		   power-up mode if the video BIOS didn't set
+		   things up.  But it's here for completeness. */
+		*val &= ~LVDS_A3_POWER_UP;
+		*val &= ~LVDS_DATA_FORMAT_DOT_ONE;
+		break;
+	}
+}
+
 static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
 			      struct drm_display_mode *mode,
 			      struct drm_display_mode *adjusted_mode,
@@ -4479,10 +4517,8 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
 		else
 			temp &= ~(LVDS_B0B3_POWER_UP | LVDS_CLKB_POWER_UP);
 
-		/* It would be nice to set 24 vs 18-bit mode (LVDS_A3_POWER_UP)
-		 * appropriately here, but we need to look more thoroughly into how
-		 * panels behave in the two modes.
-		 */
+		intel_lvds_select_pixel_format(&temp);
+
 		/* set the dithering flag on LVDS as needed */
 		if (INTEL_INFO(dev)->gen >= 4) {
 			if (dev_priv->lvds_dither)
@@ -4506,6 +4542,7 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
 			temp &= ~(LVDS_HSYNC_POLARITY | LVDS_VSYNC_POLARITY);
 			temp |= lvds_sync;
 		}
+
 		I915_WRITE(LVDS, temp);
 	}
 
@@ -5001,10 +5038,8 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 		else
 			temp &= ~(LVDS_B0B3_POWER_UP | LVDS_CLKB_POWER_UP);
 
-		/* It would be nice to set 24 vs 18-bit mode (LVDS_A3_POWER_UP)
-		 * appropriately here, but we need to look more thoroughly into how
-		 * panels behave in the two modes.
-		 */
+		intel_lvds_select_pixel_format(&temp);
+
 		if (adjusted_mode->flags & DRM_MODE_FLAG_NHSYNC)
 			lvds_sync |= LVDS_HSYNC_POLARITY;
 		if (adjusted_mode->flags & DRM_MODE_FLAG_NVSYNC)
@@ -5021,6 +5056,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 			temp &= ~(LVDS_HSYNC_POLARITY | LVDS_VSYNC_POLARITY);
 			temp |= lvds_sync;
 		}
+
 		I915_WRITE(PCH_LVDS, temp);
 	}
 
-- 
1.7.4.1

[PATCH 2/2] drm/i915: Implement manual override of LVDS single/dual channel mode

[Chris Wilson]

From: Mike Isely <isely@isely.net>

The logic for LVDS setup in the Intel driver needs to know whether the
LVDS port should be in single or dual channel mode when calculating
video timing.  It had been answering this question by probing the
current hardware configuration, under the assumption that the video
BIOS would have already set it up.  But the video BIOS would actually
have to know how to set up the LVDS port for the connected device,
which is a bad assumption if the display device is not integral to the
processor board - a situation that can exist for embedded situation.
This is yet one more case where the Intel driver had been implicitly
relying on the video BIOS for display configuration.

This changes creates a new kernel option, lvds_channels, which can be
used to override the probe.  Setting this allows the user to specify
the number of channels in use, avoiding the possibly erroneous probe
of the hardware.  Almost nobody should ever need to touch this option,
and its default value of zero is interpreted to preserve existing
probe-the-hardware behavior.  But if the video BIOS gets this "wrong",
then it can be overridden by setting lvds_channels to 1 or 2,
indicating single or dual channel LVDS mode, respectively.

Signed-off-by: Mike Isely <isely@isely.net>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_drv.c      |    4 ++++
 drivers/gpu/drm/i915/i915_drv.h      |    1 +
 drivers/gpu/drm/i915/intel_display.c |   32 +++++++++++++++++---------------
 3 files changed, 22 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 29ee1e3..16a2532 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -71,6 +71,10 @@ unsigned int i915_lvds_24bit = 0;
 module_param_named(lvds_24bit, i915_lvds_24bit, int, 0600);
 MODULE_PARM_DESC(lvds_24bit, "LVDS 24 bit pixel format: 0=leave untouched (default), 1=24 bit '2.0' format, 2=24 bit '2.1' format, 3=force older 18 bit format");
 
+unsigned int i915_lvds_channels = 0;
+module_param_named(lvds_channels, i915_lvds_channels, int, 0600);
+MODULE_PARM_DESC(lvds_channels, "LVDS channels in use: 0=(default) probe hardware 1=single 2=dual");
+
 static struct drm_driver driver;
 extern int intel_agp_enabled;
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 97153f0..2112af3 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -977,6 +977,7 @@ extern unsigned int i915_fbpercrtc;
 extern int i915_panel_ignore_lid;
 extern unsigned int i915_powersave;
 extern unsigned int i915_semaphores;
+extern unsigned int i915_lvds_channels;
 extern unsigned int i915_lvds_downclock;
 extern unsigned int i915_panel_use_ssc;
 extern int i915_vbt_sdvo_panel_type;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index f84dd21..7734d1e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -354,6 +354,17 @@ static const intel_limit_t intel_limits_ironlake_display_port = {
         .find_pll = intel_find_pll_ironlake_dp,
 };
 
+static bool intel_lvds_is_dual_channel_mode(struct drm_i915_private *dev_priv,
+					    int lvds_reg)
+{
+	/* Did the user specify the number of channels? */
+	if (i915_lvds_channels)
+		return i915_lvds_channels == 2;
+
+	/* No, let's probe the current status of the hardware instead. */
+	return (I915_READ(lvds_reg) & LVDS_CLKB_POWER_MASK) == LVDS_CLKB_POWER_UP;
+}
+
 static const intel_limit_t *intel_ironlake_limit(struct drm_crtc *crtc,
 						int refclk)
 {
@@ -362,8 +373,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 (intel_lvds_is_dual_channel_mode(dev_priv, PCH_LVDS)) {
 			/* LVDS dual channel */
 			if (refclk == 100000)
 				limit = &intel_limits_ironlake_dual_lvds_100m;
@@ -391,8 +401,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 (intel_lvds_is_dual_channel_mode(dev_priv, LVDS))
 			/* LVDS with dual channel */
 			limit = &intel_limits_g4x_dual_channel_lvds;
 		else
@@ -523,14 +532,7 @@ intel_find_best_PLL(const intel_limit_t *limit, struct drm_crtc *crtc,
 
 	if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS) &&
 	    (I915_READ(LVDS)) != 0) {
-		/*
-		 * For LVDS, if the panel is on, just rely on its current
-		 * settings for dual-channel.  We haven't figured out how to
-		 * 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 (intel_lvds_is_dual_channel_mode(dev_priv, LVDS))
 			clock.p2 = limit->p2.p2_fast;
 		else
 			clock.p2 = limit->p2.p2_slow;
@@ -594,8 +596,8 @@ intel_g4x_find_best_PLL(const intel_limit_t *limit, struct drm_crtc *crtc,
 			lvds_reg = PCH_LVDS;
 		else
 			lvds_reg = LVDS;
-		if ((I915_READ(lvds_reg) & LVDS_CLKB_POWER_MASK) ==
-		    LVDS_CLKB_POWER_UP)
+
+		if (intel_lvds_is_dual_channel_mode(dev_priv, lvds_reg))
 			clock.p2 = limit->p2.p2_fast;
 		else
 			clock.p2 = limit->p2.p2_slow;
@@ -4914,7 +4916,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 	if (is_lvds) {
 		if ((intel_panel_use_ssc(dev_priv) &&
 		     dev_priv->lvds_ssc_freq == 100) ||
-		    (I915_READ(PCH_LVDS) & LVDS_CLKB_POWER_MASK) == LVDS_CLKB_POWER_UP)
+		    intel_lvds_is_dual_channel_mode(dev_priv, PCH_LVDS))
 			factor = 25;
 	} else if (is_sdvo && is_tv)
 		factor = 20;
-- 
1.7.4.1

Re: Manual control of LVDS pixel formats and dual channel mode

[Mike Isely]

On Wed, 13 Apr 2011, Chris Wilson wrote:

> Mike, I wasn't keen on introducing this option to disable the LVDS fixed
> mode. I think we want to allow the user to specify the fixed mode to use
> instead, without the intermediate stop-gap solution of completely disabling
> the probe. Later we can extend that ability (if possible or still desirable
> on reflection) so that we can specify all the LVDS configuration options
> in the single string.

Chris:

Keep in mind that if the probe action is returning wrong results, that's 
worse than no results.  This isn't a question about "supplying missing 
data" so much as it a case of "overriding bad data".  Without that fixed 
mode disabling ability, it is very possible that the panel will simply 
be set up wrong with a corrupted display, with no possible means for the 
user to repair the problem.  That's what led me to make the change, and 
that same change had been living peacefully in the userspace xorg 
driver, up until the point that UMS had been removed from it.  Right now 
without the ability to disable fixed mode there simply *is no* 
workaround.

Now with that said I understand where you're driving at here and if we 
can get a means to actually specify an independent fixed mode solution 
then that clearly also solves the problem and I agree it's more elegant.  
My only worry is the fact that the more elegant solution is also more 
complex may result in there being no solution implemented, which is even 
worse than the other two choices (disable fixed mode vs specifying fixed 
mode).  I guess I just want to make sure that if the ability to disable 
fixed mode is not merged that there at least be continuous progress 
towards being able to set the fixed mode.  And I'm willing to do 
whatever I can to help code and/or test there.


> 
> But first can you check that I've correctly forward ported your other
> patches on top of Eric's refactoring.

Thanks.  Will do.  Stay tuned.

  -Mike


-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8

Re: Manual control of LVDS pixel formats and dual channel mode

[Chris Wilson]

On Wed, 13 Apr 2011 02:27:18 -0500 (CDT), Mike Isely <isely@isely.net> wrote:
> Keep in mind that if the probe action is returning wrong results, that's 
> worse than no results.  This isn't a question about "supplying missing 
> data" so much as it a case of "overriding bad data".  Without that fixed 
> mode disabling ability, it is very possible that the panel will simply 
> be set up wrong with a corrupted display, with no possible means for the 
> user to repair the problem.  That's what led me to make the change, and 
> that same change had been living peacefully in the userspace xorg 
> driver, up until the point that UMS had been removed from it.  Right now 
> without the ability to disable fixed mode there simply *is no* 
> workaround.

Point taken. If we can't get a resolution on i915.lvds_fixed_mode= within
the next week, let's focus on the workaround instead.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: Manual control of LVDS pixel formats and dual channel mode

[Mike Isely]

On Wed, 13 Apr 2011, Chris Wilson wrote:

> On Wed, 13 Apr 2011 02:27:18 -0500 (CDT), Mike Isely <isely@isely.net> wrote:
> > Keep in mind that if the probe action is returning wrong results, that's 
> > worse than no results.  This isn't a question about "supplying missing 
> > data" so much as it a case of "overriding bad data".  Without that fixed 
> > mode disabling ability, it is very possible that the panel will simply 
> > be set up wrong with a corrupted display, with no possible means for the 
> > user to repair the problem.  That's what led me to make the change, and 
> > that same change had been living peacefully in the userspace xorg 
> > driver, up until the point that UMS had been removed from it.  Right now 
> > without the ability to disable fixed mode there simply *is no* 
> > workaround.
> 
> Point taken. If we can't get a resolution on i915.lvds_fixed_mode= within
> the next week, let's focus on the workaround instead.
> -Chris

Sounds good.

The remaining updated patches look good as well.

  -Mike
   isely@pobox.com
   isely@isely.net


-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8