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

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

[Jonathan Gray]

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);
 #endif
     return NULL;
 }
-- 
2.10.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH libdrm 2/5] xf86drm: implement drmParseSubsystemType for OpenBSD

[Jonathan Gray]

Implement drmParseSubsystemType for OpenBSD by always returning
DRM_BUS_PCI.  No non-pci drm drivers are in the kernel and this is
unlikely to change anytime soon as the existing ones aren't permissively
licensed.

Signed-off-by: Jonathan Gray <jsg@jsg.id.au>
---
 xf86drm.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/xf86drm.c b/xf86drm.c
index 216220c..b355c83 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -2872,6 +2872,8 @@ static int drmParseSubsystemType(int maj, int min)
         return DRM_BUS_PCI;
 
     return -EINVAL;
+#elif defined(__OpenBSD__)
+	return DRM_BUS_PCI;
 #else
 #warning "Missing implementation of drmParseSubsystemType"
     return -EINVAL;
-- 
2.10.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH libdrm 3/5] xf86drm: implement drmParsePciDeviceInfo for OpenBSD

[Jonathan Gray]

Implement drmParsePciDeviceInfo for OpenBSD by using the new
DRM_IOCTL_GET_PCIINFO ioctl.

Signed-off-by: Jonathan Gray <jsg@jsg.id.au>
---
 xf86drm.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/xf86drm.c b/xf86drm.c
index b355c83..581527b 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -102,6 +102,26 @@
 #define DRM_MAJOR 226 /* Linux */
 #endif
 
+#ifdef __OpenBSD__
+
+#define X_PRIVSEP
+
+struct drm_pciinfo {
+	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;
+};
+
+#define DRM_IOCTL_GET_PCIINFO	DRM_IOR(0x15, struct drm_pciinfo)
+
+#endif
+
 #define DRM_MSG_VERBOSITY 3
 
 #define memclear(s) memset(&s, 0, sizeof(s))
@@ -2991,6 +3011,37 @@ static int drmParsePciDeviceInfo(const char *d_name,
     device->subdevice_id = config[46] | (config[47] << 8);
 
     return 0;
+#elif defined(__OpenBSD__)
+    struct drm_pciinfo pinfo;
+    char buf[PATH_MAX + 1];
+    int fd, n;
+
+    n = snprintf(buf, sizeof(buf), "%s/%s", DRM_DIR_NAME, d_name);
+    if (n == -1 || n >= sizeof(buf))
+        return -errno;
+
+#ifndef X_PRIVSEP
+    fd = open(buf, O_RDWR, 0);
+#else
+    fd = priv_open_device(buf);
+#endif
+
+    if (fd < 0)
+        return -errno;
+
+    if (drmIoctl(fd, DRM_IOCTL_GET_PCIINFO, &pinfo)) {
+        close(fd);
+        return -errno;
+    }
+    close(fd);
+
+    device->vendor_id = pinfo.vendor_id;
+    device->device_id = pinfo.device_id;
+    device->revision_id = pinfo.revision_id;
+    device->subvendor_id = pinfo.subvendor_id;
+    device->subdevice_id = pinfo.subdevice_id;
+
+    return 0;
 #else
 #warning "Missing implementation of drmParsePciDeviceInfo"
     return -EINVAL;
-- 
2.10.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH libdrm 4/5] xf86drm: implement drmParsePciBusInfo for OpenBSD

[Jonathan Gray]

Implement drmParsePciBusInfo for OpenBSD by using the new
DRM_IOCTL_GET_PCIINFO ioctl.

Signed-off-by: Jonathan Gray <jsg@jsg.id.au>
---
 xf86drm.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/xf86drm.c b/xf86drm.c
index 581527b..2a60b2e 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -2936,6 +2936,26 @@ static int drmParsePciBusInfo(int maj, int min, drmPciBusInfoPtr info)
     info->func = func;
 
     return 0;
+#elif defined(__OpenBSD__)
+    struct drm_pciinfo pinfo;
+    int fd;
+
+    fd = drmOpenMinor(min, 0, DRM_NODE_PRIMARY);
+    if (fd < 0)
+        return -errno;
+
+    if (drmIoctl(fd, DRM_IOCTL_GET_PCIINFO, &pinfo)) {
+        close(fd);
+        return -errno;
+    }
+    close(fd);
+
+    info->domain = pinfo.domain;
+    info->bus = pinfo.bus;
+    info->dev = pinfo.dev;
+    info->func = pinfo.func;
+
+    return 0;
 #else
 #warning "Missing implementation of drmParsePciBusInfo"
     return -EINVAL;
-- 
2.10.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH libdrm 5/5] xf86drm: implement an OpenBSD specific drmGetDevice

[Jonathan Gray]

This avoids walking all of /dev and directly maps the fd to a path to a
primary node.

Signed-off-by: Jonathan Gray <jsg@jsg.id.au>
---
 xf86drm.c | 41 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/xf86drm.c b/xf86drm.c
index 2a60b2e..a4b2506 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -3175,6 +3175,46 @@ static void drmFoldDuplicatedDevices(drmDevicePtr local_devices[], int count)
  */
 int drmGetDevice(int fd, drmDevicePtr *device)
 {
+#ifdef __OpenBSD__
+    drmDevicePtr d;
+    struct stat sbuf;
+    char node[PATH_MAX + 1];
+    char d_name[PATH_MAX + 1];
+    int maj, min, n;
+    int ret;
+    int max_count = 1;
+
+    if (fd == -1 || device == NULL)
+        return -EINVAL;
+
+    if (fstat(fd, &sbuf))
+        return -errno;
+
+    maj = major(sbuf.st_rdev);
+    min = minor(sbuf.st_rdev);
+
+    if (maj != DRM_MAJOR || !S_ISCHR(sbuf.st_mode))
+        return -EINVAL;
+
+    n = snprintf(d_name, PATH_MAX, "drm%d", min);
+    if (n == -1 || n >= PATH_MAX)
+      return -errno;
+
+    n = snprintf(node, PATH_MAX, DRM_DEV_NAME, DRM_DIR_NAME, min);
+    if (n == -1 || n >= PATH_MAX)
+      return -errno;
+    if (stat(node, &sbuf))
+        return -EINVAL;
+
+    ret = drmProcessPciDevice(&d, d_name, node, DRM_NODE_PRIMARY,
+			      maj, min, true);
+    if (ret)
+        return ret;
+
+    *device = d;
+
+    return 0;
+#else
     drmDevicePtr *local_devices;
     drmDevicePtr d;
     DIR *sysdir;
@@ -3282,6 +3322,7 @@ free_devices:
 free_locals:
     free(local_devices);
     return ret;
+#endif
 }
 
 /**
-- 
2.10.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Emil Velikov]

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH libdrm 2/5] xf86drm: implement drmParseSubsystemType for OpenBSD

[Emil Velikov]

On 26 November 2016 at 00:40, Jonathan Gray <jsg@jsg.id.au> wrote:
> Implement drmParseSubsystemType for OpenBSD by always returning
> DRM_BUS_PCI.  No non-pci drm drivers are in the kernel and this is
> unlikely to change anytime soon as the existing ones aren't permissively
> licensed.
>
A few noticeable X11 MIT style licensed drivers include qxl, vgem and
virtio. Two of which PCI ones and vgem in it's own unique category ;-)

There was a question about re-licensing [some] drivers a few years ago
at XDC in France. IIRC devs were fine with it, yet it doesn't seem
like things changed much.
Have you/fellow BSD developers considered reaching out [as a group] to
interested drivers/developers ? Pardon if I've already
mentioned/already asked about this.

Thanks
Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH libdrm 3/5] xf86drm: implement drmParsePciDeviceInfo for OpenBSD

[Emil Velikov]

On 26 November 2016 at 00:40, Jonathan Gray <jsg@jsg.id.au> wrote:
> Implement drmParsePciDeviceInfo for OpenBSD by using the new
> DRM_IOCTL_GET_PCIINFO ioctl.
>
> Signed-off-by: Jonathan Gray <jsg@jsg.id.au>
> ---
>  xf86drm.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 51 insertions(+)
>
> diff --git a/xf86drm.c b/xf86drm.c
> index b355c83..581527b 100644
> --- a/xf86drm.c
> +++ b/xf86drm.c
> @@ -102,6 +102,26 @@
>  #define DRM_MAJOR 226 /* Linux */
>  #endif
>
> +#ifdef __OpenBSD__
> +
> +#define X_PRIVSEP
> +
> +struct drm_pciinfo {
> +       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;
> +};
> +
> +#define DRM_IOCTL_GET_PCIINFO  DRM_IOR(0x15, struct drm_pciinfo)
> +
> +#endif
> +
>  #define DRM_MSG_VERBOSITY 3
>
>  #define memclear(s) memset(&s, 0, sizeof(s))
> @@ -2991,6 +3011,37 @@ static int drmParsePciDeviceInfo(const char *d_name,
>      device->subdevice_id = config[46] | (config[47] << 8);
>
>      return 0;
> +#elif defined(__OpenBSD__)
> +    struct drm_pciinfo pinfo;
> +    char buf[PATH_MAX + 1];
> +    int fd, n;
> +
> +    n = snprintf(buf, sizeof(buf), "%s/%s", DRM_DIR_NAME, d_name);
> +    if (n == -1 || n >= sizeof(buf))
> +        return -errno;
> +
> +#ifndef X_PRIVSEP
> +    fd = open(buf, O_RDWR, 0);
> +#else
> +    fd = priv_open_device(buf);
> +#endif
> +
Since X_PRIVSEP is always set one can drop the ifndef case alongside
the define X_PRIVSEP all together. At the same time, priv_open_device
isn't defined thus one might well use drmOpenMinor() like in 4/5 ?

Sidenote: In the future we might fold drmParsePciBusInfo and
drmParsePciDeviceInfo, but for the moment we have to keep them
separate :-(

Thanks
Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH libdrm 5/5] xf86drm: implement an OpenBSD specific drmGetDevice

[Emil Velikov]

On 26 November 2016 at 00:40, Jonathan Gray <jsg@jsg.id.au> wrote:
> This avoids walking all of /dev and directly maps the fd to a path to a
> primary node.
>
I realise that the code is pretty ugly/bad/etc, but I would stay way
from similar optimisations. As-is it will just work as you guys get
support for render nodes/other.
That is unless things are noticeably slow [or bad in general].

Thanks
Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH libdrm 5/5] xf86drm: implement an OpenBSD specific drmGetDevice

[Jonathan Gray]

On Tue, Nov 29, 2016 at 08:03:58PM +0000, Emil Velikov wrote:
> On 26 November 2016 at 00:40, Jonathan Gray <jsg@jsg.id.au> wrote:
> > This avoids walking all of /dev and directly maps the fd to a path to a
> > primary node.
> >
> I realise that the code is pretty ugly/bad/etc, but I would stay way
> from similar optimisations. As-is it will just work as you guys get
> support for render nodes/other.
> That is unless things are noticeably slow [or bad in general].
> 
> Thanks
> Emil

/dev/ has 1200 files on a machine here, drm nodes aren't in a drm
specific directory as on linux.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Jonathan Gray]

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH libdrm 2/5] xf86drm: implement drmParseSubsystemType for OpenBSD

[Jonathan Gray]

On Tue, Nov 29, 2016 at 07:46:31PM +0000, Emil Velikov wrote:
> On 26 November 2016 at 00:40, Jonathan Gray <jsg@jsg.id.au> wrote:
> > Implement drmParseSubsystemType for OpenBSD by always returning
> > DRM_BUS_PCI.  No non-pci drm drivers are in the kernel and this is
> > unlikely to change anytime soon as the existing ones aren't permissively
> > licensed.
> >
> A few noticeable X11 MIT style licensed drivers include qxl, vgem and
> virtio. Two of which PCI ones and vgem in it's own unique category ;-)
> 
> There was a question about re-licensing [some] drivers a few years ago
> at XDC in France. IIRC devs were fine with it, yet it doesn't seem
> like things changed much.
> Have you/fellow BSD developers considered reaching out [as a group] to
> interested drivers/developers ? Pardon if I've already
> mentioned/already asked about this.
> 
> Thanks
> Emil

There is no particular driver, cirrus, gma500 etc might be nice but
really what limited spare time we have that involves drm is dealt
dealing with the constant churn of i915.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH libdrm 3/5] xf86drm: implement drmParsePciDeviceInfo for OpenBSD

[Jonathan Gray]

On Tue, Nov 29, 2016 at 07:55:13PM +0000, Emil Velikov wrote:
> On 26 November 2016 at 00:40, Jonathan Gray <jsg@jsg.id.au> wrote:
> > Implement drmParsePciDeviceInfo for OpenBSD by using the new
> > DRM_IOCTL_GET_PCIINFO ioctl.
> >
> > Signed-off-by: Jonathan Gray <jsg@jsg.id.au>
> > ---
> >  xf86drm.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 51 insertions(+)
> >
> > diff --git a/xf86drm.c b/xf86drm.c
> > index b355c83..581527b 100644
> > --- a/xf86drm.c
> > +++ b/xf86drm.c
> > @@ -102,6 +102,26 @@
> >  #define DRM_MAJOR 226 /* Linux */
> >  #endif
> >
> > +#ifdef __OpenBSD__
> > +
> > +#define X_PRIVSEP
> > +
> > +struct drm_pciinfo {
> > +       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;
> > +};
> > +
> > +#define DRM_IOCTL_GET_PCIINFO  DRM_IOR(0x15, struct drm_pciinfo)
> > +
> > +#endif
> > +
> >  #define DRM_MSG_VERBOSITY 3
> >
> >  #define memclear(s) memset(&s, 0, sizeof(s))
> > @@ -2991,6 +3011,37 @@ static int drmParsePciDeviceInfo(const char *d_name,
> >      device->subdevice_id = config[46] | (config[47] << 8);
> >
> >      return 0;
> > +#elif defined(__OpenBSD__)
> > +    struct drm_pciinfo pinfo;
> > +    char buf[PATH_MAX + 1];
> > +    int fd, n;
> > +
> > +    n = snprintf(buf, sizeof(buf), "%s/%s", DRM_DIR_NAME, d_name);
> > +    if (n == -1 || n >= sizeof(buf))
> > +        return -errno;
> > +
> > +#ifndef X_PRIVSEP
> > +    fd = open(buf, O_RDWR, 0);
> > +#else
> > +    fd = priv_open_device(buf);
> > +#endif
> > +
> Since X_PRIVSEP is always set one can drop the ifndef case alongside
> the define X_PRIVSEP all together. At the same time, priv_open_device
> isn't defined thus one might well use drmOpenMinor() like in 4/5 ?

Then we'd have to find a minor number and type based on the d_name
string argument to drmParsePciDeviceInfo.  The priv_open_device part is
really a different patch.

> 
> Sidenote: In the future we might fold drmParsePciBusInfo and
> drmParsePciDeviceInfo, but for the moment we have to keep them
> separate :-(

Annoying that one takes a minor and one takes a string...
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH libdrm 3/5] xf86drm: implement drmParsePciDeviceInfo for OpenBSD

[Emil Velikov]

On 30 November 2016 at 00:38, Jonathan Gray <jsg@jsg.id.au> wrote:
> On Tue, Nov 29, 2016 at 07:55:13PM +0000, Emil Velikov wrote:
>> On 26 November 2016 at 00:40, Jonathan Gray <jsg@jsg.id.au> wrote:
>> > Implement drmParsePciDeviceInfo for OpenBSD by using the new
>> > DRM_IOCTL_GET_PCIINFO ioctl.
>> >
>> > Signed-off-by: Jonathan Gray <jsg@jsg.id.au>
>> > ---
>> >  xf86drm.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
>> >  1 file changed, 51 insertions(+)
>> >
>> > diff --git a/xf86drm.c b/xf86drm.c
>> > index b355c83..581527b 100644
>> > --- a/xf86drm.c
>> > +++ b/xf86drm.c
>> > @@ -102,6 +102,26 @@
>> >  #define DRM_MAJOR 226 /* Linux */
>> >  #endif
>> >
>> > +#ifdef __OpenBSD__
>> > +
>> > +#define X_PRIVSEP
>> > +
>> > +struct drm_pciinfo {
>> > +       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;
>> > +};
>> > +
>> > +#define DRM_IOCTL_GET_PCIINFO  DRM_IOR(0x15, struct drm_pciinfo)
>> > +
>> > +#endif
>> > +
>> >  #define DRM_MSG_VERBOSITY 3
>> >
>> >  #define memclear(s) memset(&s, 0, sizeof(s))
>> > @@ -2991,6 +3011,37 @@ static int drmParsePciDeviceInfo(const char *d_name,
>> >      device->subdevice_id = config[46] | (config[47] << 8);
>> >
>> >      return 0;
>> > +#elif defined(__OpenBSD__)
>> > +    struct drm_pciinfo pinfo;
>> > +    char buf[PATH_MAX + 1];
>> > +    int fd, n;
>> > +
>> > +    n = snprintf(buf, sizeof(buf), "%s/%s", DRM_DIR_NAME, d_name);
>> > +    if (n == -1 || n >= sizeof(buf))
>> > +        return -errno;
>> > +
>> > +#ifndef X_PRIVSEP
>> > +    fd = open(buf, O_RDWR, 0);
>> > +#else
>> > +    fd = priv_open_device(buf);
>> > +#endif
>> > +
>> Since X_PRIVSEP is always set one can drop the ifndef case alongside
>> the define X_PRIVSEP all together. At the same time, priv_open_device
>> isn't defined thus one might well use drmOpenMinor() like in 4/5 ?
>
> Then we'd have to find a minor number and type based on the d_name
> string argument to drmParsePciDeviceInfo.  The priv_open_device part is
> really a different patch.
>
>>
>> Sidenote: In the future we might fold drmParsePciBusInfo and
>> drmParsePciDeviceInfo, but for the moment we have to keep them
>> separate :-(
>
> Annoying that one takes a minor and one takes a string...
It's a po-tay-to po-tah-to case on our end :-) A maj/min pair seems
better, patch coming in a bit.

Thanks
Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Emil Velikov]

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH libdrm 2/5] xf86drm: implement drmParseSubsystemType for OpenBSD

[Emil Velikov]

On 30 November 2016 at 00:23, Jonathan Gray <jsg@jsg.id.au> wrote:
> On Tue, Nov 29, 2016 at 07:46:31PM +0000, Emil Velikov wrote:
>> On 26 November 2016 at 00:40, Jonathan Gray <jsg@jsg.id.au> wrote:
>> > Implement drmParseSubsystemType for OpenBSD by always returning
>> > DRM_BUS_PCI.  No non-pci drm drivers are in the kernel and this is
>> > unlikely to change anytime soon as the existing ones aren't permissively
>> > licensed.
>> >
>> A few noticeable X11 MIT style licensed drivers include qxl, vgem and
>> virtio. Two of which PCI ones and vgem in it's own unique category ;-)
>>
>> There was a question about re-licensing [some] drivers a few years ago
>> at XDC in France. IIRC devs were fine with it, yet it doesn't seem
>> like things changed much.
>> Have you/fellow BSD developers considered reaching out [as a group] to
>> interested drivers/developers ? Pardon if I've already
>> mentioned/already asked about this.
>>
>> Thanks
>> Emil
>
> There is no particular driver, cirrus, gma500 etc might be nice but
> really what limited spare time we have that involves drm is dealt
> dealing with the constant churn of i915.
Ack. AFAICT keeping track is a slightly different ball-game, which
does eat a lot of time.

<offtopic>
Consider reusing/sharing of the kernel compat layer that FreeBSD and
others did. If you haven't already.

As one starts incrementally picking each [git] commit it should be
easier to track regressions and pick fixes.
</offtopic>

-Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH libdrm 5/5] xf86drm: implement an OpenBSD specific drmGetDevice

[Emil Velikov]

On 30 November 2016 at 00:00, Jonathan Gray <jsg@jsg.id.au> wrote:
> On Tue, Nov 29, 2016 at 08:03:58PM +0000, Emil Velikov wrote:
>> On 26 November 2016 at 00:40, Jonathan Gray <jsg@jsg.id.au> wrote:
>> > This avoids walking all of /dev and directly maps the fd to a path to a
>> > primary node.
>> >
>> I realise that the code is pretty ugly/bad/etc, but I would stay way
>> from similar optimisations. As-is it will just work as you guys get
>> support for render nodes/other.
>> That is unless things are noticeably slow [or bad in general].
>>
>> Thanks
>> Emil
>
> /dev/ has 1200 files on a machine here, drm nodes aren't in a drm
> specific directory as on linux.
Eeek ...1200, there's only ~160 over here.

Is it against OpenBSD policy/philosophy to nest things (using
sub-folders), a matter of carefully updating this to avoid breakage
(shortage to time/manpower), or it's mostly a matter of personal taste
?

Regardless of the reason, please include your comment in the code.
Also please mention that this works only for card nodes.
And don't forget the (dummy) drmParseSubsystemType call.

Thanks
Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel