Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31478 )
Change subject: mediatek/mt8183: Add display driver ......................................................................
Patch Set 3:
(3 comments)
Is it possible to merge with 8173/ddp.c, then move to common/ddp.c?
https://review.coreboot.org/#/c/31478/3/src/soc/mediatek/mt8183/include/soc/... File src/soc/mediatek/mt8183/include/soc/ddp.h:
https://review.coreboot.org/#/c/31478/3/src/soc/mediatek/mt8183/include/soc/... PS3, Line 53: check_member(mmsys_cfg_regs, reserved_0x11C, 0x11C); : check_member(mmsys_cfg_regs, disp_ovl0_mout_en, 0xF00); : check_member(mmsys_cfg_regs, reserved_0xF10, 0xF10); : check_member(mmsys_cfg_regs, disp_path0_sel_in, 0xF24); : check_member(mmsys_cfg_regs, disp_ovl0_2l_sel_in, 0xF38); : check_member(mmsys_cfg_regs, reserved_0xF3C, 0xF3C); : check_member(mmsys_cfg_regs, disp_rdma0_sout_sel_in, 0xF50); I think these are not needed. We should check only critical addresses, probably just start of functional reg (mmsys_cg_con0) and end of reg (dpi0_sel_sout_sel_in).
https://review.coreboot.org/#/c/31478/3/src/soc/mediatek/mt8183/include/soc/... PS3, Line 212: check_member(disp_ovl_regs, reserved_0x018, 0x018); : check_member(disp_ovl_regs, roi_size, 0x020); : check_member(disp_ovl_regs, reserved_0x0B0, 0x0B0); : check_member(disp_ovl_regs, reserved_0x140, 0x140); : check_member(disp_ovl_regs, reserved_0xF00, 0xF00); : check_member(disp_ovl_regs, l0_addr, 0xF40); not needed
https://review.coreboot.org/#/c/31478/3/src/soc/mediatek/mt8183/include/soc/... PS3, Line 272: check_member(disp_color_regs, cfg_main, 0x400); : check_member(disp_color_regs, start, 0xC00); : check_member(disp_color_regs, width, 0xC50); not needed