[coreboot-gerrit] Change in coreboot[master]: northbridge/via/cn700: Add IORESOURCE_BRIDGE resources to AG...

[Martin Roth (Code Review)]

Martin Roth has submitted this change and it was merged. ( https://review.coreboot.org/18894 )

Change subject: northbridge/via/cn700: Add IORESOURCE_BRIDGE resources to AGP bridge
......................................................................


northbridge/via/cn700: Add IORESOURCE_BRIDGE resources to AGP bridge

Without them the BS_DEV_RESOURCES stage won't traverse the bridge and
the graphics controller would be left without resources assigned.

Even worse, the resources would stay based in offset 0 which confuses
the MTRR setting code and causes a good chunk of the DRAM to be set
to type write combining.

With the patch applied, the resources are set:

 Show resources in subtree (Root Device)...After assigning values.
...
    PCI: 00:01.0 child on link 0 PCI: 01:00.0
+   PCI: 00:01.0 resource base ffff size 0 align 0 gran 0 limit ffff flags 60080100 index 0
+   PCI: 00:01.0 resource base f8000000 size 4000000 align 26 gran 0 limit fbffffff flags 60081200 index 1
+   PCI: 00:01.0 resource base fc000000 size 1010000 align 24 gran 0 limit fd00ffff flags 60080200 index 2
     PCI: 01:00.0
-    PCI: 01:00.0 resource base 0 size 4000000 align 26 gran 26 limit ffffffff flags 1200 index 10
-    PCI: 01:00.0 resource base 0 size 1000000 align 24 gran 24 limit ffffffff flags 200 index 14
-    PCI: 01:00.0 resource base 0 size 10000 align 16 gran 16 limit ffffffff flags 2200 index 30
+    PCI: 01:00.0 resource base f8000000 size 4000000 align 26 gran 26 limit fbffffff flags 60001200 index 10
+    PCI: 01:00.0 resource base fc000000 size 1000000 align 24 gran 24 limit fcffffff flags 60000200 index 14
+    PCI: 01:00.0 resource base fd000000 size 10000 align 16 gran 16 limit fd00ffff flags 60002200 index 30

And the caching mode is set properly:

 MTRR: Physical address space:
-0x0000000000000000 - 0x0000000004000000 size 0x04000000 type 1
-0x0000000004000000 - 0x000000000e000000 size 0x0a000000 type 6
-0x000000000e000000 - 0x0000000100000000 size 0xf2000000 type 0
+0x0000000000000000 - 0x00000000000a0000 size 0x000a0000 type 6
+0x00000000000a0000 - 0x00000000000c0000 size 0x00020000 type 0
+0x00000000000c0000 - 0x000000000e000000 size 0x0df40000 type 6
+0x000000000e000000 - 0x00000000f8000000 size 0xea000000 type 0
+0x00000000f8000000 - 0x00000000fc000000 size 0x04000000 type 1
+0x00000000fc000000 - 0x0000000100000000 size 0x04000000 type 0

The problem was also spot and discussed here:
http://coreboot.coreboot.narkive.com/E9eGauzH/via-c7-on-bcom-winnet-p680-l1-l2-cache-very-slow

Change-Id: Idb4979b206838dd6455b2a16de14dc74f83af921
Signed-off-by: Lubomir Rintel <lkundrak at v3.sk>
Reviewed-on: https://review.coreboot.org/18894
Tested-by: build bot (Jenkins)
Reviewed-by: Martin Roth <martinroth at google.com>
---
M src/northbridge/via/cn700/agp.c
1 file changed, 31 insertions(+), 1 deletion(-)

Approvals:
  build bot (Jenkins): Verified
  Martin Roth: Looks good to me, approved



diff --git a/src/northbridge/via/cn700/agp.c b/src/northbridge/via/cn700/agp.c
index c4375ea..55e5fe3 100644
--- a/src/northbridge/via/cn700/agp.c
+++ b/src/northbridge/via/cn700/agp.c
@@ -152,8 +152,38 @@
 	pci_write_config8(dev, 0x45, 0x72);
 }
 
+static void agp_bridge_read_resources(device_t dev)
+{
+	struct resource *resource;
+
+	resource = new_resource(dev, 0);
+	if (resource) {
+		resource->base	= 0;
+		resource->size	= 0;
+		resource->limit = 0xffffUL;
+		resource->flags = IORESOURCE_IO | IORESOURCE_BRIDGE;
+	}
+
+	resource = new_resource(dev, 1);
+	if (resource) {
+		resource->base = 0;
+		resource->size = 0;
+		resource->limit = 0xfffffffffULL;
+		resource->flags = IORESOURCE_MEM | IORESOURCE_PREFETCH;
+		resource->flags |= IORESOURCE_BRIDGE;
+	}
+
+	resource = new_resource(dev, 2);
+	if (resource) {
+		resource->base = 0;
+		resource->size = 0;
+		resource->limit = 0xffffffffULL;
+		resource->flags = IORESOURCE_MEM | IORESOURCE_BRIDGE;
+	}
+}
+
 static const struct device_operations agp_bridge_operations = {
-	.read_resources   = DEVICE_NOOP,
+	.read_resources   = agp_bridge_read_resources,
 	.set_resources    = pci_dev_set_resources,
 	.enable_resources = pci_bus_enable_resources,
 	.init             = agp_bridge_init,

-- 
To view, visit https://review.coreboot.org/18894
To unsubscribe, visit https://review.coreboot.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Idb4979b206838dd6455b2a16de14dc74f83af921
Gerrit-PatchSet: 5
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Owner: Lubomir Rintel <lkundrak at v3.sk>
Gerrit-Reviewer: Martin Roth <martinroth at google.com>
Gerrit-Reviewer: Paul Menzel <paulepanter at users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins)