| Message ID | FD5696F5CDD4B04D92C97DB96167DECA16198248@SCYBEXDAG04.amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 25 May 2015 at 12:18, Zhou, Jammy <Jammy.Zhou@amd.com> wrote: > Hi Emil, > > Do you have chance to have a look at this new version? Thanks! > Hi guys sorry for the delay. Looking at my original email response it seems that I wasn't clear enough about my concerns. They can be looked at as independent and/or grouped up, depending on your liking: - Adding a new interface using which it's not obvious how one can simplify/ease existing related implementations. As mentioned before, the claiming duplication that already exists is more complete than this API. - No open-source software uses the interface. Do you have any patches in progress that convert project foo to using the API? Otherwise is seems like an open-source project catering after a closed source project needs. - Adding new dependencies. Previously libpciaccess, now libudev. The former is used only by a single function in libdrm_intel, while the latter was optional. v3 makes the use of libudev a hard requirement. Using the library in mesa has caused problems with Steam (and possibly other binary programs/games) which ships their own version of various libraries. Now that the patch is merged, the last issue has emerged (in a particular form). Is there any plan to convert an open-source project to use this API ? Thanks Emil
On Thu, May 28, 2015 at 8:44 AM, Emil Velikov <emil.l.velikov@gmail.com> wrote: > On 25 May 2015 at 12:18, Zhou, Jammy <Jammy.Zhou@amd.com> wrote: >> Hi Emil, >> >> Do you have chance to have a look at this new version? Thanks! >> > Hi guys sorry for the delay. > > Looking at my original email response it seems that I wasn't clear > enough about my concerns. They can be looked at as independent and/or > grouped up, depending on your liking: > > - Adding a new interface using which it's not obvious how one can > simplify/ease existing related implementations. > As mentioned before, the claiming duplication that already exists is > more complete than this API. > > - No open-source software uses the interface. > Do you have any patches in progress that convert project foo to using > the API? Otherwise is seems like an open-source project catering after > a closed source project needs. It's actually in preparation for upcoming open source releases. > > - Adding new dependencies. > Previously libpciaccess, now libudev. The former is used only by a > single function in libdrm_intel, while the latter was optional. v3 > makes the use of libudev a hard requirement. > Using the library in mesa has caused problems with Steam (and possibly > other binary programs/games) which ships their own version of various > libraries. Can you think of a better method for device enumeration? Rolling our own seem like a waste of effort. > > Now that the patch is merged, the last issue has emerged (in a > particular form). Is there any plan to convert an open-source project > to use this API ? The first users actually will be open source they are just not ready for public release just yet. We are trying to reduce our internal patch queue and get stuff upstream early and thought others might be interested, but if there are concerns we can revert it for now and re-submit it when we are ready to release the relevant code. Alex
Hi Alex, On 28 May 2015 at 14:16, Alex Deucher <alexdeucher@gmail.com> wrote: > On Thu, May 28, 2015 at 8:44 AM, Emil Velikov <emil.l.velikov@gmail.com> wrote: >> On 25 May 2015 at 12:18, Zhou, Jammy <Jammy.Zhou@amd.com> wrote: >>> Hi Emil, >>> >>> Do you have chance to have a look at this new version? Thanks! >>> >> Hi guys sorry for the delay. >> >> Looking at my original email response it seems that I wasn't clear >> enough about my concerns. They can be looked at as independent and/or >> grouped up, depending on your liking: >> >> - Adding a new interface using which it's not obvious how one can >> simplify/ease existing related implementations. >> As mentioned before, the claiming duplication that already exists is >> more complete than this API. >> >> - No open-source software uses the interface. >> Do you have any patches in progress that convert project foo to using >> the API? Otherwise is seems like an open-source project catering after >> a closed source project needs. > > It's actually in preparation for upcoming open source releases. > Ack. Fair enough - the commit message did not mention any open source users. The follow-up response mentioned xserver and mesa for which this API doesn't seem like a good fit. >> >> - Adding new dependencies. >> Previously libpciaccess, now libudev. The former is used only by a >> single function in libdrm_intel, while the latter was optional. v3 >> makes the use of libudev a hard requirement. >> Using the library in mesa has caused problems with Steam (and possibly >> other binary programs/games) which ships their own version of various >> libraries. > > Can you think of a better method for device enumeration? Rolling our > own seem like a waste of effort. > Implementation might be uglier (as do other parts of libdrm), but I can envision an API that does not require a new dependency, plus it works with platform devices. So I'm not sure if it qualifies as better or not. > >> >> Now that the patch is merged, the last issue has emerged (in a >> particular form). Is there any plan to convert an open-source project >> to use this API ? > > The first users actually will be open source they are just not ready > for public release just yet. We are trying to reduce our internal > patch queue and get stuff upstream early and thought others might be > interested, but if there are concerns we can revert it for now and > re-submit it when we are ready to release the relevant code. > If you can share some pseudo code on what the requirements are/how it should be used I can prep an alternative solution. A one that works for your usecase and existing ones. In the mean while can we revert it, pretty please ? Thanks Emil
On Thu, May 28, 2015 at 10:22 AM, Emil Velikov <emil.l.velikov@gmail.com> wrote: > Hi Alex, > > On 28 May 2015 at 14:16, Alex Deucher <alexdeucher@gmail.com> wrote: >> On Thu, May 28, 2015 at 8:44 AM, Emil Velikov <emil.l.velikov@gmail.com> wrote: >>> On 25 May 2015 at 12:18, Zhou, Jammy <Jammy.Zhou@amd.com> wrote: >>>> Hi Emil, >>>> >>>> Do you have chance to have a look at this new version? Thanks! >>>> >>> Hi guys sorry for the delay. >>> >>> Looking at my original email response it seems that I wasn't clear >>> enough about my concerns. They can be looked at as independent and/or >>> grouped up, depending on your liking: >>> >>> - Adding a new interface using which it's not obvious how one can >>> simplify/ease existing related implementations. >>> As mentioned before, the claiming duplication that already exists is >>> more complete than this API. >>> >>> - No open-source software uses the interface. >>> Do you have any patches in progress that convert project foo to using >>> the API? Otherwise is seems like an open-source project catering after >>> a closed source project needs. >> >> It's actually in preparation for upcoming open source releases. >> > Ack. Fair enough - the commit message did not mention any open source > users. The follow-up response mentioned xserver and mesa for which > this API doesn't seem like a good fit. > >>> >>> - Adding new dependencies. >>> Previously libpciaccess, now libudev. The former is used only by a >>> single function in libdrm_intel, while the latter was optional. v3 >>> makes the use of libudev a hard requirement. >>> Using the library in mesa has caused problems with Steam (and possibly >>> other binary programs/games) which ships their own version of various >>> libraries. >> >> Can you think of a better method for device enumeration? Rolling our >> own seem like a waste of effort. >> > Implementation might be uglier (as do other parts of libdrm), but I > can envision an API that does not require a new dependency, plus it > works with platform devices. So I'm not sure if it qualifies as better > or not. > >> >>> >>> Now that the patch is merged, the last issue has emerged (in a >>> particular form). Is there any plan to convert an open-source project >>> to use this API ? >> >> The first users actually will be open source they are just not ready >> for public release just yet. We are trying to reduce our internal >> patch queue and get stuff upstream early and thought others might be >> interested, but if there are concerns we can revert it for now and >> re-submit it when we are ready to release the relevant code. >> > If you can share some pseudo code on what the requirements are/how it > should be used I can prep an alternative solution. A one that works > for your usecase and existing ones. In the mean while can we revert > it, pretty please ? Sure, go for it. Jammy or Frank might be able to provide some pseudo code in the interim. Alex > > Thanks > Emil
> Jammy or Frank might be able to provide some pseudo code in the interim. I think the requirement here is quite simple. We would like to have some interface to enumerate the GPU devices on the system, and select some specific device for different purposes (i.e, rendering, computing, displaying, etc). Current libdrm interfaces are just for single device, and there is no good consideration for multiple GPUs yet. Regards, Jammy -----Original Message----- From: Alex Deucher [mailto:alexdeucher@gmail.com] Sent: Friday, May 29, 2015 10:09 PM To: Emil Velikov Cc: Zhou, Jammy; Deucher, Alexander; Min, Frank; dri-devel@lists.freedesktop.org Subject: Re: [PATCH] Add device enumeration interface (v3) On Thu, May 28, 2015 at 10:22 AM, Emil Velikov <emil.l.velikov@gmail.com> wrote: > Hi Alex, > > On 28 May 2015 at 14:16, Alex Deucher <alexdeucher@gmail.com> wrote: >> On Thu, May 28, 2015 at 8:44 AM, Emil Velikov <emil.l.velikov@gmail.com> wrote: >>> On 25 May 2015 at 12:18, Zhou, Jammy <Jammy.Zhou@amd.com> wrote: >>>> Hi Emil, >>>> >>>> Do you have chance to have a look at this new version? Thanks! >>>> >>> Hi guys sorry for the delay. >>> >>> Looking at my original email response it seems that I wasn't clear >>> enough about my concerns. They can be looked at as independent >>> and/or grouped up, depending on your liking: >>> >>> - Adding a new interface using which it's not obvious how one can >>> simplify/ease existing related implementations. >>> As mentioned before, the claiming duplication that already exists is >>> more complete than this API. >>> >>> - No open-source software uses the interface. >>> Do you have any patches in progress that convert project foo to >>> using the API? Otherwise is seems like an open-source project >>> catering after a closed source project needs. >> >> It's actually in preparation for upcoming open source releases. >> > Ack. Fair enough - the commit message did not mention any open source > users. The follow-up response mentioned xserver and mesa for which > this API doesn't seem like a good fit. > >>> >>> - Adding new dependencies. >>> Previously libpciaccess, now libudev. The former is used only by a >>> single function in libdrm_intel, while the latter was optional. v3 >>> makes the use of libudev a hard requirement. >>> Using the library in mesa has caused problems with Steam (and >>> possibly other binary programs/games) which ships their own version >>> of various libraries. >> >> Can you think of a better method for device enumeration? Rolling our >> own seem like a waste of effort. >> > Implementation might be uglier (as do other parts of libdrm), but I > can envision an API that does not require a new dependency, plus it > works with platform devices. So I'm not sure if it qualifies as better > or not. > >> >>> >>> Now that the patch is merged, the last issue has emerged (in a >>> particular form). Is there any plan to convert an open-source >>> project to use this API ? >> >> The first users actually will be open source they are just not ready >> for public release just yet. We are trying to reduce our internal >> patch queue and get stuff upstream early and thought others might be >> interested, but if there are concerns we can revert it for now and >> re-submit it when we are ready to release the relevant code. >> > If you can share some pseudo code on what the requirements are/how it > should be used I can prep an alternative solution. A one that works > for your usecase and existing ones. In the mean while can we revert > it, pretty please ? Sure, go for it. Jammy or Frank might be able to provide some pseudo code in the interim. Alex > > Thanks > Emil
Hi Emil,
Here is the pseudo code:
int drmGetPciDevices(drmPciDevicePtr devSet, uint16_t vendorId)
{
If(devSet == NULL && vendorId == 0)
Probe all the graphic device and return the number of it
Else If(devSet != NULL && vendorId != 0)
Probe the specific vender graphic device and return it in the array devSet
Else If(devSet ==NULL && vendorId != 0)
Probe the specific vendorId graphic device and return the number of it
Else
Should not happen
}
Best Regards,
Frank
-----Original Message-----
From: Zhou, Jammy
Sent: Monday, June 01, 2015 10:12 AM
To: Alex Deucher; Emil Velikov
Cc: Deucher, Alexander; Min, Frank; dri-devel@lists.freedesktop.org
Subject: RE: [PATCH] Add device enumeration interface (v3)
> Jammy or Frank might be able to provide some pseudo code in the interim.
I think the requirement here is quite simple. We would like to have some interface to enumerate the GPU devices on the system, and select some specific device for different purposes (i.e, rendering, computing, displaying, etc). Current libdrm interfaces are just for single device, and there is no good consideration for multiple GPUs yet.
Regards,
Jammy
-----Original Message-----
From: Alex Deucher [mailto:alexdeucher@gmail.com]
Sent: Friday, May 29, 2015 10:09 PM
To: Emil Velikov
Cc: Zhou, Jammy; Deucher, Alexander; Min, Frank; dri-devel@lists.freedesktop.org
Subject: Re: [PATCH] Add device enumeration interface (v3)
On Thu, May 28, 2015 at 10:22 AM, Emil Velikov <emil.l.velikov@gmail.com> wrote:
> Hi Alex,
>
> On 28 May 2015 at 14:16, Alex Deucher <alexdeucher@gmail.com> wrote:
>> On Thu, May 28, 2015 at 8:44 AM, Emil Velikov <emil.l.velikov@gmail.com> wrote:
>>> On 25 May 2015 at 12:18, Zhou, Jammy <Jammy.Zhou@amd.com> wrote:
>>>> Hi Emil,
>>>>
>>>> Do you have chance to have a look at this new version? Thanks!
>>>>
>>> Hi guys sorry for the delay.
>>>
>>> Looking at my original email response it seems that I wasn't clear
>>> enough about my concerns. They can be looked at as independent
>>> and/or grouped up, depending on your liking:
>>>
>>> - Adding a new interface using which it's not obvious how one can
>>> simplify/ease existing related implementations.
>>> As mentioned before, the claiming duplication that already exists is
>>> more complete than this API.
>>>
>>> - No open-source software uses the interface.
>>> Do you have any patches in progress that convert project foo to
>>> using the API? Otherwise is seems like an open-source project
>>> catering after a closed source project needs.
>>
>> It's actually in preparation for upcoming open source releases.
>>
> Ack. Fair enough - the commit message did not mention any open source
> users. The follow-up response mentioned xserver and mesa for which
> this API doesn't seem like a good fit.
>
>>>
>>> - Adding new dependencies.
>>> Previously libpciaccess, now libudev. The former is used only by a
>>> single function in libdrm_intel, while the latter was optional. v3
>>> makes the use of libudev a hard requirement.
>>> Using the library in mesa has caused problems with Steam (and
>>> possibly other binary programs/games) which ships their own version
>>> of various libraries.
>>
>> Can you think of a better method for device enumeration? Rolling our
>> own seem like a waste of effort.
>>
> Implementation might be uglier (as do other parts of libdrm), but I
> can envision an API that does not require a new dependency, plus it
> works with platform devices. So I'm not sure if it qualifies as better
> or not.
>
>>
>>>
>>> Now that the patch is merged, the last issue has emerged (in a
>>> particular form). Is there any plan to convert an open-source
>>> project to use this API ?
>>
>> The first users actually will be open source they are just not ready
>> for public release just yet. We are trying to reduce our internal
>> patch queue and get stuff upstream early and thought others might be
>> interested, but if there are concerns we can revert it for now and
>> re-submit it when we are ready to release the relevant code.
>>
> If you can share some pseudo code on what the requirements are/how it
> should be used I can prep an alternative solution. A one that works
> for your usecase and existing ones. In the mean while can we revert
> it, pretty please ?
Sure, go for it. Jammy or Frank might be able to provide some pseudo code in the interim.
Alex
>
> Thanks
> Emil
Hi Frank, On 1 June 2015 at 04:06, Min, Frank <Frank.Min@amd.com> wrote: > Hi Emil, > Here is the pseudo code: > > int drmGetPciDevices(drmPciDevicePtr devSet, uint16_t vendorId) > { > If(devSet == NULL && vendorId == 0) > Probe all the graphic device and return the number of it > Else If(devSet != NULL && vendorId != 0) > Probe the specific vender graphic device and return it in the array devSet > Else If(devSet ==NULL && vendorId != 0) > Probe the specific vendorId graphic device and return the number of it > Else > Should not happen > } > I believe that you misread my original question - I requested pseudo-code of the API should be used. Thanks Emil
Hello Jammy, On 1 June 2015 at 03:12, Zhou, Jammy <Jammy.Zhou@amd.com> wrote: >> Jammy or Frank might be able to provide some pseudo code in the interim. > > I think the requirement here is quite simple. We would like to have some interface to enumerate the GPU devices on the system, and select some specific device for different purposes (i.e, rendering, computing, displaying, etc). Current libdrm interfaces are just for single device, and there is no good consideration for multiple GPUs yet. > I believe I've read the same quote earlier, although still explains the goal, rather than the requirements :'-( Can you elaborate on what exactly the interface should provide - I can think of the following: - Number of devices. - ^^ + masked by vendor id/module name. This one seem like an overkill imho.. - Flags indicating if the device has primary/control/render node. - Location (bus info) for the device(s) - PCI/platform device information. Does the above cover all the things that you have planned for the interface to provide ? Thanks Emil
> Does the above cover all the things that you have planned for the interface to provide ? Yes, exactly. That can cover what we need. Thanks. Regards, Jammy -----Original Message----- From: Emil Velikov [mailto:emil.l.velikov@gmail.com] Sent: Thursday, June 04, 2015 12:04 AM To: Zhou, Jammy Cc: Alex Deucher; Deucher, Alexander; Min, Frank; dri-devel@lists.freedesktop.org Subject: Re: [PATCH] Add device enumeration interface (v3) Hello Jammy, On 1 June 2015 at 03:12, Zhou, Jammy <Jammy.Zhou@amd.com> wrote: >> Jammy or Frank might be able to provide some pseudo code in the interim. > > I think the requirement here is quite simple. We would like to have some interface to enumerate the GPU devices on the system, and select some specific device for different purposes (i.e, rendering, computing, displaying, etc). Current libdrm interfaces are just for single device, and there is no good consideration for multiple GPUs yet. > I believe I've read the same quote earlier, although still explains the goal, rather than the requirements :'-( Can you elaborate on what exactly the interface should provide - I can think of the following: - Number of devices. - ^^ + masked by vendor id/module name. This one seem like an overkill imho.. - Flags indicating if the device has primary/control/render node. - Location (bus info) for the device(s) - PCI/platform device information. Does the above cover all the things that you have planned for the interface to provide ? Thanks Emil
diff --git a/Makefile.am b/Makefile.am index 13df80c..ffd334a 100644 --- a/Makefile.am +++ b/Makefile.am @@ -95,12 +95,15 @@ 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@ \ + @LIBUDEV_LIBS@ libdrm_la_CPPFLAGS = -I$(top_srcdir)/include/drm AM_CFLAGS = \ $(WARN_CFLAGS) \ - $(VALGRIND_CFLAGS) + $(VALGRIND_CFLAGS) \ + $(LIBUDEV_CFLAGS) libdrm_la_SOURCES = $(LIBDRM_FILES) diff --git a/xf86drm.c b/xf86drm.c index f7c45f8..d2704de 100644 --- a/xf86drm.c +++ b/xf86drm.c @@ -63,6 +63,7 @@ #include "xf86drm.h" #include "libdrm_macros.h" +#include "libudev.h" #if defined(__FreeBSD__) || defined(__FreeBSD_kernel__) || defined(__DragonFly__) #define DRM_MAJOR 145 @@ -2817,3 +2818,105 @@ 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 drmGetPciDevices(drmPciDevicePtr devSet, uint16_t vendorId) { + struct udev *udev = NULL; + struct udev_enumerate *udev_enumerate; + struct udev_list_entry *list_entry; + struct udev_device *device; + int drmDevCount = 0; + int vendor = 0; + int devid = 0; + int devclass = 0; + int subvendor = 0; + int subdevid = 0; + int revid = 0xff; + int domain = 0; + int bus = 0; + int dev = 0; + int func = 0; + char config[64] = {0}; + char node[128] = {'\0'}; + char slot[] = "0000:00:00.0"; + char *info = NULL; + int minor = 0xff; + int fd = 0; + int ret = 0; + + udev = udev_new(); + if (udev == NULL) { + fprintf(stderr, "no context\n"); + return -EINVAL; + } + udev_enumerate = udev_enumerate_new(udev); + if (udev_enumerate == NULL) + return -EINVAL; + udev_enumerate_add_match_subsystem(udev_enumerate, "drm"); + udev_enumerate_add_match_property(udev_enumerate, "DEVTYPE", +"drm_minor"); + + udev_enumerate_scan_devices(udev_enumerate); + + udev_list_entry_foreach(list_entry, udev_enumerate_get_list_entry(udev_enumerate)) { + device = udev_device_new_from_syspath(udev_enumerate_get_udev(udev_enumerate), + udev_list_entry_get_name(list_entry)); + if (device != NULL) { + info = udev_device_get_property_value(device, "MINOR"); + if (!strcmp(info, "0")) { + strcpy(node, udev_device_get_syspath(device)); + info = strstr(node, "/drm"); + strncpy(slot, info - strlen(slot), strlen(slot)); + if (sscanf(slot, "%4x:%2x:%2x.%1x", &domain, &bus, &dev, &func) != 4) { + domain = 0; + bus = 0; + dev = 0; + func = 0; + } + strcpy(node + strlen(node), "/device/config"); + + fd = open(node, O_RDONLY); + if (fd >= 0) { + ret = read(fd, config, 64); + if (ret == 64) { + vendor = config[0] + (config[1] << 8); + devid = config[2] + (config[3] << 8); + revid = config[8]; + devclass = config[9] + (config[10] << 8) + (config[11] << 16); + subdevid = config[44] + (config[45] << 8); + } + close(fd); + } + + if((vendorId == 0) || (vendorId == vendor)) { + if(devSet != NULL) { + devSet[drmDevCount].domain = domain; + devSet[drmDevCount].bus = bus; + devSet[drmDevCount].dev = dev; + devSet[drmDevCount].func = func; + devSet[drmDevCount].vendor_id = vendor; + devSet[drmDevCount].device_id = devid; + devSet[drmDevCount].subdevice_id = subdevid; + devSet[drmDevCount].subvendor_id = subvendor; + devSet[drmDevCount].revision_id = revid; + } + drmDevCount++; + } + } + } + udev_device_unref(device); + } + udev_enumerate_unref(udev_enumerate); + udev_unref(udev); + + return drmDevCount; +} diff --git a/xf86drm.h b/xf86drm.h index 40c55c9..2610934 100644 --- a/xf86drm.h +++ b/xf86drm.h @@ -342,6 +342,24 @@ typedef struct _drmSetVersion { int drm_dd_minor; } drmSetVersion, *drmSetVersionPtr; +/** + * Structure to a general pci gpu device + * + * \sa drmGetDevices() + * +*/ +typedef struct _drmPciDevice { + uint16_t domain; + uint8_t bus; + uint8_t dev; + uint8_t func; + uint16_t vendor_id; + uint16_t device_id; + uint16_t subvendor_id; + uint16_t subdevice_id; + uint8_t revision_id; +} drmPciDevice, *drmPciDevicePtr; + #define __drm_dummy_lock(lock) (*(__volatile__ unsigned int *)lock) #define DRM_LOCK_HELD 0x80000000U /**< Hardware lock is held */ @@ -552,6 +570,7 @@ do { register unsigned int __old __asm("o0"); \ /* General user-level programmer's API: unprivileged */ extern int drmAvailable(void); extern int drmOpen(const char *name, const char *busid); +extern int drmGetPciDevices(drmPciDevicePtr devSet, uint16_t vendorId); #define DRM_NODE_PRIMARY 0 #define DRM_NODE_CONTROL 1