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

[PATCH libdrm v2 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.

v2: use type parameter to select dev name instead of always
    using DRM_DEV_NAME

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

diff --git a/xf86drm.c b/xf86drm.c
index f117716..f93ebc0 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -2818,7 +2818,40 @@ static char *drmGetMinorNameForFD(int fd, int type)
 out_close_dir:
     closedir(sysdir);
 #else
-#warning "Missing implementation of drmGetMinorNameForFD"
+    struct stat sbuf;
+    char buf[PATH_MAX + 1];
+    const char *dev_name;
+    unsigned int maj, min;
+    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;
+
+    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 NULL;
+    };
+
+    n = snprintf(buf, sizeof(buf), 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 v2 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 f93ebc0..35c1fc4 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -2887,6 +2887,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 v2 3/5] xf86drm: implement drmParsePciDeviceInfo for OpenBSD

[Jonathan Gray]

Implement drmParsePciDeviceInfo for OpenBSD by using the new
DRM_IOCTL_GET_PCIINFO ioctl.

v2: adapt to drmParsePciDeviceInfo changes and use drmOpenMinor

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 35c1fc4..6465953 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -102,6 +102,22 @@
 #define DRM_MAJOR 226 /* Linux */
 #endif
 
+#ifdef __OpenBSD__
+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))
@@ -3061,6 +3077,31 @@ static int drmParsePciDeviceInfo(int maj, int min,
         return parse_config_sysfs_file(maj, min, device);
 
     return 0;
+#elif defined(__OpenBSD__)
+    struct drm_pciinfo pinfo;
+    int fd, type;
+
+    type = drmGetMinorType(min);
+    if (type == -1)
+        return -ENODEV;
+
+    fd = drmOpenMinor(min, 0, type);
+    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 v2 4/5] xf86drm: implement drmParsePciBusInfo for OpenBSD

[Jonathan Gray]

Implement drmParsePciBusInfo for OpenBSD by using the new
DRM_IOCTL_GET_PCIINFO ioctl.

v2: use drmGetMinorType to get node type instead of always
    using DRM_NODE_PRIMARY.

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

diff --git a/xf86drm.c b/xf86drm.c
index 6465953..ea24108 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -2947,6 +2947,30 @@ static int drmParsePciBusInfo(int maj, int min, drmPciBusInfoPtr info)
     info->func = func;
 
     return 0;
+#elif defined(__OpenBSD__)
+    struct drm_pciinfo pinfo;
+    int fd, type;
+
+    type = drmGetMinorType(min);
+    if (type == -1)
+        return -ENODEV;
+
+    fd = drmOpenMinor(min, 0, type);
+    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 v2 5/5] xf86drm: implement an OpenBSD specific drmGetDevice2

[Jonathan Gray]

DRI devices on OpenBSD are not in their own directory.  They reside in
/dev with a large number of statically generated /dev nodes.

Avoid stat'ing all of /dev on OpenBSD by implementing this custom path.

v2:
   - use drmGetMinorType to get node type
   - adapt to drmProcessPciDevice changes
   - verify drmParseSubsystemType type is PCI
   - add a comment describing why this was added

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

diff --git a/xf86drm.c b/xf86drm.c
index ea24108..9981be4 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -3248,6 +3248,67 @@ drm_device_validate_flags(uint32_t flags)
  */
 int drmGetDevice2(int fd, uint32_t flags, drmDevicePtr *device)
 {
+#ifdef __OpenBSD__
+    /*
+     * DRI device nodes on OpenBSD are not in their own directory, they reside
+     * in /dev along with a large number of statically generated /dev nodes.
+     * Avoid stat'ing all of /dev needlessly by implementing this custom path.
+     */
+    drmDevicePtr     d;
+    struct stat      sbuf;
+    char             node[PATH_MAX + 1];
+    const char      *dev_name;
+    int              node_type, subsystem_type;
+    int              maj, min, n, ret;
+
+    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;
+
+    node_type = drmGetMinorType(min);
+    if (node_type == -1)
+        return -ENODEV;
+
+    switch (node_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(node, PATH_MAX, dev_name, DRM_DIR_NAME, min);
+    if (n == -1 || n >= PATH_MAX)
+      return -errno;
+    if (stat(node, &sbuf))
+        return -EINVAL;
+
+    subsystem_type = drmParseSubsystemType(maj, min);
+    if (subsystem_type != DRM_BUS_PCI)
+        return -ENODEV;
+
+    ret = drmProcessPciDevice(&d, node, node_type, maj, min, true, flags);
+    if (ret)
+        return ret;
+
+    *device = d;
+
+    return 0;
+#else
     drmDevicePtr *local_devices;
     drmDevicePtr d;
     DIR *sysdir;
@@ -3357,6 +3418,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 v2 5/5] xf86drm: implement an OpenBSD specific drmGetDevice2

[Emil Velikov]

On 1 December 2016 at 04:18, Jonathan Gray <jsg@jsg.id.au> wrote:
> DRI devices on OpenBSD are not in their own directory.  They reside in
> /dev with a large number of statically generated /dev nodes.
>
> Avoid stat'ing all of /dev on OpenBSD by implementing this custom path.
>
> v2:
>    - use drmGetMinorType to get node type
>    - adapt to drmProcessPciDevice changes
>    - verify drmParseSubsystemType type is PCI
>    - add a comment describing why this was added
>
Thanks for the update Jonathan.

I've pulled v2 in master,
Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Jonathan Gray]

On Mon, Dec 05, 2016 at 05:56:40PM +0000, Emil Velikov wrote:
> On 1 December 2016 at 04:18, Jonathan Gray <jsg@jsg.id.au> wrote:
> > DRI devices on OpenBSD are not in their own directory.  They reside in
> > /dev with a large number of statically generated /dev nodes.
> >
> > Avoid stat'ing all of /dev on OpenBSD by implementing this custom path.
> >
> > v2:
> >    - use drmGetMinorType to get node type
> >    - adapt to drmProcessPciDevice changes
> >    - verify drmParseSubsystemType type is PCI
> >    - add a comment describing why this was added
> >
> Thanks for the update Jonathan.
> 
> I've pulled v2 in master,
> Emil

Thanks, going over what went in I see drmGetMinorNameForFD and
the OpenBSD drmGetDevice2 paths need to be adjusted to have the correct
minor for the control/render nodes.

ie

base = drmGetMinorBase(type);
if (min < base)
	return error;

min -= base;

I'll send another patch for this.

And the common code could be split into a shared function?

drmGetDeviceNameFromFd2 would do the same thing as
drmGetDeviceNameFromFd on OpenBSD as far as I can tell so that could be
another shared function instead of the current "missing implementation"
warning.  Or should drmGetDeviceNameFromFd purposefully not handle
render/control nodes?
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Emil Velikov]

On 6 December 2016 at 05:12, Jonathan Gray <jsg@jsg.id.au> wrote:
> On Mon, Dec 05, 2016 at 05:56:40PM +0000, Emil Velikov wrote:
>> On 1 December 2016 at 04:18, Jonathan Gray <jsg@jsg.id.au> wrote:
>> > DRI devices on OpenBSD are not in their own directory.  They reside in
>> > /dev with a large number of statically generated /dev nodes.
>> >
>> > Avoid stat'ing all of /dev on OpenBSD by implementing this custom path.
>> >
>> > v2:
>> >    - use drmGetMinorType to get node type
>> >    - adapt to drmProcessPciDevice changes
>> >    - verify drmParseSubsystemType type is PCI
>> >    - add a comment describing why this was added
>> >
>> Thanks for the update Jonathan.
>>
>> I've pulled v2 in master,
>> Emil
>
> Thanks, going over what went in I see drmGetMinorNameForFD and
> the OpenBSD drmGetDevice2 paths need to be adjusted to have the correct
> minor for the control/render nodes.
>
> ie
>
> base = drmGetMinorBase(type);
> if (min < base)
>         return error;
>
> min -= base;
>
> I'll send another patch for this.
>
> And the common code could be split into a shared function?
>
I don't have a strong preference either way, bth.

> drmGetDeviceNameFromFd2 would do the same thing as
> drmGetDeviceNameFromFd on OpenBSD as far as I can tell so that could be
> another shared function instead of the current "missing implementation"
> warning.  Or should drmGetDeviceNameFromFd purposefully not handle
> render/control nodes?
drmGetDeviceNameFromFd has historically handled only card nodes, so
I'd keep that as-is.
The "2" should handle any node imaginable.

Please, spin the patches when you can and give the OpenBSD
implementations a test. I'd like to get those in for the next release
- in the next week or so. This way we can use the less annoying
drmGetDevice[s]2 in Mesa :-)

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

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

[Emil Velikov]

Hi Jonathan,

On 1 December 2016 at 04:18, Jonathan Gray <jsg@jsg.id.au> wrote:

> --- a/xf86drm.c
> +++ b/xf86drm.c
> @@ -3248,6 +3248,67 @@ drm_device_validate_flags(uint32_t flags)
>   */
>  int drmGetDevice2(int fd, uint32_t flags, drmDevicePtr *device)
>  {
> +#ifdef __OpenBSD__
> +    /*
> +     * DRI device nodes on OpenBSD are not in their own directory, they reside
> +     * in /dev along with a large number of statically generated /dev nodes.
> +     * Avoid stat'ing all of /dev needlessly by implementing this custom path.
> +     */
I was in the area, fixing the odd bug and doing some cleanups and a
question came to mind:

Is there some obstacle of placing the drm nodes under /dev/dri/? It
would involve a check/update through the OpenBSD tree, yet no obvious
downsides comes to mind.
I think it would make things more distinct and obvious. IIRC the
OpenBSD xserver does some checking which /dev node opened, the
suggestion should help there.

What do you think?
Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Jonathan Gray]

On Thu, Jun 21, 2018 at 03:24:49PM +0100, Emil Velikov wrote:
> Hi Jonathan,
> 
> On 1 December 2016 at 04:18, Jonathan Gray <jsg@jsg.id.au> wrote:
> 
> > --- a/xf86drm.c
> > +++ b/xf86drm.c
> > @@ -3248,6 +3248,67 @@ drm_device_validate_flags(uint32_t flags)
> >   */
> >  int drmGetDevice2(int fd, uint32_t flags, drmDevicePtr *device)
> >  {
> > +#ifdef __OpenBSD__
> > +    /*
> > +     * DRI device nodes on OpenBSD are not in their own directory, they reside
> > +     * in /dev along with a large number of statically generated /dev nodes.
> > +     * Avoid stat'ing all of /dev needlessly by implementing this custom path.
> > +     */
> I was in the area, fixing the odd bug and doing some cleanups and a
> question came to mind:
> 
> Is there some obstacle of placing the drm nodes under /dev/dri/? It
> would involve a check/update through the OpenBSD tree, yet no obvious
> downsides comes to mind.
> I think it would make things more distinct and obvious. IIRC the
> OpenBSD xserver does some checking which /dev node opened, the
> suggestion should help there.
> 
> What do you think?
> Emil

There are no other devices under a sub-directory besides /dev/fd/.
I don't think anyone is going to be onboard with changing things for
drm nodes.  Though drm device nodes names will have to be revisted
when/if render nodes etc get supported.  drmR drmC etc have not
been proposed yet.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Emil Velikov]

On 21 June 2018 at 16:32, Jonathan Gray <jsg@jsg.id.au> wrote:
> On Thu, Jun 21, 2018 at 03:24:49PM +0100, Emil Velikov wrote:
>> Hi Jonathan,
>>
>> On 1 December 2016 at 04:18, Jonathan Gray <jsg@jsg.id.au> wrote:
>>
>> > --- a/xf86drm.c
>> > +++ b/xf86drm.c
>> > @@ -3248,6 +3248,67 @@ drm_device_validate_flags(uint32_t flags)
>> >   */
>> >  int drmGetDevice2(int fd, uint32_t flags, drmDevicePtr *device)
>> >  {
>> > +#ifdef __OpenBSD__
>> > +    /*
>> > +     * DRI device nodes on OpenBSD are not in their own directory, they reside
>> > +     * in /dev along with a large number of statically generated /dev nodes.
>> > +     * Avoid stat'ing all of /dev needlessly by implementing this custom path.
>> > +     */
>> I was in the area, fixing the odd bug and doing some cleanups and a
>> question came to mind:
>>
>> Is there some obstacle of placing the drm nodes under /dev/dri/? It
>> would involve a check/update through the OpenBSD tree, yet no obvious
>> downsides comes to mind.
>> I think it would make things more distinct and obvious. IIRC the
>> OpenBSD xserver does some checking which /dev node opened, the
>> suggestion should help there.
>>
>> What do you think?
>> Emil
>
> There are no other devices under a sub-directory besides /dev/fd/.
> I don't think anyone is going to be onboard with changing things for
> drm nodes.  Though drm device nodes names will have to be revisted
> when/if render nodes etc get supported.  drmR drmC etc have not
> been proposed yet.

I see, that's enlighening.

Out of curiosity: personally, do you see any technical issues with a
sub-directory approach?

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

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

[Jonathan Gray]

On Tue, Jun 26, 2018 at 11:38:20AM +0100, Emil Velikov wrote:
> On 21 June 2018 at 16:32, Jonathan Gray <jsg@jsg.id.au> wrote:
> > On Thu, Jun 21, 2018 at 03:24:49PM +0100, Emil Velikov wrote:
> >> Hi Jonathan,
> >>
> >> On 1 December 2016 at 04:18, Jonathan Gray <jsg@jsg.id.au> wrote:
> >>
> >> > --- a/xf86drm.c
> >> > +++ b/xf86drm.c
> >> > @@ -3248,6 +3248,67 @@ drm_device_validate_flags(uint32_t flags)
> >> >   */
> >> >  int drmGetDevice2(int fd, uint32_t flags, drmDevicePtr *device)
> >> >  {
> >> > +#ifdef __OpenBSD__
> >> > +    /*
> >> > +     * DRI device nodes on OpenBSD are not in their own directory, they reside
> >> > +     * in /dev along with a large number of statically generated /dev nodes.
> >> > +     * Avoid stat'ing all of /dev needlessly by implementing this custom path.
> >> > +     */
> >> I was in the area, fixing the odd bug and doing some cleanups and a
> >> question came to mind:
> >>
> >> Is there some obstacle of placing the drm nodes under /dev/dri/? It
> >> would involve a check/update through the OpenBSD tree, yet no obvious
> >> downsides comes to mind.
> >> I think it would make things more distinct and obvious. IIRC the
> >> OpenBSD xserver does some checking which /dev node opened, the
> >> suggestion should help there.
> >>
> >> What do you think?
> >> Emil
> >
> > There are no other devices under a sub-directory besides /dev/fd/.
> > I don't think anyone is going to be onboard with changing things for
> > drm nodes.  Though drm device nodes names will have to be revisted
> > when/if render nodes etc get supported.  drmR drmC etc have not
> > been proposed yet.
> 
> I see, that's enlighening.
> 
> Out of curiosity: personally, do you see any technical issues with a
> sub-directory approach?

I get that you want a single path but it seems these functions were
really designed assuming the approach was going to be a sub-directory.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Mark Kettenis]

> Date: Tue, 26 Jun 2018 20:58:18 +1000
> From: Jonathan Gray <jsg@jsg.id.au>
> 
> On Tue, Jun 26, 2018 at 11:38:20AM +0100, Emil Velikov wrote:
> > On 21 June 2018 at 16:32, Jonathan Gray <jsg@jsg.id.au> wrote:
> > > On Thu, Jun 21, 2018 at 03:24:49PM +0100, Emil Velikov wrote:
> > >> Hi Jonathan,
> > >>
> > >> On 1 December 2016 at 04:18, Jonathan Gray <jsg@jsg.id.au> wrote:
> > >>
> > >> > --- a/xf86drm.c
> > >> > +++ b/xf86drm.c
> > >> > @@ -3248,6 +3248,67 @@ drm_device_validate_flags(uint32_t flags)
> > >> >   */
> > >> >  int drmGetDevice2(int fd, uint32_t flags, drmDevicePtr *device)
> > >> >  {
> > >> > +#ifdef __OpenBSD__
> > >> > +    /*
> > >> > +     * DRI device nodes on OpenBSD are not in their own directory, they reside
> > >> > +     * in /dev along with a large number of statically generated /dev nodes.
> > >> > +     * Avoid stat'ing all of /dev needlessly by implementing this custom path.
> > >> > +     */
> > >> I was in the area, fixing the odd bug and doing some cleanups and a
> > >> question came to mind:
> > >>
> > >> Is there some obstacle of placing the drm nodes under /dev/dri/? It
> > >> would involve a check/update through the OpenBSD tree, yet no obvious
> > >> downsides comes to mind.
> > >> I think it would make things more distinct and obvious. IIRC the
> > >> OpenBSD xserver does some checking which /dev node opened, the
> > >> suggestion should help there.
> > >>
> > >> What do you think?
> > >> Emil
> > >
> > > There are no other devices under a sub-directory besides /dev/fd/.
> > > I don't think anyone is going to be onboard with changing things for
> > > drm nodes.  Though drm device nodes names will have to be revisted
> > > when/if render nodes etc get supported.  drmR drmC etc have not
> > > been proposed yet.
> > 
> > I see, that's enlighening.
> > 
> > Out of curiosity: personally, do you see any technical issues with a
> > sub-directory approach?
> 
> I get that you want a single path but it seems these functions were
> really designed assuming the approach was going to be a sub-directory.

The bigger picture here is that this code is primarily used to query
for information about an open drm file descriptor.  The Linux
implementation does a lot of filesystem groveling to achieve that.
Especially the bits that descend into /sys are never going to work on
OpenBSD.  If a more OS-agnostic approach is wanted, I would say an
ioctl to return the relevant information would be more appropriate.
This is actually what we did for OpenBSD where we implemented
DRM_IOCTL_GET_PCIINFO to implement drmParsePciBusInfo.  Such an
approach avoids the issue of mapping the file descriptor back to a
file system path.  Also note that mapping and open file descriptor to
a file system path is fundamentally impossible on Unix without
cheating.  Although cheating is fairly easy for devices.

If mapping a file descriptor back to a filesystem path is necessary,
OpenBSD actually implements devname(3) which uses a database to map
device major/minor pairs back to a device name.  This is actually a
function that was introduced in 4.4BSD, and as far as I can tell it is
still present in FreeBSD and NetBSD as well.  We could change the
OpenBSD implementation of drmGetDevice2() to use that, but it wouldn't
really make the code simpler.

All in all, I think it is best to keep the Linux and OpenBSD code
separate here.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Emil Velikov]

Hi guys,

Above all, yes the current approach looks a bit funky.
Given the constrains (cannot use ioctl and libudev) it's rather reasonable.

That said, ideas for improvements are always welcome.

On 26 June 2018 at 13:03, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>> Date: Tue, 26 Jun 2018 20:58:18 +1000
>> From: Jonathan Gray <jsg@jsg.id.au>
>>
>> On Tue, Jun 26, 2018 at 11:38:20AM +0100, Emil Velikov wrote:
>> > On 21 June 2018 at 16:32, Jonathan Gray <jsg@jsg.id.au> wrote:
>> > > On Thu, Jun 21, 2018 at 03:24:49PM +0100, Emil Velikov wrote:
>> > >> Hi Jonathan,
>> > >>
>> > >> On 1 December 2016 at 04:18, Jonathan Gray <jsg@jsg.id.au> wrote:
>> > >>
>> > >> > --- a/xf86drm.c
>> > >> > +++ b/xf86drm.c
>> > >> > @@ -3248,6 +3248,67 @@ drm_device_validate_flags(uint32_t flags)
>> > >> >   */
>> > >> >  int drmGetDevice2(int fd, uint32_t flags, drmDevicePtr *device)
>> > >> >  {
>> > >> > +#ifdef __OpenBSD__
>> > >> > +    /*
>> > >> > +     * DRI device nodes on OpenBSD are not in their own directory, they reside
>> > >> > +     * in /dev along with a large number of statically generated /dev nodes.
>> > >> > +     * Avoid stat'ing all of /dev needlessly by implementing this custom path.
>> > >> > +     */
>> > >> I was in the area, fixing the odd bug and doing some cleanups and a
>> > >> question came to mind:
>> > >>
>> > >> Is there some obstacle of placing the drm nodes under /dev/dri/? It
>> > >> would involve a check/update through the OpenBSD tree, yet no obvious
>> > >> downsides comes to mind.
>> > >> I think it would make things more distinct and obvious. IIRC the
>> > >> OpenBSD xserver does some checking which /dev node opened, the
>> > >> suggestion should help there.
>> > >>
>> > >> What do you think?
>> > >> Emil
>> > >
>> > > There are no other devices under a sub-directory besides /dev/fd/.
>> > > I don't think anyone is going to be onboard with changing things for
>> > > drm nodes.  Though drm device nodes names will have to be revisted
>> > > when/if render nodes etc get supported.  drmR drmC etc have not
>> > > been proposed yet.
>> >
>> > I see, that's enlighening.
>> >
>> > Out of curiosity: personally, do you see any technical issues with a
>> > sub-directory approach?
>>
>> I get that you want a single path but it seems these functions were
>> really designed assuming the approach was going to be a sub-directory.
Right, I should have said "Ignoring the actual implementation, do you
see any technical issues..."

> The bigger picture here is that this code is primarily used to query
> for information about an open drm file descriptor.  The Linux
> implementation does a lot of filesystem groveling to achieve that.
> Especially the bits that descend into /sys are never going to work on
> OpenBSD.  If a more OS-agnostic approach is wanted, I would say an
> ioctl to return the relevant information would be more appropriate.
> This is actually what we did for OpenBSD where we implemented
> DRM_IOCTL_GET_PCIINFO to implement drmParsePciBusInfo.

An ioctl approach has two issues:
 - linux aims to keep the uabi 'forever' as such adding extra ioctl's
is fairly hard
This particular instance was discussed and rejected with linux devs.
 - on linux the device (even the bus it's on) can be powered off
Issuing an ioctl will wake up the device (which can be slow) even if
you don't end up using that device.

Admittedly you'd want the power-off machinery in !linux at some point.
GPUs power hungry beasts, even when they're not used ;-)

As I mentioned libudev (yes I know you're not using it), it's also a
no-go since apps ship with their own version of it, causing all sorts
of grief.
We tried that in Mesa and after over a year of various fixes, I nuked
it in favour of drmDevice.

>  Such an
> approach avoids the issue of mapping the file descriptor back to a
> file system path.  Also note that mapping and open file descriptor to
> a file system path is fundamentally impossible on Unix without
> cheating.  Although cheating is fairly easy for devices.
>
Agreed, I've skimmed through the code of lsof and ouch...

> If mapping a file descriptor back to a filesystem path is necessary,
> OpenBSD actually implements devname(3) which uses a database to map
> device major/minor pairs back to a device name.  This is actually a
> function that was introduced in 4.4BSD, and as far as I can tell it is
> still present in FreeBSD and NetBSD as well.  We could change the
> OpenBSD implementation of drmGetDevice2() to use that, but it wouldn't
> really make the code simpler.
>
> All in all, I think it is best to keep the Linux and OpenBSD code
> separate here.
While I can see the current approach seems foobar, my question was orthogonal.

Namely: is there a thread/documentation covering the technical reasons
behind the lack of sub-directories in /dev/?

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