nickey.yang@rock-chips.com has posted comments on this change. ( https://review.coreboot.org/19477 )
Change subject: rockchip/rk3399: Add MIPI driver ......................................................................
Patch Set 4:
(7 comments)
https://review.coreboot.org/#/c/19477/2/src/soc/rockchip/rk3399/mipi.c File src/soc/rockchip/rk3399/mipi.c:
Line 68: return dptdin_map[i].testdin;
Okay, fair enough. But shouldn't the comparison be (dptdin_map[i].max_mbps
if max_mbps = 100; when we use >= will return 0x10,but we want 0x20
Line 192: pclk = edid->mode.pixel_clock * MSEC_PER_SEC;
That is not a good reason to have bad code, though. You're doing a lot of i
Done
Line 406:
Does this need to be part of the structure? Do you have more than one MIPI
rk3288 and rk3399 have the same mipi ip,it will be more convenient if we want to support 3288 in the future
https://review.coreboot.org/#/c/19477/3/src/soc/rockchip/rk3399/mipi.c File src/soc/rockchip/rk3399/mipi.c:
PS3, Line 193: big
*bigger
Done
PS3, Line 194: i->lanes * bpp / 8
This is also a bad integer division in the 18bpp case, btw. You should prob
Done
PS3, Line 429: ret = rk_mipi_dsi_dcs_transfer(&rk_mipi, MIPI_DCS_EXIT_SLEEP_
Check return value?
Done
PS3, Line 431: return;
here too
Done