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

[Tomi Valkeinen <tomi.valkeinen@ti.com>]

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

On Wed, 2012-03-21 at 15:21 +0530, Chandrabhanu Mahapatra wrote:
> 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 |   97 +++++++++++++++++++++++++++++----------
>  1 files changed, 72 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
> index 5a1963e..d8a1672 100644
> --- a/drivers/video/omap2/dss/dispc.c
> +++ b/drivers/video/omap2/dss/dispc.c
> @@ -1622,6 +1622,57 @@ static void calc_dma_rotation_offset(u8 rotation, bool mirror,
>  	}
>  }
>  
> +static int check_horiz_timing(enum omap_channel channel, u16 pos_x,
> +		u16 width, u16 height, u16 out_width, u16 out_height)

I think the function could be named check_horiz_timing_omap3 or
something. It's kinda hard to realize that this is an omap3 work-around.
Perhaps a short comment above the function would be in order.

> +{
> +	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);
> +	lclk = dispc_mgr_lclk_rate(channel);
> +
> +	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)
> @@ -1702,7 +1753,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);
> @@ -1778,6 +1829,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(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)
> @@ -1785,7 +1839,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) {
> @@ -1801,6 +1855,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(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");
> @@ -1901,7 +1961,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;
>  
> @@ -2472,32 +2532,19 @@ unsigned long dispc_fclk_rate(void)
>  
>  unsigned long dispc_mgr_lclk_rate(enum omap_channel channel)
>  {
> -	struct platform_device *dsidev;
> -	int lcd;
> -	unsigned long r;
> -	u32 l;
> +	unsigned long r = dispc_fclk_rate();
>  
> -	l = dispc_read_reg(DISPC_DIVISORo(channel));
> +	if (dispc_mgr_is_lcd(channel)) {
> +		u32 l;
> +		int lcd;
>  
> -	lcd = FLD_GET(l, 23, 16);
> +		l = dispc_read_reg(DISPC_DIVISORo(channel));
> +		lcd = FLD_GET(l, 23, 16);
>  
> -	switch (dss_get_lcd_clk_source(channel)) {
> -	case OMAP_DSS_CLK_SRC_FCK:
> -		r = clk_get_rate(dispc.dss_clk);
> -		break;
> -	case OMAP_DSS_CLK_SRC_DSI_PLL_HSDIV_DISPC:
> -		dsidev = dsi_get_dsidev_from_id(0);
> -		r = dsi_get_pll_hsdiv_dispc_rate(dsidev);
> -		break;
> -	case OMAP_DSS_CLK_SRC_DSI2_PLL_HSDIV_DISPC:
> -		dsidev = dsi_get_dsidev_from_id(1);
> -		r = dsi_get_pll_hsdiv_dispc_rate(dsidev);
> -		break;
> -	default:
> -		BUG();
> +		return r / lcd;
> +	} else {
> +		return r;
>  	}
> -
> -	return r / lcd;
>  }

I don't the change to dispc_mgr_lclk_rate has anything to do with this
patch (fixing omap3 sync lost error). Shouldn't it be a separate fix?

 Tomi


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