Re: [PATCH V2 01/10] ARM: PMU: Add runtime PM Support
On 08/01/2012 03:47 PM, Will Deacon wrote: > On Wed, Aug 01, 2012 at 12:07:16AM +0100, Jon Hunter wrote: >> I have just updated my pmu branch for omap3/4. You can pull my latest >> patches from [1]. > > Great, thanks for that. I've pushed out to perf/omap4 and I've also > included your runtime PM hooks in my perf/updates branch. I have a fair > amount of cleanup sitting in there as well, so I'll post that to the list > at -rc1 as a candidate for -next. I'll obviously leave the omap-specific > bits for others to handle, although I'll continue maintaining my branch > until that's hit mainline. I just updated today with Paul's latest patch he sent. I don't think much changed from what I had in there yesterday. >> In the latest series I have: >> 1. Pulled in an omap3 clock domain fix from Paul [2] >> 2. Rebased Paul's patch [3] on top of [2]. I have also made a couple >>updates to this patch and I need to get Paul's feedback. >> 3. I have removed the OMAP3 PMU dependency on ETM from the Kconfig. > > Fantastic! I actually already removed the OMAP3 Kconfig dependency in my > perf/updates branch as part of removing CPU_HAS_PMU entirely, so you > probably don't need to pursue that patch. Great! Glad I don't have to worry about that. >> I will probably send out the series once I align with Paul on the above >> changes and the HWMOD stuff I have added for OMAP3 with Benoit. > > Okey doke. If you need me to update my copy of your runtime PM patch, > please shout (that seems to be a done deal afaict). Yes it is good as far as I am concerned. I am going to be out for a couple weeks, but when I get back I will get the OMAP stuff out. I should probably doing some testing on -next too with your latest changes. Cheers Jon -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 01/10] ARM: PMU: Add runtime PM Support
On Wed, Aug 01, 2012 at 12:07:16AM +0100, Jon Hunter wrote: > I have just updated my pmu branch for omap3/4. You can pull my latest > patches from [1]. Great, thanks for that. I've pushed out to perf/omap4 and I've also included your runtime PM hooks in my perf/updates branch. I have a fair amount of cleanup sitting in there as well, so I'll post that to the list at -rc1 as a candidate for -next. I'll obviously leave the omap-specific bits for others to handle, although I'll continue maintaining my branch until that's hit mainline. > In the latest series I have: > 1. Pulled in an omap3 clock domain fix from Paul [2] > 2. Rebased Paul's patch [3] on top of [2]. I have also made a couple >updates to this patch and I need to get Paul's feedback. > 3. I have removed the OMAP3 PMU dependency on ETM from the Kconfig. Fantastic! I actually already removed the OMAP3 Kconfig dependency in my perf/updates branch as part of removing CPU_HAS_PMU entirely, so you probably don't need to pursue that patch. > I will probably send out the series once I align with Paul on the above > changes and the HWMOD stuff I have added for OMAP3 with Benoit. Okey doke. If you need me to update my copy of your runtime PM patch, please shout (that seems to be a done deal afaict). Will -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 01/10] ARM: PMU: Add runtime PM Support
Hi Will, On 07/31/2012 10:14 AM, Will Deacon wrote: > On Thu, Jul 26, 2012 at 04:16:15PM +0100, Jon Hunter wrote: >> On 07/26/2012 10:05 AM, Will Deacon wrote: >>> Ok, thanks for updating me. I've pushed the patches out onto my >>> perf/omap4-dev branch as that seems to be a good place to collate the >>> current state of things. I've not tried doing anything with it yet though. >> >> Ok, great. Let me know if you have any problems. I have ran some initial >> testing on omap3430, omap4430 and omap4460 and it seems ok (AFAICT). > > Just an FYI that I gave up maintaining the old perf/omap4 branch and instead > moved everything from perf/omap4-dev onto that as the rebase was getting > painful. I have just updated my pmu branch for omap3/4. You can pull my latest patches from [1]. In the latest series I have: 1. Pulled in an omap3 clock domain fix from Paul [2] 2. Rebased Paul's patch [3] on top of [2]. I have also made a couple updates to this patch and I need to get Paul's feedback. 3. I have removed the OMAP3 PMU dependency on ETM from the Kconfig. I have tested this on OMAP3430, OMAP4430 and OMAP4460. Seems to be working well my end so let me know if you have chance to try. I will probably send out the series once I align with Paul on the above changes and the HWMOD stuff I have added for OMAP3 with Benoit. Cheers Jon [1] g...@gitorious.org:linux-omap-dev/linux-omap-dev.git -b dev-pmu [2] http://marc.info/?l=linux-omap&m=134333691309305&w=2 [3] http://marc.info/?l=linux-omap&m=134212809112530&w=2 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 01/10] ARM: PMU: Add runtime PM Support
On Thu, Jul 26, 2012 at 04:16:15PM +0100, Jon Hunter wrote: > On 07/26/2012 10:05 AM, Will Deacon wrote: > > Ok, thanks for updating me. I've pushed the patches out onto my > > perf/omap4-dev branch as that seems to be a good place to collate the > > current state of things. I've not tried doing anything with it yet though. > > Ok, great. Let me know if you have any problems. I have ran some initial > testing on omap3430, omap4430 and omap4460 and it seems ok (AFAICT). Just an FYI that I gave up maintaining the old perf/omap4 branch and instead moved everything from perf/omap4-dev onto that as the rebase was getting painful. Will -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 01/10] ARM: PMU: Add runtime PM Support
On 07/26/2012 10:05 AM, Will Deacon wrote: > On Thu, Jul 26, 2012 at 01:41:23AM +0100, Jon Hunter wrote: >> Hi Will, > > Hi Jon, > >> On 07/05/2012 07:40 PM, Jon Hunter wrote: >> I just wanted to let you know that I have been updating the PMU patches >> for OMAP. Latest can be found here [1]. I have not included Paul's patch >> [2] in this series at the moment, because although it works well for >> OMAP4, I need to create the HWMOD structures for OMAP3. Without these >> structures it breaks OMAP3 PMU support. Once this is done we should be >> able to get rid for the OMAP3 dependency on ETM. Unfortunately, Benoit >> (Mr HWMOD) is out on his holidays but should be back next week and then >> maybe he can give me some guidance on this HWMOD stuff. > > Ok, thanks for updating me. I've pushed the patches out onto my > perf/omap4-dev branch as that seems to be a good place to collate the > current state of things. I've not tried doing anything with it yet though. Ok, great. Let me know if you have any problems. I have ran some initial testing on omap3430, omap4430 and omap4460 and it seems ok (AFAICT). > Do you know when Benoit will be back online? I *believe* Monday. However, I am sure he will have a lot of catching up to do. Anything particular or you just want to get this stuff finalised/done? Cheers Jon -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 01/10] ARM: PMU: Add runtime PM Support
On Thu, Jul 26, 2012 at 01:41:23AM +0100, Jon Hunter wrote: > Hi Will, Hi Jon, > On 07/05/2012 07:40 PM, Jon Hunter wrote: > I just wanted to let you know that I have been updating the PMU patches > for OMAP. Latest can be found here [1]. I have not included Paul's patch > [2] in this series at the moment, because although it works well for > OMAP4, I need to create the HWMOD structures for OMAP3. Without these > structures it breaks OMAP3 PMU support. Once this is done we should be > able to get rid for the OMAP3 dependency on ETM. Unfortunately, Benoit > (Mr HWMOD) is out on his holidays but should be back next week and then > maybe he can give me some guidance on this HWMOD stuff. Ok, thanks for updating me. I've pushed the patches out onto my perf/omap4-dev branch as that seems to be a good place to collate the current state of things. I've not tried doing anything with it yet though. Do you know when Benoit will be back online? Will -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 01/10] ARM: PMU: Add runtime PM Support
Hi Will, On 07/05/2012 07:40 PM, Jon Hunter wrote: > Hi Will, > > On 07/02/2012 05:01 PM, Will Deacon wrote: >> On Mon, Jul 02, 2012 at 05:50:38PM +0100, Jon Hunter wrote: >>> On 07/02/2012 04:55 AM, Will Deacon wrote: Did you have any luck getting to the bottom of this? >>> >>> I am still waiting for feedback from design. They were trying to confirm >>> my observations. Unfortunately, it is taking some time. I will ping them >>> again. >> >> Ok, thanks. If pinging doesn't work, bribery can be quite effective with >> hardware guys :) > > Yes looks like I am going to need to get creative :-) > It would be good to take your PMU suspend/resume patches once we know that they will get used. >>> >>> Yes that would be good. I could drop the 4460 specific changes for now >>> and make 4460 work in the same way as 4430 (using CTI) for the time >>> being and see if we can get these in. However, I recall that was not >>> working for you, but it was working fine for me. >> >> Indeed, that hack didn't help me and I'd rather not commit it if it only >> partially fixes the problem. A better bet might just be to go with your >> original approach and see how many bug reports we receive. That might also >> help us narrow down the problem, but it's your call. > > Ok, I think that that is best. > >> In the meantime, I pushed what I think is the latest drop of your series to >> a new branch on kernel.org: >> >> git://git.kernel.org/pub/scm/linux/kernel/git/will/linux.git perf/omap4-dev > > Looks good. I just wanted to let you know that I have been updating the PMU patches for OMAP. Latest can be found here [1]. I have not included Paul's patch [2] in this series at the moment, because although it works well for OMAP4, I need to create the HWMOD structures for OMAP3. Without these structures it breaks OMAP3 PMU support. Once this is done we should be able to get rid for the OMAP3 dependency on ETM. Unfortunately, Benoit (Mr HWMOD) is out on his holidays but should be back next week and then maybe he can give me some guidance on this HWMOD stuff. Cheers Jon [1] https://gitorious.org/linux-omap-dev dev-pmu [2] http://marc.info/?l=linux-omap&m=134212809112530&w=2 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 01/10] ARM: PMU: Add runtime PM Support
Hi Will, On 07/02/2012 05:01 PM, Will Deacon wrote: > On Mon, Jul 02, 2012 at 05:50:38PM +0100, Jon Hunter wrote: >> On 07/02/2012 04:55 AM, Will Deacon wrote: >>> >>> Did you have any luck getting to the bottom of this? >> >> I am still waiting for feedback from design. They were trying to confirm >> my observations. Unfortunately, it is taking some time. I will ping them >> again. > > Ok, thanks. If pinging doesn't work, bribery can be quite effective with > hardware guys :) Yes looks like I am going to need to get creative :-) >>> It would be good to take your PMU suspend/resume patches once we know that >>> they will get used. >> >> Yes that would be good. I could drop the 4460 specific changes for now >> and make 4460 work in the same way as 4430 (using CTI) for the time >> being and see if we can get these in. However, I recall that was not >> working for you, but it was working fine for me. > > Indeed, that hack didn't help me and I'd rather not commit it if it only > partially fixes the problem. A better bet might just be to go with your > original approach and see how many bug reports we receive. That might also > help us narrow down the problem, but it's your call. Ok, I think that that is best. > In the meantime, I pushed what I think is the latest drop of your series to > a new branch on kernel.org: > > git://git.kernel.org/pub/scm/linux/kernel/git/will/linux.git perf/omap4-dev Looks good. Cheers Jon -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 01/10] ARM: PMU: Add runtime PM Support
On Mon, Jul 02, 2012 at 05:50:38PM +0100, Jon Hunter wrote: > On 07/02/2012 04:55 AM, Will Deacon wrote: > > > > Did you have any luck getting to the bottom of this? > > I am still waiting for feedback from design. They were trying to confirm > my observations. Unfortunately, it is taking some time. I will ping them > again. Ok, thanks. If pinging doesn't work, bribery can be quite effective with hardware guys :) > > It would be good to take your PMU suspend/resume patches once we know that > > they will get used. > > Yes that would be good. I could drop the 4460 specific changes for now > and make 4460 work in the same way as 4430 (using CTI) for the time > being and see if we can get these in. However, I recall that was not > working for you, but it was working fine for me. Indeed, that hack didn't help me and I'd rather not commit it if it only partially fixes the problem. A better bet might just be to go with your original approach and see how many bug reports we receive. That might also help us narrow down the problem, but it's your call. In the meantime, I pushed what I think is the latest drop of your series to a new branch on kernel.org: git://git.kernel.org/pub/scm/linux/kernel/git/will/linux.git perf/omap4-dev Will -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 01/10] ARM: PMU: Add runtime PM Support
Hi Will, On 07/02/2012 04:55 AM, Will Deacon wrote: > Hi Jon, > > Did you have any luck getting to the bottom of this? I am still waiting for feedback from design. They were trying to confirm my observations. Unfortunately, it is taking some time. I will ping them again. > It would be good to take your PMU suspend/resume patches once we know that > they will get used. Yes that would be good. I could drop the 4460 specific changes for now and make 4460 work in the same way as 4430 (using CTI) for the time being and see if we can get these in. However, I recall that was not working for you, but it was working fine for me. > On Tue, Jun 12, 2012 at 11:41:27PM +0100, Jon Hunter wrote: >> On 06/12/2012 04:31 PM, Will Deacon wrote: >>> That's understandable -- one of the CPUs is likely more loaded than the >>> other. However, I'd like to confirm whether or not you see what I see. With >>> the 4430_init hack on a 4460, if I run: >>> >>> # taskset 0x2 perf top >>> >>> then I get no samples. If I do: >>> >>> # taskset 0x1 perf top >>> >>> then I *do* get samples and from *both* CPUs. So it smells more like an >>> issue poking some configuration registers from CPU1 rather than the IRQ >>> path being broken. As I said before, if I don't do the extra init hack >>> then I don't get this problem (but event counters don't tick). >> >> In both cases, I see interrupts on both CPUs. However, typically more on >> the CPU that perf is running on (which is probably to be expected). And >> I confirm that the only change I made was ... > > [...] > >> When you boot the kernel what 4460 rev does it show (very early in the >> kernel boot log)? Mine shows ... >> >> [0.00] OMAP4460 ES1.1 > > Snap: [0.00] OMAP4460 ES1.1 Ok. >> However, the A9 version has not changed between ES1.0 and ES1.1. Both >> should be r2p10. > > Yup, that's what /proc/cpuinfo says. Hmmm ... so that does not explain the observation that you made with 4460. Cheers Jon -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 01/10] ARM: PMU: Add runtime PM Support
Hi Jon, Did you have any luck getting to the bottom of this? It would be good to take your PMU suspend/resume patches once we know that they will get used. On Tue, Jun 12, 2012 at 11:41:27PM +0100, Jon Hunter wrote: > On 06/12/2012 04:31 PM, Will Deacon wrote: > > That's understandable -- one of the CPUs is likely more loaded than the > > other. However, I'd like to confirm whether or not you see what I see. With > > the 4430_init hack on a 4460, if I run: > > > > # taskset 0x2 perf top > > > > then I get no samples. If I do: > > > > # taskset 0x1 perf top > > > > then I *do* get samples and from *both* CPUs. So it smells more like an > > issue poking some configuration registers from CPU1 rather than the IRQ > > path being broken. As I said before, if I don't do the extra init hack > > then I don't get this problem (but event counters don't tick). > > In both cases, I see interrupts on both CPUs. However, typically more on > the CPU that perf is running on (which is probably to be expected). And > I confirm that the only change I made was ... [...] > When you boot the kernel what 4460 rev does it show (very early in the > kernel boot log)? Mine shows ... > > [0.00] OMAP4460 ES1.1 Snap: [0.00] OMAP4460 ES1.1 > However, the A9 version has not changed between ES1.0 and ES1.1. Both > should be r2p10. Yup, that's what /proc/cpuinfo says. Will -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 01/10] ARM: PMU: Add runtime PM Support
On 06/12/2012 04:31 PM, Will Deacon wrote: > On Tue, Jun 12, 2012 at 10:17:16PM +0100, Jon Hunter wrote: >> Hi Will, > > Hi Jon, > >> On 06/12/2012 04:28 AM, Will Deacon wrote: >>> >>> Well, I tried that and the results are pretty whacky: the event counters do >>> indeed tick but interrupts only fire if I pin the perf task to CPU1! What's >>> more, the interrupts do fire on both cores when they're working... >> >> I tried this, and I see that interrupts occur on both, however, it seems >> that the majority occur on one CPU and only a few on the other. So it >> does appear that one CPU is getting a lot more interrupts. > > That's understandable -- one of the CPUs is likely more loaded than the > other. However, I'd like to confirm whether or not you see what I see. With > the 4430_init hack on a 4460, if I run: > > # taskset 0x2 perf top > > then I get no samples. If I do: > > # taskset 0x1 perf top > > then I *do* get samples and from *both* CPUs. So it smells more like an > issue poking some configuration registers from CPU1 rather than the IRQ > path being broken. As I said before, if I don't do the extra init hack > then I don't get this problem (but event counters don't tick). In both cases, I see interrupts on both CPUs. However, typically more on the CPU that perf is running on (which is probably to be expected). And I confirm that the only change I made was ... diff --git a/arch/arm/mach-omap2/pmu.c b/arch/arm/mach-omap2/pmu.c index f90d958..042881b 100644 --- a/arch/arm/mach-omap2/pmu.c +++ b/arch/arm/mach-omap2/pmu.c @@ -286,7 +286,7 @@ static int __init omap_init_pmu(void) * interrupts and so the CTI IRQs are used and this requires additional * sub-systems to be enabled. */ - if (cpu_is_omap443x()) + if (cpu_is_omap44xx()) r = omap4430_init_pmu(); else When you boot the kernel what 4460 rev does it show (very early in the kernel boot log)? Mine shows ... [0.00] OMAP4460 ES1.1 However, the A9 version has not changed between ES1.0 and ES1.1. Both should be r2p10. >> From a PMU programming standpoint, if we just use "perf top" are the >> event counters not used/programmed? > > Just using perf top should use the cycle counter as the event source. Ok, so no event counters are used. >> And when we use "perf top -e instructions" is it the "software >> increment" event that the event counter(s) are monitoring? I am just >> trying to understand how the counters are being programmed and then I >> can ask the design folks an intelligent question :-) > > It depends on the CPU. For Cortex-A9, `instructions' maps to event 0x68, > which isn't a perfect match. If you want to specify a hex value for the > event code, you can do: > > # perf top -e rNN > > where NN is the hex event number. On A9, r11 would give you cycles via > an event counter. Ok, thanks. >> By the way, I don't suppose there is any debugfs entry to dump the PMU >> registers? > > 'fraid not, but there is some debug code in perf_event_v7.c that you > could call if you wanted to (just #define DEBUG at the top of the file). Thanks Jon -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 01/10] ARM: PMU: Add runtime PM Support
On Tue, Jun 12, 2012 at 10:17:16PM +0100, Jon Hunter wrote: > Hi Will, Hi Jon, > On 06/12/2012 04:28 AM, Will Deacon wrote: > > > > Well, I tried that and the results are pretty whacky: the event counters do > > indeed tick but interrupts only fire if I pin the perf task to CPU1! What's > > more, the interrupts do fire on both cores when they're working... > > I tried this, and I see that interrupts occur on both, however, it seems > that the majority occur on one CPU and only a few on the other. So it > does appear that one CPU is getting a lot more interrupts. That's understandable -- one of the CPUs is likely more loaded than the other. However, I'd like to confirm whether or not you see what I see. With the 4430_init hack on a 4460, if I run: # taskset 0x2 perf top then I get no samples. If I do: # taskset 0x1 perf top then I *do* get samples and from *both* CPUs. So it smells more like an issue poking some configuration registers from CPU1 rather than the IRQ path being broken. As I said before, if I don't do the extra init hack then I don't get this problem (but event counters don't tick). > From a PMU programming standpoint, if we just use "perf top" are the > event counters not used/programmed? Just using perf top should use the cycle counter as the event source. > And when we use "perf top -e instructions" is it the "software > increment" event that the event counter(s) are monitoring? I am just > trying to understand how the counters are being programmed and then I > can ask the design folks an intelligent question :-) It depends on the CPU. For Cortex-A9, `instructions' maps to event 0x68, which isn't a perfect match. If you want to specify a hex value for the event code, you can do: # perf top -e rNN where NN is the hex event number. On A9, r11 would give you cycles via an event counter. > By the way, I don't suppose there is any debugfs entry to dump the PMU > registers? 'fraid not, but there is some debug code in perf_event_v7.c that you could call if you wanted to (just #define DEBUG at the top of the file). Will -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 01/10] ARM: PMU: Add runtime PM Support
Hi Will, On 06/12/2012 04:28 AM, Will Deacon wrote: > On Mon, Jun 11, 2012 at 08:01:23PM +0100, Jon Hunter wrote: >> Hi Will, > > Hello, > >> On 06/11/2012 12:39 PM, Will Deacon wrote: >>> This looks better to me, so I took it for a spin on my 4460 (thanks >>> Nicolas!) >>> and noticed that only the cycle counter seems to tick -- the event counters >>> always return 0 deltas (that is, they don't increment). Booting the same SD >>> card on a 4430 (same MLO, u-boot, kernel and filesystem) I see that the >>> event counters function correctly there. >> >> Thanks for the feedback. Being somewhat new to PMU, I was mainly using >> PERF to test and verify that with "perf top" I was seeing interrupts. >> How do I check what the event counters are returning? Any perf tests I >> could use? > > You can continue to use perf top, just specify an event other than cycles: > > # perf top -e instructions > > for example. You can also use perf stat, but that probably won't be > triggering irqs. > >> By the way, as a quick test you could modify the code in omap_init_pmu() >> to call omap4430_init_pmu() for all omap4 devices as follows ... >> >> if (cpu_is_omap44xx()) >> return omap4430_init_pmu(); >> >> I was hoping for 4460/70 we would not need to keep the debugss and other >> domains on and hence, I called the above function omap4430_init_pmu(). >> However this function works for all omap4 devices, it just turns on more >> power domains. > > Well, I tried that and the results are pretty whacky: the event counters do > indeed tick but interrupts only fire if I pin the perf task to CPU1! What's > more, the interrupts do fire on both cores when they're working... I tried this, and I see that interrupts occur on both, however, it seems that the majority occur on one CPU and only a few on the other. So it does appear that one CPU is getting a lot more interrupts. > Without the above change, I can generate cycle counter interrupts regardless > of which CPU I run execute perf. Yes, I see this to. From more testing, I see that as soon as I turn off the debugss clock domain "perf top -e instructions" stops working. So I assume that the event counters are returning 0 in this case. >From a PMU programming standpoint, if we just use "perf top" are the event counters not used/programmed? And when we use "perf top -e instructions" is it the "software increment" event that the event counter(s) are monitoring? I am just trying to understand how the counters are being programmed and then I can ask the design folks an intelligent question :-) By the way, I don't suppose there is any debugfs entry to dump the PMU registers? >>> It also seems that we can remove the dependency on CONFIG_OMAP3_EMU with >>> these >>> patches but I don't have any OMAP3 hardware to check if we get any >>> regressions >>> on older platforms. Do your patches only deal with OMAP4? >> >> It *should* work for all omap2+. So far I have tested an omap3 beagle >> but I have not tested an omap2 device. Again the extent of my testing >> was to run "perf top" and verify interrupts we being generated. I >> realise that this may not be sufficient and so if you have a more >> exhaustive test you recommend let me know. > > Well, try the above as well as what you're currently doing and that should > test the basics. If that works, I'll happily drop the Kconfig dependency on > OMAP3_EMU (which has been a regular source of confusion). I still think that there is something I need to understand better here. Cheers Jon -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 01/10] ARM: PMU: Add runtime PM Support
On Mon, Jun 11, 2012 at 08:01:23PM +0100, Jon Hunter wrote: > Hi Will, Hello, > On 06/11/2012 12:39 PM, Will Deacon wrote: > > This looks better to me, so I took it for a spin on my 4460 (thanks > > Nicolas!) > > and noticed that only the cycle counter seems to tick -- the event counters > > always return 0 deltas (that is, they don't increment). Booting the same SD > > card on a 4430 (same MLO, u-boot, kernel and filesystem) I see that the > > event counters function correctly there. > > Thanks for the feedback. Being somewhat new to PMU, I was mainly using > PERF to test and verify that with "perf top" I was seeing interrupts. > How do I check what the event counters are returning? Any perf tests I > could use? You can continue to use perf top, just specify an event other than cycles: # perf top -e instructions for example. You can also use perf stat, but that probably won't be triggering irqs. > By the way, as a quick test you could modify the code in omap_init_pmu() > to call omap4430_init_pmu() for all omap4 devices as follows ... > > if (cpu_is_omap44xx()) > return omap4430_init_pmu(); > > I was hoping for 4460/70 we would not need to keep the debugss and other > domains on and hence, I called the above function omap4430_init_pmu(). > However this function works for all omap4 devices, it just turns on more > power domains. Well, I tried that and the results are pretty whacky: the event counters do indeed tick but interrupts only fire if I pin the perf task to CPU1! What's more, the interrupts do fire on both cores when they're working... Without the above change, I can generate cycle counter interrupts regardless of which CPU I run execute perf. > > It also seems that we can remove the dependency on CONFIG_OMAP3_EMU with > > these > > patches but I don't have any OMAP3 hardware to check if we get any > > regressions > > on older platforms. Do your patches only deal with OMAP4? > > It *should* work for all omap2+. So far I have tested an omap3 beagle > but I have not tested an omap2 device. Again the extent of my testing > was to run "perf top" and verify interrupts we being generated. I > realise that this may not be sufficient and so if you have a more > exhaustive test you recommend let me know. Well, try the above as well as what you're currently doing and that should test the basics. If that works, I'll happily drop the Kconfig dependency on OMAP3_EMU (which has been a regular source of confusion). Cheers, Will -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 01/10] ARM: PMU: Add runtime PM Support
Hi Will, On 06/11/2012 12:39 PM, Will Deacon wrote: > On Fri, Jun 08, 2012 at 04:24:32PM +0100, Jon Hunter wrote: >> Hi Will, > > Hi Jon, > >> Here is an updated version. I was going to send out a V3, but I wanted >> to wait to see if others had more comments first. > > This looks better to me, so I took it for a spin on my 4460 (thanks Nicolas!) > and noticed that only the cycle counter seems to tick -- the event counters > always return 0 deltas (that is, they don't increment). Booting the same SD > card on a 4430 (same MLO, u-boot, kernel and filesystem) I see that the > event counters function correctly there. Thanks for the feedback. Being somewhat new to PMU, I was mainly using PERF to test and verify that with "perf top" I was seeing interrupts. How do I check what the event counters are returning? Any perf tests I could use? By the way, as a quick test you could modify the code in omap_init_pmu() to call omap4430_init_pmu() for all omap4 devices as follows ... if (cpu_is_omap44xx()) return omap4430_init_pmu(); I was hoping for 4460/70 we would not need to keep the debugss and other domains on and hence, I called the above function omap4430_init_pmu(). However this function works for all omap4 devices, it just turns on more power domains. > It also seems that we can remove the dependency on CONFIG_OMAP3_EMU with these > patches but I don't have any OMAP3 hardware to check if we get any regressions > on older platforms. Do your patches only deal with OMAP4? It *should* work for all omap2+. So far I have tested an omap3 beagle but I have not tested an omap2 device. Again the extent of my testing was to run "perf top" and verify interrupts we being generated. I realise that this may not be sufficient and so if you have a more exhaustive test you recommend let me know. Cheers Jon -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 01/10] ARM: PMU: Add runtime PM Support
On Fri, Jun 08, 2012 at 04:24:32PM +0100, Jon Hunter wrote: > Hi Will, Hi Jon, > Here is an updated version. I was going to send out a V3, but I wanted > to wait to see if others had more comments first. This looks better to me, so I took it for a spin on my 4460 (thanks Nicolas!) and noticed that only the cycle counter seems to tick -- the event counters always return 0 deltas (that is, they don't increment). Booting the same SD card on a 4430 (same MLO, u-boot, kernel and filesystem) I see that the event counters function correctly there. It also seems that we can remove the dependency on CONFIG_OMAP3_EMU with these patches but I don't have any OMAP3 hardware to check if we get any regressions on older platforms. Do your patches only deal with OMAP4? Cheers, Will -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 01/10] ARM: PMU: Add runtime PM Support
Hi Will,
On 06/08/2012 04:47 AM, Will Deacon wrote:
> Hi Jon,
>
> On Thu, Jun 07, 2012 at 10:22:03PM +0100, Jon Hunter wrote:
>> diff --git a/arch/arm/kernel/perf_event.c b/arch/arm/kernel/perf_event.c
>> index 186c8cb..00adb98 100644
>> --- a/arch/arm/kernel/perf_event.c
>> +++ b/arch/arm/kernel/perf_event.c
>> @@ -20,6 +20,7 @@
>> #include
>> #include
>> #include
>> +#include
>>
>> #include
>> #include
>> @@ -367,8 +368,6 @@ armpmu_release_hardware(struct arm_pmu *armpmu)
>> {
>> int i, irq, irqs;
>> struct platform_device *pmu_device = armpmu->plat_device;
>> -struct arm_pmu_platdata *plat =
>> -dev_get_platdata(&pmu_device->dev);
>>
>> irqs = min(pmu_device->num_resources, num_possible_cpus());
>>
>> @@ -376,14 +375,12 @@ armpmu_release_hardware(struct arm_pmu *armpmu)
>> if (!cpumask_test_and_clear_cpu(i, &armpmu->active_irqs))
>> continue;
>> irq = platform_get_irq(pmu_device, i);
>> -if (irq >= 0) {
>> -if (plat && plat->disable_irq)
>> -plat->disable_irq(irq);
>> +if (irq >= 0)
>> free_irq(irq, armpmu);
>> -}
>> }
>>
>> release_pmu(armpmu->type);
>> +pm_runtime_put_sync(&armpmu->plat_device->dev);
>
> We probably want to suspend the device before releasing it, otherwise we
> could race against somebody else trying to initialise the PMU again.
>
>>
>> static int
>> @@ -403,6 +400,8 @@ armpmu_reserve_hardware(struct arm_pmu *armpmu)
>> return err;
>> }
>>
>> +pm_runtime_get_sync(&armpmu->plat_device->dev);
>
> Better pass &pmu_device->dev instead (similarly in release).
>
>> +
>> plat = dev_get_platdata(&pmu_device->dev);
>> if (plat && plat->handle_irq)
>> handle_irq = armpmu_platform_irq;
>> @@ -440,8 +439,7 @@ armpmu_reserve_hardware(struct arm_pmu *armpmu)
>> irq);
>> armpmu_release_hardware(armpmu);
>> return err;
>> -} else if (plat && plat->enable_irq)
>> -plat->enable_irq(irq);
>> +}
>>
>> cpumask_set_cpu(i, &armpmu->active_irqs);
>> }
>> @@ -584,6 +582,28 @@ static void armpmu_disable(struct pmu *pmu)
>> armpmu->stop();
>> }
>
> There are potential failure paths in the reservation code here where we
> don't `put' the PMU back. Can you move the pm_runtime_get_sync call later in
> the function, or does it have to called before we enable the IRQ line?
>
>> +#ifdef CONFIG_PM_RUNTIME
>> +static int armpmu_runtime_resume(struct device *dev)
>> +{
>> +struct arm_pmu_platdata *plat = dev_get_platdata(dev);
>> +
>> +if (plat->runtime_resume)
>
> I think you need to check plat too since it may be NULL on other platforms.
>
>> +return plat->runtime_resume(dev);
>> +
>> +return 0;
>> +}
>> +
>> +static int armpmu_runtime_suspend(struct device *dev)
>> +{
>> +struct arm_pmu_platdata *plat = dev_get_platdata(dev);
>> +
>> +if (plat->runtime_suspend)
>
> Same here.
Here is an updated version. I was going to send out a V3, but I wanted
to wait to see if others had more comments first.
Cheers
Jon
>From c53dcbe091928d293f14e890c2b38be57692ccca Mon Sep 17 00:00:00 2001
From: Jon Hunter
Date: Thu, 31 May 2012 13:05:20 -0500
Subject: [PATCH] ARM: PMU: Add runtime PM Support
Add runtime PM support to the ARM PMU driver so that devices such as OMAP
supporting dynamic PM can use the platform->runtime_* hooks to initialise
hardware at runtime. Without having these runtime PM hooks in place any
configuration of the PMU hardware would be lost when low power states are
entered and hence would prevent PMU from working.
This change also replaces the PMU platform functions enable_irq and disable_irq
added by Ming Lei with runtime_resume and runtime_suspend funtions. Ming had
added the enable_irq and disable_irq functions as a method to configure the
cross trigger interface on OMAP4 for routing the PMU interrupts. By adding
runtime PM support, we can move the code called by enable_irq and disable_irq
into the runtime PM callbacks runtime_resume and runtime_suspend.
Cc: Ming Lei
Cc: Will Deacon
Cc: Benoit Cousson
Cc: Paul Walmsley
Cc: Kevin Hilman
Signed-off-by: Jon Hunter
---
arch/arm/include/asm/pmu.h | 20
arch/arm/kernel/perf_event.c | 41 +
2 files changed, 45 insertions(+), 16 deletions(-)
diff --git a/arch/arm/include/asm/pmu.h b/arch/arm/include/asm/pmu.h
index 90114fa..77b023b 100644
--- a/arch/arm/include/asm/pmu.h
+++ b/arch/arm/include/asm/pmu.h
@@ -31,18 +31,22 @@ enum arm_pmu_type {
* interrupt and passed the address of the low level handler,
* and can be used to implement any platform specific handling
* before or after calling it.
- * @enable_irq: an optional handler
Re: [PATCH V2 01/10] ARM: PMU: Add runtime PM Support
Hi Will,
On 06/08/2012 04:47 AM, Will Deacon wrote:
> Hi Jon,
>
> On Thu, Jun 07, 2012 at 10:22:03PM +0100, Jon Hunter wrote:
>> diff --git a/arch/arm/kernel/perf_event.c b/arch/arm/kernel/perf_event.c
>> index 186c8cb..00adb98 100644
>> --- a/arch/arm/kernel/perf_event.c
>> +++ b/arch/arm/kernel/perf_event.c
>> @@ -20,6 +20,7 @@
>> #include
>> #include
>> #include
>> +#include
>>
>> #include
>> #include
>> @@ -367,8 +368,6 @@ armpmu_release_hardware(struct arm_pmu *armpmu)
>> {
>> int i, irq, irqs;
>> struct platform_device *pmu_device = armpmu->plat_device;
>> -struct arm_pmu_platdata *plat =
>> -dev_get_platdata(&pmu_device->dev);
>>
>> irqs = min(pmu_device->num_resources, num_possible_cpus());
>>
>> @@ -376,14 +375,12 @@ armpmu_release_hardware(struct arm_pmu *armpmu)
>> if (!cpumask_test_and_clear_cpu(i, &armpmu->active_irqs))
>> continue;
>> irq = platform_get_irq(pmu_device, i);
>> -if (irq >= 0) {
>> -if (plat && plat->disable_irq)
>> -plat->disable_irq(irq);
>> +if (irq >= 0)
>> free_irq(irq, armpmu);
>> -}
>> }
>>
>> release_pmu(armpmu->type);
>> +pm_runtime_put_sync(&armpmu->plat_device->dev);
>
> We probably want to suspend the device before releasing it, otherwise we
> could race against somebody else trying to initialise the PMU again.
Good point, will change that.
>>
>> static int
>> @@ -403,6 +400,8 @@ armpmu_reserve_hardware(struct arm_pmu *armpmu)
>> return err;
>> }
>>
>> +pm_runtime_get_sync(&armpmu->plat_device->dev);
>
> Better pass &pmu_device->dev instead (similarly in release).
Ok.
>> +
>> plat = dev_get_platdata(&pmu_device->dev);
>> if (plat && plat->handle_irq)
>> handle_irq = armpmu_platform_irq;
>> @@ -440,8 +439,7 @@ armpmu_reserve_hardware(struct arm_pmu *armpmu)
>> irq);
>> armpmu_release_hardware(armpmu);
>> return err;
>> -} else if (plat && plat->enable_irq)
>> -plat->enable_irq(irq);
>> +}
>>
>> cpumask_set_cpu(i, &armpmu->active_irqs);
>> }
>> @@ -584,6 +582,28 @@ static void armpmu_disable(struct pmu *pmu)
>> armpmu->stop();
>> }
>
> There are potential failure paths in the reservation code here where we
> don't `put' the PMU back. Can you move the pm_runtime_get_sync call later in
> the function, or does it have to called before we enable the IRQ line?
Yes we can move that. It does not need to be before we enable the IRQ line.
>> +#ifdef CONFIG_PM_RUNTIME
>> +static int armpmu_runtime_resume(struct device *dev)
>> +{
>> +struct arm_pmu_platdata *plat = dev_get_platdata(dev);
>> +
>> +if (plat->runtime_resume)
>
> I think you need to check plat too since it may be NULL on other platforms.
Ah, good point. I will add that.
>> +return plat->runtime_resume(dev);
>> +
>> +return 0;
>> +}
>> +
>> +static int armpmu_runtime_suspend(struct device *dev)
>> +{
>> +struct arm_pmu_platdata *plat = dev_get_platdata(dev);
>> +
>> +if (plat->runtime_suspend)
>
> Same here.
Ok, yes. Thanks for the feedback!
Cheers
Jon
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 01/10] ARM: PMU: Add runtime PM Support
Hi Jon,
On Thu, Jun 07, 2012 at 10:22:03PM +0100, Jon Hunter wrote:
> diff --git a/arch/arm/kernel/perf_event.c b/arch/arm/kernel/perf_event.c
> index 186c8cb..00adb98 100644
> --- a/arch/arm/kernel/perf_event.c
> +++ b/arch/arm/kernel/perf_event.c
> @@ -20,6 +20,7 @@
> #include
> #include
> #include
> +#include
>
> #include
> #include
> @@ -367,8 +368,6 @@ armpmu_release_hardware(struct arm_pmu *armpmu)
> {
> int i, irq, irqs;
> struct platform_device *pmu_device = armpmu->plat_device;
> - struct arm_pmu_platdata *plat =
> - dev_get_platdata(&pmu_device->dev);
>
> irqs = min(pmu_device->num_resources, num_possible_cpus());
>
> @@ -376,14 +375,12 @@ armpmu_release_hardware(struct arm_pmu *armpmu)
> if (!cpumask_test_and_clear_cpu(i, &armpmu->active_irqs))
> continue;
> irq = platform_get_irq(pmu_device, i);
> - if (irq >= 0) {
> - if (plat && plat->disable_irq)
> - plat->disable_irq(irq);
> + if (irq >= 0)
> free_irq(irq, armpmu);
> - }
> }
>
> release_pmu(armpmu->type);
> + pm_runtime_put_sync(&armpmu->plat_device->dev);
We probably want to suspend the device before releasing it, otherwise we
could race against somebody else trying to initialise the PMU again.
>
> static int
> @@ -403,6 +400,8 @@ armpmu_reserve_hardware(struct arm_pmu *armpmu)
> return err;
> }
>
> + pm_runtime_get_sync(&armpmu->plat_device->dev);
Better pass &pmu_device->dev instead (similarly in release).
> +
> plat = dev_get_platdata(&pmu_device->dev);
> if (plat && plat->handle_irq)
> handle_irq = armpmu_platform_irq;
> @@ -440,8 +439,7 @@ armpmu_reserve_hardware(struct arm_pmu *armpmu)
> irq);
> armpmu_release_hardware(armpmu);
> return err;
> - } else if (plat && plat->enable_irq)
> - plat->enable_irq(irq);
> + }
>
> cpumask_set_cpu(i, &armpmu->active_irqs);
> }
> @@ -584,6 +582,28 @@ static void armpmu_disable(struct pmu *pmu)
> armpmu->stop();
> }
There are potential failure paths in the reservation code here where we
don't `put' the PMU back. Can you move the pm_runtime_get_sync call later in
the function, or does it have to called before we enable the IRQ line?
> +#ifdef CONFIG_PM_RUNTIME
> +static int armpmu_runtime_resume(struct device *dev)
> +{
> + struct arm_pmu_platdata *plat = dev_get_platdata(dev);
> +
> + if (plat->runtime_resume)
I think you need to check plat too since it may be NULL on other platforms.
> + return plat->runtime_resume(dev);
> +
> + return 0;
> +}
> +
> +static int armpmu_runtime_suspend(struct device *dev)
> +{
> + struct arm_pmu_platdata *plat = dev_get_platdata(dev);
> +
> + if (plat->runtime_suspend)
Same here.
Will
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
