Re: add a new DRM_IOCTL_GET_PCIINFO ioctl
> Date: Wed, 23 Nov 2016 11:46:41 +1100
> From: Jonathan Gray
>
> On Tue, Nov 22, 2016 at 09:26:21PM +0100, Mark Kettenis wrote:
> > > Date: Sat, 19 Nov 2016 19:27:25 +1100
> > > From: Jonathan Gray
> > >
> > > To pull pci information from the kernel for drm devices we need a common
> > > drm ioctl. This is a requirement for implementing functions in libdrm
> > > which are used by Mesa >= 13.
> > >
> > > To not clash with drm headers this is added via pciio.h at kettenis'
> > > suggestion.
> > >
> > > The ioctl number reuses that of DRM_IOCTL_ADD_MAP, a DRI1 ioctl
> > > we dropped support for, to avoid using a number that might be later
> > > used in linux.
> >
> > Sorry for dropping the ball on this. My original thought was that we
> > would have a 'p' ioctl instead of a 'd' ioctl with a name that is more
> > in line with the existing ioctls in . Having a #define
> > for DRM_IOCTL_GET_PCIINFO in feels wrong.
> >
> > Looking at your diffs, I wonder if we shouldn't just add the
> > DRM_IOCTL_GET_PCIINFO define to for the kernel,
> > and put a copy (protectected by #ifdef __OpenBSD__) in
> > libdrm/xf86drm.c.
>
> Sure, updated diff
ok kettenis@
> Index: drm.h
> ===
> RCS file: /cvs/src/sys/dev/pci/drm/drm.h,v
> retrieving revision 1.20
> diff -u -p -r1.20 drm.h
> --- drm.h 23 Sep 2015 23:12:11 - 1.20
> +++ drm.h 23 Nov 2016 00:30:50 -
> @@ -706,6 +706,20 @@ struct drm_event_vblank {
> u_int32_treserved;
> };
>
> +#ifdef __OpenBSD__
> +struct drm_pciinfo {
> + uint16_tdomain;
> + uint8_t bus;
> + uint8_t dev;
> + uint8_t func;
> + uint16_tvendor_id;
> + uint16_tdevice_id;
> + uint16_tsubvendor_id;
> + uint16_tsubdevice_id;
> + uint8_t revision_id;
> +};
> +#endif
> +
> #include "drm_mode.h"
>
> #define DRM_IOCTL_BASE 'd'
> @@ -734,7 +748,11 @@ struct drm_event_vblank {
> #define DRM_IOCTL_BLOCK DRM_IOWR(0x12, struct drm_block)
> #define DRM_IOCTL_UNBLOCKDRM_IOWR(0x13, struct drm_block)
> #define DRM_IOCTL_CONTROLDRM_IOW( 0x14, struct drm_control)
> +#ifdef __OpenBSD__
> +#define DRM_IOCTL_GET_PCIINFODRM_IOR( 0x15, struct
> drm_pciinfo)
> +#else
> #define DRM_IOCTL_ADD_MAPDRM_IOWR(0x15, struct drm_map)
> +#endif
> #define DRM_IOCTL_ADD_BUFS DRM_IOWR(0x16, struct drm_buf_desc)
> #define DRM_IOCTL_MARK_BUFS DRM_IOW( 0x17, struct drm_buf_desc)
> #define DRM_IOCTL_INFO_BUFS DRM_IOWR(0x18, struct drm_buf_info)
> Index: drm_drv.c
> ===
> RCS file: /cvs/src/sys/dev/pci/drm/drm_drv.c,v
> retrieving revision 1.149
> diff -u -p -r1.149 drm_drv.c
> --- drm_drv.c 15 Sep 2016 02:00:17 - 1.149
> +++ drm_drv.c 23 Nov 2016 00:30:51 -
> @@ -81,6 +81,7 @@ int drm_version(struct drm_device *, vo
> int drm_setversion(struct drm_device *, void *, struct drm_file *);
> int drm_getmagic(struct drm_device *, void *, struct drm_file *);
> int drm_authmagic(struct drm_device *, void *, struct drm_file *);
> +int drm_getpciinfo(struct drm_device *, void *, struct drm_file *);
> int drm_file_cmp(struct drm_file *, struct drm_file *);
> SPLAY_PROTOTYPE(drm_file_tree, drm_file, link, drm_file_cmp);
>
> @@ -120,6 +121,8 @@ static struct drm_ioctl_desc drm_ioctls[
> DRM_IOCTL_DEF(DRM_IOCTL_SET_SAREA_CTX, drm_setsareactx,
> DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
> DRM_IOCTL_DEF(DRM_IOCTL_GET_SAREA_CTX, drm_getsareactx, DRM_AUTH),
> #else
> + DRM_IOCTL_DEF(DRM_IOCTL_GET_PCIINFO, drm_getpciinfo,
> DRM_UNLOCKED|DRM_RENDER_ALLOW),
> +
> DRM_IOCTL_DEF(DRM_IOCTL_SET_SAREA_CTX, drm_noop,
> DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
> DRM_IOCTL_DEF(DRM_IOCTL_GET_SAREA_CTX, drm_noop, DRM_AUTH),
> #endif
> @@ -1345,5 +1348,23 @@ int drm_pcie_get_speed_cap_mask(struct d
>
> DRM_INFO("probing gen 2 caps for device 0x%04x:0x%04x = %x/%x\n",
> PCI_VENDOR(id), PCI_PRODUCT(id), lnkcap, lnkcap2);
> + return 0;
> +}
> +
> +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;
> }
>
Re: add a new DRM_IOCTL_GET_PCIINFO ioctl
On Tue, Nov 22, 2016 at 09:26:21PM +0100, Mark Kettenis wrote:
> > Date: Sat, 19 Nov 2016 19:27:25 +1100
> > From: Jonathan Gray
> >
> > To pull pci information from the kernel for drm devices we need a common
> > drm ioctl. This is a requirement for implementing functions in libdrm
> > which are used by Mesa >= 13.
> >
> > To not clash with drm headers this is added via pciio.h at kettenis'
> > suggestion.
> >
> > The ioctl number reuses that of DRM_IOCTL_ADD_MAP, a DRI1 ioctl
> > we dropped support for, to avoid using a number that might be later
> > used in linux.
>
> Sorry for dropping the ball on this. My original thought was that we
> would have a 'p' ioctl instead of a 'd' ioctl with a name that is more
> in line with the existing ioctls in . Having a #define
> for DRM_IOCTL_GET_PCIINFO in feels wrong.
>
> Looking at your diffs, I wonder if we shouldn't just add the
> DRM_IOCTL_GET_PCIINFO define to for the kernel,
> and put a copy (protectected by #ifdef __OpenBSD__) in
> libdrm/xf86drm.c.
Sure, updated diff
Index: drm.h
===
RCS file: /cvs/src/sys/dev/pci/drm/drm.h,v
retrieving revision 1.20
diff -u -p -r1.20 drm.h
--- drm.h 23 Sep 2015 23:12:11 - 1.20
+++ drm.h 23 Nov 2016 00:30:50 -
@@ -706,6 +706,20 @@ struct drm_event_vblank {
u_int32_treserved;
};
+#ifdef __OpenBSD__
+struct drm_pciinfo {
+ uint16_tdomain;
+ uint8_t bus;
+ uint8_t dev;
+ uint8_t func;
+ uint16_tvendor_id;
+ uint16_tdevice_id;
+ uint16_tsubvendor_id;
+ uint16_tsubdevice_id;
+ uint8_t revision_id;
+};
+#endif
+
#include "drm_mode.h"
#define DRM_IOCTL_BASE 'd'
@@ -734,7 +748,11 @@ struct drm_event_vblank {
#define DRM_IOCTL_BLOCKDRM_IOWR(0x12, struct drm_block)
#define DRM_IOCTL_UNBLOCK DRM_IOWR(0x13, struct drm_block)
#define DRM_IOCTL_CONTROL DRM_IOW( 0x14, struct drm_control)
+#ifdef __OpenBSD__
+#define DRM_IOCTL_GET_PCIINFO DRM_IOR( 0x15, struct drm_pciinfo)
+#else
#define DRM_IOCTL_ADD_MAP DRM_IOWR(0x15, struct drm_map)
+#endif
#define DRM_IOCTL_ADD_BUFS DRM_IOWR(0x16, struct drm_buf_desc)
#define DRM_IOCTL_MARK_BUFSDRM_IOW( 0x17, struct drm_buf_desc)
#define DRM_IOCTL_INFO_BUFSDRM_IOWR(0x18, struct drm_buf_info)
Index: drm_drv.c
===
RCS file: /cvs/src/sys/dev/pci/drm/drm_drv.c,v
retrieving revision 1.149
diff -u -p -r1.149 drm_drv.c
--- drm_drv.c 15 Sep 2016 02:00:17 - 1.149
+++ drm_drv.c 23 Nov 2016 00:30:51 -
@@ -81,6 +81,7 @@ intdrm_version(struct drm_device *, vo
int drm_setversion(struct drm_device *, void *, struct drm_file *);
int drm_getmagic(struct drm_device *, void *, struct drm_file *);
int drm_authmagic(struct drm_device *, void *, struct drm_file *);
+int drm_getpciinfo(struct drm_device *, void *, struct drm_file *);
int drm_file_cmp(struct drm_file *, struct drm_file *);
SPLAY_PROTOTYPE(drm_file_tree, drm_file, link, drm_file_cmp);
@@ -120,6 +121,8 @@ static struct drm_ioctl_desc drm_ioctls[
DRM_IOCTL_DEF(DRM_IOCTL_SET_SAREA_CTX, drm_setsareactx,
DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
DRM_IOCTL_DEF(DRM_IOCTL_GET_SAREA_CTX, drm_getsareactx, DRM_AUTH),
#else
+ DRM_IOCTL_DEF(DRM_IOCTL_GET_PCIINFO, drm_getpciinfo,
DRM_UNLOCKED|DRM_RENDER_ALLOW),
+
DRM_IOCTL_DEF(DRM_IOCTL_SET_SAREA_CTX, drm_noop,
DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
DRM_IOCTL_DEF(DRM_IOCTL_GET_SAREA_CTX, drm_noop, DRM_AUTH),
#endif
@@ -1345,5 +1348,23 @@ int drm_pcie_get_speed_cap_mask(struct d
DRM_INFO("probing gen 2 caps for device 0x%04x:0x%04x = %x/%x\n",
PCI_VENDOR(id), PCI_PRODUCT(id), lnkcap, lnkcap2);
+ return 0;
+}
+
+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;
}
Re: add a new DRM_IOCTL_GET_PCIINFO ioctl
> Date: Sat, 19 Nov 2016 19:27:25 +1100
> From: Jonathan Gray
>
> To pull pci information from the kernel for drm devices we need a common
> drm ioctl. This is a requirement for implementing functions in libdrm
> which are used by Mesa >= 13.
>
> To not clash with drm headers this is added via pciio.h at kettenis'
> suggestion.
>
> The ioctl number reuses that of DRM_IOCTL_ADD_MAP, a DRI1 ioctl
> we dropped support for, to avoid using a number that might be later
> used in linux.
Sorry for dropping the ball on this. My original thought was that we
would have a 'p' ioctl instead of a 'd' ioctl with a name that is more
in line with the existing ioctls in . Having a #define
for DRM_IOCTL_GET_PCIINFO in feels wrong.
Looking at your diffs, I wonder if we shouldn't just add the
DRM_IOCTL_GET_PCIINFO define to for the kernel,
and put a copy (protectected by #ifdef __OpenBSD__) in
libdrm/xf86drm.c.
The implementation looks good, but...
> Index: dev/pci/drm/drm_drv.c
> ===
> RCS file: /cvs/src/sys/dev/pci/drm/drm_drv.c,v
> retrieving revision 1.149
> diff -u -p -r1.149 drm_drv.c
> --- dev/pci/drm/drm_drv.c 15 Sep 2016 02:00:17 - 1.149
> +++ dev/pci/drm/drm_drv.c 19 Nov 2016 07:12:06 -
> @@ -50,6 +50,7 @@
> #include
> #include /* for TIOCSGRP */
> #include
> +#include /* for DRM_IOCTL_GET_PCIINFO */
>
> #include
> #include
> @@ -81,6 +82,7 @@ int drm_version(struct drm_device *, vo
> int drm_setversion(struct drm_device *, void *, struct drm_file *);
> int drm_getmagic(struct drm_device *, void *, struct drm_file *);
> int drm_authmagic(struct drm_device *, void *, struct drm_file *);
> +int drm_getpciinfo(struct drm_device *, void *, struct drm_file *);
> int drm_file_cmp(struct drm_file *, struct drm_file *);
> SPLAY_PROTOTYPE(drm_file_tree, drm_file, link, drm_file_cmp);
>
> @@ -120,6 +122,8 @@ static struct drm_ioctl_desc drm_ioctls[
> DRM_IOCTL_DEF(DRM_IOCTL_SET_SAREA_CTX, drm_setsareactx,
> DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
> DRM_IOCTL_DEF(DRM_IOCTL_GET_SAREA_CTX, drm_getsareactx, DRM_AUTH),
> #else
> + DRM_IOCTL_DEF(DRM_IOCTL_GET_PCIINFO, drm_getpciinfo, 0),
> +
> DRM_IOCTL_DEF(DRM_IOCTL_SET_SAREA_CTX, drm_noop,
> DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
> DRM_IOCTL_DEF(DRM_IOCTL_GET_SAREA_CTX, drm_noop, DRM_AUTH),
> #endif
> @@ -254,11 +258,12 @@ pledge_ioctl_drm(struct proc *p, long co
> if (ioctl->flags & DRM_RENDER_ALLOW)
> return 0;
>
> + switch (com) {
> + case DRM_IOCTL_GET_PCIINFO:
...I think we should just mark DRM_IOCTL_GET_PCIINFO as DRM_RENDER_ALLOW.
> /*
>* These are dangerous, but we have to allow them until we
>* have prime/dma-buf support.
>*/
> - switch (com) {
> case DRM_IOCTL_GET_MAGIC:
> case DRM_IOCTL_GEM_OPEN:
> return 0;
> @@ -1345,5 +1350,23 @@ int drm_pcie_get_speed_cap_mask(struct d
>
> DRM_INFO("probing gen 2 caps for device 0x%04x:0x%04x = %x/%x\n",
> PCI_VENDOR(id), PCI_PRODUCT(id), lnkcap, lnkcap2);
> + return 0;
> +}
> +
> +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;
> }
> Index: sys/pciio.h
> ===
> RCS file: /cvs/src/sys/sys/pciio.h,v
> retrieving revision 1.7
> diff -u -p -r1.7 pciio.h
> --- sys/pciio.h 5 Sep 2010 18:14:33 - 1.7
> +++ sys/pciio.h 19 Nov 2016 07:12:06 -
> @@ -60,6 +60,18 @@ struct pci_vga {
> int pv_decode;
> };
>
> +struct drm_pciinfo {
> + uint16_tdomain;
> + uint8_t bus;
> + uint8_t dev;
> + uint8_t func;
> + uint16_tvendor_id;
> + uint16_tdevice_id;
> + uint16_tsubvendor_id;
> + uint16_tsubdevice_id;
> + uint8_t revision_id;
> +};
> +
> #define PCI_VGA_UNLOCK 0x00
> #define PCI_VGA_LOCK0x01
> #define PCI_VGA_TRYLOCK 0x02
> @@ -74,5 +86,7 @@ struct pci_vga {
> #define PCIOCGETVGA _IOWR('p', 6, struct pci_vga)
> #define PCIOCSETVGA _IOWR('p', 7, struct pci_vga)
> #define PCIOCREADMASK _IOWR('p', 8, struct pci_io)
> +
> +#define DRM_IOCTL_GET_PCIINFO _IOR('d', 0x15, struct drm_pciinfo)
>
> #endif /* !_SYS_PCIIO_H_ */
>
>
add a new DRM_IOCTL_GET_PCIINFO ioctl
To pull pci information from the kernel for drm devices we need a common
drm ioctl. This is a requirement for implementing functions in libdrm
which are used by Mesa >= 13.
To not clash with drm headers this is added via pciio.h at kettenis'
suggestion.
The ioctl number reuses that of DRM_IOCTL_ADD_MAP, a DRI1 ioctl
we dropped support for, to avoid using a number that might be later
used in linux.
The currently unused values in the latest linux drm.h seem to be
0x0e
0x0f
0x2f
0x3b
0x3c
0x3d
0x3e
0xbf - 0xff ?
(driver specific ioctls 0x40 -> 0x9f)
Index: dev/pci/drm/drm_drv.c
===
RCS file: /cvs/src/sys/dev/pci/drm/drm_drv.c,v
retrieving revision 1.149
diff -u -p -r1.149 drm_drv.c
--- dev/pci/drm/drm_drv.c 15 Sep 2016 02:00:17 - 1.149
+++ dev/pci/drm/drm_drv.c 19 Nov 2016 07:12:06 -
@@ -50,6 +50,7 @@
#include
#include /* for TIOCSGRP */
#include
+#include /* for DRM_IOCTL_GET_PCIINFO */
#include
#include
@@ -81,6 +82,7 @@ intdrm_version(struct drm_device *, vo
int drm_setversion(struct drm_device *, void *, struct drm_file *);
int drm_getmagic(struct drm_device *, void *, struct drm_file *);
int drm_authmagic(struct drm_device *, void *, struct drm_file *);
+int drm_getpciinfo(struct drm_device *, void *, struct drm_file *);
int drm_file_cmp(struct drm_file *, struct drm_file *);
SPLAY_PROTOTYPE(drm_file_tree, drm_file, link, drm_file_cmp);
@@ -120,6 +122,8 @@ static struct drm_ioctl_desc drm_ioctls[
DRM_IOCTL_DEF(DRM_IOCTL_SET_SAREA_CTX, drm_setsareactx,
DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
DRM_IOCTL_DEF(DRM_IOCTL_GET_SAREA_CTX, drm_getsareactx, DRM_AUTH),
#else
+ DRM_IOCTL_DEF(DRM_IOCTL_GET_PCIINFO, drm_getpciinfo, 0),
+
DRM_IOCTL_DEF(DRM_IOCTL_SET_SAREA_CTX, drm_noop,
DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
DRM_IOCTL_DEF(DRM_IOCTL_GET_SAREA_CTX, drm_noop, DRM_AUTH),
#endif
@@ -254,11 +258,12 @@ pledge_ioctl_drm(struct proc *p, long co
if (ioctl->flags & DRM_RENDER_ALLOW)
return 0;
+ switch (com) {
+ case DRM_IOCTL_GET_PCIINFO:
/*
* These are dangerous, but we have to allow them until we
* have prime/dma-buf support.
*/
- switch (com) {
case DRM_IOCTL_GET_MAGIC:
case DRM_IOCTL_GEM_OPEN:
return 0;
@@ -1345,5 +1350,23 @@ int drm_pcie_get_speed_cap_mask(struct d
DRM_INFO("probing gen 2 caps for device 0x%04x:0x%04x = %x/%x\n",
PCI_VENDOR(id), PCI_PRODUCT(id), lnkcap, lnkcap2);
+ return 0;
+}
+
+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;
}
Index: sys/pciio.h
===
RCS file: /cvs/src/sys/sys/pciio.h,v
retrieving revision 1.7
diff -u -p -r1.7 pciio.h
--- sys/pciio.h 5 Sep 2010 18:14:33 - 1.7
+++ sys/pciio.h 19 Nov 2016 07:12:06 -
@@ -60,6 +60,18 @@ struct pci_vga {
int pv_decode;
};
+struct drm_pciinfo {
+ uint16_tdomain;
+ uint8_t bus;
+ uint8_t dev;
+ uint8_t func;
+ uint16_tvendor_id;
+ uint16_tdevice_id;
+ uint16_tsubvendor_id;
+
