Deep color support and bug fixes

Deep color support and bug fixes

[Jesse Barnes]

A couple of bug fixes, and some patches to support deep color on Intel
chipsets.

Thanks,
Jesse

[PATCH 1/9] drm: add bit depth parsing

[Jesse Barnes]

EDID 1.4 digital monitors report the bit depth supported in the input
field.  Add support for parsing this out and storing the info in the
display_info structure for use by drivers.

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/drm_edid.c |   55 ++++++++++++++++++++++++++++++++++++++++++-
 include/drm/drm_crtc.h     |    1 +
 include/drm/drm_edid.h     |   17 ++++++++++++-
 3 files changed, 70 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index adc9358..3518e1e 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1413,6 +1413,58 @@ end:
 EXPORT_SYMBOL(drm_detect_monitor_audio);
 
 /**
+ * drm_add_display_info - pull display info out if present
+ * @edid: EDID data
+ * @info: display info (attached to connector)
+ *
+ * Grab any available display info and stuff it into the drm_display_info
+ * structure that's part of the connector.  Useful for tracking bpp and
+ * color spaces.
+ */
+static void drm_add_display_info(struct edid *edid,
+				 struct drm_display_info *info)
+{
+	info->width_mm = edid->width_cm * 10;
+	info->height_mm = edid->height_cm * 10;
+
+	/* driver figures it out in this case */
+	info->bpc = 0;
+	info->color_formats = 0;
+
+	/* Only defined for 1.4 with digital displays */
+	if (edid->revision < 4)
+		return;
+
+	if (!(edid->input & DRM_EDID_INPUT_DIGITAL))
+		return;
+
+	switch (edid->input & DRM_EDID_DIGITAL_DEPTH_MASK) {
+	case DRM_EDID_DIGITAL_DEPTH_6:
+		info->bpc = 6;
+		break;
+	case DRM_EDID_DIGITAL_DEPTH_8:
+		info->bpc = 8;
+		break;
+	case DRM_EDID_DIGITAL_DEPTH_10:
+		info->bpc = 10;
+		break;
+	case DRM_EDID_DIGITAL_DEPTH_12:
+		info->bpc = 12;
+		break;
+	case DRM_EDID_DIGITAL_DEPTH_14:
+		info->bpc = 14;
+		break;
+	case DRM_EDID_DIGITAL_DEPTH_16:
+		info->bpc = 16;
+		break;
+	case DRM_EDID_DIGITAL_DEPTH_UNDEF:
+	default:
+		info->bpc = 0;
+		break;
+	}
+}
+
+/**
  * drm_add_edid_modes - add modes from EDID data, if available
  * @connector: connector we're probing
  * @edid: edid data
@@ -1460,8 +1512,7 @@ int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid)
 	if (quirks & (EDID_QUIRK_PREFER_LARGE_60 | EDID_QUIRK_PREFER_LARGE_75))
 		edid_fixup_preferred(connector, quirks);
 
-	connector->display_info.width_mm = edid->width_cm * 10;
-	connector->display_info.height_mm = edid->height_cm * 10;
+	drm_add_display_info(edid, &connector->display_info);
 
 	return num_modes;
 }
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index d94684b..ee1cb70 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -198,6 +198,7 @@ struct drm_display_info {
 	unsigned int min_vfreq, max_vfreq;
 	unsigned int min_hfreq, max_hfreq;
 	unsigned int pixel_clock;
+	unsigned int bpc;
 
 	enum subpixel_order subpixel_order;
 
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index 5881fad..9b9bf94 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -155,7 +155,22 @@ struct detailed_timing {
 #define DRM_EDID_INPUT_SEPARATE_SYNCS  (1 << 3)
 #define DRM_EDID_INPUT_BLANK_TO_BLACK  (1 << 4)
 #define DRM_EDID_INPUT_VIDEO_LEVEL     (3 << 5)
-#define DRM_EDID_INPUT_DIGITAL         (1 << 7) /* bits below must be zero if set */
+#define DRM_EDID_INPUT_DIGITAL         (1 << 7)
+#define DRM_EDID_DIGITAL_DEPTH_MASK    (7 << 4)
+#define DRM_EDID_DIGITAL_DEPTH_UNDEF   (0 << 4)
+#define DRM_EDID_DIGITAL_DEPTH_6       (1 << 4)
+#define DRM_EDID_DIGITAL_DEPTH_8       (2 << 4)
+#define DRM_EDID_DIGITAL_DEPTH_10      (3 << 4)
+#define DRM_EDID_DIGITAL_DEPTH_12      (4 << 4)
+#define DRM_EDID_DIGITAL_DEPTH_14      (5 << 4)
+#define DRM_EDID_DIGITAL_DEPTH_16      (6 << 4)
+#define DRM_EDID_DIGITAL_DEPTH_RSVD    (7 << 4)
+#define DRM_EDID_DIGITAL_TYPE_UNDEF    (0)
+#define DRM_EDID_DIGITAL_TYPE_DVI      (1)
+#define DRM_EDID_DIGITAL_TYPE_HDMI_A   (2)
+#define DRM_EDID_DIGITAL_TYPE_HDMI_B   (3)
+#define DRM_EDID_DIGITAL_TYPE_MDDI     (4)
+#define DRM_EDID_DIGITAL_TYPE_DP       (5)
 
 #define DRM_EDID_FEATURE_DEFAULT_GTF      (1 << 0)
 #define DRM_EDID_FEATURE_PREFERRED_TIMING (1 << 1)
-- 
1.7.4.1

[PATCH 2/9] drm: parse color format support for digital displays

[Jesse Barnes]

EDID 1.4 digital displays report the color spaces they support in the
features block.  Add support for grabbing this data and stuffing it into
the display_info struct for driver use.

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/drm_edid.c |    6 ++++++
 include/drm/drm_crtc.h     |    5 ++++-
 include/drm/drm_edid.h     |    8 ++++++++
 3 files changed, 18 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 3518e1e..0a9357c 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1462,6 +1462,12 @@ static void drm_add_display_info(struct edid *edid,
 		info->bpc = 0;
 		break;
 	}
+
+	info->color_formats = DRM_COLOR_FORMAT_RGB444;
+	if (info->color_formats & DRM_EDID_FEATURE_RGB_YCRCB444)
+		info->color_formats = DRM_COLOR_FORMAT_YCRCB444;
+	if (info->color_formats & DRM_EDID_FEATURE_RGB_YCRCB422)
+		info->color_formats = DRM_COLOR_FORMAT_YCRCB422;
 }
 
 /**
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index ee1cb70..9573e0c 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -183,7 +183,9 @@ enum subpixel_order {
 	SubPixelNone,
 };
 
-
+#define DRM_COLOR_FORMAT_RGB444		(1<<0)
+#define DRM_COLOR_FORMAT_YCRCB444	(1<<1)
+#define DRM_COLOR_FORMAT_YCRCB422	(1<<2)
 /*
  * Describes a given display (e.g. CRT or flat panel) and its limitations.
  */
@@ -201,6 +203,7 @@ struct drm_display_info {
 	unsigned int bpc;
 
 	enum subpixel_order subpixel_order;
+	u32 color_formats;
 
 	char *raw_edid; /* if any */
 };
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index 9b9bf94..eacb415 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -175,7 +175,15 @@ struct detailed_timing {
 #define DRM_EDID_FEATURE_DEFAULT_GTF      (1 << 0)
 #define DRM_EDID_FEATURE_PREFERRED_TIMING (1 << 1)
 #define DRM_EDID_FEATURE_STANDARD_COLOR   (1 << 2)
+/* If analog */
 #define DRM_EDID_FEATURE_DISPLAY_TYPE     (3 << 3) /* 00=mono, 01=rgb, 10=non-rgb, 11=unknown */
+/* If digital */
+#define DRM_EDID_FEATURE_COLOR_MASK	  (3 << 3)
+#define DRM_EDID_FEATURE_RGB		  (0 << 3)
+#define DRM_EDID_FEATURE_RGB_YCRCB444	  (1 << 3)
+#define DRM_EDID_FEATURE_RGB_YCRCB422	  (2 << 3)
+#define DRM_EDID_FEATURE_RGB_YCRCB	  (3 << 3) /* both 4:4:4 and 4:2:2 */
+
 #define DRM_EDID_FEATURE_PM_ACTIVE_OFF    (1 << 5)
 #define DRM_EDID_FEATURE_PM_SUSPEND       (1 << 6)
 #define DRM_EDID_FEATURE_PM_STANDBY       (1 << 7)
-- 
1.7.4.1

[PATCH 3/9] drm/i915: don't set SDVO color range on ILK+

[Jesse Barnes]

These bits are reserved on ILK+ (ILK+ provides this feature in the
transcoder and pipe configuration instead, which we already set).

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/intel_hdmi.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index f289b86..a6871d8 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -125,7 +125,8 @@ static void intel_hdmi_mode_set(struct drm_encoder *encoder,
 	u32 sdvox;
 
 	sdvox = SDVO_ENCODING_HDMI | SDVO_BORDER_ENABLE;
-	sdvox |= intel_hdmi->color_range;
+	if (!HAS_PCH_SPLIT(dev))
+		sdvox |= intel_hdmi->color_range;
 	if (adjusted_mode->flags & DRM_MODE_FLAG_PVSYNC)
 		sdvox |= SDVO_VSYNC_ACTIVE_HIGH;
 	if (adjusted_mode->flags & DRM_MODE_FLAG_PHSYNC)
-- 
1.7.4.1

Re: [PATCH 3/9] drm/i915: don't set SDVO color range on ILK+

[Keith Packard]

[-- Attachment #1.1: Type: text/plain, Size: 367 bytes --]

On Wed, 11 May 2011 10:48:04 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> These bits are reserved on ILK+ (ILK+ provides this feature in the
> transcoder and pipe configuration instead, which we already set).
> 
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>

Reviewed-by: Keith Packard <keithp@keithp.com>

-- 
keith.packard@intel.com

[-- Attachment #1.2: Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

[PATCH 4/9] drm/i915: don't set transcoder bpc on CougarPoint

[Jesse Barnes]

This prevents us from setting reserved or incorrect bits on CougarPoint.

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/intel_display.c |   15 +++++++++------
 1 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 6e967cd..15800cc 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1157,12 +1157,15 @@ static void intel_enable_transcoder(struct drm_i915_private *dev_priv,
 
 	reg = TRANSCONF(pipe);
 	val = I915_READ(reg);
-	/*
-	 * make the BPC in transcoder be consistent with
-	 * that in pipeconf reg.
-	 */
-	val &= ~PIPE_BPC_MASK;
-	val |= I915_READ(PIPECONF(pipe)) & PIPE_BPC_MASK;
+
+	if (HAS_PCH_IBX(dev_priv->dev)) {
+		/*
+		 * make the BPC in transcoder be consistent with
+		 * that in pipeconf reg.
+		 */
+		val &= ~PIPE_BPC_MASK;
+		val |= I915_READ(PIPECONF(pipe)) & PIPE_BPC_MASK;
+	}
 	I915_WRITE(reg, val | TRANS_ENABLE);
 	if (wait_for(I915_READ(reg) & TRANS_STATE_ENABLE, 100))
 		DRM_ERROR("failed to enable transcoder %d\n", pipe);
-- 
1.7.4.1

Re: [PATCH 4/9] drm/i915: don't set transcoder bpc on CougarPoint

[Keith Packard]

[-- Attachment #1.1: Type: text/plain, Size: 247 bytes --]

On Wed, 11 May 2011 10:48:05 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:

> This prevents us from setting reserved or incorrect bits on
> CougarPoint.

Reviewed-by: Keith Packard <keithp@keithp.com>

-- 
keith.packard@intel.com

[-- Attachment #1.2: Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

[PATCH 5/9] drm/i915: set bpc for DP transcoder

[Jesse Barnes]

This may not be the default value, so pull the bpc out of the pipe reg
and write it to the DP transcoder so proper dithering and signaling
occurs.

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/intel_display.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 15800cc..116760f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2620,6 +2620,7 @@ static void ironlake_pch_enable(struct drm_crtc *crtc)
 	/* For PCH DP, enable TRANS_DP_CTL */
 	if (HAS_PCH_CPT(dev) &&
 	    intel_pipe_has_type(crtc, INTEL_OUTPUT_DISPLAYPORT)) {
+		u32 bpc = (I915_READ(PIPECONF(pipe)) & PIPE_BPC_MASK) >> 5;
 		reg = TRANS_DP_CTL(pipe);
 		temp = I915_READ(reg);
 		temp &= ~(TRANS_DP_PORT_SEL_MASK |
@@ -2627,7 +2628,7 @@ static void ironlake_pch_enable(struct drm_crtc *crtc)
 			  TRANS_DP_BPC_MASK);
 		temp |= (TRANS_DP_OUTPUT_ENABLE |
 			 TRANS_DP_ENH_FRAMING);
-		temp |= TRANS_DP_8BPC;
+		temp |= bpc << 9; /* same format but at 11:9 */
 
 		if (crtc->mode.flags & DRM_MODE_FLAG_PHSYNC)
 			temp |= TRANS_DP_HSYNC_ACTIVE_HIGH;
-- 
1.7.4.1

[PATCH 6/9] drm/i915: split out Ironlake pipe bpp picking code

[Jesse Barnes]

Figuring out which pipe bpp to use is a bit painful.  It depends on both
the encoder and display configuration attached to a pipe.  For instance,
to drive a 24bpp framebuffer out to an 18bpp panel, we need to use 6bpc
on the pipe but also enable dithering.  But driving that same
framebuffer to a DisplayPort output on another pipe means using 8bpc and
no dithering.

So split out and enhance the code to handle the various cases, returning
an appropriate pipe bpp as well as whether dithering should be enabled.

Save the resulting pipe bpp in the intel_crtc struct for use by encoders
in calculating bandwidth requirements (defaults to 24bpp on pre-ILK).

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/intel_display.c |  184 ++++++++++++++++++++++++++--------
 drivers/gpu/drm/i915/intel_drv.h     |    1 +
 2 files changed, 143 insertions(+), 42 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 116760f..8880aa6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4348,6 +4348,123 @@ static inline bool intel_panel_use_ssc(struct drm_i915_private *dev_priv)
 	return dev_priv->lvds_use_ssc && i915_panel_use_ssc;
 }
 
+/**
+ * intel_choose_pipe_bpp_dither - figure out what color depth the pipe should send
+ * @crtc: CRTC structure
+ *
+ * A pipe may be connected to one or more outputs.  Based on the depth of the
+ * attached framebuffer, choose a good color depth to use on the pipe.
+ *
+ * If possible, match the pipe depth to the fb depth.  In some cases, this
+ * isn't ideal, because the connected output supports a lesser or restricted
+ * set of depths.  Resolve that here:
+ *    LVDS typically supports only 6bpc, so clamp down in that case
+ *    HDMI supports only 8bpc or 12bpc, so clamp to 8bpc with dither for 10bpc
+ *    Displays may support a restricted set as well, check EDID and clamp as
+ *      appropriate.
+ *
+ * RETURNS:
+ * Dithering requirement (i.e. false if display bpc and pipe bpc match,
+ * true if they don't match).
+ */
+static bool intel_choose_pipe_bpp_dither(struct drm_crtc *crtc,
+					 unsigned int *pipe_bpp)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_encoder *encoder;
+	struct drm_connector *connector;
+	unsigned int display_bpc = UINT_MAX, fb_bpc, bpc;
+
+	fb_bpc = crtc->fb->depth / 3;
+
+	/* Walk the encoders & connectors on this crtc, get min bpc */
+	list_for_each_entry(encoder, &dev->mode_config.encoder_list, head) {
+		struct intel_encoder *intel_encoder = to_intel_encoder(encoder);
+
+		if (encoder->crtc != crtc)
+			continue;
+
+		if (intel_encoder->type == INTEL_OUTPUT_LVDS) {
+			unsigned int lvds_bpc;
+
+			if ((I915_READ(PCH_LVDS) & LVDS_A3_POWER_MASK) ==
+			    LVDS_A3_POWER_UP)
+				lvds_bpc = 8;
+			else
+				lvds_bpc = 6;
+
+			if (lvds_bpc < display_bpc)
+				display_bpc = lvds_bpc;
+			continue;
+		}
+
+		if (intel_encoder->type == INTEL_OUTPUT_EDP) {
+			/* Use VBT settings if we have an eDP panel */
+			unsigned int edp_bpc = dev_priv->edp.bpp / 3;
+
+			if (edp_bpc < display_bpc)
+				display_bpc = edp_bpc;
+			continue;
+		}
+
+		/* Not one of the known troublemakers, check the EDID */
+		list_for_each_entry(connector, &dev->mode_config.connector_list,
+				    head) {
+			if (connector->encoder != encoder)
+				continue;
+
+			if (connector->display_info.bpc < display_bpc)
+				display_bpc = connector->display_info.bpc;
+		}
+
+		/*
+		 * HDMI is either 12 or 8, so if the display let 10bpc sneak
+		 * through, clamp it down.  (Note: >12bpc will be caught below.)
+		 */
+		if (intel_encoder->type == INTEL_OUTPUT_HDMI) {
+			if (display_bpc > 8 && display_bpc < 12)
+				display_bpc = 12;
+			else
+				display_bpc = 8;
+		}
+	}
+
+	/*
+	 * We could just drive the pipe at the highest bpc all the time and
+	 * enable dithering as needed, but that costs bandwidth.  So choose
+	 * the minimum value that expresses the full color range of the fb but
+	 * also stays within the max display bpc discovered above.
+	 */
+
+	switch (crtc->fb->depth) {
+	case 8:
+		bpc = 8; /* since we go through a colormap */
+		break;
+	case 15:
+	case 16:
+		bpc = 6; /* min is 18bpp */
+		break;
+	case 24:
+		bpc = min((unsigned int)8, display_bpc);
+		break;
+	case 30:
+		bpc = min((unsigned int)10, display_bpc);
+		break;
+	case 48:
+		bpc = min((unsigned int)12, display_bpc);
+		break;
+	default:
+		DRM_DEBUG("unsupported depth, assuming 24 bits\n");
+		bpc = min((unsigned int)8, display_bpc);
+		break;
+	}
+
+	*pipe_bpp = bpc * 3;
+
+	return display_bpc != bpc;
+}
+
 static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
 			      struct drm_display_mode *mode,
 			      struct drm_display_mode *adjusted_mode,
@@ -4760,7 +4877,9 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 	struct fdi_m_n m_n = {0};
 	u32 temp;
 	u32 lvds_sync = 0;
-	int target_clock, pixel_multiplier, lane, link_bw, bpp, factor;
+	int target_clock, pixel_multiplier, lane, link_bw, factor;
+	unsigned int pipe_bpp;
+	bool dither;
 
 	list_for_each_entry(encoder, &mode_config->encoder_list, base.head) {
 		if (encoder->base.crtc != crtc)
@@ -4887,56 +5006,37 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 	/* determine panel color depth */
 	temp = I915_READ(PIPECONF(pipe));
 	temp &= ~PIPE_BPC_MASK;
-	if (is_lvds) {
-		/* the BPC will be 6 if it is 18-bit LVDS panel */
-		if ((I915_READ(PCH_LVDS) & LVDS_A3_POWER_MASK) == LVDS_A3_POWER_UP)
-			temp |= PIPE_8BPC;
-		else
-			temp |= PIPE_6BPC;
-	} else if (has_edp_encoder) {
-		switch (dev_priv->edp.bpp/3) {
-		case 8:
-			temp |= PIPE_8BPC;
-			break;
-		case 10:
-			temp |= PIPE_10BPC;
-			break;
-		case 6:
-			temp |= PIPE_6BPC;
-			break;
-		case 12:
-			temp |= PIPE_12BPC;
-			break;
-		}
-	} else
-		temp |= PIPE_8BPC;
-	I915_WRITE(PIPECONF(pipe), temp);
-
-	switch (temp & PIPE_BPC_MASK) {
-	case PIPE_8BPC:
-		bpp = 24;
+	dither = intel_choose_pipe_bpp_dither(crtc, &pipe_bpp);
+	switch (pipe_bpp) {
+	case 18:
+		temp |= PIPE_6BPC;
 		break;
-	case PIPE_10BPC:
-		bpp = 30;
+	case 24:
+		temp |= PIPE_8BPC;
 		break;
-	case PIPE_6BPC:
-		bpp = 18;
+	case 30:
+		temp |= PIPE_10BPC;
 		break;
-	case PIPE_12BPC:
-		bpp = 36;
+	case 36:
+		temp |= PIPE_12BPC;
 		break;
 	default:
-		DRM_ERROR("unknown pipe bpc value\n");
-		bpp = 24;
+		WARN(1, "intel_choose_pipe_bpp returned invalid value\n");
+		temp |= PIPE_8BPC;
+		pipe_bpp = 24;
+		break;
 	}
 
+	intel_crtc->bpp = pipe_bpp;
+	I915_WRITE(PIPECONF(pipe), temp);
+
 	if (!lane) {
 		/*
 		 * Account for spread spectrum to avoid
 		 * oversubscribing the link. Max center spread
 		 * is 2.5%; use 5% for safety's sake.
 		 */
-		u32 bps = target_clock * bpp * 21 / 20;
+		u32 bps = target_clock * intel_crtc->bpp * 21 / 20;
 		lane = bps / (link_bw * 8) + 1;
 	}
 
@@ -4944,7 +5044,8 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 
 	if (pixel_multiplier > 1)
 		link_bw *= pixel_multiplier;
-	ironlake_compute_m_n(bpp, lane, target_clock, link_bw, &m_n);
+	ironlake_compute_m_n(intel_crtc->bpp, lane, target_clock, link_bw,
+			     &m_n);
 
 	/* Ironlake: try to setup display ref clock before DPLL
 	 * enabling. This is only under driver's control after
@@ -5147,14 +5248,12 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 		I915_WRITE(PCH_LVDS, temp);
 	}
 
-	/* set the dithering flag and clear for anything other than a panel. */
 	pipeconf &= ~PIPECONF_DITHER_EN;
 	pipeconf &= ~PIPECONF_DITHER_TYPE_MASK;
-	if (dev_priv->lvds_dither && (is_lvds || has_edp_encoder)) {
+	if ((is_lvds && dev_priv->lvds_dither) || dither) {
 		pipeconf |= PIPECONF_DITHER_EN;
 		pipeconf |= PIPECONF_DITHER_TYPE_ST1;
 	}
-
 	if (is_dp || intel_encoder_is_pch_edp(&has_edp_encoder->base)) {
 		intel_dp_set_m_n(crtc, mode, adjusted_mode);
 	} else {
@@ -6447,6 +6546,7 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
 
 	intel_crtc_reset(&intel_crtc->base);
 	intel_crtc->active = true; /* force the pipe off on setup_init_config */
+	intel_crtc->bpp = 24; /* default for pre-Ironlake */
 
 	if (HAS_PCH_SPLIT(dev)) {
 		intel_helper_funcs.prepare = ironlake_crtc_prepare;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 021c614..af8ff89 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -171,6 +171,7 @@ struct intel_crtc {
 	int16_t cursor_x, cursor_y;
 	int16_t cursor_width, cursor_height;
 	bool cursor_visible;
+	unsigned int bpp;
 };
 
 #define to_intel_crtc(x) container_of(x, struct intel_crtc, base)
-- 
1.7.4.1

Re: [PATCH 6/9] drm/i915: split out Ironlake pipe bpp picking code

[Chris Wilson]

On Wed, 11 May 2011 10:48:07 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> +static bool intel_choose_pipe_bpp_dither(struct drm_crtc *crtc,
> +					 unsigned int *pipe_bpp)
> +{
> +	struct drm_device *dev = crtc->dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct drm_encoder *encoder;
> +	struct drm_connector *connector;
> +	unsigned int display_bpc = UINT_MAX, fb_bpc, bpc;
> +
> +	fb_bpc = crtc->fb->depth / 3;

Very misleading. Set but never used.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

[PATCH 7/9] drm/i915: split out plane update code

[Jesse Barnes]

Updating the planes is device specific, so create a new display callback
and use it in pipe_set_base.  (In fact we could go even further, valid
display plane bits have changed with each generation, as has tiled
buffer handling.)

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/i915_drv.h      |    2 +
 drivers/gpu/drm/i915/intel_display.c |  107 +++++++++++++++++++++++++++++++---
 2 files changed, 100 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 2df37ef..961e883 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -211,6 +211,8 @@ struct drm_i915_display_funcs {
 	void (*fdi_link_train)(struct drm_crtc *crtc);
 	void (*init_clock_gating)(struct drm_device *dev);
 	void (*init_pch_clock_gating)(struct drm_device *dev);
+	int (*update_plane)(struct drm_crtc *crtc, struct drm_framebuffer *fb,
+			    int x, int y);
 	/* clock updates for mode set */
 	/* cursor updates */
 	/* render clock increase/decrease */
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 8880aa6..8c13dfa 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1839,10 +1839,8 @@ err_interruptible:
 	return ret;
 }
 
-/* Assume fb object is pinned & idle & fenced and just update base pointers */
-static int
-intel_pipe_set_base_atomic(struct drm_crtc *crtc, struct drm_framebuffer *fb,
-			   int x, int y, enum mode_set_atomic state)
+static int i9xx_update_plane(struct drm_crtc *crtc, struct drm_framebuffer *fb,
+			     int x, int y)
 {
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -1885,7 +1883,7 @@ intel_pipe_set_base_atomic(struct drm_crtc *crtc, struct drm_framebuffer *fb,
 		dspcntr |= DISPPLANE_32BPP_NO_ALPHA;
 		break;
 	default:
-		DRM_ERROR("Unknown color depth\n");
+		DRM_ERROR("Unknown color depth %d\n", fb->bits_per_pixel);
 		return -EINVAL;
 	}
 	if (INTEL_INFO(dev)->gen >= 4) {
@@ -1895,10 +1893,6 @@ intel_pipe_set_base_atomic(struct drm_crtc *crtc, struct drm_framebuffer *fb,
 			dspcntr &= ~DISPPLANE_TILED;
 	}
 
-	if (HAS_PCH_SPLIT(dev))
-		/* must disable */
-		dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
-
 	I915_WRITE(reg, dspcntr);
 
 	Start = obj->gtt_offset;
@@ -1915,6 +1909,99 @@ intel_pipe_set_base_atomic(struct drm_crtc *crtc, struct drm_framebuffer *fb,
 		I915_WRITE(DSPADDR(plane), Start + Offset);
 	POSTING_READ(reg);
 
+	return 0;
+}
+
+static int ironlake_update_plane(struct drm_crtc *crtc,
+				 struct drm_framebuffer *fb, int x, int y)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	struct intel_framebuffer *intel_fb;
+	struct drm_i915_gem_object *obj;
+	int plane = intel_crtc->plane;
+	unsigned long Start, Offset;
+	u32 dspcntr;
+	u32 reg;
+
+	switch (plane) {
+	case 0:
+	case 1:
+		break;
+	default:
+		DRM_ERROR("Can't update plane %d in SAREA\n", plane);
+		return -EINVAL;
+	}
+
+	intel_fb = to_intel_framebuffer(fb);
+	obj = intel_fb->obj;
+
+	reg = DSPCNTR(plane);
+	dspcntr = I915_READ(reg);
+	/* Mask out pixel format bits in case we change it */
+	dspcntr &= ~DISPPLANE_PIXFORMAT_MASK;
+	switch (fb->bits_per_pixel) {
+	case 8:
+		dspcntr |= DISPPLANE_8BPP;
+		break;
+	case 16:
+		if (fb->depth != 16)
+			return -EINVAL;
+
+		dspcntr |= DISPPLANE_16BPP;
+		break;
+	case 24:
+	case 32:
+		if (fb->depth == 24)
+			dspcntr |= DISPPLANE_32BPP_NO_ALPHA;
+		else if (fb->depth == 30)
+			dspcntr |= DISPPLANE_32BPP_30BIT_NO_ALPHA;
+		else
+			return -EINVAL;
+		break;
+	default:
+		DRM_ERROR("Unknown color depth %d\n", fb->bits_per_pixel);
+		return -EINVAL;
+	}
+
+	if (obj->tiling_mode != I915_TILING_NONE)
+		dspcntr |= DISPPLANE_TILED;
+	else
+		dspcntr &= ~DISPPLANE_TILED;
+
+	/* must disable */
+	dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
+
+	I915_WRITE(reg, dspcntr);
+
+	Start = obj->gtt_offset;
+	Offset = y * fb->pitch + x * (fb->bits_per_pixel / 8);
+
+	DRM_DEBUG_KMS("Writing base %08lX %08lX %d %d %d\n",
+		      Start, Offset, x, y, fb->pitch);
+	I915_WRITE(DSPSTRIDE(plane), fb->pitch);
+	I915_WRITE(DSPSURF(plane), Start);
+	I915_WRITE(DSPTILEOFF(plane), (y << 16) | x);
+	I915_WRITE(DSPADDR(plane), Offset);
+	POSTING_READ(reg);
+
+	return 0;
+}
+
+/* Assume fb object is pinned & idle & fenced and just update base pointers */
+static int
+intel_pipe_set_base_atomic(struct drm_crtc *crtc, struct drm_framebuffer *fb,
+			   int x, int y, enum mode_set_atomic state)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	int ret;
+
+	ret = dev_priv->display.update_plane(crtc, fb, x, y);
+	if (ret)
+		return ret;
+
 	intel_update_fbc(dev);
 	intel_increase_pllclock(crtc);
 
@@ -7543,9 +7630,11 @@ static void intel_init_display(struct drm_device *dev)
 	if (HAS_PCH_SPLIT(dev)) {
 		dev_priv->display.dpms = ironlake_crtc_dpms;
 		dev_priv->display.crtc_mode_set = ironlake_crtc_mode_set;
+		dev_priv->display.update_plane = ironlake_update_plane;
 	} else {
 		dev_priv->display.dpms = i9xx_crtc_dpms;
 		dev_priv->display.crtc_mode_set = i9xx_crtc_mode_set;
+		dev_priv->display.update_plane = i9xx_update_plane;
 	}
 
 	if (I915_HAS_FBC(dev)) {
-- 
1.7.4.1

[PATCH 8/9] drm/i915: use pipe bpp in DP link bandwidth calculations

[Jesse Barnes]

The pipe may be driving various bpp values depending on the display
configuration, so take that into account when calculating link bandwidth
requirements.

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/intel_dp.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index cb8578b..0be064e 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -684,7 +684,7 @@ intel_dp_set_m_n(struct drm_crtc *crtc, struct drm_display_mode *mode,
 	struct drm_encoder *encoder;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	int lane_count = 4, bpp = 24;
+	int lane_count = 4;
 	struct intel_dp_m_n m_n;
 	int pipe = intel_crtc->pipe;
 
@@ -703,7 +703,6 @@ intel_dp_set_m_n(struct drm_crtc *crtc, struct drm_display_mode *mode,
 			break;
 		} else if (is_edp(intel_dp)) {
 			lane_count = dev_priv->edp.lanes;
-			bpp = dev_priv->edp.bpp;
 			break;
 		}
 	}
@@ -713,7 +712,7 @@ intel_dp_set_m_n(struct drm_crtc *crtc, struct drm_display_mode *mode,
 	 * the number of bytes_per_pixel post-LUT, which we always
 	 * set up for 8-bits of R/G/B, or 3 bytes total.
 	 */
-	intel_dp_compute_m_n(bpp, lane_count,
+	intel_dp_compute_m_n(intel_crtc->bpp, lane_count,
 			     mode->clock, adjusted_mode->clock, &m_n);
 
 	if (HAS_PCH_SPLIT(dev)) {
-- 
1.7.4.1

[PATCH 9/9] drm/i915: use pipe bpp when setting HDMI bpc

[Jesse Barnes]

The Intel HDMI encoder can support 8bpc or 12bpc.  Set the appropriate
value based on the pipe bpp when configuring the output.

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/intel_hdmi.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index a6871d8..5cf7b23 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -132,6 +132,11 @@ static void intel_hdmi_mode_set(struct drm_encoder *encoder,
 	if (adjusted_mode->flags & DRM_MODE_FLAG_PHSYNC)
 		sdvox |= SDVO_HSYNC_ACTIVE_HIGH;
 
+	if (intel_crtc->bpp > 24)
+		sdvox |= COLOR_FORMAT_12bpc;
+	else
+		sdvox |= COLOR_FORMAT_8bpc;
+
 	/* Required on CPT */
 	if (intel_hdmi->has_hdmi_sink && HAS_PCH_CPT(dev))
 		sdvox |= HDMI_MODE_SELECT;
-- 
1.7.4.1

Re: Deep color support and bug fixes

[Chris Wilson]

On Wed, 11 May 2011 10:48:01 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> A couple of bug fixes, and some patches to support deep color on Intel
> chipsets.

My main concern with extended fb->depth support is in validation that the
crtc+encoder supports the requested fb, and that we have sufficient
logging of the bpc (inc fb->depth/fb->bpp) changes during modeswitch.
There are some restrictions that you add in mode_set() that need to be
raised to intel_framebuffer_init() so that the error is detected early.
But we should also be checking the requested fb->depth is valid during
preparation, and error early.

>From the DDX, we declare the fb depth before we know what's attached, and
once established that depth is permanent (if only due to the complexity of
invalidating all client cached state for the old depth). So we're stuck
with the user getting it right. In a composite-only, pixmap-per-crtc model
changing depths on the fly is possible and clients can choose to take
advantage of the deep-color support, or not.

So here goes -depth 30...
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: Deep color support and bug fixes

[Chris Wilson]

On Fri, 24 Jun 2011 10:32:16 +0100, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> So here goes -depth 30...

So other than a couple of rendering paths where we write 8bpc data into
the depth-30 fb (go flash, go!), X looks good. Textured XVideo should just
work, GLX doesn't advertise a 10-bpc visual or fbconfig, so will only work
if we can use a render copy for copy-swaps.

However, I did need to:

diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index 9236965..ef4f29c 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -560,7 +560,9 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set)
 			mode_changed = true;
 		} else if (set->fb == NULL) {
 			mode_changed = true;
-		} else
+		} else if (set->fb->depth != set->crtc->fb->depth)
+			mode_changed = true;
+		else
 			fb_changed = true;
 	}
 
for the kernel to spot the mode switch when starting X. Lack of logging
ftl. ;-)

For confirmation:

PIPEACONF: 0xc0000020 (enabled, active, 10bpc)
TRANSACONF: 0xc0000020 (enable, active)
FDI_RXA_CTL: 0xb0192050 (enable, train pattern not train, port width X4, 10bpc,link_reverse_strap_overwrite no, dmi_link_reverse no, FDI PLL enable,FS ecc disable, FE ecc disable, FS err report disable, FE err report disable,scrambing enable, enhanced framing enable, PCDClk)


Tested-by: Chris Wilson <chris@chris-wilson.co.uk>
But for my review by, I want to see some logging and validation ;-)
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: Deep color support and bug fixes

[Chris Wilson]

On Fri, 24 Jun 2011 11:24:08 +0100, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Fri, 24 Jun 2011 10:32:16 +0100, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > So here goes -depth 30...
> 
> So other than a couple of rendering paths where we write 8bpc data into
> the depth-30 fb (go flash, go!), X looks good.

Looking at the xtrace and what the xserver/ddx is doing, it appears that
flash is b0rked. Quel surprise.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre