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 11:
(8 comments)
https://review.coreboot.org/c/coreboot/+/34773/11/src/soc/mediatek/common/ds... File src/soc/mediatek/common/dsi.c:
https://review.coreboot.org/c/coreboot/+/34773/11/src/soc/mediatek/common/ds... PS11, Line 271: while (read32(&dsi0->dsi_intsta) & (1 << 31)) {
1u
Done
https://review.coreboot.org/c/coreboot/+/34773/11/src/soc/mediatek/common/ds... PS11, Line 272: printk(BIOS_ERR, "%s wait dsi no busy\n", __func__);
Please use more elaborate error messages.
Done
https://review.coreboot.org/c/coreboot/+/34773/11/src/soc/mediatek/common/ds... PS11, Line 273: mdelay(20);
20 ms is quite a lot. Why that much? […]
Done
https://review.coreboot.org/c/coreboot/+/34773/11/src/soc/mediatek/common/ds... PS11, Line 274: }
Should their be an abort condition after a certain time to avoid an infinite loop?
Done
https://review.coreboot.org/c/coreboot/+/34773/11/src/soc/mediatek/common/ds... PS11, Line 312: if (intsta_0 & CMD_DONE_INT_FLAG)
The negative could be moved in the while condition.
We don't want to do udelay() when the read has right flags set, so the check can't be in while.
https://review.coreboot.org/c/coreboot/+/34773/11/src/soc/mediatek/common/ds... PS11, Line 315: } while (!stopwatch_expired(&sw));
Please print out as a debug message, how long it took.
that's something > 400us right? I don't think such debug message would be helpful, maybe skip?
https://review.coreboot.org/c/coreboot/+/34773/11/src/soc/mediatek/common/ds... PS11, Line 318: printk(BIOS_ERR, "dsi send cmd time-out(400uS)\n");
Space before (, and spell: 400 us […]
Done
https://review.coreboot.org/c/coreboot/+/34773/11/src/soc/mediatek/common/ds... PS11, Line 368: printk(BIOS_ERR, "%s: Unknown cmd: %d, abort\n",
… abort panel initialization?
Done