Re: [PATCH 2/2] xfree86: use udev to provide device enumeration for kms devices
Well the problem is I've no idea what hotplug on any other OS is going to look like, and I really don't want to invent an abstraction without input from either someone a) who cares about another OS b) has time to help me now, not in 6 months. Well, okay, but there's two things here. You have config/udev growing the ability to plug output devices, and you have hw/xfree86 growing a new device enumeration method during initial config. You're not _actually_ addressing hotplug yet, you've gotten as far as coldplug. Well this code is for addressing hotplug eventually, the code is all written to do it, just doesn't make sense until I finish lots of other bits. The thing is if I don't know what the OS-specific info that needs to go from udev-driver via the xf86 DDX, I can't abstract it. For input we've all but just made this information udev/evdev and Linux info, with no guarantee that other OSes could use it. So maybe I can just pretend I'm being OS friendly and remove the name udev. We'd of course need better naming for BUS_UDEV then? I was thinking BUS_PLATFORM. Blue. No, yellow. Yeah I could live with that. The only non-Linux non-PCI driver I know of is wsfb. But there's still dummy (and maybe nested?), and there's still fbdev, and I guess I don't have a good idea of how to express those. Hmm. Probably until I do I can't lobby for dropping the pre-pciaccess probe yet. Withdrawn. Yeah virtual drivers need old-skool probe. So the plan is to have two sets of xf86Screens, and if a driver support hotplug (it tells the server via a driverfunc call) you pass it a flag saying it should add a hotplug screen not a toplevel protocol screen. Is that just an optimization or is there a semantic difference? Phrased another way, why are not all screens hotplug screens? Is it just ease of migrating the drivers? Ease of migrating the drivers is probably top of the list, pointless-ness of doing hotplug on non-KMS drivers. I don't mind doing simple automatic API tweaks in drivers, but I'm not really wanting to do in depth surgery in order to support hotplug on the 5 or drivers I care about. The current v3 plan is to have one ScreenRec/ScrnInfoRec per protocol screen, with linkages to one ScreenRec/ScrnInfoRec per hotplugable driver. These structures for now would remain the same as otherwise I don't think it'll ever get merged. So we continue to have screenInfo.screens and xf86Screens containing arrays of the protocol screens. Then screenInfo.gpuscreens and xf86GPUScreens arrays containing the hotpluggable screens. If a driver doesn't support hotplug then it acts like it does now, we have one ScreenRec protocol/gpu combined screen. If a driver supports hotplug then we create one protocol ScreenRec and one GPU ScreenRec. If someone plugs in a device and the main driver is a single ScreenRec nothing happens, if someone plugs in a device and the main driver is hotplug capable we'll create another GPU screen rec and attach it in the correct place, depending on what type of slave it its initially, and once shatter works better, it can be its own rendering master. Well it could pick up headless things, but the drivers should deal with that in their probe code and fall out. If we ever get to adding render nodes we'd have to match on those as well at some point. Yeah, looks like nothing's calling drm_get_minor() that way yet. I guess my preference was to _not_ bind to render-only nodes at this level, and start doing that as an EGL-level decision even under X. But we need to bind to headless devices for PRIME to work on GPUs with no outputs. No USB support? I don't think the concept of Primary device applies to a USB connected device, since we generally use it for insane things like int10 and stuff. Ah yeah, forgot that context. But come to that, we could just as well assert that BUS_ODEV simply requires that the OS have handled POSTing, and not have the notion of Primary defined at all. Primary is also used on the switchable GPU laptops to denote who is running the show, like whether Intel should be running the show with nouveau as an offload slave or nouveau running the show with intel as an output slave. Heh. To me that sounds like incentive. I have an almost reasonable idea of how that code works, I just hate it. Some bits of xf86pciBus.c seem like stuff I'd rather not know, MatchPciInstances is the main, seems like a lot of code for whatever its doing. There is no way to enumerate non-PCI video devices without a list of them, KMS drivers is the only way to make it all work. I'm leaving the old probing functions intact for non-Linux and binary drivers for now, but I could be tempted to rip them out on Linux, if I added PCI graphics device to the udev probing I suppose. Well, binary drivers are going to involve a kernel driver. Maybe it makes sense to think about this as ask udev about kernel graphics services instead of ask udev about
[PATCH 2/2] xfree86: use udev to provide device enumeration for kms devices
From: Dave Airlie airl...@redhat.com On Linux in order for future hotplug work, we are required to interface to udev to detect device creation/removal. In order to try and get some earlier testing on this, this patch adds the ability to use udev for device enumeration on Linux. At startup the list of drm/kms devices is probed and this info is used to load drivers. A new driver probing method is introduced that passes the udev device info to the driver for probing. The probing integrates with the pci probing code and will fallback to the pci probe and old school probe functions in turn. The flags parameter to the probe function will be used later to provide hotplug and gpu screen flags for the driver to behave in a different way. This patch changes the driver ABI, all drivers should at least be set with a NULL udev probe function after this commit. Signed-off-by: Dave Airlie airl...@redhat.com --- config/udev.c | 29 configure.ac | 18 ++- hw/xfree86/common/Makefile.am | 10 +- hw/xfree86/common/xf86.h |5 + hw/xfree86/common/xf86Bus.c| 28 +++- hw/xfree86/common/xf86Bus.h|1 + hw/xfree86/common/xf86fbBus.c |4 + hw/xfree86/common/xf86pciBus.c | 20 +++- hw/xfree86/common/xf86str.h| 15 ++ hw/xfree86/common/xf86udev.c | 304 hw/xfree86/common/xf86udev.h | 42 ++ include/dix-config.h.in|3 + include/hotplug.h |4 + include/xorg-config.h.in |3 + include/xorg-server.h.in |3 + 15 files changed, 478 insertions(+), 11 deletions(-) create mode 100644 hw/xfree86/common/xf86udev.c create mode 100644 hw/xfree86/common/xf86udev.h diff --git a/config/udev.c b/config/udev.c index 1995184..700cc8b 100644 --- a/config/udev.c +++ b/config/udev.c @@ -366,3 +366,32 @@ config_udev_fini(void) udev_monitor = NULL; udev_unref(udev); } + +#ifdef CONFIG_UDEV_KMS +int +config_udev_probe_kms_outputs(config_udev_kms_probe_proc_ptr probe_callback) +{ +struct udev *udev; +struct udev_enumerate *enumerate; +struct udev_list_entry *devices, *device; + +udev = udev_monitor_get_udev(udev_monitor); +enumerate = udev_enumerate_new(udev); +if (!enumerate) +return 0; + +udev_enumerate_add_match_subsystem(enumerate, drm); +udev_enumerate_add_match_sysname(enumerate, card[0-9]*); +udev_enumerate_scan_devices(enumerate); +devices = udev_enumerate_get_list_entry(enumerate); +udev_list_entry_foreach(device, devices) { +const char *syspath = udev_list_entry_get_name(device); +struct udev_device *udev_device = udev_device_new_from_syspath(udev, syspath); +probe_callback(udev_device); +udev_device_unref(udev_device); +} +udev_enumerate_unref(enumerate); +return 0; +} +#endif + diff --git a/configure.ac b/configure.ac index 97ceab1..610cae9 100644 --- a/configure.ac +++ b/configure.ac @@ -615,6 +615,7 @@ AC_ARG_ENABLE(dbe, AS_HELP_STRING([--disable-dbe], [Build DBE extensi AC_ARG_ENABLE(xf86bigfont,AS_HELP_STRING([--enable-xf86bigfont], [Build XF86 Big Font extension (default: disabled)]), [XF86BIGFONT=$enableval], [XF86BIGFONT=no]) AC_ARG_ENABLE(dpms, AS_HELP_STRING([--disable-dpms], [Build DPMS extension (default: enabled)]), [DPMSExtension=$enableval], [DPMSExtension=yes]) AC_ARG_ENABLE(config-udev,AS_HELP_STRING([--enable-config-udev], [Build udev support (default: auto)]), [CONFIG_UDEV=$enableval], [CONFIG_UDEV=auto]) +AC_ARG_ENABLE(config-udev-kms,AS_HELP_STRING([--enable-config-udev-kms], [Build udev kms support (default: auto)]), [CONFIG_UDEV_KMS=$enableval], [CONFIG_UDEV_KMS=auto]) AC_ARG_ENABLE(config-dbus,AS_HELP_STRING([--enable-config-dbus], [Build D-BUS API support (default: no)]), [CONFIG_DBUS_API=$enableval], [CONFIG_DBUS_API=no]) AC_ARG_ENABLE(config-hal, AS_HELP_STRING([--disable-config-hal], [Build HAL support (default: auto)]), [CONFIG_HAL=$enableval], [CONFIG_HAL=auto]) AC_ARG_ENABLE(config-wscons, AS_HELP_STRING([--enable-config-wscons], [Build wscons config support (default: auto)]), [CONFIG_WSCONS=$enableval], [CONFIG_WSCONS=auto]) @@ -704,6 +705,7 @@ case $host_os in CONFIG_DBUS_API=no CONFIG_HAL=no CONFIG_UDEV=no + CONFIG_UDEV_KMS=no DGA=no DRI2=no INT10MODULE=no @@ -838,11 +840,16 @@ AM_CONDITIONAL(CONFIG_UDEV, [test x$CONFIG_UDEV = xyes]) if test x$CONFIG_UDEV = xyes; then CONFIG_DBUS_API=no CONFIG_HAL=no + if test x$CONFIG_UDEV_KMS = xauto; then + CONFIG_UDEV_KMS=$HAVE_LIBUDEV + fi if ! test x$HAVE_LIBUDEV = xyes; then AC_MSG_ERROR([udev configuration API requested, but libudev is not installed]) fi AC_DEFINE(CONFIG_UDEV, 1, [Use libudev for input hotplug]) - + if
Re: [PATCH 2/2] xfree86: use udev to provide device enumeration for kms devices
On 5/8/12 9:40 AM, Dave Airlie wrote: On Linux in order for future hotplug work, we are required to interface to udev to detect device creation/removal. In order to try and get some earlier testing on this, this patch adds the ability to use udev for device enumeration on Linux. The big thing I don't really like about this patch is how it talks about udev so much. The driver hook should be platformProbe or osProbe or something instead, and the fact that it's udev on Linux is just an OS detail. The probing integrates with the pci probing code and will fallback to the pci probe and old school probe functions in turn. What do we need to do to drop the old probe code? XXX DUDE The flags parameter to the probe function will be used later to provide hotplug and gpu screen flags for the driver to behave in a different way. Explain this a bit? What would a driver need to do differently between the hotplug and coldplug cases? Not that I'm opposed to having a flag here, just want to know what you envision it for. +int +config_udev_probe_kms_outputs(config_udev_kms_probe_proc_ptr probe_callback) Kind of hate the naming collision here with randr outputs. Possibly we should start saying idev vs odev for input/output devices? Not a big deal I guess. +{ +struct udev *udev; +struct udev_enumerate *enumerate; +struct udev_list_entry *devices, *device; + +udev = udev_monitor_get_udev(udev_monitor); +enumerate = udev_enumerate_new(udev); +if (!enumerate) +return 0; + +udev_enumerate_add_match_subsystem(enumerate, drm); +udev_enumerate_add_match_sysname(enumerate, card[0-9]*); I think this means we'll try to pick up everything with a drm node at all, which would include headless things, and I'm not totally sure that's what we want. @@ -22,9 +22,13 @@ if DGA DGASOURCES = xf86DGA.c endif +if CONFIG_UDEV_KMS +UDEVSOURCES = xf86udev.c +endif + If we were making this an OS detail, this kinda belongs under os-support/ instead. -I$(srcdir)/../loader -I$(srcdir)/../parser \ -I$(srcdir)/../vbe -I$(srcdir)/../int10 \ -I$(srcdir)/../vgahw -I$(srcdir)/../dixmods/extmod \ - -I$(srcdir)/../modes -I$(srcdir)/../ramdac + -I$(srcdir)/../modes -I$(srcdir)/../ramdac @LIBDRM_CFLAGS@ Would prefer if this was: UDEVSOURCES_CFLAGS = @LIBDRM_CFLAGS@ or whatever the appropriate automake is for per-target. #if (defined(__sparc__) || defined(__sparc)) !defined(__OpenBSD__) extern _X_EXPORT Bool sbusSlotClaimed; #endif + +#if defined(XSERVER_UDEV_KMS) +extern _X_EXPORT int udevSlotClaimed; +#endif + I really hate all of the SlotClaimed variables. Feels like there's a prettier solution. +case BUS_UDEV: +if (!pEnt-bus.id.udev-pdev) +return FALSE; +return ((pEnt-bus.id.udev-pdev-domain == primaryBus.id.pci-domain) +(pEnt-bus.id.udev-pdev-bus == primaryBus.id.pci-bus) +(pEnt-bus.id.udev-pdev-dev == primaryBus.id.pci-dev) +(pEnt-bus.id.udev-pdev-func == primaryBus.id.pci-func)); No USB support? diff --git a/hw/xfree86/common/xf86fbBus.c b/hw/xfree86/common/xf86fbBus.c index 1e51623..9c984f6 100644 --- a/hw/xfree86/common/xf86fbBus.c +++ b/hw/xfree86/common/xf86fbBus.c @@ -54,6 +54,10 @@ xf86ClaimFbSlot(DriverPtr drvp, int chipset, GDevPtr dev, Bool active) EntityPtr p; int num; +#ifdef XSERVER_UDEV_KMS +if (udevSlotClaimed) +return -1; +#endif #ifdef XSERVER_LIBPCIACCESS if (pciSlotClaimed) return -1; And this is why I hate them. This is saying if there are any platform-enumerated devices at all, you no longer get to use fbdev. Which is inconsistent with... diff --git a/hw/xfree86/common/xf86pciBus.c b/hw/xfree86/common/xf86pciBus.c index d758260..e5f1388 100644 --- a/hw/xfree86/common/xf86pciBus.c +++ b/hw/xfree86/common/xf86pciBus.c @@ -367,7 +367,15 @@ xf86GetPciInfoForEntity(int entityIndex) return NULL; p = xf86Entities[entityIndex]; -return (p-bus.type == BUS_PCI) ? p-bus.id.pci : NULL; +switch (p-bus.type) { +case BUS_PCI: +return p-bus.id.pci; +case BUS_UDEV: +return p-bus.id.udev-pdev; +default: +break; +} +return NULL; Where we go out of our way to allow both BUS_PCI and BUS_UDEV. One idea would be to have udev enumerate both DRM-backed and raw PCI/USB/misc video devices, and pass the distinction in the Probe function as a flag. If you did that you wouldn't need to try to keep other bus types working _with_ BUS_UDEV. More appealingly, if you did that you could probably redo the entirety of initial configuration to be, er, comprehensible. I suspect you're going to find some assumptions in the existing code that conflict with the idea of hotplug anyway. +#ifdef CONFIG_UDEV_KMS +if ((p-bus.type == BUS_UDEV) (p-bus.id.udev-pdev)) { +struct
Re: [PATCH 2/2] xfree86: use udev to provide device enumeration for kms devices
On Tue, May 8, 2012 at 6:54 PM, Adam Jackson a...@nwnk.net wrote: On 5/8/12 9:40 AM, Dave Airlie wrote: On Linux in order for future hotplug work, we are required to interface to udev to detect device creation/removal. In order to try and get some earlier testing on this, this patch adds the ability to use udev for device enumeration on Linux. The big thing I don't really like about this patch is how it talks about udev so much. The driver hook should be platformProbe or osProbe or something instead, and the fact that it's udev on Linux is just an OS detail. Well the problem is I've no idea what hotplug on any other OS is going to look like, and I really don't want to invent an abstraction without input from either someone a) who cares about another OS b) has time to help me now, not in 6 months. The thing is we hit drm code in these codepaths and I've no idea if they would be Linux specific or not, but I suppose I can shift a lot of stuff into os-support if necessary. We'd of course need better naming for BUS_UDEV then? The probing integrates with the pci probing code and will fallback to the pci probe and old school probe functions in turn. What do we need to do to drop the old probe code? XXX DUDE Like the old code is going to be needed on non-Linux OSes for non-PCI devices, I've no idea how to make that go away without other OS developers investing in it. (other than the remove it all and make them suffer). The flags parameter to the probe function will be used later to provide hotplug and gpu screen flags for the driver to behave in a different way. Explain this a bit? What would a driver need to do differently between the hotplug and coldplug cases? Not that I'm opposed to having a flag here, just want to know what you envision it for. So the plan is to have two sets of xf86Screens, and if a driver support hotplug (it tells the server via a driverfunc call) you pass it a flag saying it should add a hotplug screen not a toplevel protocol screen. +int +config_udev_probe_kms_outputs(config_udev_kms_probe_proc_ptr probe_callback) Kind of hate the naming collision here with randr outputs. Possibly we should start saying idev vs odev for input/output devices? Not a big deal I guess. Yeah maybe odev is a good name for the BUS as well. + + udev_enumerate_add_match_subsystem(enumerate, drm); + udev_enumerate_add_match_sysname(enumerate, card[0-9]*); I think this means we'll try to pick up everything with a drm node at all, which would include headless things, and I'm not totally sure that's what we want. Well it could pick up headless things, but the drivers should deal with that in their probe code and fall out. If we ever get to adding render nodes we'd have to match on those as well at some point. If we were making this an OS detail, this kinda belongs under os-support/ instead. yeah I'll look at that. Would prefer if this was: UDEVSOURCES_CFLAGS = @LIBDRM_CFLAGS@ or whatever the appropriate automake is for per-target. I've no idea how to do that, will have to investigate. #if (defined(__sparc__) || defined(__sparc)) !defined(__OpenBSD__) extern _X_EXPORT Bool sbusSlotClaimed; #endif + +#if defined(XSERVER_UDEV_KMS) +extern _X_EXPORT int udevSlotClaimed; +#endif + I really hate all of the SlotClaimed variables. Feels like there's a prettier solution. Yeah I'm sure this could be a lot nicer, but its something I feel this code doesn't make any worse. it just doesn't make it any better, apart from the previous patch falling out to make claimed meaningful. + case BUS_UDEV: + if (!pEnt-bus.id.udev-pdev) + return FALSE; + return ((pEnt-bus.id.udev-pdev-domain == primaryBus.id.pci-domain) + (pEnt-bus.id.udev-pdev-bus == primaryBus.id.pci-bus) + (pEnt-bus.id.udev-pdev-dev == primaryBus.id.pci-dev) + (pEnt-bus.id.udev-pdev-func == primaryBus.id.pci-func)); No USB support? I don't think the concept of Primary device applies to a USB connected device, since we generally use it for insane things like int10 and stuff. And this is why I hate them. This is saying if there are any platform-enumerated devices at all, you no longer get to use fbdev. Which is inconsistent with... Well fbdev can still bind via the pci routine, and hopefully with claiming fixed if you specifiy it in the xorg.confg Where we go out of our way to allow both BUS_PCI and BUS_UDEV. One idea would be to have udev enumerate both DRM-backed and raw PCI/USB/misc video devices, and pass the distinction in the Probe function as a flag. If you did that you wouldn't need to try to keep other bus types working _with_ BUS_UDEV. More appealingly, if you did that you could probably redo the entirety of initial configuration to be, er, comprehensible. I suspect you're going to find some assumptions in the existing code that conflict with the idea of
Re: [PATCH 2/2] xfree86: use udev to provide device enumeration for kms devices
On Tue, May 8, 2012 at 7:12 PM, Dave Airlie airl...@gmail.com wrote: On Tue, May 8, 2012 at 6:54 PM, Adam Jackson a...@nwnk.net wrote: On 5/8/12 9:40 AM, Dave Airlie wrote: On Linux in order for future hotplug work, we are required to interface to udev to detect device creation/removal. In order to try and get some earlier testing on this, this patch adds the ability to use udev for device enumeration on Linux. The big thing I don't really like about this patch is how it talks about udev so much. The driver hook should be platformProbe or osProbe or something instead, and the fact that it's udev on Linux is just an OS detail. I've thought about this some more and I can't come up with anything sensible with a generic interface. The info we need to pass the drivers to probe is os-specific info, I don't see the point of a generic interface that is passing non-generic information into drivers. Like look at the contents of xf86_udev_device, what in there isn't udev specific? what info should be in there that another OS would need etc. Like yes we could rewrite the whole lot, but I don't want another pciaccess, evolution already, not rewrite and break. Dave. ___ 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
Re: [PATCH 2/2] xfree86: use udev to provide device enumeration for kms devices
On 5/8/12 2:12 PM, Dave Airlie wrote: On Tue, May 8, 2012 at 6:54 PM, Adam Jackson a...@nwnk.net wrote: The big thing I don't really like about this patch is how it talks about udev so much. The driver hook should be platformProbe or osProbe or something instead, and the fact that it's udev on Linux is just an OS detail. Well the problem is I've no idea what hotplug on any other OS is going to look like, and I really don't want to invent an abstraction without input from either someone a) who cares about another OS b) has time to help me now, not in 6 months. Well, okay, but there's two things here. You have config/udev growing the ability to plug output devices, and you have hw/xfree86 growing a new device enumeration method during initial config. You're not _actually_ addressing hotplug yet, you've gotten as far as coldplug. The thing is we hit drm code in these codepaths and I've no idea if they would be Linux specific or not, but I suppose I can shift a lot of stuff into os-support if necessary. We'd of course need better naming for BUS_UDEV then? I was thinking BUS_PLATFORM. Blue. No, yellow. The probing integrates with the pci probing code and will fallback to the pci probe and old school probe functions in turn. What do we need to do to drop the old probe code? XXX DUDE Like the old code is going to be needed on non-Linux OSes for non-PCI devices, I've no idea how to make that go away without other OS developers investing in it. (other than the remove it all and make them suffer). Whoops, that XXX was supposed to be me going off and auditing the drivers and including the results before sending the email. Go me. The only non-Linux non-PCI driver I know of is wsfb. But there's still dummy (and maybe nested?), and there's still fbdev, and I guess I don't have a good idea of how to express those. Hmm. Probably until I do I can't lobby for dropping the pre-pciaccess probe yet. Withdrawn. So the plan is to have two sets of xf86Screens, and if a driver support hotplug (it tells the server via a driverfunc call) you pass it a flag saying it should add a hotplug screen not a toplevel protocol screen. Is that just an optimization or is there a semantic difference? Phrased another way, why are not all screens hotplug screens? Is it just ease of migrating the drivers? Well it could pick up headless things, but the drivers should deal with that in their probe code and fall out. If we ever get to adding render nodes we'd have to match on those as well at some point. Yeah, looks like nothing's calling drm_get_minor() that way yet. I guess my preference was to _not_ bind to render-only nodes at this level, and start doing that as an EGL-level decision even under X. No USB support? I don't think the concept of Primary device applies to a USB connected device, since we generally use it for insane things like int10 and stuff. Ah yeah, forgot that context. But come to that, we could just as well assert that BUS_ODEV simply requires that the OS have handled POSTing, and not have the notion of Primary defined at all. I've thought about doing that, but it is too much ripping up of code that nobody has any idea of how it magically works. Heh. To me that sounds like incentive. I have an almost reasonable idea of how that code works, I just hate it. There is no way to enumerate non-PCI video devices without a list of them, KMS drivers is the only way to make it all work. I'm leaving the old probing functions intact for non-Linux and binary drivers for now, but I could be tempted to rip them out on Linux, if I added PCI graphics device to the udev probing I suppose. Well, binary drivers are going to involve a kernel driver. Maybe it makes sense to think about this as ask udev about kernel graphics services instead of ask udev about KMS, which would give the userspace driver a place to hook in. - ajax ___ 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