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

[Chris Wilson <chris@chris-wilson.co.uk>]

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