Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34773 )
Change subject: soc/mediatek: dsi: Support sending MIPI init commands ......................................................................
Patch Set 13:
(6 comments)
https://review.coreboot.org/c/coreboot/+/34773/12/src/soc/mediatek/common/ds... File src/soc/mediatek/common/dsi.c:
https://review.coreboot.org/c/coreboot/+/34773/12/src/soc/mediatek/common/ds... PS12, Line 265: u32 delay_us,
edit: I just remembered, we have wait_us() now... […]
Done
https://review.coreboot.org/c/coreboot/+/34773/12/src/soc/mediatek/common/ds... PS12, Line 314: write32(&dsi0->dsi_cmdq[i], 0);
Is this necessary? I have a feeling that this is only here because the weird read-modify-write for e […]
Done
https://review.coreboot.org/c/coreboot/+/34773/12/src/soc/mediatek/common/ds... PS12, Line 318: clrsetbits_le32(cmdq0 + ((cmdq_off + i) & (0xfffffffc)),
Actually, I've seen this same pattern over and over again, almost every SPI/I2C/whatever driver has […]
Done
https://review.coreboot.org/c/coreboot/+/34773/12/src/soc/mediatek/common/ds... PS12, Line 324: clrsetbits_le32(&dsi0->dsi_cmdq_size, CMDQ_SIZE, cmdq_size);
nit: Does this need to be a clrsetbits? The rest of this driver isn't shy about setting unused bits […]
Done
https://review.coreboot.org/c/coreboot/+/34773/12/src/soc/mediatek/common/ds... PS12, Line 333: assert(init);
nit: maybe just do the […]
Done
https://review.coreboot.org/c/coreboot/+/34773/12/src/soc/mediatek/common/ds... PS12, Line 355: type = MIPI_DSI_DCS_LONG_WRITE;
A lot of this reminds me of Rockchip code, we should probably spend some time looking for things to […]
Ack