Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34990 )
Change subject: mediatek/mt8183: Use different DRAM frequencies for eMCP DDR ......................................................................
Patch Set 29:
(2 comments)
https://review.coreboot.org/c/coreboot/+/34990/28/src/soc/mediatek/mt8183/em... File src/soc/mediatek/mt8183/emi.c:
https://review.coreboot.org/c/coreboot/+/34990/28/src/soc/mediatek/mt8183/em... PS28, Line 22: enum { : LP4X_LOW, : LP4X_MIDDLE, : LP4X_HIGH, : LP4X_FREQ_NUM, : }; : : #if CONFIG(MT8183_DRAM_EMCP) : static u8 lp4x_freq[LP4X_FREQ_NUM] = { : [LP4X_LOW] = LP4X_DDR1600, : [LP4X_MIDDLE] = LP4X_DDR3200, : [LP4X_HIGH] = LP4X_DDR3600, : }; : #else : static u8 lp4x_freq[LP4X_FREQ_NUM] = { : [LP4X_LOW] = LP4X_DDR1600, : [LP4X_MIDDLE] = LP4X_DDR2400, : [LP4X_HIGH] = LP4X_DDR3200, : }; : #endif
will it be better if we do this inside a function? […]
Ack
https://review.coreboot.org/c/coreboot/+/34990/29/src/soc/mediatek/mt8183/em... File src/soc/mediatek/mt8183/emi.c:
https://review.coreboot.org/c/coreboot/+/34990/29/src/soc/mediatek/mt8183/em... PS29, Line 23: #if CONFIG(MT8183_DRAM_EMCP) : static u8 freq_tbl[DRAM_DFS_SHUFFLE_MAX] = { : [DRAM_DFS_SHUFFLE_1] = LP4X_DDR3600, : [DRAM_DFS_SHUFFLE_2] = LP4X_DDR3200, : [DRAM_DFS_SHUFFLE_3] = LP4X_DDR1600, : }; : #else : static u8 freq_tbl[DRAM_DFS_SHUFFLE_MAX] = { : [DRAM_DFS_SHUFFLE_1] = LP4X_DDR3200, : [DRAM_DFS_SHUFFLE_2] = LP4X_DDR2400, : [DRAM_DFS_SHUFFLE_3] = LP4X_DDR1600, : }; : #endif Let's support both and let the code later do selection. ie..,
static const u8 const freq_shuffle[...] = { ... };
static const u8 const freq_shuffle_emcp[...] = { ... };
And in mt_set_emi:
u8 current_freqsel;
if (CONFIG(MT8183_DRAM_EMCP)) current_freqsel = freq_shuffle_emcp[DRAM_DFS_SHUFFLE_1]; else ...