Change in coreboot[master]: src/northbridge/amd/agesa: fix the device resource assignment
Mike Banon has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/41431 )
Change subject: src/northbridge/amd/agesa: fix the device resource assignment ......................................................................
src/northbridge/amd/agesa: fix the device resource assignment
The chipset code was incorrectly adding memory resources to the domain device after resource allocation occurred. It's not possible to get the correct view of the address space, and it's generally incorrect to not add resources during read_resources(). This change fixes the order by adding resources during read_resources().
This successfully fixes the boot problems of fam15tn A88XM-E but fails to fix a fam16kb AM1I-A. After fam16kb will be fixed, this change should be updated with a fix for fam14 as well.
Inspired by CB:41369.
Signed-off-by: Mike Banon mikebdp2@gmail.com Change-Id: I5df6d0150e777cd21f6f9a2fc776f3548da8201d --- M src/northbridge/amd/agesa/family15tn/northbridge.c M src/northbridge/amd/agesa/family16kb/northbridge.c 2 files changed, 12 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/31/41431/1
diff --git a/src/northbridge/amd/agesa/family15tn/northbridge.c b/src/northbridge/amd/agesa/family15tn/northbridge.c index 9d41e7a..0194ea8 100644 --- a/src/northbridge/amd/agesa/family15tn/northbridge.c +++ b/src/northbridge/amd/agesa/family15tn/northbridge.c @@ -666,6 +666,8 @@ u32 reset_memhole = 1; #endif
+ domain_read_resources(dev); + pci_tolm = 0xffffffffUL; for (link = dev->link_list; link; link = link->next) { pci_tolm = find_pci_tolm(link); @@ -749,17 +751,18 @@ }
add_uma_resource_below_tolm(dev, 7); - +/* for (link = dev->link_list; link; link = link->next) { if (link->children) { assign_resources(link); } } +*/ }
static struct device_operations pci_domain_ops = { - .read_resources = domain_read_resources, - .set_resources = domain_set_resources, + .read_resources = domain_set_resources, + .set_resources = pci_domain_set_resources, .scan_bus = pci_domain_scan_bus, };
diff --git a/src/northbridge/amd/agesa/family16kb/northbridge.c b/src/northbridge/amd/agesa/family16kb/northbridge.c index cedc7da..0467b27 100644 --- a/src/northbridge/amd/agesa/family16kb/northbridge.c +++ b/src/northbridge/amd/agesa/family16kb/northbridge.c @@ -683,6 +683,8 @@ u32 reset_memhole = 1; #endif
+ domain_read_resources(dev); + pci_tolm = 0xffffffffUL; for (link = dev->link_list; link; link = link->next) { pci_tolm = find_pci_tolm(link); @@ -768,11 +770,13 @@
add_uma_resource_below_tolm(dev, 7);
+/* for (link = dev->link_list; link; link = link->next) { if (link->children) { assign_resources(link); } } +*/ }
static const char *domain_acpi_name(const struct device *dev) @@ -784,8 +788,8 @@ }
static struct device_operations pci_domain_ops = { - .read_resources = domain_read_resources, - .set_resources = domain_set_resources, + .read_resources = domain_set_resources, + .set_resources = pci_domain_set_resources, .scan_bus = pci_domain_scan_bus, .acpi_name = domain_acpi_name, };
Mike Banon has uploaded a new patch set (#2). ( https://review.coreboot.org/c/coreboot/+/41431 )
Change subject: src/northbridge/amd/agesa: add resources during read_resources() ......................................................................
src/northbridge/amd/agesa: add resources during read_resources()
The chipset code was incorrectly adding memory resources to the domain device after resource allocation occurred. It's not possible to get the correct view of the address space, and it's generally incorrect to not add resources during read_resources(). This change fixes the order by adding resources during read_resources().
This successfully fixes the boot problems of fam15tn A88XM-E but fails to fix a fam16kb AM1I-A. After fam16kb will be fixed, this change should be updated with a fix for fam14 as well.
Inspired by CB:41369.
Signed-off-by: Mike Banon mikebdp2@gmail.com Change-Id: I5df6d0150e777cd21f6f9a2fc776f3548da8201d --- M src/northbridge/amd/agesa/family15tn/northbridge.c M src/northbridge/amd/agesa/family16kb/northbridge.c 2 files changed, 12 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/31/41431/2
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41431 )
Change subject: src/northbridge/amd/agesa: add resources during read_resources() ......................................................................
Patch Set 2:
(2 comments)
Is this still needed? Sorry if it fell into oblivion.
https://review.coreboot.org/c/coreboot/+/41431/2/src/northbridge/amd/agesa/f... File src/northbridge/amd/agesa/family15tn/northbridge.c:
https://review.coreboot.org/c/coreboot/+/41431/2/src/northbridge/amd/agesa/f... PS2, Line 764: set Not right either
https://review.coreboot.org/c/coreboot/+/41431/2/src/northbridge/amd/agesa/f... File src/northbridge/amd/agesa/family16kb/northbridge.c:
https://review.coreboot.org/c/coreboot/+/41431/2/src/northbridge/amd/agesa/f... PS2, Line 791: set That does not look right.
Mike Banon has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41431 )
Change subject: src/northbridge/amd/agesa: add resources during read_resources() ......................................................................
Patch Set 2:
Patch Set 2:
(2 comments)
Is this still needed? Sorry if it fell into oblivion.
This was my dirty hack to get a v4 resource allocator working for these AMD boards. Maybe it could be used as the base for a new attempt, but I'm not sure if I can pull this out by myself (to be honest I barely understand this code)
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41431 )
Change subject: src/northbridge/amd/agesa: add resources during read_resources() ......................................................................
Patch Set 2:
Patch Set 2:
Patch Set 2:
(2 comments)
Is this still needed? Sorry if it fell into oblivion.
This was my dirty hack to get a v4 resource allocator working for these AMD boards. Maybe it could be used as the base for a new attempt, but I'm not sure if I can pull this out by myself (to be honest I barely understand this code)
You can check Intel northbridges as an example.
read_resources should inform the allocator of the resources it has to take into account when allocating, e.g. MMIO or I/O ranges, which could be dynamically allocated (PCI BARs) or are fixed at a specific address (MCHBAR, DMIBAR, EPBAR; they are configurable but we hardcode the base addresses).
Then, the allocator does its thing.
set_resources then writes the allocator results into the hardware registers. For fixed resources, this is a no-op.
-
Angel Pons (Code Review) -
Mike Banon (Code Review)