On 07/22/2015 04:42 PM, Karol Kosik wrote: > Implementation of new drivers matching algorithm. New approach > doesn't add duplicate drivers and ease drivers matching phase. > > Signed-off-by: Karol Kosik <kko...@nvidia.com>
Reviewed-by: Aaron Plattner <aplatt...@nvidia.com> though it might be good for someone else to take a look too. In case anyone was wondering why this patch helps, the problem is that xf86PlatformMatchDriver() loops over every platform device, which means that if you have a whole bunch of GPUs, it keeps adding the same drivers over and over again, overflowing the 20-element deviceList[]. > --- > hw/xfree86/common/xf86AutoConfig.c | 124 > +++++++++++++++++++---------------- > hw/xfree86/common/xf86MatchDrivers.h | 40 +++++++++++ > hw/xfree86/common/xf86pciBus.c | 52 ++++++--------- > hw/xfree86/common/xf86pciBus.h | 13 ++-- > hw/xfree86/common/xf86platformBus.c | 31 +++------ > hw/xfree86/common/xf86platformBus.h | 5 +- > 6 files changed, 150 insertions(+), 115 deletions(-) > create mode 100644 hw/xfree86/common/xf86MatchDrivers.h > > diff --git a/hw/xfree86/common/xf86AutoConfig.c > b/hw/xfree86/common/xf86AutoConfig.c > index 6b8d0eb..440434c 100644 > --- a/hw/xfree86/common/xf86AutoConfig.c > +++ b/hw/xfree86/common/xf86AutoConfig.c > @@ -37,6 +37,7 @@ > #include "xf86Parser.h" > #include "xf86tokens.h" > #include "xf86Config.h" > +#include "xf86MatchDrivers.h" > #include "xf86Priv.h" > #include "xf86_OSlib.h" > #include "xf86platformBus.h" > @@ -89,7 +90,7 @@ > static const char **builtinConfig = NULL; > static int builtinLines = 0; > > -static void listPossibleVideoDrivers(char *matches[], int nmatches); > +static void listPossibleVideoDrivers(XF86MatchedDrivers *md); > > /* > * A built-in config file is stored as an array of strings, with each string > @@ -140,33 +141,58 @@ AppendToConfig(const char *s) > AppendToList(s, &builtinConfig, &builtinLines); > } > > +void > +xf86AddMatchedDriver(XF86MatchedDrivers *md, const char *driver) > +{ > + int j; > + int nmatches = md->nmatches; > + > + for (j = 0; j < nmatches; ++j) { > + if (xf86NameCmp(md->matches[j], driver) == 0) { > + // Driver already in matched drivers > + return; > + } > + } > + > + if (nmatches < MATCH_DRIVERS_LIMIT) { > + md->matches[nmatches] = xnfstrdup(driver); > + md->nmatches++; > + } > + else { > + xf86Msg(X_WARNING, "Too many drivers registered, can't add %s\n", > driver); > + } > +} > + > Bool > xf86AutoConfig(void) > { > - char *deviceList[20]; > - char **p; > + XF86MatchedDrivers md; > + int i; > const char **cp; > char buf[1024]; > ConfigStatus ret; > > - listPossibleVideoDrivers(deviceList, 20); > + listPossibleVideoDrivers(&md); > > - for (p = deviceList; *p; p++) { > - snprintf(buf, sizeof(buf), BUILTIN_DEVICE_SECTION, *p, 0, *p); > + for (i = 0; i < md.nmatches; i++) { > + snprintf(buf, sizeof(buf), BUILTIN_DEVICE_SECTION, > + md.matches[i], 0, md.matches[i]); > AppendToConfig(buf); > - snprintf(buf, sizeof(buf), BUILTIN_SCREEN_SECTION, *p, 0, *p, 0); > + snprintf(buf, sizeof(buf), BUILTIN_SCREEN_SECTION, > + md.matches[i], 0, md.matches[i], 0); > AppendToConfig(buf); > } > > AppendToConfig(BUILTIN_LAYOUT_SECTION_PRE); > - for (p = deviceList; *p; p++) { > - snprintf(buf, sizeof(buf), BUILTIN_LAYOUT_SCREEN_LINE, *p, 0); > + for (i = 0; i < md.nmatches; i++) { > + snprintf(buf, sizeof(buf), BUILTIN_LAYOUT_SCREEN_LINE, > + md.matches[i], 0); > AppendToConfig(buf); > } > AppendToConfig(BUILTIN_LAYOUT_SECTION_POST); > > - for (p = deviceList; *p; p++) { > - free(*p); > + for (i = 0; i < md.nmatches; i++) { > + free(md.matches[i]); > } > > xf86MsgVerb(X_DEFAULT, 0, > @@ -190,22 +216,19 @@ xf86AutoConfig(void) > } > > static void > -listPossibleVideoDrivers(char *matches[], int nmatches) > +listPossibleVideoDrivers(XF86MatchedDrivers *md) > { > int i; > > - for (i = 0; i < nmatches; i++) { > - matches[i] = NULL; > - } > - i = 0; > + md->nmatches = 0; > > #ifdef XSERVER_PLATFORM_BUS > - i = xf86PlatformMatchDriver(matches, nmatches); > + xf86PlatformMatchDriver(md); > #endif > #ifdef sun > /* Check for driver type based on /dev/fb type and if valid, use > it instead of PCI bus probe results */ > - if (xf86Info.consoleFd >= 0 && (i < (nmatches - 1))) { > + if (xf86Info.consoleFd >= 0) { > struct vis_identifier visid; > const char *cp; > int iret; > @@ -231,7 +254,7 @@ listPossibleVideoDrivers(char *matches[], int nmatches) > > /* Special case from before the general case was set */ > if (strcmp(visid.name, "NVDAnvda") == 0) { > - matches[i++] = xnfstrdup("nvidia"); > + xf86AddMatchedDriver(md, "nvidia"); > } > > /* General case - split into vendor name (initial all-caps > @@ -241,55 +264,48 @@ listPossibleVideoDrivers(char *matches[], int nmatches) > /* find end of all uppercase vendor section */ > } > if ((cp != visid.name) && (*cp != '\0')) { > - char *driverName = xnfstrdup(cp); > char *vendorName = xnfstrdup(visid.name); > > vendorName[cp - visid.name] = '\0'; > > - matches[i++] = vendorName; > - matches[i++] = driverName; > + xf86AddMatchedDriver(md, vendorName); > + xf86AddMatchedDriver(md, cp); > + > + free(vendorName); > } > } > } > } > #endif > #ifdef __sparc__ > - if (i < (nmatches - 1)) > - { > - char *sbusDriver = sparcDriverName(); > + char *sbusDriver = sparcDriverName(); > > - if (sbusDriver) > - matches[i++] = xnfstrdup(sbusDriver); > - } > + if (sbusDriver) > + xf86AddMatchedDriver(md, sbusDriver); > #endif > #ifdef XSERVER_LIBPCIACCESS > - if (i < (nmatches - 1)) > - i += xf86PciMatchDriver(&matches[i], nmatches - i); > + xf86PciMatchDriver(md); > #endif > > #if defined(__linux__) > - matches[i++] = xnfstrdup("modesetting"); > + xf86AddMatchedDriver(md, "modesetting"); > #endif > > #if !defined(sun) > /* Fallback to platform default frame buffer driver */ > - if (i < (nmatches - 1)) { > #if !defined(__linux__) && defined(__sparc__) > - matches[i++] = xnfstrdup("wsfb"); > + xf86AddMatchedDriver(md, "wsfb"); > #else > - matches[i++] = xnfstrdup("fbdev"); > + xf86AddMatchedDriver(md, "fbdev"); > #endif > - } > #endif /* !sun */ > > /* Fallback to platform default hardware */ > - if (i < (nmatches - 1)) { > #if defined(__i386__) || defined(__amd64__) || defined(__hurd__) > - matches[i++] = xnfstrdup("vesa"); > + xf86AddMatchedDriver(md, "vesa"); > #elif defined(__sparc__) && !defined(sun) > - matches[i++] = xnfstrdup("sunffb"); > + xf86AddMatchedDriver(md, "sunffb"); > #endif > - } > } > > /* copy a screen section and enter the desired driver > @@ -335,8 +351,8 @@ GDevPtr > autoConfigDevice(GDevPtr preconf_device) > { > GDevPtr ptr = NULL; > - char *matches[20]; /* If we have more than 20 drivers we're in > trouble */ > - int num_matches = 0, num_screens = 0, i; > + XF86MatchedDrivers md; > + int num_screens = 0, i; > screenLayoutPtr slp; > > if (!xf86configptr) { > @@ -363,10 +379,10 @@ autoConfigDevice(GDevPtr preconf_device) > } > if (!ptr->driver) { > /* get all possible video drivers and count them */ > - listPossibleVideoDrivers(matches, 20); > - for (; matches[num_matches]; num_matches++) { > + listPossibleVideoDrivers(&md); > + for (i = 0; i < md.nmatches; i++) { > xf86Msg(X_DEFAULT, "Matched %s as autoconfigured driver %d\n", > - matches[num_matches], num_matches); > + md.matches[i], i); > } > > slp = xf86ConfigLayout.screens; > @@ -376,12 +392,12 @@ autoConfigDevice(GDevPtr preconf_device) > * minus one for the already existing first one > * plus one for the terminating NULL */ > for (; slp[num_screens].screen; num_screens++); > - xf86ConfigLayout.screens = xnfcalloc(num_screens + num_matches, > + xf86ConfigLayout.screens = xnfcalloc(num_screens + md.nmatches, > sizeof(screenLayoutRec)); > xf86ConfigLayout.screens[0] = slp[0]; > > /* do the first match and set that for the original first screen > */ > - ptr->driver = matches[0]; > + ptr->driver = md.matches[0]; > if (!xf86ConfigLayout.screens[0].screen->device) { > xf86ConfigLayout.screens[0].screen->device = ptr; > ptr->myScreenSection = xf86ConfigLayout.screens[0].screen; > @@ -390,8 +406,8 @@ autoConfigDevice(GDevPtr preconf_device) > /* for each other driver found, copy the first screen, insert it > * into the list of screens and set the driver */ > i = 0; > - while (i++ < num_matches) { > - if (!copyScreen(slp[0].screen, ptr, i, matches[i])) > + while (i++ < md.nmatches) { > + if (!copyScreen(slp[0].screen, ptr, i, md.matches[i])) > return NULL; > } > > @@ -400,19 +416,17 @@ autoConfigDevice(GDevPtr preconf_device) > * > * TODO Handle rest of multiple screen sections */ > for (i = 1; i < num_screens; i++) { > - xf86ConfigLayout.screens[i + num_matches] = slp[i]; > + xf86ConfigLayout.screens[i + md.nmatches] = slp[i]; > } > - xf86ConfigLayout.screens[num_screens + num_matches - 1].screen = > + xf86ConfigLayout.screens[num_screens + md.nmatches - 1].screen = > NULL; > free(slp); > } > else { > /* layout does not have any screens, not much to do */ > - ptr->driver = matches[0]; > - for (i = 1; matches[i]; i++) { > - if (matches[i] != matches[0]) { > - free(matches[i]); > - } > + ptr->driver = md.matches[0]; > + for (i = 1; i < md.nmatches; i++) { > + free(md.matches[i]); > } > } > } > diff --git a/hw/xfree86/common/xf86MatchDrivers.h > b/hw/xfree86/common/xf86MatchDrivers.h > new file mode 100644 > index 0000000..4663af4 > --- /dev/null > +++ b/hw/xfree86/common/xf86MatchDrivers.h > @@ -0,0 +1,40 @@ > +/* > + * Copyright © 2015 NVIDIA Corporation > + * > + * Permission is hereby granted, free of charge, to any person obtaining a > + * copy of this software and associated documentation files (the "Software"), > + * to deal in the Software without restriction, including without limitation > + * the rights to use, copy, modify, merge, publish, distribute, sublicense, > + * and/or sell copies of the Software, and to permit persons to whom the > + * Software is furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice (including the next > + * paragraph) shall be included in all copies or substantial portions of the > + * Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER > + * DEALINGS IN THE SOFTWARE. > + */ > + > +#ifndef _xf86_match_drivers_h > +#define _xf86_match_drivers_h > + > +#define MATCH_DRIVERS_LIMIT 20 > + > +typedef struct _XF86MatchedDrivers { > + char *matches[MATCH_DRIVERS_LIMIT]; > + int nmatches; > +} XF86MatchedDrivers; > + > +/* > + * prototypes > + */ > +void xf86AddMatchedDriver(XF86MatchedDrivers *, const char *); > + > +#endif /* _xf86_match_drivers_h */ > + > diff --git a/hw/xfree86/common/xf86pciBus.c b/hw/xfree86/common/xf86pciBus.c > index 8158c2b..e81dc69 100644 > --- a/hw/xfree86/common/xf86pciBus.c > +++ b/hw/xfree86/common/xf86pciBus.c > @@ -1061,9 +1061,8 @@ xf86ConfigPciEntity(ScrnInfoPtr pScrn, int scrnFlag, > int entityIndex, > return pScrn; > } > > -int > -xf86VideoPtrToDriverList(struct pci_device *dev, > - char *returnList[], int returnListMax) > +void > +xf86VideoPtrToDriverList(struct pci_device *dev, XF86MatchedDrivers *md) > { > int i; > > @@ -1266,10 +1265,9 @@ xf86VideoPtrToDriverList(struct pci_device *dev, > default: > break; > } > - for (i = 0; (i < returnListMax) && (driverList[i] != NULL); i++) { > - returnList[i] = xnfstrdup(driverList[i]); > + for (i = 0; driverList[i] != NULL; i++) { > + xf86AddMatchedDriver(md, driverList[i]); > } > - return i; /* Number of entries added */ > } > > #ifdef __linux__ > @@ -1293,23 +1291,23 @@ xchomp(char *line) > * don't export their PCI ID's properly. If distros don't end up using this > * feature it can and should be removed because the symbol-based resolution > * scheme should be the primary one */ > -int > +void > xf86MatchDriverFromFiles(uint16_t match_vendor, uint16_t match_chip, > - char *matches[], int nmatches) > + XF86MatchedDrivers *md) > { > DIR *idsdir; > FILE *fp; > struct dirent *direntry; > - char *line = NULL; > + char *line = NULL, *tmpMatch; > size_t len; > ssize_t read; > char path_name[256], vendor_str[5], chip_str[5]; > uint16_t vendor, chip; > - int i = 0, j; > + int j; > > idsdir = opendir(PCI_TXT_IDS_PATH); > if (!idsdir) > - return 0; > + return; > > xf86Msg(X_INFO, > "Scanning %s directory for additional PCI ID's supported by the > drivers\n", > @@ -1360,10 +1358,10 @@ xf86MatchDriverFromFiles(uint16_t match_vendor, > uint16_t match_chip, > } > } > if (vendor == match_vendor && chip == match_chip) { > - matches[i] = > + tmpMatch = > (char *) malloc(sizeof(char) * > strlen(direntry->d_name) - 3); > - if (!matches[i]) { > + if (!tmpMatch) { > xf86Msg(X_ERROR, > "Could not allocate space for the module > name. Exiting.\n"); > goto end; > @@ -1373,16 +1371,17 @@ xf86MatchDriverFromFiles(uint16_t match_vendor, > uint16_t match_chip, > * taking off anything after the first '.' */ > for (j = 0; j < (strlen(direntry->d_name) - 3); j++) > { > if (direntry->d_name[j] == '.') { > - matches[i][j] = '\0'; > + tmpMatch[j] = '\0'; > break; > } > else { > - matches[i][j] = direntry->d_name[j]; > + tmpMatch[j] = direntry->d_name[j]; > } > } > + xf86AddMatchedDriver(md, tmpMatch); > xf86Msg(X_INFO, "Matched %s from file name %s\n", > - matches[i], direntry->d_name); > - i++; > + tmpMatch, direntry->d_name); > + free(tmpMatch); > } > } > else { > @@ -1396,18 +1395,12 @@ xf86MatchDriverFromFiles(uint16_t match_vendor, > uint16_t match_chip, > end: > free(line); > closedir(idsdir); > - return i; > } > #endif /* __linux__ */ > > -/** > - * @return The numbers of found devices that match with the current system > - * drivers. > - */ > -int > -xf86PciMatchDriver(char *matches[], int nmatches) > +void > +xf86PciMatchDriver(XF86MatchedDrivers *md) > { > - int i = 0; > struct pci_device *info = NULL; > struct pci_device_iterator *iter; > > @@ -1422,15 +1415,12 @@ xf86PciMatchDriver(char *matches[], int nmatches) > pci_iterator_destroy(iter); > #ifdef __linux__ > if (info) > - i += xf86MatchDriverFromFiles(info->vendor_id, info->device_id, > - matches, nmatches); > + xf86MatchDriverFromFiles(info->vendor_id, info->device_id, md); > #endif > > - if ((info != NULL) && (i < nmatches)) { > - i += xf86VideoPtrToDriverList(info, &(matches[i]), nmatches - i); > + if (info != NULL) { > + xf86VideoPtrToDriverList(info, md); > } > - > - return i; > } > > Bool > diff --git a/hw/xfree86/common/xf86pciBus.h b/hw/xfree86/common/xf86pciBus.h > index 45b5a0f..14ae976 100644 > --- a/hw/xfree86/common/xf86pciBus.h > +++ b/hw/xfree86/common/xf86pciBus.h > @@ -33,11 +33,13 @@ > #ifndef _XF86_PCI_BUS_H > #define _XF86_PCI_BUS_H > > +#include "xf86MatchDrivers.h" > + > void xf86PciProbe(void); > Bool xf86PciAddMatchingDev(DriverPtr drvp); > Bool xf86PciProbeDev(DriverPtr drvp); > void xf86PciIsolateDevice(const char *argument); > -int xf86PciMatchDriver(char *matches[], int nmatches); > +void xf86PciMatchDriver(XF86MatchedDrivers *md); > Bool xf86PciConfigure(void *busData, struct pci_device *pDev); > void xf86PciConfigureNewDev(void *busData, struct pci_device *pVideo, > GDevRec * GDev, int *chipset); > @@ -47,10 +49,9 @@ void xf86PciConfigureNewDev(void *busData, struct > pci_device *pVideo, > ((x)->func == (y)->func) && \ > ((x)->dev == (y)->dev)) > > -int > +void > xf86MatchDriverFromFiles(uint16_t match_vendor, uint16_t match_chip, > - char *matches[], int nmatches); > -int > -xf86VideoPtrToDriverList(struct pci_device *dev, > - char *returnList[], int returnListMax); > + XF86MatchedDrivers *md); > +void > +xf86VideoPtrToDriverList(struct pci_device *dev, XF86MatchedDrivers *md); > #endif /* _XF86_PCI_BUS_H */ > diff --git a/hw/xfree86/common/xf86platformBus.c > b/hw/xfree86/common/xf86platformBus.c > index f1e9423..c414ff0 100644 > --- a/hw/xfree86/common/xf86platformBus.c > +++ b/hw/xfree86/common/xf86platformBus.c > @@ -212,14 +212,10 @@ OutputClassMatches(const XF86ConfOutputClassPtr oclass, > int index) > return TRUE; > } > > -static int > -xf86OutputClassDriverList(int index, char *matches[], int nmatches) > +static void > +xf86OutputClassDriverList(int index, XF86MatchedDrivers *md) > { > XF86ConfOutputClassPtr cl; > - int i = 0; > - > - if (nmatches == 0) > - return 0; > > for (cl = xf86configptr->conf_outputclass_lst; cl; cl = cl->list.next) { > if (OutputClassMatches(cl, index)) { > @@ -229,24 +225,19 @@ xf86OutputClassDriverList(int index, char *matches[], > int nmatches) > cl->identifier, path); > xf86Msg(X_NONE, "\tloading driver: %s\n", cl->driver); > > - matches[i++] = xstrdup(cl->driver); > + xf86AddMatchedDriver(md, cl->driver); > } > - > - if (i >= nmatches) > - break; > } > - > - return i; > } > > /** > * @return The numbers of found devices that match with the current system > * drivers. > */ > -int > -xf86PlatformMatchDriver(char *matches[], int nmatches) > +void > +xf86PlatformMatchDriver(XF86MatchedDrivers *md) > { > - int i, j = 0; > + int i; > struct pci_device *info = NULL; > int pass = 0; > > @@ -258,21 +249,19 @@ xf86PlatformMatchDriver(char *matches[], int nmatches) > else if (!xf86IsPrimaryPlatform(&xf86_platform_devices[i]) && > (pass == 0)) > continue; > > - j += xf86OutputClassDriverList(i, &matches[j], nmatches - j); > + xf86OutputClassDriverList(i, md); > > info = xf86_platform_devices[i].pdev; > #ifdef __linux__ > if (info) > - j += xf86MatchDriverFromFiles(info->vendor_id, > info->device_id, > - &matches[j], nmatches - j); > + xf86MatchDriverFromFiles(info->vendor_id, info->device_id, > md); > #endif > > - if ((info != NULL) && (j < nmatches)) { > - j += xf86VideoPtrToDriverList(info, &(matches[j]), nmatches > - j); > + if (info != NULL) { > + xf86VideoPtrToDriverList(info, md); > } > } > } > - return j; > } > > int > diff --git a/hw/xfree86/common/xf86platformBus.h > b/hw/xfree86/common/xf86platformBus.h > index a7335b9..0f0184f 100644 > --- a/hw/xfree86/common/xf86platformBus.h > +++ b/hw/xfree86/common/xf86platformBus.h > @@ -25,6 +25,7 @@ > #define XF86_PLATFORM_BUS_H > > #include "hotplug.h" > +#include "xf86MatchDrivers.h" > > struct xf86_platform_device { > struct OdevAttributes *attribs; > @@ -151,8 +152,8 @@ _xf86_get_platform_device_int_attrib(struct > xf86_platform_device *device, int at > extern _X_EXPORT Bool > xf86PlatformDeviceCheckBusID(struct xf86_platform_device *device, const char > *busid); > > -extern _X_EXPORT int > -xf86PlatformMatchDriver(char *matches[], int nmatches); > +extern _X_EXPORT void > +xf86PlatformMatchDriver(XF86MatchedDrivers *); > > extern void xf86platformVTProbe(void); > extern void xf86platformPrimary(void); > -- Aaron _______________________________________________ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel