[PATCH 0/2] Automatic 6bpc dither for DisplayPort

[PATCH 0/2] Automatic 6bpc dither for DisplayPort

[Adam Jackson]

This fixes a rather irritating limitation of active dp->foo converters.
Many such (including all DP->VGA chips I can find on the market) come
equipped with only 2 DP lanes, which clips your dotclock to 144MHz at
8bpc.  Of the standard DMT modes, that means you lose 1600x1200@60 and
above.  Since these adaptors are marketed as supporting 1920x1200, we
might as well make that work, otherwise it's the driver that comes
out looking bad.

Granted you'll end up with 6bpc, which is hideous, but you're using
VGA so you don't get to complain about image quality.

Tested with a DP->VGA converter on an HP LP2480zx.

- ajax

[PATCH 1/2] drm/i915: intel_choose_pipe_bpp_dither messages should be DRM_DEBUG_KMS

[Adam Jackson]

Shouldn't hide these behind _DRIVER, they're all KMS-related.

Signed-off-by: Adam Jackson <ajax@redhat.com>
---
 drivers/gpu/drm/i915/intel_display.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 04411ad..bd9c878 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4633,7 +4633,7 @@ static bool intel_choose_pipe_bpp_dither(struct drm_crtc *crtc,
 				lvds_bpc = 6;
 
 			if (lvds_bpc < display_bpc) {
-				DRM_DEBUG_DRIVER("clamping display bpc (was %d) to LVDS (%d)\n", display_bpc, lvds_bpc);
+				DRM_DEBUG_KMS("clamping display bpc (was %d) to LVDS (%d)\n", display_bpc, lvds_bpc);
 				display_bpc = lvds_bpc;
 			}
 			continue;
@@ -4644,7 +4644,7 @@ static bool intel_choose_pipe_bpp_dither(struct drm_crtc *crtc,
 			unsigned int edp_bpc = dev_priv->edp.bpp / 3;
 
 			if (edp_bpc < display_bpc) {
-				DRM_DEBUG_DRIVER("clamping display bpc (was %d) to eDP (%d)\n", display_bpc, edp_bpc);
+				DRM_DEBUG_KMS("clamping display bpc (was %d) to eDP (%d)\n", display_bpc, edp_bpc);
 				display_bpc = edp_bpc;
 			}
 			continue;
@@ -4659,7 +4659,7 @@ static bool intel_choose_pipe_bpp_dither(struct drm_crtc *crtc,
 			/* Don't use an invalid EDID bpc value */
 			if (connector->display_info.bpc &&
 			    connector->display_info.bpc < display_bpc) {
-				DRM_DEBUG_DRIVER("clamping display bpc (was %d) to EDID reported max of %d\n", display_bpc, connector->display_info.bpc);
+				DRM_DEBUG_KMS("clamping display bpc (was %d) to EDID reported max of %d\n", display_bpc, connector->display_info.bpc);
 				display_bpc = connector->display_info.bpc;
 			}
 		}
@@ -4670,10 +4670,10 @@ static bool intel_choose_pipe_bpp_dither(struct drm_crtc *crtc,
 		 */
 		if (intel_encoder->type == INTEL_OUTPUT_HDMI) {
 			if (display_bpc > 8 && display_bpc < 12) {
-				DRM_DEBUG_DRIVER("forcing bpc to 12 for HDMI\n");
+				DRM_DEBUG_KMS("forcing bpc to 12 for HDMI\n");
 				display_bpc = 12;
 			} else {
-				DRM_DEBUG_DRIVER("forcing bpc to 8 for HDMI\n");
+				DRM_DEBUG_KMS("forcing bpc to 8 for HDMI\n");
 				display_bpc = 8;
 			}
 		}
@@ -4711,8 +4711,8 @@ static bool intel_choose_pipe_bpp_dither(struct drm_crtc *crtc,
 
 	display_bpc = min(display_bpc, bpc);
 
-	DRM_DEBUG_DRIVER("setting pipe bpc to %d (max display bpc %d)\n",
-			 bpc, display_bpc);
+	DRM_DEBUG_KMS("setting pipe bpc to %d (max display bpc %d)\n",
+		      bpc, display_bpc);
 
 	*pipe_bpp = display_bpc * 3;
 
-- 
1.7.6.4

[PATCH 2/2] drm/i915/dp: Dither down to 6bpc if it makes the mode fit

[Adam Jackson]

Some active adaptors (VGA usually) only have two lanes at 2.7GHz.
That's a maximum pixel clock of 144MHz at 8bpc, but 192MHz at 6bpc.

Signed-off-by: Adam Jackson <ajax@redhat.com>
---
 drivers/gpu/drm/i915/intel_display.c |   22 ++++++++++++++++++++--
 drivers/gpu/drm/i915/intel_dp.c      |   31 ++++++++++++++++++++++++-------
 drivers/gpu/drm/i915/intel_drv.h     |    1 +
 3 files changed, 45 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index bd9c878..d563110 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4591,6 +4591,7 @@ static inline bool intel_panel_use_ssc(struct drm_i915_private *dev_priv)
 /**
  * intel_choose_pipe_bpp_dither - figure out what color depth the pipe should send
  * @crtc: CRTC structure
+ * @mode: requested mode
  *
  * 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.
@@ -4602,13 +4603,15 @@ static inline bool intel_panel_use_ssc(struct drm_i915_private *dev_priv)
  *    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.
+ *    DP may want to dither down to 6bpc to fit larger modes
  *
  * 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)
+					 unsigned int *pipe_bpp,
+					 struct drm_display_mode *mode)
 {
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -4679,6 +4682,11 @@ static bool intel_choose_pipe_bpp_dither(struct drm_crtc *crtc,
 		}
 	}
 
+	if (mode->private_flags & INTEL_MODE_DP_FORCE_6BPC) {
+		DRM_DEBUG_KMS("Dithering DP to 6bpc\n");
+		display_bpc = 6;
+	}
+
 	/*
 	 * We could just drive the pipe at the highest bpc all the time and
 	 * enable dithering as needed, but that costs bandwidth.  So choose
@@ -4940,6 +4948,16 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
 			pipeconf &= ~PIPECONF_DOUBLE_WIDE;
 	}
 
+	/* default to 8bpc */
+	pipeconf &= ~(PIPECONF_BPP_MASK | PIPECONF_DITHER_EN);
+	if (is_dp) {
+		if (mode->private_flags & INTEL_MODE_DP_FORCE_6BPC) {
+			pipeconf |= PIPECONF_BPP_6 |
+				    PIPECONF_DITHER_EN |
+				    PIPECONF_DITHER_TYPE_ST1;
+		}
+	}
+
 	dpll |= DPLL_VCO_ENABLE;
 
 	DRM_DEBUG_KMS("Mode for pipe %c:\n", pipe == 0 ? 'A' : 'B');
@@ -5335,7 +5353,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 	/* determine panel color depth */
 	temp = I915_READ(PIPECONF(pipe));
 	temp &= ~PIPE_BPC_MASK;
-	dither = intel_choose_pipe_bpp_dither(crtc, &pipe_bpp);
+	dither = intel_choose_pipe_bpp_dither(crtc, &pipe_bpp, mode);
 	switch (pipe_bpp) {
 	case 18:
 		temp |= PIPE_6BPC;
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 44fef5e..da534b4 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -177,13 +177,16 @@ intel_dp_link_clock(uint8_t link_bw)
 
 /* I think this is a fiction */
 static int
-intel_dp_link_required(struct drm_device *dev, struct intel_dp *intel_dp, int pixel_clock)
+intel_dp_link_required(struct drm_device *dev, struct intel_dp *intel_dp,
+		       int pixel_clock, int check_bpp)
 {
 	struct drm_crtc *crtc = intel_dp->base.base.crtc;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	int bpp = 24;
 
-	if (intel_crtc)
+	if (check_bpp)
+		bpp = check_bpp;
+	else if (intel_crtc)
 		bpp = intel_crtc->bpp;
 
 	return (pixel_clock * bpp + 7) / 8;
@@ -215,10 +218,23 @@ intel_dp_mode_valid(struct drm_connector *connector,
 
 	/* only refuse the mode on non eDP since we have seen some weird eDP panels
 	   which are outside spec tolerances but somehow work by magic */
-	if (!is_edp(intel_dp) &&
-	    (intel_dp_link_required(connector->dev, intel_dp, mode->clock)
-	     > intel_dp_max_data_rate(max_link_clock, max_lanes)))
-		return MODE_CLOCK_HIGH;
+	if (!is_edp(intel_dp)) {
+		int max_rate = intel_dp_max_data_rate(max_link_clock,
+						      max_lanes);
+		int mode_rate = intel_dp_link_required(connector->dev,
+						       intel_dp,
+						       mode->clock, 0);
+
+		if (mode_rate > max_rate) {
+			mode_rate = intel_dp_link_required(connector->dev,
+							   intel_dp,
+							   mode->clock, 18);
+			if (mode_rate > max_rate)
+				return MODE_CLOCK_HIGH;
+			else
+				mode->private_flags |= INTEL_MODE_DP_FORCE_6BPC;
+		}
+	}
 
 	if (mode->clock < 10000)
 		return MODE_CLOCK_LOW;
@@ -603,6 +619,7 @@ intel_dp_mode_fixup(struct drm_encoder *encoder, struct drm_display_mode *mode,
 	int lane_count, clock;
 	int max_lane_count = intel_dp_max_lane_count(intel_dp);
 	int max_clock = intel_dp_max_link_bw(intel_dp) == DP_LINK_BW_2_7 ? 1 : 0;
+	int bpp = mode->private_flags & INTEL_MODE_DP_FORCE_6BPC ? 18 : 0;
 	static int bws[2] = { DP_LINK_BW_1_62, DP_LINK_BW_2_7 };
 
 	if (is_edp(intel_dp) && dev_priv->panel_fixed_mode) {
@@ -620,7 +637,7 @@ intel_dp_mode_fixup(struct drm_encoder *encoder, struct drm_display_mode *mode,
 		for (clock = 0; clock <= max_clock; clock++) {
 			int link_avail = intel_dp_max_data_rate(intel_dp_link_clock(bws[clock]), lane_count);
 
-			if (intel_dp_link_required(encoder->dev, intel_dp, mode->clock)
+			if (intel_dp_link_required(encoder->dev, intel_dp, mode->clock, bpp)
 					<= link_avail) {
 				intel_dp->link_bw = bws[clock];
 				intel_dp->lane_count = lane_count;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index fe1099d..edf1adf 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -110,6 +110,7 @@
 /* drm_display_mode->private_flags */
 #define INTEL_MODE_PIXEL_MULTIPLIER_SHIFT (0x0)
 #define INTEL_MODE_PIXEL_MULTIPLIER_MASK (0xf << INTEL_MODE_PIXEL_MULTIPLIER_SHIFT)
+#define INTEL_MODE_DP_FORCE_6BPC (0x10)
 
 static inline void
 intel_mode_set_pixel_multiplier(struct drm_display_mode *mode,
-- 
1.7.6.4

Re: [PATCH 0/2] Automatic 6bpc dither for DisplayPort

[Adam Jackson]

On Mon, 2011-10-10 at 16:33 -0400, Adam Jackson wrote:
> This fixes a rather irritating limitation of active dp->foo converters.
> Many such (including all DP->VGA chips I can find on the market) come
> equipped with only 2 DP lanes, which clips your dotclock to 144MHz at
> 8bpc.  Of the standard DMT modes, that means you lose 1600x1200@60 and
> above.  Since these adaptors are marketed as supporting 1920x1200, we
> might as well make that work, otherwise it's the driver that comes
> out looking bad.

After the fix in drm-intel-next to fix the DP bandwidth math this series
is probably less useful, since 1920x1200 will in fact fit in 2x2.7G:

(193250 * 24 + 9) / 10 == 463800

But if you have a Really Very Nice CRT then it might still be desirable.
2048x1536 @ 60Hz GTF for example:

(266950 * 24 + 9) / 10 == 640680
(266950 * 18 + 9) / 10 == 480510

I can respin patch 2 if there's still interest, but it might be
difficult to find a monitor to test against.  Patch 1 is still valid
though.

- ajax

Re: [PATCH 0/2] Automatic 6bpc dither for DisplayPort

[Keith Packard]

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

On Tue, 25 Oct 2011 12:25:45 -0400, Adam Jackson <ajax@redhat.com> wrote:

> I can respin patch 2 if there's still interest, but it might be
> difficult to find a monitor to test against.  Patch 1 is still valid
> though.

I've applied patch 1. I don't have a monitor to test patch 2 against, so
if someone does manage to test it, please let me know. Otherwise, I'll
leave it out of the kernel.

-- 
keith.packard@intel.com

[-- Attachment #1.2: Type: application/pgp-signature, Size: 827 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