Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34771 )
Change subject: soc/mediatek: dsi: Refactor PHY timing calculation ......................................................................
Patch Set 12:
(4 comments)
https://review.coreboot.org/c/coreboot/+/34771/11/src/soc/mediatek/common/ds... File src/soc/mediatek/common/dsi.c:
https://review.coreboot.org/c/coreboot/+/34771/11/src/soc/mediatek/common/ds... PS11, Line 87: phy_timing->da_hs_exit = 2U * phy_timing->lpx;
So how do these work out when you override lpx? Either you need to move these calculations below the […]
seems like I left few values not set, corrected (lpx was fixed value on 8173 so I only need to re-code all values)
https://review.coreboot.org/c/coreboot/+/34771/11/src/soc/mediatek/common/ds... PS11, Line 94: phy_timing->clk_hs_exit = 2U * phy_timing->lpx;
Same for this one (lpx-dependent and must be overridden for 8173).
Done
https://review.coreboot.org/c/coreboot/+/34771/11/src/soc/mediatek/common/ds... PS11, Line 105: da_hs_sync
Did you intentionally leave this at 0 for the common implementation?
Done
https://review.coreboot.org/c/coreboot/+/34771/11/src/soc/mediatek/common/ds... PS11, Line 248: struct mtk_phy_timing phy_timing;
What's the point of passing this struct in? If it's only for passing stuff to the override, can't yo […]
Because it'll be used for other functions soon (video timing).
-
Hung-Te Lin (Code Review)