On Tue, 2012-03-27 at 16:37 +0530, Mahapatra, Chandrabhanu wrote:

On Tue, Mar 27, 2012 at 4:27 PM, Tomi Valkeinen [off-list ref] wrote:

quoted

On Wed, 2012-03-21 at 15:21 +0530, Chandrabhanu Mahapatra wrote:

quoted

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ä [off-list ref] in Linux OMAP kernel. Ville
Syrjälä [off-list ref] had added checks for video overlay horizontal
timing and DISPC horizontal blanking length limitations.

Signed-off-by: Chandrabhanu Mahapatra <redacted>
---
 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. 

quoted

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