Nico Huber has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/30881
Change subject: nb/intel/i945: Reduce pcidev_on_root() calls ......................................................................
nb/intel/i945: Reduce pcidev_on_root() calls
Also removes one call for 0:2.0 (integrated graphics) that might be disabled.
Change-Id: I494aa366030b77baf431f29ba331f13f7c567025 Signed-off-by: Nico Huber nico.h@gmx.de --- M src/northbridge/intel/i945/northbridge.c 1 file changed, 7 insertions(+), 7 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/81/30881/1
diff --git a/src/northbridge/intel/i945/northbridge.c b/src/northbridge/intel/i945/northbridge.c index 2b51b5e..10a7ba4 100644 --- a/src/northbridge/intel/i945/northbridge.c +++ b/src/northbridge/intel/i945/northbridge.c @@ -66,6 +66,7 @@ unsigned long long tomk, tomk_stolen; uint64_t uma_memory_base = 0, uma_memory_size = 0; uint64_t tseg_memory_base = 0, tseg_memory_size = 0; + struct device *const d0f0 = pcidev_on_root(0, 0);
pci_domain_read_resources(dev);
@@ -75,17 +76,14 @@ pci_tolm = find_pci_tolm(dev->link_list); printk(BIOS_DEBUG, "pci_tolm: 0x%x\n", pci_tolm);
- printk(BIOS_SPEW, "Base of stolen memory: 0x%08x\n", - pci_read_config32(pcidev_on_root(2, 0), BSM)); - - tolud = pci_read_config8(pcidev_on_root(0, 0), TOLUD); + tolud = pci_read_config8(d0f0, TOLUD); printk(BIOS_SPEW, "Top of Low Used DRAM: 0x%08x\n", tolud << 24);
tomk = tolud << 14; tomk_stolen = tomk;
/* Note: subtract IGD device and TSEG */ - reg16 = pci_read_config16(pcidev_on_root(0, 0), GGC); + reg16 = pci_read_config16(d0f0, GGC); if (!(reg16 & 2)) { printk(BIOS_DEBUG, "IGD decoded, subtracting "); int uma_size = decode_igd_memory_size((reg16 >> 4) & 7); @@ -96,10 +94,12 @@ /* For reserving UMA memory in the memory map */ uma_memory_base = tomk_stolen * 1024ULL; uma_memory_size = uma_size * 1024ULL; + + printk(BIOS_SPEW, "Base of stolen memory: 0x%08x\n", + (unsigned int)uma_memory_base); }
- tseg_sizek = decode_tseg_size(pci_read_config8(pcidev_on_root(0, 0), - ESMRAMC)) >> 10; + tseg_sizek = decode_tseg_size(pci_read_config8(d0f0, ESMRAMC)) >> 10; printk(BIOS_DEBUG, "TSEG decoded, subtracting %dM\n", tseg_sizek >> 10); tomk_stolen -= tseg_sizek; tseg_memory_base = tomk_stolen * 1024ULL;
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30881 )
Change subject: nb/intel/i945: Reduce pcidev_on_root() calls ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/30881/1/src/northbridge/intel/i945/northbrid... File src/northbridge/intel/i945/northbridge.c:
https://review.coreboot.org/#/c/30881/1/src/northbridge/intel/i945/northbrid... PS1, Line 87: if (!(reg16 & 2)) { AFAICT, this bit is only about legacy VGA cycles...
Comment above where we set it (gma.c) also says that memory is still stolen.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30881 )
Change subject: nb/intel/i945: Reduce pcidev_on_root() calls ......................................................................
Patch Set 1: Code-Review+2
(1 comment)
https://review.coreboot.org/#/c/30881/1/src/northbridge/intel/i945/northbrid... File src/northbridge/intel/i945/northbridge.c:
https://review.coreboot.org/#/c/30881/1/src/northbridge/intel/i945/northbrid... PS1, Line 87: if (!(reg16 & 2)) {
AFAICT, this bit is only about legacy VGA cycles... […]
Not sure I understand what you mean here. Memory stolen regardless of this bit being set? Looks like we only reserve UMA resource when bit is not set.
Followup needed? As regression fix I think this can still go in...
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30881 )
Change subject: nb/intel/i945: Reduce pcidev_on_root() calls ......................................................................
Patch Set 1:
this let the board I have to boot. I can see SeaBIOS msg , but as soon as linux try to run X, it went to black screen and got "no signal". looks like there is a conflict between IGD and the external GPU. here is the log: https://pastebin.com/Eid035HV
I'll try other tests and keep you updated.
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30881 )
Change subject: nb/intel/i945: Reduce pcidev_on_root() calls ......................................................................
Patch Set 1:
this let the board I have to boot. I can see SeaBIOS msg , but as soon as linux try to run X, it went to black screen and got "no signal". looks like there is a conflict between IGD and the external GPU. here is the log: https://pastebin.com/Eid035HV
I'll try other tests and keep you updated.
sorry for "looks like there is a conflict between IGD and the external GPU.", this is not true. using lspci, there is no IGD.
The board boots fine in console mode, however, when Linux start X, it fails.
inside Xorg log, I can see: UnloadSubModule VBE unloading VBE Screen(s) found, but none have a usable configuration.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30881 )
Change subject: nb/intel/i945: Reduce pcidev_on_root() calls ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/30881/1/src/northbridge/intel/i945/northbrid... File src/northbridge/intel/i945/northbridge.c:
https://review.coreboot.org/#/c/30881/1/src/northbridge/intel/i945/northbrid... PS1, Line 87: if (!(reg16 & 2)) {
Not sure I understand what you mean here. […]
It depends a lot on how i945 handles the stolen memory, e.g. what happens when the CPU tries to access it anyway? Also, not well documented is whether the memory is stolen even with 0:2.0 disabled in the DEVEN register (what we do too). But there is trouble beside this, see below.
We have two cases when we disable the IGD. 1. in romstage if a VGA adapter is found in the PEG slot. 2. in ramstage if a VGA adapter is found in another slot and ONBOARD_VGA_IS_PRIMARY is unset.
The 2. case seems borked. It hides IGD through DEVEN and that shifts cbmem_top(). I'd say we shouldn't hide IGD in ramstage, dis- abling VGA decoding should be enough for !ONBOARD_VGA_IS_PRIMARY. Then we could change the check here to `DEVEN & x`.
Still needs somebody to test the changes... I might still have the original hardware at work for that this was written, though little time.
Nico Huber has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/30881 )
Change subject: nb/intel/i945: Reduce pcidev_on_root() calls ......................................................................
nb/intel/i945: Reduce pcidev_on_root() calls
Also removes one call for 0:2.0 (integrated graphics) that might be disabled.
Change-Id: I494aa366030b77baf431f29ba331f13f7c567025 Signed-off-by: Nico Huber nico.h@gmx.de Reviewed-on: https://review.coreboot.org/c/30881 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/northbridge/intel/i945/northbridge.c 1 file changed, 7 insertions(+), 7 deletions(-)
Approvals: build bot (Jenkins): Verified Kyösti Mälkki: Looks good to me, approved
diff --git a/src/northbridge/intel/i945/northbridge.c b/src/northbridge/intel/i945/northbridge.c index 2b51b5e..10a7ba4 100644 --- a/src/northbridge/intel/i945/northbridge.c +++ b/src/northbridge/intel/i945/northbridge.c @@ -66,6 +66,7 @@ unsigned long long tomk, tomk_stolen; uint64_t uma_memory_base = 0, uma_memory_size = 0; uint64_t tseg_memory_base = 0, tseg_memory_size = 0; + struct device *const d0f0 = pcidev_on_root(0, 0);
pci_domain_read_resources(dev);
@@ -75,17 +76,14 @@ pci_tolm = find_pci_tolm(dev->link_list); printk(BIOS_DEBUG, "pci_tolm: 0x%x\n", pci_tolm);
- printk(BIOS_SPEW, "Base of stolen memory: 0x%08x\n", - pci_read_config32(pcidev_on_root(2, 0), BSM)); - - tolud = pci_read_config8(pcidev_on_root(0, 0), TOLUD); + tolud = pci_read_config8(d0f0, TOLUD); printk(BIOS_SPEW, "Top of Low Used DRAM: 0x%08x\n", tolud << 24);
tomk = tolud << 14; tomk_stolen = tomk;
/* Note: subtract IGD device and TSEG */ - reg16 = pci_read_config16(pcidev_on_root(0, 0), GGC); + reg16 = pci_read_config16(d0f0, GGC); if (!(reg16 & 2)) { printk(BIOS_DEBUG, "IGD decoded, subtracting "); int uma_size = decode_igd_memory_size((reg16 >> 4) & 7); @@ -96,10 +94,12 @@ /* For reserving UMA memory in the memory map */ uma_memory_base = tomk_stolen * 1024ULL; uma_memory_size = uma_size * 1024ULL; + + printk(BIOS_SPEW, "Base of stolen memory: 0x%08x\n", + (unsigned int)uma_memory_base); }
- tseg_sizek = decode_tseg_size(pci_read_config8(pcidev_on_root(0, 0), - ESMRAMC)) >> 10; + tseg_sizek = decode_tseg_size(pci_read_config8(d0f0, ESMRAMC)) >> 10; printk(BIOS_DEBUG, "TSEG decoded, subtracting %dM\n", tseg_sizek >> 10); tomk_stolen -= tseg_sizek; tseg_memory_base = tomk_stolen * 1024ULL;
-
HAOUAS Elyes (Code Review) -
Kyösti Mälkki (Code Review)
-
Nico Huber (Code Review)