| Message ID | 20180427113117.19066-1-robert.foss@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 27 April 2018 at 12:31, Robert Foss <robert.foss@collabora.com> wrote: > drmHandleMatch is intended to allow for userspace to filter out > devices that it does not want to open. > > Opening specific devices using paths alone is not a reliable due to > probing order. This function intends to provide a mechanism > for filtering out devices that don't fit what you need using an > extensible set of filters. > > drm_match_key_t is intended to be extended with whatever > filter that would come in handy down the line. > > As a catch-all filter, the DRM_MATCH_FUNCTION was included > which allows the caller to filter based on an arbitrary function. > > An function pointer filter could of course filter based on > anything. But for the sake of convenience a few other simple > filters have been included. > > If the function pointer filter ends up being called with a > boilerplate fp by mutliple libdrm users, perhaps that funtion > could be moved into libdrm at a future date. > > Signed-off-by: Robert Foss <robert.foss@collabora.com> > --- > > This patch implements a simple function for matching DRM device FDs > against the desired properties of a device that you are looking for. > > The discussion that led to this series can be found here: > https://lists.freedesktop.org/archives/mesa-dev/2018-January/183878.html > > The RFC can be found here: > https://www.spinics.net/lists/dri-devel/msg172398.html > > Since the RFC I opted to rework the patch to be more extensible. > The previous implementation would have been problematic if the > drmVersion or drmDevice structs ever needed to be changed or > removed. Or indeed if more properties were to become interesting. > > As it is now, it is basially implemented as per Daniel Stones suggestion in > the RFC discussion. > > Changes since RFC: > - Reworked proposal to be not be based on structs in order to be more > flexible. > - Now uses filters of different types. > - Caller can supply any number of predefined and function pointer > filter. > > xf86drm.h | 24 ++++++++++++++++++++ > xf86drmMode.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 94 insertions(+) > Instead of spending too much time thinking of a future-proof API, can I suggest getting a handful of users first. And only after that lifting it to a helper - If needed. Here are some examples - anv [1] and xf86-video-amdgpu [2]. Note latter is not merged yet. Practically any implementation is as trivial as: ret = drmGetDevices2(...) for (i = 0; i < ret; i++) { compare_device_specifics(...) } drmFreeDevices(...) What do you guys think? Emil [1] https://cgit.freedesktop.org/mesa/mesa/tree/src/intel/vulkan/anv_device.c#n615 [2] https://lists.freedesktop.org/archives/amd-gfx/2018-April/020936.html
Hey Emil, On 27.04.2018 15:48, Emil Velikov wrote: > On 27 April 2018 at 12:31, Robert Foss <robert.foss@collabora.com> wrote: >> drmHandleMatch is intended to allow for userspace to filter out >> devices that it does not want to open. >> >> Opening specific devices using paths alone is not a reliable due to >> probing order. This function intends to provide a mechanism >> for filtering out devices that don't fit what you need using an >> extensible set of filters. >> >> drm_match_key_t is intended to be extended with whatever >> filter that would come in handy down the line. >> >> As a catch-all filter, the DRM_MATCH_FUNCTION was included >> which allows the caller to filter based on an arbitrary function. >> >> An function pointer filter could of course filter based on >> anything. But for the sake of convenience a few other simple >> filters have been included. >> >> If the function pointer filter ends up being called with a >> boilerplate fp by mutliple libdrm users, perhaps that funtion >> could be moved into libdrm at a future date. >> >> Signed-off-by: Robert Foss <robert.foss@collabora.com> >> --- >> >> This patch implements a simple function for matching DRM device FDs >> against the desired properties of a device that you are looking for. >> >> The discussion that led to this series can be found here: >> https://lists.freedesktop.org/archives/mesa-dev/2018-January/183878.html >> >> The RFC can be found here: >> https://www.spinics.net/lists/dri-devel/msg172398.html >> >> Since the RFC I opted to rework the patch to be more extensible. >> The previous implementation would have been problematic if the >> drmVersion or drmDevice structs ever needed to be changed or >> removed. Or indeed if more properties were to become interesting. >> >> As it is now, it is basially implemented as per Daniel Stones suggestion in >> the RFC discussion. >> >> Changes since RFC: >> - Reworked proposal to be not be based on structs in order to be more >> flexible. >> - Now uses filters of different types. >> - Caller can supply any number of predefined and function pointer >> filter. >> >> xf86drm.h | 24 ++++++++++++++++++++ >> xf86drmMode.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 94 insertions(+) >> > Instead of spending too much time thinking of a future-proof API, can > I suggest getting a handful of users first. > And only after that lifting it to a helper - If needed. > > Here are some examples - anv [1] and xf86-video-amdgpu [2]. Note > latter is not merged yet. > Practically any implementation is as trivial as: > > ret = drmGetDevices2(...) > for (i = 0; i < ret; i++) { > compare_device_specifics(...) > } > drmFreeDevices(...) > > What do you guys think? I was hoping to have all probing functionality eventually implemented in libdrm, since it is duplicated in multiple codebases. Where this function&patch would be the first step. But there seems to be some friction to this idea, so maybe it dropping this patch is the way to go forward. > Emil > > [1] https://cgit.freedesktop.org/mesa/mesa/tree/src/intel/vulkan/anv_device.c#n615 > [2] https://lists.freedesktop.org/archives/amd-gfx/2018-April/020936.html > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel >
diff --git a/xf86drm.h b/xf86drm.h index 7773d71a8030..71bf39baf9b2 100644 --- a/xf86drm.h +++ b/xf86drm.h @@ -863,6 +863,30 @@ extern int drmGetDevices2(uint32_t flags, drmDevicePtr devices[], int max_device extern int drmDevicesEqual(drmDevicePtr a, drmDevicePtr b); +typedef enum drm_match_key { + /* Match against DRM_NODE_{PRIMARY,RENDER,...} type */ + DRM_MATCH_NODE_TYPE = 1, + DRM_MATCH_DRIVER_NAME = 2, + DRM_MATCH_BUS_PCI_VENDOR = 3, + DRM_MATCH_FUNCTION = 4, +} drm_match_key_t; + +typedef struct drm_match_func { + void *data; + int (*fp)(int, void*); /* Intended arguments are fp(fd, data) */ +} drm_match_func_t; + +typedef struct drm_match { + drm_match_key_t type; + union { + int s; + uint16_t u16; + char *str; + drm_match_func_t func; + }; +} drm_match_t; +extern int drmHandleMatch(int fd, drm_match_t *filters, int nbr_filters); + extern int drmSyncobjCreate(int fd, uint32_t flags, uint32_t *handle); extern int drmSyncobjDestroy(int fd, uint32_t handle); extern int drmSyncobjHandleToFD(int fd, uint32_t handle, int *obj_fd); diff --git a/xf86drmMode.c b/xf86drmMode.c index 9a15b5e78dda..28446e7d4ac6 100644 --- a/xf86drmMode.c +++ b/xf86drmMode.c @@ -946,6 +946,76 @@ int drmHandleEvent(int fd, drmEventContextPtr evctx) return 0; } +int drmHandleMatch(int fd, drm_match_t *filters, int nbr_filters) +{ + if (fd < 0) + goto error; + + if (nbr_filters > 0 && filters == NULL) + goto error; + + drmVersionPtr ver = drmGetVersion(fd); + if (!ver) + goto fail; + + drmDevicePtr dev = NULL; + if (drmGetDevice2(fd, 0, &dev) != 0) { + goto fail; + } + + for (int i = 0; i < nbr_filters; i++) { + drm_match_t *f = &filters[i]; + switch (f->type) { + case DRM_MATCH_NODE_TYPE: + if (!(dev->available_nodes & (1 << f->s))) + goto fail; + break; + case DRM_MATCH_DRIVER_NAME: + if (!f->str) + goto error; + + /* This bypass is used by when the driver name is used + by the Android property_get() func, when it hasn't found + the property and the string is empty as a result. */ + if (strlen(f->str) == 0) + continue; + + if (strncmp(ver->name, f->str, strlen(ver->name))) + goto fail; + break; + case DRM_MATCH_BUS_PCI_VENDOR: + if (dev->bustype != DRM_BUS_PCI) + goto fail; + if (dev->deviceinfo.pci->vendor_id != f->u16) + goto fail; + break; + case DRM_MATCH_FUNCTION: + if (!f->func.fp) + goto error; + int (*fp)(int, void*) = f->func.fp; + void *data = f->func.data; + if (!fp(fd, data)) + goto fail; + break; + default: + goto error; + } + } + +success: + drmFreeVersion(ver); + drmFreeDevice(&dev); + return 0; +error: + drmFreeVersion(ver); + drmFreeDevice(&dev); + return -EINVAL; +fail: + drmFreeVersion(ver); + drmFreeDevice(&dev); + return 1; +} + int drmModePageFlip(int fd, uint32_t crtc_id, uint32_t fb_id, uint32_t flags, void *user_data) {
drmHandleMatch is intended to allow for userspace to filter out devices that it does not want to open. Opening specific devices using paths alone is not a reliable due to probing order. This function intends to provide a mechanism for filtering out devices that don't fit what you need using an extensible set of filters. drm_match_key_t is intended to be extended with whatever filter that would come in handy down the line. As a catch-all filter, the DRM_MATCH_FUNCTION was included which allows the caller to filter based on an arbitrary function. An function pointer filter could of course filter based on anything. But for the sake of convenience a few other simple filters have been included. If the function pointer filter ends up being called with a boilerplate fp by mutliple libdrm users, perhaps that funtion could be moved into libdrm at a future date. Signed-off-by: Robert Foss <robert.foss@collabora.com> --- This patch implements a simple function for matching DRM device FDs against the desired properties of a device that you are looking for. The discussion that led to this series can be found here: https://lists.freedesktop.org/archives/mesa-dev/2018-January/183878.html The RFC can be found here: https://www.spinics.net/lists/dri-devel/msg172398.html Since the RFC I opted to rework the patch to be more extensible. The previous implementation would have been problematic if the drmVersion or drmDevice structs ever needed to be changed or removed. Or indeed if more properties were to become interesting. As it is now, it is basially implemented as per Daniel Stones suggestion in the RFC discussion. Changes since RFC: - Reworked proposal to be not be based on structs in order to be more flexible. - Now uses filters of different types. - Caller can supply any number of predefined and function pointer filter. xf86drm.h | 24 ++++++++++++++++++++ xf86drmMode.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 94 insertions(+)