oprofile and ARM A9 hardware counter

[Kevin Hilman]

Jon Hunter <jon-hunter at ti.com> writes:

> Hi Benoit,
>
> On 05/08/2012 06:01 AM, Cousson, Benoit wrote:
>
> [...]
>
>>>>> 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.
>
> No hacking intended here, just getting the flags correct ;-)
>
> So let me start from the beginning ...
>
> 1. I agree that for the EMU CD that the valid HW states are HW_AUTO and
> SW_WKUP.
>
> 2. When the EMU CD is active (due to something like PMU), we want to
> keep the CD in the SW_WKUP state, otherwise we can automatically
> transition to idle and reset the IP (at least for omap4430).

> 3. When the EMU CD is inactive, we want to keep the CD in the HW_AUTO
> state because SW_SLEEP is NOT supported.
>
> In the current code, we have the CLKDM_CAN_DISABLE_AUTO flag disabled
> and the CLKDM_CAN_ENABLE_AUTO flag enabled. If CLKDM_CAN_ENABLE_AUTO is
> set then the omap_pm_clkdms_setup() function will place the CD into
> HW_AUTO regardless of CLKDM_CAN_DISABLE_AUTO, and the next time the
> hwmod _enable() is called it is in the HW_AUTO state and so it is
> allowed to idle. This is not what we want. Do you agree?
>
> If I set CLKDM_CAN_DISABLE_AUTO flag and disable CLKDM_CAN_ENABLE_AUTO,
> then I do not have the above problem.
>
> To be honest, with you the more I look and test the code, the more
> confused I am by the definition of the CLKDM_CAN_HWSUP ...
>
> #define CLKDM_CAN_HWSUP	(CLKDM_CAN_ENABLE_AUTO | CLKDM_CAN_DISABLE_AUTO)
>
> When I look at where these flags are used, I see that
> CLKDM_CAN_ENABLE_AUTO is used in clkdm_allow_idle and
> CLKDM_CAN_DISABLE_AUTO is used in clkdm_deny_idle. So this implies that ...
>
> CLKDM_CAN_ENABLE_AUTO = Supports HW_AUTO state when CD is active
> CLKDM_CAN_DISABLE_AUTO = Does NOT supports HW_AUTO state when CD is active
>
> Are the above the correct definitions? 

Not quite. 

These flags describe the capabilities as defined in CLKTRCTRL field of
the CLKSTCTRL register (e.g. CM_EMU_CLKSTCTRL)

CLKDM_CAN_ENABLE_AUTO: IP supports HW_AUTO state (and it can be enabled)
CLKDM_CAN_DISABLE_AUTO: HW_AUTO feature can be disabled (a.k.a. NO_SLEEP)

Note that in OMAP4, the latter called NO_SLEEP in the TRM, but in OMAP3
it's described as "The automatic hardware-supervised mode is disabled"

What is confusing to me is that the OMAP4 TRM doesn't list the NO_SLEEP
mode as supported by the EMU.   It seems to me that if the IP supports
HW_AUTO, it should be able to be enabled *and* disabled. 

Maybe Paul/Benoit can clarify.

Kevin