Fix for dispc's error "omapfb omapfb: irq error status 4020"

PATCH Rick Bronson 6 participants 7 messages Nov 7, 2008 – Jan 9, 2009 Framebuffer layer

Share & tools

Rick Bronson

author

2008-11-07 20:55 UTC #0 lore raw

Folks,

Please take a look at this change to drivers/video/omap/dispc.c. It
addresses a problem seen on some boots of OMAP's. On about 1 in 30 boots one gets an endless stream of interrupts from the DISPC_IRQ_SYNC_LOST bit in the DISPC_IRQSTATUS register. The following messages are printed.

omapfb omapfb: irq error status 4020

The correct solution to this problem is to perform a "soft reset"
but that requires a bit of re-architecturing of the code as the init routine would have to be split up, etc. See the OMAP pdf, search for "To clear a synchronization lost interrupt".

This patch allows the above error message to get printed once and
then disables further DISPC_IRQ_SYNC_LOST interrups.

Comments?

Thanks,

Rick

drivers/video/omap/dispc.c renamed from drivers/video/omap/dispc.c.~1~ +15 -8

@@ -157,7 +157,7 @@ struct resmap {
 
 #define MAX_IRQ_HANDLERS            4
 
-static struct {
+static struct omapfb_dispc {
 	void __iomem	*base;
 
 	struct omapfb_mem_desc	mem_desc;

@@ -169,6 +169,7 @@ static struct {
 
 	int		ext_mode;
 
+	u32	irq_error_mask;  /* mask used for errors */
 	struct {
 		u32	irq_mask;
 		void	(*callback)(void *);

@@ -812,16 +813,16 @@ static void set_lcd_timings(void)
 	panel->pixel_clock = fck / lck_div / pck_div / 1000;
 }
 
-static void recalc_irq_mask(void)
+static void recalc_irq_mask(struct omapfb_dispc *p_dispc)
 {
 	int i;
-	unsigned long irq_mask = DISPC_IRQ_MASK_ERROR;
+	unsigned long irq_mask = p_dispc->irq_error_mask;
 
 	for (i = 0; i < MAX_IRQ_HANDLERS; i++) {
-		if (!dispc.irq_handlers[i].callback)
+		if (!p_dispc->irq_handlers[i].callback)
 			continue;
 
-		irq_mask |= dispc.irq_handlers[i].irq_mask;
+		irq_mask |= p_dispc->irq_handlers[i].irq_mask;
 	}
 
 	enable_lcd_clocks(1);

@@ -843,7 +844,7 @@ int omap_dispc_request_irq(unsigned long
 		dispc.irq_handlers[i].irq_mask = irq_mask;
 		dispc.irq_handlers[i].callback = callback;
 		dispc.irq_handlers[i].data = data;
-		recalc_irq_mask();
+		recalc_irq_mask(&dispc);
 
 		return 0;
 	}

@@ -863,7 +864,7 @@ void omap_dispc_free_irq(unsigned long i
 			dispc.irq_handlers[i].irq_mask = 0;
 			dispc.irq_handlers[i].callback = NULL;
 			dispc.irq_handlers[i].data = NULL;
-			recalc_irq_mask();
+			recalc_irq_mask(&dispc);
 			return;
 		}
 	}

@@ -884,6 +885,10 @@ static irqreturn_t omap_dispc_irq_handle
 		complete(&dispc.frame_done);
 
 	if (stat & DISPC_IRQ_MASK_ERROR) {
+		if (stat & DISPC_IRQ_SYNC_LOST) {  /* only allow sync lost once or we end up... */
+			dispc.irq_error_mask &= ~DISPC_IRQ_SYNC_LOST;  /* with a barrage of interrupts */
+			recalc_irq_mask(&dispc);
+			}
 		if (printk_ratelimit()) {
 			dev_err(dispc.fbdev->dev, "irq error status %04x\n",
 				stat & 0x7fff);

@@ -897,6 +902,7 @@ static irqreturn_t omap_dispc_irq_handle
 	}
 
 	dispc_write_reg(DISPC_IRQSTATUS, stat);
+	stat = dispc_read_reg(DISPC_IRQSTATUS);
 
 	enable_lcd_clocks(0);

@@ -1432,7 +1438,8 @@ static int omap_dispc_init(struct omapfb
 	l = dispc_read_reg(DISPC_IRQSTATUS);
 	dispc_write_reg(DISPC_IRQSTATUS, l);
 
-	recalc_irq_mask();
+	dispc.irq_error_mask = DISPC_IRQ_MASK_ERROR;  /* init error mask */
+	recalc_irq_mask(&dispc);
 
 	if ((r = request_irq(INT_24XX_DSS_IRQ, omap_dispc_irq_handler,
 			   0, MODULE_NAME, fbdev)) < 0) {

Koen Kooi

2008-11-07 21:38 UTC #1 lore raw

Op 7 nov 2008, om 21:55 heeft Rick Bronson het volgende geschreven:

Quoted 15 lines

Folks,
Please take a look at this change to drivers/video/omap/dispc.c. It
addresses a problem seen on some boots of OMAP's. On about 1 in 30 boots one gets an endless stream of interrupts from the DISPC_IRQ_SYNC_LOST bit in the DISPC_IRQSTATUS register. The following messages are printed.
omapfb omapfb: irq error status 4020
The correct solution to this problem is to perform a "soft reset"
but that requires a bit of re-architecturing of the code as the init routine would have to be split up, etc. See the OMAP pdf, search for "To clear a synchronization lost interrupt".

Quoted 4 lines

This patch allows the above error message to get printed once and
then disables further DISPC_IRQ_SYNC_LOST interrups.
Comments?

I can trigger this with all boots using Tomi's DSS patches on a rev C prototype beagle (ES3.0 3530), so those probably need a similar patch :)

regards,

Koen

Quoted 103 lines

Thanks,

Rick

--- linux-omap-2.6/drivers/video/omap/dispc.c.~1~ 2008-11-04
14:23:38.000000000 -0800
+++ linux-omap-2.6/drivers/video/omap/dispc.c 2008-11-07
12:34:54.000000000 -0800

drivers/video/omap/dispc.c renamed from drivers/video/omap/dispc.c.~1~ +9 -7

@@ -157,7 +157,7 @@ struct resmap {
 
 #define MAX_IRQ_HANDLERS            4
 
-static struct {
+static struct omapfb_dispc {
 	void __iomem	*base;
 
 	struct omapfb_mem_desc	mem_desc;

@@ -169,6 +169,7 @@ static struct {
 
 	int		ext_mode;
 
+	u32	irq_error_mask;  /* mask used for errors */
 	struct {
 		u32	irq_mask;
 		void	(*callback)(void *);

@@ -812,16 +813,16 @@ static void set_lcd_timings(void)
 	panel->pixel_clock = fck / lck_div / pck_div / 1000;
 }
 
-static void recalc_irq_mask(void)
+static void recalc_irq_mask(struct omapfb_dispc *p_dispc)
 {
 	int i;
-	unsigned long irq_mask = DISPC_IRQ_MASK_ERROR;
+	unsigned long irq_mask = p_dispc->irq_error_mask;
 
 	for (i = 0; i < MAX_IRQ_HANDLERS; i++) {
-		if (!dispc.irq_handlers[i].callback)
+		if (!p_dispc->irq_handlers[i].callback)
 			continue;
 
-		irq_mask |= dispc.irq_handlers[i].irq_mask;
+		irq_mask |= p_dispc->irq_handlers[i].irq_mask;
 	}
 
 	enable_lcd_clocks(1);

@@ -843,7 +844,7 @@ int omap_dispc_request_irq(unsigned long
 		dispc.irq_handlers[i].irq_mask = irq_mask;
 		dispc.irq_handlers[i].callback = callback;
 		dispc.irq_handlers[i].data = data;
-		recalc_irq_mask();
+		recalc_irq_mask(&dispc);
 
 		return 0;
 	}

@@ -863,7 +864,7 @@ void omap_dispc_free_irq(unsigned long i
 			dispc.irq_handlers[i].irq_mask = 0;
 			dispc.irq_handlers[i].callback = NULL;
 			dispc.irq_handlers[i].data = NULL;
-			recalc_irq_mask();
+			recalc_irq_mask(&dispc);
 			return;
 		}
 	}

@@ -884,6 +885,10 @@ static irqreturn_t omap_dispc_irq_handle
 		complete(&dispc.frame_done);
 
 	if (stat & DISPC_IRQ_MASK_ERROR) {
+		if (stat & DISPC_IRQ_SYNC_LOST) {  /* only allow sync lost once

or we end up... */

(quoted diff) +1 -0

+			dispc.irq_error_mask &= ~DISPC_IRQ_SYNC_LOST;  /* with a barrage

of interrupts */

drivers/video/omap/dispc.c renamed from drivers/video/omap/dispc.c.~1~ +2 -0

+			recalc_irq_mask(&dispc);
+			}

		if (printk_ratelimit()) {
			dev_err(dispc.fbdev->dev, "irq error status %04x\n",
				stat & 0x7fff);

drivers/video/omap/dispc.c renamed from drivers/video/omap/dispc.c.~1~ +3 -2

@@ -897,6 +902,7 @@ static irqreturn_t omap_dispc_irq_handle
 	}
 
 	dispc_write_reg(DISPC_IRQSTATUS, stat);
+	stat = dispc_read_reg(DISPC_IRQSTATUS);
 
 	enable_lcd_clocks(0);

@@ -1432,7 +1438,8 @@ static int omap_dispc_init(struct omapfb
 	l = dispc_read_reg(DISPC_IRQSTATUS);
 	dispc_write_reg(DISPC_IRQSTATUS, l);
 
-	recalc_irq_mask();
+	dispc.irq_error_mask = DISPC_IRQ_MASK_ERROR;  /* init error mask */
+	recalc_irq_mask(&dispc);
 
 	if ((r = request_irq(INT_24XX_DSS_IRQ, omap_dispc_irq_handler,
 			   0, MODULE_NAME, fbdev)) < 0) {
 
--

To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Tomi Valkeinen

2008-11-10 08:31 UTC #2 lore raw

In reply to Rick Bronson

Hi,

Quoted 15 lines · ext Rick Bronson

Folks,
Please take a look at this change to drivers/video/omap/dispc.c. It
addresses a problem seen on some boots of OMAP's. On about 1 in 30 boots one gets an endless stream of interrupts from the DISPC_IRQ_SYNC_LOST bit in the DISPC_IRQSTATUS register. The following messages are printed.
omapfb omapfb: irq error status 4020
The correct solution to this problem is to perform a "soft reset"
but that requires a bit of re-architecturing of the code as the init routine would have to be split up, etc. See the OMAP pdf, search for "To clear a synchronization lost interrupt".

Quoted 4 lines

This patch allows the above error message to get printed once and
then disables further DISPC_IRQ_SYNC_LOST interrups.
Comments?

I think the correct solution would be to find out why we get sync lost errors and fix that. Your patch just hides the problem.

However, I agree that it's not good to just keep spamming the error, possibly making the board freeze. The new DSS turns the display off after 100 error messages. But I don't think just hiding the interrupt is a good change.

Quoted 4 lines

Thanks,
Rick

Tomi

Quoted 93 lines

drivers/video/omap/dispc.c renamed from drivers/video/omap/dispc.c.~1~ +15 -9

@@ -157,7 +157,7 @@ struct resmap {
 
 #define MAX_IRQ_HANDLERS            4
 
-static struct {
+static struct omapfb_dispc {
 	void __iomem	*base;
 
 	struct omapfb_mem_desc	mem_desc;

@@ -169,6 +169,7 @@ static struct {
 
 	int		ext_mode;
 
+	u32	irq_error_mask;  /* mask used for errors */
 	struct {
 		u32	irq_mask;
 		void	(*callback)(void *);

@@ -812,16 +813,16 @@ static void set_lcd_timings(void)
 	panel->pixel_clock = fck / lck_div / pck_div / 1000;
 }
 
-static void recalc_irq_mask(void)
+static void recalc_irq_mask(struct omapfb_dispc *p_dispc)
 {
 	int i;
-	unsigned long irq_mask = DISPC_IRQ_MASK_ERROR;
+	unsigned long irq_mask = p_dispc->irq_error_mask;
 
 	for (i = 0; i < MAX_IRQ_HANDLERS; i++) {
-		if (!dispc.irq_handlers[i].callback)
+		if (!p_dispc->irq_handlers[i].callback)
 			continue;
 
-		irq_mask |= dispc.irq_handlers[i].irq_mask;
+		irq_mask |= p_dispc->irq_handlers[i].irq_mask;
 	}
 
 	enable_lcd_clocks(1);

@@ -843,7 +844,7 @@ int omap_dispc_request_irq(unsigned long
 		dispc.irq_handlers[i].irq_mask = irq_mask;
 		dispc.irq_handlers[i].callback = callback;
 		dispc.irq_handlers[i].data = data;
-		recalc_irq_mask();
+		recalc_irq_mask(&dispc);
 
 		return 0;
 	}

@@ -863,7 +864,7 @@ void omap_dispc_free_irq(unsigned long i
 			dispc.irq_handlers[i].irq_mask = 0;
 			dispc.irq_handlers[i].callback = NULL;
 			dispc.irq_handlers[i].data = NULL;
-			recalc_irq_mask();
+			recalc_irq_mask(&dispc);
 			return;
 		}
 	}

@@ -884,6 +885,10 @@ static irqreturn_t omap_dispc_irq_handle
 		complete(&dispc.frame_done);
 
 	if (stat & DISPC_IRQ_MASK_ERROR) {
+		if (stat & DISPC_IRQ_SYNC_LOST) {  /* only allow sync lost once or we end up... */
+			dispc.irq_error_mask &= ~DISPC_IRQ_SYNC_LOST;  /* with a barrage of interrupts */
+			recalc_irq_mask(&dispc);
+			}
 		if (printk_ratelimit()) {
 			dev_err(dispc.fbdev->dev, "irq error status %04x\n",
 				stat & 0x7fff);

@@ -897,6 +902,7 @@ static irqreturn_t omap_dispc_irq_handle
 	}
 
 	dispc_write_reg(DISPC_IRQSTATUS, stat);
+	stat = dispc_read_reg(DISPC_IRQSTATUS);
 
 	enable_lcd_clocks(0);

@@ -1432,7 +1438,8 @@ static int omap_dispc_init(struct omapfb
 	l = dispc_read_reg(DISPC_IRQSTATUS);
 	dispc_write_reg(DISPC_IRQSTATUS, l);
 
-	recalc_irq_mask();
+	dispc.irq_error_mask = DISPC_IRQ_MASK_ERROR;  /* init error mask */
+	recalc_irq_mask(&dispc);
 
 	if ((r = request_irq(INT_24XX_DSS_IRQ, omap_dispc_irq_handler,
 			   0, MODULE_NAME, fbdev)) < 0) {
 
--

To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Hiremath, Vaibhav

2008-11-10 08:40 UTC #3 lore raw

Thanks,
Vaibhav Hiremath

Quoted 43 lines

-----Original Message-----
From: [email protected] [mailto:linux-omap- [email protected]] On Behalf Of Tomi Valkeinen
Sent: Monday, November 10, 2008 2:01 PM
To: [email protected]
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: Fix for dispc's error "omapfb omapfb: irq error status 4020"
Hi,
Quoted 3 lines · ext Rick Bronson
Folks,
Please take a look at this change to drivers/video/omap/dispc.c.
It
Quoted 1 line
addresses a problem seen on some boots of OMAP's. On about 1 in
30
Quoted 7 lines
boots one gets an endless stream of interrupts from the DISPC_IRQ_SYNC_LOST bit in the DISPC_IRQSTATUS register. The following messages are printed.
omapfb omapfb: irq error status 4020
The correct solution to this problem is to perform a "soft
reset"
Quoted 1 line
but that requires a bit of re-architecturing of the code as the
init
Quoted 1 line
routine would have to be split up, etc. See the OMAP pdf, search
for
Quoted 3 lines
"To clear a synchronization lost interrupt".
This patch allows the above error message to get printed once
and
Quoted 3 lines
then disables further DISPC_IRQ_SYNC_LOST interrups.
Comments?
I think the correct solution would be to find out why we get sync lost
errors and fix that. Your patch just hides the problem.

Quoted 6 lines

However, I agree that it's not good to just keep spamming the error, possibly making the board freeze. The new DSS turns the display off after 100 error messages. But I don't think just hiding the interrupt is
a good change.

[Hiremath, Vaibhav] Yes, it is not a good practice to hide any interrupts. But I think even instead of turning of the DSS, you should reset the DSS and reconfigure it with default values or preserve the state before resetting and configure it again. I agree to the point that, it requires lot of re-architectureing to the current driver. But I believe it should be possible with new omap2 version.

Quoted 114 lines

Quoted 4 lines
Thanks,
Rick
Tomi
Quoted 2 lines
--- linux-omap-2.6/drivers/video/omap/dispc.c.~1~ 2008-11-04
14:23:38.000000000 -0800
Quoted 1 line
+++ linux-omap-2.6/drivers/video/omap/dispc.c 2008-11-07
12:34:54.000000000 -0800
Quoted 57 lines
drivers/video/omap/dispc.c renamed from drivers/video/omap/dispc.c.~1~ +8 -7
@@ -157,7 +157,7 @@ struct resmap {
 
 #define MAX_IRQ_HANDLERS            4
 
-static struct {
+static struct omapfb_dispc {
 	void __iomem	*base;
 
 	struct omapfb_mem_desc	mem_desc;
@@ -169,6 +169,7 @@ static struct {
 
 	int		ext_mode;
 
+	u32	irq_error_mask;  /* mask used for errors */
 	struct {
 		u32	irq_mask;
 		void	(*callback)(void *);
@@ -812,16 +813,16 @@ static void set_lcd_timings(void)
 	panel->pixel_clock = fck / lck_div / pck_div / 1000;
 }
 
-static void recalc_irq_mask(void)
+static void recalc_irq_mask(struct omapfb_dispc *p_dispc)
 {
 	int i;
-	unsigned long irq_mask = DISPC_IRQ_MASK_ERROR;
+	unsigned long irq_mask = p_dispc->irq_error_mask;
 
 	for (i = 0; i < MAX_IRQ_HANDLERS; i++) {
-		if (!dispc.irq_handlers[i].callback)
+		if (!p_dispc->irq_handlers[i].callback)
 			continue;
 
-		irq_mask |= dispc.irq_handlers[i].irq_mask;
+		irq_mask |= p_dispc->irq_handlers[i].irq_mask;
 	}
 
 	enable_lcd_clocks(1);
@@ -843,7 +844,7 @@ int omap_dispc_request_irq(unsigned long
 		dispc.irq_handlers[i].irq_mask = irq_mask;
 		dispc.irq_handlers[i].callback = callback;
 		dispc.irq_handlers[i].data = data;
-		recalc_irq_mask();
+		recalc_irq_mask(&dispc);
 
 		return 0;
 	}
@@ -863,7 +864,7 @@ void omap_dispc_free_irq(unsigned long i
 			dispc.irq_handlers[i].irq_mask = 0;
 			dispc.irq_handlers[i].callback = NULL;
 			dispc.irq_handlers[i].data = NULL;
-			recalc_irq_mask();
+			recalc_irq_mask(&dispc);
 			return;
 		}
 	}
drivers/video/omap/dispc.c static irqreturn_t omap_dispc_irq_handle · line 888
Quoted 4 lines
drivers/video/omap/dispc.c renamed from drivers/video/omap/dispc.c.~1~ +1 -0
@@ -884,6 +885,10 @@ static irqreturn_t omap_dispc_irq_handle
 		complete(&dispc.frame_done);
 
 	if (stat & DISPC_IRQ_MASK_ERROR) {
+		if (stat & DISPC_IRQ_SYNC_LOST) {  /* only allow sync
lost once or we end up... */
Review static irqreturn_t omap_dispc_irq_handle · line 889
Quoted 1 line
(quoted diff) +1 -0
+			dispc.irq_error_mask &= ~DISPC_IRQ_SYNC_LOST;  /*
with a barrage of interrupts */
drivers/video/omap/dispc.c static irqreturn_t omap_dispc_irq_handle
Quoted 4 lines
drivers/video/omap/dispc.c renamed from drivers/video/omap/dispc.c.~1~ +2 -0
+			recalc_irq_mask(&dispc);
+			}
 		if (printk_ratelimit()) {
 			dev_err(dispc.fbdev->dev, "irq error status
%04x\n",
Quoted 10 lines
drivers/video/omap/dispc.c renamed from drivers/video/omap/dispc.c.~1~ +1 -0
 				stat & 0x7fff);
@@ -897,6 +902,7 @@ static irqreturn_t omap_dispc_irq_handle
 	}
 
 	dispc_write_reg(DISPC_IRQSTATUS, stat);
+	stat = dispc_read_reg(DISPC_IRQSTATUS);
 
 	enable_lcd_clocks(0);
 
drivers/video/omap/dispc.c static int omap_dispc_init(struct omapfb · line 1441
Quoted 5 lines
drivers/video/omap/dispc.c renamed from drivers/video/omap/dispc.c.~1~ +1 -1
@@ -1432,7 +1438,8 @@ static int omap_dispc_init(struct omapfb
 	l = dispc_read_reg(DISPC_IRQSTATUS);
 	dispc_write_reg(DISPC_IRQSTATUS, l);
 
-	recalc_irq_mask();
+	dispc.irq_error_mask = DISPC_IRQ_MASK_ERROR;  /* init error
mask */
Quoted 7 lines
drivers/video/omap/dispc.c renamed from drivers/video/omap/dispc.c.~1~ +1 -1
+	recalc_irq_mask(&dispc);
 
 	if ((r = request_irq(INT_24XX_DSS_IRQ, omap_dispc_irq_handler,
 			   0, MODULE_NAME, fbdev)) < 0) {
 
--
To unsubscribe from this list: send the line "unsubscribe linux-
omap" in
Quoted 2 lines
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux- omap" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Tomi Valkeinen

2008-11-13 14:06 UTC #4 lore raw

In reply to Rick Bronson

Hi,

Quoted 9 lines · ext Rick Bronson

Folks,
Please take a look at this change to drivers/video/omap/dispc.c. It
addresses a problem seen on some boots of OMAP's. On about 1 in 30 boots one gets an endless stream of interrupts from the DISPC_IRQ_SYNC_LOST bit in the DISPC_IRQSTATUS register. The following messages are printed.
omapfb omapfb: irq error status 4020

Can you try the patch below if it fixes this problem for you? On my new display subsystem adding a sleep between enabling clocks and doing the soft reset removed the problem.

I tried a bit with different sleep times. With 1ms sleep I still got sync losts. With 10ms I didn't, but I went safe and put 40ms.

Tomi

drivers/video/omap/dispc.c +5 -0

index c140c21..bdfda0c 100644
@@ -24,6 +24,7 @@
 #include <linux/vmalloc.h>
 #include <linux/clk.h>
 #include <linux/io.h>
+#include <linux/delay.h>
 
 #include <mach/sram.h>
 #include <mach/omapfb.h>

@@ -1402,6 +1403,10 @@ static int omap_dispc_init(struct omapfb_device *fbdev, int ext_mode,
                /* Reset monitoring works only w/ the 54M clk */
                enable_digit_clocks(1);
 
+               /* We need to wait here a bit, otherwise we sometimes start to get
+                * synclost errors. */
+               msleep(40);
+
                /* Soft reset */
                MOD_REG_FLD(DISPC_SYSCONFIG, 1 << 1, 1 << 1);

Tony Lindgren

2008-11-26 17:32 UTC #5 lore raw

Quoted 11 lines · Tomi Valkeinen

Hi,
Quoted 8 lines · ext Rick Bronson
Folks,
Please take a look at this change to drivers/video/omap/dispc.c. It
addresses a problem seen on some boots of OMAP's. On about 1 in 30 boots one gets an endless stream of interrupts from the DISPC_IRQ_SYNC_LOST bit in the DISPC_IRQSTATUS register. The following messages are printed.

Quoted 8 lines

Quoted 1 line
omapfb omapfb: irq error status 4020
Can you try the patch below if it fixes this problem for you? On my new display subsystem adding a sleep between enabling clocks and doing the soft reset removed the problem.
I tried a bit with different sleep times. With 1ms sleep I still got sync losts. With 10ms I didn't, but I went safe and put 40ms.

Here's my canned fbdev reply for this tread too:

Guys, please take this discussion to fbdev mailing list and cc linux-omap list. I'm not going to push any more omap fbdev patches, so please send the patches against mainline kernel to fbdev list.

Regards,

Tony

Quoted 33 lines

Tomi

drivers/video/omap/dispc.c +5 -1

index c140c21..bdfda0c 100644
@@ -24,6 +24,7 @@
 #include <linux/vmalloc.h>
 #include <linux/clk.h>
 #include <linux/io.h>
+#include <linux/delay.h>
 
 #include <mach/sram.h>
 #include <mach/omapfb.h>

@@ -1402,6 +1403,10 @@ static int omap_dispc_init(struct omapfb_device *fbdev, int ext_mode,
                /* Reset monitoring works only w/ the 54M clk */
                enable_digit_clocks(1);
 
+               /* We need to wait here a bit, otherwise we sometimes start to get
+                * synclost errors. */
+               msleep(40);
+
                /* Soft reset */
                MOD_REG_FLD(DISPC_SYSCONFIG, 1 << 1, 1 << 1);
 
 
--

To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Grazvydas Ignotas

2009-01-09 21:57 UTC #6 lore raw

In reply to Tomi Valkeinen

Quoted 11 lines · Tomi Valkeinen

Hi,
Quoted 8 lines · ext Rick Bronson
Folks,
Please take a look at this change to drivers/video/omap/dispc.c. It
addresses a problem seen on some boots of OMAP's. On about 1 in 30 boots one gets an endless stream of interrupts from the DISPC_IRQ_SYNC_LOST bit in the DISPC_IRQSTATUS register. The following messages are printed.

Quoted 8 lines

Quoted 1 line
omapfb omapfb: irq error status 4020
Can you try the patch below if it fixes this problem for you? On my new display subsystem adding a sleep between enabling clocks and doing the soft reset removed the problem.
I tried a bit with different sleep times. With 1ms sleep I still got sync losts. With 10ms I didn't, but I went safe and put 40ms.

Unfortunately sleeping there doesn't help for pandora. I've tried 10ms, 40ms, 400ms (this seems to be used by DSS2 now).

However because we are also setting up the display in u-boot for pandora, I can hook up irq handler early and wait for frame done irq before resetting DSS (patch attached). With this I've done ~50 reboots now and hadn't got any sync lost interrupts yet.

In case display is not enabled in bootloader waiting for interrupt just times out there, so no harm made. This probably could substitute that sleep, so maybe this solution could be considered for DSS2.

drivers/video/omap/dispc.c +22 ~~-11~~

index c140c21..e73d04a 100644
@@ -1398,10 +1398,31 @@ static int omap_dispc_init(struct omapfb_device *fbdev, int ext_mode,
 	}
 #endif
 
+	l = dispc_read_reg(DISPC_IRQSTATUS);
+	dispc_write_reg(DISPC_IRQSTATUS, l);
+
+	recalc_irq_mask();
+
+	if ((r = request_irq(INT_24XX_DSS_IRQ, omap_dispc_irq_handler,
+			   0, MODULE_NAME, fbdev)) < 0) {
+		dev_err(dispc.fbdev->dev, "can't get DSS IRQ\n");
+		goto fail1;
+	}
+
 	if (!skip_init) {
 		/* Reset monitoring works only w/ the 54M clk */
 		enable_digit_clocks(1);
 
+		/* We have to wait here to avoid occasional SYNC LOST errors */
+		MOD_REG_FLD(DISPC_IRQENABLE, 1, 1);
+		omap_dispc_enable_lcd_out(0);	/* if bootloader enabled it */
+		if (!wait_for_completion_timeout(&dispc.frame_done,
+				msecs_to_jiffies(400))) {
+			dev_err(dispc.fbdev->dev,
+				"timeout waiting for FRAME DONE\n");
+		}
+		MOD_REG_FLD(DISPC_IRQENABLE, 1, 0);
+
 		/* Soft reset */
 		MOD_REG_FLD(DISPC_SYSCONFIG, 1 << 1, 1 << 1);

@@ -1410,7 +1431,7 @@ static int omap_dispc_init(struct omapfb_device *fbdev, int ext_mode,
 				dev_err(dispc.fbdev->dev, "soft reset failed\n");
 				r = -ENODEV;
 				enable_digit_clocks(0);
-				goto fail1;
+				goto fail2;
 			}
 		}

@@ -1429,16 +1450,6 @@ static int omap_dispc_init(struct omapfb_device *fbdev, int ext_mode,
 	l |= 1 << 9;
 	dispc_write_reg(DISPC_CONFIG, l);
 
-	l = dispc_read_reg(DISPC_IRQSTATUS);
-	dispc_write_reg(DISPC_IRQSTATUS, l);
-
-	recalc_irq_mask();
-
-	if ((r = request_irq(INT_24XX_DSS_IRQ, omap_dispc_irq_handler,
-			   0, MODULE_NAME, fbdev)) < 0) {
-		dev_err(dispc.fbdev->dev, "can't get DSS IRQ\n");
-		goto fail1;
-	}
 
 	/* L3 firewall setting: enable access to OCM RAM */
 	__raw_writel(0x402000b0, IO_ADDRESS(0x680050a0));