[PATCH] OMAP1: PM: Fix omapfb/lcd on Amstrad Delta broken when PM set

[PATCH] OMAP1: PM: Fix omapfb/lcd on Amstrad Delta broken when PM set

[Janusz Krzysztofik]

With CONFIG_PM=y, the omapfb/lcd device on Amstrad Delta, after initially
starting correctly, breaks with the following error messages:

omapfb omapfb: resetting (status 0xffffff96,reset count 1)
...
omapfb omapfb: resetting (status 0xffffff96,reset count 100)
omapfb omapfb: too many reset attempts, giving up.

Looking closer at this I have found that it had been broken almost 2 years ago
with commit 2418996e3b100114edb2ae110d5d4acb928909d2, PM fixes for OMAP1.

The definite reason for broken omapfb/lcd_ams_delta in PM mode appeared to be
ARM_IDLECT1:IDLIF_ARM (bit 6) put into idle. The patch below fixes it.

Created and tested against linux-2.6.32-rc5

Signed-off-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>

---
Hi,

I already reported this issue a few months ago, when
drivers/video/omap/lcd_ams_delta.c was proposed to get into mainline - see
http://www.spinics.net/lists/linux-omap/msg14546.html. Now I've found some
time to look at it again.

Since PM area is quite new to me, I am not sure if there may be a better
solution. AFAICS, the standard way to prevent an ARM_CLKCT1 bit being switched
to idle is to enable a clock that uses it (tipb_ck, dma_ck, or tc_ck or one of
its children in this case, right?).

I assume there is no bug in omapfb nor lcdc, as that would be already
detected. Maybe it would be better to fix drivers/video/omap/lcd_ams_delta.c
(or arch/arm/mach-omap1/board-ams-delta), but I don't know what clock should I
enable, if any.

--- linux-2.6.32-rc5/arch/arm/mach-omap1/pm.c.orig	2009-10-16 02:41:50.000000000 +0200
+++ linux-2.6.32-rc5/arch/arm/mach-omap1/pm.c	2009-10-29 22:07:58.000000000 +0100
@@ -45,6 +45,7 @@
 
 #include <asm/irq.h>
 #include <asm/atomic.h>
+#include <asm/mach-types.h>
 #include <asm/mach/time.h>
 #include <asm/mach/irq.h>
 
@@ -139,7 +140,7 @@ void omap1_pm_idle(void)
 	use_idlect1 = omap_dm_timer_modify_idlect_mask(use_idlect1);
 #endif
 
-	if (omap_dma_running())
+	if ((omap_dma_running()) || (machine_is_ams_delta()))
 		use_idlect1 &= ~(1 << 6);
 
 	/* We should be able to remove the do_sleep variable and multiple

Re: [PATCH] OMAP1: PM: Fix omapfb/lcd on Amstrad Delta broken when PM set

[Janusz Krzysztofik]

Thursday 29 October 2009 23:39:44 Janusz Krzysztofik napisał(a):
>
> Since PM area is quite new to me, I am not sure if there may be a better
> solution. AFAICS, the standard way to prevent an ARM_CLKCT1 bit being

s/ARM_CLKCT1/ARM_IDLECT1/

> switched to idle is to enable a clock that uses it (tipb_ck, dma_ck, or
> tc_ck or one of its children in this case, right?).

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

Re: [PATCH] OMAP1: PM: Fix omapfb/lcd on Amstrad Delta broken when PM set

[Janusz Krzysztofik]

Thursday 29 October 2009 23:39:44 Janusz Krzysztofik napisał(a):
> With CONFIG_PM=y, the omapfb/lcd device on Amstrad Delta, after initially
> starting correctly, breaks with the following error messages:
>
> omapfb omapfb: resetting (status 0xffffff96,reset count 1)
> ...
> omapfb omapfb: resetting (status 0xffffff96,reset count 100)
> omapfb omapfb: too many reset attempts, giving up.
>
> Looking closer at this I have found that it had been broken almost 2 years
> ago with commit 2418996e3b100114edb2ae110d5d4acb928909d2, PM fixes for
> OMAP1.
>
> The definite reason for broken omapfb/lcd_ams_delta in PM mode appeared to
> be ARM_IDLECT1:IDLIF_ARM (bit 6) put into idle. The patch below fixes it.
>
> Since PM area is quite new to me, I am not sure if there may be a better
> solution. AFAICS, the standard way to prevent an ARM_CLKCT1 bit being
> switched to idle is to enable a clock that uses it (tipb_ck, dma_ck, or
> tc_ck or one of its children in this case, right?).
>
> I assume there is no bug in omapfb nor lcdc, as that would be already
> detected. Maybe it would be better to fix
> drivers/video/omap/lcd_ams_delta.c (or
> arch/arm/mach-omap1/board-ams-delta), but I don't know what clock should I
> enable, if any.

More looking at it, I found that might be omap_dma_running() from 
arch/arm/plat-omap/dma.c that needs correction. It already checks for LCD dma 
running for OMAP1610, but does nothing similiar for 1510. I have revisited 
http://focus.ti.com/lit/ug/spru674/spru674.pdf, but found no hint how to do 
that in a 1610 similiar way.

Thanks,
Janusz
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] OMAP1: PM: Fix omapfb/lcd on Amstrad Delta broken when PM set

[Tony Lindgren]

* Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> [091030 07:43]:
> Thursday 29 October 2009 23:39:44 Janusz Krzysztofik napisał(a):
> > With CONFIG_PM=y, the omapfb/lcd device on Amstrad Delta, after initially
> > starting correctly, breaks with the following error messages:
> >
> > omapfb omapfb: resetting (status 0xffffff96,reset count 1)
> > ...
> > omapfb omapfb: resetting (status 0xffffff96,reset count 100)
> > omapfb omapfb: too many reset attempts, giving up.
> >
> > Looking closer at this I have found that it had been broken almost 2 years
> > ago with commit 2418996e3b100114edb2ae110d5d4acb928909d2, PM fixes for
> > OMAP1.
> >
> > The definite reason for broken omapfb/lcd_ams_delta in PM mode appeared to
> > be ARM_IDLECT1:IDLIF_ARM (bit 6) put into idle. The patch below fixes it.
> >
> > Since PM area is quite new to me, I am not sure if there may be a better
> > solution. AFAICS, the standard way to prevent an ARM_CLKCT1 bit being
> > switched to idle is to enable a clock that uses it (tipb_ck, dma_ck, or
> > tc_ck or one of its children in this case, right?).
> >
> > I assume there is no bug in omapfb nor lcdc, as that would be already
> > detected. Maybe it would be better to fix
> > drivers/video/omap/lcd_ams_delta.c (or
> > arch/arm/mach-omap1/board-ams-delta), but I don't know what clock should I
> > enable, if any.
> 
> More looking at it, I found that might be omap_dma_running() from 
> arch/arm/plat-omap/dma.c that needs correction. It already checks for LCD dma 
> running for OMAP1610, but does nothing similiar for 1510. I have revisited 
> http://focus.ti.com/lit/ug/spru674/spru674.pdf, but found no hint how to do 
> that in a 1610 similiar way.

Hmm to me it looks like the OMAP_DMA_CCR_EN should be set in one of the
channels if enabled. Maybe you need add a similar check somewhere in
the *_lcd_dma_* functions too in dma.c?

Regards,

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

Re: [PATCH] OMAP1: PM: Fix omapfb/lcd on Amstrad Delta broken when PM set

[Janusz Krzysztofik]

Friday 30 October 2009 18:33:18 Tony Lindgren napisał(a):
> * Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> [091030 07:43]:
> > Thursday 29 October 2009 23:39:44 Janusz Krzysztofik napisał(a):
> > > With CONFIG_PM=y, the omapfb/lcd device on Amstrad Delta, after
> > > initially starting correctly, breaks with the following error messages:
> > >
> > > omapfb omapfb: resetting (status 0xffffff96,reset count 1)
> > > ...
> > > omapfb omapfb: resetting (status 0xffffff96,reset count 100)
> > > omapfb omapfb: too many reset attempts, giving up.
> > >
> > > Looking closer at this I have found that it had been broken almost 2
> > > years ago with commit 2418996e3b100114edb2ae110d5d4acb928909d2, PM
> > > fixes for OMAP1.
> > >
> > > The definite reason for broken omapfb/lcd_ams_delta in PM mode appeared
> > > to be ARM_IDLECT1:IDLIF_ARM (bit 6) put into idle. The patch below
> > > fixes it.
> > >
> > > Since PM area is quite new to me, I am not sure if there may be a
> > > better solution. AFAICS, the standard way to prevent an ARM_CLKCT1 bit
> > > being switched to idle is to enable a clock that uses it (tipb_ck,
> > > dma_ck, or tc_ck or one of its children in this case, right?).
> > >
> > > I assume there is no bug in omapfb nor lcdc, as that would be already
> > > detected. Maybe it would be better to fix
> > > drivers/video/omap/lcd_ams_delta.c (or
> > > arch/arm/mach-omap1/board-ams-delta), but I don't know what clock
> > > should I enable, if any.
> >
> > More looking at it, I found that might be omap_dma_running() from
> > arch/arm/plat-omap/dma.c that needs correction. It already checks for LCD
> > dma running for OMAP1610, but does nothing similiar for 1510. I have
> > revisited http://focus.ti.com/lit/ug/spru674/spru674.pdf, but found no
> > hint how to do that in a 1610 similiar way.
>
> Hmm to me it looks like the OMAP_DMA_CCR_EN should be set in one of the
> channels if enabled. Maybe you need add a similar check somewhere in
> the *_lcd_dma_* functions too in dma.c?

Tony,
It sounds reasonable, but the problem is that in the OMAP5910 documentation I 
can find no DMA_CCR equivalent in the LCD dedicated DMA channel register set, 
nor EN equivalent in the DMA_LCD_CTRL register.

I have had a look at *_lcd_dma_*, as you suggested, and found this:

        /*
         * Set the Enable bit only if an external controller is
         * connected. Otherwise the OMAP internal controller will
         * start the transfer when it gets enabled.
         */
        if (enable_1510_mode || !lcd_dma.ext_ctrl)
                return;

That may suggest checking for LCD controller, not DMA channel, enable bit 
could give an answer if LCD DMA is likely to be running or not. So maybe 
adding a function to drivers/video/omap/lcdc.c that would check for 
OMAP_LCDC_CONTROL:OMAP_LCDC_CTRL_LCD_EN, then invoke that function from 
omap_dma_running() in case of omap1510 could be a proper solution.

However, that would affect not only Amstrad Delta, but all 1510 based 
machines. Since there were no reports about broken LCD DMA on 1510, I'd 
rather get a confirmation from omap guys, more experienced than me, that the 
solution proposed is correct and should work not only for my Amstrad Delta 
before I get that way.

Thanks,
Janusz
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] OMAP1: PM: Fix omapfb/lcd on Amstrad Delta broken when PM set

[Tony Lindgren]

* Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> [091031 18:20]:
> Friday 30 October 2009 18:33:18 Tony Lindgren napisał(a):
> > * Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> [091030 07:43]:
> > > Thursday 29 October 2009 23:39:44 Janusz Krzysztofik napisał(a):
> > > > With CONFIG_PM=y, the omapfb/lcd device on Amstrad Delta, after
> > > > initially starting correctly, breaks with the following error messages:
> > > >
> > > > omapfb omapfb: resetting (status 0xffffff96,reset count 1)
> > > > ...
> > > > omapfb omapfb: resetting (status 0xffffff96,reset count 100)
> > > > omapfb omapfb: too many reset attempts, giving up.
> > > >
> > > > Looking closer at this I have found that it had been broken almost 2
> > > > years ago with commit 2418996e3b100114edb2ae110d5d4acb928909d2, PM
> > > > fixes for OMAP1.
> > > >
> > > > The definite reason for broken omapfb/lcd_ams_delta in PM mode appeared
> > > > to be ARM_IDLECT1:IDLIF_ARM (bit 6) put into idle. The patch below
> > > > fixes it.
> > > >
> > > > Since PM area is quite new to me, I am not sure if there may be a
> > > > better solution. AFAICS, the standard way to prevent an ARM_CLKCT1 bit
> > > > being switched to idle is to enable a clock that uses it (tipb_ck,
> > > > dma_ck, or tc_ck or one of its children in this case, right?).
> > > >
> > > > I assume there is no bug in omapfb nor lcdc, as that would be already
> > > > detected. Maybe it would be better to fix
> > > > drivers/video/omap/lcd_ams_delta.c (or
> > > > arch/arm/mach-omap1/board-ams-delta), but I don't know what clock
> > > > should I enable, if any.
> > >
> > > More looking at it, I found that might be omap_dma_running() from
> > > arch/arm/plat-omap/dma.c that needs correction. It already checks for LCD
> > > dma running for OMAP1610, but does nothing similiar for 1510. I have
> > > revisited http://focus.ti.com/lit/ug/spru674/spru674.pdf, but found no
> > > hint how to do that in a 1610 similiar way.
> >
> > Hmm to me it looks like the OMAP_DMA_CCR_EN should be set in one of the
> > channels if enabled. Maybe you need add a similar check somewhere in
> > the *_lcd_dma_* functions too in dma.c?
> 
> Tony,
> It sounds reasonable, but the problem is that in the OMAP5910 documentation I 
> can find no DMA_CCR equivalent in the LCD dedicated DMA channel register set, 
> nor EN equivalent in the DMA_LCD_CTRL register.
> 
> I have had a look at *_lcd_dma_*, as you suggested, and found this:
> 
>         /*
>          * Set the Enable bit only if an external controller is
>          * connected. Otherwise the OMAP internal controller will
>          * start the transfer when it gets enabled.
>          */
>         if (enable_1510_mode || !lcd_dma.ext_ctrl)
>                 return;
> 
> That may suggest checking for LCD controller, not DMA channel, enable bit 
> could give an answer if LCD DMA is likely to be running or not. So maybe 
> adding a function to drivers/video/omap/lcdc.c that would check for 
> OMAP_LCDC_CONTROL:OMAP_LCDC_CTRL_LCD_EN, then invoke that function from 
> omap_dma_running() in case of omap1510 could be a proper solution.
> 
> However, that would affect not only Amstrad Delta, but all 1510 based 
> machines. Since there were no reports about broken LCD DMA on 1510, I'd 
> rather get a confirmation from omap guys, more experienced than me, that the 
> solution proposed is correct and should work not only for my Amstrad Delta 
> before I get that way.

That sounds like a reasonable way to fix it to me.

Regards,

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

Re: [PATCH] OMAP1: PM: Fix omapfb/lcd on Amstrad Delta broken when PM set

[Paul Walmsley]

Hello Janusz et al,

On Fri, 30 Oct 2009, Janusz Krzysztofik wrote:

> More looking at it, I found that might be omap_dma_running() from 
> arch/arm/plat-omap/dma.c that needs correction. It already checks for LCD dma 
> running for OMAP1610, but does nothing similiar for 1510. I have revisited 
> http://focus.ti.com/lit/ug/spru674/spru674.pdf, but found no hint how to do 
> that in a 1610 similiar way.

Speaking of this code, here's a project suggestion if someone has some 
spare time.  All of the LCD DMA code in plat-omap/dma.c appears to be 
OMAP1-only (and apparently only is available on a subset of OMAP1 chips).  
It would be great if this code could be moved to mach-omap1/lcd_dma.c or 
a similar place.


- Paul

Re: [PATCH] OMAP1: PM: Fix omapfb/lcd on Amstrad Delta broken when PM set

[Janusz Krzysztofik]

Tuesday 10 November 2009 11:50:52 Paul Walmsley napisał(a):
> Hello Janusz et al,
>
> On Fri, 30 Oct 2009, Janusz Krzysztofik wrote:
> > More looking at it, I found that might be omap_dma_running() from
> > arch/arm/plat-omap/dma.c that needs correction. It already checks for LCD
> > dma running for OMAP1610, but does nothing similiar for 1510. I have
> > revisited http://focus.ti.com/lit/ug/spru674/spru674.pdf, but found no
> > hint how to do that in a 1610 similiar way.
>
> Speaking of this code, here's a project suggestion if someone has some
> spare time.  All of the LCD DMA code in plat-omap/dma.c appears to be
> OMAP1-only (and apparently only is available on a subset of OMAP1 chips).
> It would be great if this code could be moved to mach-omap1/lcd_dma.c or
> a similar place.

Paul,

Thank you for putting me into right direction (I hope :-)). If there are no 
more comments, I'll try to take care of what you suggest after the situation 
with my pending patches AND Tomi Valkeinen's DSS2 patch series, that both 
more or less fiddle with LCD DMA related files, gets clear.

Thanks,
Janusz
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html