Nearly 1 year ping. On 07/28/2015 10:16 AM, Aaron Plattner wrote: > 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 <[email protected]> > > Reviewed-by: Aaron Plattner <[email protected]> > > 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); >> > >
_______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
