[coreboot-gerrit] [XL] Change in coreboot[main]: soc/mediatek/mt8196: Add eDP driver

[Bincai Liu (Code Review)]

Attention is currently required from: Hung-Te Lin, Jarried Lin, Jitao Shi, Yidi Lin, Yu-Ping Wu.

Bincai Liu has posted comments on this change by Jarried Lin. ( https://review.coreboot.org/c/coreboot/+/85949?usp=email )

Change subject: soc/mediatek/mt8196: Add eDP driver
......................................................................

Patch Set 7:

(53 comments)

Commit Message:

https://review.coreboot.org/c/coreboot/+/85949/comment/4670aca5_32ca8ec4?usp... :
PS2, Line 10: traning
> Do you mean `training`?

Done

https://review.coreboot.org/c/coreboot/+/85949/comment/421d0045_786cb290?usp... :
PS2, Line 11:
> Please mention what DVO is in the commit message.

Done

File src/soc/mediatek/mt8196/dp_intf.c:

https://review.coreboot.org/c/coreboot/+/85949/comment/9a8acc99_8345f613?usp... :
PS2, Line 91: 0x20
> will fix next patchset

Done

https://review.coreboot.org/c/coreboot/+/85949/comment/5495a009_d8ee3b5c?usp... :
PS2, Line 99: gs_level
> not done

Done

https://review.coreboot.org/c/coreboot/+/85949/comment/9bb8fa2f_05fc7b1b?usp... :
PS2, Line 162: 0x8000
> will fix next patchset

Done

https://review.coreboot.org/c/coreboot/+/85949/comment/691811f4_2bce5fd2?usp... :
PS2, Line 198: mtk_dvo_power_on
> will fix next patchset

Done

File src/soc/mediatek/mt8196/dp_intf.c:

https://review.coreboot.org/c/coreboot/+/85949/comment/20c9b6a9_31cd56a2?usp... :
PS4, Line 102: mtk_dvo_gs[MTK_DVO_8K_30FPS]
> will fix next patchset

Done

https://review.coreboot.org/c/coreboot/+/85949/comment/011686ef_aa3a49cf?usp... :
PS4, Line 106: 0xFFFFFFF
> will fix next patchset

Done

https://review.coreboot.org/c/coreboot/+/85949/comment/ad3f7693_accb1357?usp... :
PS4, Line 108: 0xFFFFFFF
> will fix next patchset

Done

https://review.coreboot.org/c/coreboot/+/85949/comment/a7664251_06648169?usp... :
PS4, Line 133: [dvo]
> will fix next patchset

Done

File src/soc/mediatek/mt8196/dp_intf.c:

https://review.coreboot.org/c/coreboot/+/85949/comment/6911dfe6_5e0879d4?usp... :
PS6, Line 32: const struct mtk_dvo_sync_param *sync)
> will fix next patchset

Done

File src/soc/mediatek/mt8196/dptx.c:

https://review.coreboot.org/c/coreboot/+/85949/comment/7a6613a4_2b0f2734?usp... :
PS2, Line 257: 	mtk_dp->train_info.linkrate = (linkrate >= mtk_dp->train_info.sys_max_linkrate) ?
> Not done. […]

Done

https://review.coreboot.org/c/coreboot/+/85949/comment/fead9273_600f082e?usp... :
PS2, Line 372: write32p(MMSYS1_CONFIG_BASE + MMSYS1_CG_CLR0, 0xFFFFFFFF);
             : 	write32p(MMSYS1_CONFIG_BASE + MMSYS1_CG_CLR1, 0x00040000);
> this clock is already configured in mainboard.c,we will remove in dptx.c and mtk_dpintf.c […]

Done

File src/soc/mediatek/mt8196/dptx.c:

https://review.coreboot.org/c/coreboot/+/85949/comment/5d11c5dc_cc552576?usp... :
PS4, Line 41: case DPTX_PATTERN_UNKNOWN:
            : 		printk(BIOS_ERR, "Pattern unknown\n");
            : 		mtk_dp_mask(mtk_dp, REG_3400_DP_TRANS_P0, 0x0, PATTERN_EN_DP_TRANS_4P_MASK);
            : 		return;
            : 	default:
            : 		mtk_dp_mask(mtk_dp, REG_3400_DP_TRANS_P0, 0x0, PATTERN_EN_DP_TRANS_4P_MASK);
            : 		return;
> will fix next patchset

Done

https://review.coreboot.org/c/coreboot/+/85949/comment/af24de55_c8a1452f?usp... :
PS4, Line 54: 					   u8 dpcd_adjust_req[DP_LANSE_ADJUST_SIZE])
> will fix next patchset

Done

https://review.coreboot.org/c/coreboot/+/85949/comment/9e1415a8_564f1855?usp... :
PS4, Line 92: lanerate
> will fix next patchset

Done

https://review.coreboot.org/c/coreboot/+/85949/comment/c97a03bd_64e1767a?usp... :
PS4, Line 94: lane_count
> will fix next patchset

Done

https://review.coreboot.org/c/coreboot/+/85949/comment/ba34ee2b_bbde2934?usp... :
PS4, Line 96: u8 target_link_rate = lanerate;
            : 	u8 target_lane_count = lanecount;
> will fix next patchset

Acknowledged

https://review.coreboot.org/c/coreboot/+/85949/comment/ea2b191e_616bc0ce?usp... :
PS4, Line 140: DPTX_PLUG_OUT
> will fix next patchset

Done

https://review.coreboot.org/c/coreboot/+/85949/comment/ed9a9582_34938ea7?usp... :
PS4, Line 385: add 26Mhz
> `Add 26MHz`

Done

https://review.coreboot.org/c/coreboot/+/85949/comment/ca8f6182_135d4f20?usp... :
PS4, Line 406: "
> will fix next patchset

Done

File src/soc/mediatek/mt8196/dptx.c:

https://review.coreboot.org/c/coreboot/+/85949/comment/c8a08199_d9a7fd7f?usp... :
PS6, Line 260: 0x1f
> 0x1F

Done

https://review.coreboot.org/c/coreboot/+/85949/comment/2b64d3d0_23930aaa?usp... :
PS6, Line 272: 0xbf
> 0xBF

Done

https://review.coreboot.org/c/coreboot/+/85949/comment/d1099b3f_cceb3024?usp... :
PS6, Line 376: int mtk_edp_init(struct mtk_dp *mtk_dp,struct edid *edid)
> > `space required after that ',' (ctx:VxV)` […]

Done

https://review.coreboot.org/c/coreboot/+/85949/comment/ebd69974_49faa28c?usp... :
PS6, Line 384: mdelay
> udelay(50) is ok. […]

Done

File src/soc/mediatek/mt8196/dptx_hal.c:

https://review.coreboot.org/c/coreboot/+/85949/comment/04faf19f_5304631b?usp... :
PS2, Line 52: 	switch (out_format) {
> will fix next patchset

Done

https://review.coreboot.org/c/coreboot/+/85949/comment/9ca86294_7609c030?usp... :
PS2, Line 146: 	switch (value) {
> will fix next patchset

Done

https://review.coreboot.org/c/coreboot/+/85949/comment/5724fd3a_1236ee68?usp... :
PS2, Line 307: mtk_edp_hal_reset_swing_pre_emphasis
> will fix next patchset

Done

https://review.coreboot.org/c/coreboot/+/85949/comment/58cd074f_1d9a4f10?usp... :
PS2, Line 369: 	if ((value << 2) <= UINT8_MAX) {
> assert not added yet

Done

File src/soc/mediatek/mt8196/dptx_hal.c:

https://review.coreboot.org/c/coreboot/+/85949/comment/b143d30a_69b8f446?usp... :
PS4, Line 39: 0xA << 2
> That's even better.

Done

https://review.coreboot.org/c/coreboot/+/85949/comment/a4c7f97c_fa43f142?usp... :
PS4, Line 53: 	switch (out_format) {
> will fix next patchset

Done

https://review.coreboot.org/c/coreboot/+/85949/comment/6e5205f1_fa5b1fba?usp... :
PS4, Line 86: 					u8 *swing_val, u8 *pre_emphasis)
> will fix next patchset

Done

https://review.coreboot.org/c/coreboot/+/85949/comment/aec9700b_9c121a8c?usp... :
PS4, Line 135: 			GENMASK(2, 0));
> will fix next patchset

Done

https://review.coreboot.org/c/coreboot/+/85949/comment/f64aff26_d51cdc4e?usp... :
PS4, Line 142: 	switch (value) {
> will fix next patchset

Done

https://review.coreboot.org/c/coreboot/+/85949/comment/78b2a6d2_3294f2d4?usp... :
PS4, Line 337: (mtk_dp_phy_read
> will fix next patchset

Done

https://review.coreboot.org/c/coreboot/+/85949/comment/ade5a676_e7d5f9f6?usp... :
PS4, Line 337: 0x146c
> will fix next patchset

Done

https://review.coreboot.org/c/coreboot/+/85949/comment/9b579e7e_352b125c?usp... :
PS4, Line 362: mtk_dp_set_swing_pre_emphasis
> will fix next patchset

Done

https://review.coreboot.org/c/coreboot/+/85949/comment/885da47b_96137066?usp... :
PS4, Line 362: pre_emphasis
> will fix next patchset

Done

https://review.coreboot.org/c/coreboot/+/85949/comment/21e0b0eb_a30caf48?usp... :
PS4, Line 363: pre_emphasis
> will fix next patchset

Done

https://review.coreboot.org/c/coreboot/+/85949/comment/7b780d3f_2517eb07?usp... :
PS4, Line 388: ,
> will fix next patchset

Done

https://review.coreboot.org/c/coreboot/+/85949/comment/7a8e0a0a_1b602eb6?usp... :
PS4, Line 389: break
> will fix next patchset

Done

File src/soc/mediatek/mt8196/dptx_hal.c:

https://review.coreboot.org/c/coreboot/+/85949/comment/26c346aa_cbde3352?usp... :
PS6, Line 316: mdelay
> will fix next Patchset, […]

Done

https://review.coreboot.org/c/coreboot/+/85949/comment/9fb0e096_2f261de3?usp... :
PS6, Line 338: mdelay
> Here is polling PHY ready,  […]

Done

File src/soc/mediatek/mt8196/include/soc/dp_intf.h:

https://review.coreboot.org/c/coreboot/+/85949/comment/8bf2daa7_0fd3d457?usp... :
PS6, Line 137: 0x6c
> will fix next patchset

Done

https://review.coreboot.org/c/coreboot/+/85949/comment/0c44d811_61036826?usp... :
PS6, Line 141: c
> will fix next patchset

Done

File src/soc/mediatek/mt8196/include/soc/dp_intf.h:

https://review.coreboot.org/c/coreboot/+/85949/comment/ffb38afb_ff1f418b?usp... :
PS4, Line 112: #define HACT_SHIFT		16
> align

Done

https://review.coreboot.org/c/coreboot/+/85949/comment/ead88df1_715f9ac9?usp... :
PS4, Line 124: #define VACT_SHIFT		16
> will fix next patchset

Done

File src/soc/mediatek/mt8196/include/soc/dp_intf.h:

https://review.coreboot.org/c/coreboot/+/85949/comment/93ce2e7f_83b03202?usp... :
PS2, Line 199: gs_level
> will be used in mt8196/dp_intf. […]

Done

File src/soc/mediatek/mt8196/include/soc/dptx_hal.h:

https://review.coreboot.org/c/coreboot/+/85949/comment/26ff98b4_e89f4ecc?usp... :
PS4, Line 26: pre_emphasis
> will be fix next patch

Done

https://review.coreboot.org/c/coreboot/+/85949/comment/44167963_8a8cb48d?usp... :
PS4, Line 31: pre_emphasis
> will be fix next patch.

Done

https://review.coreboot.org/c/coreboot/+/85949/comment/58ddf07b_b50bf28f?usp... :
PS4, Line 32: pre_emphasis
> will be fix next patch

Done

File src/soc/mediatek/mt8196/include/soc/dptx_hal.h:

https://review.coreboot.org/c/coreboot/+/85949/comment/824e5b2e_05613794?usp... :
PS2, Line 38: void mtk_edp_phy_wait_aux_ldo_ready(struct mtk_dp *mtk_dp);
            : void mtk_edp_hal_phy_set_idle_pattern(struct mtk_dp *mtk_dp, u8 lane_count, bool enable);
            : void mtk_edp_set_swing_pre_emphasis
> will be change to dptx_hal_ next patchset

Done

File src/soc/mediatek/mt8196/include/soc/dptx_reg.h:

https://review.coreboot.org/c/coreboot/+/85949/comment/ee962bab_07035eb0?usp... :
PS4, Line 204: #define DP_TRAIN_VOLTAGE_PREEMPHASIS_LEVEL_3		0x3
> will fix next patchset

Done