diff mbox

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

Message ID 6bbd9e73b48eeb3f9f7615b19949f405@mail.gmail.com (mailing list archive)
State RFC
Delegated to: Kevin Hilman
Headers show

Commit Message

Santosh Shilimkar Feb. 4, 2011, 11:39 a.m. UTC
None

Comments

Pihet-XID, Jean June 16, 2011, 3:30 p.m. UTC | #1
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.

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?

>>
>> 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

Furthermore the main point of discussion to me is: is it advised to go
into wfi without self refresh requested? Can anyone confirm this?

> +
> +
> +/*
> + * Local variables
> + */
> +omap3_do_wfi_sram_addr:
> +       .word omap3_do_wfi_sram
> +kernel_flush:
> +       .word v7_flush_dcache_all
> +
> +/* ===================================
> + * == WFI instruction => Enter idle ==
> + * ===================================
> + */
> +
> +/*
> + * Do WFI instruction
> + * Includes the resume path for non-OFF modes
> + *
> + * This code gets copied to internal SRAM and is accessible
> + * from both SDRAM and SRAM:
> + * - executed from SRAM for non-off modes (omap3_do_wfi_sram),
> + * - executed from SDRAM for OFF mode (omap3_do_wfi).
> + */
> +ENTRY(omap3_do_wfi)
>        ldr     r4, sdrc_power          @ read the SDRC_POWER register
>        ldr     r5, [r4]                @ read the contents of SDRC_POWER
>        orr     r5, r5, #0x40           @ enable self refresh on idle req
> @@ -302,7 +348,7 @@ omap3_do_wfi:
>
>  /*
>  * ===================================
> - * == Resume path for non-OFF modes ==
> + * == Resume path for OFF/RET modes ==
>  * ===================================
>  */
>        nop
> @@ -315,15 +361,113 @@ omap3_do_wfi:
>        nop
>        nop
>        nop
> -       bl wait_sdrc_ok
...

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
Santosh Shilimkar June 16, 2011, 4:11 p.m. UTC | #2
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.

Regards
Santosh
--
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
Jean Pihet June 17, 2011, 8:58 a.m. UTC | #3
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
Santosh Shilimkar June 17, 2011, 9:13 a.m. UTC | #4
On 6/17/2011 2:28 PM, Jean Pihet wrote:
> Hi Santosh,
>

[....]


>>>> -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
>>>

[....]

>>> 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'.
>
Thanks. We needed this to be in mainline.

> 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.
> "
>
What is written here is completely right and I never said
anything against it. What I mentioned is if the CORE
clock-domain is under HW supervision, SDRC can idle
and hence the DDR can enter into self refresh.

Ofocurse on OMAP3 all clock-domain has static deps set
and hence above assumption is ok. The update I mentioned
in the code will make it complete even without auto-dep
assumption.

Anyways if that is the only point we are contesting, I
am OK to not have that change part of the patch because
it would work becasuse of auto-deps.

Regards
Santosh

--
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
Kevin Hilman June 17, 2011, 3:59 p.m. UTC | #5
Hi Santosh,

Santosh Shilimkar <santosh.shilimkar@ti.com> writes:

> On 6/17/2011 2:28 PM, Jean Pihet wrote:
>> Hi Santosh,
>>
>
> [....]
>
>
>>>>> -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
>>>>
>
> [....]
>
>>>> 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'.
>>
> Thanks. We needed this to be in mainline.
>
>> 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.
>> "
>>
> What is written here is completely right and I never said
> anything against it. What I mentioned is if the CORE
> clock-domain is under HW supervision, SDRC can idle
> and hence the DDR can enter into self refresh.
>
> Ofocurse on OMAP3 all clock-domain has static deps set
> and hence above assumption is ok. The update I mentioned
> in the code will make it complete even without auto-dep
> assumption.
>
> Anyways if that is the only point we are contesting, I
> am OK to not have that change part of the patch because
> it would work becasuse of auto-deps.

Sorry I haven't followed the whole thread...

Can you please clarify what would need to be updated if auto-deps were
removed?  We are hoping to remove them when we have full hwmod
conversion.

Kevin
--
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
Santosh Shilimkar June 17, 2011, 4:50 p.m. UTC | #6
On 6/17/2011 9:29 PM, Kevin Hilman wrote:
> Hi Santosh,
>
> Santosh Shilimkar<santosh.shilimkar@ti.com>  writes:
>
>> On 6/17/2011 2:28 PM, Jean Pihet wrote:
>>> Hi Santosh,
>>>
>>
>> [....]
>>
>>
>>>>>> -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
>>>>>
>>
>> [....]
>>
>>>>> 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'.
>>>
>> Thanks. We needed this to be in mainline.
>>
>>> 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.
>>> "
>>>
>> What is written here is completely right and I never said
>> anything against it. What I mentioned is if the CORE
>> clock-domain is under HW supervision, SDRC can idle
>> and hence the DDR can enter into self refresh.
>>
>> Ofocurse on OMAP3 all clock-domain has static deps set
>> and hence above assumption is ok. The update I mentioned
>> in the code will make it complete even without auto-dep
>> assumption.
>>
>> Anyways if that is the only point we are contesting, I
>> am OK to not have that change part of the patch because
>> it would work becasuse of auto-deps.
>
> Sorry I haven't followed the whole thread...
>
> Can you please clarify what would need to be updated if auto-deps were
> removed?  We are hoping to remove them when we have full hwmod
> conversion.
>
Sure.
I sent an update on Jean's original patch to check the CORE clock
domain state(HW_SUP or SW_WKUP). If we are in HW_SUP and the SDRC
self refresh bit enabled, SDRC can idle along with CORE. So
in that case and _only_ that case we need to execute WFI from
SRAM. I thought that's a right way to go and not depend on
auto-deps.

Jean in his refreshed patch dropped that update
with above auto-dep reasoning. For sure, with autodep
set, we won't need that additional logic because
CORE CD can never IDLE without MPU CD being in idle.

Regards
Santosh
--
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
diff mbox

Patch

diff --git a/arch/arm/mach-omap2/pm.h b/arch/arm/mach-omap2/pm.h
index 1c1b0ab..ae9dec0 100644
--- a/arch/arm/mach-omap2/pm.h
+++ b/arch/arm/mach-omap2/pm.h
@@ -87,18 +87,29 @@  extern int pm_dbg_regset_init(int reg_set);
 #define pm_dbg_regset_init(reg_set) do {} while (0);
 #endif /* CONFIG_PM_DEBUG */

+/* 24xx */
 extern void omap24xx_idle_loop_suspend(void);
+extern unsigned int omap24xx_idle_loop_suspend_sz;

 extern void omap24xx_cpu_suspend(u32 dll_ctrl, void __iomem
*sdrc_dlla_ctrl,
 					void __iomem *sdrc_power);
+extern unsigned int omap24xx_cpu_suspend_sz;
+
+/* 3xxx */
 extern void omap34xx_cpu_suspend(u32 *addr, int save_state);
+
+/* omap3_do_wfi function pointer and size, for copy to SRAM */
+extern void omap3_do_wfi(void);
+extern unsigned int omap3_do_wfi_sz;
+/* ... and its pointer from SRAM after copy */
+extern void (*omap3_do_wfi_sram)(void);
+
+/* save_secure_ram_context function pointer and size, for copy to SRAM */
 extern void save_secure_ram_context(u32 *addr);
-extern void omap3_save_scratchpad_contents(void);

-extern unsigned int omap24xx_idle_loop_suspend_sz;
 extern unsigned int save_secure_ram_context_sz;
-extern unsigned int omap24xx_cpu_suspend_sz;
-extern unsigned int omap34xx_cpu_suspend_sz;
+
+extern void omap3_save_scratchpad_contents(void);

 #define PM_RTA_ERRATUM_i608		(1 << 0)
 #define PM_SDRC_WAKEUP_ERRATUM_i583	(1 << 1)
diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 1b81e8d..cdfd627 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -83,9 +83,8 @@  struct power_state {

 static LIST_HEAD(pwrst_list);

-static void (*_omap_sram_idle)(u32 *addr, int save_state);
-
 static int (*_omap_save_secure_sram)(u32 *addr);
+void (*omap3_do_wfi_sram)(void);

 static struct powerdomain *mpu_pwrdm, *neon_pwrdm;
 static struct powerdomain *core_pwrdm, *per_pwrdm;
@@ -357,9 +356,6 @@  void omap_sram_idle(void)
 	int core_prev_state, per_prev_state;
 	u32 sdrc_pwr = 0;

-	if (!_omap_sram_idle)
-		return;
-
 	pwrdm_clear_all_prev_pwrst(mpu_pwrdm);
 	pwrdm_clear_all_prev_pwrst(neon_pwrdm);
 	pwrdm_clear_all_prev_pwrst(core_pwrdm);
@@ -444,7 +440,7 @@  void omap_sram_idle(void)
 	 * get saved. The restore path then reads from this
 	 * location and restores them back.
 	 */
-	_omap_sram_idle(omap3_arm_context, save_state);
+	omap34xx_cpu_suspend(omap3_arm_context, save_state);
 	cpu_init();

 	if (is_suspending())
@@ -993,10 +989,17 @@  static int __init clkdms_setup(struct clockdomain
*clkdm, void *unused)
 	return 0;
 }

+/*
+ * Push functions to SRAM
+ *
+ * The minimum set of functions is pushed to SRAM for execution:
+ * - omap3_do_wfi for erratum i581 WA,
+ * - save_secure_ram_context for security extensions.
+ */
 void omap_push_sram_idle(void)
 {
-	_omap_sram_idle = omap_sram_push(omap34xx_cpu_suspend,
-					omap34xx_cpu_suspend_sz);
+	omap3_do_wfi_sram = omap_sram_push(omap3_do_wfi, omap3_do_wfi_sz);
+
 	if (omap_type() != OMAP2_DEVICE_TYPE_GP)
 		_omap_save_secure_sram =
omap_sram_push(save_secure_ram_context,
 				save_secure_ram_context_sz);
diff --git a/arch/arm/mach-omap2/sleep34xx.S
b/arch/arm/mach-omap2/sleep34xx.S
index 98d8232..8a987f7 100644
--- a/arch/arm/mach-omap2/sleep34xx.S
+++ b/arch/arm/mach-omap2/sleep34xx.S
@@ -42,6 +42,7 @@ 
 					OMAP3430_PM_PREPWSTST
 #define PM_PWSTCTRL_MPU_P	OMAP3430_PRM_BASE + MPU_MOD +
OMAP2_PM_PWSTCTRL
 #define CM_IDLEST1_CORE_V	OMAP34XX_CM_REGADDR(CORE_MOD, CM_IDLEST1)
+#define CM_CLKSTCTRL_CORE_V	OMAP34XX_CM_REGADDR(CORE_MOD,
OMAP2_CM_CLKSTCTRL)
 #define CM_IDLEST_CKGEN_V	OMAP34XX_CM_REGADDR(PLL_MOD, CM_IDLEST)