[PATCH 1/2] Add device enumeration interface (v2)
> 1) work for vendor A but not for B or 2) will be OK for B but will produce > false positives for A. For such kind of cases, IMHO we probably can have vendor specific implementations. > A trivial lookup in sysfs will be able to provide all the required > information, won't you agree ? Yes, I agree. I think we can use the udev interfaces for such kind of enumeration by doing the match on the drm subsystem (the assumption is that the drm driver is loaded for the graphics device already). We will do some prototyping with udev for this. Regards, Jammy -Original Message- From: Emil Velikov [mailto:emil.l.veli...@gmail.com] Sent: Wednesday, April 29, 2015 12:18 AM To: Zhou, Jammy Cc: dri-devel at lists.freedesktop.org; Min, Frank Subject: Re: [PATCH 1/2] Add device enumeration interface (v2) On 28 April 2015 at 04:26, Zhou, Jammy wrote: > Hi Emil, > > This interface is intended for multiple GPU support. For example, we need to > know how many GPU devices are available on the system (and for some specific > vendor) in the client drivers, and then we can select proper devices for > rendering/compute/etc. For current mesa and Xserver implementations, the > device enumeration is done separately. I think it will be helpful if we can > have such kind of function in libdrm core, which can also be leveraged by > other new APIs requiring multi GPU support. > Hmm I'm not sure how the proposed interface will ease either mesa or xserver's implementation. The former is used only for clover(opencl) and already handles platform devices. While for the server I believe (haven't checked) that it just "throws" the PCI device information to the ddx and lets the latter do its thing. >> Any particular reason why "3D controller" (0x32000) is omitted ? > No. For AMD cards, we currently have 0x3 and 0x38000. Is 0x32000 used by > Nvidia cards? If so, I think we should add it as well. > What I am thinking is that using heuristics such as these will either 1) work for vendor A but not for B or 2) will be OK for B but will produce false positives for A. >> Using libpciaccess, will give you the number of PCI devices available on the >> system rather than the ones accessible - think about platform devices and/or >> devices without a drm driver. > This interface is just to enumerate the PCIE GPU devices on the system. With > regard to which ones are accessible, we can use drmOpen/drmOpenWithType to > check, and I don't want to have duplicated functionalities for these > interfaces. And for those non-PCIE platform devices (mostly on ARM > platforms), this interface shouldn't be used, and instead the client drivers > should handle it by themselves. > I am against duplication, to the point that I may have alienated a person or two :-\ Although this function as-is won't bring much benefit to mesa/xserver afaict. Plus it would be nice to keep an open mind for platform world, so that things will just work when AMD decides to go that road. Not to mention that iterating through all the devices in drmOpen* just to find that the device at pci:X:Y provides only a primary/render node seems a bit wasteful. A trivial lookup in sysfs will be able to provide all the required information, won't you agree ? Cheers, Emil
[PATCH 1/2] Add device enumeration interface (v2)
On 28 April 2015 at 04:26, Zhou, Jammy wrote: > Hi Emil, > > This interface is intended for multiple GPU support. For example, we need to > know how many GPU devices are available on the system (and for some specific > vendor) in the client drivers, and then we can select proper devices for > rendering/compute/etc. For current mesa and Xserver implementations, the > device enumeration is done separately. I think it will be helpful if we can > have such kind of function in libdrm core, which can also be leveraged by > other new APIs requiring multi GPU support. > Hmm I'm not sure how the proposed interface will ease either mesa or xserver's implementation. The former is used only for clover(opencl) and already handles platform devices. While for the server I believe (haven't checked) that it just "throws" the PCI device information to the ddx and lets the latter do its thing. >> Any particular reason why "3D controller" (0x32000) is omitted ? > No. For AMD cards, we currently have 0x3 and 0x38000. Is 0x32000 used by > Nvidia cards? If so, I think we should add it as well. > What I am thinking is that using heuristics such as these will either 1) work for vendor A but not for B or 2) will be OK for B but will produce false positives for A. >> Using libpciaccess, will give you the number of PCI devices available on the >> system rather than the ones accessible - think about platform devices and/or >> devices without a drm driver. > This interface is just to enumerate the PCIE GPU devices on the system. With > regard to which ones are accessible, we can use drmOpen/drmOpenWithType to > check, and I don't want to have duplicated functionalities for these > interfaces. And for those non-PCIE platform devices (mostly on ARM > platforms), this interface shouldn't be used, and instead the client drivers > should handle it by themselves. > I am against duplication, to the point that I may have alienated a person or two :-\ Although this function as-is won't bring much benefit to mesa/xserver afaict. Plus it would be nice to keep an open mind for platform world, so that things will just work when AMD decides to go that road. Not to mention that iterating through all the devices in drmOpen* just to find that the device at pci:X:Y provides only a primary/render node seems a bit wasteful. A trivial lookup in sysfs will be able to provide all the required information, won't you agree ? Cheers, Emil
[PATCH 1/2] Add device enumeration interface (v2)
Hi Emil,
This interface is intended for multiple GPU support. For example, we need to
know how many GPU devices are available on the system (and for some specific
vendor) in the client drivers, and then we can select proper devices for
rendering/compute/etc. For current mesa and Xserver implementations, the device
enumeration is done separately. I think it will be helpful if we can have such
kind of function in libdrm core, which can also be leveraged by other new APIs
requiring multi GPU support.
> Any particular reason why "3D controller" (0x32000) is omitted ?
No. For AMD cards, we currently have 0x3 and 0x38000. Is 0x32000 used by
Nvidia cards? If so, I think we should add it as well.
> Using libpciaccess, will give you the number of PCI devices available on the
> system rather than the ones accessible - think about platform devices and/or
> devices without a drm driver.
This interface is just to enumerate the PCIE GPU devices on the system. With
regard to which ones are accessible, we can use drmOpen/drmOpenWithType to
check, and I don't want to have duplicated functionalities for these
interfaces. And for those non-PCIE platform devices (mostly on ARM platforms),
this interface shouldn't be used, and instead the client drivers should handle
it by themselves.
Regards,
Jammy
-Original Message-
From: Emil Velikov [mailto:emil.l.veli...@gmail.com]
Sent: Tuesday, April 28, 2015 3:07 AM
To: Zhou, Jammy; dri-devel at lists.freedesktop.org
Cc: emil.l.velikov at gmail.com; Min, Frank
Subject: Re: [PATCH 1/2] Add device enumeration interface (v2)
Hi Jammy, Frank
As far as I can see you're trying to get a different version of drmGetBusid().
With the DRM_IOCTL_{G,S}ET_UNIQUE ioctl being lovely as it is I do see your
point, but I'm not sure that the current design will be too useful.
Do we have any upcoming users for this new function, can you share a bit about
the usecase ?
On 24/04/15 03:44, Jammy Zhou wrote:
> drmGetDevices interface is added to enumernate GPU devices on the
> system
>
> v2: rebase the code and some improvement for the coding style
>
> Signed-off-by: Frank Min
> Signed-off-by: Jammy Zhou (v2)
> ---
> Makefile.am | 3 ++-
> xf86drm.c | 48
> xf86drm.h | 18 ++
> 3 files changed, 68 insertions(+), 1 deletion(-)
>
> diff --git a/Makefile.am b/Makefile.am index 42d3d7f..8236ed8 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -89,7 +89,8 @@ SUBDIRS = \
> libdrm_la_LTLIBRARIES = libdrm.la
> libdrm_ladir = $(libdir)
> libdrm_la_LDFLAGS = -version-number 2:4:0 -no-undefined
> -libdrm_la_LIBADD = @CLOCK_LIB@
> +libdrm_la_LIBADD = @CLOCK_LIB@ \
> + @PCIACCESS_LIBS@
>
> libdrm_la_CPPFLAGS = -I$(top_srcdir)/include/drm AM_CFLAGS = \ diff
> --git a/xf86drm.c b/xf86drm.c index ffc53b8..4d67861 100644
> --- a/xf86drm.c
> +++ b/xf86drm.c
> @@ -63,6 +63,7 @@
>
> #include "xf86drm.h"
> #include "libdrm.h"
> +#include
>
> #if defined(__FreeBSD__) || defined(__FreeBSD_kernel__) ||
> defined(__DragonFly__) #define DRM_MAJOR 145 @@ -2817,3 +2818,50 @@
> char *drmGetRenderDeviceNameFromFd(int fd) {
> return drmGetMinorNameForFD(fd, DRM_NODE_RENDER); }
> +
> +/**
> + * Enumerate the GPU devices on the system
> + *
> + * \param devs device array set to return the device information
> + * (if NULL, the number of device is returned)
> + * \param vendor the vendor ID for GPU devices to list
> + * (optional, if not specified, all GPU devices are returned)
> + *
> + * \return the number of GPU devices
> + */
> +int drmGetDevices(drmDevicePtr devs, uint16_t vendor) {
> + struct pci_device_iterator * iter;
> + struct pci_device * dev;
> + uint32_t count = 0;
> +
> + if (pci_system_init())
> + return -EINVAL;
> +
> + iter = pci_slot_match_iterator_create(NULL);
> + if (!iter)
> + return -EINVAL;
> +
> + while ((dev = pci_device_next(iter))) {
> + if (((dev->device_class == 0x3) ||
> + (dev->device_class == 0x38000)) &&
Any particular reason why "3D controller" (0x32000) is omitted ?
> + ((vendor == 0) || (dev->vendor_id == vendor))){
> + if (devs) {
> + devs[count].domain = dev->domain;
> + devs[count].bus = dev->bus;
> + devs[count].dev = dev->dev;
> + devs[count].func = dev->func;
> + devs[count].vendor_id = dev->vendor_id;
> + devs[count].device_id =
[PATCH 1/2] Add device enumeration interface (v2)
Hi Jammy, Frank
As far as I can see you're trying to get a different version of
drmGetBusid(). With the DRM_IOCTL_{G,S}ET_UNIQUE ioctl being lovely as
it is I do see your point, but I'm not sure that the current design will
be too useful.
Do we have any upcoming users for this new function, can you share a bit
about the usecase ?
On 24/04/15 03:44, Jammy Zhou wrote:
> drmGetDevices interface is added to enumernate GPU devices on the system
>
> v2: rebase the code and some improvement for the coding style
>
> Signed-off-by: Frank Min
> Signed-off-by: Jammy Zhou (v2)
> ---
> Makefile.am | 3 ++-
> xf86drm.c | 48
> xf86drm.h | 18 ++
> 3 files changed, 68 insertions(+), 1 deletion(-)
>
> diff --git a/Makefile.am b/Makefile.am
> index 42d3d7f..8236ed8 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -89,7 +89,8 @@ SUBDIRS = \
> libdrm_la_LTLIBRARIES = libdrm.la
> libdrm_ladir = $(libdir)
> libdrm_la_LDFLAGS = -version-number 2:4:0 -no-undefined
> -libdrm_la_LIBADD = @CLOCK_LIB@
> +libdrm_la_LIBADD = @CLOCK_LIB@ \
> + @PCIACCESS_LIBS@
>
> libdrm_la_CPPFLAGS = -I$(top_srcdir)/include/drm
> AM_CFLAGS = \
> diff --git a/xf86drm.c b/xf86drm.c
> index ffc53b8..4d67861 100644
> --- a/xf86drm.c
> +++ b/xf86drm.c
> @@ -63,6 +63,7 @@
>
> #include "xf86drm.h"
> #include "libdrm.h"
> +#include
>
> #if defined(__FreeBSD__) || defined(__FreeBSD_kernel__) ||
> defined(__DragonFly__)
> #define DRM_MAJOR 145
> @@ -2817,3 +2818,50 @@ char *drmGetRenderDeviceNameFromFd(int fd)
> {
> return drmGetMinorNameForFD(fd, DRM_NODE_RENDER);
> }
> +
> +/**
> + * Enumerate the GPU devices on the system
> + *
> + * \param devs device array set to return the device information
> + * (if NULL, the number of device is returned)
> + * \param vendor the vendor ID for GPU devices to list
> + * (optional, if not specified, all GPU devices are returned)
> + *
> + * \return the number of GPU devices
> + */
> +int drmGetDevices(drmDevicePtr devs, uint16_t vendor)
> +{
> + struct pci_device_iterator * iter;
> + struct pci_device * dev;
> + uint32_t count = 0;
> +
> + if (pci_system_init())
> + return -EINVAL;
> +
> + iter = pci_slot_match_iterator_create(NULL);
> + if (!iter)
> + return -EINVAL;
> +
> + while ((dev = pci_device_next(iter))) {
> + if (((dev->device_class == 0x3) ||
> + (dev->device_class == 0x38000)) &&
Any particular reason why "3D controller" (0x32000) is omitted ?
> + ((vendor == 0) || (dev->vendor_id == vendor))){
> + if (devs) {
> + devs[count].domain = dev->domain;
> + devs[count].bus = dev->bus;
> + devs[count].dev = dev->dev;
> + devs[count].func = dev->func;
> + devs[count].vendor_id = dev->vendor_id;
> + devs[count].device_id = dev->device_id;
> + devs[count].subvendor_id = dev->subvendor_id;
> + devs[count].subdevice_id = dev->subdevice_id;
> + devs[count].revision_id = dev->revision;
> + }
> + count++;
> + }
> + }
> +
> + pci_iterator_destroy(iter);
> + pci_system_cleanup();
Using libpciaccess, will give you the number of PCI devices available on
the system rather than the ones accessible - think about platform
devices and/or devices without a drm driver.
Another solution will be to get the information based on the
primary/control/render nodes available. This will also allow one to know
what the device can be used for - be that via a separate parameter set
by the function or having different functions altogether.
Cheers
Emil
[PATCH 1/2] Add device enumeration interface (v2)
drmGetDevices interface is added to enumernate GPU devices on the system
v2: rebase the code and some improvement for the coding style
Signed-off-by: Frank Min
Signed-off-by: Jammy Zhou (v2)
---
Makefile.am | 3 ++-
xf86drm.c | 48
xf86drm.h | 18 ++
3 files changed, 68 insertions(+), 1 deletion(-)
diff --git a/Makefile.am b/Makefile.am
index 42d3d7f..8236ed8 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -89,7 +89,8 @@ SUBDIRS = \
libdrm_la_LTLIBRARIES = libdrm.la
libdrm_ladir = $(libdir)
libdrm_la_LDFLAGS = -version-number 2:4:0 -no-undefined
-libdrm_la_LIBADD = @CLOCK_LIB@
+libdrm_la_LIBADD = @CLOCK_LIB@ \
+ @PCIACCESS_LIBS@
libdrm_la_CPPFLAGS = -I$(top_srcdir)/include/drm
AM_CFLAGS = \
diff --git a/xf86drm.c b/xf86drm.c
index ffc53b8..4d67861 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -63,6 +63,7 @@
#include "xf86drm.h"
#include "libdrm.h"
+#include
#if defined(__FreeBSD__) || defined(__FreeBSD_kernel__) ||
defined(__DragonFly__)
#define DRM_MAJOR 145
@@ -2817,3 +2818,50 @@ char *drmGetRenderDeviceNameFromFd(int fd)
{
return drmGetMinorNameForFD(fd, DRM_NODE_RENDER);
}
+
+/**
+ * Enumerate the GPU devices on the system
+ *
+ * \param devs device array set to return the device information
+ * (if NULL, the number of device is returned)
+ * \param vendor the vendor ID for GPU devices to list
+ * (optional, if not specified, all GPU devices are returned)
+ *
+ * \return the number of GPU devices
+ */
+int drmGetDevices(drmDevicePtr devs, uint16_t vendor)
+{
+ struct pci_device_iterator * iter;
+ struct pci_device * dev;
+ uint32_t count = 0;
+
+ if (pci_system_init())
+ return -EINVAL;
+
+ iter = pci_slot_match_iterator_create(NULL);
+ if (!iter)
+ return -EINVAL;
+
+ while ((dev = pci_device_next(iter))) {
+ if (((dev->device_class == 0x3) ||
+ (dev->device_class == 0x38000)) &&
+ ((vendor == 0) || (dev->vendor_id == vendor))){
+ if (devs) {
+ devs[count].domain = dev->domain;
+ devs[count].bus = dev->bus;
+ devs[count].dev = dev->dev;
+ devs[count].func = dev->func;
+ devs[count].vendor_id = dev->vendor_id;
+ devs[count].device_id = dev->device_id;
+ devs[count].subvendor_id = dev->subvendor_id;
+ devs[count].subdevice_id = dev->subdevice_id;
+ devs[count].revision_id = dev->revision;
+ }
+ count++;
+ }
+ }
+
+ pci_iterator_destroy(iter);
+ pci_system_cleanup();
+ return count;
+}
diff --git a/xf86drm.h b/xf86drm.h
index 40c55c9..6150e71 100644
--- a/xf86drm.h
+++ b/xf86drm.h
@@ -752,6 +752,24 @@ extern int drmPrimeFDToHandle(int fd, int prime_fd,
uint32_t *handle);
extern char *drmGetPrimaryDeviceNameFromFd(int fd);
extern char *drmGetRenderDeviceNameFromFd(int fd);
+/**
+ * Structure for a general pcie gpu device
+ */
+typedef struct _drmDevice {
+ uint16_t domain;
+ uint8_t bus;
+ uint8_t dev;
+ uint8_t func;
+ uint8_t revision_id;
+ uint16_t vendor_id;
+ uint16_t device_id;
+ uint16_t subvendor_id;
+ uint16_t subdevice_id;
+} drmDevice, *drmDevicePtr;
+
+
+extern int drmGetDevices(drmDevicePtr devs, uint16_t vendor);
+
#if defined(__cplusplus) || defined(c_plusplus)
}
#endif
--
1.9.1
