Hello Duan huayang,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/44569
to review the following change.
Change subject: soc/mediatek/mt8192: Do dram fast calibration ......................................................................
soc/mediatek/mt8192: Do dram fast calibration
Load params from flash and use those params do dram fast calibration
Signed-off-by: Huayang Duan huayang.duan@mediatek.com Change-Id: I45a4fedc623aecfd000c5860e0e85175f45b8ded --- M src/soc/mediatek/mt8192/Kconfig M src/soc/mediatek/mt8192/Makefile.inc M src/soc/mediatek/mt8192/emi.c M src/soc/mediatek/mt8192/include/soc/emi.h A src/soc/mediatek/mt8192/memory.c 5 files changed, 162 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/69/44569/1
diff --git a/src/soc/mediatek/mt8192/Kconfig b/src/soc/mediatek/mt8192/Kconfig index 2412204..55b9999 100644 --- a/src/soc/mediatek/mt8192/Kconfig +++ b/src/soc/mediatek/mt8192/Kconfig @@ -15,4 +15,27 @@ select VBOOT_SEPARATE_VERSTAGE select VBOOT_RETURN_FROM_VERSTAGE
+config DEBUG_DRAM + bool "Output verbose DRAM related debug messages" + default y + help + This option enables additional DRAM related debug messages. + +config MT8192_DRAM_EMCP + bool + default n + help + The eMCP platform should select this option to run at different DRAM + frequencies. + +config MT8192_DRAM_DVFS + bool + default n + help + This options enables DRAM calibration with multiple frequencies (low, + medium and high) for DVFS feature. + +config MEMORY_TEST + bool + default y endif diff --git a/src/soc/mediatek/mt8192/Makefile.inc b/src/soc/mediatek/mt8192/Makefile.inc index 01db141..19ab639 100644 --- a/src/soc/mediatek/mt8192/Makefile.inc +++ b/src/soc/mediatek/mt8192/Makefile.inc @@ -19,6 +19,8 @@ romstage-y += emi.c romstage-y += flash_controller.c romstage-y += ../common/gpio.c gpio.c +romstage-y += ../common/mmu_operations.c +romstage-y += memory.c dramc_param.c ../common/memory_test.c romstage-$(CONFIG_SPI_FLASH) += ../common/spi.c spi.c romstage-y += ../common/timer.c romstage-y += ../common/uart.c diff --git a/src/soc/mediatek/mt8192/emi.c b/src/soc/mediatek/mt8192/emi.c index 7b1d3c2..9e48918 100644 --- a/src/soc/mediatek/mt8192/emi.c +++ b/src/soc/mediatek/mt8192/emi.c @@ -8,3 +8,7 @@
return dram_size; } + +void mt_set_emi(const struct dramc_data *dparam) +{ +} diff --git a/src/soc/mediatek/mt8192/include/soc/emi.h b/src/soc/mediatek/mt8192/include/soc/emi.h index 0348573..02a90be 100644 --- a/src/soc/mediatek/mt8192/include/soc/emi.h +++ b/src/soc/mediatek/mt8192/include/soc/emi.h @@ -3,8 +3,11 @@ #ifndef SOC_MEDIATEK_MT8192_EMI_H #define SOC_MEDIATEK_MT8192_EMI_H
-#include <types.h> +#include <soc/dramc_param.h>
size_t sdram_size(void); +void mt_set_emi(const struct dramc_data *dparam); +void mt_mem_init(struct dramc_param_ops *dparam_ops); +int complex_mem_test(u8 *start, unsigned int len);
#endif /* SOC_MEDIATEK_MT8192_EMI_H */ diff --git a/src/soc/mediatek/mt8192/memory.c b/src/soc/mediatek/mt8192/memory.c new file mode 100644 index 0000000..84c13b7 --- /dev/null +++ b/src/soc/mediatek/mt8192/memory.c @@ -0,0 +1,129 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <assert.h> +#include <bootmode.h> +#include <cbfs.h> +#include <console/console.h> +#include <ip_checksum.h> +#include <security/vboot/vboot_common.h> +#include <soc/emi.h> +#include <symbols.h> + +static int mt_mem_test(const struct dramc_data *dparam) +{ + if (CONFIG(MEMORY_TEST)) { + u8 *addr = _dram; + const struct ddr_base_info *ddr_info = &dparam->ddr_info; + + for (u8 rank = RANK_0; rank < ddr_info->support_ranks; rank++) { + int i = complex_mem_test(addr, 0x2000); + + printk(BIOS_DEBUG, "[MEM] complex R/W mem test %s.\n", + (i == 0) ? "pass" : "fail"); + + if (i != 0) { + printk(BIOS_ERR, "DRAM memory test failed\n"); + return -1; + } + + addr += ddr_info->rank_size[rank]; + } + } + + return 0; +} + +static u32 compute_checksum(const struct dramc_param *dparam) +{ + return (u32)compute_ip_checksum(&dparam->dramc_datas, + sizeof(dparam->dramc_datas)); +} + +static int dram_run_fast_calibration(const struct dramc_param *dparam) +{ + if (!is_valid_dramc_param(dparam)) { + printk(BIOS_WARNING, "Invalid DRAM calibration data from flash\n"); + dump_param_header((void *)dparam); + return -1; + } + + const u32 checksum = compute_checksum(dparam); + if (dparam->header.checksum != checksum) { + printk(BIOS_ERR, + "Invalid DRAM calibration checksum from flash " + "(expected: %#x, saved: %#x)\n", + checksum, dparam->header.checksum); + return DRAMC_ERR_INVALID_CHECKSUM; + } + + printk(BIOS_INFO, "DRAM calibration data valid pass\n"); + mt_set_emi(&dparam->dramc_datas); + if (mt_mem_test(&dparam->dramc_datas) == 0) + return 0; + + return DRAMC_ERR_FAST_CALIBRATION; +} + +static void mem_init_set_default_config(struct dramc_param *dparam, + u32 ddr_geometry) +{ + memset(dparam, 0, sizeof(*dparam)); + + if (CONFIG(MT8192_DRAM_EMCP)) + dparam->dramc_datas.ddr_info.ddr_type = DDR_TYPE_EMCP; + + if (CONFIG(MT8192_DRAM_DVFS)) + dparam->dramc_datas.ddr_info.config_dvfs = DRAMC_ENABLE_DVFS; + dparam->dramc_datas.ddr_info.ddr_geometry = ddr_geometry; + + printk(BIOS_INFO, "DRAM-K: ddr_type:%d, config_dvfs:%d, ddr_geometry:%d\n", + dparam->dramc_datas.ddr_info.ddr_type, + dparam->dramc_datas.ddr_info.config_dvfs, + dparam->dramc_datas.ddr_info.ddr_geometry); +} + +static void mt_mem_init_run(struct dramc_param_ops *dparam_ops, u32 ddr_geometry) +{ + struct dramc_param *dparam = dparam_ops->param; + + /* Load calibration params from flash and run fast calibration */ + mem_init_set_default_config(dparam, ddr_geometry); + if (dparam_ops->read_from_flash(dparam)) { + printk(BIOS_INFO, "DRAM-K: Fast Calibration\n"); + if (dram_run_fast_calibration(dparam) != 0) { + printk(BIOS_ERR, "Failed to run fast calibration\n"); + + /* need erase the flash data immediately when fast calibration fail */ + memset(dparam, 0xa5, sizeof(*dparam)); + dparam_ops->write_to_flash(dparam); + } else { + printk(BIOS_INFO, "Fast Calibration PASS\n"); + return; + } + } else { + printk(BIOS_WARNING, "Failed to read calibration data from flash\n"); + } +} + +static void setup_dramc_voltage_before_calibration(void) +{ +//TODO +} + +static void restore_dramc_voltage_after_calibration(void) +{ +//TODO +//switch_dramc_voltage_to_auto_mode() + +/* After DRAM calibration, restore vcore voltage to default setting */ +//pmic_set_vcore_vol(800000); +} + +void mt_mem_init(struct dramc_param_ops *dparam_ops) +{ + const struct sdram_info *sdram_param = get_sdram_config(); + + setup_dramc_voltage_before_calibration(); + mt_mem_init_run(dparam_ops, sdram_param->ddr_geometry); + restore_dramc_voltage_after_calibration(); +}
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44569 )
Change subject: soc/mediatek/mt8192: Do dram fast calibration ......................................................................
Patch Set 1:
(5 comments)
https://review.coreboot.org/c/coreboot/+/44569/1/src/soc/mediatek/mt8192/Kco... File src/soc/mediatek/mt8192/Kconfig:
https://review.coreboot.org/c/coreboot/+/44569/1/src/soc/mediatek/mt8192/Kco... PS1, Line 26: n I think we're always going to always use EMCP, so this should default to y?
https://review.coreboot.org/c/coreboot/+/44569/1/src/soc/mediatek/mt8192/mem... File src/soc/mediatek/mt8192/memory.c:
https://review.coreboot.org/c/coreboot/+/44569/1/src/soc/mediatek/mt8192/mem... PS1, Line 8: #include <security/vboot/vboot_common.h> you don't need this in current implementation (may need it in follow up, but let's discuss later)
https://review.coreboot.org/c/coreboot/+/44569/1/src/soc/mediatek/mt8192/mem... PS1, Line 97: 0xa5 why not 0xff? A5 implies there'll be additional writes after erasing (0xff)
https://review.coreboot.org/c/coreboot/+/44569/1/src/soc/mediatek/mt8192/mem... PS1, Line 126: setup_dramc_voltage_before_calibration(); I think you can just remove the empty function and this call. Add them only when really needed.
https://review.coreboot.org/c/coreboot/+/44569/1/src/soc/mediatek/mt8192/mem... PS1, Line 128: restore_dramc_voltage_after_calibration(); I think you can just remove the empty function and this call. Add them only when really needed.
Duan huayang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44569 )
Change subject: soc/mediatek/mt8192: Do dram fast calibration ......................................................................
Patch Set 1:
(5 comments)
https://review.coreboot.org/c/coreboot/+/44569/1/src/soc/mediatek/mt8192/Kco... File src/soc/mediatek/mt8192/Kconfig:
https://review.coreboot.org/c/coreboot/+/44569/1/src/soc/mediatek/mt8192/Kco... PS1, Line 26: n
I think we're always going to always use EMCP, so this should default to y?
Done
https://review.coreboot.org/c/coreboot/+/44569/1/src/soc/mediatek/mt8192/mem... File src/soc/mediatek/mt8192/memory.c:
https://review.coreboot.org/c/coreboot/+/44569/1/src/soc/mediatek/mt8192/mem... PS1, Line 8: #include <security/vboot/vboot_common.h>
you don't need this in current implementation (may need it in follow up, but let's discuss later)
Done
https://review.coreboot.org/c/coreboot/+/44569/1/src/soc/mediatek/mt8192/mem... PS1, Line 97: 0xa5
why not 0xff? A5 implies there'll be additional writes after erasing (0xff)
I just want to have a distinguish with the default value(0xff).
https://review.coreboot.org/c/coreboot/+/44569/1/src/soc/mediatek/mt8192/mem... PS1, Line 126: setup_dramc_voltage_before_calibration();
I think you can just remove the empty function and this call. Add them only when really needed.
Done
https://review.coreboot.org/c/coreboot/+/44569/1/src/soc/mediatek/mt8192/mem... PS1, Line 128: restore_dramc_voltage_after_calibration();
I think you can just remove the empty function and this call. Add them only when really needed.
Done
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Julius Werner, Yu-Ping Wu, Duan huayang,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44569
to look at the new patch set (#2).
Change subject: soc/mediatek/mt8192: Do dram fast calibration ......................................................................
soc/mediatek/mt8192: Do dram fast calibration
Load params from flash and use those params do dram fast calibration
Signed-off-by: Huayang Duan huayang.duan@mediatek.com Change-Id: I45a4fedc623aecfd000c5860e0e85175f45b8ded --- M src/soc/mediatek/mt8192/Kconfig M src/soc/mediatek/mt8192/Makefile.inc M src/soc/mediatek/mt8192/emi.c M src/soc/mediatek/mt8192/include/soc/emi.h A src/soc/mediatek/mt8192/memory.c 5 files changed, 145 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/69/44569/2
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44569 )
Change subject: soc/mediatek/mt8192: Do dram fast calibration ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44569/3/src/soc/mediatek/mt8192/mem... File src/soc/mediatek/mt8192/memory.c:
https://review.coreboot.org/c/coreboot/+/44569/3/src/soc/mediatek/mt8192/mem... PS3, Line 9: #include <symbols.h> maybe not used?
Duan huayang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44569 )
Change subject: soc/mediatek/mt8192: Do dram fast calibration ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44569/3/src/soc/mediatek/mt8192/mem... File src/soc/mediatek/mt8192/memory.c:
https://review.coreboot.org/c/coreboot/+/44569/3/src/soc/mediatek/mt8192/mem... PS3, Line 9: #include <symbols.h>
maybe not used?
_dram was define in _dram;
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44569 )
Change subject: soc/mediatek/mt8192: Do dram fast calibration ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44569/3/src/soc/mediatek/mt8192/mem... File src/soc/mediatek/mt8192/memory.c:
https://review.coreboot.org/c/coreboot/+/44569/3/src/soc/mediatek/mt8192/mem... PS3, Line 9: #include <symbols.h>
_dram was define in _dram;
indeed. Thx
HAOUAS Elyes has uploaded a new patch set (#4) to the change originally created by CK HU. ( https://review.coreboot.org/c/coreboot/+/44569 )
Change subject: soc/mediatek/mt8192: Do dram fast calibration ......................................................................
soc/mediatek/mt8192: Do dram fast calibration
Load params from flash and use those params do dram fast calibration
Signed-off-by: Huayang Duan huayang.duan@mediatek.com Change-Id: I45a4fedc623aecfd000c5860e0e85175f45b8ded --- M src/soc/mediatek/mt8192/Kconfig M src/soc/mediatek/mt8192/Makefile.inc M src/soc/mediatek/mt8192/emi.c M src/soc/mediatek/mt8192/include/soc/emi.h A src/soc/mediatek/mt8192/memory.c 5 files changed, 145 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/69/44569/4
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Julius Werner, Yu-Ping Wu, Duan huayang, HAOUAS Elyes,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44569
to look at the new patch set (#5).
Change subject: soc/mediatek/mt8192: Do dram fast calibration ......................................................................
soc/mediatek/mt8192: Do dram fast calibration
Load params from flash and use those params do dram fast calibration
Signed-off-by: Huayang Duan huayang.duan@mediatek.com Change-Id: I45a4fedc623aecfd000c5860e0e85175f45b8ded --- M src/soc/mediatek/mt8192/Kconfig M src/soc/mediatek/mt8192/Makefile.inc M src/soc/mediatek/mt8192/emi.c M src/soc/mediatek/mt8192/include/soc/emi.h A src/soc/mediatek/mt8192/memory.c 5 files changed, 145 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/69/44569/5
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44569 )
Change subject: soc/mediatek/mt8192: Do dram fast calibration ......................................................................
Patch Set 5:
chmod -x src/soc/mediatek/mt8192/memory.c
Duan huayang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44569 )
Change subject: soc/mediatek/mt8192: Do dram fast calibration ......................................................................
Patch Set 5:
Patch Set 5:
chmod -x src/soc/mediatek/mt8192/memory.c
done
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44569 )
Change subject: soc/mediatek/mt8192: Do dram fast calibration ......................................................................
Patch Set 5:
Patch Set 5:
Patch Set 5:
chmod -x src/soc/mediatek/mt8192/memory.c
done
https://qa.coreboot.org/job/coreboot-gerrit/139582/testReport/junit/(root)/l...
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44569 )
Change subject: soc/mediatek/mt8192: Do dram fast calibration ......................................................................
Patch Set 5:
Patch Set 5:
chmod -x src/soc/mediatek/mt8192/memory.c
done
You probably forgot to upload the new version
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44569 )
Change subject: soc/mediatek/mt8192: Do dram fast calibration ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44569/5/src/soc/mediatek/mt8192/Kco... File src/soc/mediatek/mt8192/Kconfig:
https://review.coreboot.org/c/coreboot/+/44569/5/src/soc/mediatek/mt8192/Kco... PS5, Line 22: This option enables additional DRAM related debug messages. We already have a debug RAM option in the *Debug* section in the top menu to my knowledge.
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44569 )
Change subject: soc/mediatek/mt8192: Do dram fast calibration ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44569/5/src/soc/mediatek/mt8192/Kco... File src/soc/mediatek/mt8192/Kconfig:
https://review.coreboot.org/c/coreboot/+/44569/5/src/soc/mediatek/mt8192/Kco... PS5, Line 22: This option enables additional DRAM related debug messages.
We already have a debug RAM option in the *Debug* section in the top menu to my knowledge.
That should be DEBUG_RAM_SETUP and HAVE_DEBUG_RAM_SETUP?
But the MTK family has been using DEBUG_DRAM for a while (mt8173, mt8183, mt8192). I think the options can be changed together in a follow up patchset. Does that sound OK?
(@CK can you do that?)
CK HU has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44569 )
Change subject: soc/mediatek/mt8192: Do dram fast calibration ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44569/5/src/soc/mediatek/mt8192/Kco... File src/soc/mediatek/mt8192/Kconfig:
https://review.coreboot.org/c/coreboot/+/44569/5/src/soc/mediatek/mt8192/Kco... PS5, Line 22: This option enables additional DRAM related debug messages.
That should be DEBUG_RAM_SETUP and HAVE_DEBUG_RAM_SETUP? […]
I could add another patch to refine those SoC.
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Julius Werner, Yu-Ping Wu, Duan huayang, HAOUAS Elyes,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44569
to look at the new patch set (#6).
Change subject: soc/mediatek/mt8192: Do dram fast calibration ......................................................................
soc/mediatek/mt8192: Do dram fast calibration
Load params from flash and use those params do dram fast calibration
Signed-off-by: Huayang Duan huayang.duan@mediatek.com Change-Id: I45a4fedc623aecfd000c5860e0e85175f45b8ded --- M src/soc/mediatek/mt8192/Kconfig M src/soc/mediatek/mt8192/Makefile.inc M src/soc/mediatek/mt8192/emi.c M src/soc/mediatek/mt8192/include/soc/emi.h A src/soc/mediatek/mt8192/memory.c 5 files changed, 145 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/69/44569/6
Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44569 )
Change subject: soc/mediatek/mt8192: Do dram fast calibration ......................................................................
Patch Set 6:
(3 comments)
Some nits.
https://review.coreboot.org/c/coreboot/+/44569/6//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44569/6//COMMIT_MSG@9 PS6, Line 9: n n.
https://review.coreboot.org/c/coreboot/+/44569/6//COMMIT_MSG@9 PS6, Line 9: do to do
https://review.coreboot.org/c/coreboot/+/44569/6/src/soc/mediatek/mt8192/mem... File src/soc/mediatek/mt8192/memory.c:
https://review.coreboot.org/c/coreboot/+/44569/6/src/soc/mediatek/mt8192/mem... PS6, Line 95: need Need to
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44569 )
Change subject: soc/mediatek/mt8192: Do dram fast calibration ......................................................................
Patch Set 6:
@CK please address yuping's comments.
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Julius Werner, Yu-Ping Wu, Duan huayang, HAOUAS Elyes,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44569
to look at the new patch set (#7).
Change subject: soc/mediatek/mt8192: Do dram fast calibration ......................................................................
soc/mediatek/mt8192: Do dram fast calibration
Load params from flash and use those params to do dram fast calibration.
Signed-off-by: Huayang Duan huayang.duan@mediatek.com Change-Id: I45a4fedc623aecfd000c5860e0e85175f45b8ded --- M src/soc/mediatek/mt8192/Kconfig M src/soc/mediatek/mt8192/Makefile.inc M src/soc/mediatek/mt8192/emi.c M src/soc/mediatek/mt8192/include/soc/dramc_param.h M src/soc/mediatek/mt8192/include/soc/emi.h A src/soc/mediatek/mt8192/memory.c 6 files changed, 146 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/69/44569/7
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44569 )
Change subject: soc/mediatek/mt8192: Do dram fast calibration ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44569/7/src/soc/mediatek/mt8192/mem... File src/soc/mediatek/mt8192/memory.c:
https://review.coreboot.org/c/coreboot/+/44569/7/src/soc/mediatek/mt8192/mem... PS7, Line 95: /* need to erase the flash data immediately when fast calibration fail */ line over 96 characters
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44569 )
Change subject: soc/mediatek/mt8192: Do dram fast calibration ......................................................................
Patch Set 7: Code-Review+2
(3 comments)
https://review.coreboot.org/c/coreboot/+/44569/6//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44569/6//COMMIT_MSG@9 PS6, Line 9: do
to do
Ack
https://review.coreboot.org/c/coreboot/+/44569/6//COMMIT_MSG@9 PS6, Line 9: n
n.
Ack
https://review.coreboot.org/c/coreboot/+/44569/6/src/soc/mediatek/mt8192/mem... File src/soc/mediatek/mt8192/memory.c:
https://review.coreboot.org/c/coreboot/+/44569/6/src/soc/mediatek/mt8192/mem... PS6, Line 95: need
Need to
Ack
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44569 )
Change subject: soc/mediatek/mt8192: Do dram fast calibration ......................................................................
Patch Set 7:
(2 comments)
https://review.coreboot.org/c/coreboot/+/44569/5/src/soc/mediatek/mt8192/Kco... File src/soc/mediatek/mt8192/Kconfig:
https://review.coreboot.org/c/coreboot/+/44569/5/src/soc/mediatek/mt8192/Kco... PS5, Line 22: This option enables additional DRAM related debug messages.
I could add another patch to refine those SoC.
Ack
https://review.coreboot.org/c/coreboot/+/44569/1/src/soc/mediatek/mt8192/mem... File src/soc/mediatek/mt8192/memory.c:
https://review.coreboot.org/c/coreboot/+/44569/1/src/soc/mediatek/mt8192/mem... PS1, Line 97: 0xa5
I just want to have a distinguish with the default value(0xff).
Ack
Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44569 )
Change subject: soc/mediatek/mt8192: Do dram fast calibration ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44569/7/src/soc/mediatek/mt8192/mem... File src/soc/mediatek/mt8192/memory.c:
https://review.coreboot.org/c/coreboot/+/44569/7/src/soc/mediatek/mt8192/mem... PS7, Line 95: /* need to erase the flash data immediately when fast calibration fail */
line over 96 characters
Please address that.
/* Need to erase the flash data immediately when fast calibration fail */
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44569 )
Change subject: soc/mediatek/mt8192: Do dram fast calibration ......................................................................
Patch Set 7: -Code-Review
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44569 )
Change subject: soc/mediatek/mt8192: Do dram fast calibration ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44569/8/src/soc/mediatek/mt8192/mem... File src/soc/mediatek/mt8192/memory.c:
https://review.coreboot.org/c/coreboot/+/44569/8/src/soc/mediatek/mt8192/mem... PS8, Line 95: /* need to erase the flash data immediately when fast calibration fail */ line over 96 characters
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44569 )
Change subject: soc/mediatek/mt8192: Do dram fast calibration ......................................................................
Patch Set 9:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44569/9/src/soc/mediatek/mt8192/mem... File src/soc/mediatek/mt8192/memory.c:
https://review.coreboot.org/c/coreboot/+/44569/9/src/soc/mediatek/mt8192/mem... PS9, Line 95: /* need to erase the flash data immediately when fast calibration fail */ line over 96 characters
Yidi Lin has uploaded a new patch set (#10) to the change originally created by CK HU. ( https://review.coreboot.org/c/coreboot/+/44569 )
Change subject: soc/mediatek/mt8192: Do dram fast calibration ......................................................................
soc/mediatek/mt8192: Do dram fast calibration
Load params from flash and use those params to do dram fast calibration.
Signed-off-by: Huayang Duan huayang.duan@mediatek.com Signed-off-by: Yidi Lin yidi.lin@mediatek.com Change-Id: I45a4fedc623aecfd000c5860e0e85175f45b8ded --- M src/soc/mediatek/mt8192/Kconfig M src/soc/mediatek/mt8192/Makefile.inc M src/soc/mediatek/mt8192/emi.c M src/soc/mediatek/mt8192/include/soc/dramc_param.h M src/soc/mediatek/mt8192/include/soc/emi.h A src/soc/mediatek/mt8192/memory.c 6 files changed, 147 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/69/44569/10
Yidi Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44569 )
Change subject: soc/mediatek/mt8192: Do dram fast calibration ......................................................................
Patch Set 10:
(2 comments)
https://review.coreboot.org/c/coreboot/+/44569/7/src/soc/mediatek/mt8192/mem... File src/soc/mediatek/mt8192/memory.c:
https://review.coreboot.org/c/coreboot/+/44569/7/src/soc/mediatek/mt8192/mem... PS7, Line 95: /* need to erase the flash data immediately when fast calibration fail */
Please address that. […]
Ack
https://review.coreboot.org/c/coreboot/+/44569/9/src/soc/mediatek/mt8192/mem... File src/soc/mediatek/mt8192/memory.c:
https://review.coreboot.org/c/coreboot/+/44569/9/src/soc/mediatek/mt8192/mem... PS9, Line 95: /* need to erase the flash data immediately when fast calibration fail */
line over 96 characters
done
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44569 )
Change subject: soc/mediatek/mt8192: Do dram fast calibration ......................................................................
Patch Set 10: Code-Review+2
Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44569 )
Change subject: soc/mediatek/mt8192: Do dram fast calibration ......................................................................
Patch Set 11:
(7 comments)
https://review.coreboot.org/c/coreboot/+/44569/11/src/soc/mediatek/mt8192/me... File src/soc/mediatek/mt8192/memory.c:
https://review.coreboot.org/c/coreboot/+/44569/11/src/soc/mediatek/mt8192/me... PS11, Line 20: . No period for consistency
https://review.coreboot.org/c/coreboot/+/44569/11/src/soc/mediatek/mt8192/me... PS11, Line 21: ( Align with BIOS_DEBUG
https://review.coreboot.org/c/coreboot/+/44569/11/src/soc/mediatek/mt8192/me... PS11, Line 57: Also compare the 'config'? See https://crrev.com/c/2411877.
https://review.coreboot.org/c/coreboot/+/44569/11/src/soc/mediatek/mt8192/me... PS11, Line 78: : One space after ":"
https://review.coreboot.org/c/coreboot/+/44569/11/src/soc/mediatek/mt8192/me... PS11, Line 91: Fast Calibration Running fast calibration
https://review.coreboot.org/c/coreboot/+/44569/11/src/soc/mediatek/mt8192/me... PS11, Line 95: /* need to erase the flash data immediately when fast calibration : fail */ /* Erase flash data after fast calibration failed */
https://review.coreboot.org/c/coreboot/+/44569/11/src/soc/mediatek/mt8192/me... PS11, Line 100: Fast Calibration PASS Fast calibration passed
Yidi Lin has uploaded a new patch set (#12) to the change originally created by CK HU. ( https://review.coreboot.org/c/coreboot/+/44569 )
Change subject: soc/mediatek/mt8192: Do dram fast calibration ......................................................................
soc/mediatek/mt8192: Do dram fast calibration
Load params from flash and use those params to do dram fast calibration.
Signed-off-by: Huayang Duan huayang.duan@mediatek.com Signed-off-by: Yidi Lin yidi.lin@mediatek.com Change-Id: I45a4fedc623aecfd000c5860e0e85175f45b8ded --- M src/soc/mediatek/mt8192/Kconfig M src/soc/mediatek/mt8192/Makefile.inc M src/soc/mediatek/mt8192/emi.c M src/soc/mediatek/mt8192/include/soc/dramc_param.h M src/soc/mediatek/mt8192/include/soc/emi.h A src/soc/mediatek/mt8192/memory.c 6 files changed, 155 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/69/44569/12
Yidi Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44569 )
Change subject: soc/mediatek/mt8192: Do dram fast calibration ......................................................................
Patch Set 12:
(7 comments)
https://review.coreboot.org/c/coreboot/+/44569/11/src/soc/mediatek/mt8192/me... File src/soc/mediatek/mt8192/memory.c:
https://review.coreboot.org/c/coreboot/+/44569/11/src/soc/mediatek/mt8192/me... PS11, Line 20: .
No period for consistency
Ack
https://review.coreboot.org/c/coreboot/+/44569/11/src/soc/mediatek/mt8192/me... PS11, Line 21: (
Align with BIOS_DEBUG
Ack
https://review.coreboot.org/c/coreboot/+/44569/11/src/soc/mediatek/mt8192/me... PS11, Line 57:
Also compare the 'config'? See https://crrev.com/c/2411877.
Ack
https://review.coreboot.org/c/coreboot/+/44569/11/src/soc/mediatek/mt8192/me... PS11, Line 78: :
One space after ":"
Ack
https://review.coreboot.org/c/coreboot/+/44569/11/src/soc/mediatek/mt8192/me... PS11, Line 91: Fast Calibration
Running fast calibration
Ack
https://review.coreboot.org/c/coreboot/+/44569/11/src/soc/mediatek/mt8192/me... PS11, Line 95: /* need to erase the flash data immediately when fast calibration : fail */
/* Erase flash data after fast calibration failed */
Ack
https://review.coreboot.org/c/coreboot/+/44569/11/src/soc/mediatek/mt8192/me... PS11, Line 100: Fast Calibration PASS
Fast calibration passed
Ack
Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44569 )
Change subject: soc/mediatek/mt8192: Do dram fast calibration ......................................................................
Patch Set 12: Code-Review+2
Hung-Te Lin has submitted this change. ( https://review.coreboot.org/c/coreboot/+/44569 )
Change subject: soc/mediatek/mt8192: Do dram fast calibration ......................................................................
soc/mediatek/mt8192: Do dram fast calibration
Load params from flash and use those params to do dram fast calibration.
Signed-off-by: Huayang Duan huayang.duan@mediatek.com Signed-off-by: Yidi Lin yidi.lin@mediatek.com Change-Id: I45a4fedc623aecfd000c5860e0e85175f45b8ded Reviewed-on: https://review.coreboot.org/c/coreboot/+/44569 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Yu-Ping Wu yupingso@google.com --- M src/soc/mediatek/mt8192/Kconfig M src/soc/mediatek/mt8192/Makefile.inc M src/soc/mediatek/mt8192/emi.c M src/soc/mediatek/mt8192/include/soc/dramc_param.h M src/soc/mediatek/mt8192/include/soc/emi.h A src/soc/mediatek/mt8192/memory.c 6 files changed, 155 insertions(+), 2 deletions(-)
Approvals: build bot (Jenkins): Verified Yu-Ping Wu: Looks good to me, approved
diff --git a/src/soc/mediatek/mt8192/Kconfig b/src/soc/mediatek/mt8192/Kconfig index 2412204..1d1cf7b 100644 --- a/src/soc/mediatek/mt8192/Kconfig +++ b/src/soc/mediatek/mt8192/Kconfig @@ -15,4 +15,27 @@ select VBOOT_SEPARATE_VERSTAGE select VBOOT_RETURN_FROM_VERSTAGE
+config DEBUG_DRAM + bool "Output verbose DRAM related debug messages" + default y + help + This option enables additional DRAM related debug messages. + +config MT8192_DRAM_EMCP + bool + default y + help + The eMCP platform should select this option to run at different DRAM + frequencies. + +config MT8192_DRAM_DVFS + bool + default n + help + This options enables DRAM calibration with multiple frequencies (low, + medium and high) for DVFS feature. + +config MEMORY_TEST + bool + default y endif diff --git a/src/soc/mediatek/mt8192/Makefile.inc b/src/soc/mediatek/mt8192/Makefile.inc index 533eae2..b047ce9 100644 --- a/src/soc/mediatek/mt8192/Makefile.inc +++ b/src/soc/mediatek/mt8192/Makefile.inc @@ -20,6 +20,8 @@ romstage-y += emi.c romstage-y += flash_controller.c romstage-y += ../common/gpio.c gpio.c +romstage-y += ../common/mmu_operations.c +romstage-y += memory.c dramc_param.c ../common/memory_test.c romstage-$(CONFIG_SPI_FLASH) += ../common/spi.c spi.c romstage-y += ../common/timer.c romstage-y += ../common/uart.c diff --git a/src/soc/mediatek/mt8192/emi.c b/src/soc/mediatek/mt8192/emi.c index 7b1d3c2..9e48918 100644 --- a/src/soc/mediatek/mt8192/emi.c +++ b/src/soc/mediatek/mt8192/emi.c @@ -8,3 +8,7 @@
return dram_size; } + +void mt_set_emi(const struct dramc_data *dparam) +{ +} diff --git a/src/soc/mediatek/mt8192/include/soc/dramc_param.h b/src/soc/mediatek/mt8192/include/soc/dramc_param.h index 89ff628..b4e982f 100644 --- a/src/soc/mediatek/mt8192/include/soc/dramc_param.h +++ b/src/soc/mediatek/mt8192/include/soc/dramc_param.h @@ -8,7 +8,7 @@ #include <soc/dramc_common_mt8192.h>
enum { - DRAMC_PARAM_HEADER_VERSION = 2, + DRAMC_PARAM_HEADER_VERSION = 3, };
enum DRAMC_PARAM_STATUS_CODES { diff --git a/src/soc/mediatek/mt8192/include/soc/emi.h b/src/soc/mediatek/mt8192/include/soc/emi.h index 0348573..02a90be 100644 --- a/src/soc/mediatek/mt8192/include/soc/emi.h +++ b/src/soc/mediatek/mt8192/include/soc/emi.h @@ -3,8 +3,11 @@ #ifndef SOC_MEDIATEK_MT8192_EMI_H #define SOC_MEDIATEK_MT8192_EMI_H
-#include <types.h> +#include <soc/dramc_param.h>
size_t sdram_size(void); +void mt_set_emi(const struct dramc_data *dparam); +void mt_mem_init(struct dramc_param_ops *dparam_ops); +int complex_mem_test(u8 *start, unsigned int len);
#endif /* SOC_MEDIATEK_MT8192_EMI_H */ diff --git a/src/soc/mediatek/mt8192/memory.c b/src/soc/mediatek/mt8192/memory.c new file mode 100644 index 0000000..b5363b0 --- /dev/null +++ b/src/soc/mediatek/mt8192/memory.c @@ -0,0 +1,121 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <assert.h> +#include <bootmode.h> +#include <cbfs.h> +#include <console/console.h> +#include <ip_checksum.h> +#include <soc/emi.h> +#include <symbols.h> + +static int mt_mem_test(const struct dramc_data *dparam) +{ + if (CONFIG(MEMORY_TEST)) { + u8 *addr = _dram; + const struct ddr_base_info *ddr_info = &dparam->ddr_info; + + for (u8 rank = RANK_0; rank < ddr_info->support_ranks; rank++) { + int i = complex_mem_test(addr, 0x2000); + + printk(BIOS_DEBUG, "[MEM] complex R/W mem test %s\n", + (i == 0) ? "pass" : "fail"); + + if (i != 0) { + printk(BIOS_ERR, "DRAM memory test failed\n"); + return -1; + } + + addr += ddr_info->rank_size[rank]; + } + } + + return 0; +} + +static u32 compute_checksum(const struct dramc_param *dparam) +{ + return (u32)compute_ip_checksum(&dparam->dramc_datas, + sizeof(dparam->dramc_datas)); +} + +static int dram_run_fast_calibration(const struct dramc_param *dparam) +{ + if (!is_valid_dramc_param(dparam)) { + printk(BIOS_WARNING, "Invalid DRAM calibration data from flash\n"); + dump_param_header((void *)dparam); + return -1; + } + + const u32 checksum = compute_checksum(dparam); + if (dparam->header.checksum != checksum) { + printk(BIOS_ERR, + "Invalid DRAM calibration checksum from flash " + "(expected: %#x, saved: %#x)\n", + checksum, dparam->header.checksum); + return DRAMC_ERR_INVALID_CHECKSUM; + } + + const u16 config = CONFIG(MT8192_DRAM_DVFS) ? DRAMC_ENABLE_DVFS : DRAMC_DISABLE_DVFS; + if (dparam->dramc_datas.ddr_info.config_dvfs != config) { + printk(BIOS_WARNING, + "Incompatible config for calibration data from flash " + "(expected: %#x, saved: %#x)\n", + config, dparam->dramc_datas.ddr_info.config_dvfs); + return -1; + } + + printk(BIOS_INFO, "DRAM calibration data valid pass\n"); + mt_set_emi(&dparam->dramc_datas); + if (mt_mem_test(&dparam->dramc_datas) == 0) + return 0; + + return DRAMC_ERR_FAST_CALIBRATION; +} + +static void mem_init_set_default_config(struct dramc_param *dparam, + u32 ddr_geometry) +{ + memset(dparam, 0, sizeof(*dparam)); + + if (CONFIG(MT8192_DRAM_EMCP)) + dparam->dramc_datas.ddr_info.ddr_type = DDR_TYPE_EMCP; + + if (CONFIG(MT8192_DRAM_DVFS)) + dparam->dramc_datas.ddr_info.config_dvfs = DRAMC_ENABLE_DVFS; + dparam->dramc_datas.ddr_info.ddr_geometry = ddr_geometry; + + printk(BIOS_INFO, "DRAM-K: ddr_type: %d, config_dvfs: %d, ddr_geometry: %d\n", + dparam->dramc_datas.ddr_info.ddr_type, + dparam->dramc_datas.ddr_info.config_dvfs, + dparam->dramc_datas.ddr_info.ddr_geometry); +} + +static void mt_mem_init_run(struct dramc_param_ops *dparam_ops, u32 ddr_geometry) +{ + struct dramc_param *dparam = dparam_ops->param; + + /* Load calibration params from flash and run fast calibration */ + mem_init_set_default_config(dparam, ddr_geometry); + if (dparam_ops->read_from_flash(dparam)) { + printk(BIOS_INFO, "DRAM-K: Running fast calibration\n"); + if (dram_run_fast_calibration(dparam) != 0) { + printk(BIOS_ERR, "Failed to run fast calibration\n"); + + /* Erase flash data after fast calibration failed */ + memset(dparam, 0xa5, sizeof(*dparam)); + dparam_ops->write_to_flash(dparam); + } else { + printk(BIOS_INFO, "Fast calibration passed\n"); + return; + } + } else { + printk(BIOS_WARNING, "Failed to read calibration data from flash\n"); + } +} + +void mt_mem_init(struct dramc_param_ops *dparam_ops) +{ + const struct sdram_info *sdram_param = get_sdram_config(); + + mt_mem_init_run(dparam_ops, sdram_param->ddr_geometry); +}
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44569 )
Change subject: soc/mediatek/mt8192: Do dram fast calibration ......................................................................
Patch Set 13:
(8 comments)
https://review.coreboot.org/c/coreboot/+/44569/13//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44569/13//COMMIT_MSG@10 PS13, Line 10: So this currently does not work yet, as there is no full RAM init support, so calibration data can never be stored in flash?
https://review.coreboot.org/c/coreboot/+/44569/13/src/soc/mediatek/mt8192/Kc... File src/soc/mediatek/mt8192/Kconfig:
https://review.coreboot.org/c/coreboot/+/44569/13/src/soc/mediatek/mt8192/Kc... PS13, Line 22: This option enables additional DRAM related debug messages. The commit message misses an explanation, why this defaults to yes.
https://review.coreboot.org/c/coreboot/+/44569/13/src/soc/mediatek/mt8192/Kc... PS13, Line 40: default y The commit message misses an explanation, why this defaults to yes.
https://review.coreboot.org/c/coreboot/+/44569/13/src/soc/mediatek/mt8192/me... File src/soc/mediatek/mt8192/memory.c:
https://review.coreboot.org/c/coreboot/+/44569/13/src/soc/mediatek/mt8192/me... PS13, Line 18: int i That’s confusing, as i is normal the loop variable. `result` or something similar would be less confusing.
https://review.coreboot.org/c/coreboot/+/44569/13/src/soc/mediatek/mt8192/me... PS13, Line 21: i == 0 Please use CB_SUCCESS and friends.
https://review.coreboot.org/c/coreboot/+/44569/13/src/soc/mediatek/mt8192/me... PS13, Line 24: printk(BIOS_ERR, "DRAM memory test failed\n"); Please use a common prefix for DRAM related messages. [MEM] would work.
https://review.coreboot.org/c/coreboot/+/44569/13/src/soc/mediatek/mt8192/me... PS13, Line 23: if (i != 0) { : printk(BIOS_ERR, "DRAM memory test failed\n"); : return -1; : } We already print fail above. Maybe print the address in the error message too.
https://review.coreboot.org/c/coreboot/+/44569/13/src/soc/mediatek/mt8192/me... PS13, Line 87: DRAM-K What does the *K* mean?
Xixi Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44569 )
Change subject: soc/mediatek/mt8192: Do dram fast calibration ......................................................................
Patch Set 13:
(8 comments)
A new patch for fix issues: https://review.coreboot.org/c/coreboot/+/46585
https://review.coreboot.org/c/coreboot/+/44569/13//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44569/13//COMMIT_MSG@10 PS13, Line 10:
So this currently does not work yet, as there is no full RAM init support, so calibration data can n […]
Dram full calibration: https://review.coreboot.org/c/coreboot/+/44570/11
https://review.coreboot.org/c/coreboot/+/44569/13/src/soc/mediatek/mt8192/Kc... File src/soc/mediatek/mt8192/Kconfig:
https://review.coreboot.org/c/coreboot/+/44569/13/src/soc/mediatek/mt8192/Kc... PS13, Line 22: This option enables additional DRAM related debug messages.
The commit message misses an explanation, why this defaults to yes.
The option contains many useful dram calibration log which helps developers analyse dram status. So, default to yes.
https://review.coreboot.org/c/coreboot/+/44569/13/src/soc/mediatek/mt8192/Kc... PS13, Line 40: default y
The commit message misses an explanation, why this defaults to yes.
Ack
https://review.coreboot.org/c/coreboot/+/44569/13/src/soc/mediatek/mt8192/me... File src/soc/mediatek/mt8192/memory.c:
https://review.coreboot.org/c/coreboot/+/44569/13/src/soc/mediatek/mt8192/me... PS13, Line 18: int i
That’s confusing, as i is normal the loop variable. […]
Ack
https://review.coreboot.org/c/coreboot/+/44569/13/src/soc/mediatek/mt8192/me... PS13, Line 21: i == 0
Please use CB_SUCCESS and friends.
Ack
https://review.coreboot.org/c/coreboot/+/44569/13/src/soc/mediatek/mt8192/me... PS13, Line 24: printk(BIOS_ERR, "DRAM memory test failed\n");
Please use a common prefix for DRAM related messages. [MEM] would work.
Ack
https://review.coreboot.org/c/coreboot/+/44569/13/src/soc/mediatek/mt8192/me... PS13, Line 23: if (i != 0) { : printk(BIOS_ERR, "DRAM memory test failed\n"); : return -1; : }
We already print fail above. Maybe print the address in the error message too.
will output the error code.
https://review.coreboot.org/c/coreboot/+/44569/13/src/soc/mediatek/mt8192/me... PS13, Line 87: DRAM-K
What does the *K* mean?
K means calibration.
-
build bot (Jenkins) (Code Review) -
CK HU (Code Review)
-
Duan huayang (Code Review)
-
HAOUAS Elyes (Code Review)
-
Hung-Te Lin (Code Review)
-
Paul Menzel (Code Review)
-
Xixi Chen (Code Review)
-
Yidi Lin (Code Review)
-
Yu-Ping Wu (Code Review)