drm/i915: Refactor panel fitting on the LVDS. (v2)

Move the common routines into separate functions to not only increase
readability, but also throwaway surplus code.

In doing so, we review the calculation of the aspect preserving scaling
and avoid the use of fixed-point until we need to calculate the accurate
scale factor.

v2: Improve comments as suggested by Jesse.

1 files changed, 105 insertions(+), 194 deletions(-)

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Eric Anholt <eric@anholt.net>

1 files changed, 105 insertions, 194 deletions

diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 6ef9388..0a2e600 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -156,31 +156,73 @@ static int intel_lvds_mode_valid(struct drm_connector *connector,
 	return MODE_OK;
 }
 
+static void
+centre_horizontally(struct drm_display_mode *mode,
+		    int width)
+{
+	u32 border, sync_pos, blank_width, sync_width;
+
+	/* keep the hsync and hblank widths constant */
+	sync_width = mode->crtc_hsync_end - mode->crtc_hsync_start;
+	blank_width = mode->crtc_hblank_end - mode->crtc_hblank_start;
+	sync_pos = (blank_width - sync_width + 1) / 2;
+
+	border = (mode->hdisplay - width + 1) / 2;
+	border += border & 1; /* make the border even */
+
+	mode->crtc_hdisplay = width;
+	mode->crtc_hblank_start = width + border;
+	mode->crtc_hblank_end = mode->crtc_hblank_start + blank_width;
+
+	mode->crtc_hsync_start = mode->crtc_hblank_start + sync_pos;
+	mode->crtc_hsync_end = mode->crtc_hsync_start + sync_width;
+}
+
+static void
+centre_vertically(struct drm_display_mode *mode,
+		  int height)
+{
+	u32 border, sync_pos, blank_width, sync_width;
+
+	/* keep the vsync and vblank widths constant */
+	sync_width = mode->crtc_vsync_end - mode->crtc_vsync_start;
+	blank_width = mode->crtc_vblank_end - mode->crtc_vblank_start;
+	sync_pos = (blank_width - sync_width + 1) / 2;
+
+	border = (mode->vdisplay - height + 1) / 2;
+
+	mode->crtc_vdisplay = height;
+	mode->crtc_vblank_start = height + border;
+	mode->crtc_vblank_end = mode->crtc_vblank_start + blank_width;
+
+	mode->crtc_vsync_start = mode->crtc_vblank_start + sync_pos;
+	mode->crtc_vsync_end = mode->crtc_vsync_start + sync_width;
+}
+
+static inline u32 panel_fitter_scaling(u32 source, u32 target)
+{
+	/*
+	 * Floating point operation is not supported. So the FACTOR
+	 * is defined, which can avoid the floating point computation
+	 * when calculating the panel ratio.
+	 */
+#define ACCURACY 12
+#define FACTOR (1 << ACCURACY)
+	u32 ratio = source * FACTOR / target;
+	return (FACTOR * ratio + FACTOR/2) / FACTOR;
+}
+
 static bool intel_lvds_mode_fixup(struct drm_encoder *encoder,
 				  struct drm_display_mode *mode,
 				  struct drm_display_mode *adjusted_mode)
 {
-	/*
-	 * float point operation is not supported . So the PANEL_RATIO_FACTOR
-	 * is defined, which can avoid the float point computation when
-	 * calculating the panel ratio.
-	 */
-#define PANEL_RATIO_FACTOR 8192
 	struct drm_device *dev = encoder->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
 	struct drm_encoder *tmp_encoder;
 	struct intel_encoder *intel_encoder = enc_to_intel_encoder(encoder);
 	struct intel_lvds_priv *lvds_priv = intel_encoder->dev_priv;
-	u32 pfit_control = 0, pfit_pgm_ratios = 0;
-	int left_border = 0, right_border = 0, top_border = 0;
-	int bottom_border = 0;
-	bool border = 0;
-	int panel_ratio, desired_ratio, vert_scale, horiz_scale;
-	int horiz_ratio, vert_ratio;
-	u32 hsync_width, vsync_width;
-	u32 hblank_width, vblank_width;
-	u32 hsync_pos, vsync_pos;
+	u32 pfit_control = 0, pfit_pgm_ratios = 0, border = 0;
 
 	/* Should never happen!! */
 	if (!IS_I965G(dev) && intel_crtc->pipe == 0) {
@@ -228,11 +270,8 @@ static bool intel_lvds_mode_fixup(struct drm_encoder *encoder,
 
 	/* Native modes don't need fitting */
 	if (adjusted_mode->hdisplay == mode->hdisplay &&
-			adjusted_mode->vdisplay == mode->vdisplay) {
-		pfit_pgm_ratios = 0;
-		border = 0;
+	    adjusted_mode->vdisplay == mode->vdisplay)
 		goto out;
-	}
 
 	/* full screen scale for now */
 	if (HAS_PCH_SPLIT(dev))
@@ -240,25 +279,9 @@ static bool intel_lvds_mode_fixup(struct drm_encoder *encoder,
 
 	/* 965+ wants fuzzy fitting */
 	if (IS_I965G(dev))
-		pfit_control |= (intel_crtc->pipe << PFIT_PIPE_SHIFT) |
-					PFIT_FILTER_FUZZY;
-
-	hsync_width = adjusted_mode->crtc_hsync_end -
-					adjusted_mode->crtc_hsync_start;
-	vsync_width = adjusted_mode->crtc_vsync_end -
-					adjusted_mode->crtc_vsync_start;
-	hblank_width = adjusted_mode->crtc_hblank_end -
-					adjusted_mode->crtc_hblank_start;
-	vblank_width = adjusted_mode->crtc_vblank_end -
-					adjusted_mode->crtc_vblank_start;
-	/*
-	 * Deal with panel fitting options. Figure out how to stretch the
-	 * image based on its aspect ratio & the current panel fitting mode.
-	 */
-	panel_ratio = adjusted_mode->hdisplay * PANEL_RATIO_FACTOR /
-				adjusted_mode->vdisplay;
-	desired_ratio = mode->hdisplay * PANEL_RATIO_FACTOR /
-				mode->vdisplay;
+		pfit_control |= ((intel_crtc->pipe << PFIT_PIPE_SHIFT) |
+				 PFIT_FILTER_FUZZY);
+
 	/*
 	 * Enable automatic panel scaling for non-native modes so that they fill
 	 * the screen.  Should be enabled before the pipe is enabled, according
@@ -276,170 +299,63 @@ static bool intel_lvds_mode_fixup(struct drm_encoder *encoder,
 		 * For centered modes, we have to calculate border widths &
 		 * heights and modify the values programmed into the CRTC.
 		 */
-		left_border = (adjusted_mode->hdisplay - mode->hdisplay) / 2;
-		right_border = left_border;
-		if (mode->hdisplay & 1)
-			right_border++;
-		top_border = (adjusted_mode->vdisplay - mode->vdisplay) / 2;
-		bottom_border = top_border;
-		if (mode->vdisplay & 1)
-			bottom_border++;
-		/* Set active & border values */
-		adjusted_mode->crtc_hdisplay = mode->hdisplay;
-		/* Keep the boder be even */
-		if (right_border & 1)
-			right_border++;
-		/* use the border directly instead of border minuse one */
-		adjusted_mode->crtc_hblank_start = mode->hdisplay +
-						right_border;
-		/* keep the blank width constant */
-		adjusted_mode->crtc_hblank_end =
-			adjusted_mode->crtc_hblank_start + hblank_width;
-		/* get the hsync pos relative to hblank start */
-		hsync_pos = (hblank_width - hsync_width) / 2;
-		/* keep the hsync pos be even */
-		if (hsync_pos & 1)
-			hsync_pos++;
-		adjusted_mode->crtc_hsync_start =
-				adjusted_mode->crtc_hblank_start + hsync_pos;
-		/* keep the hsync width constant */
-		adjusted_mode->crtc_hsync_end =
-				adjusted_mode->crtc_hsync_start + hsync_width;
-		adjusted_mode->crtc_vdisplay = mode->vdisplay;
-		/* use the border instead of border minus one */
-		adjusted_mode->crtc_vblank_start = mode->vdisplay +
-						bottom_border;
-		/* keep the vblank width constant */
-		adjusted_mode->crtc_vblank_end =
-				adjusted_mode->crtc_vblank_start + vblank_width;
-		/* get the vsync start postion relative to vblank start */
-		vsync_pos = (vblank_width - vsync_width) / 2;
-		adjusted_mode->crtc_vsync_start =
-				adjusted_mode->crtc_vblank_start + vsync_pos;
-		/* keep the vsync width constant */
-		adjusted_mode->crtc_vsync_end =
-				adjusted_mode->crtc_vsync_start + vsync_width;
-		border = 1;
+		centre_horizontally(adjusted_mode, mode->hdisplay);
+		centre_vertically(adjusted_mode, mode->vdisplay);
+		border = LVDS_BORDER_ENABLE;
 		break;
+
 	case DRM_MODE_SCALE_ASPECT:
-		/* Scale but preserve the spect ratio */
-		pfit_control |= PFIT_ENABLE;
+		/* Scale but preserve the aspect ratio */
 		if (IS_I965G(dev)) {
+			u32 scaled_width = adjusted_mode->hdisplay * mode->vdisplay;
+			u32 scaled_height = mode->hdisplay * adjusted_mode->vdisplay;
+
+			pfit_control |= PFIT_ENABLE;
 			/* 965+ is easy, it does everything in hw */
-			if (panel_ratio > desired_ratio)
+			if (scaled_width > scaled_height)
 				pfit_control |= PFIT_SCALING_PILLAR;
-			else if (panel_ratio < desired_ratio)
+			else if (scaled_width < scaled_height)
 				pfit_control |= PFIT_SCALING_LETTER;
 			else
 				pfit_control |= PFIT_SCALING_AUTO;
 		} else {
+			u32 scaled_width = adjusted_mode->hdisplay * mode->vdisplay;
+			u32 scaled_height = mode->hdisplay * adjusted_mode->vdisplay;
 			/*
 			 * For earlier chips we have to calculate the scaling
 			 * ratio by hand and program it into the
 			 * PFIT_PGM_RATIO register
 			 */
-			u32 horiz_bits, vert_bits, bits = 12;
-			horiz_ratio = mode->hdisplay * PANEL_RATIO_FACTOR/
-						adjusted_mode->hdisplay;
-			vert_ratio = mode->vdisplay * PANEL_RATIO_FACTOR/
-						adjusted_mode->vdisplay;
-			horiz_scale = adjusted_mode->hdisplay *
-					PANEL_RATIO_FACTOR / mode->hdisplay;
-			vert_scale = adjusted_mode->vdisplay *
-					PANEL_RATIO_FACTOR / mode->vdisplay;
-
-			/* retain aspect ratio */
-			if (panel_ratio > desired_ratio) { /* Pillar */
-				u32 scaled_width;
-				scaled_width = mode->hdisplay * vert_scale /
-						PANEL_RATIO_FACTOR;
-				horiz_ratio = vert_ratio;
-				pfit_control |= (VERT_AUTO_SCALE |
-						 VERT_INTERP_BILINEAR |
-						 HORIZ_INTERP_BILINEAR);
-				/* Pillar will have left/right borders */
-				left_border = (adjusted_mode->hdisplay -
-						scaled_width) / 2;
-				right_border = left_border;
-				if (mode->hdisplay & 1) /* odd resolutions */
-					right_border++;
-				/* keep the border be even */
-				if (right_border & 1)
-					right_border++;
-				adjusted_mode->crtc_hdisplay = scaled_width;
-				/* use border instead of border minus one */
-				adjusted_mode->crtc_hblank_start =
-					scaled_width + right_border;
-				/* keep the hblank width constant */
-				adjusted_mode->crtc_hblank_end =
-					adjusted_mode->crtc_hblank_start +
-							hblank_width;
-				/*
-				 * get the hsync start pos relative to
-				 * hblank start
-				 */
-				hsync_pos = (hblank_width - hsync_width) / 2;
-				/* keep the hsync_pos be even */
-				if (hsync_pos & 1)
-					hsync_pos++;
-				adjusted_mode->crtc_hsync_start =
-					adjusted_mode->crtc_hblank_start +
-							hsync_pos;
-				/* keept hsync width constant */
-				adjusted_mode->crtc_hsync_end =
-					adjusted_mode->crtc_hsync_start +
-							hsync_width;
-				border = 1;
-			} else if (panel_ratio < desired_ratio) { /* letter */
-				u32 scaled_height = mode->vdisplay *
-					horiz_scale / PANEL_RATIO_FACTOR;
-				vert_ratio = horiz_ratio;
-				pfit_control |= (HORIZ_AUTO_SCALE |
+			if (scaled_width > scaled_height) { /* pillar */
+				centre_horizontally(adjusted_mode, scaled_height / mode->vdisplay);
+
+				border = LVDS_BORDER_ENABLE;
+				if (mode->vdisplay != adjusted_mode->vdisplay) {
+					u32 bits = panel_fitter_scaling(mode->vdisplay, adjusted_mode->vdisplay);
+					pfit_pgm_ratios |= (bits << PFIT_HORIZ_SCALE_SHIFT |
+							    bits << PFIT_VERT_SCALE_SHIFT);
+					pfit_control |= (PFIT_ENABLE |
+							 VERT_INTERP_BILINEAR |
+							 HORIZ_INTERP_BILINEAR);
+				}
+			} else if (scaled_width < scaled_height) { /* letter */
+				centre_vertically(adjusted_mode, scaled_width / mode->hdisplay);
+
+				border = LVDS_BORDER_ENABLE;
+				if (mode->hdisplay != adjusted_mode->hdisplay) {
+					u32 bits = panel_fitter_scaling(mode->hdisplay, adjusted_mode->hdisplay);
+					pfit_pgm_ratios |= (bits << PFIT_HORIZ_SCALE_SHIFT |
+							    bits << PFIT_VERT_SCALE_SHIFT);
+					pfit_control |= (PFIT_ENABLE |
+							 VERT_INTERP_BILINEAR |
+							 HORIZ_INTERP_BILINEAR);
+				}
+			} else
+				/* Aspects match, Let hw scale both directions */
+				pfit_control |= (PFIT_ENABLE |
+						 VERT_AUTO_SCALE | HORIZ_AUTO_SCALE |
 						 VERT_INTERP_BILINEAR |
 						 HORIZ_INTERP_BILINEAR);
-				/* Letterbox will have top/bottom border */
-				top_border = (adjusted_mode->vdisplay -
-					scaled_height) / 2;
-				bottom_border = top_border;
-				if (mode->vdisplay & 1)
-					bottom_border++;
-				adjusted_mode->crtc_vdisplay = scaled_height;
-				/* use border instead of border minus one */
-				adjusted_mode->crtc_vblank_start =
-					scaled_height + bottom_border;
-				/* keep the vblank width constant */
-				adjusted_mode->crtc_vblank_end =
-					adjusted_mode->crtc_vblank_start +
-							vblank_width;
-				/*
-				 * get the vsync start pos relative to
-				 * vblank start
-				 */
-				vsync_pos = (vblank_width - vsync_width) / 2;
-				adjusted_mode->crtc_vsync_start =
-					adjusted_mode->crtc_vblank_start +
-							vsync_pos;
-				/* keep the vsync width constant */
-				adjusted_mode->crtc_vsync_end =
-					adjusted_mode->crtc_vsync_start +
-							vsync_width;
-				border = 1;
-			} else {
-			/* Aspects match, Let hw scale both directions */
-				pfit_control |= (VERT_AUTO_SCALE |
-						 HORIZ_AUTO_SCALE |
-						 VERT_INTERP_BILINEAR |
-						 HORIZ_INTERP_BILINEAR);
-			}
-			horiz_bits = (1 << bits) * horiz_ratio /
-					PANEL_RATIO_FACTOR;
-			vert_bits = (1 << bits) * vert_ratio /
-					PANEL_RATIO_FACTOR;
-			pfit_pgm_ratios =
-				((vert_bits << PFIT_VERT_SCALE_SHIFT) &
-						PFIT_VERT_SCALE_MASK) |
-				((horiz_bits << PFIT_HORIZ_SCALE_SHIFT) &
-						PFIT_HORIZ_SCALE_MASK);
 		}
 		break;
 
@@ -456,6 +372,7 @@ static bool intel_lvds_mode_fixup(struct drm_encoder *encoder,
 					 VERT_INTERP_BILINEAR |
 					 HORIZ_INTERP_BILINEAR);
 		break;
+
 	default:
 		break;
 	}
@@ -463,14 +380,8 @@ static bool intel_lvds_mode_fixup(struct drm_encoder *encoder,
 out:
 	lvds_priv->pfit_control = pfit_control;
 	lvds_priv->pfit_pgm_ratios = pfit_pgm_ratios;
-	/*
-	 * When there exists the border, it means that the LVDS_BORDR
-	 * should be enabled.
-	 */
-	if (border)
-		dev_priv->lvds_border_bits |= LVDS_BORDER_ENABLE;
-	else
-		dev_priv->lvds_border_bits &= ~(LVDS_BORDER_ENABLE);
+	dev_priv->lvds_border_bits = border;
+
 	/*
 	 * XXX: It would be nice to support lower refresh rates on the
 	 * panels to reduce power consumption, and perhaps match the