[PATCH libdrm 1/5] xf86drm: implement drmGetMinorNameForFD for non-sysfs

[Emil Velikov]

On 30 November 2016 at 00:15, Jonathan Gray <jsg at 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 at 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 at 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