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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux