On Mon, 02 Feb 2015 14:13:18 +0100 Hans de Goede hdegoede@redhat.com wrote:
Hi,
Thanks for the dram timing patches. Since they do not make any changes without them being explictly enabled in Kconfig, I've queued up patches 1 & 2 into u-boot-sunxi/next for merging upstream.
I would like to see this one handled slightly different though, see below.
I assume you're also preparing some defconfig patches to use the new settings on boards where we know exactly which DRAM chips are used (e.g. cubieboard and olinuxino boards) and thus what the proper timings are ?
In fact I'm working on providing a more feature complete update for
http://lists.denx.de/pipermail/u-boot/2015-January/202306.html
with integrated DRAM reliability tests and the ability to probe the DRAM and CPU overclocking limits. Which should allow us to collect some data from more real devices and make decisions about what kind of configuration we want to have set for them by default.
Also a small wiki page explaining how to figure out the right timing based on dram chip markings would be welcome.
On 31-01-15 23:27, Siarhei Siamashka wrote:
Higher vdd-dll voltage allows to use higher dram and mbus clock speeds. The vdd-int/vdd-dll voltage is currently set to 1.25V by default, which is much lower than 1.4V, allowed by datasheets (the exact maximum differs for different SoCs).
There are different use cases. For a tablet, it may be preferable to favor longer battery life and use lower dram clock speed & voltage. But for non-battery powered devices, especially the ones driving full-hd monitors, memory performance may be a lot more important than the power consumption.
A configurable vdd-dll voltage provides better flexibility.
Signed-off-by: Siarhei Siamashka siarhei.siamashka@gmail.com
arch/arm/include/asm/arch-sunxi/dram.h | 1 + arch/arm/include/asm/arch-sunxi/dram_sun4i.h | 1 + board/sunxi/Kconfig | 8 ++++++++ board/sunxi/board.c | 10 +++++++++- board/sunxi/dram_sun4i_auto.c | 6 ++++++ board/sunxi/dram_sun5i_auto.c | 7 +++++++ 6 files changed, 32 insertions(+), 1 deletion(-)
diff --git a/arch/arm/include/asm/arch-sunxi/dram.h b/arch/arm/include/asm/arch-sunxi/dram.h index 7ff43e6..61ed77c 100644 --- a/arch/arm/include/asm/arch-sunxi/dram.h +++ b/arch/arm/include/asm/arch-sunxi/dram.h @@ -25,6 +25,7 @@ #endif
unsigned long sunxi_dram_init(void); +u32 sunxi_dram_get_min_dll_volt(void);
/*
- Wait up to 1s for value to be set in given part of reg.
diff --git a/arch/arm/include/asm/arch-sunxi/dram_sun4i.h b/arch/arm/include/asm/arch-sunxi/dram_sun4i.h index 40c385a..32493cc 100644 --- a/arch/arm/include/asm/arch-sunxi/dram_sun4i.h +++ b/arch/arm/include/asm/arch-sunxi/dram_sun4i.h @@ -88,6 +88,7 @@ struct dram_para { u32 emr3; u32 dqs_gating_delay; u32 active_windowing;
u32 min_dll_volt; };
#define DRAM_CCR_COMMAND_RATE_1T (0x1 << 5)
diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig index 4a21589..3092edc 100644 --- a/board/sunxi/Kconfig +++ b/board/sunxi/Kconfig @@ -99,6 +99,14 @@ config DRAM_DQS_GATING_DELAY is usually good enough, unless running at really high DRAM clocks speeds (up to 600MHz). If unsure, keep as 0.
+config DRAM_MIN_DLL_VOLT
- int "sunxi dram odt_en value"
- default 1250
- ---help---
- Set the minimum VDD-DLL/VDD-INT voltage (mV), required for
- reliable DRAM operation. On Allwinner A10/A13/A20 devices with
- AXP209 PMIC it is provided from DCDC3.
- choice prompt "sunxi dram timings" default DRAM_TIMINGS_VENDOR_MAGIC
Can you please split this patch in 2:
- Add a CONFIG_AXP209_POWER, mirroring config AXP221_POWER
in drivers/power/Kconfig (put it at the top of the Kconfig file please), and update all the sunxi defconfig-s which currently set AXP209_POWER in CONFIG_SYS_EXTRA_OPTIONS to instead set +S:CONFIG_AXP209_POWER=y in there defconfigs
- Add a config AXP209_DCDC3_VOLT with a default of 1250
(copy & paste from AXP221_DCDC1_VOLT) and then in board/sunxi/board.c simply replace:
power_failed |= axp209_set_dcdc3(min_dll_volt); with: power_failed |= axp209_set_dcdc3(CONFIG_AXP209_DCDC3_VOLT);
This approach has 3 advantages:
- it is consistent with what we're doing for the axp221
- it cleans up a bit of our CONFIG_SYS_EXTRA_OPTIONS (ab)use
- it avoids the IMHO unnecessary indirection you add with the suggested sunxi_dram_get_min_dll_volt approach
The fundamental difference of your approach is that it focuses on the voltage "producer" (the DCDC3 rail of the PMIC) instead of the voltage "consumer" (the VDD-INT/VDD-DLL pins of the SoC). We even already have a minor inconsistency on A10s, which has a different PMIC and uses DCDC4 for powering VDD-INT/VDD-DLL instead of DCDC3 on A10/A13/A20.
I see the 'required minimum voltage' as an important part of the DRAM settings. And I want it to be a part of the 'dram_para' structure, so that the SPL binary (the relevant sectors on the SD card) can be edited by external tools to update the DRAM configuration.
Thanks & Regards,
Hans
diff --git a/board/sunxi/board.c b/board/sunxi/board.c index b70e00c..ce6aef5 100644 --- a/board/sunxi/board.c +++ b/board/sunxi/board.c @@ -154,11 +154,19 @@ void i2c_init_board(void) clock_twi_onoff(0, 1); }
+__weak u32 sunxi_dram_get_min_dll_volt(void) +{
- return 0;
+}
#ifdef CONFIG_SPL_BUILD void sunxi_board_init(void) { int power_failed = 0; unsigned long ramsize;
u32 min_dll_volt = sunxi_dram_get_min_dll_volt();
if (min_dll_volt < 1250)
min_dll_volt = 1250;#ifdef CONFIG_AXP152_POWER power_failed = axp152_init();
@@ -170,7 +178,7 @@ void sunxi_board_init(void) #ifdef CONFIG_AXP209_POWER power_failed |= axp209_init(); power_failed |= axp209_set_dcdc2(1400);
- power_failed |= axp209_set_dcdc3(1250);
- power_failed |= axp209_set_dcdc3(min_dll_volt); power_failed |= axp209_set_ldo2(3000); power_failed |= axp209_set_ldo3(2800); power_failed |= axp209_set_ldo4(2800);
diff --git a/board/sunxi/dram_sun4i_auto.c b/board/sunxi/dram_sun4i_auto.c index 09e0c9a..c0c8a28 100644 --- a/board/sunxi/dram_sun4i_auto.c +++ b/board/sunxi/dram_sun4i_auto.c @@ -27,9 +27,15 @@ static struct dram_para dram_para = { .emr1 = CONFIG_DRAM_EMR1, .emr3 = 0, .dqs_gating_delay = CONFIG_DRAM_DQS_GATING_DELAY,
.min_dll_volt = CONFIG_DRAM_MIN_DLL_VOLT, };
unsigned long sunxi_dram_init(void) { return dramc_init(&dram_para); }
+u32 sunxi_dram_get_min_dll_volt(void) +{
- return dram_para.min_dll_volt;
+} diff --git a/board/sunxi/dram_sun5i_auto.c b/board/sunxi/dram_sun5i_auto.c index e52d54c..30febe5 100644 --- a/board/sunxi/dram_sun5i_auto.c +++ b/board/sunxi/dram_sun5i_auto.c @@ -30,9 +30,16 @@ static struct dram_para dram_para = { .emr1 = CONFIG_DRAM_EMR1, .emr3 = 0, .dqs_gating_delay = CONFIG_DRAM_DQS_GATING_DELAY,
.min_dll_volt = CONFIG_DRAM_MIN_DLL_VOLT, };
unsigned long sunxi_dram_init(void) { return dramc_init(&dram_para); }
+u32 sunxi_dram_get_min_dll_volt(void) +{
- return dram_para.min_dll_volt;
+}