Change in coreboot[master]: soc/mediatek/mt8183: Set correct threshold of EMI bandwidth for DVFS
huayang duan has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/39034 )
Change subject: soc/mediatek/mt8183: Set correct threshold of EMI bandwidth for DVFS ......................................................................
soc/mediatek/mt8183: Set correct threshold of EMI bandwidth for DVFS
The threshold of EMI bandwidth should have difference setting for discrete and EMCP DDR, because the discrete DDR and EMCP DDR have different DVFS tables.
BRANCH=kukui BUG=none TEST=bootup pass
Change-Id: I82c3c70bcd90df3fdd613c0353aba0f176bc82bc Signed-off-by: Huayang Duan huayang.duan@mediatek.com --- M src/soc/mediatek/mt8183/emi.c 1 file changed, 4 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/34/39034/1
diff --git a/src/soc/mediatek/mt8183/emi.c b/src/soc/mediatek/mt8183/emi.c index cf104f8..7e535dc 100644 --- a/src/soc/mediatek/mt8183/emi.c +++ b/src/soc/mediatek/mt8183/emi.c @@ -299,8 +299,10 @@
setbits32(&emi_mpu->mpu_ctrl_d[1], 0x1 << 4); setbits32(&emi_mpu->mpu_ctrl_d[7], 0x1 << 4); - - write32(&emi_regs->bwct0, 0x0a000705); + if (CONFIG_MT8183_DRAM_EMCP) + write32(&emi_regs->bwct0, 0x0d000705); + else + write32(&emi_regs->bwct0, 0x0a000705); write32(&emi_regs->bwct0_3rd, 0x0);
/* EMI QoS 0.5 */
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39034 )
Change subject: soc/mediatek/mt8183: Set correct threshold of EMI bandwidth for DVFS ......................................................................
Patch Set 1:
(3 comments)
https://review.coreboot.org/c/coreboot/+/39034/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39034/1//COMMIT_MSG@9 PS1, Line 9: have difference setting different setting
or
should differ
https://review.coreboot.org/c/coreboot/+/39034/1//COMMIT_MSG@12 PS1, Line 12: What problem is caused? Failed boot?
What is the difference between d (1101) and a (1010), that means, do the three changed bits do?
https://review.coreboot.org/c/coreboot/+/39034/1//COMMIT_MSG@15 PS1, Line 15: TEST=bootup pass Before it failed?
Hello Yu-Ping Wu, Duan huayang, Julius Werner, Derek Waldner, Hung-Te Lin, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39034
to look at the new patch set (#2).
Change subject: soc/mediatek/mt8183: Set correct threshold of EMI bandwidth for DVFS switch ......................................................................
soc/mediatek/mt8183: Set correct threshold of EMI bandwidth for DVFS switch
The threshold level of EMI bandwidth should have different setting for discrete and EMCP DDR in DVFS switch policy, if the bandwidth reach to threshold level, system will notify DVFS module do DVFS switch.
The discrete DDR and EMCP DDR have different DVFS tables, for EMCP DDR, the DRAM frequency are 1600Mbps, 3200Mbps, 3600Mbps, for discrtet DDR, the DRAM frequecy are 1600Mbps, 2400Mbps, 3200Mbps. So the threshold level of discrete and EMCP DDR must have diff setting.
BRANCH=kukui BUG=none TEST=bootup pass
Change-Id: I82c3c70bcd90df3fdd613c0353aba0f176bc82bc Signed-off-by: Huayang Duan huayang.duan@mediatek.com --- M src/soc/mediatek/mt8183/emi.c 1 file changed, 4 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/34/39034/2
Hello Yu-Ping Wu, Duan huayang, Julius Werner, Derek Waldner, Hung-Te Lin, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39034
to look at the new patch set (#3).
Change subject: soc/mediatek/mt8183: Set correct threshold of EMI bandwidth for DVFS switch ......................................................................
soc/mediatek/mt8183: Set correct threshold of EMI bandwidth for DVFS switch
The threshold level of EMI bandwidth should have different setting for discrete and EMCP DDR in DVFS switch policy, if the bandwidth reach to threshold level, system will notify DVFS module do DVFS switch.
The discrete DDR and EMCP DDR have different DVFS tables, for EMCP DDR, the DRAM frequency are 1600Mbps, 3200Mbps, 3600Mbps, for discrtet DDR, the DRAM frequecy are 1600Mbps, 2400Mbps, 3200Mbps. So the threshold level of discrete and EMCP DDR must have diff setting.
BRANCH=kukui BUG=none TEST=bootup pass
Change-Id: I82c3c70bcd90df3fdd613c0353aba0f176bc82bc Signed-off-by: Huayang Duan huayang.duan@mediatek.com --- M src/soc/mediatek/mt8183/emi.c 1 file changed, 4 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/34/39034/3
Hello Yu-Ping Wu, Duan huayang, Julius Werner, Derek Waldner, Hung-Te Lin, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39034
to look at the new patch set (#4).
Change subject: soc/mediatek/mt8183: Set correct threshold of EMI bandwidth for DVFS switch ......................................................................
soc/mediatek/mt8183: Set correct threshold of EMI bandwidth for DVFS switch
The threshold level of EMI bandwidth should have different setting between discrete and EMCP DDR in DVFS switch policy. If the emi total bandwidth reach to threshold level, system will notify DVFS module do DVFS switch for system preformance or lower power require.
The discrete DDR and EMCP DDR have different DVFS tables, for EMCP DDR, the DRAM frequency are 1600Mbps, 3200Mbps, 3600Mbps, for discrtet DDR, the DRAM frequecy are 1600Mbps, 2400Mbps, 3200Mbps. So the threshold level of discrete and EMCP DDR must have diff setting.
BRANCH=kukui BUG=none TEST=bootup pass
Change-Id: I82c3c70bcd90df3fdd613c0353aba0f176bc82bc Signed-off-by: Huayang Duan huayang.duan@mediatek.com --- M src/soc/mediatek/mt8183/emi.c 1 file changed, 4 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/34/39034/4
Duan huayang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39034 )
Change subject: soc/mediatek/mt8183: Set correct threshold of EMI bandwidth for DVFS switch ......................................................................
Patch Set 4:
(3 comments)
https://review.coreboot.org/c/coreboot/+/39034/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39034/1//COMMIT_MSG@9 PS1, Line 9: have difference setting
different setting […]
Done
https://review.coreboot.org/c/coreboot/+/39034/1//COMMIT_MSG@12 PS1, Line 12:
What problem is caused? Failed boot? […]
D is 13,A is 10, this is threshold level of DVFS switch policy. EMCP and discrete DDR have different DRAM frequency table. for EMCP DDR, the DRAM frequency are 1600Mbps, 3200Mbps, 3600Mbps, for 1600Mbps auto switch to 3200Mbps need threshold level reach to 0x7,for 3200Mbps auto switch to 3600Mbps need threshold level reach to 0xD, but discrtet DDR, the DRAM frequecy are 1600Mbps, 2400Mbps, 3200Mbps. for 2400Mbps auto switch to 3200Mbps only need threshold level reach to 0xA old setting will make DVFS switch more frequently, it affect the low power only.
https://review.coreboot.org/c/coreboot/+/39034/1//COMMIT_MSG@15 PS1, Line 15: TEST=bootup pass
Before it failed?
ditto
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39034 )
Change subject: soc/mediatek/mt8183: Set correct threshold of EMI bandwidth for DVFS switch ......................................................................
Patch Set 4: Code-Review+1
Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39034 )
Change subject: soc/mediatek/mt8183: Set correct threshold of EMI bandwidth for DVFS switch ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39034/4/src/soc/mediatek/mt8183/emi... File src/soc/mediatek/mt8183/emi.c:
https://review.coreboot.org/c/coreboot/+/39034/4/src/soc/mediatek/mt8183/emi... PS4, Line 305: 0x0a000705 I checked the datasheet and thought BW_2ND_INT_BW_THR should be set in bits 22:16 instead of the highest 8 bits. @huayang could you double check this?
Duan huayang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39034 )
Change subject: soc/mediatek/mt8183: Set correct threshold of EMI bandwidth for DVFS switch ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39034/4/src/soc/mediatek/mt8183/emi... File src/soc/mediatek/mt8183/emi.c:
https://review.coreboot.org/c/coreboot/+/39034/4/src/soc/mediatek/mt8183/emi... PS4, Line 305: 0x0a000705
I checked the datasheet and thought BW_2ND_INT_BW_THR should be set in bits 22:16 instead of the hig […]
the datasheet have mistake here, from the hardware design, BW_2ND_INT_BW_THR should be [30:24], not [22:16], however, the logic of DRAM driver is correct, only the datasheet offset is wrong.
Yu-Ping Wu has uploaded a new patch set (#5) to the change originally created by huayang duan. ( https://review.coreboot.org/c/coreboot/+/39034 )
Change subject: soc/mediatek/mt8183: Correct EMI bandwidth threshold for DVFS switch ......................................................................
soc/mediatek/mt8183: Correct EMI bandwidth threshold for DVFS switch
Because eMCP and discrete DDR devices have different DVFS tables, their EMI bandwidth thresholds should also be different. When the EMI total bandwidth reaches the threshold, the system will notify DVFS module to perform DVFS switch for system performance in low power states.
This patch increases the threshold from 0xa to 0xd for eMCP DDR devices so that DVFS switch will be less likely to happen.
BRANCH=kukui BUG=b:142358843 TEST=bootup pass
Change-Id: I82c3c70bcd90df3fdd613c0353aba0f176bc82bc Signed-off-by: Huayang Duan huayang.duan@mediatek.com --- M src/soc/mediatek/mt8183/emi.c 1 file changed, 4 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/34/39034/5
Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39034 )
Change subject: soc/mediatek/mt8183: Correct EMI bandwidth threshold for DVFS switch ......................................................................
Patch Set 5:
(3 comments)
https://review.coreboot.org/c/coreboot/+/39034/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39034/1//COMMIT_MSG@12 PS1, Line 12:
D is 13,A is 10, this is threshold level of DVFS switch policy. […]
Ack
https://review.coreboot.org/c/coreboot/+/39034/1//COMMIT_MSG@15 PS1, Line 15: TEST=bootup pass
ditto
Ack
https://review.coreboot.org/c/coreboot/+/39034/4/src/soc/mediatek/mt8183/emi... File src/soc/mediatek/mt8183/emi.c:
https://review.coreboot.org/c/coreboot/+/39034/4/src/soc/mediatek/mt8183/emi... PS4, Line 305: 0x0a000705
the datasheet have mistake here, from the hardware design, BW_2ND_INT_BW_THR should be [30:24], not […]
Ack
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39034 )
Change subject: soc/mediatek/mt8183: Correct EMI bandwidth threshold for DVFS switch ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39034/4/src/soc/mediatek/mt8183/emi... File src/soc/mediatek/mt8183/emi.c:
https://review.coreboot.org/c/coreboot/+/39034/4/src/soc/mediatek/mt8183/emi... PS4, Line 305: 0x0a000705
Ack
Please add a comment, that the datasheet is incorrect.
Hello Hung-Te Lin, build bot (Jenkins), Paul Menzel, Julius Werner, Derek Waldner, Yu-Ping Wu, Duan huayang,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39034
to look at the new patch set (#6).
Change subject: soc/mediatek/mt8183: Correct EMI bandwidth threshold for DVFS switch ......................................................................
soc/mediatek/mt8183: Correct EMI bandwidth threshold for DVFS switch
Because eMCP and discrete DDR devices have different DVFS tables, their EMI bandwidth thresholds should also be different. When the EMI total bandwidth reaches the threshold, the system will notify DVFS module to perform DVFS switch for system performance in low power states.
This patch increases the threshold from 0xa to 0xd for eMCP DDR devices so that DVFS switch will be less likely to happen.
The datasheet of register bwct0 have mistake here, from the hardware design, BW_2ND_INT_BW_THR should be [30:24], not [22:16]. However, the logic of DRAM driver is correct, same as the hardware design, so only the datasheet of reg bwct0's offset is wrong.
BRANCH=kukui BUG=b:142358843 TEST=bootup pass
Change-Id: I82c3c70bcd90df3fdd613c0353aba0f176bc82bc Signed-off-by: Huayang Duan huayang.duan@mediatek.com --- M src/soc/mediatek/mt8183/emi.c 1 file changed, 4 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/34/39034/6
Duan huayang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39034 )
Change subject: soc/mediatek/mt8183: Correct EMI bandwidth threshold for DVFS switch ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39034/4/src/soc/mediatek/mt8183/emi... File src/soc/mediatek/mt8183/emi.c:
https://review.coreboot.org/c/coreboot/+/39034/4/src/soc/mediatek/mt8183/emi... PS4, Line 305: 0x0a000705
Please add a comment, that the datasheet is incorrect.
Done
Yu-Ping Wu has uploaded a new patch set (#7) to the change originally created by huayang duan. ( https://review.coreboot.org/c/coreboot/+/39034 )
Change subject: soc/mediatek/mt8183: Correct EMI bandwidth threshold for DVFS switch ......................................................................
soc/mediatek/mt8183: Correct EMI bandwidth threshold for DVFS switch
Because eMCP and discrete DDR devices have different DVFS tables, their EMI bandwidth thresholds should also be different. When the EMI total bandwidth reaches the threshold, the system will notify DVFS module to perform DVFS switch for system performance in low power states.
This patch increases the threshold from 0xa to 0xd for eMCP DDR devices so that DVFS switch will be less likely to happen.
The register table of EMI_BWCT0 is incorrect in the datasheet. According to the hardware design, BW_2ND_INT_BW_THR should be in bits [30:24] instead of [22:16]. However, the logic in DRAM driver is correct, aligned with the hardware design, so we don't need to correct it.
BRANCH=kukui BUG=b:142358843 TEST=bootup pass
Change-Id: I82c3c70bcd90df3fdd613c0353aba0f176bc82bc Signed-off-by: Huayang Duan huayang.duan@mediatek.com --- M src/soc/mediatek/mt8183/emi.c 1 file changed, 4 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/34/39034/7
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39034 )
Change subject: soc/mediatek/mt8183: Correct EMI bandwidth threshold for DVFS switch ......................................................................
Patch Set 7: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/39034 )
Change subject: soc/mediatek/mt8183: Correct EMI bandwidth threshold for DVFS switch ......................................................................
soc/mediatek/mt8183: Correct EMI bandwidth threshold for DVFS switch
Because eMCP and discrete DDR devices have different DVFS tables, their EMI bandwidth thresholds should also be different. When the EMI total bandwidth reaches the threshold, the system will notify DVFS module to perform DVFS switch for system performance in low power states.
This patch increases the threshold from 0xa to 0xd for eMCP DDR devices so that DVFS switch will be less likely to happen.
The register table of EMI_BWCT0 is incorrect in the datasheet. According to the hardware design, BW_2ND_INT_BW_THR should be in bits [30:24] instead of [22:16]. However, the logic in DRAM driver is correct, aligned with the hardware design, so we don't need to correct it.
BRANCH=kukui BUG=b:142358843 TEST=bootup pass
Change-Id: I82c3c70bcd90df3fdd613c0353aba0f176bc82bc Signed-off-by: Huayang Duan huayang.duan@mediatek.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/39034 Reviewed-by: Hung-Te Lin hungte@chromium.org Reviewed-by: Paul Menzel paulepanter@users.sourceforge.net Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/mediatek/mt8183/emi.c 1 file changed, 4 insertions(+), 2 deletions(-)
Approvals: build bot (Jenkins): Verified Paul Menzel: Looks good to me, but someone else must approve Hung-Te Lin: Looks good to me, approved
diff --git a/src/soc/mediatek/mt8183/emi.c b/src/soc/mediatek/mt8183/emi.c index 07d1cc8..e7cbda1 100644 --- a/src/soc/mediatek/mt8183/emi.c +++ b/src/soc/mediatek/mt8183/emi.c @@ -299,8 +299,10 @@
setbits32(&emi_mpu->mpu_ctrl_d[1], 0x1 << 4); setbits32(&emi_mpu->mpu_ctrl_d[7], 0x1 << 4); - - write32(&emi_regs->bwct0, 0x0a000705); + if (CONFIG(MT8183_DRAM_EMCP)) + write32(&emi_regs->bwct0, 0x0d000705); + else + write32(&emi_regs->bwct0, 0x0a000705); write32(&emi_regs->bwct0_3rd, 0x0);
/* EMI QoS 0.5 */
-
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)