NetBSD Problem Report #51786

From www@NetBSD.org  Fri Jan  6 04:14:26 2017
Return-Path: <www@NetBSD.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(Client CN "mail.netbsd.org", Issuer "Postmaster NetBSD.org" (verified OK))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 433C67A16E
	for <gnats-bugs@gnats.NetBSD.org>; Fri,  6 Jan 2017 04:14:26 +0000 (UTC)
Message-Id: <20170106041424.46B097A291@mollari.NetBSD.org>
Date: Fri,  6 Jan 2017 04:14:24 +0000 (UTC)
From: davshao@gmail.com
Reply-To: davshao@gmail.com
To: gnats-bugs@NetBSD.org
Subject: mesa 13.0.2 libdrm 2.4.74 Radeon needs kernel support, new ioctl or sysctl
X-Send-Pr-Version: www-1.0

>Number:         51786
>Category:       kern
>Synopsis:       mesa 13.0.2 libdrm 2.4.74 Radeon needs kernel support, new ioctl or sysctl
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    riastradh
>State:          closed
>Class:          change-request
>Submitter-Id:   net
>Arrival-Date:   Fri Jan 06 04:15:00 +0000 2017
>Closed-Date:    Sun Aug 18 05:56:24 +0000 2019
>Last-Modified:  Sun Aug 18 05:56:24 +0000 2019
>Originator:     David Shao
>Release:        NetBSD 7.99.54 (GENERIC) amd64
>Organization:
>Environment:
NetBSD xxxxxx.xxx 7.99.54 NetBSD 7.99.54 (GENERIC) #4: Wed Jan  4 12:21:59 PST 2017  xxxxxx@xxxxxx.xxx:/usr/obj/sys/arch/amd64/compile/GENERIC amd64

>Description:
The NetBSD kernel developer community may need to make a decision relatively soon to enable graphics acceleration on Radeon cards using mesa 13.0.2 and beyond.

On a NetBSD 7.99.54 amd64 machine with a Sapphire Radeon HD6450 graphics card, pkgsrc X11_TYPE=modular, I have mesa 13.0.2 running with glamor/egl support.  The big change for porting purposes of the mesa 13 series is the switch to using libdrm 2.4.74 to probe for pci graphics hardware.  Specifically in the libdrm 2.4.74 file xf86drm.c, the function drmGetDevice() call is given a file descriptor fd, which for many systems corresponds to device /dev/dri/card0.  drmGetDevice() is then expected to fill in first values for the bus (domain, bus, slot, function) and then fill in information on the actual video card (vendor, device, subvendor, subdevice, revision).  

The kernel has this information.  It seems to me there are two natural ways for BSDs for the kernel to communicate back to userland: using an ioctl or through sysctl.  Both approaches are currently being tried:  The first by OpenBSD, the second by proposed patches to FreeBSD ports on the freebsd-x11 mailing list.  A cursory glance at sysctl information on NetBSD amd64 did not reveal to me information equivalent to what is needed by libdrm.  

I have verified almost line by line copying of OpenBSD's added ioctl is sufficient, the patches provided below.  The one change is that I was not certain any existing ioctl number could be used, whereas OpenBSD seized one that had only been used for drm1.  I therefore chose the so-far unused 0x0f instead of 0x15.

Note this added ioctl does not seem necessary for Intel integrated graphics, at least on the IvyBridge machine I tested.  It is possible only those running Radeon and perhaps Nouveau might need this ioctl to get mesa 13.0.2 with as complete graphics acceleration as possible enabled. 

It is certainly possible to use sysctl instead of ioctl to communicate this information to libdrm.  On FreeBSD and DragonFly using sysctl is somewhat a mess, because the information is divided between two subtrees, and it is rather clumsy to match up sysctl subtree entries to the fd for /dev/dri/card0.  If NetBSD were to implement these sysctl's from scratch, it is quite possible to make access to the information much easier than for FreeBSD or DragonFly.

I therefore have no opinion, I simply ask the NetBSD kernel developer community to make a decision, soon.
>How-To-Repeat:

>Fix:
I apologize if any of these patches are redundant or unnecessary for the task:

--- sys/external/bsd/drm2/dist/drm/drm_drv.c.orig	2014-07-17 22:13:56.000000000 -0700
+++ sys/external/bsd/drm2/dist/drm/drm_drv.c	2017-01-01 19:18:45.417145611 -0800
@@ -72,6 +72,10 @@
 	DRM_IOCTL_DEF(DRM_IOCTL_SET_CLIENT_CAP, drm_setclientcap, 0),
 	DRM_IOCTL_DEF(DRM_IOCTL_SET_VERSION, drm_setversion, DRM_MASTER),

+#ifdef __NetBSD__
+/* From OpenBSD sys/dev/pci/drm/drm_drv.c */
+	DRM_IOCTL_DEF(DRM_IOCTL_GET_PCIINFO, drm_getpciinfo, DRM_UNLOCKED|DRM_RENDER_ALLOW),
+#endif
 	DRM_IOCTL_DEF(DRM_IOCTL_SET_UNIQUE, drm_setunique, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
 	DRM_IOCTL_DEF(DRM_IOCTL_BLOCK, drm_noop, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
 	DRM_IOCTL_DEF(DRM_IOCTL_UNBLOCK, drm_noop, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),

--- sys/external/bsd/drm2/dist/drm/drm_ioctl.c.orig	2015-03-06 21:43:11.000000000 -0800
+++ sys/external/bsd/drm2/dist/drm/drm_ioctl.c	2017-01-01 19:23:39.973037423 -0800
@@ -81,6 +81,27 @@
 	master->unique_size = 0;
 }

+#ifdef __NetBSD__
+/* From OpenBSD sys/dev/pci/drm/drm_drv.c */
+int
+drm_getpciinfo(struct drm_device *dev, void *data, struct drm_file *file_priv)
+{
+        struct drm_pciinfo *info = data;
+
+        info->domain = 0;
+        info->bus = dev->pdev->bus->number;
+        info->dev = PCI_SLOT(dev->pdev->devfn);
+        info->func = PCI_FUNC(dev->pdev->devfn);
+        info->vendor_id = dev->pdev->vendor;
+        info->device_id = dev->pdev->device;
+        info->subvendor_id = dev->pdev->subsystem_vendor;
+        info->subdevice_id = dev->pdev->subsystem_device;
+        info->revision_id = 0;
+
+        return 0;
+}
+#endif
+
 /**
  * Set the bus id.
  *

--- sys/external/bsd/drm2/dist/include/drm/drmP.h.orig	2015-04-28 22:22:44.000000000 -0700
+++ sys/external/bsd/drm2/dist/include/drm/drmP.h	2017-01-01 19:13:24.699068087 -0800
@@ -1445,6 +1445,11 @@
 				/* Misc. IOCTL support (drm_ioctl.h) */
 extern int drm_irq_by_busid(struct drm_device *dev, void *data,
 			    struct drm_file *file_priv);
+#ifdef __NetBSD__
+/* From OpenBSD sys/dev/pci/drm/drm_drv.c */
+extern int drm_getpciinfo(struct drm_device *dev, void *data,
+			    struct drm_file *);
+#endif
 extern int drm_getunique(struct drm_device *dev, void *data,
 			 struct drm_file *file_priv);
 extern int drm_setunique(struct drm_device *dev, void *data,

--- sys/external/bsd/drm2/dist/uapi/drm/drm.h.orig	2014-09-17 15:22:45.000000000 -0700
+++ sys/external/bsd/drm2/dist/uapi/drm/drm.h	2017-01-01 22:58:34.518731069 -0800
@@ -147,6 +147,21 @@
 	char __user *desc;	  /**< User-space buffer to hold desc */
 };

+#ifdef __NetBSD__
+/* From OpenBSD src/sys/dev/pci/drm/drm.h */
+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;
+};
+#endif
+
 /**
  * DRM_IOCTL_GET_UNIQUE ioctl argument type.
  *
@@ -703,7 +718,10 @@
 #define DRM_IOCTL_GEM_OPEN		DRM_IOWR(0x0b, struct drm_gem_open)
 #define DRM_IOCTL_GET_CAP		DRM_IOWR(0x0c, struct drm_get_cap)
 #define DRM_IOCTL_SET_CLIENT_CAP	DRM_IOW( 0x0d, struct drm_set_client_cap)
-
+#ifdef __NetBSD__
+/* From OpenBSD src/sys/dev/pci/drm/drm.h */
+#define DRM_IOCTL_GET_PCIINFO           DRM_IOR( 0x0f, struct drm_pciinfo)
+#endif
 #define DRM_IOCTL_SET_UNIQUE		DRM_IOW( 0x10, struct drm_unique)
 #define DRM_IOCTL_AUTH_MAGIC		DRM_IOW( 0x11, struct drm_auth)
 #define DRM_IOCTL_BLOCK			DRM_IOWR(0x12, struct drm_block)

--- sys/external/bsd/drm2/drm/drm_drv.c.orig	2015-11-12 22:31:10.000000000 -0800
+++ sys/external/bsd/drm2/drm/drm_drv.c	2017-01-01 22:19:18.451282104 -0800
@@ -121,6 +121,10 @@
 	DRM_IOCTL_DEF(DRM_IOCTL_SET_CLIENT_CAP, drm_setclientcap, 0),
 	DRM_IOCTL_DEF(DRM_IOCTL_SET_VERSION, drm_setversion, DRM_MASTER),

+#ifdef __NetBSD__
+/* From OpenBSD sys/dev/pci/drm/drm_drv.c */
+        DRM_IOCTL_DEF(DRM_IOCTL_GET_PCIINFO, drm_getpciinfo, DRM_UNLOCKED|DRM_RENDER_ALLOW),
+#endif
 	DRM_IOCTL_DEF(DRM_IOCTL_SET_UNIQUE, drm_setunique, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
 	DRM_IOCTL_DEF(DRM_IOCTL_BLOCK, drm_noop, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
 	DRM_IOCTL_DEF(DRM_IOCTL_UNBLOCK, drm_noop, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),

--- sys/external/bsd/drm/dist/shared-core/drm.h.orig	2009-06-19 18:07:10.000000000 -0700
+++ sys/external/bsd/drm/dist/shared-core/drm.h	2017-01-01 22:47:19.574093173 -0800
@@ -994,6 +994,22 @@
 	uint64_t size;
 };

+#ifdef __NetBSD__
+/* From OpenBSD sys/dev/pci/drm/drm.h */
+
+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;
+};
+#endif
+
 #include "drm_mode.h"

 /**
@@ -1021,6 +1037,11 @@
 #define DRM_IOCTL_GEM_FLINK		DRM_IOWR(0x0a, struct drm_gem_flink)
 #define DRM_IOCTL_GEM_OPEN		DRM_IOWR(0x0b, struct drm_gem_open)

+#ifdef __NetBSD__
+/* OpenBSD assigns 0x15 */
+#define DRM_IOCTL_GET_PCIINFO           DRM_IOR( 0x0f, struct drm_pciinfo)
+#endif
+
 #define DRM_IOCTL_SET_UNIQUE		DRM_IOW( 0x10, struct drm_unique)
 #define DRM_IOCTL_AUTH_MAGIC		DRM_IOW( 0x11, struct drm_auth)
 #define DRM_IOCTL_BLOCK			DRM_IOWR(0x12, struct drm_block)

>Release-Note:

>Audit-Trail:

Responsible-Changed-From-To: kern-bug-people->riastradh
Responsible-Changed-By: dholland@NetBSD.org
Responsible-Changed-When: Fri, 06 Jan 2017 22:18:01 +0000
Responsible-Changed-Why:
over to drm-bug-people


From: "Taylor R Campbell" <riastradh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/51786 CVS commit: xsrc/external/mit/libdrm/dist
Date: Sun, 9 Sep 2018 02:00:29 +0000

 Module Name:	xsrc
 Committed By:	riastradh
 Date:		Sun Sep  9 02:00:29 UTC 2018

 Modified Files:
 	xsrc/external/mit/libdrm/dist: xf86drm.c

 Log Message:
 Implement drmParsePciBusInfo and drmParsePciDeviceInfo.

 Uses drmGetBusid for bus info, and pciN with libpci for device info.

 Aims to address PR pkg/51795 without needing the kernel changes
 suggested in PR kern/51786.  Compile-tested only sofar -- should
 confirm before closing any PRs.

 (Sorry it took so long for me to find a round tuit!)

 XXX pullup-7
 XXX pullup-8

 ...along with src/external/mit/xorg/lib/libdrm/Makefile r1.16 for
 libpci.


 To generate a diff of this commit:
 cvs rdiff -u -r1.16 -r1.17 xsrc/external/mit/libdrm/dist/xf86drm.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: "Taylor R Campbell" <riastradh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/51786 CVS commit: xsrc/external/mit/libdrm/dist
Date: Sun, 9 Sep 2018 03:05:37 +0000

 Module Name:	xsrc
 Committed By:	riastradh
 Date:		Sun Sep  9 03:05:37 UTC 2018

 Modified Files:
 	xsrc/external/mit/libdrm/dist: xf86drm.c

 Log Message:
 Don't make setversion failure fatal -- only master can set it.

 PR pkg/51795
 PR kern/51786

 XXX pullup-7
 XXX pullup-8


 To generate a diff of this commit:
 cvs rdiff -u -r1.17 -r1.18 xsrc/external/mit/libdrm/dist/xf86drm.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: "Taylor R Campbell" <riastradh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/51786 CVS commit: xsrc/external/mit/libdrm/dist
Date: Sun, 9 Sep 2018 03:24:46 +0000

 Module Name:	xsrc
 Committed By:	riastradh
 Date:		Sun Sep  9 03:24:46 UTC 2018

 Modified Files:
 	xsrc/external/mit/libdrm/dist: xf86drm.c

 Log Message:
 Let's get the sense of strncmp correct, shall we?

 PR pkg/51795
 PR kern/51786

 XXX pullup-7
 XXX pullup-8


 To generate a diff of this commit:
 cvs rdiff -u -r1.18 -r1.19 xsrc/external/mit/libdrm/dist/xf86drm.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: open->closed
State-Changed-By: maya@NetBSD.org
State-Changed-When: Sun, 18 Aug 2019 05:56:24 +0000
State-Changed-Why:
Fixed without new ioctl.


>Unformatted:

Home	PR Database Search