Hi.

Since I switched to libpciaccess, I've been seeing errors like:

> error setting MTRR (base = 0x14200000, size = 0x00800000, type = 1) Invalid 
> argument (22)

It's because the GPU I'm using (a siliconmotion SM720) has a register
area at the beginning of the PCI BAR which shouldn't be set to WC, and
the remaining memory (the actual framebuffer) isn't aligned at a size
multiple.

I've been running a modified version of libpciaccess (diff attached)
that splits the memory range on properly aligned chunks before setting
the MTRRs.

If it turns out to be a reasonable approach, I'm willing to document
it more thoroughly and format it for git-am.

I've only implemented it for the linux_sysfs backend, but the logic at
pci_mapping_split is somewhat generic and another source file might be
better suited for it.

diff --git a/src/linux_sysfs.c b/src/linux_sysfs.c
index 8c3cf67..b99cb31 100644
--- a/src/linux_sysfs.c
+++ b/src/linux_sysfs.c
@@ -95,6 +95,16 @@ static const struct pci_system_methods linux_sysfs_methods = {
 
 static int populate_entries(struct pci_system * pci_sys);
 
+#ifdef HAVE_MTRR
+static int pci_device_linux_sysfs_set_mtrr(struct pci_device_mapping *map);
+
+static int pci_device_linux_sysfs_unset_mtrr(struct pci_device_mapping *map);
+
+#endif
+
+static int pci_mapping_split(struct pci_device_mapping *map,
+			  int (*proc)(struct pci_device_mapping*));
+
 
 /**
  * Attempt to access PCI subsystem using Linux's sysfs interface.
@@ -547,13 +557,6 @@ pci_device_linux_sysfs_map_range(struct pci_device *dev,
     const int open_flags = ((map->flags & PCI_DEV_MAP_FLAG_WRITABLE) != 0) 
         ? O_RDWR : O_RDONLY;
     const off_t offset = map->base - dev->regions[map->region].base_addr;
-#ifdef HAVE_MTRR
-    struct mtrr_sentry sentry = {
-	.base = map->base,
-        .size = map->size,
-	.type = MTRR_TYPE_UNCACHABLE
-    };
-#endif
 
     /* For WC mappings, try sysfs resourceN_wc file first */
     if ((map->flags & PCI_DEV_MAP_FLAG_WRITE_COMBINE) &&
@@ -582,22 +585,12 @@ pci_device_linux_sysfs_map_range(struct pci_device *dev,
     }
 
 #ifdef HAVE_MTRR
-    if ((map->flags & PCI_DEV_MAP_FLAG_CACHABLE) != 0) {
-        sentry.type = MTRR_TYPE_WRBACK;
-    } else if ((map->flags & PCI_DEV_MAP_FLAG_WRITE_COMBINE) != 0) {
-        sentry.type = MTRR_TYPE_WRCOMB;
-    }
+    if (pci_sys->mtrr_fd != -1 &&
+	(map->flags & (PCI_DEV_MAP_FLAG_CACHABLE |
+		       PCI_DEV_MAP_FLAG_WRITE_COMBINE))){
+
+	(void) pci_mapping_split(map, pci_device_linux_sysfs_set_mtrr);
 
-    if (pci_sys->mtrr_fd != -1 && sentry.type != MTRR_TYPE_UNCACHABLE) {
-	if (ioctl(pci_sys->mtrr_fd, MTRRIOC_ADD_ENTRY, &sentry) < 0) {
-	    /* FIXME: Should we report an error in this case?
-	     */
-	    fprintf(stderr, "error setting MTRR "
-		    "(base = 0x%08lx, size = 0x%08x, type = %u) %s (%d)\n",
-		    sentry.base, sentry.size, sentry.type,
-		    strerror(errno), errno);
-/*            err = errno;*/
-	}
 	/* KLUDGE ALERT -- rewrite the PTEs to turn off the CD and WT bits */
 	mprotect (map->memory, map->size, PROT_NONE);
 	err = mprotect (map->memory, map->size, PROT_READ|PROT_WRITE);
@@ -644,35 +637,17 @@ pci_device_linux_sysfs_unmap_range(struct pci_device *dev,
 				   struct pci_device_mapping *map)
 {
     int err = 0;
-#ifdef HAVE_MTRR
-    struct mtrr_sentry sentry = {
-	.base = map->base,
-        .size = map->size,
-	.type = MTRR_TYPE_UNCACHABLE
-    };
-#endif
 
     err = pci_device_generic_unmap_range (dev, map);
     if (err)
 	return err;
     
 #ifdef HAVE_MTRR
-    if ((map->flags & PCI_DEV_MAP_FLAG_CACHABLE) != 0) {
-        sentry.type = MTRR_TYPE_WRBACK;
-    } else if ((map->flags & PCI_DEV_MAP_FLAG_WRITE_COMBINE) != 0) {
-        sentry.type = MTRR_TYPE_WRCOMB;
-    }
+    if (pci_sys->mtrr_fd != -1 &&
+	(map->flags & (PCI_DEV_MAP_FLAG_CACHABLE |
+		       PCI_DEV_MAP_FLAG_WRITE_COMBINE))){
 
-    if (pci_sys->mtrr_fd != -1 && sentry.type != MTRR_TYPE_UNCACHABLE) {
-	if (ioctl(pci_sys->mtrr_fd, MTRRIOC_DEL_ENTRY, &sentry) < 0) {
-	    /* FIXME: Should we report an error in this case?
-	     */
-	    fprintf(stderr, "error setting MTRR "
-		    "(base = 0x%08lx, size = 0x%08x, type = %u) %s (%d)\n",
-		    sentry.base, sentry.size, sentry.type,
-		    strerror(errno), errno);
-/*            err = errno;*/
-	}
+	(void) pci_mapping_split(map, pci_device_linux_sysfs_unset_mtrr);
     }
 #endif
 
@@ -698,3 +673,83 @@ static void pci_device_linux_sysfs_enable(struct pci_device *dev)
     write( fd, "1", 1 );
     close(fd);
 }
+
+#ifdef HAVE_MTRR
+
+static int
+pci_device_linux_sysfs_set_mtrr(struct pci_device_mapping *map)
+{
+    struct mtrr_sentry sentry = {
+	.base = map->base,
+        .size = map->size,
+	.type = ((map->flags & PCI_DEV_MAP_FLAG_CACHABLE) ? MTRR_TYPE_WRBACK :
+		 (map->flags & PCI_DEV_MAP_FLAG_WRITE_COMBINE) ? MTRR_TYPE_WRCOMB :
+		 MTRR_TYPE_UNCACHABLE)
+    };
+
+    if (ioctl(pci_sys->mtrr_fd, MTRRIOC_ADD_ENTRY, &sentry) < 0) {
+	/* FIXME: Should we report an error in this case?
+	 */
+	fprintf(stderr, "error setting MTRR "
+		"(base = 0x%08lx, size = 0x%08x, type = %u) %s (%d)\n",
+		sentry.base, sentry.size, sentry.type,
+		strerror(errno), errno);
+
+	return errno;
+    }
+
+    return 0;
+}
+
+static int
+pci_device_linux_sysfs_unset_mtrr(struct pci_device_mapping *map)
+{
+    struct mtrr_sentry sentry = {
+	.base = map->base,
+        .size = map->size,
+	.type = ((map->flags & PCI_DEV_MAP_FLAG_CACHABLE) ? MTRR_TYPE_WRBACK :
+		 (map->flags & PCI_DEV_MAP_FLAG_WRITE_COMBINE) ? MTRR_TYPE_WRCOMB :
+		 MTRR_TYPE_UNCACHABLE)
+    };
+
+    if (ioctl(pci_sys->mtrr_fd, MTRRIOC_DEL_ENTRY, &sentry) < 0) {
+	/* FIXME: Should we report an error in this case?
+	 */
+	fprintf(stderr, "error setting MTRR "
+		"(base = 0x%08lx, size = 0x%08x, type = %u) %s (%d)\n",
+		sentry.base, sentry.size, sentry.type,
+		strerror(errno), errno);
+
+	return errno;
+    }
+
+    return 0;
+}
+
+#endif /* HAVE_MTRR */
+
+static int
+pci_mapping_split(struct pci_device_mapping *map,
+		  int (*proc)(struct pci_device_mapping*))
+{
+    struct pci_device_mapping sub = *map;
+    pciaddr_t rem = map->size;
+    int err;
+
+    while(rem > 0){
+	for(sub.size=1;
+	    (sub.base & sub.size) == 0 && sub.size < rem;
+	    sub.size <<= 1)
+	    ;
+
+	err = (*proc)(&sub);
+
+	if(err)
+	    return err;
+
+	sub.base += sub.size;
+	rem -= sub.size;
+    }
+
+    return 0;
+}

Attachment: pgpm2pEcySWvW.pgp
Description: PGP signature

_______________________________________________
xorg mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/xorg

Reply via email to