Re: [coreboot] [PATCH] Fix AMD HD 3200 uma graphics problems inWin7 (revised)

[Scott Duplichan]

-----Original Message-----
From: [e-mail] [mailto:[e-mail]] On Behalf Of Arne Georg Gleditsch
Sent: Thursday, November 04, 2010 04:38 AM
To: Scott Duplichan
Cc: 'Peter Stuge'; 'Carl-Daniel Hailfinger'; [e-mail]
Subject: Re: [coreboot] [PATCH] Fix AMD HD 3200 uma graphics problems inWin7 (revised)

"Scott Duplichan"  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.