Discussion:
[PATCH] config/udev: get driver suggestions from udev
Aaron Plattner
2012-10-17 22:02:53 UTC
Permalink
If the udev device corresponding to a platform drm device has an "xorg_drivers"
property associated with it, treat that as a comma-separated list of driver
suggestions to use. Otherwise, fall back to the existing PCI ID table.

This lets people create a udev rule to associate an X driver with a given drm
device using rules that look like this:

SUBSYSTEM=="drm", DRIVERS=="i915", ENV{xorg_drivers}="intel"

Signed-off-by: Aaron Plattner <***@nvidia.com>
---
config/udev.c | 23 ++++++++++++++++++++---
hw/xfree86/common/xf86platformBus.c | 20 +++++++++++++++++++-
include/hotplug.h | 2 ++
3 files changed, 41 insertions(+), 4 deletions(-)

diff --git a/config/udev.c b/config/udev.c
index 454838f..43e8775 100644
--- a/config/udev.c
+++ b/config/udev.c
@@ -55,6 +55,7 @@ static struct udev_monitor *udev_monitor;
#ifdef CONFIG_UDEV_KMS
static Bool
config_udev_odev_setup_attribs(const char *path, const char *syspath,
+ const char *suggested_drivers,
config_odev_probe_proc_ptr probe_callback);
#endif

@@ -94,13 +95,18 @@ device_added(struct udev_device *udev_device)
#ifdef CONFIG_UDEV_KMS
if (!strcmp(udev_device_get_subsystem(udev_device), "drm")) {
const char *sysname = udev_device_get_sysname(udev_device);
+ const char *suggested_drivers;

if (strncmp(sysname, "card", 4) != 0)
return;

LogMessage(X_INFO, "config/udev: Adding drm device (%s)\n", path);

- config_udev_odev_setup_attribs(path, syspath, NewGPUDeviceRequest);
+ suggested_drivers = udev_device_get_property_value(udev_device,
+ "xorg_drivers");
+
+ config_udev_odev_setup_attribs(path, syspath, suggested_drivers,
+ NewGPUDeviceRequest);
return;
}
#endif
@@ -271,7 +277,7 @@ device_removed(struct udev_device *device)
if (!path)
return;

- config_udev_odev_setup_attribs(path, syspath, DeleteGPUDeviceRequest);
+ config_udev_odev_setup_attribs(path, syspath, NULL, DeleteGPUDeviceRequest);
return;
}
#endif
@@ -421,6 +427,7 @@ config_udev_fini(void)

static Bool
config_udev_odev_setup_attribs(const char *path, const char *syspath,
+ const char *suggested_drivers,
config_odev_probe_proc_ptr probe_callback)
{
struct OdevAttributes *attribs = config_odev_allocate_attribute_list();
@@ -437,6 +444,13 @@ config_udev_odev_setup_attribs(const char *path, const char *syspath,
if (ret == FALSE)
goto fail;

+ if (suggested_drivers) {
+ ret = config_odev_add_attribute(attribs, ODEV_ATTRIB_SUGGESTED_DRIVERS,
+ suggested_drivers);
+ if (ret == FALSE)
+ goto fail;
+ }
+
/* ownership of attribs is passed to probe layer */
probe_callback(attribs);
return TRUE;
@@ -471,6 +485,8 @@ config_udev_odev_probe(config_odev_probe_proc_ptr probe_callback)
struct udev_device *udev_device = udev_device_new_from_syspath(udev, syspath);
const char *path = udev_device_get_devnode(udev_device);
const char *sysname = udev_device_get_sysname(udev_device);
+ const char *suggested_drivers =
+ udev_device_get_property_value(udev_device, "xorg_drivers");

if (!path || !syspath)
goto no_probe;
@@ -479,7 +495,8 @@ config_udev_odev_probe(config_odev_probe_proc_ptr probe_callback)
else if (strncmp(sysname, "card", 4) != 0)
goto no_probe;

- config_udev_odev_setup_attribs(path, syspath, probe_callback);
+ config_udev_odev_setup_attribs(path, syspath, suggested_drivers,
+ probe_callback);

no_probe:
udev_device_unref(udev_device);
diff --git a/hw/xfree86/common/xf86platformBus.c b/hw/xfree86/common/xf86platformBus.c
index 0525e39..d48b48d 100644
--- a/hw/xfree86/common/xf86platformBus.c
+++ b/hw/xfree86/common/xf86platformBus.c
@@ -183,6 +183,7 @@ xf86PlatformMatchDriver(char *matches[], int nmatches)

for (pass = 0; pass < 2; pass++) {
for (i = 0; i < xf86_num_platform_devices; i++) {
+ const char *suggested_drivers;

if (xf86IsPrimaryPlatform(&xf86_platform_devices[i]) && (pass == 1))
continue;
@@ -199,7 +200,24 @@ xf86PlatformMatchDriver(char *matches[], int nmatches)
/* find end of matches list */
}

- if ((info != NULL) && (j < nmatches)) {
+ /* see if the kernel suggested any drivers */
+ suggested_drivers =
+ xf86_get_platform_attrib(i, ODEV_ATTRIB_SUGGESTED_DRIVERS);
+ if (suggested_drivers) {
+ char *tmp = strdup(suggested_drivers);
+
+ if (tmp) {
+ char *tok;
+ for (tok = strtok(tmp, ","); tok && j < nmatches;
+ tok = strtok(NULL, ",")) {
+ matches[j++] = strdup(tok);
+ }
+ free(tmp);
+ }
+ }
+ else if ((info != NULL) && (j < nmatches)) {
+ /* use the PCI ID match table if no suggestions were provided by
+ * the kernel driver */
j += xf86VideoPtrToDriverList(info, &(matches[j]), nmatches - j);
}
}
diff --git a/include/hotplug.h b/include/hotplug.h
index 2a95b45..6e89c51 100644
--- a/include/hotplug.h
+++ b/include/hotplug.h
@@ -61,6 +61,8 @@ config_odev_free_attributes(struct OdevAttributes *attribs);
#define ODEV_ATTRIB_SYSPATH 2
/* DRI-style bus id */
#define ODEV_ATTRIB_BUSID 3
+/* Comma-separated list of X.Org drivers suggested by the kernel driver */
+#define ODEV_ATTRIB_SUGGESTED_DRIVERS 4

typedef void (*config_odev_probe_proc_ptr)(struct OdevAttributes *attribs);
void config_odev_probe(config_odev_probe_proc_ptr probe_callback);
--
1.7.12
Peter Hutterer
2012-10-17 23:12:07 UTC
Permalink
Post by Aaron Plattner
If the udev device corresponding to a platform drm device has an "xorg_drivers"
property associated with it, treat that as a comma-separated list of driver
suggestions to use. Otherwise, fall back to the existing PCI ID table.
This lets people create a udev rule to associate an X driver with a given drm
SUBSYSTEM=="drm", DRIVERS=="i915", ENV{xorg_drivers}="intel"
we had a similar suggestion for input drivers when we added udev support.
but to keep the xorg configuration in X, we then added InputClass and
xorg.conf.d snippets. With this patch, you'd have xorg configuration in
udev. Is this really what you want, or is it worth investigating VideoClass
support?

Cheers,
Peter
Post by Aaron Plattner
---
config/udev.c | 23 ++++++++++++++++++++---
hw/xfree86/common/xf86platformBus.c | 20 +++++++++++++++++++-
include/hotplug.h | 2 ++
3 files changed, 41 insertions(+), 4 deletions(-)
diff --git a/config/udev.c b/config/udev.c
index 454838f..43e8775 100644
--- a/config/udev.c
+++ b/config/udev.c
@@ -55,6 +55,7 @@ static struct udev_monitor *udev_monitor;
#ifdef CONFIG_UDEV_KMS
static Bool
config_udev_odev_setup_attribs(const char *path, const char *syspath,
+ const char *suggested_drivers,
config_odev_probe_proc_ptr probe_callback);
#endif
@@ -94,13 +95,18 @@ device_added(struct udev_device *udev_device)
#ifdef CONFIG_UDEV_KMS
if (!strcmp(udev_device_get_subsystem(udev_device), "drm")) {
const char *sysname = udev_device_get_sysname(udev_device);
+ const char *suggested_drivers;
if (strncmp(sysname, "card", 4) != 0)
return;
LogMessage(X_INFO, "config/udev: Adding drm device (%s)\n", path);
- config_udev_odev_setup_attribs(path, syspath, NewGPUDeviceRequest);
+ suggested_drivers = udev_device_get_property_value(udev_device,
+ "xorg_drivers");
+
+ config_udev_odev_setup_attribs(path, syspath, suggested_drivers,
+ NewGPUDeviceRequest);
return;
}
#endif
@@ -271,7 +277,7 @@ device_removed(struct udev_device *device)
if (!path)
return;
- config_udev_odev_setup_attribs(path, syspath, DeleteGPUDeviceRequest);
+ config_udev_odev_setup_attribs(path, syspath, NULL, DeleteGPUDeviceRequest);
return;
}
#endif
@@ -421,6 +427,7 @@ config_udev_fini(void)
static Bool
config_udev_odev_setup_attribs(const char *path, const char *syspath,
+ const char *suggested_drivers,
config_odev_probe_proc_ptr probe_callback)
{
struct OdevAttributes *attribs = config_odev_allocate_attribute_list();
@@ -437,6 +444,13 @@ config_udev_odev_setup_attribs(const char *path, const char *syspath,
if (ret == FALSE)
goto fail;
+ if (suggested_drivers) {
+ ret = config_odev_add_attribute(attribs, ODEV_ATTRIB_SUGGESTED_DRIVERS,
+ suggested_drivers);
+ if (ret == FALSE)
+ goto fail;
+ }
+
/* ownership of attribs is passed to probe layer */
probe_callback(attribs);
return TRUE;
@@ -471,6 +485,8 @@ config_udev_odev_probe(config_odev_probe_proc_ptr probe_callback)
struct udev_device *udev_device = udev_device_new_from_syspath(udev, syspath);
const char *path = udev_device_get_devnode(udev_device);
const char *sysname = udev_device_get_sysname(udev_device);
+ const char *suggested_drivers =
+ udev_device_get_property_value(udev_device, "xorg_drivers");
if (!path || !syspath)
goto no_probe;
@@ -479,7 +495,8 @@ config_udev_odev_probe(config_odev_probe_proc_ptr probe_callback)
else if (strncmp(sysname, "card", 4) != 0)
goto no_probe;
- config_udev_odev_setup_attribs(path, syspath, probe_callback);
+ config_udev_odev_setup_attribs(path, syspath, suggested_drivers,
+ probe_callback);
udev_device_unref(udev_device);
diff --git a/hw/xfree86/common/xf86platformBus.c b/hw/xfree86/common/xf86platformBus.c
index 0525e39..d48b48d 100644
--- a/hw/xfree86/common/xf86platformBus.c
+++ b/hw/xfree86/common/xf86platformBus.c
@@ -183,6 +183,7 @@ xf86PlatformMatchDriver(char *matches[], int nmatches)
for (pass = 0; pass < 2; pass++) {
for (i = 0; i < xf86_num_platform_devices; i++) {
+ const char *suggested_drivers;
if (xf86IsPrimaryPlatform(&xf86_platform_devices[i]) && (pass == 1))
continue;
@@ -199,7 +200,24 @@ xf86PlatformMatchDriver(char *matches[], int nmatches)
/* find end of matches list */
}
- if ((info != NULL) && (j < nmatches)) {
+ /* see if the kernel suggested any drivers */
+ suggested_drivers =
+ xf86_get_platform_attrib(i, ODEV_ATTRIB_SUGGESTED_DRIVERS);
+ if (suggested_drivers) {
+ char *tmp = strdup(suggested_drivers);
+
+ if (tmp) {
+ char *tok;
+ for (tok = strtok(tmp, ","); tok && j < nmatches;
+ tok = strtok(NULL, ",")) {
+ matches[j++] = strdup(tok);
+ }
+ free(tmp);
+ }
+ }
+ else if ((info != NULL) && (j < nmatches)) {
+ /* use the PCI ID match table if no suggestions were provided by
+ * the kernel driver */
j += xf86VideoPtrToDriverList(info, &(matches[j]), nmatches - j);
}
}
diff --git a/include/hotplug.h b/include/hotplug.h
index 2a95b45..6e89c51 100644
--- a/include/hotplug.h
+++ b/include/hotplug.h
@@ -61,6 +61,8 @@ config_odev_free_attributes(struct OdevAttributes *attribs);
#define ODEV_ATTRIB_SYSPATH 2
/* DRI-style bus id */
#define ODEV_ATTRIB_BUSID 3
+/* Comma-separated list of X.Org drivers suggested by the kernel driver */
+#define ODEV_ATTRIB_SUGGESTED_DRIVERS 4
typedef void (*config_odev_probe_proc_ptr)(struct OdevAttributes *attribs);
void config_odev_probe(config_odev_probe_proc_ptr probe_callback);
--
1.7.12
_______________________________________________
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
Aaron Plattner
2012-10-17 23:15:01 UTC
Permalink
Post by Peter Hutterer
Post by Aaron Plattner
If the udev device corresponding to a platform drm device has an "xorg_drivers"
property associated with it, treat that as a comma-separated list of driver
suggestions to use. Otherwise, fall back to the existing PCI ID table.
This lets people create a udev rule to associate an X driver with a given drm
SUBSYSTEM=="drm", DRIVERS=="i915", ENV{xorg_drivers}="intel"
we had a similar suggestion for input drivers when we added udev support.
but to keep the xorg configuration in X, we then added InputClass and
xorg.conf.d snippets. With this patch, you'd have xorg configuration in
udev. Is this really what you want, or is it worth investigating VideoClass
support?
Hmm, maybe. That would be a lot more work, but might be more flexible in the
long run. I'll see if I can find some time to work on that.

-- Aaron
Dave Airlie
2012-10-17 23:40:11 UTC
Permalink
Post by Aaron Plattner
Post by Peter Hutterer
Post by Aaron Plattner
If the udev device corresponding to a platform drm device has an "xorg_drivers"
property associated with it, treat that as a comma-separated list of driver
suggestions to use. Otherwise, fall back to the existing PCI ID table.
This lets people create a udev rule to associate an X driver with a given drm
SUBSYSTEM=="drm", DRIVERS=="i915", ENV{xorg_drivers}="intel"
we had a similar suggestion for input drivers when we added udev support.
but to keep the xorg configuration in X, we then added InputClass and
xorg.conf.d snippets. With this patch, you'd have xorg configuration in
udev. Is this really what you want, or is it worth investigating VideoClass
support?
Hmm, maybe. That would be a lot more work, but might be more flexible in
the long run. I'll see if I can find some time to work on that.
Though I'm not sure we really VideoClass type situations, we generally
have a real driver or fallback to modesetting per set of devices.

we don't really have the synaptics situation where one driver can
drive loads of non-synaptics.

Dave.
Peter Hutterer
2012-10-18 00:43:57 UTC
Permalink
Post by Dave Airlie
Post by Aaron Plattner
Post by Peter Hutterer
Post by Aaron Plattner
If the udev device corresponding to a platform drm device has an "xorg_drivers"
property associated with it, treat that as a comma-separated list of driver
suggestions to use. Otherwise, fall back to the existing PCI ID table.
This lets people create a udev rule to associate an X driver with a given drm
SUBSYSTEM=="drm", DRIVERS=="i915", ENV{xorg_drivers}="intel"
we had a similar suggestion for input drivers when we added udev support.
but to keep the xorg configuration in X, we then added InputClass and
xorg.conf.d snippets. With this patch, you'd have xorg configuration in
udev. Is this really what you want, or is it worth investigating VideoClass
support?
Hmm, maybe. That would be a lot more work, but might be more flexible in
the long run. I'll see if I can find some time to work on that.
Though I'm not sure we really VideoClass type situations, we generally
have a real driver or fallback to modesetting per set of devices.
we don't really have the synaptics situation where one driver can
drive loads of non-synaptics.
Except evdev, we generally don't use "catchall classes". wacom and synaptics
only get applied for such devices, the rest of the InputClass features is
overloading configurations. e.g. for a product of this name, set that
option. I think this is quite similar to what would apply here.
The above example Aaron gave would be something like

Section "VideoClass"
Identifier "blah"
MatchDRMDriver "i915"
Driver "intel"
EndSection

Cheers,
Peter
Dan Nicholson
2012-10-18 04:04:39 UTC
Permalink
Post by Peter Hutterer
Post by Dave Airlie
Post by Aaron Plattner
Post by Peter Hutterer
Post by Aaron Plattner
If the udev device corresponding to a platform drm device has an "xorg_drivers"
property associated with it, treat that as a comma-separated list of driver
suggestions to use. Otherwise, fall back to the existing PCI ID table.
This lets people create a udev rule to associate an X driver with a
given
Post by Peter Hutterer
Post by Dave Airlie
Post by Aaron Plattner
Post by Peter Hutterer
Post by Aaron Plattner
drm
SUBSYSTEM=="drm", DRIVERS=="i915", ENV{xorg_drivers}="intel"
we had a similar suggestion for input drivers when we added udev support.
but to keep the xorg configuration in X, we then added InputClass and
xorg.conf.d snippets. With this patch, you'd have xorg configuration in
udev. Is this really what you want, or is it worth investigating VideoClass
support?
Hmm, maybe. That would be a lot more work, but might be more flexible in
the long run. I'll see if I can find some time to work on that.
Though I'm not sure we really VideoClass type situations, we generally
have a real driver or fallback to modesetting per set of devices.
we don't really have the synaptics situation where one driver can
drive loads of non-synaptics.
Except evdev, we generally don't use "catchall classes". wacom and synaptics
only get applied for such devices, the rest of the InputClass features is
overloading configurations. e.g. for a product of this name, set that
option. I think this is quite similar to what would apply here.
The above example Aaron gave would be something like
Section "VideoClass"
Identifier "blah"
MatchDRMDriver "i915"
Driver "intel"
EndSection
This was something I always intended to work on after InputClass but never
got started. At the very least it would be far nicer than the hardcoded PCI
table. It would be a good project for somebody.

Dan
Alan Coopersmith
2012-10-18 04:54:53 UTC
Permalink
Post by Peter Hutterer
Is this really what you want, or is it worth investigating VideoClass
support?
Hmm, maybe. That would be a lot more work, but might be more flexible in the
long run. I'll see if I can find some time to work on that.
Also more portable to non-udev platforms, which is why I've wanted something
like that for a long time, but had no time to think up a design & code it.
--
-Alan Coopersmith- ***@oracle.com
Oracle Solaris Engineering - http://blogs.oracle.com/alanc
Loading...