| Message ID | 1424694176-3210-1-git-send-email-emil.l.velikov@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Emil, On 23/02/15 12:22, Emil Velikov wrote: > Currently most places assume reliable master <> render node mapping. > Although this may work in some cases, it is not correct. > > Add a couple of helpers that hide the details and provide the name of > the master/render device name, given an render/master FD. > > v2: > - Rename Device and Primary to Master (aka the /dev/dri/cardX device). > - Check for the file via readdir_r() rather than stat(). > - Wrap the check into a single function. > - Return NULL for non-linux platforms. > > Cc: Frank Binns <frank.binns@imgtec.com> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > Cc: David Herrmann <dh.herrmann@googlemail.com> > Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com> > --- > xf86drm.c | 82 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > xf86drm.h | 3 +++ > 2 files changed, 85 insertions(+) > > diff --git a/xf86drm.c b/xf86drm.c > index e117bc6..d4a4dc6 100644 > --- a/xf86drm.c > +++ b/xf86drm.c > @@ -40,6 +40,8 @@ > #include <string.h> > #include <strings.h> > #include <ctype.h> > +#include <dirent.h> > +#include <stddef.h> > #include <fcntl.h> > #include <errno.h> > #include <signal.h> > @@ -522,6 +524,20 @@ static int drmGetMinorType(int minor) > } > } > > +static const char *drmGetMinorName(int type) > +{ > + switch (type) { > + case DRM_NODE_PRIMARY: > + return "card"; > + case DRM_NODE_CONTROL: > + return "controlD"; > + case DRM_NODE_RENDER: > + return "renderD"; > + default: > + return NULL; > + } > +} > + > /** > * Open the device by bus ID. > * > @@ -2736,3 +2752,69 @@ int drmPrimeFDToHandle(int fd, int prime_fd, uint32_t *handle) > return 0; > } > > +static char *drmGetMinorNameForFD(int fd, int type) > +{ > +#ifdef __linux__ > + DIR *sysdir; > + struct dirent *pent, *ent; > + struct stat sbuf; > + const char *name = drmGetMinorName(type); > + const int len = strlen(name); This will cause a segfault if 'name' is NULL. > + char dev_name[64], buf[64]; > + long name_max; > + int maj, min; > + > + if (!name) > + return NULL; > + > + if (fstat(fd, &sbuf)) > + return NULL; > + > + maj = major(sbuf.st_rdev); > + min = minor(sbuf.st_rdev); > + > + if (maj != DRM_MAJOR || !S_ISCHR(sbuf.st_mode)) > + return NULL; > + > + snprintf(buf, sizeof(buf), "/sys/dev/char/%d:%d/device/drm", maj, min); > + > + sysdir = opendir(buf); > + if (!sysdir) > + return NULL; > + > + name_max = fpathconf(dirfd(sysdir), _PC_NAME_MAX); > + if (name_max == -1) > + goto out_close_dir; > + > + pent = malloc(offsetof(struct dirent, d_name) + name_max + 1); > + if (pent == NULL) > + goto out_close_dir; > + > + while (readdir_r(sysdir, pent, &ent) == 0 && ent != NULL) { > + if (strncmp(ent->d_name, name, len) == 0) { > + free(pent); > + closedir(sysdir); > + > + snprintf(dev_name, sizeof(dev_name), DRM_DIR_NAME "/%s", > + ent->d_name); > + return strdup(dev_name); > + } > + } > + > + free(pent); > + > +out_close_dir: > + closedir(sysdir); > +#endif > + return NULL; > +} > + > +char *drmGetMasterNameFromRenderFD(int fd) I think drmGetPrimaryDeviceNameFromFd would be more appropriate given the node type is 'primary', the type of the fd doesn't matter afaics and for consistency with other drmGet* functions. However, given that's a bit of a mouthful I guess the 'Device' part could be dropped. > +{ > + return drmGetMinorNameForFD(fd, DRM_NODE_PRIMARY); > +} > + > +char *drmGetRenderNameFromMasterFD(int fd) As above, maybe drmGetRenderDeviceNameFromFd? With those things changed/fixed you can have a: Reviewed-by: Frank Binns <frank.binns@imgtec.com> Thanks Frank > +{ > + return drmGetMinorNameForFD(fd, DRM_NODE_RENDER); > +} > diff --git a/xf86drm.h b/xf86drm.h > index afd38a1..5fdf27b 100644 > --- a/xf86drm.h > +++ b/xf86drm.h > @@ -749,6 +749,9 @@ extern int drmGetNodeTypeFromFd(int fd); > extern int drmPrimeHandleToFD(int fd, uint32_t handle, uint32_t flags, int *prime_fd); > extern int drmPrimeFDToHandle(int fd, int prime_fd, uint32_t *handle); > > +extern char *drmGetRenderNameFromMasterFD(int fd); > +extern char *drmGetMasterNameFromRenderFD(int fd); > + > #if defined(__cplusplus) || defined(c_plusplus) > } > #endif
On 23 February 2015 at 14:35, Frank Binns <frank.binns@imgtec.com> wrote: > Hi Emil, > > On 23/02/15 12:22, Emil Velikov wrote: >> Currently most places assume reliable master <> render node mapping. >> Although this may work in some cases, it is not correct. >> >> Add a couple of helpers that hide the details and provide the name of >> the master/render device name, given an render/master FD. >> >> v2: >> - Rename Device and Primary to Master (aka the /dev/dri/cardX device). >> - Check for the file via readdir_r() rather than stat(). >> - Wrap the check into a single function. >> - Return NULL for non-linux platforms. >> >> Cc: Frank Binns <frank.binns@imgtec.com> >> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> >> Cc: David Herrmann <dh.herrmann@googlemail.com> >> Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com> >> --- >> xf86drm.c | 82 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ >> xf86drm.h | 3 +++ >> 2 files changed, 85 insertions(+) >> >> diff --git a/xf86drm.c b/xf86drm.c >> index e117bc6..d4a4dc6 100644 >> --- a/xf86drm.c >> +++ b/xf86drm.c >> @@ -40,6 +40,8 @@ >> #include <string.h> >> #include <strings.h> >> #include <ctype.h> >> +#include <dirent.h> >> +#include <stddef.h> >> #include <fcntl.h> >> #include <errno.h> >> #include <signal.h> >> @@ -522,6 +524,20 @@ static int drmGetMinorType(int minor) >> } >> } >> >> +static const char *drmGetMinorName(int type) >> +{ >> + switch (type) { >> + case DRM_NODE_PRIMARY: >> + return "card"; >> + case DRM_NODE_CONTROL: >> + return "controlD"; >> + case DRM_NODE_RENDER: >> + return "renderD"; >> + default: >> + return NULL; >> + } >> +} >> + >> /** >> * Open the device by bus ID. >> * >> @@ -2736,3 +2752,69 @@ int drmPrimeFDToHandle(int fd, int prime_fd, uint32_t *handle) >> return 0; >> } >> >> +static char *drmGetMinorNameForFD(int fd, int type) >> +{ >> +#ifdef __linux__ >> + DIR *sysdir; >> + struct dirent *pent, *ent; >> + struct stat sbuf; >> + const char *name = drmGetMinorName(type); >> + const int len = strlen(name); > This will cause a segfault if 'name' is NULL. > >> + char dev_name[64], buf[64]; >> + long name_max; >> + int maj, min; >> + >> + if (!name) >> + return NULL; >> + >> + if (fstat(fd, &sbuf)) >> + return NULL; >> + >> + maj = major(sbuf.st_rdev); >> + min = minor(sbuf.st_rdev); >> + >> + if (maj != DRM_MAJOR || !S_ISCHR(sbuf.st_mode)) >> + return NULL; >> + >> + snprintf(buf, sizeof(buf), "/sys/dev/char/%d:%d/device/drm", maj, min); >> + >> + sysdir = opendir(buf); >> + if (!sysdir) >> + return NULL; >> + >> + name_max = fpathconf(dirfd(sysdir), _PC_NAME_MAX); >> + if (name_max == -1) >> + goto out_close_dir; >> + >> + pent = malloc(offsetof(struct dirent, d_name) + name_max + 1); >> + if (pent == NULL) >> + goto out_close_dir; >> + >> + while (readdir_r(sysdir, pent, &ent) == 0 && ent != NULL) { >> + if (strncmp(ent->d_name, name, len) == 0) { >> + free(pent); >> + closedir(sysdir); >> + >> + snprintf(dev_name, sizeof(dev_name), DRM_DIR_NAME "/%s", >> + ent->d_name); >> + return strdup(dev_name); >> + } >> + } >> + >> + free(pent); >> + >> +out_close_dir: >> + closedir(sysdir); >> +#endif >> + return NULL; >> +} >> + >> +char *drmGetMasterNameFromRenderFD(int fd) > I think drmGetPrimaryDeviceNameFromFd would be more appropriate given > the node type is 'primary', Most places that I've seen call "/dev/dri/cardX" master node, although with the introduction of DRM_NODE_PRIMARY I believe your suggestion may be better. > the type of the fd doesn't matter afaics and > for consistency with other drmGet* functions. However, given that's a > bit of a mouthful I guess the 'Device' part could be dropped. > I was thinking about this initially then decided to keep the Render/MasterFD explicit. Mainly because I'm don't know how cumbersome the *BSD implementation will be. But with that said it none of the BSD guys objects within 1-2 weeks I'll go with your suggestion. Thanks Emil
diff --git a/xf86drm.c b/xf86drm.c index e117bc6..d4a4dc6 100644 --- a/xf86drm.c +++ b/xf86drm.c @@ -40,6 +40,8 @@ #include <string.h> #include <strings.h> #include <ctype.h> +#include <dirent.h> +#include <stddef.h> #include <fcntl.h> #include <errno.h> #include <signal.h> @@ -522,6 +524,20 @@ static int drmGetMinorType(int minor) } } +static const char *drmGetMinorName(int type) +{ + switch (type) { + case DRM_NODE_PRIMARY: + return "card"; + case DRM_NODE_CONTROL: + return "controlD"; + case DRM_NODE_RENDER: + return "renderD"; + default: + return NULL; + } +} + /** * Open the device by bus ID. * @@ -2736,3 +2752,69 @@ int drmPrimeFDToHandle(int fd, int prime_fd, uint32_t *handle) return 0; } +static char *drmGetMinorNameForFD(int fd, int type) +{ +#ifdef __linux__ + DIR *sysdir; + struct dirent *pent, *ent; + struct stat sbuf; + const char *name = drmGetMinorName(type); + const int len = strlen(name); + char dev_name[64], buf[64]; + long name_max; + int maj, min; + + if (!name) + return NULL; + + if (fstat(fd, &sbuf)) + return NULL; + + maj = major(sbuf.st_rdev); + min = minor(sbuf.st_rdev); + + if (maj != DRM_MAJOR || !S_ISCHR(sbuf.st_mode)) + return NULL; + + snprintf(buf, sizeof(buf), "/sys/dev/char/%d:%d/device/drm", maj, min); + + sysdir = opendir(buf); + if (!sysdir) + return NULL; + + name_max = fpathconf(dirfd(sysdir), _PC_NAME_MAX); + if (name_max == -1) + goto out_close_dir; + + pent = malloc(offsetof(struct dirent, d_name) + name_max + 1); + if (pent == NULL) + goto out_close_dir; + + while (readdir_r(sysdir, pent, &ent) == 0 && ent != NULL) { + if (strncmp(ent->d_name, name, len) == 0) { + free(pent); + closedir(sysdir); + + snprintf(dev_name, sizeof(dev_name), DRM_DIR_NAME "/%s", + ent->d_name); + return strdup(dev_name); + } + } + + free(pent); + +out_close_dir: + closedir(sysdir); +#endif + return NULL; +} + +char *drmGetMasterNameFromRenderFD(int fd) +{ + return drmGetMinorNameForFD(fd, DRM_NODE_PRIMARY); +} + +char *drmGetRenderNameFromMasterFD(int fd) +{ + return drmGetMinorNameForFD(fd, DRM_NODE_RENDER); +} diff --git a/xf86drm.h b/xf86drm.h index afd38a1..5fdf27b 100644 --- a/xf86drm.h +++ b/xf86drm.h @@ -749,6 +749,9 @@ extern int drmGetNodeTypeFromFd(int fd); extern int drmPrimeHandleToFD(int fd, uint32_t handle, uint32_t flags, int *prime_fd); extern int drmPrimeFDToHandle(int fd, int prime_fd, uint32_t *handle); +extern char *drmGetRenderNameFromMasterFD(int fd); +extern char *drmGetMasterNameFromRenderFD(int fd); + #if defined(__cplusplus) || defined(c_plusplus) } #endif
Currently most places assume reliable master <> render node mapping. Although this may work in some cases, it is not correct. Add a couple of helpers that hide the details and provide the name of the master/render device name, given an render/master FD. v2: - Rename Device and Primary to Master (aka the /dev/dri/cardX device). - Check for the file via readdir_r() rather than stat(). - Wrap the check into a single function. - Return NULL for non-linux platforms. Cc: Frank Binns <frank.binns@imgtec.com> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: David Herrmann <dh.herrmann@googlemail.com> Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com> --- xf86drm.c | 82 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ xf86drm.h | 3 +++ 2 files changed, 85 insertions(+)