| Message ID | 20161126004034.53376-1-jsg@jsg.id.au (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 26 November 2016 at 00:40, Jonathan Gray <jsg@jsg.id.au> wrote: > Implement drmGetMinorNameForFD for systems without sysfs by > adapting drm_get_device_name_for_fd() from the Mesa loader. > > Signed-off-by: Jonathan Gray <jsg@jsg.id.au> > --- > xf86drm.c | 20 +++++++++++++++++++- > 1 file changed, 19 insertions(+), 1 deletion(-) > > diff --git a/xf86drm.c b/xf86drm.c > index ed924a7..216220c 100644 > --- a/xf86drm.c > +++ b/xf86drm.c > @@ -2818,7 +2818,25 @@ static char *drmGetMinorNameForFD(int fd, int type) > out_close_dir: > closedir(sysdir); > #else > -#warning "Missing implementation of drmGetMinorNameForFD" > + struct stat sbuf; > + unsigned int maj, min; > + char buf[PATH_MAX + 1]; > + int n; > + > + 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; > + > + n = snprintf(buf, sizeof(buf), DRM_DEV_NAME, DRM_DIR_NAME, min); > + if (n == -1 || n >= sizeof(buf)) > + return NULL; > + > + return strdup(buf); Doesn't look too good I'm afraid: - you ignore the node type, making the whole helper and API that depends on it useless. Note: mesa wants to know the render node name for the given fd. We can replace with drmGetDevice but I'd like to check the double-auth [and related fun] trimming things down before changing things. - implementation seems identical to drmGetDeviceNameFromFd(). Barring a few trivial bits of course. Speaking of which there is drmGetDeviceNameFromFd2 which attributes for any node type(s) - the present primary, control and render plus any future ones. I'm leaning towards using it in the next (or one after) version in mesa. Have you and fellow OpenBSD developers considered render nodes. Do you have any preliminary ideas how it will be exposed, such that you can build a comprehensive interface here ? Thanks Emil
On Tue, Nov 29, 2016 at 07:22:34PM +0000, Emil Velikov wrote: > On 26 November 2016 at 00:40, Jonathan Gray <jsg@jsg.id.au> wrote: > > Implement drmGetMinorNameForFD for systems without sysfs by > > adapting drm_get_device_name_for_fd() from the Mesa loader. > > > > Signed-off-by: Jonathan Gray <jsg@jsg.id.au> > > --- > > xf86drm.c | 20 +++++++++++++++++++- > > 1 file changed, 19 insertions(+), 1 deletion(-) > > > > diff --git a/xf86drm.c b/xf86drm.c > > index ed924a7..216220c 100644 > > --- a/xf86drm.c > > +++ b/xf86drm.c > > @@ -2818,7 +2818,25 @@ static char *drmGetMinorNameForFD(int fd, int type) > > out_close_dir: > > closedir(sysdir); > > #else > > -#warning "Missing implementation of drmGetMinorNameForFD" > > + struct stat sbuf; > > + unsigned int maj, min; > > + char buf[PATH_MAX + 1]; > > + int n; > > + > > + 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; > > + > > + n = snprintf(buf, sizeof(buf), DRM_DEV_NAME, DRM_DIR_NAME, min); > > + if (n == -1 || n >= sizeof(buf)) > > + return NULL; > > + > > + return strdup(buf); > Doesn't look too good I'm afraid: > - you ignore the node type, making the whole helper and API that > depends on it useless. > Note: mesa wants to know the render node name for the given fd. We can > replace with drmGetDevice but I'd like to check the double-auth [and > related fun] trimming things down before changing things. It could be changed to handle the type along the lines of base = drmGetMinorBase(type); if (min < base) return -EINVAL; switch (type) { case DRM_NODE_PRIMARY: dev_name = DRM_DEV_NAME; break; case DRM_NODE_CONTROL: dev_name = DRM_CONTROL_DEV_NAME; break; case DRM_NODE_RENDER: dev_name = DRM_RENDER_DEV_NAME; break; default: return -EINVAL; }; n = snprintf(buf, sizeof(buf), dev_name, DRM_DIR_NAME, min - base); > > - implementation seems identical to drmGetDeviceNameFromFd(). Barring > a few trivial bits of course. > Speaking of which there is drmGetDeviceNameFromFd2 which attributes > for any node type(s) - the present primary, control and render plus > any future ones. > I'm leaning towards using it in the next (or one after) version in mesa. > > Have you and fellow OpenBSD developers considered render nodes. Do you > have any preliminary ideas how it will be exposed, such that you can > build a comprehensive interface here ? > > Thanks > Emil
On 30 November 2016 at 00:15, Jonathan Gray <jsg@jsg.id.au> wrote: > On Tue, Nov 29, 2016 at 07:22:34PM +0000, Emil Velikov wrote: >> On 26 November 2016 at 00:40, Jonathan Gray <jsg@jsg.id.au> wrote: >> > Implement drmGetMinorNameForFD for systems without sysfs by >> > adapting drm_get_device_name_for_fd() from the Mesa loader. >> > >> > Signed-off-by: Jonathan Gray <jsg@jsg.id.au> >> > --- >> > xf86drm.c | 20 +++++++++++++++++++- >> > 1 file changed, 19 insertions(+), 1 deletion(-) >> > >> > diff --git a/xf86drm.c b/xf86drm.c >> > index ed924a7..216220c 100644 >> > --- a/xf86drm.c >> > +++ b/xf86drm.c >> > @@ -2818,7 +2818,25 @@ static char *drmGetMinorNameForFD(int fd, int type) >> > out_close_dir: >> > closedir(sysdir); >> > #else >> > -#warning "Missing implementation of drmGetMinorNameForFD" >> > + struct stat sbuf; >> > + unsigned int maj, min; >> > + char buf[PATH_MAX + 1]; >> > + int n; >> > + >> > + 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; >> > + >> > + n = snprintf(buf, sizeof(buf), DRM_DEV_NAME, DRM_DIR_NAME, min); >> > + if (n == -1 || n >= sizeof(buf)) >> > + return NULL; >> > + >> > + return strdup(buf); >> Doesn't look too good I'm afraid: >> - you ignore the node type, making the whole helper and API that >> depends on it useless. >> Note: mesa wants to know the render node name for the given fd. We can >> replace with drmGetDevice but I'd like to check the double-auth [and >> related fun] trimming things down before changing things. > > It could be changed to handle the type along the lines of > > base = drmGetMinorBase(type); > > if (min < base) > return -EINVAL; > > switch (type) { > case DRM_NODE_PRIMARY: > dev_name = DRM_DEV_NAME; > break; > case DRM_NODE_CONTROL: > dev_name = DRM_CONTROL_DEV_NAME; > break; > case DRM_NODE_RENDER: > dev_name = DRM_RENDER_DEV_NAME; > break; > default: > return -EINVAL; > }; > > n = snprintf(buf, sizeof(buf), dev_name, DRM_DIR_NAME, min - base); > Far better, thanks ! >> >> - implementation seems identical to drmGetDeviceNameFromFd(). Barring >> a few trivial bits of course. >> Speaking of which there is drmGetDeviceNameFromFd2 which attributes >> for any node type(s) - the present primary, control and render plus >> any future ones. >> I'm leaning towards using it in the next (or one after) version in mesa. >> >> Have you and fellow OpenBSD developers considered render nodes. Do you >> have any preliminary ideas how it will be exposed, such that you can >> build a comprehensive interface here ? >> ... and to answer my question - the control/render node names are already set/defined for OpenBSD. Memory is failing :-\ -Emil
diff --git a/xf86drm.c b/xf86drm.c index ed924a7..216220c 100644 --- a/xf86drm.c +++ b/xf86drm.c @@ -2818,7 +2818,25 @@ static char *drmGetMinorNameForFD(int fd, int type) out_close_dir: closedir(sysdir); #else -#warning "Missing implementation of drmGetMinorNameForFD" + struct stat sbuf; + unsigned int maj, min; + char buf[PATH_MAX + 1]; + int n; + + 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; + + n = snprintf(buf, sizeof(buf), DRM_DEV_NAME, DRM_DIR_NAME, min); + if (n == -1 || n >= sizeof(buf)) + return NULL; + + return strdup(buf); #endif return NULL; }
Implement drmGetMinorNameForFD for systems without sysfs by adapting drm_get_device_name_for_fd() from the Mesa loader. Signed-off-by: Jonathan Gray <jsg@jsg.id.au> --- xf86drm.c | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-)