huayang duan has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38490 )
Change subject: soc/mediatek/mt8183: improve the DRAMC runtime config flow ......................................................................
soc/mediatek/mt8183: improve the DRAMC runtime config flow
move channel loop at the top level to reduce the redundancy logic.
Change-Id: Iea623d1bd1f7d736e81f66f191a1bf8476d30404 Signed-off-by: Huayang Duan huayang.duan@mediatek.com --- M src/soc/mediatek/mt8183/dramc_pi_basic_api.c M src/soc/mediatek/mt8183/dramc_pi_calibration_api.c M src/soc/mediatek/mt8183/emi.c M src/soc/mediatek/mt8183/include/soc/dramc_pi_api.h 4 files changed, 126 insertions(+), 132 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/38490/1
diff --git a/src/soc/mediatek/mt8183/dramc_pi_basic_api.c b/src/soc/mediatek/mt8183/dramc_pi_basic_api.c index c7d6c74..e9b1bd6 100644 --- a/src/soc/mediatek/mt8183/dramc_pi_basic_api.c +++ b/src/soc/mediatek/mt8183/dramc_pi_basic_api.c @@ -285,13 +285,12 @@ (0x1 << 29) | (0xf << 4) | (0x1 << 0), (0x1 << 29) | (0x0 << 4) | (0x1 << 0));
- for (u8 b = 0; b < 2; b++) { + for (u8 b = 0; b < 2; b++) clrsetbits32(&ch[chn].phy.b[b].dq[9], - (0x7 << 28) | (0x7 << 24), - (0x1 << 28) | (0x0 << 24)); - setbits32(&ch[chn].phy.b[b].dq[5], 0x1 << 31); - } + (0x7 << 28) | (0x7 << 24), (0x1 << 28) | (0x0 << 24)); clrbits32(&ch[chn].phy.ca_cmd[10], (0x7 << 28) | (0x7 << 24)); + for (u8 b = 0; b < 2; b++) + setbits32(&ch[chn].phy.b[b].dq[5], 0x1 << 31);
setbits32(&ch[chn].phy.b0_rxdvs[0], (0x1 << 28) | (0x1 << 31)); setbits32(&ch[chn].phy.b1_rxdvs[0], (0x1 << 28) | (0x1 << 31)); @@ -321,16 +320,14 @@ clrbits32(&ch[chn].phy.ca_cmd[6], 0x1 << 31); }
-static void dramc_hw_gating_init(void) +static void dramc_hw_gating_init(size_t chn) { - for (size_t chn = 0; chn < CHANNEL_MAX; chn++) { - clrbits32(&ch[chn].ao.stbcal, - (0x7 << 22) | (0x3 << 14) | (0x1 << 19) | (0x1 << 21)); - setbits32(&ch[chn].ao.stbcal, (0x1 << 20) | (0x3 << 28)); - setbits32(&ch[chn].phy.misc_ctrl1, 0x1 << 24); + clrbits32(&ch[chn].ao.stbcal, + (0x7 << 22) | (0x3 << 14) | (0x1 << 19) | (0x1 << 21)); + setbits32(&ch[chn].ao.stbcal, (0x1 << 20) | (0x3 << 28)); + setbits32(&ch[chn].phy.misc_ctrl1, 0x1 << 24);
- dramc_hw_dqs_gating_tracking(chn); - } + dramc_hw_dqs_gating_tracking(chn); }
static void dramc_impedance_tracking_enable(void) @@ -348,24 +345,25 @@ setbits32(&ch[chn].ao.refctrl0, (0x1 << 2) | (0x1 << 3)); }
-static void dramc_phy_low_power_enable(void) +static void dramc_phy_low_power_enable(size_t chn) { - for (size_t chn = 0; chn < CHANNEL_MAX; chn++) { - for (size_t b = 0; b < 2; b++) { - clrbits32(&ch[chn].phy.b[b].dll_fine_tune[2], - 0x3fffff << 10); - write32(&ch[chn].phy.b[b].dll_fine_tune[3], 0x2e800); - } - clrsetbits32(&ch[chn].phy.ca_dll_fine_tune[2], - 0x3fffff << 10, 0x2 << 10); + for (size_t b = 0; b < 2; b++) { + clrbits32(&ch[chn].phy.b[b].dll_fine_tune[2], + 0x3fffff << 10); + write32(&ch[chn].phy.b[b].dll_fine_tune[3], 0x2e800); } - write32(&ch[0].phy.ca_dll_fine_tune[3], 0xba000); - write32(&ch[1].phy.ca_dll_fine_tune[3], 0x3a000); + clrsetbits32(&ch[chn].phy.ca_dll_fine_tune[2], + 0x3fffff << 10, 0x2 << 10); + + if (chn == CHANNEL_A) + write32(&ch[chn].phy.ca_dll_fine_tune[3], 0xba000); + else + write32(&ch[chn].phy.ca_dll_fine_tune[3], 0x3a000); }
static void dramc_dummy_read_for_tracking_enable(u8 chn) { - setbits32(&ch[chn].ao.dummy_rd, 0x3 << 16); + clrsetbits32(&ch[chn].ao.dummy_rd, 0x3 << 16, 0x2 << 16);
for (size_t r = 0; r < 2; r++) for (size_t i = 0; i < 4; i++) @@ -380,7 +378,7 @@ clrbits32(&ch[chn].ao.rk[r].dummy_rd_bk, 0x7 << 0); }
- clrbits32(&ch[chn].ao.dummy_rd, 0x1 << 25 | 0x1 << 20); + setbits32(&ch[chn].ao.dummy_rd, 0x1 << 25 | 0x1 << 20); }
static void dramc_set_CKE_2_rank_independent(u8 chn) @@ -421,46 +419,41 @@
void dramc_runtime_config(void) { - clrbits32(&ch[0].ao.refctrl0, 0x1 << 29); - clrbits32(&ch[1].ao.refctrl0, 0x1 << 29); + for (u8 chn = 0; chn < CHANNEL_MAX; chn++) + clrbits32(&ch[chn].ao.refctrl0, 0x1 << 29);
transfer_pll_to_spm_control(); setbits32(&mtk_spm->spm_power_on_val0, 0x1 << 25);
- for (u8 chn = 0; chn < CHANNEL_MAX; chn++) + for (size_t chn = 0; chn < CHANNEL_MAX; chn++) { dramc_hw_dqsosc(chn);
/* RX_TRACKING: ON */ - for (u8 chn = 0; chn < CHANNEL_MAX; chn++) dramc_rx_input_delay_tracking(chn);
/* HW_GATING: ON */ - dramc_hw_gating_init(); - dramc_hw_gating_onoff(CHANNEL_A, true); - dramc_hw_gating_onoff(CHANNEL_B, true); + dramc_hw_gating_init(chn); + dramc_hw_gating_onoff(chn, true);
/* HW_GATING DBG: OFF */ - for (size_t chn = 0; chn < CHANNEL_MAX; chn++) clrbits32(&ch[chn].ao.stbcal2, (0x3 << 4) | (0x3 << 8) | (0x1 << 28));
/* DUMMY_READ_FOR_TRACKING: ON */ - for (u8 chn = 0; chn < CHANNEL_MAX; chn++) dramc_dummy_read_for_tracking_enable(chn);
/* ZQCS_ENABLE_LP4: ON */ - clrbits32(&ch[0].ao.spcmdctrl, 0x1 << 30); - clrbits32(&ch[1].ao.spcmdctrl, 0x1 << 30); + clrbits32(&ch[chn].ao.spcmdctrl, 0x1 << 30);
/* LOWPOWER_GOLDEN_SETTINGS(DCM): ON */ - dramc_phy_low_power_enable(); - dramc_enable_phy_dcm(true); + dramc_phy_low_power_enable(chn); + dramc_enable_phy_dcm(chn, true);
/* DUMMY_READ_FOR_DQS_GATING_RETRY: OFF */ - for (size_t chn = 0; chn < CHANNEL_MAX; chn++) for (size_t shu = 0; shu < DRAM_DFS_SHUFFLE_MAX; shu++) clrbits32(&ch[chn].ao.shu[shu].dqsg_retry, (0x1 << 1) | (0x3 << 13)); + }
/* SPM_CONTROL_AFTERK: ON */ write32(&ch[0].phy.misc_spm_ctrl0, 0xfbffefff); @@ -496,9 +489,10 @@
/* DRAM DRS DISABLE */ clrsetbits32(&ch[chn].ao.drsctrl, - (0x1 << 21) | (0x3f << 12) | (0xf << 8) | (0x1 << 6), + (0x1 << 21) | (0x7f << 12) | (0xf << 8) | + (0x1 << 6) | (0x1 << 4)| (0x1 << 2), (0x1 << 19) | (0x3 << 12) | (0x8 << 8) | - (0x3 << 4) | (0x1 << 2) | (0x1 << 0)); + (0x1 << 5) | (0x1 << 2) | (0x1 << 0)); setbits32(&ch[chn].ao.dummy_rd, 0x3 << 26); } dramc_dqs_precalculation_preset(); diff --git a/src/soc/mediatek/mt8183/dramc_pi_calibration_api.c b/src/soc/mediatek/mt8183/dramc_pi_calibration_api.c index f93214e..bdb14ee 100644 --- a/src/soc/mediatek/mt8183/dramc_pi_calibration_api.c +++ b/src/soc/mediatek/mt8183/dramc_pi_calibration_api.c @@ -286,18 +286,16 @@ dramc_mode_reg_write_by_rank(chn, rank, 12, final_vref); }
-static void dramc_read_dbi_onoff(bool on) +static void dramc_read_dbi_onoff(size_t chn, bool on) { - for (size_t chn = 0; chn < CHANNEL_MAX; chn++) - for (size_t b = 0; b < 2; b++) - SET32_BITFIELDS(&ch[chn].phy.shu[0].b[b].dq[7], - SHU1_B0_DQ7_R_DMDQMDBI_SHU_B0, on); + for (size_t b = 0; b < 2; b++) + SET32_BITFIELDS(&ch[chn].phy.shu[0].b[b].dq[7], + SHU1_B0_DQ7_R_DMDQMDBI_SHU_B0, on); }
-static void dramc_write_dbi_onoff(bool onoff) +static void dramc_write_dbi_onoff(size_t chn, bool onoff) { - for (size_t chn = 0; chn < CHANNEL_MAX; chn++) - SET32_BITFIELDS(&ch[chn].ao.shu[0].wodt, SHU1_WODT_DBIWR, onoff); + SET32_BITFIELDS(&ch[chn].ao.shu[0].wodt, SHU1_WODT_DBIWR, onoff); }
static void dramc_phy_dcm_2_channel(u8 chn, bool en) @@ -317,64 +315,61 @@ ((en ? 0x7 : 0) << 16) | ((en ? 0x7 : 0) << 20)); }
-void dramc_enable_phy_dcm(bool en) +void dramc_enable_phy_dcm(u8 chn, bool en) { - for (size_t chn = 0; chn < CHANNEL_MAX ; chn++) { - clrbits32(&ch[chn].phy.b[0].dll_fine_tune[1], 0x1 << 20); - clrbits32(&ch[chn].phy.b[1].dll_fine_tune[1], 0x1 << 20); - clrbits32(&ch[chn].phy.ca_dll_fine_tune[1], 0x1 << 20); + clrbits32(&ch[chn].phy.b[0].dll_fine_tune[1], 0x1 << 20); + clrbits32(&ch[chn].phy.b[1].dll_fine_tune[1], 0x1 << 20); + clrbits32(&ch[chn].phy.ca_dll_fine_tune[1], 0x1 << 20);
- for (size_t i = 0; i < DRAM_DFS_SHUFFLE_MAX; i++) { - struct ddrphy_ao_shu *shu = &ch[chn].phy.shu[i]; - setbits32(&shu->b[0].dll[0], 0x1); - setbits32(&shu->b[1].dll[0], 0x1); - setbits32(&shu->ca_dll[0], 0x1); - } - - clrsetbits32(&ch[chn].ao.dramc_pd_ctrl, - (0x1 << 0) | (0x1 << 1) | (0x1 << 2) | - (0x1 << 5) | (0x1 << 26) | (0x1 << 30) | (0x1 << 31), - ((en ? 0x1 : 0) << 0) | ((en ? 0x1 : 0) << 1) | - ((en ? 0x1 : 0) << 2) | ((en ? 0 : 0x1) << 5) | - ((en ? 0 : 0x1) << 26) | ((en ? 0x1 : 0) << 30) | - ((en ? 0x1 : 0) << 31)); - - /* DCM on: CHANNEL_EMI free run; DCM off: mem_dcm */ - write32(&ch[chn].phy.misc_cg_ctrl2, - 0x8060033e | (0x40 << (en ? 0x1 : 0))); - write32(&ch[chn].phy.misc_cg_ctrl2, - 0x8060033f | (0x40 << (en ? 0x1 : 0))); - write32(&ch[chn].phy.misc_cg_ctrl2, - 0x8060033e | (0x40 << (en ? 0x1 : 0))); - - clrsetbits32(&ch[chn].phy.misc_ctrl3, 0x3 << 26, - (en ? 0 : 0x3) << 26); - for (size_t i = 0; i < DRAM_DFS_SHUFFLE_MAX; i++) { - u32 mask = 0x7 << 17; - u32 value = (en ? 0x7 : 0) << 17; - struct ddrphy_ao_shu *shu = &ch[chn].phy.shu[i]; - - clrsetbits32(&shu->b[0].dq[7], mask, value); - clrsetbits32(&shu->b[1].dq[7], mask, value); - clrsetbits32(&shu->ca_cmd[7], mask, value); - } - - dramc_phy_dcm_2_channel(chn, en); + for (size_t i = 0; i < DRAM_DFS_SHUFFLE_MAX; i++) { + struct ddrphy_ao_shu *shu = &ch[chn].phy.shu[i]; + setbits32(&shu->b[0].dll[0], 0x1); + setbits32(&shu->b[1].dll[0], 0x1); + setbits32(&shu->ca_dll[0], 0x1); } + + clrsetbits32(&ch[chn].ao.dramc_pd_ctrl, + (0x1 << 0) | (0x1 << 1) | (0x1 << 2) | + (0x1 << 5) | (0x1 << 26) | (0x1 << 30) | (0x1 << 31), + ((en ? 0x1 : 0) << 0) | ((en ? 0x1 : 0) << 1) | + ((en ? 0x1 : 0) << 2) | ((en ? 0 : 0x1) << 5) | + ((en ? 0 : 0x1) << 26) | ((en ? 0x1 : 0) << 30) | + ((en ? 0x1 : 0) << 31)); + + /* DCM on: CHANNEL_EMI free run; DCM off: mem_dcm */ + write32(&ch[chn].phy.misc_cg_ctrl2, + 0x8060033e | (0x40 << (en ? 0x1 : 0))); + write32(&ch[chn].phy.misc_cg_ctrl2, + 0x8060033f | (0x40 << (en ? 0x1 : 0))); + write32(&ch[chn].phy.misc_cg_ctrl2, + 0x8060033e | (0x40 << (en ? 0x1 : 0))); + + clrsetbits32(&ch[chn].phy.misc_ctrl3, 0x3 << 26, + (en ? 0 : 0x3) << 26); + for (size_t i = 0; i < DRAM_DFS_SHUFFLE_MAX; i++) { + u32 mask = 0x7 << 17; + u32 value = (en ? 0x7 : 0) << 17; + struct ddrphy_ao_shu *shu = &ch[chn].phy.shu[i]; + + clrsetbits32(&shu->b[0].dq[7], mask, value); + clrsetbits32(&shu->b[1].dq[7], mask, value); + clrsetbits32(&shu->ca_cmd[7], mask, value); + } + + dramc_phy_dcm_2_channel(chn, en); }
-static void dramc_reset_delay_chain_before_calibration(void) +static void dramc_reset_delay_chain_before_calibration(size_t chn) { - for (size_t chn = 0; chn < CHANNEL_MAX; chn++) - for (size_t rank = 0; rank < RANK_MAX; rank++) { - struct dramc_ddrphy_regs_shu_rk *rk; - rk = &ch[chn].phy.shu[0].rk[rank]; - clrbits32(&rk->ca_cmd[0], 0xffffff << 0); - clrbits32(&rk->b[0].dq[0], 0xfffffff << 0); - clrbits32(&rk->b[1].dq[0], 0xfffffff << 0); - clrbits32(&rk->b[0].dq[1], 0xf << 0); - clrbits32(&rk->b[1].dq[1], 0xf << 0); - } + for (size_t rank = 0; rank < RANK_MAX; rank++) { + struct dramc_ddrphy_regs_shu_rk *rk; + rk = &ch[chn].phy.shu[0].rk[rank]; + clrbits32(&rk->ca_cmd[0], 0xffffff << 0); + clrbits32(&rk->b[0].dq[0], 0xfffffff << 0); + clrbits32(&rk->b[1].dq[0], 0xfffffff << 0); + clrbits32(&rk->b[0].dq[1], 0xf << 0); + clrbits32(&rk->b[1].dq[1], 0xf << 0); + } }
void dramc_hw_gating_onoff(u8 chn, bool on) @@ -398,26 +393,26 @@
void dramc_apply_config_before_calibration(u8 freq_group) { - dramc_enable_phy_dcm(false); - dramc_reset_delay_chain_before_calibration(); - - setbits32(&ch[0].ao.shu[0].conf[3], 0x1ff << 16); - setbits32(&ch[0].ao.spcmdctrl, 0x1 << 24); - clrsetbits32(&ch[0].ao.shu[0].scintv, 0x1f << 1, 0x1b << 1); - - for (size_t shu = DRAM_DFS_SHUFFLE_1; shu < DRAM_DFS_SHUFFLE_MAX; shu++) - setbits32(&ch[0].ao.shu[shu].conf[3], 0x1ff << 0); - - clrbits32(&ch[0].ao.dramctrl, 0x1 << 18); - clrbits32(&ch[0].ao.spcmdctrl, 0x1 << 31); - clrbits32(&ch[0].ao.spcmdctrl, 0x1 << 30); - clrbits32(&ch[0].ao.dqsoscr, 0x1 << 26); - clrbits32(&ch[0].ao.dqsoscr, 0x1 << 25); - - dramc_write_dbi_onoff(false); - dramc_read_dbi_onoff(false); - for (size_t chn = 0; chn < CHANNEL_MAX; chn++) { + dramc_enable_phy_dcm(chn, false); + dramc_reset_delay_chain_before_calibration(chn); + + setbits32(&ch[chn].ao.shu[0].conf[3], 0x1ff << 16); + setbits32(&ch[chn].ao.spcmdctrl, 0x1 << 24); + clrsetbits32(&ch[chn].ao.shu[0].scintv, 0x1f << 1, 0x1b << 1); + + for (size_t shu = DRAM_DFS_SHUFFLE_1; shu < DRAM_DFS_SHUFFLE_MAX; shu++) + setbits32(&ch[chn].ao.shu[shu].conf[3], 0x1ff << 0); + + clrbits32(&ch[chn].ao.dramctrl, 0x1 << 18); + clrbits32(&ch[chn].ao.spcmdctrl, 0x1 << 31); + clrbits32(&ch[chn].ao.spcmdctrl, 0x1 << 30); + clrbits32(&ch[chn].ao.dqsoscr, 0x1 << 26); + clrbits32(&ch[chn].ao.dqsoscr, 0x1 << 25); + + dramc_write_dbi_onoff(chn, false); + dramc_read_dbi_onoff(chn, false); + setbits32(&ch[chn].ao.spcmdctrl, 0x1 << 29); setbits32(&ch[chn].ao.dqsoscr, 0x1 << 24); for (size_t shu = DRAM_DFS_SHUFFLE_1; shu < DRAM_DFS_SHUFFLE_MAX; shu++) @@ -791,7 +786,6 @@ SET32_BITFIELDS(&ch[chn].ao.spcmd, SPCMD_DQSGCNTRST, 0); SET32_BITFIELDS(&ch[chn].phy.misc_ctrl1, MISC_CTRL1_R_DMSTBENCMP_RK, rank); - }
static void set_selph_gating_value(uint32_t *addr, u8 dly, u8 dly_p1) diff --git a/src/soc/mediatek/mt8183/emi.c b/src/soc/mediatek/mt8183/emi.c index 4680a0e..cf1369e 100644 --- a/src/soc/mediatek/mt8183/emi.c +++ b/src/soc/mediatek/mt8183/emi.c @@ -64,6 +64,8 @@ struct optimize_ac_time { u8 rfc; u8 rfc_05t; + u8 rfc_pb; + u8 rfrc_pb05t; u16 tx_ref_cnt; };
@@ -324,19 +326,23 @@ static void dramc_ac_timing_optimize(u8 freq_group) { struct optimize_ac_time rf_cab_opt[LP4X_DDRFREQ_MAX] = { - [LP4X_DDR1600] = {.rfc = 44, .rfc_05t = 0, .tx_ref_cnt = 62}, - [LP4X_DDR2400] = {.rfc = 72, .rfc_05t = 0, .tx_ref_cnt = 91}, - [LP4X_DDR3200] = {.rfc = 100, .rfc_05t = 0, .tx_ref_cnt = 119}, - [LP4X_DDR3600] = {.rfc = 118, .rfc_05t = 1, .tx_ref_cnt = 138}, + [LP4X_DDR1600] = {.rfc = 44, .rfc_05t = 0, .rfc_pb = 16, .rfrc_pb05t = 0, .tx_ref_cnt = 62}, + [LP4X_DDR2400] = {.rfc = 72, .rfc_05t = 0, .rfc_pb = 30, .rfrc_pb05t = 0, .tx_ref_cnt = 91}, + [LP4X_DDR3200] = {.rfc = 100, .rfc_05t = 0, .rfc_pb = 44, .rfrc_pb05t = 0, .tx_ref_cnt = 119}, + [LP4X_DDR3600] = {.rfc = 118, .rfc_05t = 1, .rfc_pb = 53, .rfrc_pb05t = 1, .tx_ref_cnt = 138}, };
for (size_t chn = 0; chn < CHANNEL_MAX; chn++) { clrsetbits32(&ch[chn].ao.shu[0].actim[3], 0xff << 16, rf_cab_opt[freq_group].rfc << 16); - clrbits32(&ch[chn].ao.shu[0].ac_time_05t, - rf_cab_opt[freq_group].rfc_05t << 2); - clrsetbits32(&ch[chn].ao.shu[0].actim[4], + clrsetbits_le32(&ch[chn].ao.shu[0].ac_time_05t, + 0x1 << 2, rf_cab_opt[freq_group].rfc_05t << 2); + clrsetbits_le32(&ch[chn].ao.shu[0].actim[4], 0x3ff << 0, rf_cab_opt[freq_group].tx_ref_cnt << 0); + clrsetbits_le32(&ch[chn].ao.shu[0].actim[3], + 0xff << 0, rf_cab_opt[freq_group].rfc_pb << 0); + clrsetbits_le32(&ch[chn].ao.shu[0].ac_time_05t, + 0x1 << 1, rf_cab_opt[freq_group].rfrc_pb05t << 1); } }
@@ -509,11 +515,11 @@ dfs_init_for_calibration(params, freq_group, shared); *first_run = false;
- dramc_dbg("Start K (current clock: %u\n", params->frequency); + dramc_dbg("Start K current clock: %u\n", params->frequency); if (dramc_calibrate_all_channels(params, freq_group, &shared->mr) != 0) return -1; dramc_ac_timing_optimize(freq_group); - dramc_dbg("K finished (current clock: %u\n", params->frequency); + dramc_dbg("K finished current clock: %u\n", params->frequency);
dramc_save_result_to_shuffle(DRAM_DFS_SHUFFLE_1, shuffle); return 0; diff --git a/src/soc/mediatek/mt8183/include/soc/dramc_pi_api.h b/src/soc/mediatek/mt8183/include/soc/dramc_pi_api.h index ea4f138..32df9e5 100644 --- a/src/soc/mediatek/mt8183/include/soc/dramc_pi_api.h +++ b/src/soc/mediatek/mt8183/include/soc/dramc_pi_api.h @@ -112,7 +112,7 @@ int dramc_calibrate_all_channels(const struct sdram_params *pams, u8 freq_group, const struct mr_value *mr); void dramc_hw_gating_onoff(u8 chn, bool onoff); -void dramc_enable_phy_dcm(bool bEn); +void dramc_enable_phy_dcm(u8 chn, bool bEn); void dramc_mode_reg_write(u8 chn, u8 mr_idx, u8 value); void dramc_cke_fix_onoff(u8 chn, bool fix_on, bool fix_off); void dramc_hw_dqsosc(u8 chn);
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38490 )
Change subject: soc/mediatek/mt8183: improve the DRAMC runtime config flow ......................................................................
Patch Set 1:
(5 comments)
https://review.coreboot.org/c/coreboot/+/38490/1/src/soc/mediatek/mt8183/dra... File src/soc/mediatek/mt8183/dramc_pi_basic_api.c:
https://review.coreboot.org/c/coreboot/+/38490/1/src/soc/mediatek/mt8183/dra... PS1, Line 493: (0x1 << 6) | (0x1 << 4)| (0x1 << 2), need consistent spacing around '|' (ctx:VxW)
https://review.coreboot.org/c/coreboot/+/38490/1/src/soc/mediatek/mt8183/emi... File src/soc/mediatek/mt8183/emi.c:
https://review.coreboot.org/c/coreboot/+/38490/1/src/soc/mediatek/mt8183/emi... PS1, Line 329: [LP4X_DDR1600] = {.rfc = 44, .rfc_05t = 0, .rfc_pb = 16, .rfrc_pb05t = 0, .tx_ref_cnt = 62}, line over 96 characters
https://review.coreboot.org/c/coreboot/+/38490/1/src/soc/mediatek/mt8183/emi... PS1, Line 330: [LP4X_DDR2400] = {.rfc = 72, .rfc_05t = 0, .rfc_pb = 30, .rfrc_pb05t = 0, .tx_ref_cnt = 91}, line over 96 characters
https://review.coreboot.org/c/coreboot/+/38490/1/src/soc/mediatek/mt8183/emi... PS1, Line 331: [LP4X_DDR3200] = {.rfc = 100, .rfc_05t = 0, .rfc_pb = 44, .rfrc_pb05t = 0, .tx_ref_cnt = 119}, line over 96 characters
https://review.coreboot.org/c/coreboot/+/38490/1/src/soc/mediatek/mt8183/emi... PS1, Line 332: [LP4X_DDR3600] = {.rfc = 118, .rfc_05t = 1, .rfc_pb = 53, .rfrc_pb05t = 1, .tx_ref_cnt = 138}, line over 96 characters
Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38490 )
Change subject: soc/mediatek/mt8183: improve the DRAMC runtime config flow ......................................................................
Patch Set 1:
(8 comments)
https://review.coreboot.org/c/coreboot/+/38490/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/38490/1//COMMIT_MSG@9 PS1, Line 9: move Move
https://review.coreboot.org/c/coreboot/+/38490/1//COMMIT_MSG@10 PS1, Line 10: BRANCH=kukui BUG=? TEST=?
https://review.coreboot.org/c/coreboot/+/38490/1/src/soc/mediatek/mt8183/dra... File src/soc/mediatek/mt8183/dramc_pi_basic_api.c:
https://review.coreboot.org/c/coreboot/+/38490/1/src/soc/mediatek/mt8183/dra... PS1, Line 290: (0x7 << 28) | (0x7 << 24), (0x1 << 28) | (0x0 << 24)); Align with previous line.
https://review.coreboot.org/c/coreboot/+/38490/1/src/soc/mediatek/mt8183/dra... PS1, Line 323: size_t Couldn't this be u8 (for consistency)?
https://review.coreboot.org/c/coreboot/+/38490/1/src/soc/mediatek/mt8183/dra... PS1, Line 326: (0x7 << 22) | (0x3 << 14) | (0x1 << 19) | (0x1 << 21)); Align with previous line.
https://review.coreboot.org/c/coreboot/+/38490/1/src/soc/mediatek/mt8183/dra... PS1, Line 361: write32(&ch[chn].phy.ca_dll_fine_tune[3], 0x3a000); write32(&ch[chn].phy.ca_dll_fine_tune[3], chn == CHANNEL_A ? 0xba000 : 0x3a000);
https://review.coreboot.org/c/coreboot/+/38490/1/src/soc/mediatek/mt8183/dra... PS1, Line 381: setbits32(&ch[chn].ao.dummy_rd, 0x1 << 25 | 0x1 << 20); Is this bug fix or improvement?
https://review.coreboot.org/c/coreboot/+/38490/1/src/soc/mediatek/mt8183/dra... PS1, Line 431: /* RX_TRACKING: ON */ Wrong indent.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38490 )
Change subject: soc/mediatek/mt8183: improve the DRAMC runtime config flow ......................................................................
Patch Set 1:
(5 comments)
https://review.coreboot.org/c/coreboot/+/38490/1/src/soc/mediatek/mt8183/dra... File src/soc/mediatek/mt8183/dramc_pi_basic_api.c:
https://review.coreboot.org/c/coreboot/+/38490/1/src/soc/mediatek/mt8183/dra... PS1, Line 323: size_t
Couldn't this be u8 (for consistency)?
It should be, the existing code uses u8 in most cases.
https://review.coreboot.org/c/coreboot/+/38490/1/src/soc/mediatek/mt8183/dra... PS1, Line 352: 0x3fffff << 10); Fits on previous line
https://review.coreboot.org/c/coreboot/+/38490/1/src/soc/mediatek/mt8183/dra... PS1, Line 428: size_t Why size_t here?
https://review.coreboot.org/c/coreboot/+/38490/1/src/soc/mediatek/mt8183/dra... PS1, Line 431: /* RX_TRACKING: ON */
Wrong indent.
Also applies to all other comments
https://review.coreboot.org/c/coreboot/+/38490/1/src/soc/mediatek/mt8183/dra... PS1, Line 493: (0x1 << 6) | (0x1 << 4)| (0x1 << 2),
need consistent spacing around '|' (ctx:VxW)
Please correct this
Another suggestion: align the "(0x1 << 12)" blocks:
(0x1 << 21) | (0x7f << 12) | (0xf << 8) | (0x1 << 6) | (0x1 << 4) | (0x1 << 2), (0x1 << 19) | (0x3 << 12) | (0x8 << 8) | (0x1 << 5) | (0x1 << 2) | (0x1 << 0));
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38490 )
Change subject: soc/mediatek/mt8183: improve the DRAMC runtime config flow ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38490/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/38490/1//COMMIT_MSG@9 PS1, Line 9: to reduce the redundancy logic Please rephrase. Reduce the size of the code?
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38490 )
Change subject: soc/mediatek/mt8183: improve the DRAMC runtime config flow ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38490/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/38490/1//COMMIT_MSG@9 PS1, Line 9: to reduce the redundancy logic
Please rephrase. […]
No, that would change the meaning. I understand that the intention is to say `to reduce redundant logic`. If that sounds too weird, one could also use `to deduplicate the logic`.
Duan huayang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38490 )
Change subject: soc/mediatek/mt8183: improve the DRAMC runtime config flow ......................................................................
Patch Set 1:
(12 comments)
https://review.coreboot.org/c/coreboot/+/38490/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/38490/1//COMMIT_MSG@9 PS1, Line 9: to reduce the redundancy logic
No, that would change the meaning. […]
Ack
https://review.coreboot.org/c/coreboot/+/38490/1//COMMIT_MSG@9 PS1, Line 9: move
Move
Done
https://review.coreboot.org/c/coreboot/+/38490/1//COMMIT_MSG@10 PS1, Line 10:
BRANCH=kukui […]
Done
https://review.coreboot.org/c/coreboot/+/38490/1/src/soc/mediatek/mt8183/dra... File src/soc/mediatek/mt8183/dramc_pi_basic_api.c:
https://review.coreboot.org/c/coreboot/+/38490/1/src/soc/mediatek/mt8183/dra... PS1, Line 290: (0x7 << 28) | (0x7 << 24), (0x1 << 28) | (0x0 << 24));
Align with previous line.
Done
https://review.coreboot.org/c/coreboot/+/38490/1/src/soc/mediatek/mt8183/dra... PS1, Line 323: size_t
It should be, the existing code uses u8 in most cases.
Done
https://review.coreboot.org/c/coreboot/+/38490/1/src/soc/mediatek/mt8183/dra... PS1, Line 326: (0x7 << 22) | (0x3 << 14) | (0x1 << 19) | (0x1 << 21));
Align with previous line.
what's the align result? like this? clrbits32(&ch[chn].ao.stbcal, (0x7 << 22) | (0x3 << 14) | (0x1 << 19) | (0x1 << 21));
https://review.coreboot.org/c/coreboot/+/38490/1/src/soc/mediatek/mt8183/dra... PS1, Line 352: 0x3fffff << 10);
Fits on previous line
Done
https://review.coreboot.org/c/coreboot/+/38490/1/src/soc/mediatek/mt8183/dra... PS1, Line 361: write32(&ch[chn].phy.ca_dll_fine_tune[3], 0x3a000);
write32(&ch[chn].phy. […]
Done
https://review.coreboot.org/c/coreboot/+/38490/1/src/soc/mediatek/mt8183/dra... PS1, Line 381: setbits32(&ch[chn].ao.dummy_rd, 0x1 << 25 | 0x1 << 20);
Is this bug fix or improvement?
is a bug fix
https://review.coreboot.org/c/coreboot/+/38490/1/src/soc/mediatek/mt8183/dra... PS1, Line 428: size_t
Why size_t here?
it should u8, update it later
https://review.coreboot.org/c/coreboot/+/38490/1/src/soc/mediatek/mt8183/dra... PS1, Line 431: /* RX_TRACKING: ON */
Also applies to all other comments
Done
https://review.coreboot.org/c/coreboot/+/38490/1/src/soc/mediatek/mt8183/dra... PS1, Line 493: (0x1 << 6) | (0x1 << 4)| (0x1 << 2),
Please correct this […]
Done
Hello Yu-Ping Wu, Duan huayang, Julius Werner, Hung-Te Lin, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38490
to look at the new patch set (#2).
Change subject: soc/mediatek/mt8183: improve the DRAMC runtime config flow ......................................................................
soc/mediatek/mt8183: improve the DRAMC runtime config flow
Move channel loop at the top level to deduplicate the logic.
BUG=b:none BRANCH=kukui TEST=Boots correctly on Kukui
Change-Id: Iea623d1bd1f7d736e81f66f191a1bf8476d30404 Signed-off-by: Huayang Duan huayang.duan@mediatek.com --- M src/soc/mediatek/mt8183/dramc_pi_basic_api.c M src/soc/mediatek/mt8183/dramc_pi_calibration_api.c M src/soc/mediatek/mt8183/emi.c M src/soc/mediatek/mt8183/include/soc/dramc_pi_api.h 4 files changed, 134 insertions(+), 140 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/38490/2
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38490 )
Change subject: soc/mediatek/mt8183: improve the DRAMC runtime config flow ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/38490/2/src/soc/mediatek/mt8183/dra... File src/soc/mediatek/mt8183/dramc_pi_basic_api.c:
https://review.coreboot.org/c/coreboot/+/38490/2/src/soc/mediatek/mt8183/dra... PS2, Line 350: size_t u8 (since all other code uses u8 for b)?
https://review.coreboot.org/c/coreboot/+/38490/2/src/soc/mediatek/mt8183/dra... PS2, Line 362: clrsetbits32 this has changed the logic (set 0x2 instead of 0x3).
Can you move this (and the setbits32 below) to a separate CL, since they should be fixing something instead?
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38490 )
Change subject: soc/mediatek/mt8183: improve the DRAMC runtime config flow ......................................................................
Patch Set 2: Code-Review+1
(2 comments)
https://review.coreboot.org/c/coreboot/+/38490/1/src/soc/mediatek/mt8183/dra... File src/soc/mediatek/mt8183/dramc_pi_basic_api.c:
https://review.coreboot.org/c/coreboot/+/38490/1/src/soc/mediatek/mt8183/dra... PS1, Line 326: (0x7 << 22) | (0x3 << 14) | (0x1 << 19) | (0x1 << 21));
what's the align result? […]
I think it means this:
clrbits32(&ch[chn].ao.stbcal, (0x7 << 22) | (0x3 << 14) | (0x1 << 19) | (0x1 << 21));
It fits in 96 characters.
https://review.coreboot.org/c/coreboot/+/38490/1/src/soc/mediatek/mt8183/dra... PS1, Line 428: size_t
it should u8, update it later
Done
Hello Hung-Te Lin, build bot (Jenkins), Angel Pons, Julius Werner, Yu-Ping Wu, Duan huayang,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38490
to look at the new patch set (#3).
Change subject: soc/mediatek/mt8183: improve the DRAMC runtime config flow ......................................................................
soc/mediatek/mt8183: improve the DRAMC runtime config flow
Move channel loop at the top level to deduplicate the logic.
BUG=b:none BRANCH=kukui TEST=Boots correctly on Kukui
Change-Id: Iea623d1bd1f7d736e81f66f191a1bf8476d30404 Signed-off-by: Huayang Duan huayang.duan@mediatek.com --- M src/soc/mediatek/mt8183/dramc_pi_basic_api.c M src/soc/mediatek/mt8183/dramc_pi_calibration_api.c M src/soc/mediatek/mt8183/emi.c M src/soc/mediatek/mt8183/include/soc/dramc_pi_api.h 4 files changed, 131 insertions(+), 138 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/38490/3
Duan huayang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38490 )
Change subject: soc/mediatek/mt8183: improve the DRAMC runtime config flow ......................................................................
Patch Set 3:
(3 comments)
https://review.coreboot.org/c/coreboot/+/38490/1/src/soc/mediatek/mt8183/dra... File src/soc/mediatek/mt8183/dramc_pi_basic_api.c:
https://review.coreboot.org/c/coreboot/+/38490/1/src/soc/mediatek/mt8183/dra... PS1, Line 326: (0x7 << 22) | (0x3 << 14) | (0x1 << 19) | (0x1 << 21));
I think it means this: […]
Done
https://review.coreboot.org/c/coreboot/+/38490/2/src/soc/mediatek/mt8183/dra... File src/soc/mediatek/mt8183/dramc_pi_basic_api.c:
https://review.coreboot.org/c/coreboot/+/38490/2/src/soc/mediatek/mt8183/dra... PS2, Line 350: size_t
u8 (since all other code uses u8 for b)?
Done
https://review.coreboot.org/c/coreboot/+/38490/2/src/soc/mediatek/mt8183/dra... PS2, Line 362: clrsetbits32
this has changed the logic (set 0x2 instead of 0x3). […]
will revert this unless change.
Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38490 )
Change subject: soc/mediatek/mt8183: improve the DRAMC runtime config flow ......................................................................
Patch Set 3:
(4 comments)
https://review.coreboot.org/c/coreboot/+/38490/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/38490/3//COMMIT_MSG@11 PS3, Line 11: BUG=b:none BUG=none
https://review.coreboot.org/c/coreboot/+/38490/1/src/soc/mediatek/mt8183/dra... File src/soc/mediatek/mt8183/dramc_pi_basic_api.c:
https://review.coreboot.org/c/coreboot/+/38490/1/src/soc/mediatek/mt8183/dra... PS1, Line 290: (0x7 << 28) | (0x7 << 24), (0x1 << 28) | (0x0 << 24));
Done
I meant
clrsetbits32(&ch[chn].phy.b[b].dq[9], (0x7 << 28) | (0x7 << 24), (0x1 << 28) | (0x0 << 24));
I know most of the code in this file doesn't align like this, but we should do it while we're at it.
https://review.coreboot.org/c/coreboot/+/38490/1/src/soc/mediatek/mt8183/dra... PS1, Line 326: (0x7 << 22) | (0x3 << 14) | (0x1 << 19) | (0x1 << 21));
Done
I meant
clrbits32(&ch[chn].ao.stbcal, (0x7 << 22) | (0x3 << 14) | (0x1 << 19) | (0x1 << 21));
https://review.coreboot.org/c/coreboot/+/38490/1/src/soc/mediatek/mt8183/dra... PS1, Line 381: setbits32(&ch[chn].ao.dummy_rd, 0x1 << 25 | 0x1 << 20);
is a bug fix
Could you move all the bug fixes to a separate CL? It's hard to tell bug fixes from improvement.
Hello Hung-Te Lin, build bot (Jenkins), Angel Pons, Julius Werner, Yu-Ping Wu, Duan huayang,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38490
to look at the new patch set (#4).
Change subject: soc/mediatek/mt8183: improve the DRAMC runtime config flow ......................................................................
soc/mediatek/mt8183: improve the DRAMC runtime config flow
Move channel loop at the top level to deduplicate the logic.
BUG=none BRANCH=kukui TEST=Boots correctly on Kukui
Change-Id: Iea623d1bd1f7d736e81f66f191a1bf8476d30404 Signed-off-by: Huayang Duan huayang.duan@mediatek.com --- M src/soc/mediatek/mt8183/dramc_pi_basic_api.c M src/soc/mediatek/mt8183/dramc_pi_calibration_api.c M src/soc/mediatek/mt8183/include/soc/dramc_pi_api.h 3 files changed, 108 insertions(+), 126 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/38490/4
Duan huayang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38490 )
Change subject: soc/mediatek/mt8183: improve the DRAMC runtime config flow ......................................................................
Patch Set 4:
(3 comments)
https://review.coreboot.org/c/coreboot/+/38490/1/src/soc/mediatek/mt8183/dra... File src/soc/mediatek/mt8183/dramc_pi_basic_api.c:
https://review.coreboot.org/c/coreboot/+/38490/1/src/soc/mediatek/mt8183/dra... PS1, Line 290: (0x7 << 28) | (0x7 << 24), (0x1 << 28) | (0x0 << 24));
I meant […]
Done
https://review.coreboot.org/c/coreboot/+/38490/1/src/soc/mediatek/mt8183/dra... PS1, Line 326: (0x7 << 22) | (0x3 << 14) | (0x1 << 19) | (0x1 << 21));
I meant […]
Done
https://review.coreboot.org/c/coreboot/+/38490/1/src/soc/mediatek/mt8183/dra... PS1, Line 381: setbits32(&ch[chn].ao.dummy_rd, 0x1 << 25 | 0x1 << 20);
Could you move all the bug fixes to a separate CL? It's hard to tell bug fixes from improvement.
Done
Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38490 )
Change subject: soc/mediatek/mt8183: improve the DRAMC runtime config flow ......................................................................
Patch Set 4:
(2 comments)
https://review.coreboot.org/c/coreboot/+/38490/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/38490/3//COMMIT_MSG@11 PS3, Line 11: BUG=b:none
BUG=none
Done
https://review.coreboot.org/c/coreboot/+/38490/2/src/soc/mediatek/mt8183/dra... File src/soc/mediatek/mt8183/dramc_pi_basic_api.c:
https://review.coreboot.org/c/coreboot/+/38490/2/src/soc/mediatek/mt8183/dra... PS2, Line 362: clrsetbits32
will revert this unless change.
Done
Yu-Ping Wu has uploaded a new patch set (#5) to the change originally created by huayang duan. ( https://review.coreboot.org/c/coreboot/+/38490 )
Change subject: soc/mediatek/mt8183: Improve the DRAMC runtime config flow ......................................................................
soc/mediatek/mt8183: Improve the DRAMC runtime config flow
Move channel loop at the top level to deduplicate the logic.
BUG=none BRANCH=kukui TEST=Boots correctly on Kukui
Change-Id: Iea623d1bd1f7d736e81f66f191a1bf8476d30404 Signed-off-by: Huayang Duan huayang.duan@mediatek.com --- M src/soc/mediatek/mt8183/dramc_pi_basic_api.c M src/soc/mediatek/mt8183/dramc_pi_calibration_api.c M src/soc/mediatek/mt8183/include/soc/dramc_pi_api.h 3 files changed, 111 insertions(+), 125 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/38490/5
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38490 )
Change subject: soc/mediatek/mt8183: Improve the DRAMC runtime config flow ......................................................................
Patch Set 5: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/38490 )
Change subject: soc/mediatek/mt8183: Improve the DRAMC runtime config flow ......................................................................
soc/mediatek/mt8183: Improve the DRAMC runtime config flow
Move channel loop at the top level to deduplicate the logic.
BUG=none BRANCH=kukui TEST=Boots correctly on Kukui
Change-Id: Iea623d1bd1f7d736e81f66f191a1bf8476d30404 Signed-off-by: Huayang Duan huayang.duan@mediatek.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/38490 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Hung-Te Lin hungte@chromium.org --- M src/soc/mediatek/mt8183/dramc_pi_basic_api.c M src/soc/mediatek/mt8183/dramc_pi_calibration_api.c M src/soc/mediatek/mt8183/include/soc/dramc_pi_api.h 3 files changed, 111 insertions(+), 125 deletions(-)
Approvals: build bot (Jenkins): Verified Hung-Te Lin: Looks good to me, approved
diff --git a/src/soc/mediatek/mt8183/dramc_pi_basic_api.c b/src/soc/mediatek/mt8183/dramc_pi_basic_api.c index c7d6c74..dc3676a 100644 --- a/src/soc/mediatek/mt8183/dramc_pi_basic_api.c +++ b/src/soc/mediatek/mt8183/dramc_pi_basic_api.c @@ -285,13 +285,13 @@ (0x1 << 29) | (0xf << 4) | (0x1 << 0), (0x1 << 29) | (0x0 << 4) | (0x1 << 0));
- for (u8 b = 0; b < 2; b++) { + for (u8 b = 0; b < 2; b++) clrsetbits32(&ch[chn].phy.b[b].dq[9], - (0x7 << 28) | (0x7 << 24), - (0x1 << 28) | (0x0 << 24)); - setbits32(&ch[chn].phy.b[b].dq[5], 0x1 << 31); - } + (0x7 << 28) | (0x7 << 24), + (0x1 << 28) | (0x0 << 24)); clrbits32(&ch[chn].phy.ca_cmd[10], (0x7 << 28) | (0x7 << 24)); + for (u8 b = 0; b < 2; b++) + setbits32(&ch[chn].phy.b[b].dq[5], 0x1 << 31);
setbits32(&ch[chn].phy.b0_rxdvs[0], (0x1 << 28) | (0x1 << 31)); setbits32(&ch[chn].phy.b1_rxdvs[0], (0x1 << 28) | (0x1 << 31)); @@ -321,16 +321,14 @@ clrbits32(&ch[chn].phy.ca_cmd[6], 0x1 << 31); }
-static void dramc_hw_gating_init(void) +static void dramc_hw_gating_init(u8 chn) { - for (size_t chn = 0; chn < CHANNEL_MAX; chn++) { - clrbits32(&ch[chn].ao.stbcal, - (0x7 << 22) | (0x3 << 14) | (0x1 << 19) | (0x1 << 21)); - setbits32(&ch[chn].ao.stbcal, (0x1 << 20) | (0x3 << 28)); - setbits32(&ch[chn].phy.misc_ctrl1, 0x1 << 24); + clrbits32(&ch[chn].ao.stbcal, + (0x7 << 22) | (0x3 << 14) | (0x1 << 19) | (0x1 << 21)); + setbits32(&ch[chn].ao.stbcal, (0x1 << 20) | (0x3 << 28)); + setbits32(&ch[chn].phy.misc_ctrl1, 0x1 << 24);
- dramc_hw_dqs_gating_tracking(chn); - } + dramc_hw_dqs_gating_tracking(chn); }
static void dramc_impedance_tracking_enable(void) @@ -348,19 +346,16 @@ setbits32(&ch[chn].ao.refctrl0, (0x1 << 2) | (0x1 << 3)); }
-static void dramc_phy_low_power_enable(void) +static void dramc_phy_low_power_enable(u8 chn) { - for (size_t chn = 0; chn < CHANNEL_MAX; chn++) { - for (size_t b = 0; b < 2; b++) { - clrbits32(&ch[chn].phy.b[b].dll_fine_tune[2], - 0x3fffff << 10); - write32(&ch[chn].phy.b[b].dll_fine_tune[3], 0x2e800); - } - clrsetbits32(&ch[chn].phy.ca_dll_fine_tune[2], - 0x3fffff << 10, 0x2 << 10); + for (u8 b = 0; b < 2; b++) { + clrbits32(&ch[chn].phy.b[b].dll_fine_tune[2], 0x3fffff << 10); + write32(&ch[chn].phy.b[b].dll_fine_tune[3], 0x2e800); } - write32(&ch[0].phy.ca_dll_fine_tune[3], 0xba000); - write32(&ch[1].phy.ca_dll_fine_tune[3], 0x3a000); + clrsetbits32(&ch[chn].phy.ca_dll_fine_tune[2], + 0x3fffff << 10, 0x2 << 10); + write32(&ch[chn].phy.ca_dll_fine_tune[3], + (chn == CHANNEL_A) ? 0xba000 : 0x3a000); }
static void dramc_dummy_read_for_tracking_enable(u8 chn) @@ -421,46 +416,41 @@
void dramc_runtime_config(void) { - clrbits32(&ch[0].ao.refctrl0, 0x1 << 29); - clrbits32(&ch[1].ao.refctrl0, 0x1 << 29); + for (u8 chn = 0; chn < CHANNEL_MAX; chn++) + clrbits32(&ch[chn].ao.refctrl0, 0x1 << 29);
transfer_pll_to_spm_control(); setbits32(&mtk_spm->spm_power_on_val0, 0x1 << 25);
- for (u8 chn = 0; chn < CHANNEL_MAX; chn++) + for (u8 chn = 0; chn < CHANNEL_MAX; chn++) { dramc_hw_dqsosc(chn);
- /* RX_TRACKING: ON */ - for (u8 chn = 0; chn < CHANNEL_MAX; chn++) + /* RX_TRACKING: ON */ dramc_rx_input_delay_tracking(chn);
- /* HW_GATING: ON */ - dramc_hw_gating_init(); - dramc_hw_gating_onoff(CHANNEL_A, true); - dramc_hw_gating_onoff(CHANNEL_B, true); + /* HW_GATING: ON */ + dramc_hw_gating_init(chn); + dramc_hw_gating_onoff(chn, true);
- /* HW_GATING DBG: OFF */ - for (size_t chn = 0; chn < CHANNEL_MAX; chn++) + /* HW_GATING DBG: OFF */ clrbits32(&ch[chn].ao.stbcal2, - (0x3 << 4) | (0x3 << 8) | (0x1 << 28)); + (0x3 << 4) | (0x3 << 8) | (0x1 << 28));
- /* DUMMY_READ_FOR_TRACKING: ON */ - for (u8 chn = 0; chn < CHANNEL_MAX; chn++) + /* DUMMY_READ_FOR_TRACKING: ON */ dramc_dummy_read_for_tracking_enable(chn);
- /* ZQCS_ENABLE_LP4: ON */ - clrbits32(&ch[0].ao.spcmdctrl, 0x1 << 30); - clrbits32(&ch[1].ao.spcmdctrl, 0x1 << 30); + /* ZQCS_ENABLE_LP4: ON */ + clrbits32(&ch[chn].ao.spcmdctrl, 0x1 << 30);
- /* LOWPOWER_GOLDEN_SETTINGS(DCM): ON */ - dramc_phy_low_power_enable(); - dramc_enable_phy_dcm(true); + /* LOWPOWER_GOLDEN_SETTINGS(DCM): ON */ + dramc_phy_low_power_enable(chn); + dramc_enable_phy_dcm(chn, true);
- /* DUMMY_READ_FOR_DQS_GATING_RETRY: OFF */ - for (size_t chn = 0; chn < CHANNEL_MAX; chn++) + /* DUMMY_READ_FOR_DQS_GATING_RETRY: OFF */ for (size_t shu = 0; shu < DRAM_DFS_SHUFFLE_MAX; shu++) clrbits32(&ch[chn].ao.shu[shu].dqsg_retry, - (0x1 << 1) | (0x3 << 13)); + (0x1 << 1) | (0x3 << 13)); + }
/* SPM_CONTROL_AFTERK: ON */ write32(&ch[0].phy.misc_spm_ctrl0, 0xfbffefff); diff --git a/src/soc/mediatek/mt8183/dramc_pi_calibration_api.c b/src/soc/mediatek/mt8183/dramc_pi_calibration_api.c index 4ccc7fc..b9634a8 100644 --- a/src/soc/mediatek/mt8183/dramc_pi_calibration_api.c +++ b/src/soc/mediatek/mt8183/dramc_pi_calibration_api.c @@ -282,18 +282,16 @@ dramc_mode_reg_write_by_rank(chn, rank, 12, final_vref); }
-static void dramc_read_dbi_onoff(bool on) +static void dramc_read_dbi_onoff(size_t chn, bool on) { - for (size_t chn = 0; chn < CHANNEL_MAX; chn++) - for (size_t b = 0; b < 2; b++) - SET32_BITFIELDS(&ch[chn].phy.shu[0].b[b].dq[7], - SHU1_B0_DQ7_R_DMDQMDBI_SHU_B0, on); + for (size_t b = 0; b < 2; b++) + SET32_BITFIELDS(&ch[chn].phy.shu[0].b[b].dq[7], + SHU1_B0_DQ7_R_DMDQMDBI_SHU_B0, on); }
-static void dramc_write_dbi_onoff(bool onoff) +static void dramc_write_dbi_onoff(size_t chn, bool onoff) { - for (size_t chn = 0; chn < CHANNEL_MAX; chn++) - SET32_BITFIELDS(&ch[chn].ao.shu[0].wodt, SHU1_WODT_DBIWR, onoff); + SET32_BITFIELDS(&ch[chn].ao.shu[0].wodt, SHU1_WODT_DBIWR, onoff); }
static void dramc_phy_dcm_2_channel(u8 chn, bool en) @@ -313,64 +311,61 @@ ((en ? 0x7 : 0) << 16) | ((en ? 0x7 : 0) << 20)); }
-void dramc_enable_phy_dcm(bool en) +void dramc_enable_phy_dcm(u8 chn, bool en) { - for (size_t chn = 0; chn < CHANNEL_MAX ; chn++) { - clrbits32(&ch[chn].phy.b[0].dll_fine_tune[1], 0x1 << 20); - clrbits32(&ch[chn].phy.b[1].dll_fine_tune[1], 0x1 << 20); - clrbits32(&ch[chn].phy.ca_dll_fine_tune[1], 0x1 << 20); + clrbits32(&ch[chn].phy.b[0].dll_fine_tune[1], 0x1 << 20); + clrbits32(&ch[chn].phy.b[1].dll_fine_tune[1], 0x1 << 20); + clrbits32(&ch[chn].phy.ca_dll_fine_tune[1], 0x1 << 20);
- for (size_t i = 0; i < DRAM_DFS_SHUFFLE_MAX; i++) { - struct ddrphy_ao_shu *shu = &ch[chn].phy.shu[i]; - setbits32(&shu->b[0].dll[0], 0x1); - setbits32(&shu->b[1].dll[0], 0x1); - setbits32(&shu->ca_dll[0], 0x1); - } - - clrsetbits32(&ch[chn].ao.dramc_pd_ctrl, - (0x1 << 0) | (0x1 << 1) | (0x1 << 2) | - (0x1 << 5) | (0x1 << 26) | (0x1 << 30) | (0x1 << 31), - ((en ? 0x1 : 0) << 0) | ((en ? 0x1 : 0) << 1) | - ((en ? 0x1 : 0) << 2) | ((en ? 0 : 0x1) << 5) | - ((en ? 0 : 0x1) << 26) | ((en ? 0x1 : 0) << 30) | - ((en ? 0x1 : 0) << 31)); - - /* DCM on: CHANNEL_EMI free run; DCM off: mem_dcm */ - write32(&ch[chn].phy.misc_cg_ctrl2, - 0x8060033e | (0x40 << (en ? 0x1 : 0))); - write32(&ch[chn].phy.misc_cg_ctrl2, - 0x8060033f | (0x40 << (en ? 0x1 : 0))); - write32(&ch[chn].phy.misc_cg_ctrl2, - 0x8060033e | (0x40 << (en ? 0x1 : 0))); - - clrsetbits32(&ch[chn].phy.misc_ctrl3, 0x3 << 26, - (en ? 0 : 0x3) << 26); - for (size_t i = 0; i < DRAM_DFS_SHUFFLE_MAX; i++) { - u32 mask = 0x7 << 17; - u32 value = (en ? 0x7 : 0) << 17; - struct ddrphy_ao_shu *shu = &ch[chn].phy.shu[i]; - - clrsetbits32(&shu->b[0].dq[7], mask, value); - clrsetbits32(&shu->b[1].dq[7], mask, value); - clrsetbits32(&shu->ca_cmd[7], mask, value); - } - - dramc_phy_dcm_2_channel(chn, en); + for (size_t i = 0; i < DRAM_DFS_SHUFFLE_MAX; i++) { + struct ddrphy_ao_shu *shu = &ch[chn].phy.shu[i]; + setbits32(&shu->b[0].dll[0], 0x1); + setbits32(&shu->b[1].dll[0], 0x1); + setbits32(&shu->ca_dll[0], 0x1); } + + clrsetbits32(&ch[chn].ao.dramc_pd_ctrl, + (0x1 << 0) | (0x1 << 1) | (0x1 << 2) | + (0x1 << 5) | (0x1 << 26) | (0x1 << 30) | (0x1 << 31), + ((en ? 0x1 : 0) << 0) | ((en ? 0x1 : 0) << 1) | + ((en ? 0x1 : 0) << 2) | ((en ? 0 : 0x1) << 5) | + ((en ? 0 : 0x1) << 26) | ((en ? 0x1 : 0) << 30) | + ((en ? 0x1 : 0) << 31)); + + /* DCM on: CHANNEL_EMI free run; DCM off: mem_dcm */ + write32(&ch[chn].phy.misc_cg_ctrl2, + 0x8060033e | (0x40 << (en ? 0x1 : 0))); + write32(&ch[chn].phy.misc_cg_ctrl2, + 0x8060033f | (0x40 << (en ? 0x1 : 0))); + write32(&ch[chn].phy.misc_cg_ctrl2, + 0x8060033e | (0x40 << (en ? 0x1 : 0))); + + clrsetbits32(&ch[chn].phy.misc_ctrl3, 0x3 << 26, + (en ? 0 : 0x3) << 26); + for (size_t i = 0; i < DRAM_DFS_SHUFFLE_MAX; i++) { + u32 mask = 0x7 << 17; + u32 value = (en ? 0x7 : 0) << 17; + struct ddrphy_ao_shu *shu = &ch[chn].phy.shu[i]; + + clrsetbits32(&shu->b[0].dq[7], mask, value); + clrsetbits32(&shu->b[1].dq[7], mask, value); + clrsetbits32(&shu->ca_cmd[7], mask, value); + } + + dramc_phy_dcm_2_channel(chn, en); }
-static void dramc_reset_delay_chain_before_calibration(void) +static void dramc_reset_delay_chain_before_calibration(size_t chn) { - for (size_t chn = 0; chn < CHANNEL_MAX; chn++) - for (size_t rank = 0; rank < RANK_MAX; rank++) { - struct dramc_ddrphy_regs_shu_rk *rk; - rk = &ch[chn].phy.shu[0].rk[rank]; - clrbits32(&rk->ca_cmd[0], 0xffffff << 0); - clrbits32(&rk->b[0].dq[0], 0xfffffff << 0); - clrbits32(&rk->b[1].dq[0], 0xfffffff << 0); - clrbits32(&rk->b[0].dq[1], 0xf << 0); - clrbits32(&rk->b[1].dq[1], 0xf << 0); - } + for (size_t rank = 0; rank < RANK_MAX; rank++) { + struct dramc_ddrphy_regs_shu_rk *rk = + &ch[chn].phy.shu[0].rk[rank]; + clrbits32(&rk->ca_cmd[0], 0xffffff << 0); + clrbits32(&rk->b[0].dq[0], 0xfffffff << 0); + clrbits32(&rk->b[1].dq[0], 0xfffffff << 0); + clrbits32(&rk->b[0].dq[1], 0xf << 0); + clrbits32(&rk->b[1].dq[1], 0xf << 0); + } }
void dramc_hw_gating_onoff(u8 chn, bool on) @@ -394,29 +389,31 @@
void dramc_apply_config_before_calibration(u8 freq_group) { - dramc_enable_phy_dcm(false); - dramc_reset_delay_chain_before_calibration(); + for (u8 chn = 0; chn < CHANNEL_MAX; chn++) { + dramc_enable_phy_dcm(chn, false); + dramc_reset_delay_chain_before_calibration(chn);
- setbits32(&ch[0].ao.shu[0].conf[3], 0x1ff << 16); - setbits32(&ch[0].ao.spcmdctrl, 0x1 << 24); - clrsetbits32(&ch[0].ao.shu[0].scintv, 0x1f << 1, 0x1b << 1); + setbits32(&ch[chn].ao.shu[0].conf[3], 0x1ff << 16); + setbits32(&ch[chn].ao.spcmdctrl, 0x1 << 24); + clrsetbits32(&ch[chn].ao.shu[0].scintv, 0x1f << 1, 0x1b << 1);
- for (size_t shu = DRAM_DFS_SHUFFLE_1; shu < DRAM_DFS_SHUFFLE_MAX; shu++) - setbits32(&ch[0].ao.shu[shu].conf[3], 0x1ff << 0); + for (u8 shu = DRAM_DFS_SHUFFLE_1; shu < DRAM_DFS_SHUFFLE_MAX; + shu++) + setbits32(&ch[chn].ao.shu[shu].conf[3], 0x1ff << 0);
- clrbits32(&ch[0].ao.dramctrl, 0x1 << 18); - clrbits32(&ch[0].ao.spcmdctrl, 0x1 << 31); - clrbits32(&ch[0].ao.spcmdctrl, 0x1 << 30); - clrbits32(&ch[0].ao.dqsoscr, 0x1 << 26); - clrbits32(&ch[0].ao.dqsoscr, 0x1 << 25); + clrbits32(&ch[chn].ao.dramctrl, 0x1 << 18); + clrbits32(&ch[chn].ao.spcmdctrl, 0x1 << 31); + clrbits32(&ch[chn].ao.spcmdctrl, 0x1 << 30); + clrbits32(&ch[chn].ao.dqsoscr, 0x1 << 26); + clrbits32(&ch[chn].ao.dqsoscr, 0x1 << 25);
- dramc_write_dbi_onoff(false); - dramc_read_dbi_onoff(false); + dramc_write_dbi_onoff(chn, false); + dramc_read_dbi_onoff(chn, false);
- for (size_t chn = 0; chn < CHANNEL_MAX; chn++) { setbits32(&ch[chn].ao.spcmdctrl, 0x1 << 29); setbits32(&ch[chn].ao.dqsoscr, 0x1 << 24); - for (size_t shu = DRAM_DFS_SHUFFLE_1; shu < DRAM_DFS_SHUFFLE_MAX; shu++) + for (u8 shu = DRAM_DFS_SHUFFLE_1; shu < DRAM_DFS_SHUFFLE_MAX; + shu++) setbits32(&ch[chn].ao.shu[shu].scintv, 0x1 << 30);
clrbits32(&ch[chn].ao.dummy_rd, (0x1 << 7) | (0x7 << 20)); @@ -787,7 +784,6 @@ SET32_BITFIELDS(&ch[chn].ao.spcmd, SPCMD_DQSGCNTRST, 0); SET32_BITFIELDS(&ch[chn].phy.misc_ctrl1, MISC_CTRL1_R_DMSTBENCMP_RK, rank); - }
static void set_selph_gating_value(uint32_t *addr, u8 dly, u8 dly_p1) diff --git a/src/soc/mediatek/mt8183/include/soc/dramc_pi_api.h b/src/soc/mediatek/mt8183/include/soc/dramc_pi_api.h index 59eb6dd..c13aa01 100644 --- a/src/soc/mediatek/mt8183/include/soc/dramc_pi_api.h +++ b/src/soc/mediatek/mt8183/include/soc/dramc_pi_api.h @@ -113,7 +113,7 @@ int dramc_calibrate_all_channels(const struct sdram_params *pams, u8 freq_group, const struct mr_value *mr); void dramc_hw_gating_onoff(u8 chn, bool onoff); -void dramc_enable_phy_dcm(bool bEn); +void dramc_enable_phy_dcm(u8 chn, bool bEn); void dramc_mode_reg_write(u8 chn, u8 mr_idx, u8 value); void dramc_cke_fix_onoff(u8 chn, bool fix_on, bool fix_off); u32 get_shu_freq(u8 shu);
-
Angel Pons (Code Review) -
build bot (Jenkins) (Code Review)
-
Duan huayang (Code Review)
-
huayang duan (Code Review)
-
Hung-Te Lin (Code Review)
-
Patrick Georgi (Code Review)
-
Paul Menzel (Code Review)
-
Yu-Ping Wu (Code Review)