[XL] Change in coreboot[main]: soc/mediatek/common/dp: Move common functions to dptx_hal_common.c

[Yu-Ping Wu (Code Review)]

Attention is currently required from: Hung-Te Lin, Yidi Lin.

Yu-Ping Wu has posted comments on this change by Yidi Lin. ( https://review.coreboot.org/c/coreboot/+/85861?usp=email )

Change subject: soc/mediatek/common/dp: Move common functions to dptx_hal_common.c
......................................................................

Patch Set 4:

(10 comments)

Commit Message:

https://review.coreboot.org/c/coreboot/+/85861/comment/409d4d85_914a6255?usp... :
PS3, Line 11: Adding
Add

https://review.coreboot.org/c/coreboot/+/85861/comment/3c0396c1_41db22c5?usp... :
PS3, Line 12: underly
underlying?

File src/soc/mediatek/common/dp/dptx_hal.c:

https://review.coreboot.org/c/coreboot/+/85861/comment/354f4011_5c3fe37d?usp... :
PS3, Line 339: 0xfff
Not related to this patch, but this looks like a bug. The value 0x2020 bits should be contained in the mask 0xfff. See the TODO comment in `mtk_dp_mask()`. MT8196's code has the same problem.

https://review.coreboot.org/c/coreboot/+/85861/comment/ee38a5ab_ed2aefe5?usp... :
PS3, Line 64: 0
(Just to keep track of the difference) This is `hbp` for mt8196.

https://review.coreboot.org/c/coreboot/+/85861/comment/43584ad4_7f653545?usp... :
PS3, Line 72: mtk_dp_mask(mtk_dp, REG_302C_DP_ENCODER0_P0,
            : 		    0 << VSP_SW_DP_ENCODER0_P0_FLDMASK_POS,
            : 		    VSP_SW_DP_ENCODER0_P0_FLDMASK);
(Just to keep track of the difference)  mt8196 does NOT have this.

https://review.coreboot.org/c/coreboot/+/85861/comment/dbda4b34_bab0e9ce?usp... :
PS3, Line 80: hsync + hbp
This is `hsync + hbp + hfp` for mt8196.

https://review.coreboot.org/c/coreboot/+/85861/comment/1496deaa_a67059a6?usp... :
PS3, Line 85: vsync + vbp
This is `vsync + vbp + vfp` for mt8196.

File src/soc/mediatek/common/dp/dptx_hal_common.c:

https://review.coreboot.org/c/coreboot/+/85861/comment/4af02de0_37a08be7?usp... :
PS3, Line 56: u32
Unrelated to this patch, but this should be `u8`.

https://review.coreboot.org/c/coreboot/+/85861/comment/4e61c93d_d8664dc0?usp... :
PS3, Line 115: 	DP_CLRSETBITS(mtk_dp, REG_303C_DP_ENCODER0_P0 + 1, val, 0x7);
This would be wrong for mt8196, where its code is `mtk_dp_mask(mtk_dp, REG_303C_DP_ENCODER0_P0, val << 8, 0x7 << 8)`. What would you define `DP_CLRSETBITS` for mt8196?

To solve the `mtk_dp_write_byte` vs `mtk_dp_mask` problem, I think we should keep this line as `mtk_dp_write_byte`, and the define mt8196's `mtk_dp_write_byte` as:

```
void mtk_dp_write_byte(...)
{
    if (addr % 2)
        mtk_dp_mask(mtk_dp, addr - 1, val << 8, mask << 8);
    else
        mtk_dp_mask(mtk_dp, addr, val, mask);
}
```

That is, mt8196 skips the `DP_TX_TOP_APB_WSTRB` writes.

https://review.coreboot.org/c/coreboot/+/85861/comment/8a651815_fb64fff0?usp... :
PS3, Line 143: 		DP_CLRSETBITS(mtk_dp, REG_3004_DP_ENCODER0_P0 + 1,
Same here.