On Tue, Mar 27, 2012 at 4:27 PM, Tomi Valkeinen <tomi.valkeinen@xxxxxx> wrote:
> 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@xxxxxxxxx> in Linux OMAP kernel. Ville
>> Syrjälä <ville.syrjala@xxxxxxxxx> had added checks for video overlay horizontal
>> timing and DISPC horizontal blanking length limitations.
>>
>> Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@xxxxxx>
>> ---
>> 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.
>
Yes, you are right! Do you mean to include some comments within the
code definition itself? The patch description includes all that this
function does.
>> +{
>> + 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
>
I am thinking to to move this fix to the third patch in the series
"OMAPDSS: DISPC: Correct DISPC functional clock usage" and make it
separate patch from the series and if possible move it ahead of the
predecimation patch series. What do you say?
--
Chandrabhanu Mahapatra
Texas Instruments India Pvt. Ltd.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
