Re: [RFC/PATCH] OMAP3: run the ASM sleep code from DDR

[Jean Pihet <jean.pihet@newoldbits.com>]

Hi Santosh,

On Thu, Jun 16, 2011 at 6:11 PM, Santosh Shilimkar
<santosh.shilimkar@ti.com> wrote:
> On 6/16/2011 9:00 PM, Pihet-XID, Jean wrote:
>>
>> Hi Santosh, Benoit, Kevin,
>>
>> I would like to revive this discussion. Can you give some feedback on
>> the self-refresh problem that is proposed in the latest patch from
>> Santosh? Cf. below for more details.
>>
> Comments below.
>
>> On Fri, Feb 4, 2011 at 12:39 PM, Santosh Shilimkar
>> <santosh.shilimkar@ti.com>  wrote:
>>>
>>> Jean,
>>>>
>>>> -----Original Message-----
>>>> From: Santosh Shilimkar [mailto:santosh.shilimkar@ti.com]
>>>> Sent: Tuesday, February 01, 2011 5:01 PM
>>>> To: Jean Pihet
>>>> Cc: linux-omap@vger.kernel.org; Jean Pihet-XID
>>>> Subject: RE: [RFC/PATCH] OMAP3: run the ASM sleep code from DDR
>>>>
>>>>> -----Original Message-----
>>>>> From: Jean Pihet [mailto:jean.pihet@newoldbits.com]
>>>>> Sent: Tuesday, February 01, 2011 4:53 PM
>>>>> To: Santosh Shilimkar
>>>>> Cc: linux-omap@vger.kernel.org; Jean Pihet-XID
>>>>> Subject: Re: [RFC/PATCH] OMAP3: run the ASM sleep code from DDR
>>>>>
>>>>
>>>> [...]
>>>>>>>
>>>>>>> Does that makes sense?
>>>>>>>
>>>>>> Actually not. Rather I will separate only the scenario's
>>>>>> where CORE low power targets are attempted and only have
>>>>>> that code run from SRAM.
>>>>>>
>>>>>> There are scenario's where CORE can be active because MODEM,
>>>>>> DSP and MPU can still hit RET, OFF. And here, the errata
>>>>>> isn't applicable.
>>>>>>
>>>>>> Unless I missed something here, I think in the code we check
>>>>>> the the CORE attempted state and based on that we can do a
>>>>>> normal WFI from DDR (no self rfersh) or WFI from
>>>>>> SRAM with self refresh enabled.
>>>>>
>>>>> No. Only the MPU attempted state is checked (this actually is the
>>>>> 'save_state' parameter passed to omap_cpu_suspend).
>>>>> So it makes sense to check the CORE attempted state in order to
>>>>> decide
>>>>> to run the WFI from internal SRAM or DDR.
>>>>>
>>>>> The MPU attempted state is used to decide whether to save the
>>>>> MPU/L1/L2 context.
>>>>>
>>>> Looks like you miss-understood my point. As I understand from
>>>> errata, as long as core clock domain can idle with core
>>>> dpll divider auto idle enabled, the errata is applicable.
>>>>
>>>> So the only check needed is to see if the core clockdomain
>>>> hw_auto or sw sleep is enabled.
>>>>
>>>> If it is suppose to be not idle(we force few C-states
>>>> where CORE inactive is avoided for faster response),
>>>> we can execute WFI from DDR with not enabling
>>>> self refresh.
>>
>> Is this the way to go?
>>
> I think so considering we need C-states for
> faster responses as well.
>
>>>>
>>>> Rest of the scenario can follow the SRAM path.
>>>>
>>>> Hope this is clear to you.
>>>
>>> As per further discussion, I have updated your
>>> patch to address above by using core clockdomain
>>> state.
>>>
>>> Have tested OFF and RET with this new update and they
>>> work as expected. Feel free to update further if needed.
>>>
>>> ------
>>>  From 49fe8164a40431807495638ec23639cc9bc53cb9 Mon Sep 17 00:00:00 2001
>>> From: Jean Pihet<j-pihet@ti.com>
>>> Date: Sat, 29 Jan 2011 20:51:19 +0530
>>> Subject: [PATCH] OMAP3: run the ASM sleep code from DDR
>>
>> ...
>>
>>>
>>> -omap3_do_wfi:
>>> +do_WFI:
>>> +       ldr     r4, cm_clkstctrl_core   @ read the CLKSTCTRL_CORE
>>> +       ldr     r5, [r4]                @ read the contents of
>>> CLKSTCTRL_CORE
>>> +       and     r5, r5, #0x3
>>> +       cmp     r5, #0x3
>>> +       beq     omap3_do_wfi            @ Jumpt to SRAM function
>>> +       mov     r1, #0
>>> +       mcr     p15, 0, r1, c7, c10, 4
>>> +       mcr     p15, 0, r1, c7, c10, 5
>>> +
>>> +       wfi                             @ wait for interrupt
>>> +
>>> +       ldmfd   sp!, {r0-r12, pc}       @ restore regs and return
>>
>> This code has a few problems:
>> - there now are 2 wfi instructions, which I would like to avoid for
>> readability and maintainability,
>> - are the mcr instructions needed here? From
>> arch/arm/include/asm/system.h it seems those are not needed for
>> __LINUX_ARM_ARCH__>= 7
>
> The are barriers and in my clean-up I have already fixed them.
> Your patch is bit old so has those things. Once you
> refresh your patch against mainline, this can be simply
> converted to DSB and DMB.
>
>>
>> Furthermore the main point of discussion to me is: is it advised to go
>> into wfi without self refresh requested? Can anyone confirm this?
>>
> You can provided you ensure that CORE and SDRC can't idle.
>
> I suggest you to create a patch against mainline and then we
> take it from there.

Re-pushed an updated patch on l-o ML: '[PATCH] OMAP3: run the ASM
sleep code from DDR'.

I deliberately omitted the code for WFI transition without
self-refresh because of the reasons mentioned here above and repeated
here (quoting myself):
"The DDR self refresh is enabled at each WFI but not necessarily hit.
It is actually triggered by the CORE idle request which depends on the
settings, the dependencies, the HW states... For example the CORE
state depends on the MPU state so if the MPU stays ON running
instructions the CORE will stay ON as well.

Also the code in wait_sdrc_ok will exit quicker if the CORE DPLL is
already locked, e.g. if the CORE did not hit a low power state. Since
the actual CORE hit state is unknow after wake-up from WFI the
wait_sdrc_ok code always run at wake-up from MPU RET.
"

>
> Regards
> Santosh
>

Regards,
Jean
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html