| Message ID | 20130515002537.GG5600@atomide.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, May 14, 2013 at 05:25:37PM -0700, Tony Lindgren wrote: > Commit a819c4f1 (ARM: OMAP3: PM: Only access IVA if one exists) > changed PM to not access IVA registers on omaps that don't have > them. Turns out we still need to idle iva2 as otherwise iva2_pwrdm > will stay on and block deeper idle states. > > Signed-off-by: Tony Lindgren <tony@atomide.com> > > --- > > Paul & Kevin, do you have better ideas for fixing this? > > --- a/arch/arm/mach-omap2/pm34xx.c > +++ b/arch/arm/mach-omap2/pm34xx.c > @@ -546,8 +546,10 @@ static void __init prcm_setup_regs(void) > /* Clear any pending PRCM interrupts */ > omap2_prm_write_mod_reg(0, OCP_MOD, OMAP3_PRM_IRQSTATUS_MPU_OFFSET); > > - if (omap3_has_iva()) > - omap3_iva_idle(); > + /* > + * We need to idle iva2_pwrdm even on am3703 with no iva2. > + */ > + omap3_iva_idle(); > > omap3_d2d_idle(); > } [Kevin, Paul, some background: Tony discovered that the am3703 needs to have omap3_iva_idle() called even though its not supposed to have an IVA.] This is potentially bad for other SoC's that don't have an IVA so I don't think its the way to go. My preference would be to set the OMAP3_HAS_IVA flag for the am3703 only since its the one with the bug. Maybe something in id.c:omap3xxx_check_features() but I don't see any existing way to check for an am3703 vs. other am37xx SoCs. Any ideas? Another option, I suppose, is a dts entry but I don't like that at all. Mark --
* Mark A. Greer <mgreer@animalcreek.com> [130514 18:57]: > On Tue, May 14, 2013 at 05:25:37PM -0700, Tony Lindgren wrote: > > Commit a819c4f1 (ARM: OMAP3: PM: Only access IVA if one exists) > > changed PM to not access IVA registers on omaps that don't have > > them. Turns out we still need to idle iva2 as otherwise iva2_pwrdm > > will stay on and block deeper idle states. > > > > Signed-off-by: Tony Lindgren <tony@atomide.com> > > > > --- > > > > Paul & Kevin, do you have better ideas for fixing this? > > > > --- a/arch/arm/mach-omap2/pm34xx.c > > +++ b/arch/arm/mach-omap2/pm34xx.c > > @@ -546,8 +546,10 @@ static void __init prcm_setup_regs(void) > > /* Clear any pending PRCM interrupts */ > > omap2_prm_write_mod_reg(0, OCP_MOD, OMAP3_PRM_IRQSTATUS_MPU_OFFSET); > > > > - if (omap3_has_iva()) > > - omap3_iva_idle(); > > + /* > > + * We need to idle iva2_pwrdm even on am3703 with no iva2. > > + */ > > + omap3_iva_idle(); > > > > omap3_d2d_idle(); > > } > > [Kevin, Paul, some background: Tony discovered that the am3703 needs > to have omap3_iva_idle() called even though its not supposed to have > an IVA.] > > This is potentially bad for other SoC's that don't have an IVA so I don't > think its the way to go. My preference would be to set the OMAP3_HAS_IVA > flag for the am3703 only since its the one with the bug. Maybe something > in id.c:omap3xxx_check_features() but I don't see any existing way to check > for an am3703 vs. other am37xx SoCs. Any ideas? > > Another option, I suppose, is a dts entry but I don't like that at all. It seems that iva2_pwrdm is there for all omap3 even if OMAP3_HAS_IVA flag is unset. And if that's the case, iva2 clocks still need to be idled in all cases. It's possible that not all steps in omap3_iva_idle() are needed though. I can debug further to see which part of the omap3_iva_idle() are needed. Mark, do you have some omap3 with no iva (other than am3703) to test the idle states with? Regards, Tony
On Wed, May 15, 2013 at 10:07:35AM -0700, Tony Lindgren wrote: > * Mark A. Greer <mgreer@animalcreek.com> [130514 18:57]: > > On Tue, May 14, 2013 at 05:25:37PM -0700, Tony Lindgren wrote: > > > Commit a819c4f1 (ARM: OMAP3: PM: Only access IVA if one exists) > > > changed PM to not access IVA registers on omaps that don't have > > > them. Turns out we still need to idle iva2 as otherwise iva2_pwrdm > > > will stay on and block deeper idle states. > > > > > > Signed-off-by: Tony Lindgren <tony@atomide.com> > > > > > > --- > > > > > > Paul & Kevin, do you have better ideas for fixing this? > > > > > > --- a/arch/arm/mach-omap2/pm34xx.c > > > +++ b/arch/arm/mach-omap2/pm34xx.c > > > @@ -546,8 +546,10 @@ static void __init prcm_setup_regs(void) > > > /* Clear any pending PRCM interrupts */ > > > omap2_prm_write_mod_reg(0, OCP_MOD, OMAP3_PRM_IRQSTATUS_MPU_OFFSET); > > > > > > - if (omap3_has_iva()) > > > - omap3_iva_idle(); > > > + /* > > > + * We need to idle iva2_pwrdm even on am3703 with no iva2. > > > + */ > > > + omap3_iva_idle(); > > > > > > omap3_d2d_idle(); > > > } > > > > [Kevin, Paul, some background: Tony discovered that the am3703 needs > > to have omap3_iva_idle() called even though its not supposed to have > > an IVA.] > > > > This is potentially bad for other SoC's that don't have an IVA so I don't > > think its the way to go. My preference would be to set the OMAP3_HAS_IVA > > flag for the am3703 only since its the one with the bug. Maybe something > > in id.c:omap3xxx_check_features() but I don't see any existing way to check > > for an am3703 vs. other am37xx SoCs. Any ideas? > > > > Another option, I suppose, is a dts entry but I don't like that at all. > > It seems that iva2_pwrdm is there for all omap3 even if OMAP3_HAS_IVA > flag is unset. And if that's the case, iva2 clocks still need to be idled > in all cases. Ahh, this takes us to the "Great TI Docs Mystery" :) -- its basically impossible to tell because despite what their docs may say, the hardware can be quite different. I'm not sure how to proceed other than trial & error with as many different SoCs as we can find. > It's possible that not all steps in omap3_iva_idle() are needed though. > I can debug further to see which part of the omap3_iva_idle() are needed. > > Mark, do you have some omap3 with no iva (other than am3703) to test the > idle states with? I have an am35xx which isn't supposed to have an IVA so I can test with it (although, I'm not sure how well the kernel works on the am35xx these days). I'm a bit busy today but I'll try booting the am35xx tomorrow and if it comes up, see what I can figure out. Mark --
On 15.05.2013 23:50, Mark A. Greer wrote: > On Wed, May 15, 2013 at 10:07:35AM -0700, Tony Lindgren wrote: >> * Mark A. Greer <mgreer@animalcreek.com> [130514 18:57]: >>> On Tue, May 14, 2013 at 05:25:37PM -0700, Tony Lindgren wrote: >>>> Commit a819c4f1 (ARM: OMAP3: PM: Only access IVA if one exists) >>>> changed PM to not access IVA registers on omaps that don't have >>>> them. Turns out we still need to idle iva2 as otherwise iva2_pwrdm >>>> will stay on and block deeper idle states. >>>> >>>> Signed-off-by: Tony Lindgren <tony@atomide.com> >>>> >>>> --- >>>> >>>> Paul & Kevin, do you have better ideas for fixing this? >>>> >>>> --- a/arch/arm/mach-omap2/pm34xx.c >>>> +++ b/arch/arm/mach-omap2/pm34xx.c >>>> @@ -546,8 +546,10 @@ static void __init prcm_setup_regs(void) >>>> /* Clear any pending PRCM interrupts */ >>>> omap2_prm_write_mod_reg(0, OCP_MOD, OMAP3_PRM_IRQSTATUS_MPU_OFFSET); >>>> >>>> - if (omap3_has_iva()) >>>> - omap3_iva_idle(); >>>> + /* >>>> + * We need to idle iva2_pwrdm even on am3703 with no iva2. >>>> + */ >>>> + omap3_iva_idle(); >>>> >>>> omap3_d2d_idle(); >>>> } >>> >>> [Kevin, Paul, some background: Tony discovered that the am3703 needs >>> to have omap3_iva_idle() called even though its not supposed to have >>> an IVA.] >>> >>> This is potentially bad for other SoC's that don't have an IVA so I don't >>> think its the way to go. My preference would be to set the OMAP3_HAS_IVA >>> flag for the am3703 only since its the one with the bug. Maybe something >>> in id.c:omap3xxx_check_features() but I don't see any existing way to check >>> for an am3703 vs. other am37xx SoCs. Any ideas? >>> >>> Another option, I suppose, is a dts entry but I don't like that at all. >> >> It seems that iva2_pwrdm is there for all omap3 even if OMAP3_HAS_IVA >> flag is unset. And if that's the case, iva2 clocks still need to be idled >> in all cases. > > Ahh, this takes us to the "Great TI Docs Mystery" :) -- its basically > impossible to tell because despite what their docs may say, the hardware > can be quite different. I'm not sure how to proceed other than trial & > error with as many different SoCs as we can find. > >> It's possible that not all steps in omap3_iva_idle() are needed though. >> I can debug further to see which part of the omap3_iva_idle() are needed. >> >> Mark, do you have some omap3 with no iva (other than am3703) to test the >> idle states with? > > I have an am35xx which isn't supposed to have an IVA so I can test with it > (although, I'm not sure how well the kernel works on the am35xx these days). > > I'm a bit busy today but I'll try booting the am35xx tomorrow and if it > comes up, see what I can figure out. I think this issue is relevant to am3517 as you can see from this thread: http://thread.gmane.org/gmane.linux.ports.arm.omap/97903 I could boot only with your patch http://www.spinics.net/lists/arm-kernel/msg168865.html I have such a system running, so if you have any other patches/ideas to test, I would do it. Yegor
* Yegor Yefremov <yegor_sub1@visionsystems.de> [130516 05:13]: > On 15.05.2013 23:50, Mark A. Greer wrote: > > On Wed, May 15, 2013 at 10:07:35AM -0700, Tony Lindgren wrote: > >> * Mark A. Greer <mgreer@animalcreek.com> [130514 18:57]: > >>> On Tue, May 14, 2013 at 05:25:37PM -0700, Tony Lindgren wrote: > >>>> Commit a819c4f1 (ARM: OMAP3: PM: Only access IVA if one exists) > >>>> changed PM to not access IVA registers on omaps that don't have > >>>> them. Turns out we still need to idle iva2 as otherwise iva2_pwrdm > >>>> will stay on and block deeper idle states. > >>>> > >>>> Signed-off-by: Tony Lindgren <tony@atomide.com> > >>>> > >>>> --- > >>>> > >>>> Paul & Kevin, do you have better ideas for fixing this? > >>>> > >>>> --- a/arch/arm/mach-omap2/pm34xx.c > >>>> +++ b/arch/arm/mach-omap2/pm34xx.c > >>>> @@ -546,8 +546,10 @@ static void __init prcm_setup_regs(void) > >>>> /* Clear any pending PRCM interrupts */ > >>>> omap2_prm_write_mod_reg(0, OCP_MOD, OMAP3_PRM_IRQSTATUS_MPU_OFFSET); > >>>> > >>>> - if (omap3_has_iva()) > >>>> - omap3_iva_idle(); > >>>> + /* > >>>> + * We need to idle iva2_pwrdm even on am3703 with no iva2. > >>>> + */ > >>>> + omap3_iva_idle(); > >>>> > >>>> omap3_d2d_idle(); > >>>> } > >>> > >>> [Kevin, Paul, some background: Tony discovered that the am3703 needs > >>> to have omap3_iva_idle() called even though its not supposed to have > >>> an IVA.] > >>> > >>> This is potentially bad for other SoC's that don't have an IVA so I don't > >>> think its the way to go. My preference would be to set the OMAP3_HAS_IVA > >>> flag for the am3703 only since its the one with the bug. Maybe something > >>> in id.c:omap3xxx_check_features() but I don't see any existing way to check > >>> for an am3703 vs. other am37xx SoCs. Any ideas? > >>> > >>> Another option, I suppose, is a dts entry but I don't like that at all. > >> > >> It seems that iva2_pwrdm is there for all omap3 even if OMAP3_HAS_IVA > >> flag is unset. And if that's the case, iva2 clocks still need to be idled > >> in all cases. > > > > Ahh, this takes us to the "Great TI Docs Mystery" :) -- its basically > > impossible to tell because despite what their docs may say, the hardware > > can be quite different. I'm not sure how to proceed other than trial & > > error with as many different SoCs as we can find. > > > >> It's possible that not all steps in omap3_iva_idle() are needed though. > >> I can debug further to see which part of the omap3_iva_idle() are needed. > >> > >> Mark, do you have some omap3 with no iva (other than am3703) to test the > >> idle states with? > > > > I have an am35xx which isn't supposed to have an IVA so I can test with it > > (although, I'm not sure how well the kernel works on the am35xx these days). > > > > I'm a bit busy today but I'll try booting the am35xx tomorrow and if it > > comes up, see what I can figure out. > > I think this issue is relevant to am3517 as you can see from this thread: http://thread.gmane.org/gmane.linux.ports.arm.omap/97903 > I could boot only with your patch http://www.spinics.net/lists/arm-kernel/msg168865.html OK sounds like Mark's patch as http://www.spinics.net/lists/arm-kernel/msg168865.html is needed as a fix. > I have such a system running, so if you have any other patches/ideas to test, I would do it. Well I think my patch does not matter for am3517 as that one has iva2 while am3703 does not. Regards, Tony
* Tony Lindgren <tony@atomide.com> [130516 11:40]: > * Yegor Yefremov <yegor_sub1@visionsystems.de> [130516 05:13]: > > > > I think this issue is relevant to am3517 as you can see from this thread: http://thread.gmane.org/gmane.linux.ports.arm.omap/97903 > > I could boot only with your patch http://www.spinics.net/lists/arm-kernel/msg168865.html > > OK sounds like Mark's patch as http://www.spinics.net/lists/arm-kernel/msg168865.html > is needed as a fix. Yegor, thinking about your boot issue more, I'm thinking that you have your board .init_time = omap3_secure_sync32k_timer_init, or some non-waking gpt timer. And that means the system won't wake to timer events. The system should still wake to timer events even if you have no wake-up events configred, so Mark's patch probably does not fix the right issue for you. Regards, Tony
On Thu, May 16, 2013 at 11:35:16AM -0700, Tony Lindgren wrote: > * Yegor Yefremov <yegor_sub1@visionsystems.de> [130516 05:13]: > > On 15.05.2013 23:50, Mark A. Greer wrote: > > > On Wed, May 15, 2013 at 10:07:35AM -0700, Tony Lindgren wrote: > > >> * Mark A. Greer <mgreer@animalcreek.com> [130514 18:57]: > > >>> On Tue, May 14, 2013 at 05:25:37PM -0700, Tony Lindgren wrote: > > >>>> Commit a819c4f1 (ARM: OMAP3: PM: Only access IVA if one exists) > > >>>> changed PM to not access IVA registers on omaps that don't have > > >>>> them. Turns out we still need to idle iva2 as otherwise iva2_pwrdm > > >>>> will stay on and block deeper idle states. > > >>>> > > >>>> Signed-off-by: Tony Lindgren <tony@atomide.com> > > >>>> > > >>>> --- > > >>>> > > >>>> Paul & Kevin, do you have better ideas for fixing this? > > >>>> > > >>>> --- a/arch/arm/mach-omap2/pm34xx.c > > >>>> +++ b/arch/arm/mach-omap2/pm34xx.c > > >>>> @@ -546,8 +546,10 @@ static void __init prcm_setup_regs(void) > > >>>> /* Clear any pending PRCM interrupts */ > > >>>> omap2_prm_write_mod_reg(0, OCP_MOD, OMAP3_PRM_IRQSTATUS_MPU_OFFSET); > > >>>> > > >>>> - if (omap3_has_iva()) > > >>>> - omap3_iva_idle(); > > >>>> + /* > > >>>> + * We need to idle iva2_pwrdm even on am3703 with no iva2. > > >>>> + */ > > >>>> + omap3_iva_idle(); > > >>>> > > >>>> omap3_d2d_idle(); > > >>>> } > > >>> > > >>> [Kevin, Paul, some background: Tony discovered that the am3703 needs > > >>> to have omap3_iva_idle() called even though its not supposed to have > > >>> an IVA.] > > >>> > > >>> This is potentially bad for other SoC's that don't have an IVA so I don't > > >>> think its the way to go. My preference would be to set the OMAP3_HAS_IVA > > >>> flag for the am3703 only since its the one with the bug. Maybe something > > >>> in id.c:omap3xxx_check_features() but I don't see any existing way to check > > >>> for an am3703 vs. other am37xx SoCs. Any ideas? > > >>> > > >>> Another option, I suppose, is a dts entry but I don't like that at all. > > >> > > >> It seems that iva2_pwrdm is there for all omap3 even if OMAP3_HAS_IVA > > >> flag is unset. And if that's the case, iva2 clocks still need to be idled > > >> in all cases. > > > > > > Ahh, this takes us to the "Great TI Docs Mystery" :) -- its basically > > > impossible to tell because despite what their docs may say, the hardware > > > can be quite different. I'm not sure how to proceed other than trial & > > > error with as many different SoCs as we can find. > > > > > >> It's possible that not all steps in omap3_iva_idle() are needed though. > > >> I can debug further to see which part of the omap3_iva_idle() are needed. > > >> > > >> Mark, do you have some omap3 with no iva (other than am3703) to test the > > >> idle states with? > > > > > > I have an am35xx which isn't supposed to have an IVA so I can test with it > > > (although, I'm not sure how well the kernel works on the am35xx these days). > > > > > > I'm a bit busy today but I'll try booting the am35xx tomorrow and if it > > > comes up, see what I can figure out. > > > > I think this issue is relevant to am3517 as you can see from this thread: http://thread.gmane.org/gmane.linux.ports.arm.omap/97903 > > I could boot only with your patch http://www.spinics.net/lists/arm-kernel/msg168865.html > > OK sounds like Mark's patch as http://www.spinics.net/lists/arm-kernel/msg168865.html > is needed as a fix. FYI, that patch works (until recently, anyway) but is an unacceptable approach. > > I have such a system running, so if you have any other patches/ideas to test, I would do it. > > Well I think my patch does not matter for am3517 as that one has iva2 > while am3703 does not. To make sure there is no confusion, the am3517 does NOT have an iva2. Tony was misled by a boot log from an older kernel that didn't have the patch that unsets the IVA feature flag. Aside: I have so far been unable to get my am3517evm to come up. Mark --
On Thu, May 16, 2013 at 12:19:20PM +0200, Yegor Yefremov wrote: > On 15.05.2013 23:50, Mark A. Greer wrote: > > On Wed, May 15, 2013 at 10:07:35AM -0700, Tony Lindgren wrote: > >> Mark, do you have some omap3 with no iva (other than am3703) to test the > >> idle states with? > > > > I have an am35xx which isn't supposed to have an IVA so I can test with it > > (although, I'm not sure how well the kernel works on the am35xx these days). > > > > I'm a bit busy today but I'll try booting the am35xx tomorrow and if it > > comes up, see what I can figure out. > > I think this issue is relevant to am3517 as you can see from this thread: http://thread.gmane.org/gmane.linux.ports.arm.omap/97903 > I could boot only with your patch http://www.spinics.net/lists/arm-kernel/msg168865.html > > I have such a system running, so if you have any other patches/ideas to test, I would do it. Yegor, since I've so far been unable to get my am3517evm to fire up, can you play around with the 3517 to see if there appears to be iva registers that can be read/written to safely? In particular, can the code that idles the iva be run safely on an am3517? Thanks, Mark --
On Tue, May 14, 2013 at 06:52:12PM -0700, Mark A. Greer wrote: > On Tue, May 14, 2013 at 05:25:37PM -0700, Tony Lindgren wrote: > > Commit a819c4f1 (ARM: OMAP3: PM: Only access IVA if one exists) > > changed PM to not access IVA registers on omaps that don't have > > them. Turns out we still need to idle iva2 as otherwise iva2_pwrdm > > will stay on and block deeper idle states. > > > > Signed-off-by: Tony Lindgren <tony@atomide.com> > > > > --- > > > > Paul & Kevin, do you have better ideas for fixing this? > > > > --- a/arch/arm/mach-omap2/pm34xx.c > > +++ b/arch/arm/mach-omap2/pm34xx.c > > @@ -546,8 +546,10 @@ static void __init prcm_setup_regs(void) > > /* Clear any pending PRCM interrupts */ > > omap2_prm_write_mod_reg(0, OCP_MOD, OMAP3_PRM_IRQSTATUS_MPU_OFFSET); > > > > - if (omap3_has_iva()) > > - omap3_iva_idle(); > > + /* > > + * We need to idle iva2_pwrdm even on am3703 with no iva2. > > + */ > > + omap3_iva_idle(); > > > > omap3_d2d_idle(); > > } > This is potentially bad for other SoC's that don't have an IVA so I don't > think its the way to go. My preference would be to set the OMAP3_HAS_IVA > flag for the am3703 only since its the one with the bug. Maybe something > in id.c:omap3xxx_check_features() but I don't see any existing way to check > for an am3703 vs. other am37xx SoCs. Any ideas? As I think about it more, I may have been overly dramatic here. Before the check that's being removed existed, omap3_iva_idle() was always called and nobody complained (including am3517). So, I'd still like to hear what Yegor can figure out but I have to believe that this patch is safe. Mark --
On Fri, May 17, 2013 at 8:56 PM, Mark A. Greer <mgreer@animalcreek.com> wrote: > On Thu, May 16, 2013 at 12:19:20PM +0200, Yegor Yefremov wrote: >> On 15.05.2013 23:50, Mark A. Greer wrote: >> > On Wed, May 15, 2013 at 10:07:35AM -0700, Tony Lindgren wrote: > >> >> Mark, do you have some omap3 with no iva (other than am3703) to test the >> >> idle states with? >> > >> > I have an am35xx which isn't supposed to have an IVA so I can test with it >> > (although, I'm not sure how well the kernel works on the am35xx these days). >> > >> > I'm a bit busy today but I'll try booting the am35xx tomorrow and if it >> > comes up, see what I can figure out. >> >> I think this issue is relevant to am3517 as you can see from this thread: http://thread.gmane.org/gmane.linux.ports.arm.omap/97903 >> I could boot only with your patch http://www.spinics.net/lists/arm-kernel/msg168865.html >> >> I have such a system running, so if you have any other patches/ideas to test, I would do it. > > Yegor, since I've so far been unable to get my am3517evm to fire up, can > you play around with the 3517 to see if there appears to be iva registers > that can be read/written to safely? In particular, can the code that idles > the iva be run safely on an am3517? I'll look into this next week. am3517 and IVA seems to be a really big mystery. In the TI forum will be stated, there is no IVA: http://e2e.ti.com/support/arm/sitara_arm/f/791/t/150961.aspx, but TRM has various appearances of IVA2 registers, but no real description/explanation of what is IVA and how to eat it. Yegor
--- a/arch/arm/mach-omap2/pm34xx.c +++ b/arch/arm/mach-omap2/pm34xx.c @@ -546,8 +546,10 @@ static void __init prcm_setup_regs(void) /* Clear any pending PRCM interrupts */ omap2_prm_write_mod_reg(0, OCP_MOD, OMAP3_PRM_IRQSTATUS_MPU_OFFSET); - if (omap3_has_iva()) - omap3_iva_idle(); + /* + * We need to idle iva2_pwrdm even on am3703 with no iva2. + */ + omap3_iva_idle(); omap3_d2d_idle(); }
Commit a819c4f1 (ARM: OMAP3: PM: Only access IVA if one exists) changed PM to not access IVA registers on omaps that don't have them. Turns out we still need to idle iva2 as otherwise iva2_pwrdm will stay on and block deeper idle states. Signed-off-by: Tony Lindgren <tony@atomide.com> --- Paul & Kevin, do you have better ideas for fixing this?