From mboxrd@z Thu Jan 1 00:00:00 1970 From: Hans de Goede Date: Wed, 11 Feb 2015 08:57:21 +0100 Subject: [U-Boot] [PATCH 3/3] sunxi: dram: Allow to configure vdd-dll voltage on sun[457]i In-Reply-To: <20150211053835.24b546fb@i7> References: <1422743227-24097-1-git-send-email-siarhei.siamashka@gmail.com> <1422743227-24097-4-git-send-email-siarhei.siamashka@gmail.com> <54CF77EE.9080102@redhat.com> <20150211053835.24b546fb@i7> Message-ID: <54DB0B61.4020503@redhat.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi, On 11-02-15 04:38, Siarhei Siamashka wrote: > On Mon, 02 Feb 2015 14:13:18 +0100 > Hans de Goede 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 >>> --- >>> 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: >> >> 1) 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 >> >> 2) 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: >> 1) it is consistent with what we're doing for the axp221 >> 2) it cleans up a bit of our CONFIG_SYS_EXTRA_OPTIONS (ab)use >> 3) 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. OK, fair enough, still can you remove all the unnecessary indirection please ? Just make dram_para a global, and directly use it in the set_dcdcd call in board.c Regards, Hans > >> >> 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; >>> +} >>> >