[PATCH] Implement PCI ID-based Driver Autoloading
Aaron Plattner
aplattner at nvidia.com
Sun Jan 27 09:24:01 PST 2008
Shouldn't this also take the other fields in struct pci_id_match into
account, like the sub-IDs and the device class?
libpciaccess already has a "match anything" macro, PCI_MATCH_ANY, and a
macro for comparing IDs, PCI_ID_COMPARE. It might be better to use those.
Also, struct pci_id_match has uint32_t fields, not uint16_t.
-- Aaron
On Sat, Jan 26, 2008 at 05:09:22PM -0500, David Nusinow wrote:
> Hello all,
>
> The attached patches implement PCI ID-based driver autoloading. The
> drivers have to be pci-reworked. They also remove the previous autoloading
> schemes, including my half-baked text file-based version and the vendor id
> based heuristic.
>
> If a driver wants to support everything from a particular vendor, they
> claim the vendor ID normally and set the device id as UINT16_MAX.
>
> A missing feature is if multiple chips claim the same ID. Currently it
> will just load the first such driver found, but ideally there should be a
> way for drivers to specify a priority or override somehow in the future.
>
> Comments and criticisms are greatly appreciated. If no one speaks up
> I'll apply these in a week to master.
>
> - David Nusinow
> From 8be59a547a5b14a9e5830155693a471f486d6ca7 Mon Sep 17 00:00:00 2001
> From: David Nusinow <dnusinow at debian.org>
> Date: Sat, 26 Jan 2008 15:11:47 -0500
> Subject: [PATCH] Load drivers based on PCI IDs
>
> This will only take place when no driver is specified in xorg.conf.
> Drivers will be scanned for PCI ID's they export in their driverrec's.
> This requires them to be pci-reworked.
>
> If no driver is found, an architecture-specific fallback is used.
>
> In the case of multiple drivers supporting the same ID, currently only the
> first driver found will be used.
> ---
> hw/xfree86/common/xf86AutoConfig.c | 109 +++++++++++++++++++++++++++++++++++-
> 1 files changed, 107 insertions(+), 2 deletions(-)
>
> diff --git a/hw/xfree86/common/xf86AutoConfig.c b/hw/xfree86/common/xf86AutoConfig.c
> index c6e1972..f3a5666 100644
> --- a/hw/xfree86/common/xf86AutoConfig.c
> +++ b/hw/xfree86/common/xf86AutoConfig.c
> @@ -33,6 +33,7 @@
> #include <xorg-config.h>
> #endif
>
> +#include <dlfcn.h>
> #include "xf86.h"
> #include "xf86Parser.h"
> #include "xf86tokens.h"
> @@ -411,6 +412,107 @@ matchDriverFromFiles (char** matches, uint16_t match_vendor, uint16_t match_chip
> }
> #endif /* __linux__ */
>
> +static char*
> +driverClaimsID(struct dirent *direntry, uint16_t match_vendor, uint16_t match_chip, char *driverpath)
> +{
> + char *retval = NULL;
> + char *modulepath = NULL;
> + char *drivername = NULL;
> + int len;
> + int buflen = 256;
> + int tmp, i;
> + void *handle;
> + const struct pci_id_match *driver_pci;
> + DriverPtr drec;
> +
> + if (!direntry)
> + goto end;
> + len = strlen(direntry->d_name);
> + if ((direntry->d_name[0] == '.') ||
> + (strcmp(&(direntry->d_name[len-3]), ".so") != 0 ) ) {
> + goto end;
> + }
> +
> + drivername = (char*)xalloc(buflen);
> + modulepath = (char*)xalloc(strlen(driverpath) + buflen);
> +
> + strcpy(modulepath, driverpath);
> + strncat(modulepath, direntry->d_name, buflen);
> + handle = dlopen(modulepath, RTLD_LAZY);
> + if (!handle) {
> + xf86Msg(X_ERROR, "Could not dlopen the module %s\n", direntry->d_name);
> + goto end;
> + }
> +
> + /* Figure out driver name to get the right symbol */
> + strncpy(drivername, direntry->d_name, buflen);
> + tmp = strlen(drivername);
> + for (i = 0 ; i < tmp ; i++) {
> + if (isalnum(drivername[i]))
> + drivername[i] = toupper(drivername[i]);
> + else
> + drivername[i] = '\0';
> + }
> + /* Get the DriverRec and look for a PCI ID match */
> + drec = NULL;
> + drec = (DriverPtr)dlsym(handle, drivername);
> + if (!drec) {
> + goto end;
> + }
> +
> + driver_pci = drec->supported_devices;
> + while (driver_pci->vendor_id != 0) {
> + if (driver_pci->vendor_id == match_vendor &&
> + driver_pci->device_id == match_chip) {
> +
> + xf86Msg(X_DEFAULT, "Driver %s supports the primary video device.\n", drec->driverName);
> + retval = (char*)xalloc(strlen(drec->driverName));
> + strcpy(retval, drec->driverName);
> + }
> + driver_pci++;
> + }
> + dlclose(handle);
> +
> + end:
> + xfree(modulepath);
> + xfree(drivername);
> + return retval;
> +}
> +
> +static void
> +matchDriverFromSymbols (char** matches, int matchmax, uint16_t match_vendor, uint16_t match_chip)
> +{
> + char *driverpath, *matcheddriver;
> + DIR *dir;
> + struct dirent *direntry;
> + int i;
> + char *p;
> +
> + xf86Msg(X_INFO, "Searching for drivers that support the primary pci video\n");
> + driverpath = (char*)xalloc(strlen(xf86ModulePath) + strlen("/drivers/") + 1);
> + strcpy(driverpath, xf86ModulePath);
> + strcat(driverpath, "/drivers/");
> + dir = opendir(driverpath);
> + if (dir) {
> + direntry = readdir(dir);
> + while (direntry) {
> + matcheddriver = driverClaimsID(direntry, match_vendor, match_chip, driverpath);
> + if (matcheddriver) {
> + for (i = 0 ; i < matchmax ; i++) {
> + p = matches[i];
> + if (p == NULL)
> + break;
> + }
> + p = matcheddriver;
> + }
> + direntry = readdir(dir);
> + }
> + } else {
> + xf86Msg(X_ERROR, "Could not open driver dir %s for reading.\n", driverpath);
> + }
> + closedir(dir);
> + xfree(driverpath);
> +}
> +
> char*
> chooseVideoDriver(void)
> {
> @@ -418,9 +520,10 @@ chooseVideoDriver(void)
> struct pci_device_iterator *iter;
> char *chosen_driver = NULL;
> int i;
> - char *matches[20]; /* If we have more than 20 drivers we're in trouble */
> + int matchmax = 20; /* If we have more than 20 drivers we're in trouble */
> + char *matches[matchmax];
>
> - for (i=0 ; i<20 ; i++)
> + for (i=0 ; i < matchmax ; i++)
> matches[i] = NULL;
>
> /* Find the primary device, and get some information about it. */
> @@ -437,6 +540,7 @@ chooseVideoDriver(void)
> ErrorF("Primary device is not PCI\n");
> }
>
> + matchDriverFromSymbols(matches, matchmax, info->vendor_id, info->device_id);
> #ifdef __linux__
> matchDriverFromFiles(matches, info->vendor_id, info->device_id);
> #endif /* __linux__ */
> @@ -448,6 +552,7 @@ chooseVideoDriver(void)
> if (info != NULL)
> chosen_driver = videoPtrToDriverName(info);
> if (chosen_driver == NULL) {
> + xf86Msg(X_DEFAULT, "Could not find a driver that explicitly supports the primary video card. Falling back to the default driver.\n");
> #if defined __i386__ || defined __amd64__ || defined __hurd__
> chosen_driver = "vesa";
> #elif defined __alpha__
> --
> 1.5.3.8
>
> From a21282a2adfc839eefc980d01a94280d0c249d06 Mon Sep 17 00:00:00 2001
> From: David Nusinow <dnusinow at debian.org>
> Date: Sat, 26 Jan 2008 15:17:46 -0500
> Subject: [PATCH] Allow drivers to claim support for all of a manufacturer's cards
>
> If the driver supports the vendor ID and claims to support a device ID of
> UNIT16_MAX then this is equivalent to claiming all cards from this vendor.
> ---
> hw/xfree86/common/xf86AutoConfig.c | 6 ++++--
> 1 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/hw/xfree86/common/xf86AutoConfig.c b/hw/xfree86/common/xf86AutoConfig.c
> index f3a5666..d77bdae 100644
> --- a/hw/xfree86/common/xf86AutoConfig.c
> +++ b/hw/xfree86/common/xf86AutoConfig.c
> @@ -462,11 +462,13 @@ driverClaimsID(struct dirent *direntry, uint16_t match_vendor, uint16_t match_ch
>
> driver_pci = drec->supported_devices;
> while (driver_pci->vendor_id != 0) {
> - if (driver_pci->vendor_id == match_vendor &&
> - driver_pci->device_id == match_chip) {
> + if (driver_pci->vendor_id == match_vendor) {
> + if ((driver_pci->device_id == match_chip) ||
> + (driver_pci->device_id == UINT16_MAX)) {
>
> xf86Msg(X_DEFAULT, "Driver %s supports the primary video device.\n", drec->driverName);
> retval = (char*)xalloc(strlen(drec->driverName));
> strcpy(retval, drec->driverName);
> + }
> }
> driver_pci++;
> }
> --
> 1.5.3.8
>
> From 8cc3685a81de02a93664408dd97cd9249c9d495c Mon Sep 17 00:00:00 2001
> From: David Nusinow <dnusinow at debian.org>
> Date: Sat, 26 Jan 2008 15:22:18 -0500
> Subject: [PATCH] Remove the old heuristic for choosing a driver based on vendor ID
>
> With the previous change this logic moves to the drivers where it belongs
> ---
> hw/xfree86/common/xf86AutoConfig.c | 57 ------------------------------------
> 1 files changed, 0 insertions(+), 57 deletions(-)
>
> diff --git a/hw/xfree86/common/xf86AutoConfig.c b/hw/xfree86/common/xf86AutoConfig.c
> index d77bdae..e1f9a9a 100644
> --- a/hw/xfree86/common/xf86AutoConfig.c
> +++ b/hw/xfree86/common/xf86AutoConfig.c
> @@ -137,61 +137,6 @@ AppendToConfig(const char *s)
> AppendToList(s, &builtinConfig, &builtinLines);
> }
>
> -static const char *
> -videoPtrToDriverName(struct pci_device *dev)
> -{
> - /*
> - * things not handled yet:
> - * amd/cyrix/nsc
> - * xgi
> - */
> -
> - switch (dev->vendor_id)
> - {
> - case 0x1142: return "apm";
> - case 0xedd8: return "ark";
> - case 0x1a03: return "ast";
> - case 0x1002: return "ati";
> - case 0x102c: return "chips";
> - case 0x1013: return "cirrus";
> - case 0x8086:
> - if ((dev->device_id == 0x00d1) || (dev->device_id == 0x7800))
> - return "i740";
> - else return "intel";
> - case 0x102b: return "mga";
> - case 0x10c8: return "neomagic";
> - case 0x105d: return "i128";
> - case 0x10de: case 0x12d2: return "nv";
> - case 0x1163: return "rendition";
> - case 0x5333:
> - switch (dev->device_id)
> - {
> - case 0x88d0: case 0x88d1: case 0x88f0: case 0x8811:
> - case 0x8812: case 0x8814: case 0x8901:
> - return "s3";
> - case 0x5631: case 0x883d: case 0x8a01: case 0x8a10:
> - case 0x8c01: case 0x8c03: case 0x8904: case 0x8a13:
> - return "s3virge";
> - default:
> - return "savage";
> - }
> - case 0x1039: return "sis";
> - case 0x126f: return "siliconmotion";
> - case 0x121a:
> - if (dev->device_id < 0x0003)
> - return "voodoo";
> - else
> - return "tdfx";
> - case 0x3d3d: return "glint";
> - case 0x1023: return "trident";
> - case 0x100c: return "tseng";
> - case 0x1106: return "via";
> - case 0x15ad: return "vmware";
> - default: break;
> - }
> - return NULL;
> -}
> -
> Bool
> xf86AutoConfig(void)
> {
> @@ -551,8 +496,6 @@ chooseVideoDriver(void)
> if (matches[0]) {
> chosen_driver = matches[0];
> } else {
> - if (info != NULL)
> - chosen_driver = videoPtrToDriverName(info);
> if (chosen_driver == NULL) {
> xf86Msg(X_DEFAULT, "Could not find a driver that explicitly supports the primary video card. Falling back to the default driver.\n");
> #if defined __i386__ || defined __amd64__ || defined __hurd__
> --
> 1.5.3.8
>
> From 6ebc77a522e563895a6a4a94f4d846678fff285f Mon Sep 17 00:00:00 2001
> From: David Nusinow <dnusinow at debian.org>
> Date: Sat, 26 Jan 2008 16:11:40 -0500
> Subject: [PATCH] Remove old file-based autoloader. The symbol-based one is preferred
>
> This also includes a simplifying fix for the symbol-based loader.
> ---
> configure.ac | 1 -
> hw/xfree86/common/xf86AutoConfig.c | 130 +-----------------------------------
> hw/xfree86/common/xf86Config.h | 1 -
> include/xorg-config.h.in | 3 -
> 4 files changed, 3 insertions(+), 132 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index 566ddcb..7ee97c8 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1043,7 +1043,6 @@ if test "x$XDMAUTH" = xyes; then
> fi
>
> AC_DEFINE_DIR(COMPILEDDEFAULTFONTPATH, FONTPATH, [Default font path])
> -AC_DEFINE_DIR(PCI_TXT_IDS_PATH, PCI_TXT_IDS_DIR, [Default PCI text file ID path])
> AC_DEFINE_DIR(SERVER_MISC_CONFIG_PATH, SERVERCONFIG, [Server miscellaneous config path])
> AC_DEFINE_DIR(BASE_FONT_PATH, FONTDIR, [Default base font path])
> AC_DEFINE_DIR(DRI_DRIVER_PATH, DRI_DRIVER_PATH, [Default DRI driver path])
> diff --git a/hw/xfree86/common/xf86AutoConfig.c b/hw/xfree86/common/xf86AutoConfig.c
> index e1f9a9a..89ec889 100644
> --- a/hw/xfree86/common/xf86AutoConfig.c
> +++ b/hw/xfree86/common/xf86AutoConfig.c
> @@ -196,22 +196,6 @@ xf86AutoConfig(void)
> return (ret == CONFIG_OK);
> }
>
> -int
> -xchomp(char *line)
> -{
> - size_t len = 0;
> -
> - if (!line) {
> - return 1;
> - }
> -
> - len = strlen(line);
> - if (line[len - 1] == '\n' && len > 0) {
> - line[len - 1] = '\0';
> - }
> - return 0;
> -}
> -
> GDevPtr
> autoConfigDevice(GDevPtr preconf_device)
> {
> @@ -253,110 +237,6 @@ autoConfigDevice(GDevPtr preconf_device)
> return ptr;
> }
>
> -#ifdef __linux__
> -/* This function is used to provide a workaround for binary drivers that
> - * 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 */
> -static void
> -matchDriverFromFiles (char** matches, uint16_t match_vendor, uint16_t match_chip)
> -{
> - DIR *idsdir;
> - FILE *fp;
> - struct dirent *direntry;
> - char *line = NULL;
> - size_t len;
> - ssize_t read;
> - char path_name[256], vendor_str[5], chip_str[5];
> - uint16_t vendor, chip;
> - int i, j;
> -
> - idsdir = opendir(PCI_TXT_IDS_PATH);
> - if (idsdir) {
> - xf86Msg(X_INFO, "Scanning %s directory for additional PCI ID's supported by the drivers\n", PCI_TXT_IDS_PATH);
> - direntry = readdir(idsdir);
> - /* Read the directory */
> - while (direntry) {
> - if (direntry->d_name[0] == '.') {
> - direntry = readdir(idsdir);
> - continue;
> - }
> - len = strlen(direntry->d_name);
> - /* A tiny bit of sanity checking. We should probably do better */
> - if (strncmp(&(direntry->d_name[len-4]), ".ids", 4) == 0) {
> - /* We need the full path name to open the file */
> - strncpy(path_name, PCI_TXT_IDS_PATH, 256);
> - strncat(path_name, "/", 1);
> - strncat(path_name, direntry->d_name, (256 - strlen(path_name) - 1));
> - fp = fopen(path_name, "r");
> - if (fp == NULL) {
> - xf86Msg(X_ERROR, "Could not open %s for reading. Exiting.\n", path_name);
> - goto end;
> - }
> - /* Read the file */
> - #ifdef __GLIBC__
> - while ((read = getline(&line, &len, fp)) != -1) {
> - #else
> - while ((line = fgetln(fp, &len)) != (char *)NULL) {
> - #endif /* __GLIBC __ */
> - xchomp(line);
> - if (isdigit(line[0])) {
> - strncpy(vendor_str, line, 4);
> - vendor_str[4] = '\0';
> - vendor = (int)strtol(vendor_str, NULL, 16);
> - if ((strlen(&line[4])) == 0) {
> - chip_str[0] = '\0';
> - chip = -1;
> - } else {
> - /* Handle trailing whitespace */
> - if (isspace(line[4])) {
> - chip_str[0] = '\0';
> - chip = -1;
> - } else {
> - /* Ok, it's a real ID */
> - strncpy(chip_str, &line[4], 4);
> - chip_str[4] = '\0';
> - chip = (int)strtol(chip_str, NULL, 16);
> - }
> - }
> - if (vendor == match_vendor && chip == match_chip ) {
> - i = 0;
> - while (matches[i]) {
> - i++;
> - }
> - matches[i] = (char*)xalloc(sizeof(char) * strlen(direntry->d_name) - 3);
> - if (!matches[i]) {
> - xf86Msg(X_ERROR, "Could not allocate space for the module name. Exiting.\n");
> - goto end;
> - }
> - /* hack off the .ids suffix. This should guard
> - * against other problems, but it will end up
> - * 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';
> - break;
> - } else {
> - matches[i][j] = direntry->d_name[j];
> - }
> - }
> - xf86Msg(X_INFO, "Matched %s from file name %s\n", matches[i], direntry->d_name);
> - }
> - } else {
> - /* TODO Handle driver overrides here */
> - }
> - }
> - fclose(fp);
> - }
> - direntry = readdir(idsdir);
> - }
> - }
> - end:
> - xfree(line);
> - closedir(idsdir);
> -}
> -#endif /* __linux__ */
> -
> static char*
> driverClaimsID(struct dirent *direntry, uint16_t match_vendor, uint16_t match_chip, char *driverpath)
> {
> @@ -432,7 +312,6 @@ matchDriverFromSymbols (char** matches, int matchmax, uint16_t match_vendor, uin
> DIR *dir;
> struct dirent *direntry;
> int i;
> - char *p;
>
> xf86Msg(X_INFO, "Searching for drivers that support the primary pci video\n");
> driverpath = (char*)xalloc(strlen(xf86ModulePath) + strlen("/drivers/") + 1);
> @@ -445,11 +324,11 @@ matchDriverFromSymbols (char** matches, int matchmax, uint16_t match_vendor, uin
> matcheddriver = driverClaimsID(direntry, match_vendor, match_chip, driverpath);
> if (matcheddriver) {
> for (i = 0 ; i < matchmax ; i++) {
> - p = matches[i];
> - if (p == NULL)
> + if (matches[i] == NULL) {
> + matches[i] = matcheddriver;
> break;
> + }
> }
> - p = matcheddriver;
> }
> direntry = readdir(dir);
> }
> @@ -488,9 +367,6 @@ chooseVideoDriver(void)
> }
>
> matchDriverFromSymbols(matches, matchmax, info->vendor_id, info->device_id);
> -#ifdef __linux__
> - matchDriverFromFiles(matches, info->vendor_id, info->device_id);
> -#endif /* __linux__ */
>
> /* TODO Handle multiple drivers claiming to support the same PCI ID */
> if (matches[0]) {
> diff --git a/hw/xfree86/common/xf86Config.h b/hw/xfree86/common/xf86Config.h
> index a174e46..3411423 100644
> --- a/hw/xfree86/common/xf86Config.h
> +++ b/hw/xfree86/common/xf86Config.h
> @@ -69,6 +69,5 @@ ConfigStatus xf86HandleConfigFile(Bool);
> Bool xf86AutoConfig(void);
> GDevPtr autoConfigDevice(GDevPtr preconf_device);
> char* chooseVideoDriver(void);
> -int xchomp(char *line);
>
> #endif /* _xf86_config_h */
> diff --git a/include/xorg-config.h.in b/include/xorg-config.h.in
> index b91ea92..97d53a2 100644
> --- a/include/xorg-config.h.in
> +++ b/include/xorg-config.h.in
> @@ -112,7 +112,4 @@
> /* Have execinfo.h */
> #undef HAVE_EXECINFO_H
>
> -/* Path to text files containing PCI IDs */
> -#undef PCI_TXT_IDS_PATH
> -
> #endif /* _XORG_CONFIG_H_ */
> --
> 1.5.3.8
>
> _______________________________________________
> xorg mailing list
> xorg at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/xorg
More information about the xorg
mailing list