oprofile and ARM A9 hardware counter

[Jean Pihet]

On Tue, May 8, 2012 at 2:37 PM, Cousson, Benoit <b-cousson at ti.com> wrote:
> Salut Jean,
>
>
> On 5/8/2012 1:23 PM, Jean Pihet wrote:
>>
>> Hi Benoit,
>>
>> On Tue, May 8, 2012 at 1:01 PM, Cousson, Benoit<b-cousson at ti.com>  wrote:
>>>
>>> Hi Kevin&  Jon,
>>>
>>>
>>>
>>> On 5/8/2012 1:28 AM, Kevin Hilman wrote:
>>>>
>>>>
>>>> Jon Hunter<jon-hunter at ti.com>    writes:
>>>>
>>>>> Hi Kevin,
>>>>>
>>>>> On 05/07/2012 12:15 PM, Kevin Hilman wrote:
>>>>>>
>>>>>>
>>>>>> Jon Hunter<jon-hunter at ti.com>    writes:
>>>>>>
>>>>>>> Hi Will,
>>>>>>>
>>>>>>> On 04/26/2012 01:07 PM, Will Deacon wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> On Wed, Apr 04, 2012 at 12:15:24PM +0100, Will Deacon wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On Wed, Apr 04, 2012 at 12:29:49AM +0100, Paul Walmsley wrote:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Part of the problem is that the clockdomain data for the emu_sys
>>>>>>>>>> clockdomain is wrong.  Here's something to try to fix it.  It
>>>>>>>>>> might
>>>>>>>>>> just
>>>>>>>>>> be enough to get it to work.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Hmm, doesn't seem to work but I do see the following in dmesg when
>>>>>>>>> I
>>>>>>>>> try to
>>>>>>>>> use perf:
>>>>>>>>>
>>>>>>>>>  powerdomain: waited too long for powerdomain emu_pwrdm to complete
>>>>>>>>> transition
>>>>>>>>>
>>>>>>>>> which is new with your patch.
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> Sorry to nag, but does anybody have a clue where to go from here? I
>>>>>>>> can
>>>>>>>> start digging in the OMAP PM code, but it's all new territory for
>>>>>>>> me.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> I did a little playing around with this today and I think that I have
>>>>>>> figured out why this was not working (see below). Please can you try
>>>>>>> the
>>>>>>> following patch? I tried this on top of your series for perf/omap4.
>>>>>>>
>>>>>>> Paul, FYI. If this works for Will then I can re-base on top of the
>>>>>>> latest linux-omap and submit to the mailing list.
>>>>>>>
>>>>>>> Also, the above error about the emu_pwrdm is odd too. I noticed that
>>>>>>> the emu_pwrdm is always in the transitioning state. And when I say
>>>>>>> always, I
>>>>>>> mean that even if I check the power domain state while u-boot is
>>>>>>> running it
>>>>>>> is in the transitioning state. So even before the kernel starts.
>>>>>>>
>>>>>>> Cheers
>>>>>>> Jon
>>>>>>>
>>>>>>>  From 9137ff9c1b382232de7443db0b51b7555846fb62 Mon Sep 17 00:00:00
>>>>>>> 2001
>>>>>>> From: Jon Hunter<jon-hunter at ti.com>
>>>>>>> Date: Fri, 4 May 2012 16:48:45 -0500
>>>>>>> Subject: [PATCH] ARM: OMAP4: Disable auto enable for EMU CLKDM
>>>>>>>
>>>>>>> The flag CLKDM_CAN_HWSUP allows the clock-domain to automatically
>>>>>>> transition
>>>>>>> to the enabled and disabled state. This means that as soon as we
>>>>>>> force
>>>>>>> a
>>>>>>> software wake-up on the clock domain, the clock domain will be
>>>>>>> allowed
>>>>>>> to idle
>>>>>>> and put back into the hardware auto state. For the EMU clock domain
>>>>>>> this is not
>>>>>>> what we want. We want to keep the clock domain in the software wakeup
>>>>>>> state
>>>>>>> while the clock domain is being used and put it back in to the
>>>>>>> hardware
>>>>>>> auto
>>>>>>> state when we have finished using the clock domain.
>>>>>>>
>>>>>>> Signed-off-by: Jon Hunter<jon-hunter at ti.com>
>>>>>>
>>>>>>
>>>>>>
>>>>>> With this patch, how is the clkdm ever idled?
>>>>>
>>>>>
>>>>>
>>>>> It does not! Sorry, I was so engrossed with figuring out why the EMU
>>>>> clkdm was being idled as soon as it was enabled, I forgot to check if
>>>>> is
>>>>> ever disabled once we terminated perf :-(
>>>>>
>>>>>> IIUC, your patch will get PMU interrupts working, but similarily to
>>>>>> previous patches in this thread, it works because it *never* allows
>>>>>> the
>>>>>> EMU clkdm to idle.  This is not a mergeable solution because it will
>>>>>> not
>>>>>> allow CORE retention (and thus full-chip retention.)
>>>>>
>>>>>
>>>>>
>>>>> Right!
>>>>>
>>>>>> What we need is a solution that allows the clkdm to idle, and then to
>>>>>> be
>>>>>> reinitialzed when it wakes up.  Due to the way (I understand) resets
>>>>>> in
>>>>>> the debugss, allowing the clkdm to idle will cause a reset, so the
>>>>>> PMU/CTI interrupts need to be reinitialzied after wakeup.
>>>>>
>>>>>
>>>>>
>>>>> Yes exactly I see that now. I have prototyped the 3 patches and this is
>>>>> working AND the EMU clkdm does go back to idle. I can send out to the
>>>>> list for review.
>>>>
>>>>
>>>>
>>>> Perfect, thanks.
>>>>
>>>>>> Kevin
>>>>>>
>>>>>> P.S. Please note there is also already a different fix in mainline for
>>>>>> the EMU clkdm data from Paul which adds the force wakeup flag and
>>>>>> removes the DISABLE_AUTO flag[1] (but leaves the ENABLE_AUTO flag,
>>>>>> because the hardware is capable.)
>>>>>
>>>>>
>>>>>
>>>>> Hmmm ... yes saw this, and you will have to excuse me as I don't fully
>>>>> follow the logic here. In fact, I am thinking we want the opposite ;-)
>>>>>
>>>>>  From looking, into this it seems to me that when PMU is running we
>>>>> want
>>>>> the EMU clock domain in software-wakeup state and when PMU is not
>>>>> running we want in the hardware auto state.
>>>>
>>>>
>>>>
>>>> So far, I'm with you.
>>>>
>>>>> By keeping the ENABLE_AUTO flag set, as soon as we enable the clock
>>>>> domain it is put right back into the HW_AUTO state
>>>>
>>>>
>>>>
>>>> This is only because it was in the HWSUP state when _enable was called.
>>>> If clkdm_deny_idle() is used, that behavior will change.
>>>>
>>>>> and hence PMU is
>>>>> not working (see _enable() function in
>>>>> arch/arm/mach-omap2/omap_hwmod.c)
>>>>>
>>>>> So really what I think we want is to remove the ENABLE_AUTO flag to
>>>>> keep
>>>>> the clock domain in software wake-up and use the DISABLE_AUTO flag to
>>>>> put the clock domain back in HW_AUTO (note this requires a patch to
>>>>> perform this 2nd part).
>>>>
>>>>
>>>>
>>>> Well, Paul will have to comment here for the final word, but IIUC, the
>>>> hwmod flags are supposed to indicate only what the HW is capable of.  If
>>>> we want to change the runtime behavior, we nee to use (or add) APIs to
>>>> change the beahvior.  In this case, clkdm_allow_idle(),
>>>> clkdm_deny_idle() are probably what is needed here.
>>>
>>>
>>>
>>> Yes, indeed, we should not hack the flags to fix that kind of issue. The
>>> flags describe what the HW is capable of, and the EMU CD can support
>>> HW_AUTO
>>> and SW_WAKEUP. AFAIK, the issue with that EMU CD is that the only valid
>>> next
>>> power state is OFF, meaning that no retention mode is supported. So any
>>> transition to idle will go to OFF and lead to a reset upon wakeup.
>>>
>>> That being said, being able to transition to OFF during idle is a
>>> perfectly
>>> valid state for most powerdomain in OMAP anyway, so we should be able to
>>> restore from OFF mode smoothly.
>>>
>>> It looks to me that what is missing here is *just* a restore path in the
>>> PMU/CTI code. But I'm probably missing some nasty details in this issue
>>> :-)
>>
>> Although it is perfectly feasible to have a seamless transition of the
>> EMU power domain, I think the PMU counters will not be accurate
>> anymore since they stop counting events when going to OFF and re-start
>> after the state restore.
>
>
> Good point, but I think the PMU might still works fine because located
> inside the MPU domain. AFAIR, only the path to access the PMU and the CTI is
> going to OFF and thus will be reset.
Ever better point ;p Thanks for the precision.
In any case my point was: can we allow the EMU CD to go OFF or prevent
it from doing so? We need to answer that question first.

> But that whole DEBUGSS is such a nightmare, that I'm not 100% sure about
> that :-)


>
> Regards,
> Benoit
>

Jean