[coreboot] [RFC] i945 problematic code for IGD device enable, BUG?

[Petr Cvek]

Hello again :-D,

I'm working on a code for a simultaneous use of IGD (GMA950) and x16 PCIe slot GPU. I've made some success, but the code which handles the IGD initialization is really weird.

The IGD is initialized by DEVEN register (DEVEN_D2F0 and DEVEN_D2F1 bits), which is first written in i945_setup_pci_express_x16(). Few times before that the DEVEN_D2F0 and DEVEN_D2F1 bits are already tested during sdram_initialize() code. Also after the reset these bits are initialized to 1. This will cause the test from:

https://elixir.bootlin.com/coreboot/4.16/source/src/northbridge/intel/i945/r...

if (!(pci_read_config8(HOST_BRIDGE, DEVEN) & (DEVEN_D2F0 | DEVEN_D2F1)))
    	integrated_graphics = false;

to be effectively always -> if (0)

also even when DEVEN_D2F0 is zeroed after chipset reset, the code located after a few lines:

https://elixir.bootlin.com/coreboot/4.16/source/src/northbridge/intel/i945/r...

pci_or_config8(IGD_DEV, 0xc1, 1 << 2);

will have a problem to work correctly. IGD_DEV is PCIe device enabled by DEVEN_D2F0 (setting DEVEN_D2F0 to 0 will remove IGD_DEV from the PCIe bus). It seems to not cause problems, but I didn't test it extensively.

Also the test in northbridge_get_tseg_base() (used everytime top of the memory is needed):

https://elixir.bootlin.com/coreboot/4.16/source/src/northbridge/intel/i945/m...

if (pci_read_config8(HOST_BRIDGE, DEVEN) & (DEVEN_D2F0 | DEVEN_D2F1))
    	/* IGD enabled, get top of Memory from BSM register */
    	tom = pci_read_config32(IGD_DEV, BSM);

If only DEVEN_D2F1 is enabled, the test will succeed, but as the IGD_DEV is now disabled by DEVEN_D2F0 it will read garbage data for the global "top of the memory" value.

I've sort of solved the missing initialization problem with CMOS config entry in mainboard_romstage_entry():

switch (get_uint_option("igd_en", 1)) {
    	case 0:
    		//disable
    		pci_and_config32(HOST_BRIDGE, DEVEN, ~(DEVEN_D2F0 | DEVEN_D2F1));
    		break;
    	case 2:
    		//enable func0 only ??? TEST
    		pci_update_config16(HOST_BRIDGE, DEVEN, ~(DEVEN_D2F1), DEVEN_D2F0);
    		break;
    	case 1:
    	default:
    		//enabled both
    		pci_or_config32(HOST_BRIDGE, DEVEN, DEVEN_D2F0 | DEVEN_D2F1);
    		break;
    }

sdram_initialize( ...

RFC:

Is this approach OK? Of course the patch will need to cover the if (0) tests and skip access to missing PCI devices. Does anybody know if GCFC (Graphics Clock Frequency Control) register (i945 datasheet, section 8.1.35, page 296) needs to be explicitly disabled when IGD PCIe access is disabled by DEVEN bits or do I need to enable IGD PCIe access with DEVEN, disable clocks in GCFC and then disable IGD PCIe access again with DEVEN bits?

Similar to GCFC settings without enabled IGD PCIe access. There are many registers in affected code, which doesn't have a name/description in the i945 datasheet. Does anybody know their function? It could help to determine if they needs to be set even when IGD will be disabled.

Best regards,
Petr