This is a multi-part message in MIME format.
--------------080702070007040304010003
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit

When drm was introduced we had the problem that agp_i810.c
and i915_drv.c both need access to the same memory region.

 http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/dev/pci/agp_i810.c.diff?r1=1.36&r2=1.37
 http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/dev/pci/agp_i810.c.diff?r1=1.39&r2=1.40

I have attached a patch which attempts to resolve the problem.

I assumed, for simplicity, that the bus space tag need not be returned
by AGP_GET_PCIMAP, since i915_drv can get the tag when attaching.

Another assumption is that the other agp drivers need not implement
get_pcimap unless there is a known conflict.

Would it be better to use AGP_GET_PCIMAP in drm_ioremap?
(sys/dev/drm/drm_memory.h)

Is this an acceptable solution?

I would appreciate any comments.

-- 
Kind regards,

Yorick Hardy

--------------080702070007040304010003
Content-Type: text/plain;
 name="drm.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="drm.patch"

--- sys/dev/drm/drm_drv.c.orig	2007-10-07 13:28:17.000000000 +0200
+++ sys/dev/drm/drm_drv.c	2007-10-07 13:32:54.000000000 +0200
@@ -526,7 +526,7 @@
 	}
 
 	for(i = 0; i<DRM_MAX_PCI_RESOURCE; i++) {
-		if (dev->pci_map_data[i].mapped > 1) {
+		if (dev->pci_map_data[i].mapped > 0) {
 			bus_space_unmap(dev->pci_map_data[i].maptype,
 					dev->pci_map_data[i].bsh,
 					dev->pci_map_data[i].size);
@@ -691,7 +691,7 @@
 	i = 0;
 
 	for (i = 0; i < DRM_MAX_PCI_RESOURCE; i++)
-		if (dev->pci_map_data[i].mapped != 0)
+		if (dev->pci_map_data[i].mapped > 0)
 		{
 			bus_space_unmap(dev->pa.pa_memt,
 					dev->pci_map_data[i].bsh,
--- sys/dev/drm/drm_memory.c.orig	2007-10-07 13:28:05.000000000 +0200
+++ sys/dev/drm/drm_memory.c	2007-10-07 13:38:35.000000000 +0200
@@ -97,8 +97,9 @@
 			map->bst = dev->pa.pa_memt;
 			map->cnt = &(dev->pci_map_data[i].mapped);
 			map->mapsize = dev->pci_map_data[i].size;
-			dev->pci_map_data[i].mapped++;
-			if (dev->pci_map_data[i].mapped > 1)
+			if (dev->pci_map_data[i].mapped >= 0)
+				dev->pci_map_data[i].mapped++;
+			if (dev->pci_map_data[i].mapped != 1)
 			{
 				map->bsh = dev->pci_map_data[i].bsh;
 				return dev->pci_map_data[i].vaddr;
--- sys/dev/pci/drm/i915_drv.c.orig	2007-10-07 15:48:45.000000000 +0200
+++ sys/dev/pci/drm/i915_drv.c	2007-10-07 18:07:43.000000000 +0200
@@ -36,6 +36,7 @@
 
 #include <dev/drm/drmP.h>
 #include <dev/drm/drm.h>
+#include <dev/pci/agpvar.h>
 #include "i915_drm.h"
 #include "i915_drv.h"
 #include "drm_pciids.h"
@@ -133,10 +134,27 @@
 {
 	struct pci_attach_args *pa = aux;
 	drm_device_t *dev = (drm_device_t *)self;
+	struct agp_softc *agpsc;
 
 	i915_configure(dev);
 
 	drm_attach(self, pa, i915_pciidlist);
+
+	agpsc = (struct agp_softc *)agp_find_device(0);
+
+	if(agpsc == NULL || agpsc->as_chipc == NULL)
+		return;
+
+        /*
+	 * AGP_I915_MMADR might be in use by agp_i810.c 
+         * AGP_I915_MMADR is 0x10 (register 0)
+	 */
+	if(AGP_GET_PCIMAP(agpsc, 0x10, &dev->pci_map_data[0].bsh)) {
+		dev->pci_map_data[0].mapped = -1; /* never unmap */
+		dev->pci_map_data[0].vaddr =
+			bus_space_vaddr(dev->pa.pa_memt,
+					dev->pci_map_data[0].bsh);
+	}
 }
 
 CFATTACH_DECL(i915drm, sizeof(drm_device_t), i915drm_probe, i915drm_attach,
--- sys/dev/pci/agpvar.h.orig	2007-10-07 16:38:28.000000000 +0200
+++ sys/dev/pci/agpvar.h	2007-10-07 16:49:28.000000000 +0200
@@ -111,6 +111,7 @@
 	int (*free_memory)(struct agp_softc *, struct agp_memory *);
 	int (*bind_memory)(struct agp_softc *, struct agp_memory *, off_t);
 	int (*unbind_memory)(struct agp_softc *, struct agp_memory *);
+	int (*get_pcimap)(struct agp_softc *, int, bus_space_handle_t *);
 };
 
 #define AGP_GET_APERTURE(sc)	 ((sc)->as_methods->get_aperture(sc))
@@ -123,6 +124,7 @@
 #define AGP_FREE_MEMORY(sc,m)	 ((sc)->as_methods->free_memory((sc),(m)))
 #define AGP_BIND_MEMORY(sc,m,o)	 ((sc)->as_methods->bind_memory((sc),(m),(o)))
 #define AGP_UNBIND_MEMORY(sc,m)	 ((sc)->as_methods->unbind_memory((sc),(m)))
+#define AGP_GET_PCIMAP(sc,r,h)	 ((sc)->as_methods->get_pcimap((sc),(r),(h)))
 
 /*
  * All chipset drivers must have this at the start of their softc.
@@ -178,6 +180,7 @@
 int agp_generic_free_memory(struct agp_softc *, struct agp_memory *);
 int agp_generic_bind_memory(struct agp_softc *, struct agp_memory *, off_t);
 int agp_generic_unbind_memory(struct agp_softc *, struct agp_memory *);
+int agp_generic_get_pcimap(struct agp_softc *, int, bus_space_handle_t *);
 
 /* The vendor has already been matched when these functions are called */
 int agp_amd_match(const struct pci_attach_args *);
--- sys/dev/pci/agp.c.orig	2007-10-07 16:48:07.000000000 +0200
+++ sys/dev/pci/agp.c	2007-10-07 16:52:10.000000000 +0200
@@ -685,6 +685,13 @@
 	return 0;
 }
 
+int
+agp_generic_get_pcimap(struct agp_softc *sc, int reg, bus_space_handle_t *h)
+{
+	printf("%s: get_pcimap not implemented\n", sc->as_dev.dv_xname);
+	return 0;
+}
+
 /* Helper functions for implementing user/kernel api */
 
 static int
--- sys/dev/pci/agp_ali.c.orig	2007-10-07 16:54:19.000000000 +0200
+++ sys/dev/pci/agp_ali.c	2007-10-07 16:54:39.000000000 +0200
@@ -74,6 +74,7 @@
 	agp_generic_free_memory,
 	agp_generic_bind_memory,
 	agp_generic_unbind_memory,
+	agp_generic_get_pcimap,
 };
 
 int
--- sys/dev/pci/agp_amd.c.orig	2007-10-07 16:54:45.000000000 +0200
+++ sys/dev/pci/agp_amd.c	2007-10-07 16:54:55.000000000 +0200
@@ -92,6 +92,7 @@
 	agp_generic_free_memory,
 	agp_generic_bind_memory,
 	agp_generic_unbind_memory,
+	agp_generic_get_pcimap,
 };
 
 
--- sys/dev/pci/agp_amd64.c.orig	2007-10-07 16:54:56.000000000 +0200
+++ sys/dev/pci/agp_amd64.c	2007-10-07 16:55:04.000000000 +0200
@@ -98,6 +98,7 @@
 	agp_generic_free_memory,
 	agp_generic_bind_memory,
 	agp_generic_unbind_memory,
+	agp_generic_get_pcimap,
 };
 
 
--- sys/dev/pci/agp_apple.c.orig	2007-10-07 16:55:07.000000000 +0200
+++ sys/dev/pci/agp_apple.c	2007-10-07 16:55:15.000000000 +0200
@@ -67,6 +67,7 @@
 	agp_generic_free_memory,
 	agp_generic_bind_memory,
 	agp_generic_unbind_memory,
+	agp_generic_get_pcimap,
 };
 
 struct agp_apple_softc {
--- sys/dev/pci/agp_i810.c.orig	2007-10-07 16:55:17.000000000 +0200
+++ sys/dev/pci/agp_i810.c	2007-10-07 18:56:54.000000000 +0200
@@ -109,6 +109,7 @@
 static int agp_i810_free_memory(struct agp_softc *, struct agp_memory *);
 static int agp_i810_bind_memory(struct agp_softc *, struct agp_memory *, off_t);
 static int agp_i810_unbind_memory(struct agp_softc *, struct agp_memory *);
+static int agp_i810_get_pcimap(struct agp_softc *, int , bus_space_handle_t *);
 static void agp_i810_powerhook(int, void *);
 
 static struct agp_methods agp_i810_methods = {
@@ -122,6 +123,7 @@
 	agp_i810_free_memory,
 	agp_i810_bind_memory,
 	agp_i810_unbind_memory,
+	agp_i810_get_pcimap,
 };
 
 /* XXXthorpej -- duplicated code (see arch/i386/pci/pchb.c) */
@@ -441,14 +443,6 @@
 		printf("%s: WARNING: unable to establish PCI power hook\n",
 		    sc->as_dev.dv_xname);
 
-#if 0
-	/*      
-	 * another device (drm) may need access to this region
-	 * we do not need it anymore
-	 */     
-	bus_space_unmap(isc->bst, isc->bsh, mmadrsize);
-#endif
-
 	return 0;
 }
 
@@ -767,7 +761,6 @@
 	 * Until the issue is solved, simply restore it.
 	 */
 
-#if 0
 	regval = bus_space_read_4(isc->bst, isc->bsh, AGP_I810_PGTBL_CTL);
 	if (regval != (isc->gatt->ag_physical | 1)) {
 		printf("agp_i810_bind_memory: PGTBL_CTL is 0x%x - fixing\n",
@@ -775,8 +768,6 @@
 		bus_space_write_4(isc->bst, isc->bsh, AGP_I810_PGTBL_CTL,
 				  isc->gatt->ag_physical | 1);
 	}
-#endif
-	regval = 0;
 
 	if (mem->am_type == 2) {
 		WRITEGTT(offset, mem->am_physical | 1);
@@ -822,6 +813,23 @@
 	return 0;
 }
 
+static int
+agp_i810_get_pcimap(struct agp_softc *sc, int reg, bus_space_handle_t *bh)
+{
+	struct agp_i810_softc *isc = sc->as_chipc;
+	if((reg == AGP_I810_MMADR && isc->chiptype == CHIP_I810) ||
+	   (reg == AGP_I915_MMADR && isc->chiptype == CHIP_I915) ||
+	   (reg == AGP_I965_MMADR && isc->chiptype == CHIP_I965)) {
+		*bh = isc->bsh;
+		return 1;
+	}
+	if(reg == AGP_I915_GTTADR && isc->chiptype == CHIP_I915) {
+		*bh = isc->gtt_bsh; /* should this be allowed? */
+		return 1;
+	}
+	return 0;
+}
+
 static void
 agp_i810_powerhook(int why, void *arg)
 {
--- sys/dev/pci/agp_intel.c.orig	2007-10-07 16:55:27.000000000 +0200
+++ sys/dev/pci/agp_intel.c	2007-10-07 16:55:38.000000000 +0200
@@ -86,6 +86,7 @@
 	agp_generic_free_memory,
 	agp_generic_bind_memory,
 	agp_generic_unbind_memory,
+	agp_generic_get_pcimap,
 };
 
 static int
--- sys/dev/pci/agp_sis.c.orig	2007-10-07 16:55:39.000000000 +0200
+++ sys/dev/pci/agp_sis.c	2007-10-07 16:55:47.000000000 +0200
@@ -72,6 +72,7 @@
 	agp_generic_free_memory,
 	agp_generic_bind_memory,
 	agp_generic_unbind_memory,
+	agp_generic_get_pcimap,
 };
 
 int
--- sys/dev/pci/agp_via.c.orig	2007-10-07 16:55:48.000000000 +0200
+++ sys/dev/pci/agp_via.c	2007-10-07 16:55:58.000000000 +0200
@@ -68,6 +68,7 @@
 	agp_generic_free_memory,
 	agp_generic_bind_memory,
 	agp_generic_unbind_memory,
+	agp_generic_get_pcimap,
 };
 
 struct agp_via_softc {

--------------080702070007040304010003--