[PATCH] ARM: davinci: turn off DDR PHY when entering deep sleep

[PATCH] ARM: davinci: turn off DDR PHY when entering deep sleep

[Marcus Folkesson]

Deepsleep is the most power saving state on the davinci-platform.
The DDR PHY consumes about 25mW and should be turned off in this low
power state.

Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
---
 arch/arm/mach-davinci/include/mach/memory.h |    2 ++
 arch/arm/mach-davinci/sleep.S               |   10 ++++++++++
 2 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-davinci/include/mach/memory.h b/arch/arm/mach-davinci/include/mach/memory.h
index 7873194..1b1b8ac 100644
--- a/arch/arm/mach-davinci/include/mach/memory.h
+++ b/arch/arm/mach-davinci/include/mach/memory.h
@@ -36,6 +36,8 @@
 #define DDR2_MCLKSTOPEN_BIT	BIT(30)
 #define DDR2_LPMODEN_BIT	BIT(31)
 
+#define DDR2_DRPHYC1R_OFFSET 	0xe4
+#define DDR_PWRDNEN_BIT 		BIT(6)
 /*
  * Increase size of DMA-consistent memory region
  */
diff --git a/arch/arm/mach-davinci/sleep.S b/arch/arm/mach-davinci/sleep.S
index 5f1e045..30713b2 100644
--- a/arch/arm/mach-davinci/sleep.S
+++ b/arch/arm/mach-davinci/sleep.S
@@ -57,6 +57,11 @@ ENTRY(davinci_cpu_suspend)
 
 	ldmia	r0, {r0-r4}
 
+	/* Turn PHY off */
+	ldr	ip, [r0, #DDR2_DRPHYC1R_OFFSET]
+	orr	ip, ip, #DDR_PWRDNEN_BIT 	
+	str	ip, [r0, #DDR2_DRPHYC1R_OFFSET]
+
 	/*
 	 * Switch DDR to self-refresh mode.
 	 */
@@ -171,6 +176,11 @@ ENTRY(davinci_cpu_suspend)
 	bic	ip, ip, #DDR2_LPMODEN_BIT
 	str	ip, [r0, #DDR2_SDRCR_OFFSET]
 
+	/* Turn PHY on */
+	ldr	ip, [r0, #DDR2_DRPHYC1R_OFFSET]
+	bic	ip, ip, #DDR_PWRDNEN_BIT 	
+	str	ip, [r0, #DDR2_DRPHYC1R_OFFSET]
+
 	/* Restore registers and return */
 	ldmfd   sp!, {r0-r12, pc}
 
-- 
1.7.5.4

Re: [PATCH] ARM: davinci: turn off DDR PHY when entering deep sleep

[Sekhar Nori]

Hi Marcus,

Thanks for the patch.

On 5/10/2012 11:22 PM, Marcus Folkesson wrote:
> Deepsleep is the most power saving state on the davinci-platform.
> The DDR PHY consumes about 25mW and should be turned off in this low
> power state.
> 
> Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
> ---
>  arch/arm/mach-davinci/include/mach/memory.h |    2 ++
>  arch/arm/mach-davinci/sleep.S               |   10 ++++++++++
>  2 files changed, 12 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/mach-davinci/include/mach/memory.h b/arch/arm/mach-davinci/include/mach/memory.h
> index 7873194..1b1b8ac 100644
> --- a/arch/arm/mach-davinci/include/mach/memory.h
> +++ b/arch/arm/mach-davinci/include/mach/memory.h

This is moved to arch/arm/mach-davinci/include/mach/ddr2.h in current
kernel.

> @@ -36,6 +36,8 @@
>  #define DDR2_MCLKSTOPEN_BIT	BIT(30)
>  #define DDR2_LPMODEN_BIT	BIT(31)
>  
> +#define DDR2_DRPHYC1R_OFFSET 	0xe4
> +#define DDR_PWRDNEN_BIT 		BIT(6)
>  /*
>   * Increase size of DMA-consistent memory region
>   */
> diff --git a/arch/arm/mach-davinci/sleep.S b/arch/arm/mach-davinci/sleep.S
> index 5f1e045..30713b2 100644
> --- a/arch/arm/mach-davinci/sleep.S
> +++ b/arch/arm/mach-davinci/sleep.S
> @@ -57,6 +57,11 @@ ENTRY(davinci_cpu_suspend)
>  
>  	ldmia	r0, {r0-r4}
>  
> +	/* Turn PHY off */
> +	ldr	ip, [r0, #DDR2_DRPHYC1R_OFFSET]
> +	orr	ip, ip, #DDR_PWRDNEN_BIT 	
> +	str	ip, [r0, #DDR2_DRPHYC1R_OFFSET]

Current TRM (section 14.2.13.1) specifies that this bit be set during
DDR initialization sequence itself (done in UBL or U-Boot/SPL). I am
checking with folks from the TI design team on whether it can be done
later on as part of the DeepSleep sequence.

It looks like IOPWRDN bit in VTPIO_CTL also needs to be set for this
configuration take effect. You probably did not have to do it because
the bootloader you are using already has this set?

How much testing has this patch undergone? Have you tested it across
multiple suspend-resume cycles? How much does the power consumed by DDR
PHY go down by (and which type of DDR)?

Thanks,
Sekhar

Re: [PATCH] ARM: davinci: turn off DDR PHY when entering deep sleep

[Marcus Folkesson]

Hi Sekhar,

2012/5/21 Sekhar Nori <nsekhar@ti.com>:
> Hi Marcus,
>
> Thanks for the patch.
>
> On 5/10/2012 11:22 PM, Marcus Folkesson wrote:
>> Deepsleep is the most power saving state on the davinci-platform.
>> The DDR PHY consumes about 25mW and should be turned off in this low
>> power state.
>>
>> Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
>> ---
>>  arch/arm/mach-davinci/include/mach/memory.h |    2 ++
>>  arch/arm/mach-davinci/sleep.S               |   10 ++++++++++
>>  2 files changed, 12 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/mach-davinci/include/mach/memory.h b/arch/arm/mach-davinci/include/mach/memory.h
>> index 7873194..1b1b8ac 100644
>> --- a/arch/arm/mach-davinci/include/mach/memory.h
>> +++ b/arch/arm/mach-davinci/include/mach/memory.h
>
> This is moved to arch/arm/mach-davinci/include/mach/ddr2.h in current
> kernel.
>
>> @@ -36,6 +36,8 @@
>>  #define DDR2_MCLKSTOPEN_BIT  BIT(30)
>>  #define DDR2_LPMODEN_BIT     BIT(31)
>>
>> +#define DDR2_DRPHYC1R_OFFSET         0xe4
>> +#define DDR_PWRDNEN_BIT              BIT(6)
>>  /*
>>   * Increase size of DMA-consistent memory region
>>   */
>> diff --git a/arch/arm/mach-davinci/sleep.S b/arch/arm/mach-davinci/sleep.S
>> index 5f1e045..30713b2 100644
>> --- a/arch/arm/mach-davinci/sleep.S
>> +++ b/arch/arm/mach-davinci/sleep.S
>> @@ -57,6 +57,11 @@ ENTRY(davinci_cpu_suspend)
>>
>>       ldmia   r0, {r0-r4}
>>
>> +     /* Turn PHY off */
>> +     ldr     ip, [r0, #DDR2_DRPHYC1R_OFFSET]
>> +     orr     ip, ip, #DDR_PWRDNEN_BIT
>> +     str     ip, [r0, #DDR2_DRPHYC1R_OFFSET]
>
> Current TRM (section 14.2.13.1) specifies that this bit be set during
> DDR initialization sequence itself (done in UBL or U-Boot/SPL). I am
> checking with folks from the TI design team on whether it can be done
> later on as part of the DeepSleep sequence.
>
> It looks like IOPWRDN bit in VTPIO_CTL also needs to be set for this
> configuration take effect. You probably did not have to do it because
> the bootloader you are using already has this set?
>
> How much testing has this patch undergone? Have you tested it across
> multiple suspend-resume cycles? How much does the power consumed by DDR
> PHY go down by (and which type of DDR)?

You are right.

I now see that it is possible to set all those bits in the bootloader
and let the PHY automatically go down in idle.
This patch is therefore unnecessary.

Thank you all for the attention!


Best regards
Marcus Folkesson

Re: [PATCH] ARM: davinci: turn off DDR PHY when entering deep sleep

[Sekhar Nori]

Hi Marcus,

On 5/24/2012 5:16 PM, Marcus Folkesson wrote:

>>> diff --git a/arch/arm/mach-davinci/sleep.S b/arch/arm/mach-davinci/sleep.S
>>> index 5f1e045..30713b2 100644
>>> --- a/arch/arm/mach-davinci/sleep.S
>>> +++ b/arch/arm/mach-davinci/sleep.S
>>> @@ -57,6 +57,11 @@ ENTRY(davinci_cpu_suspend)
>>>
>>>       ldmia   r0, {r0-r4}
>>>
>>> +     /* Turn PHY off */
>>> +     ldr     ip, [r0, #DDR2_DRPHYC1R_OFFSET]
>>> +     orr     ip, ip, #DDR_PWRDNEN_BIT
>>> +     str     ip, [r0, #DDR2_DRPHYC1R_OFFSET]
>>
>> Current TRM (section 14.2.13.1) specifies that this bit be set during
>> DDR initialization sequence itself (done in UBL or U-Boot/SPL). I am
>> checking with folks from the TI design team on whether it can be done
>> later on as part of the DeepSleep sequence.
>>
>> It looks like IOPWRDN bit in VTPIO_CTL also needs to be set for this
>> configuration take effect. You probably did not have to do it because
>> the bootloader you are using already has this set?
>>
>> How much testing has this patch undergone? Have you tested it across
>> multiple suspend-resume cycles? How much does the power consumed by DDR
>> PHY go down by (and which type of DDR)?
> 
> You are right.
> 
> I now see that it is possible to set all those bits in the bootloader
> and let the PHY automatically go down in idle.
> This patch is therefore unnecessary.

It will be great if you can check if the current U-Boot/SPL for AM18x
sets these bits as part of the DDR initialization and if not, submit a
patch to U-Boot list for that?

BTW, I am yet to hear from the designers on whether this can be done
outside of DDR initialization step. If they confirm this can be done, it
will be useful to take this patch in to minimize the bootloader dependency.

Thanks,
Sekhar