Change in coreboot[master]: soc/mediatek/mt8192: Limit DRAM calibration frequency count to reduce...
Hello Duan huayang,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/44731
to review the following change.
Change subject: soc/mediatek/mt8192: Limit DRAM calibration frequency count to reduce bootup time ......................................................................
soc/mediatek/mt8192: Limit DRAM calibration frequency count to reduce bootup time
Signed-off-by: Huayang Duan huayang.duan@mediatek.com Change-Id: Id664c0623318a37ed5b10c4aa5d62507187cfdac --- M src/soc/mediatek/mt8192/Kconfig M src/soc/mediatek/mt8192/dramc_pi_main.c M src/soc/mediatek/mt8192/dramc_utility.c M src/soc/mediatek/mt8192/include/soc/dramc_pi_api.h 4 files changed, 71 insertions(+), 9 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/31/44731/1
diff --git a/src/soc/mediatek/mt8192/Kconfig b/src/soc/mediatek/mt8192/Kconfig index 1d1cf7b..7547032 100644 --- a/src/soc/mediatek/mt8192/Kconfig +++ b/src/soc/mediatek/mt8192/Kconfig @@ -35,6 +35,14 @@ This options enables DRAM calibration with multiple frequencies (low, medium and high) for DVFS feature.
+config MT8192_DRAM_DVFS_LIMIT_FREQ_CNT + bool + default n + select MT8192_DRAM_DVFS + help + This options limit DRAM frequency calibration count from total 7 to 3, + other frequency will directly use the low frequency shu result. + config MEMORY_TEST bool default y diff --git a/src/soc/mediatek/mt8192/dramc_pi_main.c b/src/soc/mediatek/mt8192/dramc_pi_main.c index 72f9a24..f93be32 100644 --- a/src/soc/mediatek/mt8192/dramc_pi_main.c +++ b/src/soc/mediatek/mt8192/dramc_pi_main.c @@ -377,10 +377,20 @@ write32(&mtk_apmixed->pllon_con3, tmp & ~(0x1 << 2)); }
+static void save_low_frequency_shu_result_to_no_k_shu(void) +{ + for (u8 k_seq_idx = CALI_SEQ0; k_seq_idx < CALI_SEQ_MAX; k_seq_idx++) { + if (!is_freq_need_k(k_seq_idx)) { + dram_dfs_shu shu = get_shu_save_by_k_shu(k_seq_idx); + dramc_info("This shu no need do calibration, use shu0 result directly\n"); + dramc_save_result_to_shuffle(DRAM_DFS_SHU0, shu); + } + } +} + void init_dram(const struct dramc_data *dparam) { u32 bc_bak; - u8 k_shuffle, k_shuffle_end; u8 pll_mode = 0; bool first_freq_k = true;
@@ -407,13 +417,12 @@ dramc_sw_impedance_cal(ODT_OFF, &cali.impedance); dramc_sw_impedance_cal(ODT_ON, &cali.impedance);
- if (ddr_info->config_dvfs == DRAMC_ENABLE_DVFS) - k_shuffle_end = CALI_SEQ_MAX; - else - k_shuffle_end = CALI_SEQ1; + for (u8 k_seq_idx = CALI_SEQ0; k_seq_idx < CALI_SEQ_MAX; k_seq_idx++) { + if (!is_freq_need_k(k_seq_idx)) + continue;
- for (k_shuffle = CALI_SEQ0; k_shuffle < k_shuffle_end; k_shuffle++) { - set_cali_datas(&cali, dparam, k_shuffle); + set_cali_datas(&cali, dparam, k_seq_idx); + dramc_info("start calibration frequency %d\n", cali.frequency); set_vcore_voltage_for_each_freq(&cali); dfs_init_for_calibration(&cali);
@@ -429,9 +438,12 @@ dramc_ac_timing_optimize(&cali); dramc_save_result_to_shuffle(DRAM_DFS_SHU0, cali.shu);
- /* for frequency switch in dramc_mode_reg_init phase */ - if (first_freq_k) + if (first_freq_k) { + save_low_frequency_shu_result_to_no_k_shu(); + + /* for frequency switch in dramc_mode_reg_init phase */ dramc_load_shuffle_to_dramc(cali.shu, DRAM_DFS_SHU1); + }
first_freq_k = false; dramc_info("frequency %d calibration finish\n", get_frequency(&cali)); diff --git a/src/soc/mediatek/mt8192/dramc_utility.c b/src/soc/mediatek/mt8192/dramc_utility.c index 526059a..e8878c1 100644 --- a/src/soc/mediatek/mt8192/dramc_utility.c +++ b/src/soc/mediatek/mt8192/dramc_utility.c @@ -13,6 +13,10 @@ u32 vcore; };
+struct freq_cali_sel { + bool freq_sel; +}; + static const struct dfs_frequency_table freq_shuffle_table[DRAM_DFS_SHU_MAX] = { /* frequency freq_group div_mode shuffle_saved vref_cali vcore*/ [CALI_SEQ0] = {800, DDRFREQ_800, DIV8_MODE, DRAM_DFS_SHU4, VREF_CALI_ON, 650000}, @@ -24,6 +28,38 @@ [CALI_SEQ6] = {1600, DDRFREQ_1600, DIV8_MODE, DRAM_DFS_SHU1, VREF_CALI_OFF, 687500}, };
+#if CONFIG(MT8192_DRAM_DVFS_LIMIT_FREQ_CNT) +static const struct freq_cali_sel cali_select[CALI_SEQ_MAX] = { + [CALI_SEQ0] = {true}, + [CALI_SEQ1] = {false}, + [CALI_SEQ2] = {false}, + [CALI_SEQ3] = {false}, + [CALI_SEQ4] = {false}, + [CALI_SEQ5] = {true}, + [CALI_SEQ6] = {true}, +}; +#elif CONFIG(MT8192_DRAM_DVFS) +static const struct freq_cali_sel cali_select[CALI_SEQ_MAX] = { + [CALI_SEQ0] = {true}, + [CALI_SEQ1] = {true}, + [CALI_SEQ2] = {true}, + [CALI_SEQ3] = {true}, + [CALI_SEQ4] = {true}, + [CALI_SEQ5] = {true}, + [CALI_SEQ6] = {true}, +}; +#else +static const struct freq_cali_sel cali_select[CALI_SEQ_MAX] = { + [CALI_SEQ0] = {true}, + [CALI_SEQ1] = {false}, + [CALI_SEQ2] = {false}, + [CALI_SEQ3] = {false}, + [CALI_SEQ4] = {false}, + [CALI_SEQ5] = {false}, + [CALI_SEQ6] = {false}, +}; +#endif + void dramc_set_broadcast(u32 onoff) { write32(&mt8192_infracfg->dramc_wbr, onoff); @@ -84,6 +120,11 @@ return cali->vref_cali; }
+bool is_freq_need_k(dram_cali_seq k_seq) +{ + return cali_select[k_seq].freq_sel; +} + dram_pinmux_type get_pinmux_type(const struct ddr_cali *cali) { return cali->pinmux_type; diff --git a/src/soc/mediatek/mt8192/include/soc/dramc_pi_api.h b/src/soc/mediatek/mt8192/include/soc/dramc_pi_api.h index c2dec82..9872cda 100644 --- a/src/soc/mediatek/mt8192/include/soc/dramc_pi_api.h +++ b/src/soc/mediatek/mt8192/include/soc/dramc_pi_api.h @@ -284,6 +284,7 @@ void dramc_tx_oe_calibration(const struct ddr_cali* cali); dram_freq_grp get_freq_group(const struct ddr_cali *cali); dram_odt_state get_odt_state(const struct ddr_cali *cali); +bool is_freq_need_k(dram_cali_seq k_seq); u8 get_fsp(const struct ddr_cali *cali); dram_dfs_shu get_shu(const struct ddr_cali *cali); dram_freq_grp get_highest_freq_group(void);
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44731 )
Change subject: soc/mediatek/mt8192: Limit DRAM calibration frequency count to reduce bootup time ......................................................................
Patch Set 1:
(4 comments)
https://review.coreboot.org/c/coreboot/+/44731/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44731/1//COMMIT_MSG@8 PS1, Line 8: Please elaborate on the strategy in the commit message, document how much the time is reduced.
Also, why limit it to three and not less?
https://review.coreboot.org/c/coreboot/+/44731/1/src/soc/mediatek/mt8192/Kco... File src/soc/mediatek/mt8192/Kconfig:
https://review.coreboot.org/c/coreboot/+/44731/1/src/soc/mediatek/mt8192/Kco... PS1, Line 43: options option
https://review.coreboot.org/c/coreboot/+/44731/1/src/soc/mediatek/mt8192/Kco... PS1, Line 44: shu Please elaborate, what this is in the Kconfig option description.
https://review.coreboot.org/c/coreboot/+/44731/1/src/soc/mediatek/mt8192/dra... File src/soc/mediatek/mt8192/dramc_pi_main.c:
https://review.coreboot.org/c/coreboot/+/44731/1/src/soc/mediatek/mt8192/dra... PS1, Line 385: dramc_info("This shu no need do calibration, use shu0 result directly\n"); What is *shu*?
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44731 )
Change subject: soc/mediatek/mt8192: Limit DRAM calibration frequency count to reduce bootup time ......................................................................
Patch Set 2:
(7 comments)
https://review.coreboot.org/c/coreboot/+/44731/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44731/2//COMMIT_MSG@8 PS2, Line 8: Please elaborate and give numbers for the boot time reduction.
Also, it’d be great if you pasted the new log messages.
https://review.coreboot.org/c/coreboot/+/44731/2/src/soc/mediatek/mt8192/Kco... File src/soc/mediatek/mt8192/Kconfig:
https://review.coreboot.org/c/coreboot/+/44731/2/src/soc/mediatek/mt8192/Kco... PS2, Line 43: This options limit
This option limits
https://review.coreboot.org/c/coreboot/+/44731/2/src/soc/mediatek/mt8192/Kco... PS2, Line 44: shu Please elaborate in the text, what shu is.
https://review.coreboot.org/c/coreboot/+/44731/2/src/soc/mediatek/mt8192/Kco... PS2, Line 43: This options limit DRAM frequency calibration count from total 7 to 3, : other frequency will directly use the low frequency shu result. Please also extend, when this option should be selected (boottime).
https://review.coreboot.org/c/coreboot/+/44731/2/src/soc/mediatek/mt8192/dra... File src/soc/mediatek/mt8192/dramc_pi_main.c:
https://review.coreboot.org/c/coreboot/+/44731/2/src/soc/mediatek/mt8192/dra... PS2, Line 382: u8 size_t would use the native size. No reason to limit.
https://review.coreboot.org/c/coreboot/+/44731/2/src/soc/mediatek/mt8192/dra... PS2, Line 385: dramc_info("This shu no need do calibration, use shu0 result directly\n");
shu %s calibration skipped due to limitted frequency count. Using shu0.
https://review.coreboot.org/c/coreboot/+/44731/2/src/soc/mediatek/mt8192/inc... File src/soc/mediatek/mt8192/include/soc/dramc_pi_api.h:
https://review.coreboot.org/c/coreboot/+/44731/2/src/soc/mediatek/mt8192/inc... PS2, Line 286: bool is_freq_need_k(dram_cali_seq k_seq); Maybe we can come up with a better function name. Would the meaning of `is_freq_needing_k` be correct?
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44731 )
Change subject: soc/mediatek/mt8192: Limit DRAM calibration frequency count to reduce bootup time ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44731/8//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44731/8//COMMIT_MSG@8 PS8, Line 8: Please elaborate, and document how much the boot time is reduced.
Yidi Lin has uploaded a new patch set (#24) to the change originally created by CK HU. ( https://review.coreboot.org/c/coreboot/+/44731 )
Change subject: soc/mediatek/mt8192: Limit DRAM calibration frequency count to reduce bootup time ......................................................................
soc/mediatek/mt8192: Limit DRAM calibration frequency count to reduce bootup time
Signed-off-by: Huayang Duan huayang.duan@mediatek.com Change-Id: Id664c0623318a37ed5b10c4aa5d62507187cfdac --- M src/soc/mediatek/mt8192/Kconfig M src/soc/mediatek/mt8192/dramc_pi_main.c M src/soc/mediatek/mt8192/dramc_utility.c M src/soc/mediatek/mt8192/include/soc/dramc_pi_api.h 4 files changed, 71 insertions(+), 9 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/31/44731/24
Yidi Lin has uploaded a new patch set (#34) to the change originally created by CK HU. ( https://review.coreboot.org/c/coreboot/+/44731 )
Change subject: soc/mediatek/mt8192: Limit DRAM calibration frequency count to reduce bootup time ......................................................................
soc/mediatek/mt8192: Limit DRAM calibration frequency count to reduce bootup time
Signed-off-by: Huayang Duan huayang.duan@mediatek.com Change-Id: Id664c0623318a37ed5b10c4aa5d62507187cfdac --- M src/soc/mediatek/mt8192/Kconfig M src/soc/mediatek/mt8192/dramc_pi_main.c M src/soc/mediatek/mt8192/dramc_utility.c M src/soc/mediatek/mt8192/include/soc/dramc_pi_api.h 4 files changed, 71 insertions(+), 9 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/31/44731/34
Yidi Lin has uploaded a new patch set (#36) to the change originally created by CK HU. ( https://review.coreboot.org/c/coreboot/+/44731 )
Change subject: soc/mediatek/mt8192: Limit DRAM calibration frequency count to reduce bootup time ......................................................................
soc/mediatek/mt8192: Limit DRAM calibration frequency count to reduce bootup time
Signed-off-by: Huayang Duan huayang.duan@mediatek.com Change-Id: Id664c0623318a37ed5b10c4aa5d62507187cfdac --- M src/soc/mediatek/mt8192/Kconfig M src/soc/mediatek/mt8192/dramc_pi_main.c M src/soc/mediatek/mt8192/dramc_utility.c M src/soc/mediatek/mt8192/include/soc/dramc_pi_api.h 4 files changed, 71 insertions(+), 9 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/31/44731/36
Yidi Lin has uploaded a new patch set (#39) to the change originally created by CK HU. ( https://review.coreboot.org/c/coreboot/+/44731 )
Change subject: soc/mediatek/mt8192: Limit DRAM calibration frequency count to reduce bootup time ......................................................................
soc/mediatek/mt8192: Limit DRAM calibration frequency count to reduce bootup time
Signed-off-by: Huayang Duan huayang.duan@mediatek.com Change-Id: Id664c0623318a37ed5b10c4aa5d62507187cfdac --- M src/soc/mediatek/mt8192/Kconfig M src/soc/mediatek/mt8192/dramc_pi_calibration_api.c M src/soc/mediatek/mt8192/dramc_pi_main.c M src/soc/mediatek/mt8192/dramc_utility.c M src/soc/mediatek/mt8192/include/soc/dramc_pi_api.h 5 files changed, 73 insertions(+), 10 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/31/44731/39
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44731 )
Change subject: soc/mediatek/mt8192: Limit DRAM calibration frequency count to reduce bootup time ......................................................................
Patch Set 40:
(5 comments)
https://review.coreboot.org/c/coreboot/+/44731/40//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44731/40//COMMIT_MSG@7 PS40, Line 7: soc/mediatek/mt8192: Limit DRAM calibration frequency count to reduce bootup time What datasheet (name, revision) is this from? Why is it a build time option, and not selected by default?
https://review.coreboot.org/c/coreboot/+/44731/40//COMMIT_MSG@8 PS40, Line 8: Please elaborate and give concrete numbers.
https://review.coreboot.org/c/coreboot/+/44731/40/src/soc/mediatek/mt8192/Kc... File src/soc/mediatek/mt8192/Kconfig:
https://review.coreboot.org/c/coreboot/+/44731/40/src/soc/mediatek/mt8192/Kc... PS40, Line 46: options limit option limits
https://review.coreboot.org/c/coreboot/+/44731/40/src/soc/mediatek/mt8192/Kc... PS40, Line 47: frequency frequencies?
https://review.coreboot.org/c/coreboot/+/44731/40/src/soc/mediatek/mt8192/dr... File src/soc/mediatek/mt8192/dramc_pi_main.c:
https://review.coreboot.org/c/coreboot/+/44731/40/src/soc/mediatek/mt8192/dr... PS40, Line 404: This shu no need do calibration
This shu does not need to be calibrated, …
Attention is currently required from: Yidi Lin. Yidi Lin has uploaded a new patch set (#47) to the change originally created by CK HU. ( https://review.coreboot.org/c/coreboot/+/44731 )
Change subject: soc/mediatek/mt8192: Limit DRAM calibration frequency count to reduce bootup time ......................................................................
soc/mediatek/mt8192: Limit DRAM calibration frequency count to reduce bootup time
Signed-off-by: Huayang Duan huayang.duan@mediatek.com Change-Id: Id664c0623318a37ed5b10c4aa5d62507187cfdac --- M src/soc/mediatek/mt8192/Kconfig M src/soc/mediatek/mt8192/dramc_pi_calibration_api.c M src/soc/mediatek/mt8192/dramc_pi_main.c M src/soc/mediatek/mt8192/dramc_utility.c M src/soc/mediatek/mt8192/include/soc/dramc_pi_api.h 5 files changed, 74 insertions(+), 10 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/31/44731/47
CK HU has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/44731 )
Change subject: soc/mediatek/mt8192: Limit DRAM calibration frequency count to reduce bootup time ......................................................................
Abandoned
Useless
-
CK HU (Code Review) -
Paul Menzel (Code Review)
-
Yidi Lin (Code Review)