Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39615 )
Change subject: sc7180: Add display hardware pipe line initialization ......................................................................
Patch Set 28:
(3 comments)
https://review.coreboot.org/c/coreboot/+/39615/25/src/mainboard/google/trogd... File src/mainboard/google/trogdor/Kconfig:
https://review.coreboot.org/c/coreboot/+/39615/25/src/mainboard/google/trogd... PS25, Line 21: select DRIVERS_TI_SN65DSI86BRIDGE
Done
I don't know why you said "Done" here, did you do anything? The patch order is still wrong, the bridge driver patch needs to be ordered *before* this patch here.
https://review.coreboot.org/c/coreboot/+/39615/28/src/mainboard/google/trogd... File src/mainboard/google/trogdor/board.h:
https://review.coreboot.org/c/coreboot/+/39615/28/src/mainboard/google/trogd... PS28, Line 21: (CONFIG(BOARD_GOOGLE_TROGDOR) && board_id() < 2) || \ : (CONFIG(BOARD_GOOGLE_LAZOR) && board_id() < 1) ? \ As discussed over email this was wrong from me -- it just needs to be
(CONFIG(TROGDOR_REV0) ? GPIO(106) : GPIO(30))
https://review.coreboot.org/c/coreboot/+/39615/28/src/mainboard/google/trogd... File src/mainboard/google/trogdor/mainboard.c:
https://review.coreboot.org/c/coreboot/+/39615/28/src/mainboard/google/trogd... PS28, Line 60: uint8_t enable Do we need this enable? If it's only ever going to be 1, just take it out (maybe get rid of the whole function because there's just one line left then, just inline that in display_startup()).