[coreboot-gerrit] Change in coreboot[master]: mb/lenovo: Dual Graphics for xx20/xx30 ThinkPads

[Patrick Rudolph (Code Review)]

Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/28393 )

Change subject: mb/lenovo: Dual Graphics for xx20/xx30 ThinkPads
......................................................................


Patch Set 2:

(1 comment)

https://review.coreboot.org/#/c/28393/2/src/mainboard/lenovo/t430s/romstage.c
File src/mainboard/lenovo/t430s/romstage.c:

https://review.coreboot.org/#/c/28393/2/src/mainboard/lenovo/t430s/romstage.c@72
PS2, Line 72: 	enum hybrid_graphics_req mode = HYBRID_GRAPHICS_INTEGRATED;
> Well, here https://review.coreboot. […]
Sorry for the confusion.
Yes there's no hybrid graphics mux.
In general it's discouraged to duplicate code. By using the proposed devicetree settings it does the same.
I understand your argument of not using the driver as there's no hybrid graphics to avoid confusion. In that case please keep the code, but also rename the CMOS for t430s to something like "enable_dual_graphics". Otherwise end users will wonder why "discrete only" is missing and report it as a bug.



-- 
To view, visit https://review.coreboot.org/28393
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8842fef0fa1235eb91abf6b7e655ed4d8598adc7
Gerrit-Change-Number: 28393
Gerrit-PatchSet: 2
Gerrit-Owner: Evgeny Zinoviev <me at ch1p.com>
Gerrit-Reviewer: Evgeny Zinoviev <me at ch1p.com>
Gerrit-Reviewer: Patrick Rudolph <siro at das-labor.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply at coreboot.org>
Gerrit-Comment-Date: Sat, 01 Sep 2018 05:51:37 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: No
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.coreboot.org/pipermail/coreboot-gerrit/attachments/20180901/6d5253d5/attachment.html>