Change in coreboot[master]: soc/mediatek/mt8192: Switch to highest DDR frequency to reduce bootup...
Hello Duan huayang,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/44730
to review the following change.
Change subject: soc/mediatek/mt8192: Switch to highest DDR frequency to reduce bootup time ......................................................................
soc/mediatek/mt8192: Switch to highest DDR frequency to reduce bootup time
Signed-off-by: Huayang Duan huayang.duan@mediatek.com Change-Id: Ib37ecc7bf3f1776d27161948e779ed1f96ee9a0c --- M src/soc/mediatek/mt8192/dramc_dvfs.c M src/soc/mediatek/mt8192/dramc_pi_main.c 2 files changed, 197 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/30/44730/1
diff --git a/src/soc/mediatek/mt8192/dramc_dvfs.c b/src/soc/mediatek/mt8192/dramc_dvfs.c index eb37ac3..fbb23bb 100644 --- a/src/soc/mediatek/mt8192/dramc_dvfs.c +++ b/src/soc/mediatek/mt8192/dramc_dvfs.c @@ -3,6 +3,11 @@ #include <soc/dramc_pi_api.h> #include <soc/dramc_register.h>
+typedef enum { + SRAM_SHU_TYPE_LOAD, + SRAM_SHU_TYPE_RESTORE, +} sram_shu_type; + void enable_dfs_hw_mode_clk(void) { for (u8 chn = 0; chn < CHANNEL_MAX; chn++) { @@ -17,6 +22,62 @@ } }
+static void no_queue_flush_wa(bool wa_enable) +{ + u32 bc_bak = 0; + static u32 perfctl0_bak = 0; + + bc_bak = dramc_get_broadcast(); + dramc_set_broadcast(DRAMC_BROADCAST_ON); + + if (wa_enable) { + perfctl0_bak = (read32(&ch[0].ao.perfctl0) >> 10) & 0x3; + SET32_BITFIELDS(&ch[0].ao.perfctl0, + PERFCTL0_RWAGEEN, 0, + PERFCTL0_EMILLATEN, 0); + } else { + SET32_BITFIELDS(&ch[0].ao.perfctl0, + PERFCTL0_RWAGEEN, perfctl0_bak & 0x1, + PERFCTL0_EMILLATEN, (perfctl0_bak >>1) & 0x1); + } + + dramc_set_broadcast(bc_bak); +} + +static void wait_sram_shu_ack(sram_shu_type type) +{ + u8 ack_state = 0, complete = 1; + + for (u8 chn = CHANNEL_A; chn < CHANNEL_MAX; chn++) { + do { + if (type == SRAM_SHU_TYPE_LOAD) + ack_state = READ32_BITFIELD(&ch[chn].phy_nao.misc_dma_debug0, + MISC_DMA_DEBUG0_SC_DR_SRAM_LOAD_ACK); + else + ack_state = READ32_BITFIELD(&ch[chn].phy_nao.misc_dma_debug0, + MISC_DMA_DEBUG0_SC_DR_SRAM_RESTORE_ACK); + } while(ack_state != complete); + } +} + +static void timing_tx_sr(u32 shu_level) +{ + u32 onoff = 0, bc_bak = 0; + + bc_bak = dramc_get_broadcast(); + dramc_set_broadcast(DRAMC_BROADCAST_ON); + + if ((shu_level == DRAM_DFS_SHU4) || (shu_level == DRAM_DFS_SHU5) || + (shu_level == DRAM_DFS_SHU6)) + onoff = 0; + else + onoff = 1; + + SET32_BITFIELDS(&ch[0].ao.refctrl1, + REFCTRL1_REF_OVERHEAD_SLOW_REFPB_ENA, onoff); + dramc_set_broadcast(bc_bak); +} + void dramc_dfs_direct_jump_rg_mode(const struct ddr_cali *cali, u8 shu_level) { u8 shu_ack = 0; @@ -109,6 +170,129 @@
pll_mode = !pll_mode; *(cali->pll_mode) = pll_mode; + dramc_dbg("%s end with pll_mode:%d\n", __func__, *(cali->pll_mode)); +} + +void dramc_dfs_direct_jump_sram_shu_rg_mode(const struct ddr_cali *cali, + dram_dfs_shu shu_level) +{ + u8 shu_ack = 0; + u8 pll_mode = *(cali->pll_mode); + u32 *shu_ack_reg = &mtk_dpm->status_4; + + if (pll_mode == PHYPLL_MODE) { + dramc_dbg("Disable CLRPLL\n"); + for (u8 chn = 0; chn < CHANNEL_MAX; chn++) + SET32_BITFIELDS(&ch[chn].phy_ao.clrpll0, CLRPLL0_RG_RCLRPLL_EN, 0); + } else { + dramc_dbg("Disable PHYPLL\n"); + for (u8 chn = 0; chn < CHANNEL_MAX; chn++) + SET32_BITFIELDS(&ch[chn].phy_ao.phypll0, PHYPLL0_RG_RPHYPLL_EN, 0); + } + + for (u8 chn = 0; chn < CHANNEL_MAX; chn++) + shu_ack |= (0x1 << chn); + + if (pll_mode == PHYPLL_MODE) + dramc_dbg("DFSDirectJump to CLRPLL, SHU_LEVEL=%d, ACK=%x\n", shu_level, shu_ack); + else + dramc_dbg("DFSDirectJump to PHYPLL, SHU_LEVEL=%d, ACK=%x\n", shu_level, shu_ack); + + for (u8 chn = 0; chn < CHANNEL_MAX; chn++) { + SET32_BITFIELDS(&ch[chn].phy_ao.misc_rg_dfs_ctrl, + MISC_RG_DFS_CTRL_RG_DDRPHY_FB_CK_EN, 1); + SET32_BITFIELDS(&ch[chn].phy_ao.misc_rg_dfs_ctrl, + MISC_RG_DFS_CTRL_RG_DR_SHU_LEVEL_SRAM_LATCH, 1); + } + + udelay(1); + for (u8 chn = 0; chn < CHANNEL_MAX; chn++) + SET32_BITFIELDS(&ch[chn].phy_ao.misc_rg_dfs_ctrl, + MISC_RG_DFS_CTRL_RG_DR_SHU_LEVEL_SRAM_LATCH, 0); + + if (pll_mode == PHYPLL_MODE) { + for (u8 chn = 0; chn < CHANNEL_MAX; chn++) { + SET32_BITFIELDS(&ch[chn].phy_ao.misc_rg_dfs_ctrl, + MISC_RG_DFS_CTRL_RG_PHYPLL_SHU_EN, 0); + SET32_BITFIELDS(&ch[chn].phy_ao.misc_rg_dfs_ctrl, + MISC_RG_DFS_CTRL_RG_DR_SHU_LEVEL, !pll_mode); + SET32_BITFIELDS(&ch[chn].phy_ao.misc_rg_dfs_ctrl, + MISC_RG_DFS_CTRL_RG_PHYPLL2_SHU_EN, 1); + } + dramc_dbg("Enable CLRPLL\n"); + } else { + for (u8 chn = 0; chn < CHANNEL_MAX; chn++) { + SET32_BITFIELDS(&ch[chn].phy_ao.misc_rg_dfs_ctrl, + MISC_RG_DFS_CTRL_RG_PHYPLL2_SHU_EN, 0); + SET32_BITFIELDS(&ch[chn].phy_ao.misc_rg_dfs_ctrl, + MISC_RG_DFS_CTRL_RG_DR_SHU_LEVEL, !pll_mode); + SET32_BITFIELDS(&ch[chn].phy_ao.misc_rg_dfs_ctrl, + MISC_RG_DFS_CTRL_RG_PHYPLL_SHU_EN, 1); + } + dramc_dbg("Enable PHYPLL\n"); + } + + udelay(1); + for (u8 chn = 0; chn < CHANNEL_MAX; chn++) { + SET32_BITFIELDS(&ch[chn].phy_ao.misc_rg_dfs_ctrl, + MISC_RG_DFS_CTRL_RG_DR_SHU_LEVEL_SRAM, shu_level); + SET32_BITFIELDS(&ch[chn].phy_ao.misc_rg_dfs_ctrl, + MISC_RG_DFS_CTRL_RG_DR_SRAM_LOAD, 1); + } + + wait_sram_shu_ack(SRAM_SHU_TYPE_LOAD); + + for (u8 chn = 0; chn < CHANNEL_MAX; chn++) + SET32_BITFIELDS(&ch[chn].phy_ao.misc_rg_dfs_ctrl, + MISC_RG_DFS_CTRL_RG_DR_SRAM_LOAD, 0); + + if (pll_mode == PHYPLL_MODE) + for (u8 chn = 0; chn < CHANNEL_MAX; chn++) + SET32_BITFIELDS(&ch[chn].phy_ao.clrpll0, CLRPLL0_RG_RCLRPLL_EN, 1); + else + for (u8 chn = 0; chn < CHANNEL_MAX; chn++) + SET32_BITFIELDS(&ch[chn].phy_ao.phypll0, PHYPLL0_RG_RPHYPLL_EN, 1); + + no_queue_flush_wa(true); + + udelay(20); + + dramc_dbg("SHUFFLE Start\n"); + for (u8 chn = 0; chn < CHANNEL_MAX; chn++) + SET32_BITFIELDS(&ch[chn].phy_ao.misc_rg_dfs_ctrl, + MISC_RG_DFS_CTRL_RG_DR_SHU_EN, 1); + + while ((READ32_BITFIELD(shu_ack_reg, LPIF_STATUS_4_SHU_EN_ACK) != shu_ack)) + dramc_dbg("wait shu_en ack.\n"); + + for (u8 chn = 0; chn < CHANNEL_MAX; chn++) + SET32_BITFIELDS(&ch[chn].phy_ao.misc_rg_dfs_ctrl, MISC_RG_DFS_CTRL_RG_DR_SHU_EN, 0); + dramc_dbg("SHUFFLE End\n"); + + if (pll_mode == PHYPLL_MODE) + for (u8 chn = 0; chn < CHANNEL_MAX; chn++) + SET32_BITFIELDS(&ch[chn].phy_ao.phypll0, PHYPLL0_RG_RPHYPLL_EN, 0); + else + for (u8 chn = 0; chn < CHANNEL_MAX; chn++) + SET32_BITFIELDS(&ch[chn].phy_ao.clrpll0, CLRPLL0_RG_RCLRPLL_EN, 0); + + for (u8 chn = 0; chn < CHANNEL_MAX; chn++) + SET32_BITFIELDS(&ch[chn].phy_ao.misc_rg_dfs_ctrl, MISC_RG_DFS_CTRL_RG_DR_SRAM_RESTORE, 1); + + wait_sram_shu_ack(SRAM_SHU_TYPE_RESTORE); + + for (u8 chn = 0; chn < CHANNEL_MAX; chn++) + SET32_BITFIELDS(&ch[chn].phy_ao.misc_rg_dfs_ctrl, MISC_RG_DFS_CTRL_RG_DR_SRAM_RESTORE, 0); + + no_queue_flush_wa(false); + + for (u8 chn = 0; chn < CHANNEL_MAX; chn++) + SET32_BITFIELDS(&ch[chn].phy_ao.misc_rg_dfs_ctrl, MISC_RG_DFS_CTRL_RG_DDRPHY_FB_CK_EN, 0); + + timing_tx_sr(shu_level); + + dramc_dbg("Shuffle flow complete\n"); + *(cali->pll_mode) = !pll_mode; }
void dvfs_settings(const struct ddr_cali *cali) diff --git a/src/soc/mediatek/mt8192/dramc_pi_main.c b/src/soc/mediatek/mt8192/dramc_pi_main.c index 2bcc449..72f9a24 100644 --- a/src/soc/mediatek/mt8192/dramc_pi_main.c +++ b/src/soc/mediatek/mt8192/dramc_pi_main.c @@ -440,5 +440,18 @@ after_calib(&cali); enable_dfs_hw_mode_clk();
+ if (CONFIG(MT8192_DRAM_DVFS)) { + dram_cali_seq bootup_cali_seq = CALI_SEQ5; + dram_dfs_shu bootup_shu = get_shu_save_by_k_shu(bootup_cali_seq); + + set_cali_datas(&cali, dparam, bootup_cali_seq); + set_vcore_voltage_for_each_freq(&cali); + + dramc_dfs_direct_jump_sram_shu_rg_mode(&cali, DRAM_DFS_SHU1); + dramc_dfs_direct_jump_sram_shu_rg_mode(&cali, bootup_shu); + dramc_info("switch to frequency %d to decrease the bootup time\n", + get_frequency_by_shu(bootup_shu)); + } + dramc_runtime_config(&cali); }
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44730 )
Change subject: soc/mediatek/mt8192: Switch to highest DDR frequency to reduce bootup time ......................................................................
Patch Set 1:
(9 comments)
https://review.coreboot.org/c/coreboot/+/44730/1/src/soc/mediatek/mt8192/dra... File src/soc/mediatek/mt8192/dramc_dvfs.c:
https://review.coreboot.org/c/coreboot/+/44730/1/src/soc/mediatek/mt8192/dra... PS1, Line 41: PERFCTL0_EMILLATEN, (perfctl0_bak >>1) & 0x1); need consistent spacing around '>>' (ctx:WxV)
https://review.coreboot.org/c/coreboot/+/44730/1/src/soc/mediatek/mt8192/dra... PS1, Line 59: } while(ack_state != complete); space required before the open parenthesis '('
https://review.coreboot.org/c/coreboot/+/44730/1/src/soc/mediatek/mt8192/dra... PS1, Line 197: dramc_dbg("DFSDirectJump to CLRPLL, SHU_LEVEL=%d, ACK=%x\n", shu_level, shu_ack); line over 96 characters
https://review.coreboot.org/c/coreboot/+/44730/1/src/soc/mediatek/mt8192/dra... PS1, Line 199: dramc_dbg("DFSDirectJump to PHYPLL, SHU_LEVEL=%d, ACK=%x\n", shu_level, shu_ack); line over 96 characters
https://review.coreboot.org/c/coreboot/+/44730/1/src/soc/mediatek/mt8192/dra... PS1, Line 269: SET32_BITFIELDS(&ch[chn].phy_ao.misc_rg_dfs_ctrl, MISC_RG_DFS_CTRL_RG_DR_SHU_EN, 0); line over 96 characters
https://review.coreboot.org/c/coreboot/+/44730/1/src/soc/mediatek/mt8192/dra... PS1, Line 280: SET32_BITFIELDS(&ch[chn].phy_ao.misc_rg_dfs_ctrl, MISC_RG_DFS_CTRL_RG_DR_SRAM_RESTORE, 1); line over 96 characters
https://review.coreboot.org/c/coreboot/+/44730/1/src/soc/mediatek/mt8192/dra... PS1, Line 285: SET32_BITFIELDS(&ch[chn].phy_ao.misc_rg_dfs_ctrl, MISC_RG_DFS_CTRL_RG_DR_SRAM_RESTORE, 0); line over 96 characters
https://review.coreboot.org/c/coreboot/+/44730/1/src/soc/mediatek/mt8192/dra... PS1, Line 289: for (u8 chn = 0; chn < CHANNEL_MAX; chn++) suspect code indent for conditional statements (8, 8)
https://review.coreboot.org/c/coreboot/+/44730/1/src/soc/mediatek/mt8192/dra... PS1, Line 290: SET32_BITFIELDS(&ch[chn].phy_ao.misc_rg_dfs_ctrl, MISC_RG_DFS_CTRL_RG_DDRPHY_FB_CK_EN, 0); line over 96 characters
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44730 )
Change subject: soc/mediatek/mt8192: Switch to highest DDR frequency to reduce bootup time ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44730/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44730/1//COMMIT_MSG@8 PS1, Line 8: Please give exact numbers, how much the time is reduced.
Hello build bot (Jenkins), Julius Werner, Duan huayang,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44730
to look at the new patch set (#2).
Change subject: soc/mediatek/mt8192: Switch to highest DDR frequency to reduce bootup time ......................................................................
soc/mediatek/mt8192: Switch to highest DDR frequency to reduce bootup time
Signed-off-by: Huayang Duan huayang.duan@mediatek.com Change-Id: Ib37ecc7bf3f1776d27161948e779ed1f96ee9a0c --- M src/soc/mediatek/mt8192/dramc_dvfs.c M src/soc/mediatek/mt8192/dramc_pi_main.c 2 files changed, 195 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/30/44730/2
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44730 )
Change subject: soc/mediatek/mt8192: Switch to highest DDR frequency to reduce bootup time ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44730/2/src/soc/mediatek/mt8192/dra... File src/soc/mediatek/mt8192/dramc_dvfs.c:
https://review.coreboot.org/c/coreboot/+/44730/2/src/soc/mediatek/mt8192/dra... PS2, Line 280: for (u8 chn = 0; chn < CHANNEL_MAX; chn++) suspect code indent for conditional statements (8, 8)
Yidi Lin has uploaded a new patch set (#3) to the change originally created by CK HU. ( https://review.coreboot.org/c/coreboot/+/44730 )
Change subject: soc/mediatek/mt8192: Switch to highest DDR frequency to reduce bootup time ......................................................................
soc/mediatek/mt8192: Switch to highest DDR frequency to reduce bootup time
Signed-off-by: Huayang Duan huayang.duan@mediatek.com Change-Id: Ib37ecc7bf3f1776d27161948e779ed1f96ee9a0c --- M src/soc/mediatek/mt8192/dramc_dvfs.c M src/soc/mediatek/mt8192/dramc_pi_main.c 2 files changed, 195 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/30/44730/3
Yidi Lin has uploaded a new patch set (#24) to the change originally created by CK HU. ( https://review.coreboot.org/c/coreboot/+/44730 )
Change subject: soc/mediatek/mt8192: Switch to highest DDR frequency to reduce bootup time ......................................................................
soc/mediatek/mt8192: Switch to highest DDR frequency to reduce bootup time
Signed-off-by: Huayang Duan huayang.duan@mediatek.com Change-Id: Ib37ecc7bf3f1776d27161948e779ed1f96ee9a0c --- M src/soc/mediatek/mt8192/dramc_dvfs.c M src/soc/mediatek/mt8192/dramc_pi_main.c 2 files changed, 195 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/30/44730/24
Yidi Lin has uploaded a new patch set (#34) to the change originally created by CK HU. ( https://review.coreboot.org/c/coreboot/+/44730 )
Change subject: soc/mediatek/mt8192: Switch to highest DDR frequency to reduce bootup time ......................................................................
soc/mediatek/mt8192: Switch to highest DDR frequency to reduce bootup time
Signed-off-by: Huayang Duan huayang.duan@mediatek.com Change-Id: Ib37ecc7bf3f1776d27161948e779ed1f96ee9a0c --- M src/soc/mediatek/mt8192/dramc_dvfs.c M src/soc/mediatek/mt8192/dramc_pi_main.c 2 files changed, 195 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/30/44730/34
Yidi Lin has uploaded a new patch set (#36) to the change originally created by CK HU. ( https://review.coreboot.org/c/coreboot/+/44730 )
Change subject: soc/mediatek/mt8192: Switch to highest DDR frequency to reduce bootup time ......................................................................
soc/mediatek/mt8192: Switch to highest DDR frequency to reduce bootup time
Signed-off-by: Huayang Duan huayang.duan@mediatek.com Change-Id: Ib37ecc7bf3f1776d27161948e779ed1f96ee9a0c --- M src/soc/mediatek/mt8192/dramc_dvfs.c M src/soc/mediatek/mt8192/dramc_pi_main.c 2 files changed, 195 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/30/44730/36
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44730 )
Change subject: soc/mediatek/mt8192: Switch to highest DDR frequency to reduce bootup time ......................................................................
Patch Set 40:
(4 comments)
https://review.coreboot.org/c/coreboot/+/44730/40//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44730/40//COMMIT_MSG@8 PS40, Line 8: Please elaborate, what the highest frequency is, and give concrete boottime numbers without and with the change.
https://review.coreboot.org/c/coreboot/+/44730/40/src/soc/mediatek/mt8192/dr... File src/soc/mediatek/mt8192/dramc_dvfs.c:
https://review.coreboot.org/c/coreboot/+/44730/40/src/soc/mediatek/mt8192/dr... PS40, Line 34: perfctl0_bak = (read32(&ch[0].ao.perfctl0) >> 10) & 0x3; Is this in the right if branch?
https://review.coreboot.org/c/coreboot/+/44730/40/src/soc/mediatek/mt8192/dr... PS40, Line 33: if (wa_enable) { : perfctl0_bak = (read32(&ch[0].ao.perfctl0) >> 10) & 0x3; : SET32_BITFIELDS(&ch[0].ao.perfctl0, : PERFCTL0_RWAGEEN, 0, : PERFCTL0_EMILLATEN, 0); : } else { : SET32_BITFIELDS(&ch[0].ao.perfctl0, : PERFCTL0_RWAGEEN, perfctl0_bak & 0x1, : PERFCTL0_EMILLATEN, (perfctl0_bak >> 1) & 0x1); : } I’d use the ternary operator:
wa_enable ? 0 : PERFCTL0_RWAGEEN, perfctl0_bak & 0x1
https://review.coreboot.org/c/coreboot/+/44730/40/src/soc/mediatek/mt8192/dr... PS40, Line 49: u8 ack_state = 0, complete = 1; Why not use a boolean?
Attention is currently required from: CK HU, Yidi Lin. Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44730 )
Change subject: soc/mediatek/mt8192: Switch to highest DDR frequency to reduce bootup time ......................................................................
Patch Set 47:
(1 comment)
Patchset:
PS47: @Yidi/XiXi, was this included in our latest dram vendor code?
Attention is currently required from: Hung-Te Lin, CK HU, Yidi Lin. Xi Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44730 )
Change subject: soc/mediatek/mt8192: Switch to highest DDR frequency to reduce bootup time ......................................................................
Patch Set 47:
(1 comment)
Patchset:
PS47:
@Yidi/XiXi, was this included in our latest dram vendor code?
Yes, already included in latest dram vendor code.
CK HU has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/44730 )
Change subject: soc/mediatek/mt8192: Switch to highest DDR frequency to reduce bootup time ......................................................................
Abandoned
Useless
-
build bot (Jenkins) (Code Review) -
CK HU (Code Review)
-
Hung-Te Lin (Code Review)
-
Paul Menzel (Code Review)
-
Xi Chen (Code Review)
-
Yidi Lin (Code Review)