On Tue, Mar 27, 2012 at 4:47 PM, Tomi Valkeinen <tomi.valkei...@ti.com> wrote: > On Tue, 2012-03-27 at 16:37 +0530, Mahapatra, Chandrabhanu wrote: >> On Tue, Mar 27, 2012 at 4:27 PM, Tomi Valkeinen <tomi.valkei...@ti.com> >> 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.syrj...@nokia.com> in Linux OMAP kernel. >> >> Ville >> >> Syrjälä <ville.syrj...@nokia.com> had added checks for video overlay >> >> horizontal >> >> timing and DISPC horizontal blanking length limitations. >> >> >> >> Signed-off-by: Chandrabhanu Mahapatra <cmahapa...@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. >> > >> >> 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. > > I mean just a short comment above the function that mentions that this > function is used to avoid the synclosts on omap3, because yadda yadda, > so that the reader will immediately realize that this is doing some > special handling. >
Ok. >> > 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? > > This patch is using dispc_mgr_lclk_rate. Does it depend on the change? > If so, it needs to be fixed before this patch. But yes, separating it > sounds fine. > > Tomi > To a certain extent it does. Moving it to the third patch will introduce certain code changes as if (dispc_mgr_is_lcd(channel)) while initialising dispc_core_clk. Anyways that can be cleaned up in the third patch. -- 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
