From 2e6f801fe1a749f6a4db2cfd8a43abec5caceae0 Mon Sep 17 00:00:00 2001 From: Ian Romanick Date: Tue, 7 Mar 2006 23:58:22 +0000 Subject: [PATCH] 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. --- ChangeLog | 37 ++++++ hw/xfree86/common/xf86.h | 1 - hw/xfree86/common/xf86DoProbe.c | 32 ++--- hw/xfree86/common/xf86Helper.c | 221 +++++++++++++++++++------------- hw/xfree86/common/xf86Init.c | 5 +- hw/xfree86/common/xf86Priv.h | 1 - hw/xfree86/common/xf86pciBus.c | 39 +----- hw/xfree86/common/xf86str.h | 25 ++++ hw/xfree86/loader/xf86sym.c | 1 - 9 files changed, 210 insertions(+), 152 deletions(-) diff --git a/ChangeLog b/ChangeLog index 1bd9272b0..873c7d3be 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,40 @@ +2006-03-07 Ian Romanick + + * hw/xfree86/common/xf86DoProbe.c: (DoProbe): + * hw/xfree86/common/xf86Priv.h: + * hw/xfree86/common/xf86Init.c: (ddxProcessArgument): + Remove DoProbeArgs. It was an empty function that was never called. + + Some refactoring in DoProbe to make the code more readable (and make + the future transition to xf86CallDriverProbe easier). + + * hw/xfree86/common/xf86Helper.c: (xf86MatchPciInstances): + Major refactoring of xf86MatchPciInstances. Primarilly, all device + matching is performed by a single, smart loop. Matching of + PCI_VENDOR_GENERIC devices is done by device class, and this + eliminates the need to call xf86CheckPciGAType (good riddance!). + + Various other changes eliminate the need to use xnfrealloc on the + instances array. When in probe-only mode or the first pass of + configure mode, the instances array isn't needed, so it is not + allocated. + + These changes will make the change to a PCI device matching scheme + more like is implemented in libpci.a (and has been discussed on the + xorg mailing list) much, much simpler. + + * hw/xfree86/common/xf86.h: + * hw/xfree86/loader/xf86sym.c: + * hw/xfree86/common/xf86pciBus.c: (FindPCIVideoInfo): + Eliminate unnecessary use of ?: operator within an if-statement. + + Remove xf86CheckPciGAType. It is no longer called by + xf86MatchPciInstances, which was previously the only place that + called it. + + * hw/xfree86/common/xf86str.h: + Add numerous comments to the fields of PciChipsets. + 2006-03-07 Eric Anholt * hw/kdrive/ephyr/ephyr_draw.c: 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 @@ -48,11 +48,6 @@ #include "xf86.h" #include "xf86Priv.h" -void -DoProbeArgs(int argc, char **argv, int i) -{ -} - void DoProbe() { @@ -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++; } - } else if (vendorID == PCI_VENDOR_GENERIC) { - for (ppPci = xf86PciVideoInfo; *ppPci != NULL; ppPci++) { - for (id = PCIchipsets; id->PCIid != -1; id++) { - if (id->PCIid == xf86CheckPciGAType(*ppPci)) { + + 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; + } + + + /* 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; - numFound++; - break; + 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; } } } } + + /* 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)); @@ -3113,35 +3113,6 @@ xf86IsPrimaryPci(pciVideoPtr pPci) pPci->func == primaryBus.id.pci.func); } -/* - * 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. */ 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; diff --git a/hw/xfree86/loader/xf86sym.c b/hw/xfree86/loader/xf86sym.c index 2b405215a..d33cc8da7 100644 --- a/hw/xfree86/loader/xf86sym.c +++ b/hw/xfree86/loader/xf86sym.c @@ -323,7 +323,6 @@ LOOKUP xfree86LookupTab[] = { SYMFUNC(xf86SetCurrentAccess) SYMFUNC(xf86IsPrimaryPci) SYMFUNC(xf86IsPrimaryIsa) - SYMFUNC(xf86CheckPciGAType) SYMFUNC(xf86PrintResList) SYMFUNC(xf86AddResToList) SYMFUNC(xf86JoinResLists)