Numerous amounts refactoring and comment adding (see ChangeLog for file by

    file details). The primary intention for these changes is to pave the
    way for the new device probing and PCI configuration code that I'm
    working on.

7 files changed, 172 insertions, 150 deletions

diff --git a/hw/xfree86/common/xf86.h b/hw/xfree86/common/xf86.h
index 5d9d61e8a..3c416a0be 100644
--- a/hw/xfree86/common/xf86.h
+++ b/hw/xfree86/common/xf86.h
@@ -116,7 +116,6 @@ void xf86EnableAccess(ScrnInfoPtr pScrn);
 void xf86SetCurrentAccess(Bool Enable, ScrnInfoPtr pScrn);
 Bool xf86IsPrimaryPci(pciVideoPtr pPci);
 Bool xf86IsPrimaryIsa(void);
-int xf86CheckPciGAType(pciVideoPtr pPci);
 /* new RAC */
 resPtr xf86AddResToList(resPtr rlist, resRange *Range, int entityIndex);
 resPtr xf86JoinResLists(resPtr rlist1, resPtr rlist2);
diff --git a/hw/xfree86/common/xf86DoProbe.c b/hw/xfree86/common/xf86DoProbe.c
index 2f5b03eb1..b74f37e11 100644
--- a/hw/xfree86/common/xf86DoProbe.c
+++ b/hw/xfree86/common/xf86DoProbe.c
@@ -49,11 +49,6 @@
 #include "xf86Priv.h"
 
 void
-DoProbeArgs(int argc, char **argv, int i)
-{
-}
-
-void
 DoProbe()
 {
     int i;
@@ -82,13 +77,12 @@ DoProbe()
 
     /* Call all of the probe functions, reporting the results. */
     for (i = 0; i < xf86NumDrivers; i++) {
+	DriverRec * const drv = xf86DriverList[i];
 
 	if (!xorgHWAccess) {
 	    xorgHWFlags flags;
-	    if (!xf86DriverList[i]->driverFunc
-		|| !xf86DriverList[i]->driverFunc(NULL,
-						  GET_REQUIRED_HW_INTERFACES,
-						  &flags)
+	    if (!drv->driverFunc
+		|| !drv->driverFunc( NULL, GET_REQUIRED_HW_INTERFACES, &flags )
 		|| NEED_IO_ENABLED(flags)) {
 		if (ioEnableFailed)
 		    continue;
@@ -100,23 +94,23 @@ DoProbe()
 	    }
 	}
 	    
-	if (xf86DriverList[i]->Probe == NULL) continue;
 
-	xf86MsgVerb(X_INFO, 3, "Probing in driver %s\n",
-	    xf86DriverList[i]->driverName);
-	probeResult =
-	    (*xf86DriverList[i]->Probe)(xf86DriverList[i], PROBE_DETECT);
+	xf86MsgVerb(X_INFO, 3, "Probing in driver %s\n",  drv->driverName);
+
+	if (drv->Probe == NULL) continue;
+
+	probeResult = (*drv->Probe)( drv, PROBE_DETECT );
 	if (!probeResult) {
 	    xf86ErrorF("Probe in driver `%s' returns FALSE\n",
-		xf86DriverList[i]->driverName);
+		drv->driverName);
 	} else {
 	    xf86ErrorF("Probe in driver `%s' returns TRUE\n",
-		xf86DriverList[i]->driverName);
+		drv->driverName);
 
 	    /* If we have a result, then call driver's Identify function */
-	    if (xf86DriverList[i]->Identify != NULL) {
-		int verbose = xf86SetVerbosity(1);
-		(*xf86DriverList[i]->Identify)(0);
+	    if (drv->Identify != NULL) {
+		const int verbose = xf86SetVerbosity(1);
+		(*drv->Identify)(0);
 		xf86SetVerbosity(verbose);
 	    }
 	}
diff --git a/hw/xfree86/common/xf86Helper.c b/hw/xfree86/common/xf86Helper.c
index 2c29b5485..fc1ced761 100644
--- a/hw/xfree86/common/xf86Helper.c
+++ b/hw/xfree86/common/xf86Helper.c
@@ -1599,6 +1599,39 @@ struct Inst {
     int 		screen;
 };
 
+
+/**
+ * Find set of unclaimed devices matching a given vendor ID.
+ * 
+ * Used by drivers to find as yet unclaimed devices matching the specified
+ * vendor ID.
+ *
+ * \param driverName     Name of the driver.  This is used to find Device
+ *                       sections in the config file.
+ * \param vendorID       PCI vendor ID of associated devices.  If zero, then
+ *                       the true vendor ID must be encoded in the \c PCIid
+ *                       fields of the \c PCIchipsets entries.
+ * \param chipsets       Symbol table used to associate chipset names with
+ *                       PCI IDs.
+ * \param devList        List of Device sections parsed from the config file.
+ * \param numDevs        Number of entries in \c devList.
+ * \param drvp           Pointer the driver's control structure.
+ * \param foundEntities  Returned list of entity indicies associated with the
+ *                       driver.
+ * 
+ * \returns
+ * The number of elements in returned in \c foundEntities on success or zero
+ * on failure.
+ *
+ * \todo
+ * This function does a bit more than short description says.  Fill in some
+ * more of the details of its operation.
+ *
+ * \todo
+ * The \c driverName parameter is redundant.  It is the same as
+ * \c DriverRec::driverName.  In a future version of this function, remove
+ * that parameter.
+ */
 int
 xf86MatchPciInstances(const char *driverName, int vendorID, 
 		      SymTabPtr chipsets, PciChipsets *PCIchipsets,
@@ -1606,7 +1639,6 @@ xf86MatchPciInstances(const char *driverName, int vendorID,
 		      int **foundEntities)
 {
     int i,j;
-    MessageType from;
     pciVideoPtr pPci, *ppPci;
     struct Inst *instances = NULL;
     int numClaimedInstances = 0;
@@ -1614,81 +1646,115 @@ xf86MatchPciInstances(const char *driverName, int vendorID,
     int numFound = 0;
     SymTabRec *c;
     PciChipsets *id;
-    GDevPtr devBus = NULL;
-    GDevPtr dev = NULL;
     int *retEntities = NULL;
 
     *foundEntities = NULL;
 
-    if (vendorID == 0) {
-	for (ppPci = xf86PciVideoInfo; *ppPci != NULL; ppPci++) {
-	    Bool foundVendor = FALSE;
-	    for (id = PCIchipsets; id->PCIid != -1; id++) {
-	        if ( (((id->PCIid & 0xFFFF0000) >> 16) == (*ppPci)->vendor)) {
-		    if (!foundVendor) {
-	                ++allocatedInstances;
-			instances = xnfrealloc(instances,
-				     allocatedInstances * sizeof(struct Inst));
-			instances[allocatedInstances - 1].pci = *ppPci;
-			instances[allocatedInstances - 1].dev = NULL;
-			instances[allocatedInstances - 1].claimed = FALSE;
-			instances[allocatedInstances - 1].foundHW = FALSE;
-			instances[allocatedInstances - 1].screen = 0;
-			foundVendor = TRUE;
-		    } 
-		    if ((id->PCIid & 0x0000FFFF) == (*ppPci)->chipType) {
-	               instances[allocatedInstances - 1].foundHW = TRUE;
-		       instances[allocatedInstances - 1].chip = id->numChipset;
-		       numFound++;
-		    }
-		}
-	    }
+
+    /* Each PCI device will contribute at least one entry.  Each device
+     * section can contribute at most one entry.  The sum of the two is
+     * guaranteed to be larger than the maximum possible number of entries.
+     * Do this calculation and memory allocation once now to eliminate the
+     * need for realloc calls inside the loop.
+     */
+    if ( !xf86DoProbe && !(xf86DoConfigure && xf86DoConfigurePass1) ) {
+	unsigned max_entries = numDevs;
+	for (ppPci = xf86PciVideoInfo ; *ppPci != NULL ; ppPci++) {
+	    max_entries++;
+	}
+
+	instances = xnfalloc( max_entries * sizeof(struct Inst) );
+    }
+
+    for (ppPci = xf86PciVideoInfo; *ppPci != NULL; ppPci++) {
+	unsigned device_class = ((*ppPci)->class << 16)
+	    | ((*ppPci)->subclass << 8) | ((*ppPci)->interface);
+	Bool foundVendor = FALSE;
+
+
+	pPci = *ppPci;
+
+	/* Convert the pre-PCI 2.0 device class for a VGA adapter to the
+	 * 2.0 version of the same class.
+	 */
+	if ( device_class == 0x00000101 ) {
+	    device_class = 0x00030000;
 	}
-    } else if (vendorID == PCI_VENDOR_GENERIC) {
-	for (ppPci = xf86PciVideoInfo; *ppPci != NULL; ppPci++) {
-	    for (id = PCIchipsets; id->PCIid != -1; id++) {
-		if (id->PCIid == xf86CheckPciGAType(*ppPci)) {
+
+
+	/* Find PCI devices that match the given vendor ID.  The vendor ID is
+	 * either specified explicitly as a parameter to the function or
+	 * implicitly encoded in the high bits of id->PCIid.
+	 *
+	 * The first device with a matching vendor is recorded, even if the
+	 * device ID doesn't match.  This is done because the Device section
+	 * in the xorg.conf file can over-ride the device ID.  A matching PCI
+	 * ID might not be found now, but after the device ID over-ride is
+	 * applied there /might/ be a match.
+	 */
+	for (id = PCIchipsets; id->PCIid != -1; id++) {
+	    const unsigned vendor_id = ((id->PCIid & 0xFFFF0000) >> 16)
+		| vendorID;
+	    const unsigned device_id = (id->PCIid & 0x0000FFFF);
+	    const unsigned match_class = 0x00030000 | id->PCIid;
+
+	    if ( (vendor_id == pPci->vendor)
+		 || ((vendorID == PCI_VENDOR_GENERIC) && (match_class == device_class)) ) {
+		if ( !foundVendor && (instances != NULL) ) {
 		    ++allocatedInstances;
-		    instances = xnfrealloc(instances,
-				  allocatedInstances * sizeof(struct Inst));
 		    instances[allocatedInstances - 1].pci = *ppPci;
 		    instances[allocatedInstances - 1].dev = NULL;
 		    instances[allocatedInstances - 1].claimed = FALSE;
-		    instances[allocatedInstances - 1].foundHW = TRUE;
-		    instances[allocatedInstances - 1].chip = id->numChipset;
+		    instances[allocatedInstances - 1].foundHW = FALSE;
 		    instances[allocatedInstances - 1].screen = 0;
-		    numFound++;
+		    foundVendor = TRUE;
 		}
-	    }
-	}
-    } else {
-	/* Find PCI devices that match the given vendor ID */
-	for (ppPci = xf86PciVideoInfo; (ppPci != NULL)
-	       && (*ppPci != NULL); ppPci++) {
-	    if ((*ppPci)->vendor == vendorID) {
-		++allocatedInstances;
-		instances = xnfrealloc(instances,
-			      allocatedInstances * sizeof(struct Inst));
-		instances[allocatedInstances - 1].pci = *ppPci;
-		instances[allocatedInstances - 1].dev = NULL;
-		instances[allocatedInstances - 1].claimed = FALSE;
-		instances[allocatedInstances - 1].foundHW = FALSE;
-	        instances[allocatedInstances - 1].screen = 0;
-
-		/* Check if the chip type is listed in the chipsets table */
-		for (id = PCIchipsets; id->PCIid != -1; id++) {
-		    if (id->PCIid == (*ppPci)->chipType) {
-			instances[allocatedInstances - 1].chip
-			    = id->numChipset;
+		if ( (device_id == pPci->chipType)
+		     || ((vendorID == PCI_VENDOR_GENERIC) 
+			 && (match_class == device_class)) ) {
+		    if ( instances != NULL ) {
 			instances[allocatedInstances - 1].foundHW = TRUE;
+			instances[allocatedInstances - 1].chip = id->numChipset;
+		    }
+
+
+		    if ( xf86DoConfigure && xf86DoConfigurePass1 ) {
+			if ( xf86CheckPciSlot(pPci->bus, pPci->device,
+					      pPci->func) ) {
+			    GDevPtr pGDev = 
+			      xf86AddDeviceToConfigure( drvp->driverName,
+							pPci, -1 );
+			    if (pGDev) {
+				/* After configure pass 1, chipID and chipRev
+				 * are treated as over-rides, so clobber them
+				 * here.
+				 */
+				pGDev->chipID = -1;
+				pGDev->chipRev = -1;
+			    }
+
+			    numFound++;
+			}
+		    }
+		    else {
 			numFound++;
-			break;
 		    }
+
+		    break;
 		}
 	    }
 	}
     }
 
+
+    /* In "probe only" or "configure" mode (signaled by instances being NULL),
+     * our work is done.  Return the number of detected devices.
+     */
+    if ( instances == NULL ) {
+	return numFound;
+    }
+
+
     /*
      * This may be debatable, but if no PCI devices with a matching vendor
      * type is found, return zero now.  It is probably not desirable to
@@ -1699,34 +1765,6 @@ xf86MatchPciInstances(const char *driverName, int vendorID,
 	return 0;
     }
 
-    if (xf86DoProbe) {
-	xfree(instances);
-	return numFound;
-    }
-
-    if (xf86DoConfigure && xf86DoConfigurePass1) {
-	GDevPtr pGDev;
-	int actualcards = 0;
-	for (i = 0; i < allocatedInstances; i++) {
-	    pPci = instances[i].pci;
-	    if (instances[i].foundHW) {
-		if (!xf86CheckPciSlot(pPci->bus, pPci->device, pPci->func))
-		    continue;
-		actualcards++;
-	    	pGDev = xf86AddDeviceToConfigure(drvp->driverName,
-						 instances[i].pci, -1);
-		if (pGDev) {
-		   /*
-		    * XF86Match???Instances() treat chipID and chipRev as
-		    * overrides, so clobber them here.
-		    */
-		   pGDev->chipID = pGDev->chipRev = -1;
-		}
-	    }
-	}
-	xfree(instances);
-	return actualcards;
-    }
 
 #ifdef DEBUG
     ErrorF("%s instances found: %d\n", driverName, allocatedInstances);
@@ -1749,9 +1787,6 @@ xf86MatchPciInstances(const char *driverName, int vendorID,
 					    pPci->device,
 					    pPci->func)) {
 		    allocatedInstances++;
-		    instances = xnfrealloc(instances,
-					   allocatedInstances * 
-					   sizeof(struct Inst));
 		    instances[allocatedInstances - 1] = instances[i];
 		    instances[allocatedInstances - 1].screen =
 		      				devList[j]->screen;
@@ -1763,9 +1798,10 @@ xf86MatchPciInstances(const char *driverName, int vendorID,
     }
 
     for (i = 0; i < allocatedInstances; i++) {
+	GDevPtr dev = NULL;
+	GDevPtr devBus = NULL;
+
 	pPci = instances[i].pci;
-	devBus = NULL;
-	dev = NULL;
 	for (j = 0; j < numDevs; j++) {
 	    if (devList[j]->busID && *devList[j]->busID) {
 		if (xf86ComparePciBusString(devList[j]->busID, pPci->bus,
@@ -1821,10 +1857,11 @@ xf86MatchPciInstances(const char *driverName, int vendorID,
      * If chipset is not valid ignore BusSlot completely.
      */
     for (i = 0; i < allocatedInstances && numClaimedInstances > 0; i++) {
+	MessageType from = X_PROBED;
+
 	if (!instances[i].claimed) {
 	    continue;
 	}
-	from = X_PROBED;
 	if (instances[i].dev->chipset) {
 	    for (c = chipsets; c->token >= 0; c++) {
 		if (xf86NameCmp(c->name, instances[i].dev->chipset) == 0)
diff --git a/hw/xfree86/common/xf86Init.c b/hw/xfree86/common/xf86Init.c
index 6e357dbc9..5af479a1a 100644
--- a/hw/xfree86/common/xf86Init.c
+++ b/hw/xfree86/common/xf86Init.c
@@ -1,5 +1,5 @@
 /* $XFree86: xc/programs/Xserver/hw/xfree86/common/xf86Init.c,v 3.212 2004/01/27 01:31:45 dawes Exp $ */
-/* $XdotOrg: xserver/xorg/hw/xfree86/common/xf86Init.c,v 1.29 2005/12/14 20:11:16 ajax Exp $ */
+/* $XdotOrg: xserver/xorg/hw/xfree86/common/xf86Init.c,v 1.30 2006/02/13 04:43:40 benh Exp $ */
 
 /*
  * Loosely based on code bearing the following copyright:
@@ -1673,9 +1673,6 @@ ddxProcessArgument(int argc, char **argv, int i)
   if (!strcmp(argv[i], "-probe"))
   {
     xf86DoProbe = TRUE;
-#if 0
-    DoProbe(argc, argv, i);
-#endif
     return 1;
   }
   if (!strcmp(argv[i], "-configure"))
diff --git a/hw/xfree86/common/xf86Priv.h b/hw/xfree86/common/xf86Priv.h
index f32fa768d..4d578b8ff 100644
--- a/hw/xfree86/common/xf86Priv.h
+++ b/hw/xfree86/common/xf86Priv.h
@@ -171,7 +171,6 @@ extern DisplayModeRec xf86DefaultModes [];
 void DoScanPci(int argc, char **argv, int i);
 
 /* xf86DoProbe.c */
-void DoProbeArgs(int argc, char **argv, int i);
 void DoProbe(void);
 void DoConfigure(void);
 
diff --git a/hw/xfree86/common/xf86pciBus.c b/hw/xfree86/common/xf86pciBus.c
index ef75fdd4a..f0fb93e0d 100644
--- a/hw/xfree86/common/xf86pciBus.c
+++ b/hw/xfree86/common/xf86pciBus.c
@@ -208,11 +208,11 @@ FindPCIVideoInfo(void)
 	const int baseclass = pcrp->pci_base_class;
 	const int subclass = pcrp->pci_sub_class;
 	
-	if (PCIINFOCLASSES(baseclass, subclass) &&
-	    (DoIsolateDeviceCheck ?
-	    (xf86IsolateDevice.bus == pcrp->busnum &&
-	     xf86IsolateDevice.device == pcrp->devnum &&
-	     xf86IsolateDevice.func == pcrp->funcnum) : 1)) {
+	if ( PCIINFOCLASSES(baseclass, subclass) &&
+	     (!DoIsolateDeviceCheck ||
+	      (xf86IsolateDevice.bus == pcrp->busnum &&
+	       xf86IsolateDevice.device == pcrp->devnum &&
+	       xf86IsolateDevice.func == pcrp->funcnum)) ) {
 	    num++;
 	    xf86PciVideoInfo = xnfrealloc(xf86PciVideoInfo,
 					  sizeof(pciVideoPtr) * (num + 1));
@@ -3114,35 +3114,6 @@ xf86IsPrimaryPci(pciVideoPtr pPci)
 }
 
 /*
- * xf86CheckPciGAType() -- return type of PCI graphics adapter.
- */
-int
-xf86CheckPciGAType(pciVideoPtr pPci)
-{
-    int i = 0;
-    pciConfigPtr pcp;
-    
-    while ((pcp = xf86PciInfo[i]) != NULL) { 
-	if (pPci->bus == pcp->busnum && pPci->device == pcp->devnum
-	    && pPci->func == pcp->funcnum) {
-	    if (pcp->pci_base_class == PCI_CLASS_PREHISTORIC &&
-		pcp->pci_sub_class == PCI_SUBCLASS_PREHISTORIC_VGA)
-		return PCI_CHIP_VGA ;
-	    if (pcp->pci_base_class == PCI_CLASS_DISPLAY &&
-		pcp->pci_sub_class == PCI_SUBCLASS_DISPLAY_VGA) {
-		if (pcp->pci_prog_if == 0)
-		    return PCI_CHIP_VGA ; 
-		if (pcp->pci_prog_if == 1)
-		    return PCI_CHIP_8514;
-	    }
-	    return -1;
-	}
-    i++;
-    }
-    return -1;
-}
-
-/*
  * xf86GetPciInfoForEntity() -- Get the pciVideoRec of entity.
  */
 pciVideoPtr
diff --git a/hw/xfree86/common/xf86str.h b/hw/xfree86/common/xf86str.h
index a4f351661..590bf4cd0 100644
--- a/hw/xfree86/common/xf86str.h
+++ b/hw/xfree86/common/xf86str.h
@@ -732,8 +732,33 @@ typedef struct {
 } IsaChipsets;
 
 typedef struct {
+    /**
+     * Key used to match this device with its name in an array of
+     * \c SymTabRec.
+     */
     int numChipset;
+
+    /**
+     * This value is quirky.  Depending on the driver, it can take on one of
+     * three meanings.  In drivers that have exactly one vendor ID (e.g.,
+     * radeon, mga, i810) the low 16-bits are the device ID.
+     *
+     * In drivers that can have multiple vendor IDs (e.g., the glint driver
+     * can have either 3dlabs' ID or TI's ID, the i740 driver can have either
+     * Intel's ID or Real3D's ID, etc.) the low 16-bits are the device ID and
+     * the high 16-bits are the vendor ID.
+     *
+     * In drivers that don't have a specific vendor (e.g., vga) contains the
+     * device ID for either the generic VGA or generic 8514 devices.  This
+     * turns out to be the same as the subclass and programming interface
+     * value (e.g., the full 24-bit class for the VGA device is 0x030000 (or 
+     * 0x000101) and for 8514 is 0x030001).
+     */
     int PCIid;
+
+    /**
+     * Resources associated with this type of device.
+     */
     resRange *resList;
 } PciChipsets;