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

["Mahapatra, Chandrabhanu" <cmahapatra@ti.com>]

On Fri, Mar 16, 2012 at 4:34 AM, Ville Syrjälä <syrjala@sci.fi> wrote:
> On Thu, Mar 15, 2012 at 05:18:52PM +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 |   67 +++++++++++++++++++++++++++++++++++++--
>>  1 files changed, 64 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
>> index 5a1963e..ebfa613 100644
>> --- a/drivers/video/omap2/dss/dispc.c
>> +++ b/drivers/video/omap2/dss/dispc.c
>> @@ -1622,6 +1622,58 @@ 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)
>> +{
>> +     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;
>> +     int pcd = REG_GET(DISPC_DIVISORo(channel), 7, 0);
>
> pcd doesn't exist for TV out, which is the reason why you see
> 'lclk/pclk' ratio used instead in the harmattan kernel.
>
Yes, you are right. It's a very important point which I had missed.

> Another thing which still seems to be wrong in the upstream code is the
> use of fclk in the scaling checks. It should be checking lclk instead.

Thanks for sighting this out. I will implement it in the future.

>> +     unsigned long nonactive, val, blank;
>> +     static const u8 limits[3] = { 8, 10, 20 };
>> +     int i;
>> +
>> +     nonactive = t.x_res + t.hfp + t.hsw + t.hbp - out_width;
>> +
>> +     /*
>> +      * Atleast DS-2 lines must have already been fetched
>> +      * before the display active video period starts.
>> +      */
>> +     val = (nonactive - pos_x) * pcd;
>> +     DSSDBG("(nonactive - pos_x) * pcd = %lu,"
>> +             " max(0, DS - 2) * width = %d\n",
>> +             val, max(0, DS - 2) * width);
>> +     if (val < max(0, DS - 2) * width)
>> +             return -EINVAL;
>> +
>> +     /*
>> +      * Only one line can be fetched during the overlay active
>> +      * period, the rest have to be fetched during the inactive
>> +      * period.
>> +      */
>> +     val = nonactive * pcd;
>> +     DSSDBG("nonactive * pcd  = %lu, max(0, DS - 1) * width = %d\n",
>> +             val, max(0, DS - 1) * width);
>> +     if (val < max(0, DS - 1) * width)
>> +             return -EINVAL;
>> +
>> +     /*
>> +      * Atleast Ceil(DS) lines should have been loaded during
>> +      * PPL (screen width) + blanking period.
>> +      */
>> +     i = 0;
>> +     if (out_height < height)
>> +             i++;
>> +     if (out_width < width)
>> +             i++;
>> +     blank = (t.hbp + t.hsw + t.hfp) * pcd;
>> +     DSSDBG("blanking period + ppl = %lu (limit = %u)\n", blank, limits[i]);
>> +     if (blank <= limits[i])
>> +             return -EINVAL;
>
> The comment and code here are totally out of sync. IIRC the hblank
> length check came directly from the TRM. I have no idea if it's correct
> for more recent OMAPs, or if it was just copy pasted from the
> old TRM when the TRM got adjusted for more recent chips. That is why I
> stuck it into a separate function in harmattan.
>

This patch is only for OMAP3 and this errata is present upto OMAP4 ES1
which we are currently not supporting.

> --
> Ville Syrjälä
> syrjala@sci.fi
> http://www.sci.fi/~syrjala/



-- 
Chandrabhanu Mahapatra
Texas Instruments India Pvt. Ltd.