Change in ...coreboot[master]: WIP: mediatek/mt8183: Add infra group DCM setting

[Weiyi Lu (Code Review)]

Hello Weiyi Lu,

I'd like you to do a code review. Please visit

https://review.coreboot.org/c/coreboot/+/31977

to review the following change.

Change subject: WIP: mediatek/mt8183: Add infra group DCM setting
......................................................................

WIP: mediatek/mt8183: Add infra group DCM setting

Add infra group DCM setting

BUG=b:80501386
BRANCH=none
TEST=Boots correctly on Kukui.

Change-Id: I4741dfb7b984deb92171f370e5fb2593829d74c2
Signed-off-by: Weiyi Lu 
---
M src/soc/mediatek/mt8183/include/soc/pll.h
M src/soc/mediatek/mt8183/pll.c
2 files changed, 15 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/77/31977/1

diff --git a/src/soc/mediatek/mt8183/include/soc/pll.h b/src/soc/mediatek/mt8183/include/soc/pll.h
index 5a5f58d..d1c7cd1 100644
--- a/src/soc/mediatek/mt8183/include/soc/pll.h
+++ b/src/soc/mediatek/mt8183/include/soc/pll.h
@@ -270,4 +270,13 @@
    SPI_HZ		= MAINPLL_D5_D2_HZ,
 };

+enum {
+	DCM_INFRA_BUS_MASK = 0x40907ffb,
+	DCM_INFRA_BUS_ON = 0x40900843,
+	DCM_INFRA_MEM_ON = 0x1 << 27,
+	DCM_INFRA_P2PRX_MASK = 0xf,
+	DCM_INFRA_PERI_MASK = 0xf03ffffb,
+	DCM_INFRA_PERI_ON = 0xf03f83e3,
+};
+
 #endif /* SOC_MEDIATEK_MT8183_PLL_H */
diff --git a/src/soc/mediatek/mt8183/pll.c b/src/soc/mediatek/mt8183/pll.c
index 61aa2de..4157c1d 100644
--- a/src/soc/mediatek/mt8183/pll.c
+++ b/src/soc/mediatek/mt8183/pll.c
@@ -344,6 +344,12 @@

/* enable infrasys DCM */
    setbits_le32(&mt8183_infracfg->infra_bus_dcm_ctrl, 0x3 << 21);
+	clrsetbits_le32(&mt8183_infracfg->infra_bus_dcm_ctrl, DCM_INFRA_BUS_MASK,
+		DCM_INFRA_BUS_ON);
+	setbits_le32(&mt8183_infracfg->mem_dcm_ctrl, DCM_INFRA_MEM_ON);
+	clrbits_le32(&mt8183_infracfg->p2p_rx_clk_on, DCM_INFRA_P2PRX_MASK);
+	clrsetbits_le32(&mt8183_infracfg->peri_bus_dcm_ctrl, DCM_INFRA_PERI_MASK,
+		DCM_INFRA_PERI_ON);

/* enable [11] for change i2c module source clock to TOPCKGEN */
    setbits_le32(&mt8183_infracfg->module_clk_sel, 0x1 << 11);

[build bot (Jenkins) (Code Review)]

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31977 )

Change subject: WIP: mediatek/mt8183: Add infra group DCM setting
......................................................................

Patch Set 1:

(2 comments)

https://review.coreboot.org/#/c/31977/1/src/soc/mediatek/mt8183/pll.c 
File src/soc/mediatek/mt8183/pll.c:

https://review.coreboot.org/#/c/31977/1/src/soc/mediatek/mt8183/pll.c@347 
PS1, Line 347: 	clrsetbits_le32(&mt8183_infracfg->infra_bus_dcm_ctrl, DCM_INFRA_BUS_MASK,
line over 80 characters

https://review.coreboot.org/#/c/31977/1/src/soc/mediatek/mt8183/pll.c@351 
PS1, Line 351: 	clrsetbits_le32(&mt8183_infracfg->peri_bus_dcm_ctrl, DCM_INFRA_PERI_MASK,
line over 80 characters

[Weiyi Lu (Code Review)]

Hello Julius Werner, Weiyi Lu,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/31977

to look at the new patch set (#2).

Change subject: WIP: mediatek/mt8183: Add infra group DCM setting
......................................................................

WIP: mediatek/mt8183: Add infra group DCM setting

Add infra group DCM setting

BUG=b:80501386
BRANCH=none
TEST=Boots correctly on Kukui.

Change-Id: I4741dfb7b984deb92171f370e5fb2593829d74c2
Signed-off-by: Weiyi Lu 
---
M src/soc/mediatek/mt8183/include/soc/pll.h
M src/soc/mediatek/mt8183/pll.c
2 files changed, 15 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/77/31977/2

[You-Cheng Syu (Code Review)]

You-Cheng Syu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31977 )

Change subject: WIP: mediatek/mt8183: Add infra group DCM setting
......................................................................

Patch Set 2:

It seems like this CL makes PLL initialization stuck

[Weiyi Lu (Code Review)]

Hello Julius Werner, You-Cheng Syu, Tristan Hsieh, Hung-Te Lin, build bot (Jenkins), Weiyi Lu,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/31977

to look at the new patch set (#3).

Change subject: WIP: mediatek/mt8183: Add infra group DCM setting
......................................................................

WIP: mediatek/mt8183: Add infra group DCM setting

Add infra group DCM setting

BUG=b:80501386
BRANCH=none
TEST=Boots correctly on Kukui.

Change-Id: I4741dfb7b984deb92171f370e5fb2593829d74c2
Signed-off-by: Weiyi Lu 
---
M src/soc/mediatek/mt8183/include/soc/pll.h
M src/soc/mediatek/mt8183/pll.c
2 files changed, 15 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/77/31977/3

[Weiyi Lu (Code Review)]

Weiyi Lu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31977 )

Change subject: WIP: mediatek/mt8183: Add infra group DCM setting
......................................................................

Patch Set 3:
> Patch Set 2:
>
> It seems like this CL makes PLL initialization stuck

Problem solved on patch set 3

[Paul Menzel (Code Review)]

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31977 )

Change subject: WIP: mediatek/mt8183: Add infra group DCM setting
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/#/c/31977/3//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/#/c/31977/3//COMMIT_MSG@10 
PS3, Line 10: 
Please describe, what DCM is in the commit message.

[Weiyi Lu (Code Review)]

Hello Julius Werner, You-Cheng Syu, Tristan Hsieh, Hung-Te Lin, build bot (Jenkins), Weiyi Lu,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/31977

to look at the new patch set (#4).

Change subject: WIP: mediatek/mt8183: Add infra group DCM setting
......................................................................

WIP: mediatek/mt8183: Add infra group DCM setting

Add infra group DCM setting.
DCM is short for Dynamic Clock Management, it'd slow down
or gate clocks while the H/W idle.

BUG=b:80501386
BRANCH=none
TEST=Boots correctly on Kukui.

Change-Id: I4741dfb7b984deb92171f370e5fb2593829d74c2
Signed-off-by: Weiyi Lu 
---
M src/soc/mediatek/mt8183/include/soc/pll.h
M src/soc/mediatek/mt8183/pll.c
2 files changed, 15 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/77/31977/4

[Hung-Te Lin (Code Review)]

Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31977 )

Change subject: WIP: mediatek/mt8183: Add infra group DCM setting
......................................................................

Patch Set 4:

Please remove the WIP label if this is ready for review.

[Hung-Te Lin (Code Review)]

Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31977 )

Change subject: WIP: mediatek/mt8183: Add infra group DCM setting
......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/#/c/31977/4//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/#/c/31977/4//COMMIT_MSG@9 
PS4, Line 9: Add infra group DCM setting.
           : DCM is short for Dynamic Clock Management, it'd slow down
           : or gate clocks while the H/W idle.
Add infra group DCM (Dynamic Clock Management) settings,
which slows down OR gate clocks while hardware is in idle state.

I'm not sure if my description above is correct. Maybe you can revise with given DCM is defined, what is the infra group DCM setting - what does it do or why do we need it?

Change in ...coreboot[master]: mediatek/mt8183: Add infra group DCM setting

[Weiyi Lu (Code Review)]

Hello Julius Werner, You-Cheng Syu, Tristan Hsieh, Hung-Te Lin, build bot (Jenkins), Weiyi Lu,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/31977

to look at the new patch set (#5).

Change subject: mediatek/mt8183: Add infra group DCM setting
......................................................................

mediatek/mt8183: Add infra group DCM setting

Add infra group DCM (Dynamic Clock Management) settings,
which slows down OR gate clocks while hardware is in idle state.

BUG=b:80501386
BRANCH=none
TEST=Boots correctly on Kukui.

Change-Id: I4741dfb7b984deb92171f370e5fb2593829d74c2
Signed-off-by: Weiyi Lu 
---
M src/soc/mediatek/mt8183/include/soc/pll.h
M src/soc/mediatek/mt8183/pll.c
2 files changed, 15 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/77/31977/5

Change in ...coreboot[master]: mediatek/mt8183: Add infra group DCM setting

[You-Cheng Syu (Code Review)]

You-Cheng Syu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31977 )

Change subject: mediatek/mt8183: Add infra group DCM setting
......................................................................

Patch Set 5: Code-Review+1

Looks good to me.
You might need to rebase to eliminate Jenkins failure.

Change in ...coreboot[master]: mediatek/mt8183: Add infra group DCM setting

[Julius Werner (Code Review)]

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31977 )

Change subject: mediatek/mt8183: Add infra group DCM setting
......................................................................

Patch Set 6: Code-Review+2

Change in ...coreboot[master]: mediatek/mt8183: Add infra group DCM setting

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/31977 )

Change subject: mediatek/mt8183: Add infra group DCM setting
......................................................................

mediatek/mt8183: Add infra group DCM setting

Add infra group DCM (Dynamic Clock Management) settings,
which slows down OR gate clocks while hardware is in idle state.

BUG=b:80501386
BRANCH=none
TEST=Boots correctly on Kukui.

Change-Id: I4741dfb7b984deb92171f370e5fb2593829d74c2
Signed-off-by: Weiyi Lu 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/31977
Tested-by: build bot (Jenkins) 
Reviewed-by: Julius Werner 
Reviewed-by: You-Cheng Syu 
---
M src/soc/mediatek/mt8183/include/soc/pll.h
M src/soc/mediatek/mt8183/pll.c
2 files changed, 15 insertions(+), 0 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Julius Werner: Looks good to me, approved
  You-Cheng Syu: Looks good to me, but someone else must approve

diff --git a/src/soc/mediatek/mt8183/include/soc/pll.h b/src/soc/mediatek/mt8183/include/soc/pll.h
index 2a72e2c..3807e00 100644
--- a/src/soc/mediatek/mt8183/include/soc/pll.h
+++ b/src/soc/mediatek/mt8183/include/soc/pll.h
@@ -270,4 +270,13 @@
    SPI_HZ		= MAINPLL_D5_D2_HZ,
 };

+enum {
+	DCM_INFRA_BUS_MASK = 0x40907ffb,
+	DCM_INFRA_BUS_ON = 0x40904203,
+	DCM_INFRA_MEM_ON = 0x1 << 27,
+	DCM_INFRA_P2PRX_MASK = 0xf,
+	DCM_INFRA_PERI_MASK = 0xf03ffffb,
+	DCM_INFRA_PERI_ON = 0xf03f83e3,
+};
+
 #endif /* SOC_MEDIATEK_MT8183_PLL_H */
diff --git a/src/soc/mediatek/mt8183/pll.c b/src/soc/mediatek/mt8183/pll.c
index 07ce660..5368077 100644
--- a/src/soc/mediatek/mt8183/pll.c
+++ b/src/soc/mediatek/mt8183/pll.c
@@ -344,6 +344,12 @@

/* enable infrasys DCM */
    setbits_le32(&mt8183_infracfg->infra_bus_dcm_ctrl, 0x3 << 21);
+	clrsetbits_le32(&mt8183_infracfg->infra_bus_dcm_ctrl,
+		DCM_INFRA_BUS_MASK, DCM_INFRA_BUS_ON);
+	setbits_le32(&mt8183_infracfg->mem_dcm_ctrl, DCM_INFRA_MEM_ON);
+	clrbits_le32(&mt8183_infracfg->p2p_rx_clk_on, DCM_INFRA_P2PRX_MASK);
+	clrsetbits_le32(&mt8183_infracfg->peri_bus_dcm_ctrl,
+		DCM_INFRA_PERI_MASK, DCM_INFRA_PERI_ON);

/* enable [11] for change i2c module source clock to TOPCKGEN */
    setbits_le32(&mt8183_infracfg->module_clk_sel, 0x1 << 11);