-----Original Message----- From: coreboot-boun...@coreboot.org [mailto:coreboot-boun...@coreboot.org] On Behalf Of Arne Georg Gleditsch Sent: Thursday, November 04, 2010 04:38 AM To: Scott Duplichan Cc: 'Peter Stuge'; 'Carl-Daniel Hailfinger'; coreboot@coreboot.org Subject: Re: [coreboot] [PATCH] Fix AMD HD 3200 uma graphics problems inWin7 (revised)
"Scott Duplichan" <sc...@notabs.org> writes: > -- Disable the family 10h processor mmconf while the RS780 mmconf is in use. ]I'm not sure I understand how this is supposed to work. Shouldn't we ]just make sure the two mmconf regions don't overlap? (Or is the overlap ]intended?) To be frank, I'm not sure what the RS780 mmconf offers over ]the Fam10 mmconf at all. For Fam0f, I assume it is useful in order to ]reach extended config space, but as far as I know that should be covered ]by the Fam10 mmconf already. No doubt there different, and possibly simpler, ways to make this work. The patch I submitted attempts to follow the method used by the AMD CIMx reference code. ]> + // disable processor pcie mmio, if enabled ]> + if (is_family10h()) { ]> + msr_t temp; ]> + pcie_mmio_save = temp = rdmsr (0xc0010058); ]> + temp.lo &= ~1; ]> + wrmsr (0xc0010058, temp); ]> + } ]> + ]> /* Get PCIe configuration space. */ ]> MMIOBase = pci_read_config32(nb_dev, 0x1c) & 0xfffffff0; ] ]This pci_read_config32 is targeting the Fam10 mmconf area, which is now ]disabled. Are we relying on the rs780 mmconf to back this address ]region at this point? When I step through this pci_read_config32 call (on simnow) I see it using the cf8/cfc method for config access, not the mmio method. ]If so, we probably should take care to make sure ]the address assignments match (or rather, document that they need to ]match). If we need to do this at all, that is. ] ]> /* Temporarily disable PCIe configuration space. */ ]> set_htiu_enable_bits(nb_dev, 0x32, 1<<28, 0); ] ]But here we disable the rs780 mmconf, no? Who's backing the mmconf ]address region now: Again it looks like all config space access is done with cf8/cfc for these calls. ] ]> + // 1E: NB_BIF_SPARE ]> set_nbmisc_enable_bits(nb_dev, 0x1e, 0xffffffff, 1<<1 | 1<<4 | 1<<6 | 1<<7); ]> /* Set a temporary Bus number. */ ]> apc18 = pci_read_config32(dev, 0x18); ] ]? ] ]Apologies if I have misunderstood something, I haven't quite kept up ]with the discussion. I'm just a bit concerned about these changes. ]-- ] Arne. -- coreboot mailing list: coreboot@coreboot.org http://www.coreboot.org/mailman/listinfo/coreboot