Patch Set 1: Code-Review-1

Mike, so turns out I did already look into this (CB:20638)
and asked you to test the effect of MULTIPLE_VGA_ADAPTERS.
Can you please do that?

Although it is a bit discouraging to get -1 for a patch that - unlike this old CB:20638 - really helps to get a G505S discrete GPU working (together with CB:31449 and CB:31450 of course - I split a big patch in these three) , thank you for reminding about your request.

Today I made multiple tests and found out that commenting out the already-existing-at-coreboot-master "if MULTIPLE_VGA_ADAPTERS" options (at this ./src/device/pci_rom.c and ./src/northbridge/amd/agesa/family15tn/northbridge.c) or even removing the code surrounded by these already existing options - did not affect anything: I have not observed any differences in discrete GPU functioning or cbmem/kernel logs. If I am including these three patches in their current version to coreboot, my discrete GPU is working even with this old code removed.

However, later I found out that code at ./src/northbridge/amd/agesa/family15tn/northbridge.c (line ~370)

#if IS_ENABLED(CONFIG_MULTIPLE_VGA_ADAPTERS)
			extern struct device *vga_pri; // the primary vga device, defined in device.c
			printk(BIOS_DEBUG, "VGA: vga_pri bus num = %d bus range [%d,%d]\n", vga_pri->bus->secondary,
					link->secondary,link->subordinate);
			/* We need to make sure the vga_pri is under the link */
			if ((vga_pri->bus->secondary >= link->secondary) &&
			    (vga_pri->bus->secondary <= link->subordinate))
#endif

has never been executed before, because a check surrounding this code

    for (link = dev->link_list; link; link = link->next) {
        if (link->bridge_ctrl & PCI_BRIDGE_CTL_VGA) { // <---- this one
            #if IS_ENABLED(CONFIG_MULTIPLE_VGA_ADAPTERS)

had been always failing for me. So although this existing code is useless to me, I don't know if it is useless to all the other coreboot boards also (could this check above be passing for some?) , and also if this primary/secondary/subordinate stuff could be used for getting a Crossfire working on this laptop.

Also, IIRC, I asked to check what Linux actually requires
to bring graphics up. So we don't have to twist coreboot
for some legacy stuff that isn't really required. If the
rumors are true, Linux only needs a way to find the Atom-
BIOS code and doesn't require the firmware to execute any
of it. So please, find out what ways Linux' driver sup-
ports to acquire the AtomBIOS. There might be much clea-
ner ways than running this nasty stuff in coreboot.

Although there's also BSD in addition to Linux (didn't want my changes to take OS in consideration), I think we could guess what a Linux driver wants by reading its' error messages which it prints in case of initialization failure - e.g. when today I tried following Rudolph's "VFCT should be used for the integrated GPU in dual graphics systems" advice above and enabled VFCT filling for integrated GPU only in both cases, the discrete GPU/driver failed to initialize and I got these familiar kernel log messages:

  radeon 0000:00:01.0: fb0: radeondrmfb frame buffer device
  [drm] Initialized radeon 2.50.0 20080528 for 0000:00:01.0 on minor 0
  [drm] initializing kernel modesetting (HAINAN 0x1002:0x6665 0x0000:0x0000 0x00).
  [drm:radeon_get_bios [radeon]] *ERROR* ACPI VFCT image header truncated
  radeon 0000:01:00.0: Invalid PCI ROM header signature: expecting 0xaa55, got 0x0000
  radeon 0000:01:00.0: Invalid PCI ROM header signature: expecting 0xaa55, got 0x0000
  [drm:radeon_get_bios [radeon]] *ERROR* Unable to locate a BIOS ROM
  radeon 0000:01:00.0: Fatal error during GPU init
  [drm] radeon: finishing device.

So it seems to me that we need the discrete GPU OpROM loaded (running isn't required, and whether could be useful - I don't know yet), and also VFCT filling for discrete GPU is required - because otherwise it does not work.

> AFAIK it is forbidden to route the IO and MMIO regions
> related to VGA Option Rom to two different PCI functions.

Yeah that just wouldn't work.

Not sure how you could even run two option roms without
redoing the routing in between.

I'm not sure about this. But a hunch tells me that the
option rom shouldn't assume the routing but check it and
act accordingly.

Please see the comment addressed to Kyösti above; in short - I have not noticed any difference from letting both OptionROMs run and cbmem/kernel logs are almost the same.

View Change

To view, visit change 31448. To unsubscribe, or for help writing mail filters, visit settings.