* [PATCH] omap3: Prevent SDRC deadlock when L3 is changing frequency @ 2010-07-21 16:02 Jon Hunter 2010-09-15 19:15 ` Paul Walmsley 2010-09-15 21:38 ` Paul Walmsley 0 siblings, 2 replies; 13+ messages in thread From: Jon Hunter @ 2010-07-21 16:02 UTC (permalink / raw) To: linux-omap; +Cc: Jon Hunter From: Jon Hunter <jon-hunter@ti.com> When changing the L3 clock frequency, the CPU is executing from internal RAM and the SDRC clock is disabled. During this time accesses made to external DDR are stalled. If the ARM subsystem attempts to access the DDR while the SDRC clock is disabled this will stall the CPU until the access to the SDRC timeouts. A timeout on the SDRC should never occur. Once a timeout occurs all the following accesses will be aborted and the DDR is no longer accessible. Although the code being executed in the internal RAM does not directly access the DDR, it was found that the branch prediction logic in the CPU may cause the CPU to prefetch code from a DDR location while the SDRC clock is disabled. This was causing an SDRC timeout which resulted in a system hang. This patch fixes this problem by ensuring the branch prediction logic is disabled while changing the L3 clock frequency. The branch prediction logic is disabled by clearing the Z-bit in the ARM AUX CTRL register. Disabling the branch prediction logic does not have any noticable impact on the execution time of this code section. The hardware observability signals were used to monitor the sdrc idle time with and without this patch when operating at different CPU frequencies (150MHz, 500MHz and 600MHz) and the total sdrc idle time when changing frequenct was in the range of 9-11us. This was measured on an omap3430 SDP running the omapzoom p-android-omap-2.6.29 branch. This change has been commited to both TI's android 2.6.29 and 2.6.32 kernels. The commits can be viewed here: http://git.omapzoom.org/?p=kernel/omap.git;a=commit;h=5679c7b1142f3cc2b9285181d53f6b40c4d0296d http://git.omapzoom.org/?p=kernel/omap.git;a=commit;h=cf16e57823575d98e9d5165aa7a498ffb751c940 This patch has been rebased on the latest linux-omap tree and tested on Kevin Hilman's pm branch. Signed-off-by: Jon Hunter <jon-hunter@ti.com> --- arch/arm/mach-omap2/sram34xx.S | 6 +++++- 1 files changed, 5 insertions(+), 1 deletions(-) diff --git a/arch/arm/mach-omap2/sram34xx.S b/arch/arm/mach-omap2/sram34xx.S index de99ba2..e87e730 100644 --- a/arch/arm/mach-omap2/sram34xx.S +++ b/arch/arm/mach-omap2/sram34xx.S @@ -129,8 +129,11 @@ ENTRY(omap3_sram_configure_core_dpll) ldr r4, [sp, #80] str r4, omap_sdrc_mr_1_val skip_cs1_params: + mrc p15, 0, r8, c1, c0, 0 @ read aux ctrl register + bic r10, r8, #0x800 @ clear Z-bit, disable branch prediction + mcr p15, 0, r10, c1, c0, 0 @ write aux ctrl register dsb @ flush buffered writes to interconnect - + isb @ prevent speculative exec past here cmp r3, #1 @ if increasing SDRC clk rate, bleq configure_sdrc @ program the SDRC regs early (for RFR) cmp r1, #SDRC_UNLOCK_DLL @ set the intended DLL state @@ -148,6 +151,7 @@ skip_cs1_params: beq return_to_sdram @ return to SDRAM code, otherwise, bl configure_sdrc @ reprogram SDRC regs now return_to_sdram: + mcr p15, 0, r8, c1, c0, 0 @ restore aux ctrl register isb @ prevent speculative exec past here mov r0, #0 @ return value ldmfd sp!, {r1-r12, pc} @ restore regs and return -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] omap3: Prevent SDRC deadlock when L3 is changing frequency 2010-07-21 16:02 [PATCH] omap3: Prevent SDRC deadlock when L3 is changing frequency Jon Hunter @ 2010-09-15 19:15 ` Paul Walmsley 2010-09-15 22:40 ` Tony Lindgren 2010-09-16 6:05 ` Woodruff, Richard 2010-09-15 21:38 ` Paul Walmsley 1 sibling, 2 replies; 13+ messages in thread From: Paul Walmsley @ 2010-09-15 19:15 UTC (permalink / raw) To: Jon Hunter; +Cc: linux-omap, khilman, tony Hi Jon, I regret the delay: On Wed, 21 Jul 2010, Jon Hunter wrote: > From: Jon Hunter <jon-hunter@ti.com> > > When changing the L3 clock frequency, the CPU is executing from internal RAM > and the SDRC clock is disabled. During this time accesses made to external > DDR are stalled. If the ARM subsystem attempts to access the DDR while the > SDRC clock is disabled this will stall the CPU until the access to the SDRC > timeouts. A timeout on the SDRC should never occur. Once a timeout occurs all > the following accesses will be aborted and the DDR is no longer accessible. > > Although the code being executed in the internal RAM does not directly access > the DDR, it was found that the branch prediction logic in the CPU may cause > the CPU to prefetch code from a DDR location while the SDRC clock is disabled. > This was causing an SDRC timeout which resulted in a system hang. > > This patch fixes this problem by ensuring the branch prediction logic is > disabled while changing the L3 clock frequency. The branch prediction logic > is disabled by clearing the Z-bit in the ARM AUX CTRL register. > > Disabling the branch prediction logic does not have any noticable impact > on the execution time of this code section. The hardware observability > signals were used to monitor the sdrc idle time with and without this > patch when operating at different CPU frequencies (150MHz, 500MHz and > 600MHz) and the total sdrc idle time when changing frequenct was in > the range of 9-11us. This was measured on an omap3430 SDP running the > omapzoom p-android-omap-2.6.29 branch. > > This change has been commited to both TI's android 2.6.29 and 2.6.32 kernels. > The commits can be viewed here: > http://git.omapzoom.org/?p=kernel/omap.git;a=commit;h=5679c7b1142f3cc2b9285181d53f6b40c4d0296d > http://git.omapzoom.org/?p=kernel/omap.git;a=commit;h=cf16e57823575d98e9d5165aa7a498ffb751c940 > > This patch has been rebased on the latest linux-omap tree and tested on > Kevin Hilman's pm branch. > > Signed-off-by: Jon Hunter <jon-hunter@ti.com> Really nice changelog. I wish every patch had a description this good. Patch looks really good, too. Queued for 2.6.37. - Paul ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] omap3: Prevent SDRC deadlock when L3 is changing frequency 2010-09-15 19:15 ` Paul Walmsley @ 2010-09-15 22:40 ` Tony Lindgren 2010-09-15 23:10 ` Paul Walmsley 2010-09-16 6:05 ` Woodruff, Richard 1 sibling, 1 reply; 13+ messages in thread From: Tony Lindgren @ 2010-09-15 22:40 UTC (permalink / raw) To: Paul Walmsley; +Cc: Jon Hunter, linux-omap, khilman * Paul Walmsley <paul@pwsan.com> [100915 12:07]: > > > > This change has been commited to both TI's android 2.6.29 and 2.6.32 kernels. > > The commits can be viewed here: > > http://git.omapzoom.org/?p=kernel/omap.git;a=commit;h=5679c7b1142f3cc2b9285181d53f6b40c4d0296d > > http://git.omapzoom.org/?p=kernel/omap.git;a=commit;h=cf16e57823575d98e9d5165aa7a498ffb751c940 > > > > This patch has been rebased on the latest linux-omap tree and tested on > > Kevin Hilman's pm branch. Sounds like a very important fix. But also a good example of how messed up things are with the TI Linux kernel development. Why has this fix it been sitting in some TI internal tree since Fri, 12 Mar 2010? This same bug has been patched in three different trees? But not in the mainline kernel? And this is happening all the time with the TI fixes. Tony ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] omap3: Prevent SDRC deadlock when L3 is changing frequency 2010-09-15 22:40 ` Tony Lindgren @ 2010-09-15 23:10 ` Paul Walmsley 2010-09-16 4:58 ` Gadiyar, Anand 0 siblings, 1 reply; 13+ messages in thread From: Paul Walmsley @ 2010-09-15 23:10 UTC (permalink / raw) To: Tony Lindgren; +Cc: Jon Hunter, linux-omap, khilman On Wed, 15 Sep 2010, Tony Lindgren wrote: > * Paul Walmsley <paul@pwsan.com> [100915 12:07]: > > > > > > This change has been commited to both TI's android 2.6.29 and 2.6.32 kernels. > > > The commits can be viewed here: > > > http://git.omapzoom.org/?p=kernel/omap.git;a=commit;h=5679c7b1142f3cc2b9285181d53f6b40c4d0296d > > > http://git.omapzoom.org/?p=kernel/omap.git;a=commit;h=cf16e57823575d98e9d5165aa7a498ffb751c940 > > > > > > This patch has been rebased on the latest linux-omap tree and tested on > > > Kevin Hilman's pm branch. > > Sounds like a very important fix. But also a good example of how > messed up things are with the TI Linux kernel development. > > Why has this fix it been sitting in some TI internal tree since > Fri, 12 Mar 2010? About two months of it is my fault, since it was posted to l-o on July 21. But all the time between 12 March and 21 July is up to TI to answer. This really could have been a useful patch for certain vendors to have that are using CORE DVFS on their currently-shipping OMAP3 devices. > This same bug has been patched in three different trees? But not in > the mainline kernel? > > And this is happening all the time with the TI fixes. Yeah. - Paul ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] omap3: Prevent SDRC deadlock when L3 is changing frequency 2010-09-15 23:10 ` Paul Walmsley @ 2010-09-16 4:58 ` Gadiyar, Anand 2010-09-16 18:57 ` Tony Lindgren 0 siblings, 1 reply; 13+ messages in thread From: Gadiyar, Anand @ 2010-09-16 4:58 UTC (permalink / raw) To: Paul Walmsley; +Cc: Tony Lindgren, Jon Hunter, linux-omap, khilman On Thu, Sep 16, 2010 at 4:40 AM, Paul Walmsley <paul@pwsan.com> wrote: > On Wed, 15 Sep 2010, Tony Lindgren wrote: > >> * Paul Walmsley <paul@pwsan.com> [100915 12:07]: >> > > >> > > This change has been commited to both TI's android 2.6.29 and 2.6.32 kernels. >> > > The commits can be viewed here: >> > > http://git.omapzoom.org/?p=kernel/omap.git;a=commit;h=5679c7b1142f3cc2b9285181d53f6b40c4d0296d >> > > http://git.omapzoom.org/?p=kernel/omap.git;a=commit;h=cf16e57823575d98e9d5165aa7a498ffb751c940 >> > > >> > > This patch has been rebased on the latest linux-omap tree and tested on >> > > Kevin Hilman's pm branch. >> >> Sounds like a very important fix. But also a good example of how >> messed up things are with the TI Linux kernel development. >> >> Why has this fix it been sitting in some TI internal tree since >> Fri, 12 Mar 2010? > Er, the Mar 12 commit date is misleading - it's likely a customer-internal tree. It was committed to a TI internal tree 29 June/15 July, and sent to the list within a week of the second revision. The people who discovered this are on customer support - we spend a significant amount of time solving bugs like these, but have very little gap before the next big one strikes. We work tight production schedules, getting products out and only just have enough time to inform internal teams, but not enough to check if these fixes apply to internal trees (mainline is far far away) and get them there. (FWIW, Jon's been responsible for discovering and fixing more bugs on OMAP than almost anyone else I know.) For this specific patch, I'm not sure if other TI PM teams even knew about the existence of this patch until end-June. > About two months of it is my fault, since it was posted to l-o on July 21. > But all the time between 12 March and 21 July is up to TI to answer. > This really could have been a useful patch for certain vendors to have > that are using CORE DVFS on their currently-shipping OMAP3 devices. Sure, and I'm certain those other vendors have an equal number of critical bug fixes sitting in their own trees, which they steadfastly refuse to share with other competing vendors until their own products are out. (I have specific examples in mind, but don't want to start another flame war). Grow up - when a bug is discovered in the field, people are not likely to share with others in the interest of their own product timelines. While this may overall be less beneficial for everyone, that is indeed how many think and work. > >> This same bug has been patched in three different trees? But not in >> the mainline kernel? >> >> And this is happening all the time with the TI fixes. > > Yeah. OMAP4 has been demonstrably different - almost every single patch reaches mainline almost as soon as it is discovered. We had new silicon woken up with near-mainline code, and new boards supported in mainline even before the number of boards were in the mid-double digits. We're working very hard to keep things this way - give us a break. Stop inciting flame wars. A little support and understanding on your part would go a long way. - Anand ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] omap3: Prevent SDRC deadlock when L3 is changing frequency 2010-09-16 4:58 ` Gadiyar, Anand @ 2010-09-16 18:57 ` Tony Lindgren 2010-09-16 20:23 ` Gadiyar, Anand 0 siblings, 1 reply; 13+ messages in thread From: Tony Lindgren @ 2010-09-16 18:57 UTC (permalink / raw) To: Gadiyar, Anand; +Cc: Paul Walmsley, Jon Hunter, linux-omap, khilman * Gadiyar, Anand <gadiyar@ti.com> [100915 21:51]: > > > About two months of it is my fault, since it was posted to l-o on July 21. > > But all the time between 12 March and 21 July is up to TI to answer. > > This really could have been a useful patch for certain vendors to have > > that are using CORE DVFS on their currently-shipping OMAP3 devices. > > Sure, and I'm certain those other vendors have an equal number of critical > bug fixes sitting in their own trees, which they steadfastly refuse to > share with > other competing vendors until their own products are out. (I have specific > examples in mind, but don't want to start another flame war). > > Grow up - when a bug is discovered in the field, people are not likely to > share with others in the interest of their own product timelines. While > this may overall be less beneficial for everyone, that is indeed how many > think and work. I don't buy this. But maybe some of TI's customers can comment on this? AFAIK everybody wants to avoid the duplicate effort of finding and fixing these bugs. TI is the only party who is even aware of all the bugs fixed in various places. And it's TI who should follow up that the bugfixes get posted in a timely manner. Regards, Tony ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] omap3: Prevent SDRC deadlock when L3 is changing frequency 2010-09-16 18:57 ` Tony Lindgren @ 2010-09-16 20:23 ` Gadiyar, Anand 2010-09-16 20:49 ` Tony Lindgren 0 siblings, 1 reply; 13+ messages in thread From: Gadiyar, Anand @ 2010-09-16 20:23 UTC (permalink / raw) To: Tony Lindgren; +Cc: Paul Walmsley, Jon Hunter, linux-omap, khilman On Fri, Sep 17, 2010 at 12:27 AM, Tony Lindgren <tony@atomide.com> wrote: > * Gadiyar, Anand <gadiyar@ti.com> [100915 21:51]: >> >> > About two months of it is my fault, since it was posted to l-o on July 21. >> > But all the time between 12 March and 21 July is up to TI to answer. >> > This really could have been a useful patch for certain vendors to have >> > that are using CORE DVFS on their currently-shipping OMAP3 devices. >> >> Sure, and I'm certain those other vendors have an equal number of critical >> bug fixes sitting in their own trees, which they steadfastly refuse to >> share with >> other competing vendors until their own products are out. (I have specific >> examples in mind, but don't want to start another flame war). >> >> Grow up - when a bug is discovered in the field, people are not likely to >> share with others in the interest of their own product timelines. While >> this may overall be less beneficial for everyone, that is indeed how many >> think and work. > > I don't buy this. But maybe some of TI's customers can comment on this? > > AFAIK everybody wants to avoid the duplicate effort of finding and fixing > these bugs. > > TI is the only party who is even aware of all the bugs fixed in various > places. And it's TI who should follow up that the bugfixes get posted > in a timely manner. > We're committed 100% to making sure mainline is working well on OMAPs. This means that any bugs we discover will be reported, and any fixes we have will make their way upstream as soon as possible. I'm not going to go and fight what's happened in the past. But for the future, we're taking steps to make sure mainline works well. For the past - OMAP3 and before - we'll do our best to scrub all internal trees for missing bugs/features/what not and hopefully this will go upstream as we discover those. Sounds fair? - Anand ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] omap3: Prevent SDRC deadlock when L3 is changing frequency 2010-09-16 20:23 ` Gadiyar, Anand @ 2010-09-16 20:49 ` Tony Lindgren 2010-09-17 0:05 ` Woodruff, Richard 0 siblings, 1 reply; 13+ messages in thread From: Tony Lindgren @ 2010-09-16 20:49 UTC (permalink / raw) To: Gadiyar, Anand; +Cc: Paul Walmsley, Jon Hunter, linux-omap, khilman * Gadiyar, Anand <gadiyar@ti.com> [100916 13:16]: > On Fri, Sep 17, 2010 at 12:27 AM, Tony Lindgren <tony@atomide.com> wrote: > > For the past - OMAP3 and before - we'll do our best to scrub all internal > trees for missing bugs/features/what not and hopefully this will > go upstream as we discover those. > > Sounds fair? Sounds fair. But it might be worth checking that you guys have a system in place for collecting omap specific fixes and promptly merging them upstream to the mainline tree. Regards, Tony ^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH] omap3: Prevent SDRC deadlock when L3 is changing frequency 2010-09-16 20:49 ` Tony Lindgren @ 2010-09-17 0:05 ` Woodruff, Richard 0 siblings, 0 replies; 13+ messages in thread From: Woodruff, Richard @ 2010-09-17 0:05 UTC (permalink / raw) To: Tony Lindgren, Gadiyar, Anand Cc: Paul Walmsley, Hunter, Jon, linux-omap, khilman > From: linux-omap-owner@vger.kernel.org [mailto:linux-omap- > owner@vger.kernel.org] On Behalf Of Tony Lindgren > Sent: Thursday, September 16, 2010 3:50 PM > Sounds fair. But it might be worth checking that you guys have a system > in place for collecting omap specific fixes and promptly merging them > upstream to the mainline tree. Many times there will be lag. In many cases the customer has written significant code. Until they publish this code or publicly post it for review its not something TI should not repost as is. I think many of the engineers do see the benefit of sharing. However, if they don't have a policy in place which allows them to post until after the production is released it won't happen early. Anymore, I like to advise a customer's team that it is ok to post patches for RFC even if they don't have time to follow up or it is against an 'old' kernel. Once this is done TI and others are free to make use of the info to better the common tree. The customer who found it will re-use the tree likely and get this plus other benefits. Its best to get this policy set at project start. Asking near launch may very well be denied. Regards, Richard W. ^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH] omap3: Prevent SDRC deadlock when L3 is changing frequency 2010-09-15 19:15 ` Paul Walmsley 2010-09-15 22:40 ` Tony Lindgren @ 2010-09-16 6:05 ` Woodruff, Richard 2010-09-16 6:24 ` Shilimkar, Santosh 1 sibling, 1 reply; 13+ messages in thread From: Woodruff, Richard @ 2010-09-16 6:05 UTC (permalink / raw) To: Paul Walmsley, Hunter, Jon; +Cc: linux-omap, khilman, tony > From: linux-omap-owner@vger.kernel.org [mailto:linux-omap- > owner@vger.kernel.org] On Behalf Of Paul Walmsley > Sent: Wednesday, September 15, 2010 2:15 PM > > This patch fixes this problem by ensuring the branch prediction logic is > > disabled while changing the L3 clock frequency. The branch prediction logic > > is disabled by clearing the Z-bit in the ARM AUX CTRL register. Small correction, Z bit is in CR register. AUX CTRL figures in with the ASA feature. > Really nice changelog. I wish every patch had a description this good. > Patch looks really good, too. Queued for 2.6.37. It is system specific if this change is required. It is probably safer to have it than not. If the AUX CTRL register has the ASA bit/feature active to allow speculative accesses to propagate past the L2 boundary the Z bit should be cleared as in the patch. However, if ASA bit is not activated then Z bit clearing should not be necessary as speculation will be squashed if there is no L2 hit (so no DDR request will be generated). It is not recommended to enable ASA bit as it is known to cause some issues on EMU/HS devices. It was also projected as loosing more than it gained across some benchmarks. Early boot loaders used to set the ASA. It was removed long back. Some kernels kept the value and opened up the lockup window. I don't recall the linux-omap open kernel having the issue. Some vendor ones did over time. Regards, Richard W. ^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH] omap3: Prevent SDRC deadlock when L3 is changing frequency 2010-09-16 6:05 ` Woodruff, Richard @ 2010-09-16 6:24 ` Shilimkar, Santosh 2010-09-17 18:30 ` Paul Walmsley 0 siblings, 1 reply; 13+ messages in thread From: Shilimkar, Santosh @ 2010-09-16 6:24 UTC (permalink / raw) To: Woodruff, Richard, Paul Walmsley, Hunter, Jon; +Cc: linux-omap, khilman, tony [-- Attachment #1: Type: text/plain, Size: 5115 bytes --] > -----Original Message----- > From: linux-omap-owner@vger.kernel.org [mailto:linux-omap- > owner@vger.kernel.org] On Behalf Of Woodruff, Richard > Sent: Thursday, September 16, 2010 11:36 AM > To: Paul Walmsley; Hunter, Jon > Cc: linux-omap; khilman@deeprootsystems.com; tony@atomide.com > Subject: RE: [PATCH] omap3: Prevent SDRC deadlock when L3 is changing > frequency > > > > From: linux-omap-owner@vger.kernel.org [mailto:linux-omap- > > owner@vger.kernel.org] On Behalf Of Paul Walmsley > > Sent: Wednesday, September 15, 2010 2:15 PM > > > > This patch fixes this problem by ensuring the branch prediction logic > is > > > disabled while changing the L3 clock frequency. The branch prediction > logic > > > is disabled by clearing the Z-bit in the ARM AUX CTRL register. > > Small correction, Z bit is in CR register. AUX CTRL figures in with the > ASA feature. > > > Really nice changelog. I wish every patch had a description this good. > > Patch looks really good, too. Queued for 2.6.37. > > It is system specific if this change is required. It is probably safer to > have it than not. > > If the AUX CTRL register has the ASA bit/feature active to allow > speculative accesses to propagate past the L2 boundary the Z bit should be > cleared as in the patch. > > However, if ASA bit is not activated then Z bit clearing should not be > necessary as speculation will be squashed if there is no L2 hit (so no DDR > request will be generated). > > It is not recommended to enable ASA bit as it is known to cause some > issues on EMU/HS devices. It was also projected as loosing more than it > gained across some benchmarks. > > Early boot loaders used to set the ASA. It was removed long back. Some > kernels kept the value and opened up the lockup window. I don't recall > the linux-omap open kernel having the issue. Some vendor ones did over > time. > The code seems to be correct but just the description has typo. The code is using control register. I just corrected the description and white space issue. Here is updated patch. Paul, You can use this version if you like >From fd4250671a1ae8deb718ac3688ea8971df7524cf Mon Sep 17 00:00:00 2001 From: Jon Hunter <jon-hunter@ti.com> Date: Thu, 16 Sep 2010 12:03:23 +0530 Subject: [PATCH] omap3: Prevent SDRC deadlock when L3 is changing frequency When changing the L3 clock frequency, the CPU is executing from internal RAM and the SDRC clock is disabled. During this time accesses made to external DDR are stalled. If the ARM subsystem attempts to access the DDR while the SDRC clock is disabled this will stall the CPU until the access to the SDRC timeouts. A timeout on the SDRC should never occur. Once a timeout occurs all the following accesses will be aborted and the DDR is no longer accessible. Although the code being executed in the internal RAM does not directly access the DDR, it was found that the branch prediction logic in the CPU may cause the CPU to prefetch code from a DDR location while the SDRC clock is disabled. This was causing an SDRC timeout which resulted in a system hang. This patch fixes this problem by ensuring the branch prediction logic is disabled while changing the L3 clock frequency. The branch prediction logic is disabled by clearing the Z-bit in the ARM CTRL register. Disabling the branch prediction logic does not have any noticable impact on the execution time of this code section. The hardware observability signals were used to monitor the sdrc idle time with and without this patch when operating at different CPU frequencies (150MHz, 500MHz and 600MHz) and the total sdrc idle time when changing frequenct was in the range of 9-11us. This was measured on an omap3430 SDP running the omapzoom p-android-omap-2.6.29 branch. Signed-off-by: Jon Hunter <jon-hunter@ti.com> --- arch/arm/mach-omap2/sram34xx.S | 6 +++++- 1 files changed, 5 insertions(+), 1 deletions(-) diff --git a/arch/arm/mach-omap2/sram34xx.S b/arch/arm/mach-omap2/sram34xx.S index de99ba2..3637274 100644 --- a/arch/arm/mach-omap2/sram34xx.S +++ b/arch/arm/mach-omap2/sram34xx.S @@ -129,8 +129,11 @@ ENTRY(omap3_sram_configure_core_dpll) ldr r4, [sp, #80] str r4, omap_sdrc_mr_1_val skip_cs1_params: + mrc p15, 0, r8, c1, c0, 0 @ read ctrl register + bic r10, r8, #0x800 @ clear Z-bit, disable branch prediction + mcr p15, 0, r10, c1, c0, 0 @ write ctrl register dsb @ flush buffered writes to interconnect - + isb @ prevent speculative exec past here cmp r3, #1 @ if increasing SDRC clk rate, bleq configure_sdrc @ program the SDRC regs early (for RFR) cmp r1, #SDRC_UNLOCK_DLL @ set the intended DLL state @@ -148,6 +151,7 @@ skip_cs1_params: beq return_to_sdram @ return to SDRAM code, otherwise, bl configure_sdrc @ reprogram SDRC regs now return_to_sdram: + mcr p15, 0, r8, c1, c0, 0 @ restore ctrl register isb @ prevent speculative exec past here mov r0, #0 @ return value ldmfd sp!, {r1-r12, pc} @ restore regs and return -- 1.6.0.4 [-- Attachment #2: 0001-omap3-Prevent-SDRC-deadlock-when-L3-is-changing-fre.patch --] [-- Type: application/octet-stream, Size: 2892 bytes --] From fd4250671a1ae8deb718ac3688ea8971df7524cf Mon Sep 17 00:00:00 2001 From: Jon Hunter <jon-hunter@ti.com> Date: Thu, 16 Sep 2010 12:03:23 +0530 Subject: [PATCH] omap3: Prevent SDRC deadlock when L3 is changing frequency When changing the L3 clock frequency, the CPU is executing from internal RAM and the SDRC clock is disabled. During this time accesses made to external DDR are stalled. If the ARM subsystem attempts to access the DDR while the SDRC clock is disabled this will stall the CPU until the access to the SDRC timeouts. A timeout on the SDRC should never occur. Once a timeout occurs all the following accesses will be aborted and the DDR is no longer accessible. Although the code being executed in the internal RAM does not directly access the DDR, it was found that the branch prediction logic in the CPU may cause the CPU to prefetch code from a DDR location while the SDRC clock is disabled. This was causing an SDRC timeout which resulted in a system hang. This patch fixes this problem by ensuring the branch prediction logic is disabled while changing the L3 clock frequency. The branch prediction logic is disabled by clearing the Z-bit in the ARM CTRL register. Disabling the branch prediction logic does not have any noticable impact on the execution time of this code section. The hardware observability signals were used to monitor the sdrc idle time with and without this patch when operating at different CPU frequencies (150MHz, 500MHz and 600MHz) and the total sdrc idle time when changing frequenct was in the range of 9-11us. This was measured on an omap3430 SDP running the omapzoom p-android-omap-2.6.29 branch. Signed-off-by: Jon Hunter <jon-hunter@ti.com> --- arch/arm/mach-omap2/sram34xx.S | 6 +++++- 1 files changed, 5 insertions(+), 1 deletions(-) diff --git a/arch/arm/mach-omap2/sram34xx.S b/arch/arm/mach-omap2/sram34xx.S index de99ba2..3637274 100644 --- a/arch/arm/mach-omap2/sram34xx.S +++ b/arch/arm/mach-omap2/sram34xx.S @@ -129,8 +129,11 @@ ENTRY(omap3_sram_configure_core_dpll) ldr r4, [sp, #80] str r4, omap_sdrc_mr_1_val skip_cs1_params: + mrc p15, 0, r8, c1, c0, 0 @ read ctrl register + bic r10, r8, #0x800 @ clear Z-bit, disable branch prediction + mcr p15, 0, r10, c1, c0, 0 @ write ctrl register dsb @ flush buffered writes to interconnect - + isb @ prevent speculative exec past here cmp r3, #1 @ if increasing SDRC clk rate, bleq configure_sdrc @ program the SDRC regs early (for RFR) cmp r1, #SDRC_UNLOCK_DLL @ set the intended DLL state @@ -148,6 +151,7 @@ skip_cs1_params: beq return_to_sdram @ return to SDRAM code, otherwise, bl configure_sdrc @ reprogram SDRC regs now return_to_sdram: + mcr p15, 0, r8, c1, c0, 0 @ restore ctrl register isb @ prevent speculative exec past here mov r0, #0 @ return value ldmfd sp!, {r1-r12, pc} @ restore regs and return -- 1.6.0.4 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* RE: [PATCH] omap3: Prevent SDRC deadlock when L3 is changing frequency 2010-09-16 6:24 ` Shilimkar, Santosh @ 2010-09-17 18:30 ` Paul Walmsley 0 siblings, 0 replies; 13+ messages in thread From: Paul Walmsley @ 2010-09-17 18:30 UTC (permalink / raw) To: Shilimkar, Santosh Cc: Woodruff, Richard, Hunter, Jon, linux-omap, khilman, tony Hi Richard, Santosh, On Thu, 16 Sep 2010, Shilimkar, Santosh wrote: > > -----Original Message----- > > From: linux-omap-owner@vger.kernel.org [mailto:linux-omap- > > owner@vger.kernel.org] On Behalf Of Woodruff, Richard > > Sent: Thursday, September 16, 2010 11:36 AM > > To: Paul Walmsley; Hunter, Jon > > Cc: linux-omap; khilman@deeprootsystems.com; tony@atomide.com > > Subject: RE: [PATCH] omap3: Prevent SDRC deadlock when L3 is changing > > frequency > > > > > > > From: linux-omap-owner@vger.kernel.org [mailto:linux-omap- > > > owner@vger.kernel.org] On Behalf Of Paul Walmsley > > > Sent: Wednesday, September 15, 2010 2:15 PM > > > > > > This patch fixes this problem by ensuring the branch prediction logic > > is > > > > disabled while changing the L3 clock frequency. The branch prediction > > logic > > > > is disabled by clearing the Z-bit in the ARM AUX CTRL register. > > > > Small correction, Z bit is in CR register. AUX CTRL figures in with the > > ASA feature. > > > > > Really nice changelog. I wish every patch had a description this good. > > > Patch looks really good, too. Queued for 2.6.37. > > > > It is system specific if this change is required. It is probably safer to > > have it than not. > > > > If the AUX CTRL register has the ASA bit/feature active to allow > > speculative accesses to propagate past the L2 boundary the Z bit should be > > cleared as in the patch. > > > > However, if ASA bit is not activated then Z bit clearing should not be > > necessary as speculation will be squashed if there is no L2 hit (so no DDR > > request will be generated). > > > > It is not recommended to enable ASA bit as it is known to cause some > > issues on EMU/HS devices. It was also projected as loosing more than it > > gained across some benchmarks. > > > > Early boot loaders used to set the ASA. It was removed long back. Some > > kernels kept the value and opened up the lockup window. I don't recall > > the linux-omap open kernel having the issue. Some vendor ones did over > > time. > > > The code seems to be correct but just the description has typo. The code > is using control register. I just corrected the description and white > space issue. Here is updated patch. > > Paul, > You can use this version if you like Thanks for the fixes, will update the patch.. - Paul ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] omap3: Prevent SDRC deadlock when L3 is changing frequency 2010-07-21 16:02 [PATCH] omap3: Prevent SDRC deadlock when L3 is changing frequency Jon Hunter 2010-09-15 19:15 ` Paul Walmsley @ 2010-09-15 21:38 ` Paul Walmsley 1 sibling, 0 replies; 13+ messages in thread From: Paul Walmsley @ 2010-09-15 21:38 UTC (permalink / raw) To: Jon Hunter; +Cc: linux-omap Jon, one other brief note: On Wed, 21 Jul 2010, Jon Hunter wrote: > diff --git a/arch/arm/mach-omap2/sram34xx.S b/arch/arm/mach-omap2/sram34xx.S > index de99ba2..e87e730 100644 > --- a/arch/arm/mach-omap2/sram34xx.S > +++ b/arch/arm/mach-omap2/sram34xx.S > @@ -129,8 +129,11 @@ ENTRY(omap3_sram_configure_core_dpll) > ldr r4, [sp, #80] > str r4, omap_sdrc_mr_1_val > skip_cs1_params: > + mrc p15, 0, r8, c1, c0, 0 @ read aux ctrl register > + bic r10, r8, #0x800 @ clear Z-bit, disable branch prediction > + mcr p15, 0, r10, c1, c0, 0 @ write aux ctrl register Please be careful with the whitespace between the opcode and the arguments - I will fix this in the current patch but it seems best to keep this consistent. - Paul ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2010-09-17 18:30 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2010-07-21 16:02 [PATCH] omap3: Prevent SDRC deadlock when L3 is changing frequency Jon Hunter 2010-09-15 19:15 ` Paul Walmsley 2010-09-15 22:40 ` Tony Lindgren 2010-09-15 23:10 ` Paul Walmsley 2010-09-16 4:58 ` Gadiyar, Anand 2010-09-16 18:57 ` Tony Lindgren 2010-09-16 20:23 ` Gadiyar, Anand 2010-09-16 20:49 ` Tony Lindgren 2010-09-17 0:05 ` Woodruff, Richard 2010-09-16 6:05 ` Woodruff, Richard 2010-09-16 6:24 ` Shilimkar, Santosh 2010-09-17 18:30 ` Paul Walmsley 2010-09-15 21:38 ` Paul Walmsley
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.