[coreboot-gerrit] Change in coreboot[master]: nb/x4x: Disable IGD when external GPU is found

[Arthur Heymans (Code Review)]

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/18504 )

Change subject: nb/x4x: Disable IGD when external GPU is found
......................................................................


Patch Set 14:

It's quite confusing what the desired behavior is.

CONFIG_ONBOARD_VGA_IS_PRIMARY has a very vague help that IMO is not totally same as its prompt. This Kconfig is present on all devices but is meaningless on most due to absence of a disable function (or the absence of Integrated VGA device). (also that Kconfig option should also not be console)
The code itself calls a disable function which most of the time effectively disables the IGD. While vendor bios are not a reference for good default behavior, the majority of them disables internal GPU in favor of PCI/AGP/PCIe ones, with only a minority offering an option to keep IGD enabled.
Some users of discrete VGA devices might not want to use the IGD in which the uma resources could be seen as a waste. This further complicated by the fact that those disable functions all (attempt to) do different things:
* i945: disabled igd and reduced uma to 0, with this function being a no-op for PEG slot since disable happens in romstage (for mobile at least)
* sandy bridge: effectively disables IGD and tries but fails to set uma to 0 (at least code looks like that)
* rs780: no idea? just disable?
* lenovo hybrid driver: calls IGD disable depending on value of nvram parameter.
* T400: enable/disable discrete with no option to have vga cycles to discrete while IGD active.

On top of that it seems that Linux does not handle IGD that well when it's not granted vga cycles: it only show stuff in the tty up to a point and in X11 I failed to set is as a display provider for the discrete gpu using prime.

I don't mind having IGD enabled as a secondary besides a discrete GPU a default as long as it boots properly, but if this is the desired behavior then it would be the first NB to achieve it :-)

-- 
To view, visit https://review.coreboot.org/18504
To unsubscribe, visit https://review.coreboot.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I74890918feb0f1ff6b971c4aaa96f1f7b75266ac
Gerrit-PatchSet: 14
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Owner: Arthur Heymans <arthur at aheymans.xyz>
Gerrit-Reviewer: Arthur Heymans <arthur at aheymans.xyz>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki at gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h at gmx.de>
Gerrit-Reviewer: Patrick Rudolph <siro at das-labor.org>
Gerrit-Reviewer: Paul Menzel <paulepanter at users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins)
Gerrit-HasComments: No