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

[RFC PATCH] 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>
---

Patch is against drm-intel-next.  Not even compile-tested yet, just
looking for feedback.  I _think_ the pre-gen5 path is right, the GM45 doc
makes it sound like the bpc and dither bits only affect DP.

 drivers/gpu/drm/i915/intel_display.c |   18 +++++++++++++++++-
 drivers/gpu/drm/i915/intel_dp.c      |   30 ++++++++++++++++++++++++++----
 drivers/gpu/drm/i915/intel_drv.h     |    1 +
 3 files changed, 44 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index b5b15bd..08bea13 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4472,6 +4472,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.
@@ -4483,13 +4484,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;
@@ -4558,6 +4561,9 @@ static bool intel_choose_pipe_bpp_dither(struct drm_crtc *crtc,
 		}
 	}
 
+	if (mode->private_flags & INTEL_MODE_DP_FORCE_6BPC)
+		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
@@ -4817,6 +4823,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');
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 4ee4243..3988087 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -198,6 +198,8 @@ intel_dp_mode_valid(struct drm_connector *connector,
 		    struct drm_display_mode *mode)
 {
 	struct intel_dp *intel_dp = intel_attached_dp(connector);
+	struct drm_crtc *crtc = intel_dp->base.base.crtc;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	struct drm_device *dev = connector->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	int max_link_clock = intel_dp_link_clock(intel_dp_max_link_bw(intel_dp));
@@ -213,10 +215,30 @@ 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);
+
+		if (mode_rate > max_rate && intel_dp->crtc) {
+			/* see if we can make it fit in 6bpc */
+			int old_bpp = intel_dp->crtc->bpp;
+			intel_dp->crtc->bpp = 18;
+			mode_rate = intel_dp_link_required(connector->dev,
+							   intel_dp,
+							   mode_clock);
+			intel_dp->crtc->bpp = old_bpp;
+
+			if (mode_rate > max_rate)
+				return MODE_CLOCK_HIGH;
+			else
+				mode->private_flags |= INTEL_MODE_DP_FORCE_6BPC;
+		} else {
+			return MODE_CLOCK_HIGH;
+		}
+	}
 
 	if (mode->clock < 10000)
 		return MODE_CLOCK_LOW;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 6e990f9..9bdeb00 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

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

[Jesse Barnes]

On Wed, 20 Jul 2011 11:08:51 -0400
Adam Jackson <ajax@redhat.com> wrote:

> 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>
> ---
> 
> Patch is against drm-intel-next.  Not even compile-tested yet, just
> looking for feedback.  I _think_ the pre-gen5 path is right, the GM45 doc
> makes it sound like the bpc and dither bits only affect DP.

Yeah, it would be good to have this.

> +	/* 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;

Maybe this part would be cleaner if you added G4x support to
choose_pipe_bpp_dither and called it here instead?

-- 
Jesse Barnes, Intel Open Source Technology Center