[PATCH V3 0/3] OMAPDSS: DISPC: Enable predecimation for DMA and VRFB

[PATCH V3 0/3] OMAPDSS: DISPC: Enable predecimation for DMA and VRFB

[Chandrabhanu Mahapatra]

Hi everyone,
the following patch set directs to enable predecimation for DMA and VRFB
which consists of two pacthes.

The first patch is based on code written by Lajos Molnar <lajos@ti.com> in
Android Kernel, which updates the code with predecimation logic thereby
increasing the downscaling ability of the DISPC module.

The second patch is based on code written by Ville Syrj채l채
<ville.syrjala@nokia.com> which aims to avoid synclost errors occurring
in OMAP3 due to some undocumented horizontal position and timing related
limitations which I faced during testing of the previous patch.

The third patch corrects the usage of dispc fclk in scaling checks by
replacing it with dispc_core_clk as per suggestions of Ville Syrj채l채.

Modifications in V3 series:
* In 1st patch a check for decim_x_min has been added to avoid assigning of
decim_x less than possible leading to assignment of scaling more than 4 times
* In 2nd patch 
 -> check_horiz_timing() has been changed to check_horiz_timing_omap3() and
	function description in code has been added
 -> clean up code of dispc_mgr_lclk_rate() has been removed
*  In 3rd patch dispc_core_clk_rate() is introduced.

I have tested these patches successfully on OMAP2, OMAP3 AND OMAP4 on the
mainline kernel v3.4rc1. Horizontal and vertical predecimation worked fine
but skewed images were seen on OMAP2 and OMAP3 on HDMI tv during horizontal
predecimation which will be addressed in the future patches.

All your comments and suggestions are welcome.

Regards,
Chandrabhanu

Chandrabhanu Mahapatra (3):
  OMAPDSS: DISPC: Enable predecimation
  OMAPDSS: DISPC: Handle synclost errors in OMAP3
  OMAPDSS: DISPC: Correct DISPC functional clock usage

 drivers/video/omap2/dss/dispc.c |  354 +++++++++++++++++++++++++++++---------
 drivers/video/omap2/dss/dss.h   |    1 +
 2 files changed, 271 insertions(+), 84 deletions(-)

--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH V3 0/3] OMAPDSS: DISPC: Enable predecimation for DMA and VRFB

[Chandrabhanu Mahapatra]

Hi everyone,
the following patch set directs to enable predecimation for DMA and VRFB
which consists of two pacthes.

The first patch is based on code written by Lajos Molnar <lajos@ti.com> in
Android Kernel, which updates the code with predecimation logic thereby
increasing the downscaling ability of the DISPC module.

The second patch is based on code written by Ville Syrj채l채
<ville.syrjala@nokia.com> which aims to avoid synclost errors occurring
in OMAP3 due to some undocumented horizontal position and timing related
limitations which I faced during testing of the previous patch.

The third patch corrects the usage of dispc fclk in scaling checks by
replacing it with dispc_core_clk as per suggestions of Ville Syrj채l채.

Modifications in V3 series:
* In 1st patch a check for decim_x_min has been added to avoid assigning of
decim_x less than possible leading to assignment of scaling more than 4 times
* In 2nd patch 
 -> check_horiz_timing() has been changed to check_horiz_timing_omap3() and
	function description in code has been added
 -> clean up code of dispc_mgr_lclk_rate() has been removed
*  In 3rd patch dispc_core_clk_rate() is introduced.

I have tested these patches successfully on OMAP2, OMAP3 AND OMAP4 on the
mainline kernel v3.4rc1. Horizontal and vertical predecimation worked fine
but skewed images were seen on OMAP2 and OMAP3 on HDMI tv during horizontal
predecimation which will be addressed in the future patches.

All your comments and suggestions are welcome.

Regards,
Chandrabhanu

Chandrabhanu Mahapatra (3):
  OMAPDSS: DISPC: Enable predecimation
  OMAPDSS: DISPC: Handle synclost errors in OMAP3
  OMAPDSS: DISPC: Correct DISPC functional clock usage

 drivers/video/omap2/dss/dispc.c |  354 +++++++++++++++++++++++++++++---------
 drivers/video/omap2/dss/dss.h   |    1 +
 2 files changed, 271 insertions(+), 84 deletions(-)

--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH V3 1/3] OMAPDSS: DISPC: Enable predecimation

[Chandrabhanu Mahapatra]

In OMAP3 and OMAP4, the DISPC Scaler can downscale an image up to 4 times, and
up to 2 times in OMAP2. However, with predecimation, the image can be reduced
to 16 times by fetching only the necessary pixels in memory. Then this
predecimated image can be downscaled further by the DISPC scaler.

The pipeline is configured to use a burst of size 8 * 128 bits which consists
of 8 mini bursts of 16 bytes each. So, horizontal predecimation more than 16
can lead to complete discarding of such mini bursts. L3 interconnect may
handover the bus to some other initiator and inturn delay the fetching of
pixels leading to underflows. So, maximum predecimation limit is fixed at 16.

Based on the downscaling required, a prior calculation of predecimation values
for width and height of an image is done. Since, Predecimation reduces quality
of an image higher priorty is given to DISPC Scaler for downscaling.

This code was successfully tested on OMAP2, OMAP3 and OMAP4. Horizontal and
vertical predecimation worked fine except for some synclost errors due to
undocumented errata in OMAP3 which are fixed later and skewed images were seen
on OMAP2 and OMAP3 during horizontal predecimation which will be addressed in
the future patches.

This code is based on code written by Lajos Molnar <lajos@ti.com> who had added
predecimation support for NV12/YUV/rotated/SDMA buffers.

Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
---
 drivers/video/omap2/dss/dispc.c |  266 +++++++++++++++++++++++++++------------
 1 files changed, 185 insertions(+), 81 deletions(-)

diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index ee30937..4ab5433 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -1431,7 +1431,7 @@ static void calc_vrfb_rotation_offset(u8 rotation, bool mirror,
 		enum omap_color_mode color_mode, bool fieldmode,
 		unsigned int field_offset,
 		unsigned *offset0, unsigned *offset1,
-		s32 *row_inc, s32 *pix_inc)
+		s32 *row_inc, s32 *pix_inc, int x_predecim, int y_predecim)
 {
 	u8 ps;
 
@@ -1477,10 +1477,10 @@ static void calc_vrfb_rotation_offset(u8 rotation, bool mirror,
 		else
 			*offset0 = 0;
 
-		*row_inc = pixinc(1 + (screen_width - width) +
-				(fieldmode ? screen_width : 0),
-				ps);
-		*pix_inc = pixinc(1, ps);
+		*row_inc = pixinc(1 +
+			(y_predecim * screen_width - x_predecim * width) +
+			(fieldmode ? screen_width : 0), ps);
+		*pix_inc = pixinc(x_predecim, ps);
 		break;
 
 	case OMAP_DSS_ROT_0 + 4:
@@ -1498,10 +1498,10 @@ static void calc_vrfb_rotation_offset(u8 rotation, bool mirror,
 			*offset0 = field_offset * screen_width * ps;
 		else
 			*offset0 = 0;
-		*row_inc = pixinc(1 - (screen_width + width) -
-				(fieldmode ? screen_width : 0),
-				ps);
-		*pix_inc = pixinc(1, ps);
+		*row_inc = pixinc(1 -
+			(y_predecim * screen_width + x_predecim * width) -
+			(fieldmode ? screen_width : 0), ps);
+		*pix_inc = pixinc(x_predecim, ps);
 		break;
 
 	default:
@@ -1515,7 +1515,7 @@ static void calc_dma_rotation_offset(u8 rotation, bool mirror,
 		enum omap_color_mode color_mode, bool fieldmode,
 		unsigned int field_offset,
 		unsigned *offset0, unsigned *offset1,
-		s32 *row_inc, s32 *pix_inc)
+		s32 *row_inc, s32 *pix_inc, int x_predecim, int y_predecim)
 {
 	u8 ps;
 	u16 fbw, fbh;
@@ -1557,10 +1557,14 @@ static void calc_dma_rotation_offset(u8 rotation, bool mirror,
 			*offset0 = *offset1 + field_offset * screen_width * ps;
 		else
 			*offset0 = *offset1;
-		*row_inc = pixinc(1 + (screen_width - fbw) +
-				(fieldmode ? screen_width : 0),
-				ps);
-		*pix_inc = pixinc(1, ps);
+		*row_inc = pixinc(1 +
+			(y_predecim * screen_width - fbw * x_predecim) +
+			(fieldmode ? screen_width : 0),	ps);
+		if (color_mode = OMAP_DSS_COLOR_YUV2 ||
+			color_mode = OMAP_DSS_COLOR_UYVY)
+			*pix_inc = pixinc(x_predecim, 2 * ps);
+		else
+			*pix_inc = pixinc(x_predecim, ps);
 		break;
 	case OMAP_DSS_ROT_90:
 		*offset1 = screen_width * (fbh - 1) * ps;
@@ -1568,9 +1572,9 @@ static void calc_dma_rotation_offset(u8 rotation, bool mirror,
 			*offset0 = *offset1 + field_offset * ps;
 		else
 			*offset0 = *offset1;
-		*row_inc = pixinc(screen_width * (fbh - 1) + 1 +
-				(fieldmode ? 1 : 0), ps);
-		*pix_inc = pixinc(-screen_width, ps);
+		*row_inc = pixinc(screen_width * (fbh * x_predecim - 1) +
+				y_predecim + (fieldmode ? 1 : 0), ps);
+		*pix_inc = pixinc(-x_predecim * screen_width, ps);
 		break;
 	case OMAP_DSS_ROT_180:
 		*offset1 = (screen_width * (fbh - 1) + fbw - 1) * ps;
@@ -1579,10 +1583,13 @@ static void calc_dma_rotation_offset(u8 rotation, bool mirror,
 		else
 			*offset0 = *offset1;
 		*row_inc = pixinc(-1 -
-				(screen_width - fbw) -
-				(fieldmode ? screen_width : 0),
-				ps);
-		*pix_inc = pixinc(-1, ps);
+			(y_predecim * screen_width - fbw * x_predecim) -
+			(fieldmode ? screen_width : 0),	ps);
+		if (color_mode = OMAP_DSS_COLOR_YUV2 ||
+			color_mode = OMAP_DSS_COLOR_UYVY)
+			*pix_inc = pixinc(-x_predecim, 2 * ps);
+		else
+			*pix_inc = pixinc(-x_predecim, ps);
 		break;
 	case OMAP_DSS_ROT_270:
 		*offset1 = (fbw - 1) * ps;
@@ -1590,9 +1597,9 @@ static void calc_dma_rotation_offset(u8 rotation, bool mirror,
 			*offset0 = *offset1 - field_offset * ps;
 		else
 			*offset0 = *offset1;
-		*row_inc = pixinc(-screen_width * (fbh - 1) - 1 -
-				(fieldmode ? 1 : 0), ps);
-		*pix_inc = pixinc(screen_width, ps);
+		*row_inc = pixinc(-screen_width * (fbh * x_predecim - 1) -
+				y_predecim - (fieldmode ? 1 : 0), ps);
+		*pix_inc = pixinc(x_predecim * screen_width, ps);
 		break;
 
 	/* mirroring */
@@ -1602,10 +1609,14 @@ static void calc_dma_rotation_offset(u8 rotation, bool mirror,
 			*offset0 = *offset1 + field_offset * screen_width * ps;
 		else
 			*offset0 = *offset1;
-		*row_inc = pixinc(screen_width * 2 - 1 +
+		*row_inc = pixinc(y_predecim * screen_width * 2 - 1 +
 				(fieldmode ? screen_width : 0),
 				ps);
-		*pix_inc = pixinc(-1, ps);
+		if (color_mode = OMAP_DSS_COLOR_YUV2 ||
+			color_mode = OMAP_DSS_COLOR_UYVY)
+			*pix_inc = pixinc(-x_predecim, 2 * ps);
+		else
+			*pix_inc = pixinc(-x_predecim, ps);
 		break;
 
 	case OMAP_DSS_ROT_90 + 4:
@@ -1614,10 +1625,10 @@ static void calc_dma_rotation_offset(u8 rotation, bool mirror,
 			*offset0 = *offset1 + field_offset * ps;
 		else
 			*offset0 = *offset1;
-		*row_inc = pixinc(-screen_width * (fbh - 1) + 1 +
-				(fieldmode ? 1 : 0),
+		*row_inc = pixinc(-screen_width * (fbh * x_predecim - 1) +
+				y_predecim + (fieldmode ? 1 : 0),
 				ps);
-		*pix_inc = pixinc(screen_width, ps);
+		*pix_inc = pixinc(x_predecim * screen_width, ps);
 		break;
 
 	case OMAP_DSS_ROT_180 + 4:
@@ -1626,10 +1637,14 @@ static void calc_dma_rotation_offset(u8 rotation, bool mirror,
 			*offset0 = *offset1 - field_offset * screen_width * ps;
 		else
 			*offset0 = *offset1;
-		*row_inc = pixinc(1 - screen_width * 2 -
+		*row_inc = pixinc(1 - y_predecim * screen_width * 2 -
 				(fieldmode ? screen_width : 0),
 				ps);
-		*pix_inc = pixinc(1, ps);
+		if (color_mode = OMAP_DSS_COLOR_YUV2 ||
+			color_mode = OMAP_DSS_COLOR_UYVY)
+			*pix_inc = pixinc(x_predecim, 2 * ps);
+		else
+			*pix_inc = pixinc(x_predecim, ps);
 		break;
 
 	case OMAP_DSS_ROT_270 + 4:
@@ -1638,10 +1653,10 @@ static void calc_dma_rotation_offset(u8 rotation, bool mirror,
 			*offset0 = *offset1 - field_offset * ps;
 		else
 			*offset0 = *offset1;
-		*row_inc = pixinc(screen_width * (fbh - 1) - 1 -
-				(fieldmode ? 1 : 0),
+		*row_inc = pixinc(screen_width * (fbh * x_predecim - 1) -
+				y_predecim - (fieldmode ? 1 : 0),
 				ps);
-		*pix_inc = pixinc(-screen_width, ps);
+		*pix_inc = pixinc(-x_predecim * screen_width, ps);
 		break;
 
 	default:
@@ -1732,13 +1747,17 @@ static unsigned long calc_fclk(enum omap_channel channel, u16 width,
 static int dispc_ovl_calc_scaling(enum omap_plane plane,
 		enum omap_channel channel, u16 width, u16 height,
 		u16 out_width, u16 out_height,
-		enum omap_color_mode color_mode, bool *five_taps)
+		enum omap_color_mode color_mode, bool *five_taps,
+		int *x_predecim, int *y_predecim)
 {
 	struct omap_overlay *ovl = omap_dss_get_overlay(plane);
 	const int maxdownscale = dss_feat_get_param_max(FEAT_PARAM_DOWNSCALE);
 	const int maxsinglelinewidth  				dss_feat_get_param_max(FEAT_PARAM_LINEWIDTH);
+	const int max_decim_limit = 16;
 	unsigned long fclk = 0;
+	int decim_x, decim_y, error, min_factor;
+	u16 in_width, in_height, in_width_max = 0;
 
 	if (width = out_width && height = out_height)
 		return 0;
@@ -1746,45 +1765,121 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
 	if ((ovl->caps & OMAP_DSS_OVL_CAP_SCALE) = 0)
 		return -EINVAL;
 
-	if (out_width < width / maxdownscale ||
-			out_width > width * 8)
+	*x_predecim = max_decim_limit;
+	*y_predecim = max_decim_limit;
+
+	if (color_mode = OMAP_DSS_COLOR_CLUT1 ||
+	    color_mode = OMAP_DSS_COLOR_CLUT2 ||
+	    color_mode = OMAP_DSS_COLOR_CLUT4 ||
+	    color_mode = OMAP_DSS_COLOR_CLUT8) {
+		*x_predecim = 1;
+		*y_predecim = 1;
+		*five_taps = false;
+		return 0;
+	}
+
+	decim_x = DIV_ROUND_UP(DIV_ROUND_UP(width, out_width), maxdownscale);
+	decim_y = DIV_ROUND_UP(DIV_ROUND_UP(height, out_height), maxdownscale);
+
+	min_factor = min(decim_x, decim_y);
+
+	if (decim_x > *x_predecim || out_width > width * 8)
 		return -EINVAL;
 
-	if (out_height < height / maxdownscale ||
-			out_height > height * 8)
+	if (decim_y > *y_predecim || out_height > height * 8)
 		return -EINVAL;
 
 	if (cpu_is_omap24xx()) {
-		if (width > maxsinglelinewidth)
-			DSSERR("Cannot scale max input width exceeded");
 		*five_taps = false;
-		fclk = calc_fclk(channel, width, height, out_width,
-								out_height);
+
+		do {
+			in_height = DIV_ROUND_UP(height, decim_y);
+			in_width = DIV_ROUND_UP(width, decim_x);
+			fclk = calc_fclk(channel, in_width, in_height,
+					out_width, out_height);
+			error = (in_width > maxsinglelinewidth || !fclk ||
+				fclk > dispc_fclk_rate());
+			if (error) {
+				if (decim_x = decim_y) {
+					decim_x = min_factor;
+					decim_y++;
+				} else {
+					swap(decim_x, decim_y);
+					if (decim_x < decim_y)
+						decim_x++;
+				}
+			}
+		} while (decim_x <= *x_predecim && decim_y <= *y_predecim &&
+				error);
+
+		if (in_width > maxsinglelinewidth) {
+			DSSERR("Cannot scale max input width exceeded");
+			return -EINVAL;
+		}
 	} else if (cpu_is_omap34xx()) {
-		if (width > (maxsinglelinewidth * 2)) {
+
+		do {
+			in_height = DIV_ROUND_UP(height, decim_y);
+			in_width = DIV_ROUND_UP(width, decim_x);
+			fclk = calc_fclk_five_taps(channel, in_width, in_height,
+				out_width, out_height, color_mode);
+
+			if (in_width > maxsinglelinewidth)
+				if (in_height > out_height &&
+					in_height < out_height * 2)
+					*five_taps = false;
+			if (!*five_taps)
+				fclk = calc_fclk(channel, in_width, in_height,
+					out_width, out_height);
+			error = (in_width > maxsinglelinewidth * 2 ||
+				(in_width > maxsinglelinewidth && *five_taps) ||
+				!fclk || fclk > dispc_fclk_rate());
+			if (error) {
+				if (decim_x = decim_y) {
+					decim_x = min_factor;
+					decim_y++;
+				} else {
+					swap(decim_x, decim_y);
+					if (decim_x < decim_y)
+						decim_x++;
+				}
+			}
+		} while (decim_x <= *x_predecim && decim_y <= *y_predecim
+			&& error);
+
+		if (in_width > (maxsinglelinewidth * 2)) {
 			DSSERR("Cannot setup scaling");
 			DSSERR("width exceeds maximum width possible");
 			return -EINVAL;
 		}
-		fclk = calc_fclk_five_taps(channel, width, height, out_width,
-						out_height, color_mode);
-		if (width > maxsinglelinewidth) {
-			if (height > out_height && height < out_height * 2)
-				*five_taps = false;
-			else {
-				DSSERR("cannot setup scaling with five taps");
-				return -EINVAL;
-			}
+
+		if (in_width > maxsinglelinewidth && *five_taps) {
+			DSSERR("cannot setup scaling with five taps");
+			return -EINVAL;
 		}
-		if (!*five_taps)
-			fclk = calc_fclk(channel, width, height, out_width,
-					out_height);
 	} else {
-		if (width > maxsinglelinewidth) {
+		int decim_x_min = decim_x;
+		in_height = DIV_ROUND_UP(height, decim_y);
+		in_width_max = dispc_fclk_rate() /
+				DIV_ROUND_UP(dispc_mgr_pclk_rate(channel),
+						out_width);
+		decim_x = DIV_ROUND_UP(width, in_width_max);
+
+		decim_x = decim_x > decim_x_min ? decim_x : decim_x_min;
+		if (decim_x > *x_predecim)
+			return -EINVAL;
+
+		do {
+			in_width = DIV_ROUND_UP(width, decim_x);
+		} while (decim_x <= *x_predecim &&
+				in_width > maxsinglelinewidth && decim_x++);
+
+		if (in_width > maxsinglelinewidth) {
 			DSSERR("Cannot scale width exceeds max line width");
 			return -EINVAL;
 		}
-		fclk = calc_fclk(channel, width, height, out_width,
+
+		fclk = calc_fclk(channel, in_width, in_height, out_width,
 				out_height);
 	}
 
@@ -1799,6 +1894,8 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
 		return -EINVAL;
 	}
 
+	*x_predecim = decim_x;
+	*y_predecim = decim_y;
 	return 0;
 }
 
@@ -1814,8 +1911,11 @@ int dispc_ovl_setup(enum omap_plane plane, struct omap_overlay_info *oi,
 	s32 pix_inc;
 	u16 frame_height = oi->height;
 	unsigned int field_offset = 0;
-	u16 outw, outh;
+	u16 in_height = oi->height;
+	u16 in_width = oi->width;
+	u16 out_width, out_height;
 	enum omap_channel channel;
+	int x_predecim = 1, y_predecim = 1;
 
 	channel = dispc_ovl_get_channel_out(plane);
 
@@ -1829,32 +1929,35 @@ int dispc_ovl_setup(enum omap_plane plane, struct omap_overlay_info *oi,
 	if (oi->paddr = 0)
 		return -EINVAL;
 
-	outw = oi->out_width = 0 ? oi->width : oi->out_width;
-	outh = oi->out_height = 0 ? oi->height : oi->out_height;
+	out_width = oi->out_width = 0 ? oi->width : oi->out_width;
+	out_height = oi->out_height = 0 ? oi->height : oi->out_height;
 
-	if (ilace && oi->height = outh)
+	if (ilace && oi->height = out_height)
 		fieldmode = 1;
 
 	if (ilace) {
 		if (fieldmode)
-			oi->height /= 2;
+			in_height /= 2;
 		oi->pos_y /= 2;
-		outh /= 2;
+		out_height /= 2;
 
 		DSSDBG("adjusting for ilace: height %d, pos_y %d, "
 				"out_height %d\n",
-				oi->height, oi->pos_y, outh);
+				in_height, oi->pos_y, out_height);
 	}
 
 	if (!dss_feat_color_mode_supported(plane, oi->color_mode))
 		return -EINVAL;
 
-	r = dispc_ovl_calc_scaling(plane, channel, oi->width, oi->height,
-			outw, outh, oi->color_mode,
-			&five_taps);
+	r = dispc_ovl_calc_scaling(plane, channel, in_width, in_height,
+			out_width, out_height, oi->color_mode, &five_taps,
+			&x_predecim, &y_predecim);
 	if (r)
 		return r;
 
+	in_width = DIV_ROUND_UP(in_width, x_predecim);
+	in_height = DIV_ROUND_UP(in_height, y_predecim);
+
 	if (oi->color_mode = OMAP_DSS_COLOR_YUV2 ||
 			oi->color_mode = OMAP_DSS_COLOR_UYVY ||
 			oi->color_mode = OMAP_DSS_COLOR_NV12)
@@ -1868,10 +1971,10 @@ int dispc_ovl_setup(enum omap_plane plane, struct omap_overlay_info *oi,
 		 * so the integer part must be added to the base address of the
 		 * bottom field.
 		 */
-		if (!oi->height || oi->height = outh)
+		if (!in_height || in_height = out_height)
 			field_offset = 0;
 		else
-			field_offset = oi->height / outh / 2;
+			field_offset = in_height / out_height / 2;
 	}
 
 	/* Fields are independent but interleaved in memory. */
@@ -1880,14 +1983,16 @@ int dispc_ovl_setup(enum omap_plane plane, struct omap_overlay_info *oi,
 
 	if (oi->rotation_type = OMAP_DSS_ROT_DMA)
 		calc_dma_rotation_offset(oi->rotation, oi->mirror,
-				oi->screen_width, oi->width, frame_height,
+				oi->screen_width, in_width, frame_height,
 				oi->color_mode, fieldmode, field_offset,
-				&offset0, &offset1, &row_inc, &pix_inc);
+				&offset0, &offset1, &row_inc, &pix_inc,
+				x_predecim, y_predecim);
 	else
 		calc_vrfb_rotation_offset(oi->rotation, oi->mirror,
-				oi->screen_width, oi->width, frame_height,
+				oi->screen_width, in_width, frame_height,
 				oi->color_mode, fieldmode, field_offset,
-				&offset0, &offset1, &row_inc, &pix_inc);
+				&offset0, &offset1, &row_inc, &pix_inc,
+				x_predecim, y_predecim);
 
 	DSSDBG("offset0 %u, offset1 %u, row_inc %d, pix_inc %d\n",
 			offset0, offset1, row_inc, pix_inc);
@@ -1906,19 +2011,18 @@ int dispc_ovl_setup(enum omap_plane plane, struct omap_overlay_info *oi,
 	dispc_ovl_set_row_inc(plane, row_inc);
 	dispc_ovl_set_pix_inc(plane, pix_inc);
 
-	DSSDBG("%d,%d %dx%d -> %dx%d\n", oi->pos_x, oi->pos_y, oi->width,
-			oi->height, outw, outh);
+	DSSDBG("%d,%d %dx%d -> %dx%d\n", oi->pos_x, oi->pos_y, in_width,
+			in_height, out_width, out_height);
 
 	dispc_ovl_set_pos(plane, oi->pos_x, oi->pos_y);
 
-	dispc_ovl_set_pic_size(plane, oi->width, oi->height);
+	dispc_ovl_set_pic_size(plane, in_width, in_height);
 
 	if (ovl->caps & OMAP_DSS_OVL_CAP_SCALE) {
-		dispc_ovl_set_scaling(plane, oi->width, oi->height,
-				   outw, outh,
-				   ilace, five_taps, fieldmode,
+		dispc_ovl_set_scaling(plane, in_width, in_height, out_width,
+				   out_height, ilace, five_taps, fieldmode,
 				   oi->color_mode, oi->rotation);
-		dispc_ovl_set_vid_size(plane, outw, outh);
+		dispc_ovl_set_vid_size(plane, out_width, out_height);
 		dispc_ovl_set_vid_color_conv(plane, cconv);
 	}
 
-- 
1.7.1

[PATCH V3 2/3] OMAPDSS: DISPC: Handle synclost errors in OMAP3

[Chandrabhanu Mahapatra]

In OMAP3 DISPC video overlays suffer from some undocumented horizontal position
and timing related limitations leading to SYNCLOST errors. Whenever the image
window is moved towards the right of the screen SYNCLOST errors become
frequent. Checks have been implemented to see that DISPC driver rejects
configuration exceeding above limitations.

This code was successfully tested on OMAP3. This code is written based on code
written by Ville Syrj채l채 <ville.syrjala@nokia.com> in Linux OMAP kernel. Ville
Syrj채l채 <ville.syrjala@nokia.com> had added checks for video overlay horizontal
timing and DISPC horizontal blanking length limitations.

Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
---
 drivers/video/omap2/dss/dispc.c |   72 +++++++++++++++++++++++++++++++++++++--
 1 files changed, 69 insertions(+), 3 deletions(-)

diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index 4ab5433..17ffa71 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -1664,6 +1664,63 @@ static void calc_dma_rotation_offset(u8 rotation, bool mirror,
 	}
 }
 
+/*
+ * This function is used to avoid synclosts in OMAP3, because of some
+ * undocumented horizontal position and timing related limitations.
+ */
+static int check_horiz_timing_omap3(enum omap_channel channel, u16 pos_x,
+		u16 width, u16 height, u16 out_width, u16 out_height)
+{
+	int DS = DIV_ROUND_UP(height, out_height);
+	struct omap_dss_device *dssdev = dispc_mgr_get_device(channel);
+	struct omap_video_timings t = dssdev->panel.timings;
+	unsigned long nonactive, lclk, pclk;
+	static const u8 limits[3] = { 8, 10, 20 };
+	u64 val, blank;
+	int i;
+
+	nonactive = t.x_res + t.hfp + t.hsw + t.hbp - out_width;
+	pclk = dispc_mgr_pclk_rate(channel);
+	if (dispc_mgr_is_lcd(channel))
+		lclk = dispc_mgr_lclk_rate(channel);
+	else
+		lclk = dispc_fclk_rate();
+
+	i = 0;
+	if (out_height < height)
+		i++;
+	if (out_width < width)
+		i++;
+	blank = div_u64((u64)(t.hbp + t.hsw + t.hfp) * lclk, pclk);
+	DSSDBG("blanking period + ppl = %llu (limit = %u)\n", blank, limits[i]);
+	if (blank <= limits[i])
+		return -EINVAL;
+
+	/*
+	 * Pixel data should be prepared before visible display point starts.
+	 * So, atleast DS-2 lines must have already been fetched by DISPC
+	 * during nonactive - pos_x period.
+	 */
+	val = div_u64((u64)(nonactive - pos_x) * lclk, pclk);
+	DSSDBG("(nonactive - pos_x) * pcd = %llu max(0, DS - 2) * width = %d\n",
+		val, max(0, DS - 2) * width);
+	if (val < max(0, DS - 2) * width)
+		return -EINVAL;
+
+	/*
+	 * All lines need to be refilled during the nonactive period of which
+	 * only one line can be loaded during the active period. So, atleast
+	 * DS - 1 lines should be loaded during nonactive period.
+	 */
+	val =  div_u64((u64)nonactive * lclk, pclk);
+	DSSDBG("nonactive * pcd  = %llu, max(0, DS - 1) * width = %d\n",
+		val, max(0, DS - 1) * width);
+	if (val < max(0, DS - 1) * width)
+		return -EINVAL;
+
+	return 0;
+}
+
 static unsigned long calc_fclk_five_taps(enum omap_channel channel, u16 width,
 		u16 height, u16 out_width, u16 out_height,
 		enum omap_color_mode color_mode)
@@ -1748,7 +1805,7 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
 		enum omap_channel channel, u16 width, u16 height,
 		u16 out_width, u16 out_height,
 		enum omap_color_mode color_mode, bool *five_taps,
-		int *x_predecim, int *y_predecim)
+		int *x_predecim, int *y_predecim, u16 pos_x)
 {
 	struct omap_overlay *ovl = omap_dss_get_overlay(plane);
 	const int maxdownscale = dss_feat_get_param_max(FEAT_PARAM_DOWNSCALE);
@@ -1824,6 +1881,9 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
 			fclk = calc_fclk_five_taps(channel, in_width, in_height,
 				out_width, out_height, color_mode);
 
+			error = check_horiz_timing_omap3(channel, pos_x,
+				in_width, in_height, out_width, out_height);
+
 			if (in_width > maxsinglelinewidth)
 				if (in_height > out_height &&
 					in_height < out_height * 2)
@@ -1831,7 +1891,7 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
 			if (!*five_taps)
 				fclk = calc_fclk(channel, in_width, in_height,
 					out_width, out_height);
-			error = (in_width > maxsinglelinewidth * 2 ||
+			error = (error || in_width > maxsinglelinewidth * 2 ||
 				(in_width > maxsinglelinewidth && *five_taps) ||
 				!fclk || fclk > dispc_fclk_rate());
 			if (error) {
@@ -1847,6 +1907,12 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
 		} while (decim_x <= *x_predecim && decim_y <= *y_predecim
 			&& error);
 
+		if (check_horiz_timing_omap3(channel, pos_x, width, height,
+			out_width, out_height)){
+				DSSERR("horizontal timing too tight\n");
+				return -EINVAL;
+		}
+
 		if (in_width > (maxsinglelinewidth * 2)) {
 			DSSERR("Cannot setup scaling");
 			DSSERR("width exceeds maximum width possible");
@@ -1951,7 +2017,7 @@ int dispc_ovl_setup(enum omap_plane plane, struct omap_overlay_info *oi,
 
 	r = dispc_ovl_calc_scaling(plane, channel, in_width, in_height,
 			out_width, out_height, oi->color_mode, &five_taps,
-			&x_predecim, &y_predecim);
+			&x_predecim, &y_predecim, oi->pos_x);
 	if (r)
 		return r;
 
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH V3 3/3] OMAPDSS: DISPC: Correct DISPC functional clock usage

[Chandrabhanu Mahapatra]

DISPC_FCLK is incorrectly used as functional clock of DISPC in scaling
calculations. So, DISPC_CORE_CLK replaces as functional clock of DISPC.
DISPC_CORE_CLK is derived from DISPC_FCLK divided by an independent DISPC
divisor LCD.

Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
---
 drivers/video/omap2/dss/dispc.c |   28 ++++++++++++++++++++++------
 drivers/video/omap2/dss/dss.h   |    1 +
 2 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index 17ffa71..cfde674 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -1813,6 +1813,7 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
 				dss_feat_get_param_max(FEAT_PARAM_LINEWIDTH);
 	const int max_decim_limit = 16;
 	unsigned long fclk = 0;
+	unsigned long dispc_core_clk = dispc_core_clk_rate(channel);
 	int decim_x, decim_y, error, min_factor;
 	u16 in_width, in_height, in_width_max = 0;
 
@@ -1855,7 +1856,7 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
 			fclk = calc_fclk(channel, in_width, in_height,
 					out_width, out_height);
 			error = (in_width > maxsinglelinewidth || !fclk ||
-				fclk > dispc_fclk_rate());
+				fclk > dispc_core_clk);
 			if (error) {
 				if (decim_x = decim_y) {
 					decim_x = min_factor;
@@ -1893,7 +1894,7 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
 					out_width, out_height);
 			error = (error || in_width > maxsinglelinewidth * 2 ||
 				(in_width > maxsinglelinewidth && *five_taps) ||
-				!fclk || fclk > dispc_fclk_rate());
+				!fclk || fclk > dispc_core_clk);
 			if (error) {
 				if (decim_x = decim_y) {
 					decim_x = min_factor;
@@ -1926,7 +1927,7 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
 	} else {
 		int decim_x_min = decim_x;
 		in_height = DIV_ROUND_UP(height, decim_y);
-		in_width_max = dispc_fclk_rate() /
+		in_width_max = dispc_core_clk /
 				DIV_ROUND_UP(dispc_mgr_pclk_rate(channel),
 						out_width);
 		decim_x = DIV_ROUND_UP(width, in_width_max);
@@ -1950,13 +1951,13 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
 	}
 
 	DSSDBG("required fclk rate = %lu Hz\n", fclk);
-	DSSDBG("current fclk rate = %lu Hz\n", dispc_fclk_rate());
+	DSSDBG("current fclk rate = %lu Hz\n", dispc_core_clk);
 
-	if (!fclk || fclk > dispc_fclk_rate()) {
+	if (!fclk || fclk > dispc_core_clk) {
 		DSSERR("failed to set up scaling, "
 			"required fclk rate = %lu Hz, "
 			"current fclk rate = %lu Hz\n",
-			fclk, dispc_fclk_rate());
+			fclk, dispc_core_clk);
 		return -EINVAL;
 	}
 
@@ -2646,6 +2647,21 @@ unsigned long dispc_mgr_pclk_rate(enum omap_channel channel)
 	}
 }
 
+unsigned long dispc_core_clk_rate(enum omap_channel channel)
+{
+	int lcd = 1;
+	unsigned long r = dispc_fclk_rate();
+
+	if (dss_has_feature(FEAT_CORE_CLK_DIV)) {
+		lcd = REG_GET(DISPC_DIVISOR, 23, 16);
+	} else {
+		if (dispc_mgr_is_lcd(channel))
+			lcd = REG_GET(DISPC_DIVISORo(channel), 23, 16);
+	}
+
+	return r / lcd ;
+}
+
 void dispc_dump_clocks(struct seq_file *s)
 {
 	int lcd, pcd;
diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
index d4b3dff..4748923 100644
--- a/drivers/video/omap2/dss/dss.h
+++ b/drivers/video/omap2/dss/dss.h
@@ -451,6 +451,7 @@ void dispc_mgr_set_pol_freq(enum omap_channel channel,
 		enum omap_panel_config config, u8 acbi, u8 acb);
 unsigned long dispc_mgr_lclk_rate(enum omap_channel channel);
 unsigned long dispc_mgr_pclk_rate(enum omap_channel channel);
+unsigned long dispc_core_clk_rate(enum omap_channel channel);
 int dispc_mgr_set_clock_div(enum omap_channel channel,
 		struct dispc_clock_info *cinfo);
 int dispc_mgr_get_clock_div(enum omap_channel channel,
-- 
1.7.1

Re: [PATCH V3 3/3] OMAPDSS: DISPC: Correct DISPC functional clock usage

[Tomi Valkeinen]

[-- Attachment #1: Type: text/plain, Size: 4271 bytes --]

On Mon, 2012-04-02 at 20:43 +0530, Chandrabhanu Mahapatra wrote:
> DISPC_FCLK is incorrectly used as functional clock of DISPC in scaling
> calculations. So, DISPC_CORE_CLK replaces as functional clock of DISPC.
> DISPC_CORE_CLK is derived from DISPC_FCLK divided by an independent DISPC
> divisor LCD.
> 
> Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
> ---
>  drivers/video/omap2/dss/dispc.c |   28 ++++++++++++++++++++++------
>  drivers/video/omap2/dss/dss.h   |    1 +
>  2 files changed, 23 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
> index 17ffa71..cfde674 100644
> --- a/drivers/video/omap2/dss/dispc.c
> +++ b/drivers/video/omap2/dss/dispc.c
> @@ -1813,6 +1813,7 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
>  				dss_feat_get_param_max(FEAT_PARAM_LINEWIDTH);
>  	const int max_decim_limit = 16;
>  	unsigned long fclk = 0;
> +	unsigned long dispc_core_clk = dispc_core_clk_rate(channel);
>  	int decim_x, decim_y, error, min_factor;
>  	u16 in_width, in_height, in_width_max = 0;
>  
> @@ -1855,7 +1856,7 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
>  			fclk = calc_fclk(channel, in_width, in_height,
>  					out_width, out_height);
>  			error = (in_width > maxsinglelinewidth || !fclk ||
> -				fclk > dispc_fclk_rate());
> +				fclk > dispc_core_clk);
>  			if (error) {
>  				if (decim_x == decim_y) {
>  					decim_x = min_factor;
> @@ -1893,7 +1894,7 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
>  					out_width, out_height);
>  			error = (error || in_width > maxsinglelinewidth * 2 ||
>  				(in_width > maxsinglelinewidth && *five_taps) ||
> -				!fclk || fclk > dispc_fclk_rate());
> +				!fclk || fclk > dispc_core_clk);
>  			if (error) {
>  				if (decim_x == decim_y) {
>  					decim_x = min_factor;
> @@ -1926,7 +1927,7 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
>  	} else {
>  		int decim_x_min = decim_x;
>  		in_height = DIV_ROUND_UP(height, decim_y);
> -		in_width_max = dispc_fclk_rate() /
> +		in_width_max = dispc_core_clk /
>  				DIV_ROUND_UP(dispc_mgr_pclk_rate(channel),
>  						out_width);
>  		decim_x = DIV_ROUND_UP(width, in_width_max);
> @@ -1950,13 +1951,13 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
>  	}
>  
>  	DSSDBG("required fclk rate = %lu Hz\n", fclk);
> -	DSSDBG("current fclk rate = %lu Hz\n", dispc_fclk_rate());
> +	DSSDBG("current fclk rate = %lu Hz\n", dispc_core_clk);
>  
> -	if (!fclk || fclk > dispc_fclk_rate()) {
> +	if (!fclk || fclk > dispc_core_clk) {
>  		DSSERR("failed to set up scaling, "
>  			"required fclk rate = %lu Hz, "
>  			"current fclk rate = %lu Hz\n",
> -			fclk, dispc_fclk_rate());
> +			fclk, dispc_core_clk);
>  		return -EINVAL;
>  	}
>  
> @@ -2646,6 +2647,21 @@ unsigned long dispc_mgr_pclk_rate(enum omap_channel channel)
>  	}
>  }
>  
> +unsigned long dispc_core_clk_rate(enum omap_channel channel)
> +{
> +	int lcd = 1;
> +	unsigned long r = dispc_fclk_rate();
> +
> +	if (dss_has_feature(FEAT_CORE_CLK_DIV)) {
> +		lcd = REG_GET(DISPC_DIVISOR, 23, 16);
> +	} else {
> +		if (dispc_mgr_is_lcd(channel))
> +			lcd = REG_GET(DISPC_DIVISORo(channel), 23, 16);
> +	}
> +
> +	return r / lcd ;
> +}
> +

I wonder if this is correct. "channel" for dispc core clock doesn't make
sense, there's no channel related to that. At least on OMAP4.

If I'm not mistaken, in omap2/3 case (i.e.
dss_has_feature(FEAT_CORE_CLK_DIV) == false) we can just use channel 0
to get the lcd divisor. Although that would mean that LCD output's
divisor affects the tv-out's scaling calculations, which feels a bit
strange...

So... I don't know... Do we really have two different dispc core clocks
int omap2/3 (for lcd output and for tv output), but only one in omap4?
If so, then the above code is ok. Have you found explanations from TRM
which say what clock is required and where?

In any case, please remove the initialization of lcd variable, and add:

else
	lcd = 1;

I think that's much clearer. And "r" variable is commonly used as a
return value. I would rename the variable to something else, say, "fck".

 Tomi


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH V3 3/3] OMAPDSS: DISPC: Correct DISPC functional clock usage

[Mahapatra, Chandrabhanu]

On Thu, Apr 19, 2012 at 6:41 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On Mon, 2012-04-02 at 20:43 +0530, Chandrabhanu Mahapatra wrote:
>> DISPC_FCLK is incorrectly used as functional clock of DISPC in scaling
>> calculations. So, DISPC_CORE_CLK replaces as functional clock of DISPC.
>> DISPC_CORE_CLK is derived from DISPC_FCLK divided by an independent DISPC
>> divisor LCD.
>>
>> Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
>> ---
>>  drivers/video/omap2/dss/dispc.c |   28 ++++++++++++++++++++++------
>>  drivers/video/omap2/dss/dss.h   |    1 +
>>  2 files changed, 23 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
>> index 17ffa71..cfde674 100644
>> --- a/drivers/video/omap2/dss/dispc.c
>> +++ b/drivers/video/omap2/dss/dispc.c
>> @@ -1813,6 +1813,7 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
>>                               dss_feat_get_param_max(FEAT_PARAM_LINEWIDTH);
>>       const int max_decim_limit = 16;
>>       unsigned long fclk = 0;
>> +     unsigned long dispc_core_clk = dispc_core_clk_rate(channel);
>>       int decim_x, decim_y, error, min_factor;
>>       u16 in_width, in_height, in_width_max = 0;
>>
>> @@ -1855,7 +1856,7 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
>>                       fclk = calc_fclk(channel, in_width, in_height,
>>                                       out_width, out_height);
>>                       error = (in_width > maxsinglelinewidth || !fclk ||
>> -                             fclk > dispc_fclk_rate());
>> +                             fclk > dispc_core_clk);
>>                       if (error) {
>>                               if (decim_x = decim_y) {
>>                                       decim_x = min_factor;
>> @@ -1893,7 +1894,7 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
>>                                       out_width, out_height);
>>                       error = (error || in_width > maxsinglelinewidth * 2 ||
>>                               (in_width > maxsinglelinewidth && *five_taps) ||
>> -                             !fclk || fclk > dispc_fclk_rate());
>> +                             !fclk || fclk > dispc_core_clk);
>>                       if (error) {
>>                               if (decim_x = decim_y) {
>>                                       decim_x = min_factor;
>> @@ -1926,7 +1927,7 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
>>       } else {
>>               int decim_x_min = decim_x;
>>               in_height = DIV_ROUND_UP(height, decim_y);
>> -             in_width_max = dispc_fclk_rate() /
>> +             in_width_max = dispc_core_clk /
>>                               DIV_ROUND_UP(dispc_mgr_pclk_rate(channel),
>>                                               out_width);
>>               decim_x = DIV_ROUND_UP(width, in_width_max);
>> @@ -1950,13 +1951,13 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
>>       }
>>
>>       DSSDBG("required fclk rate = %lu Hz\n", fclk);
>> -     DSSDBG("current fclk rate = %lu Hz\n", dispc_fclk_rate());
>> +     DSSDBG("current fclk rate = %lu Hz\n", dispc_core_clk);
>>
>> -     if (!fclk || fclk > dispc_fclk_rate()) {
>> +     if (!fclk || fclk > dispc_core_clk) {
>>               DSSERR("failed to set up scaling, "
>>                       "required fclk rate = %lu Hz, "
>>                       "current fclk rate = %lu Hz\n",
>> -                     fclk, dispc_fclk_rate());
>> +                     fclk, dispc_core_clk);
>>               return -EINVAL;
>>       }
>>
>> @@ -2646,6 +2647,21 @@ unsigned long dispc_mgr_pclk_rate(enum omap_channel channel)
>>       }
>>  }
>>
>> +unsigned long dispc_core_clk_rate(enum omap_channel channel)
>> +{
>> +     int lcd = 1;
>> +     unsigned long r = dispc_fclk_rate();
>> +
>> +     if (dss_has_feature(FEAT_CORE_CLK_DIV)) {
>> +             lcd = REG_GET(DISPC_DIVISOR, 23, 16);
>> +     } else {
>> +             if (dispc_mgr_is_lcd(channel))
>> +                     lcd = REG_GET(DISPC_DIVISORo(channel), 23, 16);
>> +     }
>> +
>> +     return r / lcd ;
>> +}
>> +
>
> I wonder if this is correct. "channel" for dispc core clock doesn't make
> sense, there's no channel related to that. At least on OMAP4.
>
> If I'm not mistaken, in omap2/3 case (i.e.
> dss_has_feature(FEAT_CORE_CLK_DIV) = false) we can just use channel 0
> to get the lcd divisor. Although that would mean that LCD output's
> divisor affects the tv-out's scaling calculations, which feels a bit
> strange...
>

Yes, you are right. I had failed to look at channel 0 or
OMAP_DSS_CHANNEL_LCD can simply be used.

> So... I don't know... Do we really have two different dispc core clocks
> int omap2/3 (for lcd output and for tv output), but only one in omap4?
> If so, then the above code is ok. Have you found explanations from TRM
> which say what clock is required and where?
>

Though the term Dispc Core Clock is never used in OMAP2/3 TRM, it is
actually the logic clock that drives the various logics of DSS
subsystem. So we can say that we have one dispc core clock for OMAP2/3
and that being the logic clock. I took two different logics as LCD
factor returned zero for TV.  However, LCD factor of DISPC_DIVISOR of
channel 0 can be used always.

> In any case, please remove the initialization of lcd variable, and add:
>
> else
>        lcd = 1;
>
> I think that's much clearer. And "r" variable is commonly used as a
> return value. I would rename the variable to something else, say, "fck".
>
>  Tomi
>

Yes, sure it makes code more understandable and clean.


-- 
Chandrabhanu Mahapatra
Texas Instruments India Pvt. Ltd.

[PATCH V3 3/3] OMAPDSS: DISPC: Correct DISPC functional clock usage

[Chandrabhanu Mahapatra]

DISPC_FCLK is incorrectly used as functional clock of DISPC in scaling
calculations. So, DISPC_CORE_CLK replaces as functional clock of DISPC.
DISPC_CORE_CLK is derived from DISPC_FCLK divided by an independent DISPC
divisor LCD.

Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
---
 drivers/video/omap2/dss/dispc.c |   25 +++++++++++++++++++------
 drivers/video/omap2/dss/dss.h   |    1 +
 2 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index 17ffa71..0417423 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -1855,7 +1855,7 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
 			fclk = calc_fclk(channel, in_width, in_height,
 					out_width, out_height);
 			error = (in_width > maxsinglelinewidth || !fclk ||
-				fclk > dispc_fclk_rate());
+				fclk > dispc_core_clk_rate());
 			if (error) {
 				if (decim_x = decim_y) {
 					decim_x = min_factor;
@@ -1893,7 +1893,7 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
 					out_width, out_height);
 			error = (error || in_width > maxsinglelinewidth * 2 ||
 				(in_width > maxsinglelinewidth && *five_taps) ||
-				!fclk || fclk > dispc_fclk_rate());
+				!fclk || fclk > dispc_core_clk_rate());
 			if (error) {
 				if (decim_x = decim_y) {
 					decim_x = min_factor;
@@ -1926,7 +1926,7 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
 	} else {
 		int decim_x_min = decim_x;
 		in_height = DIV_ROUND_UP(height, decim_y);
-		in_width_max = dispc_fclk_rate() /
+		in_width_max = dispc_core_clk_rate() /
 				DIV_ROUND_UP(dispc_mgr_pclk_rate(channel),
 						out_width);
 		decim_x = DIV_ROUND_UP(width, in_width_max);
@@ -1950,13 +1950,13 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
 	}
 
 	DSSDBG("required fclk rate = %lu Hz\n", fclk);
-	DSSDBG("current fclk rate = %lu Hz\n", dispc_fclk_rate());
+	DSSDBG("current fclk rate = %lu Hz\n", dispc_core_clk_rate());
 
-	if (!fclk || fclk > dispc_fclk_rate()) {
+	if (!fclk || fclk > dispc_core_clk_rate()) {
 		DSSERR("failed to set up scaling, "
 			"required fclk rate = %lu Hz, "
 			"current fclk rate = %lu Hz\n",
-			fclk, dispc_fclk_rate());
+			fclk, dispc_core_clk_rate());
 		return -EINVAL;
 	}
 
@@ -2646,6 +2646,19 @@ unsigned long dispc_mgr_pclk_rate(enum omap_channel channel)
 	}
 }
 
+unsigned long dispc_core_clk_rate(void)
+{
+	int lcd;
+	unsigned long fclk = dispc_fclk_rate();
+
+	if (dss_has_feature(FEAT_CORE_CLK_DIV))
+		lcd = REG_GET(DISPC_DIVISOR, 23, 16);
+	else
+		lcd = REG_GET(DISPC_DIVISORo(OMAP_DSS_CHANNEL_LCD), 23, 16);
+
+	return fclk / lcd;
+}
+
 void dispc_dump_clocks(struct seq_file *s)
 {
 	int lcd, pcd;
diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
index d4b3dff..1bde34c 100644
--- a/drivers/video/omap2/dss/dss.h
+++ b/drivers/video/omap2/dss/dss.h
@@ -451,6 +451,7 @@ void dispc_mgr_set_pol_freq(enum omap_channel channel,
 		enum omap_panel_config config, u8 acbi, u8 acb);
 unsigned long dispc_mgr_lclk_rate(enum omap_channel channel);
 unsigned long dispc_mgr_pclk_rate(enum omap_channel channel);
+unsigned long dispc_core_clk_rate(void);
 int dispc_mgr_set_clock_div(enum omap_channel channel,
 		struct dispc_clock_info *cinfo);
 int dispc_mgr_get_clock_div(enum omap_channel channel,
-- 
1.7.1

Re: [PATCH V3 3/3] OMAPDSS: DISPC: Correct DISPC functional clock usage

[Tomi Valkeinen]

[-- Attachment #1: Type: text/plain, Size: 1327 bytes --]

On Fri, 2012-04-20 at 19:01 +0530, Chandrabhanu Mahapatra wrote:
> DISPC_FCLK is incorrectly used as functional clock of DISPC in scaling
> calculations. So, DISPC_CORE_CLK replaces as functional clock of DISPC.
> DISPC_CORE_CLK is derived from DISPC_FCLK divided by an independent DISPC
> divisor LCD.
> 
> Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
> ---
>  drivers/video/omap2/dss/dispc.c |   25 +++++++++++++++++++------
>  drivers/video/omap2/dss/dss.h   |    1 +
>  2 files changed, 20 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
> index 17ffa71..0417423 100644
> --- a/drivers/video/omap2/dss/dispc.c
> +++ b/drivers/video/omap2/dss/dispc.c
> @@ -1855,7 +1855,7 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
>  			fclk = calc_fclk(channel, in_width, in_height,
>  					out_width, out_height);
>  			error = (in_width > maxsinglelinewidth || !fclk ||
> -				fclk > dispc_fclk_rate());
> +				fclk > dispc_core_clk_rate());

The patch is now otherwise fine, but I think it needs some more
renaming. Now the code mixes fclk and core-clk names, which is rather
confusing. I guess the calc_fclk should actually be calc_req_core_clk?
And the fclk variable core_clk (or cclk or something)?

 Tomi


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH V3 3/3] OMAPDSS: DISPC: Correct DISPC functional clock usage

[Mahapatra, Chandrabhanu]

On Fri, Apr 20, 2012 at 7:59 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On Fri, 2012-04-20 at 19:01 +0530, Chandrabhanu Mahapatra wrote:
>> DISPC_FCLK is incorrectly used as functional clock of DISPC in scaling
>> calculations. So, DISPC_CORE_CLK replaces as functional clock of DISPC.
>> DISPC_CORE_CLK is derived from DISPC_FCLK divided by an independent DISPC
>> divisor LCD.
>>
>> Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
>> ---
>>  drivers/video/omap2/dss/dispc.c |   25 +++++++++++++++++++------
>>  drivers/video/omap2/dss/dss.h   |    1 +
>>  2 files changed, 20 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
>> index 17ffa71..0417423 100644
>> --- a/drivers/video/omap2/dss/dispc.c
>> +++ b/drivers/video/omap2/dss/dispc.c
>> @@ -1855,7 +1855,7 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
>>                       fclk = calc_fclk(channel, in_width, in_height,
>>                                       out_width, out_height);
>>                       error = (in_width > maxsinglelinewidth || !fclk ||
>> -                             fclk > dispc_fclk_rate());
>> +                             fclk > dispc_core_clk_rate());
>
> The patch is now otherwise fine, but I think it needs some more
> renaming. Now the code mixes fclk and core-clk names, which is rather
> confusing. I guess the calc_fclk should actually be calc_req_core_clk?
> And the fclk variable core_clk (or cclk or something)?
>
>  Tomi
>

As per TRM dispc_core_clk is also a functional clock, so I think above
code should be fine. But, to avoid confusion of names the renaming of
fclk, calc_fclk and calc_fclk_five_taps can be done.


-- 
Chandrabhanu Mahapatra
Texas Instruments India Pvt. Ltd.

Re: [PATCH V3 3/3] OMAPDSS: DISPC: Correct DISPC functional clock usage

[Tomi Valkeinen]

[-- Attachment #1: Type: text/plain, Size: 1922 bytes --]

On Mon, 2012-04-23 at 10:05 +0530, Mahapatra, Chandrabhanu wrote:
> On Fri, Apr 20, 2012 at 7:59 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:

> > The patch is now otherwise fine, but I think it needs some more
> > renaming. Now the code mixes fclk and core-clk names, which is rather
> > confusing. I guess the calc_fclk should actually be calc_req_core_clk?
> > And the fclk variable core_clk (or cclk or something)?
> >
> >  Tomi
> >
> 
> As per TRM dispc_core_clk is also a functional clock, so I think above
> code should be fine. But, to avoid confusion of names the renaming of
> fclk, calc_fclk and calc_fclk_five_taps can be done.

Well, that is true is a sense, if you consider a "functional clock"
something that is used to drive some function of the HW. But then we
could also call the pixel clock a functional clock, as it also drives
some functionality. And true, TRM seems to refer to the core clock as
"dispc internal functional clock" at times.

But in this case, looking at the clock tree, DISPC_FCLK is the clock
coming to DISPC, which is then divided by the logic clock divisor to get
the logic clock (or core clock, or DISPC internal functional clock). And
I think the TRM, at least the OMAP2/3 versions, mostly speak of the
DISPC_FCLK as the functional clock, and that's what the driver also has
meant with "functional clock".

So to avoid any confusion, I'd suggest renaming them. At least I don't
see any benefit in having multiple clocks called "functional clock" for
dispc...

That said, I wonder if the DISPC_FCLK, LCD1_CLK and LCD2_CLK are used at
all before they are divided with the logic clock divider. If they are
not, then it could make sense to speak of the logic clock as the
functional clock, and call the incoming clocks something else.

But for the time being, I think we should continue calling the internal
fclk as core-clock.

 Tomi


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH V3 3/3] OMAPDSS: DISPC: Correct DISPC functional clock usage

[Chandrabhanu Mahapatra]

DISPC_FCLK is incorrectly used as functional clock of DISPC in scaling
calculations. So, DISPC_CORE_CLK replaces as functional clock of DISPC.
DISPC_CORE_CLK is derived from DISPC_FCLK divided by an independent DISPC
divisor LCD.

Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
---
 drivers/video/omap2/dss/dispc.c |   67 +++++++++++++++++++++++---------------
 drivers/video/omap2/dss/dss.h   |    1 +
 2 files changed, 41 insertions(+), 27 deletions(-)

diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index 17ffa71..f1be23f 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -1721,11 +1721,11 @@ static int check_horiz_timing_omap3(enum omap_channel channel, u16 pos_x,
 	return 0;
 }
 
-static unsigned long calc_fclk_five_taps(enum omap_channel channel, u16 width,
-		u16 height, u16 out_width, u16 out_height,
+static unsigned long calc_core_clk_five_taps(enum omap_channel channel,
+		u16 width, u16 height, u16 out_width, u16 out_height,
 		enum omap_color_mode color_mode)
 {
-	u32 fclk = 0;
+	u32 core_clk = 0;
 	u64 tmp, pclk = dispc_mgr_pclk_rate(channel);
 
 	if (height <= out_height && width <= out_width)
@@ -1737,7 +1737,7 @@ static unsigned long calc_fclk_five_taps(enum omap_channel channel, u16 width,
 
 		tmp = pclk * height * out_width;
 		do_div(tmp, 2 * out_height * ppl);
-		fclk = tmp;
+		core_clk = tmp;
 
 		if (height > 2 * out_height) {
 			if (ppl = out_width)
@@ -1745,23 +1745,23 @@ static unsigned long calc_fclk_five_taps(enum omap_channel channel, u16 width,
 
 			tmp = pclk * (height - 2 * out_height) * out_width;
 			do_div(tmp, 2 * out_height * (ppl - out_width));
-			fclk = max(fclk, (u32) tmp);
+			core_clk = max_t(u32, core_clk, tmp);
 		}
 	}
 
 	if (width > out_width) {
 		tmp = pclk * width;
 		do_div(tmp, out_width);
-		fclk = max(fclk, (u32) tmp);
+		core_clk = max_t(u32, core_clk, tmp);
 
 		if (color_mode = OMAP_DSS_COLOR_RGB24U)
-			fclk <<= 1;
+			core_clk <<= 1;
 	}
 
-	return fclk;
+	return core_clk;
 }
 
-static unsigned long calc_fclk(enum omap_channel channel, u16 width,
+static unsigned long calc_core_clk(enum omap_channel channel, u16 width,
 		u16 height, u16 out_width, u16 out_height)
 {
 	unsigned int hf, vf;
@@ -1812,7 +1812,7 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
 	const int maxsinglelinewidth  				dss_feat_get_param_max(FEAT_PARAM_LINEWIDTH);
 	const int max_decim_limit = 16;
-	unsigned long fclk = 0;
+	unsigned long core_clk = 0;
 	int decim_x, decim_y, error, min_factor;
 	u16 in_width, in_height, in_width_max = 0;
 
@@ -1852,10 +1852,10 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
 		do {
 			in_height = DIV_ROUND_UP(height, decim_y);
 			in_width = DIV_ROUND_UP(width, decim_x);
-			fclk = calc_fclk(channel, in_width, in_height,
+			core_clk = calc_core_clk(channel, in_width, in_height,
 					out_width, out_height);
-			error = (in_width > maxsinglelinewidth || !fclk ||
-				fclk > dispc_fclk_rate());
+			error = (in_width > maxsinglelinewidth || !core_clk ||
+				core_clk > dispc_core_clk_rate());
 			if (error) {
 				if (decim_x = decim_y) {
 					decim_x = min_factor;
@@ -1878,8 +1878,8 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
 		do {
 			in_height = DIV_ROUND_UP(height, decim_y);
 			in_width = DIV_ROUND_UP(width, decim_x);
-			fclk = calc_fclk_five_taps(channel, in_width, in_height,
-				out_width, out_height, color_mode);
+			core_clk = calc_core_clk_five_taps(channel, in_width,
+				in_height, out_width, out_height, color_mode);
 
 			error = check_horiz_timing_omap3(channel, pos_x,
 				in_width, in_height, out_width, out_height);
@@ -1889,11 +1889,11 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
 					in_height < out_height * 2)
 					*five_taps = false;
 			if (!*five_taps)
-				fclk = calc_fclk(channel, in_width, in_height,
-					out_width, out_height);
+				core_clk = calc_core_clk(channel, in_width,
+					in_height, out_width, out_height);
 			error = (error || in_width > maxsinglelinewidth * 2 ||
 				(in_width > maxsinglelinewidth && *five_taps) ||
-				!fclk || fclk > dispc_fclk_rate());
+				!core_clk || core_clk > dispc_core_clk_rate());
 			if (error) {
 				if (decim_x = decim_y) {
 					decim_x = min_factor;
@@ -1926,7 +1926,7 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
 	} else {
 		int decim_x_min = decim_x;
 		in_height = DIV_ROUND_UP(height, decim_y);
-		in_width_max = dispc_fclk_rate() /
+		in_width_max = dispc_core_clk_rate() /
 				DIV_ROUND_UP(dispc_mgr_pclk_rate(channel),
 						out_width);
 		decim_x = DIV_ROUND_UP(width, in_width_max);
@@ -1945,18 +1945,18 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
 			return -EINVAL;
 		}
 
-		fclk = calc_fclk(channel, in_width, in_height, out_width,
-				out_height);
+		core_clk = calc_core_clk(channel, in_width, in_height,
+				out_width, out_height);
 	}
 
-	DSSDBG("required fclk rate = %lu Hz\n", fclk);
-	DSSDBG("current fclk rate = %lu Hz\n", dispc_fclk_rate());
+	DSSDBG("required core clk rate = %lu Hz\n", core_clk);
+	DSSDBG("current core clk rate = %lu Hz\n", dispc_core_clk_rate());
 
-	if (!fclk || fclk > dispc_fclk_rate()) {
+	if (!core_clk || core_clk > dispc_core_clk_rate()) {
 		DSSERR("failed to set up scaling, "
-			"required fclk rate = %lu Hz, "
-			"current fclk rate = %lu Hz\n",
-			fclk, dispc_fclk_rate());
+			"required core clk rate = %lu Hz, "
+			"current core clk rate = %lu Hz\n",
+			core_clk, dispc_core_clk_rate());
 		return -EINVAL;
 	}
 
@@ -2646,6 +2646,19 @@ unsigned long dispc_mgr_pclk_rate(enum omap_channel channel)
 	}
 }
 
+unsigned long dispc_core_clk_rate(void)
+{
+	int lcd;
+	unsigned long fclk = dispc_fclk_rate();
+
+	if (dss_has_feature(FEAT_CORE_CLK_DIV))
+		lcd = REG_GET(DISPC_DIVISOR, 23, 16);
+	else
+		lcd = REG_GET(DISPC_DIVISORo(OMAP_DSS_CHANNEL_LCD), 23, 16);
+
+	return fclk / lcd;
+}
+
 void dispc_dump_clocks(struct seq_file *s)
 {
 	int lcd, pcd;
diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
index d4b3dff..1bde34c 100644
--- a/drivers/video/omap2/dss/dss.h
+++ b/drivers/video/omap2/dss/dss.h
@@ -451,6 +451,7 @@ void dispc_mgr_set_pol_freq(enum omap_channel channel,
 		enum omap_panel_config config, u8 acbi, u8 acb);
 unsigned long dispc_mgr_lclk_rate(enum omap_channel channel);
 unsigned long dispc_mgr_pclk_rate(enum omap_channel channel);
+unsigned long dispc_core_clk_rate(void);
 int dispc_mgr_set_clock_div(enum omap_channel channel,
 		struct dispc_clock_info *cinfo);
 int dispc_mgr_get_clock_div(enum omap_channel channel,
-- 
1.7.1

Re: [PATCH V3 3/3] OMAPDSS: DISPC: Correct DISPC functional clock usage

[Tomi Valkeinen]

[-- Attachment #1: Type: text/plain, Size: 371 bytes --]

On Mon, 2012-04-23 at 12:16 +0530, Chandrabhanu Mahapatra wrote:
> DISPC_FCLK is incorrectly used as functional clock of DISPC in scaling
> calculations. So, DISPC_CORE_CLK replaces as functional clock of DISPC.
> DISPC_CORE_CLK is derived from DISPC_FCLK divided by an independent DISPC
> divisor LCD.

Thanks, I've applied the series to "dev" branch.

 Tomi


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]