* [RFCv2 PATCH 0/5] virtual GEM provider
@ 2012-02-08 23:19 Ben Widawsky
2012-02-08 23:19 ` [PATCH 1/5] drm/vgem: " Ben Widawsky
` (5 more replies)
0 siblings, 6 replies; 22+ messages in thread
From: Ben Widawsky @ 2012-02-08 23:19 UTC (permalink / raw)
To: dri-devel; +Cc: Ben Widawsky
Incorporated some of the feedback given to Adam's original patch.
My intention is to use this to do some dmabuf work/testing with i915
since it seemed too difficult to get some of Dave Airlie's stuff
working, and I really don't feel like learning anything about nouveau if
I can avoid it. (though I plan to do that later as well).
I think what's missing is a shrinker, left in size for mmap (didn't see
anyone respond to Daniel Vetter's comment), munmap/remap probably won't
work, do quite a bit more testing; that's all I can think of at the
moment.
The most basic of tests for this are here:
git://people.freedesktop.org/~bwidawsk/vgem-gpu-tools master
I've also pushed this repo here:
git://people.freedesktop.org/~bwidawsk/drm-intel vgem
Adam Jackson (1):
drm/vgem: virtual GEM provider
Ben Widawsky (4):
drm/vgem: fops should be separate and constified
drm/vgem: Add a drm_vgem_gem_object
drm/vgem: getparam ioctl
drm/vgem: properly implement mmap
drivers/gpu/drm/Kconfig | 8 +
drivers/gpu/drm/Makefile | 1 +
drivers/gpu/drm/vgem/Makefile | 4 +
drivers/gpu/drm/vgem/vgem_drv.c | 377 +++++++++++++++++++++++++++++++++++++++
include/drm/vgem_drm.h | 62 +++++++
5 files changed, 452 insertions(+), 0 deletions(-)
create mode 100644 drivers/gpu/drm/vgem/Makefile
create mode 100644 drivers/gpu/drm/vgem/vgem_drv.c
create mode 100644 include/drm/vgem_drm.h
--
1.7.9
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 1/5] drm/vgem: virtual GEM provider
2012-02-08 23:19 [RFCv2 PATCH 0/5] virtual GEM provider Ben Widawsky
@ 2012-02-08 23:19 ` Ben Widawsky
2012-02-08 23:28 ` Chris Wilson
2012-02-16 13:52 ` Jakob Bornecrantz
2012-02-08 23:19 ` [PATCH 2/5] drm/vgem: fops should be separate and constified Ben Widawsky
` (4 subsequent siblings)
5 siblings, 2 replies; 22+ messages in thread
From: Ben Widawsky @ 2012-02-08 23:19 UTC (permalink / raw)
To: dri-devel
From: Adam Jackson <ajax@redhat.com>
This is about as minimal of a virtual GEM service as possible. My plan
is to use this with non-native-3D hardware for buffer sharing between X
and DRI.
The current drisw winsys assumes an unmodified X server, which means
it's hopelessly inefficient for both the push direction of SwapBuffers/
DrawPixels and the pull direction of GLX_EXT_texture_from_pixmap. I'm
still working through the details of what the xserver support will look
like, but in broad strokes it's "use vgem for CreatePixmap and
optionally the shadowfb".
Obviously alpha quality, mostly looking for feedback on the approach or
any glaring bugs. Eventually I'd like to see solutions for sharing gem
objects between drm devices and/or the dma_buf API, but that's a ways
down the road.
Signed-off-by: Adam Jackson <ajax@redhat.com>
---
drivers/gpu/drm/Kconfig | 8 ++
drivers/gpu/drm/Makefile | 1 +
drivers/gpu/drm/vgem/Makefile | 4 +
drivers/gpu/drm/vgem/vgem_drv.c | 262 +++++++++++++++++++++++++++++++++++++++
include/drm/vgem_drm.h | 51 ++++++++
5 files changed, 326 insertions(+), 0 deletions(-)
create mode 100644 drivers/gpu/drm/vgem/Makefile
create mode 100644 drivers/gpu/drm/vgem/vgem_drv.c
create mode 100644 include/drm/vgem_drm.h
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 2418429..566c468 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -159,6 +159,14 @@ config DRM_SAVAGE
Choose this option if you have a Savage3D/4/SuperSavage/Pro/Twister
chipset. If M is selected the module will be called savage.
+config DRM_VGEM
+ tristate "Virtual GEM provider"
+ depends on DRM
+ help
+ Choose this option to get a virtual graphics memory manager,
+ as used by Mesa's software renderer for enhanced performance.
+ If M is selected the module will be called vgem.
+
source "drivers/gpu/drm/exynos/Kconfig"
source "drivers/gpu/drm/vmwgfx/Kconfig"
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 0cde1b8..021bf8a 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -34,6 +34,7 @@ obj-$(CONFIG_DRM_SIS) += sis/
obj-$(CONFIG_DRM_SAVAGE)+= savage/
obj-$(CONFIG_DRM_VMWGFX)+= vmwgfx/
obj-$(CONFIG_DRM_VIA) +=via/
+obj-$(CONFIG_DRM_VGEM) += vgem/
obj-$(CONFIG_DRM_NOUVEAU) +=nouveau/
obj-$(CONFIG_DRM_EXYNOS) +=exynos/
obj-$(CONFIG_DRM_GMA500) += gma500/
diff --git a/drivers/gpu/drm/vgem/Makefile b/drivers/gpu/drm/vgem/Makefile
new file mode 100644
index 0000000..3f4c7b8
--- /dev/null
+++ b/drivers/gpu/drm/vgem/Makefile
@@ -0,0 +1,4 @@
+ccflags-y := -Iinclude/drm
+vgem-y := vgem_drv.o
+
+obj-$(CONFIG_DRM_VGEM) += vgem.o
diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c
new file mode 100644
index 0000000..82c6787
--- /dev/null
+++ b/drivers/gpu/drm/vgem/vgem_drv.c
@@ -0,0 +1,262 @@
+/*
+ * Copyright 2011 Red Hat, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software")
+ * to deal in the software without restriction, including without limitation
+ * on the rights to use, copy, modify, merge, publish, distribute, sub
+ * license, and/or sell copies of the Software, and to permit persons to whom
+ * them Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTIBILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS BE LIABLE FOR ANY CLAIM, DAMAGES, OR OTHER LIABILITY, WHETHER
+ * IN AN ACTION OF CONTRACT, TORT, OR OTHERWISE, ARISING FROM, OUT OF OR IN
+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+ *
+ * Authors:
+ * Adam Jackson <ajax@redhat.com>
+ */
+
+/**
+ * This is vgem, a (non-hardware-backed) GEM service. This is used by Mesa's
+ * software renderer and the X server for efficient buffer sharing.
+ */
+
+#include "drmP.h"
+#include "drm.h"
+#include "vgem_drm.h"
+#include <linux/module.h>
+#include <linux/ramfs.h>
+
+#define DRIVER_NAME "vgem"
+#define DRIVER_DESC "Virtual GEM provider"
+#define DRIVER_DATE "20120112"
+#define DRIVER_MAJOR 1
+#define DRIVER_MINOR 0
+
+static int vgem_load(struct drm_device *dev, unsigned long flags)
+{
+ return 0;
+}
+
+static int vgem_unload(struct drm_device *dev)
+{
+ return 0;
+}
+
+static void vgem_preclose(struct drm_device *dev, struct drm_file *file)
+{
+}
+
+static void vgem_lastclose(struct drm_device *dev)
+{
+}
+
+static int vgem_gem_init_object(struct drm_gem_object *obj)
+{
+ return 0;
+}
+
+static void vgem_gem_free_object(struct drm_gem_object *obj)
+{
+ if (obj->map_list.map)
+ drm_gem_free_mmap_offset(obj);
+
+ drm_gem_object_release(obj);
+}
+
+/* XXX I don't think this is ever hit */
+static int vgem_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
+{
+ BUG();
+}
+
+static struct vm_operations_struct vgem_gem_vm_ops = {
+ .fault = vgem_gem_fault,
+ .open = drm_gem_vm_open,
+ .close = drm_gem_vm_close,
+};
+
+/* ioctls */
+
+static struct drm_gem_object *vgem_gem_create(struct drm_device *dev,
+ struct drm_file *file,
+ unsigned int *handle,
+ unsigned long size)
+{
+ int err;
+ struct drm_gem_object *gem_object;
+
+ size = roundup(size, PAGE_SIZE);
+
+ gem_object = kzalloc(sizeof(*gem_object), GFP_KERNEL);
+ if (!gem_object)
+ return ERR_PTR(-ENOMEM);
+
+ if ((err = drm_gem_object_init(dev, gem_object, size)))
+ goto out;
+
+ if ((err = drm_gem_create_mmap_offset(gem_object)))
+ goto mmap_out;
+
+ if ((err = drm_gem_handle_create(file, gem_object, handle)))
+ goto handle_out;
+
+ drm_gem_object_unreference_unlocked(gem_object);
+
+ return gem_object;
+
+handle_out:
+ drm_gem_free_mmap_offset(gem_object);
+
+mmap_out:
+ drm_gem_object_release(gem_object);
+
+out:
+ kfree(gem_object);
+
+ return ERR_PTR(err);
+}
+
+static int vgem_gem_create_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *file)
+{
+ struct vgem_gem_create *args = data;
+ struct drm_gem_object *gem_object;
+
+ gem_object = vgem_gem_create(dev, file, &args->handle, args->size);
+
+ if (IS_ERR(gem_object))
+ return PTR_ERR(gem_object);
+
+ return 0;
+}
+
+static int vgem_gem_mmap_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *file)
+{
+ struct vgem_gem_mmap *args = data;
+ struct drm_gem_object *obj;
+ uintptr_t addr;
+
+ obj = drm_gem_object_lookup(dev, file, args->handle);
+ if (!obj)
+ return -ENOENT;
+
+ obj->filp->private_data = obj;
+
+ down_write(¤t->mm->mmap_sem);
+ addr = do_mmap(obj->filp, 0, args->size, PROT_READ | PROT_WRITE,
+ MAP_SHARED, 0);
+ up_write(¤t->mm->mmap_sem);
+
+ drm_gem_object_unreference_unlocked(obj);
+
+ if (IS_ERR((void *)addr))
+ return PTR_ERR((void *)addr);
+
+ args->mapped = addr;
+
+ return 0;
+}
+
+static struct drm_ioctl_desc vgem_ioctls[] = {
+ DRM_IOCTL_DEF_DRV(VGEM_GEM_CREATE, vgem_gem_create_ioctl,
+ DRM_UNLOCKED | DRM_AUTH),
+ DRM_IOCTL_DEF_DRV(VGEM_GEM_MMAP, vgem_gem_mmap_ioctl,
+ DRM_UNLOCKED | DRM_AUTH),
+};
+
+static struct drm_driver vgem_driver = {
+ .driver_features = DRIVER_BUS_PLATFORM | DRIVER_GEM,
+ .load = vgem_load,
+ .unload = vgem_unload,
+ .preclose = vgem_preclose,
+ .lastclose = vgem_lastclose,
+ .gem_init_object = vgem_gem_init_object,
+ .gem_free_object = vgem_gem_free_object,
+ .gem_vm_ops = &vgem_gem_vm_ops,
+ .ioctls = vgem_ioctls,
+ .fops = {
+ .owner = THIS_MODULE,
+ .open = drm_open,
+ .mmap = drm_gem_mmap,
+ .poll = drm_poll,
+ .read = drm_read,
+ .unlocked_ioctl = drm_ioctl,
+ .release = drm_release,
+ },
+ .name = DRIVER_NAME,
+ .desc = DRIVER_DESC,
+ .date = DRIVER_DATE,
+ .major = DRIVER_MAJOR,
+ .minor = DRIVER_MINOR,
+};
+
+static int vgem_platform_probe(struct platform_device *pdev)
+{
+ vgem_driver.num_ioctls = DRM_ARRAY_SIZE(vgem_ioctls);
+
+ return drm_platform_init(&vgem_driver, pdev);
+}
+
+static int vgem_platform_remove(struct platform_device *pdev)
+{
+ drm_platform_exit(&vgem_driver, pdev);
+
+ return 0;
+}
+
+static struct platform_driver vgem_platform_driver = {
+ .probe = vgem_platform_probe,
+ .remove = __devexit_p(vgem_platform_remove),
+ .driver = {
+ .owner = THIS_MODULE,
+ .name = DRIVER_NAME,
+ },
+};
+
+static struct platform_device *vgem_device;
+
+static int __init vgem_init(void)
+{
+ int ret;
+
+ if ((ret = platform_driver_register(&vgem_platform_driver)))
+ return ret;
+
+ vgem_device = platform_device_alloc("vgem", -1);
+ if (!vgem_device) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ ret = platform_device_add(vgem_device);
+ if (!ret)
+ return 0;
+
+out:
+ platform_device_put(vgem_device);
+ platform_driver_unregister(&vgem_platform_driver);
+
+ return ret;
+}
+
+static void __exit vgem_exit(void)
+{
+ platform_device_unregister(vgem_device);
+ platform_driver_unregister(&vgem_platform_driver);
+}
+
+module_init(vgem_init);
+module_exit(vgem_exit);
+
+MODULE_AUTHOR("Red Hat, Inc.");
+MODULE_DESCRIPTION(DRIVER_DESC);
+MODULE_LICENSE("GPL and additional rights");
diff --git a/include/drm/vgem_drm.h b/include/drm/vgem_drm.h
new file mode 100644
index 0000000..d73b537
--- /dev/null
+++ b/include/drm/vgem_drm.h
@@ -0,0 +1,51 @@
+/*
+ * Copyright 2011 Red Hat, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software")
+ * to deal in the software without restriction, including without limitation
+ * on the rights to use, copy, modify, merge, publish, distribute, sub
+ * license, and/or sell copies of the Software, and to permit persons to whom
+ * them Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTIBILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS BE LIABLE FOR ANY CLAIM, DAMAGES, OR OTHER LIABILITY, WHETHER
+ * IN AN ACTION OF CONTRACT, TORT, OR OTHERWISE, ARISING FROM, OUT OF OR IN
+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+#ifndef VGEM_DRM_H
+#define VGEM_DRM_H
+
+/* Bare API largely ripped off from exynos driver */
+
+struct vgem_gem_create {
+ unsigned int size;
+ unsigned int flags;
+ unsigned int handle;
+};
+
+struct vgem_gem_mmap {
+ unsigned int handle;
+ unsigned int size;
+ uint64_t mapped;
+};
+
+#define DRM_VGEM_GEM_CREATE 0x00
+#define DRM_VGEM_GEM_MMAP 0x01
+
+#define DRM_IOCTL_VGEM_GEM_CREATE \
+ DRM_IOWR(DRM_COMMAND_BASE + DRM_VGEM_GEM_CREATE, \
+ struct vgem_gem_create)
+
+#define DRM_IOCTL_VGEM_GEM_MMAP \
+ DRM_IOWR(DRM_COMMAND_BASE + DRM_VGEM_GEM_MMAP, \
+ struct vgem_gem_mmap)
+
+#endif
--
1.7.9
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 2/5] drm/vgem: fops should be separate and constified
2012-02-08 23:19 [RFCv2 PATCH 0/5] virtual GEM provider Ben Widawsky
2012-02-08 23:19 ` [PATCH 1/5] drm/vgem: " Ben Widawsky
@ 2012-02-08 23:19 ` Ben Widawsky
2012-02-08 23:19 ` [PATCH 3/5] drm/vgem: Add a drm_vgem_gem_object Ben Widawsky
` (3 subsequent siblings)
5 siblings, 0 replies; 22+ messages in thread
From: Ben Widawsky @ 2012-02-08 23:19 UTC (permalink / raw)
To: dri-devel; +Cc: Ben Widawsky
...or else it just won't build.
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
drivers/gpu/drm/vgem/vgem_drv.c | 20 +++++++++++---------
1 files changed, 11 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c
index 82c6787..3084389 100644
--- a/drivers/gpu/drm/vgem/vgem_drv.c
+++ b/drivers/gpu/drm/vgem/vgem_drv.c
@@ -173,6 +173,16 @@ static struct drm_ioctl_desc vgem_ioctls[] = {
DRM_UNLOCKED | DRM_AUTH),
};
+static const struct file_operations vgem_driver_fops = {
+ .owner = THIS_MODULE,
+ .open = drm_open,
+ .mmap = drm_gem_mmap,
+ .poll = drm_poll,
+ .read = drm_read,
+ .unlocked_ioctl = drm_ioctl,
+ .release = drm_release,
+};
+
static struct drm_driver vgem_driver = {
.driver_features = DRIVER_BUS_PLATFORM | DRIVER_GEM,
.load = vgem_load,
@@ -183,15 +193,7 @@ static struct drm_driver vgem_driver = {
.gem_free_object = vgem_gem_free_object,
.gem_vm_ops = &vgem_gem_vm_ops,
.ioctls = vgem_ioctls,
- .fops = {
- .owner = THIS_MODULE,
- .open = drm_open,
- .mmap = drm_gem_mmap,
- .poll = drm_poll,
- .read = drm_read,
- .unlocked_ioctl = drm_ioctl,
- .release = drm_release,
- },
+ .fops = &vgem_driver_fops,
.name = DRIVER_NAME,
.desc = DRIVER_DESC,
.date = DRIVER_DATE,
--
1.7.9
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 3/5] drm/vgem: Add a drm_vgem_gem_object
2012-02-08 23:19 [RFCv2 PATCH 0/5] virtual GEM provider Ben Widawsky
2012-02-08 23:19 ` [PATCH 1/5] drm/vgem: " Ben Widawsky
2012-02-08 23:19 ` [PATCH 2/5] drm/vgem: fops should be separate and constified Ben Widawsky
@ 2012-02-08 23:19 ` Ben Widawsky
2012-02-08 23:19 ` [PATCH 4/5] drm/vgem: getparam ioctl Ben Widawsky
` (2 subsequent siblings)
5 siblings, 0 replies; 22+ messages in thread
From: Ben Widawsky @ 2012-02-08 23:19 UTC (permalink / raw)
To: dri-devel; +Cc: Ben Widawsky
and use it
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
drivers/gpu/drm/vgem/vgem_drv.c | 13 ++++++++++---
1 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c
index 3084389..d47bd71 100644
--- a/drivers/gpu/drm/vgem/vgem_drv.c
+++ b/drivers/gpu/drm/vgem/vgem_drv.c
@@ -40,6 +40,10 @@
#define DRIVER_MAJOR 1
#define DRIVER_MINOR 0
+struct drm_vgem_gem_object {
+ struct drm_gem_object base;
+};
+
static int vgem_load(struct drm_device *dev, unsigned long flags)
{
return 0;
@@ -90,15 +94,18 @@ static struct drm_gem_object *vgem_gem_create(struct drm_device *dev,
unsigned int *handle,
unsigned long size)
{
- int err;
+ struct drm_vgem_gem_object *obj;
struct drm_gem_object *gem_object;
+ int err;
size = roundup(size, PAGE_SIZE);
- gem_object = kzalloc(sizeof(*gem_object), GFP_KERNEL);
- if (!gem_object)
+ obj = kzalloc(sizeof(*obj), GFP_KERNEL);
+ if (!obj)
return ERR_PTR(-ENOMEM);
+ gem_object = &obj->base;
+
if ((err = drm_gem_object_init(dev, gem_object, size)))
goto out;
--
1.7.9
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 4/5] drm/vgem: getparam ioctl
2012-02-08 23:19 [RFCv2 PATCH 0/5] virtual GEM provider Ben Widawsky
` (2 preceding siblings ...)
2012-02-08 23:19 ` [PATCH 3/5] drm/vgem: Add a drm_vgem_gem_object Ben Widawsky
@ 2012-02-08 23:19 ` Ben Widawsky
2012-02-15 21:22 ` Adam Jackson
2012-02-08 23:19 ` [PATCH 5/5] drm/vgem: properly implement mmap Ben Widawsky
2012-02-14 0:14 ` [RFCv2 PATCH 0/5] virtual GEM provider Adam Jackson
5 siblings, 1 reply; 22+ messages in thread
From: Ben Widawsky @ 2012-02-08 23:19 UTC (permalink / raw)
To: dri-devel; +Cc: Ben Widawsky
Similar to i915, it's nice to be able to query this device uniquely and
get some info
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
drivers/gpu/drm/vgem/vgem_drv.c | 29 +++++++++++++++++++++--------
include/drm/vgem_drm.h | 11 +++++++++++
2 files changed, 32 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c
index d47bd71..7a7a05f 100644
--- a/drivers/gpu/drm/vgem/vgem_drv.c
+++ b/drivers/gpu/drm/vgem/vgem_drv.c
@@ -158,26 +158,39 @@ static int vgem_gem_mmap_ioctl(struct drm_device *dev, void *data,
obj->filp->private_data = obj;
- down_write(¤t->mm->mmap_sem);
- addr = do_mmap(obj->filp, 0, args->size, PROT_READ | PROT_WRITE,
- MAP_SHARED, 0);
- up_write(¤t->mm->mmap_sem);
+ BUG_ON(!obj->map_list.map);
- drm_gem_object_unreference_unlocked(obj);
+ args->mapped = obj->map_list.hash.key << PAGE_SHIFT;
- if (IS_ERR((void *)addr))
- return PTR_ERR((void *)addr);
+ return 0;
+}
- args->mapped = addr;
+static int vgem_gem_getparam_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *file)
+{
+ struct vgem_gem_getparam *args = data;
+ int value=0, ret;
+
+ switch (args->param) {
+ case VGEM_PARAM_IS_VGEM:
+ value = 1;
+ }
+
+ ret = copy_to_user(args->value, &value, sizeof(int));
+ if (ret)
+ return ret;
return 0;
}
+
static struct drm_ioctl_desc vgem_ioctls[] = {
DRM_IOCTL_DEF_DRV(VGEM_GEM_CREATE, vgem_gem_create_ioctl,
DRM_UNLOCKED | DRM_AUTH),
DRM_IOCTL_DEF_DRV(VGEM_GEM_MMAP, vgem_gem_mmap_ioctl,
DRM_UNLOCKED | DRM_AUTH),
+ DRM_IOCTL_DEF_DRV(VGEM_GEM_GETPARAM, vgem_gem_getparam_ioctl,
+ DRM_UNLOCKED),
};
static const struct file_operations vgem_driver_fops = {
diff --git a/include/drm/vgem_drm.h b/include/drm/vgem_drm.h
index d73b537..df83503 100644
--- a/include/drm/vgem_drm.h
+++ b/include/drm/vgem_drm.h
@@ -37,8 +37,15 @@ struct vgem_gem_mmap {
uint64_t mapped;
};
+struct vgem_gem_getparam {
+#define VGEM_PARAM_IS_VGEM 1
+ unsigned int param;
+ unsigned int *value;
+};
+
#define DRM_VGEM_GEM_CREATE 0x00
#define DRM_VGEM_GEM_MMAP 0x01
+#define DRM_VGEM_GEM_GETPARAM 0x02
#define DRM_IOCTL_VGEM_GEM_CREATE \
DRM_IOWR(DRM_COMMAND_BASE + DRM_VGEM_GEM_CREATE, \
@@ -48,4 +55,8 @@ struct vgem_gem_mmap {
DRM_IOWR(DRM_COMMAND_BASE + DRM_VGEM_GEM_MMAP, \
struct vgem_gem_mmap)
+#define DRM_IOCTL_VGEM_GEM_GETPARAM \
+ DRM_IOWR(DRM_COMMAND_BASE + DRM_VGEM_GEM_GETPARAM, \
+ struct vgem_gem_getparam)
+
#endif
--
1.7.9
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 5/5] drm/vgem: properly implement mmap
2012-02-08 23:19 [RFCv2 PATCH 0/5] virtual GEM provider Ben Widawsky
` (3 preceding siblings ...)
2012-02-08 23:19 ` [PATCH 4/5] drm/vgem: getparam ioctl Ben Widawsky
@ 2012-02-08 23:19 ` Ben Widawsky
2012-02-08 23:36 ` Chris Wilson
2012-02-14 2:18 ` Eric Anholt
2012-02-14 0:14 ` [RFCv2 PATCH 0/5] virtual GEM provider Adam Jackson
5 siblings, 2 replies; 22+ messages in thread
From: Ben Widawsky @ 2012-02-08 23:19 UTC (permalink / raw)
To: dri-devel; +Cc: Ben Widawsky
Mostly copied from i915 gtt mmaps, this will properly fault in pages as
the user tries to use them. The only thing of note are that no
prefaulting occurs, so perhaps some kind of madvise will happen later if
needed.
The only other thing missing right not is shrinker support, which will
come next after I figure out if locking is actually required right now.
Hmm, and now that I think about it, mremap, and munmap may not work
either.
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
drivers/gpu/drm/vgem/vgem_drv.c | 105 ++++++++++++++++++++++++++++++++++++--
1 files changed, 99 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c
index 7a7a05f..16f88ee 100644
--- a/drivers/gpu/drm/vgem/vgem_drv.c
+++ b/drivers/gpu/drm/vgem/vgem_drv.c
@@ -33,6 +33,7 @@
#include "vgem_drm.h"
#include <linux/module.h>
#include <linux/ramfs.h>
+#include <linux/shmem_fs.h>
#define DRIVER_NAME "vgem"
#define DRIVER_DESC "Virtual GEM provider"
@@ -40,8 +41,11 @@
#define DRIVER_MAJOR 1
#define DRIVER_MINOR 0
+#define to_vgem_bo(x) container_of(x, struct drm_vgem_gem_object, base)
+
struct drm_vgem_gem_object {
struct drm_gem_object base;
+ struct page **pages;
};
static int vgem_load(struct drm_device *dev, unsigned long flags)
@@ -67,21 +71,109 @@ static int vgem_gem_init_object(struct drm_gem_object *obj)
return 0;
}
+static void vgem_gem_put_pages(struct drm_vgem_gem_object *obj)
+{
+ int num_pages = obj->base.size / PAGE_SIZE;
+ int i;
+
+ for (i = 0; i < num_pages; i++) {
+ page_cache_release(obj->pages[i]);
+ }
+
+ drm_free_large(obj->pages);
+ obj->pages = NULL;
+}
+
static void vgem_gem_free_object(struct drm_gem_object *obj)
{
- if (obj->map_list.map)
+ struct drm_vgem_gem_object *vgem_obj = to_vgem_bo(obj);
+
+ if (obj)
drm_gem_free_mmap_offset(obj);
drm_gem_object_release(obj);
+
+ if (vgem_obj->pages)
+ vgem_gem_put_pages(vgem_obj);
+
+ kfree(vgem_obj);
+}
+
+static int vgem_gem_get_pages(struct drm_vgem_gem_object *obj)
+{
+ struct address_space *mapping;
+ gfp_t gfpmask = __GFP_NORETRY | __GFP_NOWARN;
+ int num_pages, i, ret = 0;
+
+ num_pages = obj->base.size / PAGE_SIZE;
+
+ if (!obj->pages) {
+ obj->pages = drm_malloc_ab(num_pages, sizeof(struct page *));
+ if (obj->pages == NULL)
+ return -ENOMEM;
+ }
+
+ mapping = obj->base.filp->f_path.dentry->d_inode->i_mapping;
+ gfpmask |= mapping_gfp_mask(mapping);
+
+ if (WARN_ON(mapping == NULL))
+ return VM_FAULT_SIGBUS;
+
+ for (i = 0; i < num_pages; i++) {
+ struct page *page;
+ page = shmem_read_mapping_page_gfp(mapping, i, gfpmask);
+ if (IS_ERR(page)) {
+ ret = PTR_ERR(page);
+ goto err_out;
+ }
+ obj->pages[i] = page;
+ }
+
+ return ret;
+
+err_out:
+ while (i--)
+ page_cache_release(obj->pages[i]);
+ drm_free_large(obj->pages);
+ obj->pages = NULL;
+ return ret;
}
-/* XXX I don't think this is ever hit */
static int vgem_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
{
- BUG();
+ struct drm_vgem_gem_object *obj = to_vgem_bo(vma->vm_private_data);
+ loff_t num_pages;
+ pgoff_t page_offset;
+ int ret;
+
+ /* We don't use vmf->pgoff since that has the fake offset */
+ page_offset = ((unsigned long)vmf->virtual_address - vma->vm_start) >>
+ PAGE_SHIFT;
+
+ num_pages = obj->base.size / PAGE_SIZE;
+
+ if (WARN_ON(page_offset > num_pages))
+ return VM_FAULT_SIGBUS;
+
+ ret = vgem_gem_get_pages(obj);
+ if (ret)
+ return ret;
+
+ ret = vm_insert_page(vma, (unsigned long)vmf->virtual_address,
+ obj->pages[page_offset]);
+
+ /* Pretty dumb handler for now */
+ switch (ret) {
+ case 0:
+ case -ERESTARTSYS:
+ case -EINTR:
+ return VM_FAULT_NOPAGE;
+ default:
+ return VM_FAULT_SIGBUS;
+ }
}
-static struct vm_operations_struct vgem_gem_vm_ops = {
+static const struct vm_operations_struct vgem_gem_vm_ops = {
.fault = vgem_gem_fault,
.open = drm_gem_vm_open,
.close = drm_gem_vm_close,
@@ -150,7 +242,6 @@ static int vgem_gem_mmap_ioctl(struct drm_device *dev, void *data,
{
struct vgem_gem_mmap *args = data;
struct drm_gem_object *obj;
- uintptr_t addr;
obj = drm_gem_object_lookup(dev, file, args->handle);
if (!obj)
@@ -160,7 +251,9 @@ static int vgem_gem_mmap_ioctl(struct drm_device *dev, void *data,
BUG_ON(!obj->map_list.map);
- args->mapped = obj->map_list.hash.key << PAGE_SHIFT;
+ args->mapped = (uint64_t)obj->map_list.hash.key << PAGE_SHIFT;
+
+ drm_gem_object_unreference_unlocked(obj);
return 0;
}
--
1.7.9
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH 1/5] drm/vgem: virtual GEM provider
2012-02-08 23:19 ` [PATCH 1/5] drm/vgem: " Ben Widawsky
@ 2012-02-08 23:28 ` Chris Wilson
2012-02-16 13:52 ` Jakob Bornecrantz
1 sibling, 0 replies; 22+ messages in thread
From: Chris Wilson @ 2012-02-08 23:28 UTC (permalink / raw)
To: Ben Widawsky, dri-devel
On Thu, 9 Feb 2012 00:19:27 +0100, Ben Widawsky <ben@bwidawsk.net> wrote:
> From: Adam Jackson <ajax@redhat.com>
> +static void vgem_gem_free_object(struct drm_gem_object *obj)
> +{
> + if (obj->map_list.map)
> + drm_gem_free_mmap_offset(obj);
> +
> + drm_gem_object_release(obj);
> +}
drm_gem_free_mmap_offset() should be pushed down into
drm_gem_object_release(). Out of the 6 distinct users of
drm_gem_object_release(), 3 call drm_gem_free_mmap_offset() just prior
and gma500 forgets to.
I do accept that it is a separate patch though ;-)
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 5/5] drm/vgem: properly implement mmap
2012-02-08 23:19 ` [PATCH 5/5] drm/vgem: properly implement mmap Ben Widawsky
@ 2012-02-08 23:36 ` Chris Wilson
2012-02-09 9:18 ` Ben Widawsky
2012-02-09 10:20 ` Daniel Vetter
2012-02-14 2:18 ` Eric Anholt
1 sibling, 2 replies; 22+ messages in thread
From: Chris Wilson @ 2012-02-08 23:36 UTC (permalink / raw)
To: dri-devel; +Cc: Ben Widawsky
On Thu, 9 Feb 2012 00:19:31 +0100, Ben Widawsky <ben@bwidawsk.net> wrote:
> Mostly copied from i915 gtt mmaps, this will properly fault in pages as
> the user tries to use them. The only thing of note are that no
> prefaulting occurs, so perhaps some kind of madvise will happen later if
> needed.
What's the goal here? Why not copy the simpler (from the driver
perspective) CPU mmaps? I presume that this will be required for dmabuf
in some form?
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 5/5] drm/vgem: properly implement mmap
2012-02-08 23:36 ` Chris Wilson
@ 2012-02-09 9:18 ` Ben Widawsky
2012-02-09 10:20 ` Daniel Vetter
1 sibling, 0 replies; 22+ messages in thread
From: Ben Widawsky @ 2012-02-09 9:18 UTC (permalink / raw)
To: Chris Wilson; +Cc: dri-devel
On 02/09/2012 12:36 AM, Chris Wilson wrote:
> On Thu, 9 Feb 2012 00:19:31 +0100, Ben Widawsky <ben@bwidawsk.net> wrote:
>> Mostly copied from i915 gtt mmaps, this will properly fault in pages as
>> the user tries to use them. The only thing of note are that no
>> prefaulting occurs, so perhaps some kind of madvise will happen later if
>> needed.
>
> What's the goal here? Why not copy the simpler (from the driver
> perspective) CPU mmaps? I presume that this will be required for dmabuf
> in some form?
> -Chris
>
That was what everyone seemed to want from the original RFC. I think
Eric said something about helping out valgrind. I don't recall the
other reasons.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 5/5] drm/vgem: properly implement mmap
2012-02-08 23:36 ` Chris Wilson
2012-02-09 9:18 ` Ben Widawsky
@ 2012-02-09 10:20 ` Daniel Vetter
1 sibling, 0 replies; 22+ messages in thread
From: Daniel Vetter @ 2012-02-09 10:20 UTC (permalink / raw)
To: Chris Wilson; +Cc: Ben Widawsky, dri-devel
On Wed, Feb 08, 2012 at 11:36:22PM +0000, Chris Wilson wrote:
> On Thu, 9 Feb 2012 00:19:31 +0100, Ben Widawsky <ben@bwidawsk.net> wrote:
> > Mostly copied from i915 gtt mmaps, this will properly fault in pages as
> > the user tries to use them. The only thing of note are that no
> > prefaulting occurs, so perhaps some kind of madvise will happen later if
> > needed.
>
> What's the goal here? Why not copy the simpler (from the driver
> perspective) CPU mmaps? I presume that this will be required for dmabuf
> in some form?
Yeah, for proper instead of hacked-up dma_buf mmap support I expect that
the expoerter needs to provide some sort of vm_insert_pfn_from_dma_buf
function. We need this so that non-coherent dma_buf objects can fake
coherency and also to allow (eventually) eviction. For that the exporter
needs to be able to handle page-faults, so I've figured we might want to
hack something together.
I know it's not a beauty, because drm isn't a fs yet ;-)
-Daniel
--
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFCv2 PATCH 0/5] virtual GEM provider
2012-02-08 23:19 [RFCv2 PATCH 0/5] virtual GEM provider Ben Widawsky
` (4 preceding siblings ...)
2012-02-08 23:19 ` [PATCH 5/5] drm/vgem: properly implement mmap Ben Widawsky
@ 2012-02-14 0:14 ` Adam Jackson
5 siblings, 0 replies; 22+ messages in thread
From: Adam Jackson @ 2012-02-14 0:14 UTC (permalink / raw)
To: Ben Widawsky; +Cc: dri-devel
On 2/8/12 6:19 PM, Ben Widawsky wrote:
> Incorporated some of the feedback given to Adam's original patch.
>
> My intention is to use this to do some dmabuf work/testing with i915
> since it seemed too difficult to get some of Dave Airlie's stuff
> working, and I really don't feel like learning anything about nouveau if
> I can avoid it. (though I plan to do that later as well).
>
> I think what's missing is a shrinker, left in size for mmap (didn't see
> anyone respond to Daniel Vetter's comment), munmap/remap probably won't
> work, do quite a bit more testing; that's all I can think of at the
> moment.
munmap appears to work. However if you do it after the close() you
segfault at at exit. That's probably suboptimal.
From an API perspective this is certainly enough for llvmpipe, and I
can work around munmap being quirky for now.
Reviewed-by: Adam Jackson <ajax@redhat.com>
- ajax
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 5/5] drm/vgem: properly implement mmap
2012-02-08 23:19 ` [PATCH 5/5] drm/vgem: properly implement mmap Ben Widawsky
2012-02-08 23:36 ` Chris Wilson
@ 2012-02-14 2:18 ` Eric Anholt
2012-02-14 7:49 ` Dave Airlie
1 sibling, 1 reply; 22+ messages in thread
From: Eric Anholt @ 2012-02-14 2:18 UTC (permalink / raw)
To: dri-devel; +Cc: Ben Widawsky
[-- Attachment #1.1: Type: text/plain, Size: 740 bytes --]
On Thu, 9 Feb 2012 00:19:31 +0100, Ben Widawsky <ben@bwidawsk.net> wrote:
> Mostly copied from i915 gtt mmaps, this will properly fault in pages as
> the user tries to use them. The only thing of note are that no
> prefaulting occurs, so perhaps some kind of madvise will happen later if
> needed.
>
> The only other thing missing right not is shrinker support, which will
> come next after I figure out if locking is actually required right now.
> Hmm, and now that I think about it, mremap, and munmap may not work
> either.
I still think the mmap ioctl should be fixed to just get the mmap
offset, and you use the normal mmap() syscall later. The
do_mmap-in-the-ioctl in the original GEM implementation was a bad idea.
[-- Attachment #1.2: Type: application/pgp-signature, Size: 197 bytes --]
[-- Attachment #2: Type: text/plain, Size: 159 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 5/5] drm/vgem: properly implement mmap
2012-02-14 2:18 ` Eric Anholt
@ 2012-02-14 7:49 ` Dave Airlie
0 siblings, 0 replies; 22+ messages in thread
From: Dave Airlie @ 2012-02-14 7:49 UTC (permalink / raw)
To: Eric Anholt; +Cc: Ben Widawsky, dri-devel
On Tue, Feb 14, 2012 at 2:18 AM, Eric Anholt <eric@anholt.net> wrote:
> On Thu, 9 Feb 2012 00:19:31 +0100, Ben Widawsky <ben@bwidawsk.net> wrote:
>> Mostly copied from i915 gtt mmaps, this will properly fault in pages as
>> the user tries to use them. The only thing of note are that no
>> prefaulting occurs, so perhaps some kind of madvise will happen later if
>> needed.
>>
>> The only other thing missing right not is shrinker support, which will
>> come next after I figure out if locking is actually required right now.
>> Hmm, and now that I think about it, mremap, and munmap may not work
>> either.
>
> I still think the mmap ioctl should be fixed to just get the mmap
> offset, and you use the normal mmap() syscall later. The
> do_mmap-in-the-ioctl in the original GEM implementation was a bad idea.
Yes not accepting an mmap inside things anymore, shouldn't have
accepted the one in GEM.
Dave.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 4/5] drm/vgem: getparam ioctl
2012-02-08 23:19 ` [PATCH 4/5] drm/vgem: getparam ioctl Ben Widawsky
@ 2012-02-15 21:22 ` Adam Jackson
2012-02-16 7:23 ` Dave Airlie
0 siblings, 1 reply; 22+ messages in thread
From: Adam Jackson @ 2012-02-15 21:22 UTC (permalink / raw)
To: Ben Widawsky; +Cc: dri-devel
On 2/8/12 6:19 PM, Ben Widawsky wrote:
> Similar to i915, it's nice to be able to query this device uniquely and
> get some info
>
> Signed-off-by: Ben Widawsky<ben@bwidawsk.net>
So, this is actually not especially useful as written. You'd like to be
able to use this to find the vgem device node, but since it's a
device-specific ioctl it collides with whatever happens to be
device-specific ioctl number 2 on whatever device you've opened. On
i915, that's I915_FLIP, and you promptly oops the machine.
Comedy. Gold.
Which is fine, really. The way I was anticipating finding the vgem node
was to just scrape the device node name out of
/sys/bus/platform/devices/vgem/cardN, because why do anything O(n) when
you could do it O(1). I guess that doesn't help OSes that aren't Linux,
and I guess if pressed to care about that I'd say hang it off drm_getcap.
- ajax
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 4/5] drm/vgem: getparam ioctl
2012-02-15 21:22 ` Adam Jackson
@ 2012-02-16 7:23 ` Dave Airlie
2012-02-20 17:12 ` Ben Widawsky
0 siblings, 1 reply; 22+ messages in thread
From: Dave Airlie @ 2012-02-16 7:23 UTC (permalink / raw)
To: Adam Jackson; +Cc: Ben Widawsky, dri-devel
On Wed, Feb 15, 2012 at 9:22 PM, Adam Jackson <ajax@redhat.com> wrote:
> On 2/8/12 6:19 PM, Ben Widawsky wrote:
>>
>> Similar to i915, it's nice to be able to query this device uniquely and
>> get some info
>>
>> Signed-off-by: Ben Widawsky<ben@bwidawsk.net>
>
>
> So, this is actually not especially useful as written. You'd like to be
> able to use this to find the vgem device node, but since it's a
> device-specific ioctl it collides with whatever happens to be
> device-specific ioctl number 2 on whatever device you've opened. On i915,
> that's I915_FLIP, and you promptly oops the machine.
>
We have DRM level caps that we could use for that if necessary.
I'm not sure we need a getparam for vgem if the only thing it reports
is VGEM status.
Though yeah what ajax said looking in /sys is the discovery path of choice.
Dave.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/5] drm/vgem: virtual GEM provider
2012-02-08 23:19 ` [PATCH 1/5] drm/vgem: " Ben Widawsky
2012-02-08 23:28 ` Chris Wilson
@ 2012-02-16 13:52 ` Jakob Bornecrantz
2012-02-20 17:10 ` Ben Widawsky
1 sibling, 1 reply; 22+ messages in thread
From: Jakob Bornecrantz @ 2012-02-16 13:52 UTC (permalink / raw)
To: Ben Widawsky; +Cc: dri-devel
----- Original Message -----
> From: Adam Jackson <ajax@redhat.com>
>
> This is about as minimal of a virtual GEM service as possible. My
> plan is to use this with non-native-3D hardware for buffer sharing
> between X and DRI.
>
> The current drisw winsys assumes an unmodified X server, which means
> it's hopelessly inefficient for both the push direction of
> SwapBuffers/DrawPixels and the pull direction of
> GLX_EXT_texture_from_pixmap. I'm still working through the details
> of what the xserver support will look like, but in broad strokes it's
> "use vgem for CreatePixmap and optionally the shadowfb".
>
> Obviously alpha quality, mostly looking for feedback on the approach
> or any glaring bugs. Eventually I'd like to see solutions for
> sharing gem objects between drm devices and/or the dma_buf API, but
> that's a ways down the road.
>
> Signed-off-by: Adam Jackson <ajax@redhat.com>
Any reason why you are not using the dumb_bo interface? I at least
would like to be able to offer vgem on the vmwgfx device when the
host has disabled 3D.
Cheers, Jakob.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/5] drm/vgem: virtual GEM provider
2012-02-16 13:52 ` Jakob Bornecrantz
@ 2012-02-20 17:10 ` Ben Widawsky
2012-02-21 15:03 ` Adam Jackson
0 siblings, 1 reply; 22+ messages in thread
From: Ben Widawsky @ 2012-02-20 17:10 UTC (permalink / raw)
To: Jakob Bornecrantz; +Cc: dri-devel
On Thu, 16 Feb 2012 05:52:12 -0800 (PST)
Jakob Bornecrantz <jakob@vmware.com> wrote:
> ----- Original Message -----
> > From: Adam Jackson <ajax@redhat.com>
> >
> > This is about as minimal of a virtual GEM service as possible. My
> > plan is to use this with non-native-3D hardware for buffer sharing
> > between X and DRI.
> >
> > The current drisw winsys assumes an unmodified X server, which means
> > it's hopelessly inefficient for both the push direction of
> > SwapBuffers/DrawPixels and the pull direction of
> > GLX_EXT_texture_from_pixmap. I'm still working through the details
> > of what the xserver support will look like, but in broad strokes it's
> > "use vgem for CreatePixmap and optionally the shadowfb".
> >
> > Obviously alpha quality, mostly looking for feedback on the approach
> > or any glaring bugs. Eventually I'd like to see solutions for
> > sharing gem objects between drm devices and/or the dma_buf API, but
> > that's a ways down the road.
> >
> > Signed-off-by: Adam Jackson <ajax@redhat.com>
>
> Any reason why you are not using the dumb_bo interface? I at least
> would like to be able to offer vgem on the vmwgfx device when the
> host has disabled 3D.
>
> Cheers, Jakob.
I thought dumb bo interface was an extension for an existing DRM GEM driver. In my
case, for prototyping dma_buf support, this is non-ideal. I'm not sure what Adam
thinks of this though (I imagine he wants a device independent driver though).
I may be completely off.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 4/5] drm/vgem: getparam ioctl
2012-02-16 7:23 ` Dave Airlie
@ 2012-02-20 17:12 ` Ben Widawsky
0 siblings, 0 replies; 22+ messages in thread
From: Ben Widawsky @ 2012-02-20 17:12 UTC (permalink / raw)
To: Dave Airlie; +Cc: dri-devel
On Thu, 16 Feb 2012 07:23:52 +0000
Dave Airlie <airlied@gmail.com> wrote:
> On Wed, Feb 15, 2012 at 9:22 PM, Adam Jackson <ajax@redhat.com> wrote:
> > On 2/8/12 6:19 PM, Ben Widawsky wrote:
> >>
> >> Similar to i915, it's nice to be able to query this device uniquely and
> >> get some info
> >>
> >> Signed-off-by: Ben Widawsky<ben@bwidawsk.net>
> >
> >
> > So, this is actually not especially useful as written. You'd like to be
> > able to use this to find the vgem device node, but since it's a
> > device-specific ioctl it collides with whatever happens to be
> > device-specific ioctl number 2 on whatever device you've opened. On i915,
> > that's I915_FLIP, and you promptly oops the machine.
> >
Hmm... it doesn't oops my machine.
>
> We have DRM level caps that we could use for that if necessary.
>
> I'm not sure we need a getparam for vgem if the only thing it reports
> is VGEM status.
The plan I had for it was to report whether or not prime support exists. But
I'll just call the prime ioctls and see if they fail instead.
>
> Though yeah what ajax said looking in /sys is the discovery path of choice.
I've done this now, it turned out quite nicely.
>
> Dave.
Thanks.
~Ben
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/5] drm/vgem: virtual GEM provider
2012-02-20 17:10 ` Ben Widawsky
@ 2012-02-21 15:03 ` Adam Jackson
2012-02-21 15:34 ` Dave Airlie
0 siblings, 1 reply; 22+ messages in thread
From: Adam Jackson @ 2012-02-21 15:03 UTC (permalink / raw)
To: Ben Widawsky; +Cc: dri-devel
[-- Attachment #1.1: Type: text/plain, Size: 1231 bytes --]
On Mon, 2012-02-20 at 18:10 +0100, Ben Widawsky wrote:
> On Thu, 16 Feb 2012 05:52:12 -0800 (PST)
> Jakob Bornecrantz <jakob@vmware.com> wrote:
> > Any reason why you are not using the dumb_bo interface? I at least
> > would like to be able to offer vgem on the vmwgfx device when the
> > host has disabled 3D.
>
> I thought dumb bo interface was an extension for an existing DRM GEM driver. In my
> case, for prototyping dma_buf support, this is non-ideal. I'm not sure what Adam
> thinks of this though (I imagine he wants a device independent driver though).
>
> I may be completely off.
Assuming by this you mean DRM_IOCTL_MODE_{CREATE,MAP,DESTROY}_DUMB, I
wasn't considering that because I wasn't expecting this device to expose
a scanout buffer. It's hard to imagine how it would, given that API,
since you'd need userspace to jam the framebuffer info in from above.
If the vmwgfx ddx wants to do this kind of fallback, great, but that's
just ddx logic, open the right drm device and change the dri driver
name. If you don't want to rely on a vgem device node existing... well,
tough, I guess, since the ioctl numbers will collide. It needs to exist
for the vesa-like case regardless.
- ajax
[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
[-- Attachment #2: Type: text/plain, Size: 159 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/5] drm/vgem: virtual GEM provider
2012-02-21 15:03 ` Adam Jackson
@ 2012-02-21 15:34 ` Dave Airlie
2012-02-21 16:08 ` Adam Jackson
0 siblings, 1 reply; 22+ messages in thread
From: Dave Airlie @ 2012-02-21 15:34 UTC (permalink / raw)
To: Adam Jackson; +Cc: dri-devel, Ben Widawsky
On Tue, Feb 21, 2012 at 3:03 PM, Adam Jackson <ajax@redhat.com> wrote:
> On Mon, 2012-02-20 at 18:10 +0100, Ben Widawsky wrote:
>> On Thu, 16 Feb 2012 05:52:12 -0800 (PST)
>> Jakob Bornecrantz <jakob@vmware.com> wrote:
>> > Any reason why you are not using the dumb_bo interface? I at least
>> > would like to be able to offer vgem on the vmwgfx device when the
>> > host has disabled 3D.
>>
>> I thought dumb bo interface was an extension for an existing DRM GEM driver. In my
>> case, for prototyping dma_buf support, this is non-ideal. I'm not sure what Adam
>> thinks of this though (I imagine he wants a device independent driver though).
>>
>> I may be completely off.
>
> Assuming by this you mean DRM_IOCTL_MODE_{CREATE,MAP,DESTROY}_DUMB, I
> wasn't considering that because I wasn't expecting this device to expose
> a scanout buffer. It's hard to imagine how it would, given that API,
> since you'd need userspace to jam the framebuffer info in from above.
Not sure what you mean there, those 3 APIs are just to create dumb
unaccelerated objects,
probably are fine for vgem's use. For scanout we create framebuffer
objects from a dumb object
then we do shove it back in from above.
So if the ioctls are doing the same thing we should just use the
generic dumb ioctls for vgem.
Dave.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/5] drm/vgem: virtual GEM provider
2012-02-21 15:34 ` Dave Airlie
@ 2012-02-21 16:08 ` Adam Jackson
2012-02-21 17:41 ` Ben Widawsky
0 siblings, 1 reply; 22+ messages in thread
From: Adam Jackson @ 2012-02-21 16:08 UTC (permalink / raw)
To: Dave Airlie; +Cc: dri-devel, Ben Widawsky
[-- Attachment #1.1: Type: text/plain, Size: 1307 bytes --]
On Tue, 2012-02-21 at 15:34 +0000, Dave Airlie wrote:
> Not sure what you mean there, those 3 APIs are just to create dumb
> unaccelerated objects,
> probably are fine for vgem's use. For scanout we create framebuffer
> objects from a dumb object
> then we do shove it back in from above.
>
> So if the ioctls are doing the same thing we should just use the
> generic dumb ioctls for vgem.
That's... not at all obvious from the comments or the ioctl argument.
/* create a dumb scanout buffer */
struct drm_mode_create_dumb {
uint32_t height;
uint32_t width;
uint32_t bpp;
uint32_t flags;
/* handle, pitch, size will be returned */
uint32_t handle;
uint32_t pitch;
uint64_t size;
};
This doesn't look like "here, kernel, I am handing you a pointer". It
looks instead like "please, kernel, create this thing for me", and the
comment above sure does imply the thing being created is explicitly for
scanout purposes.
If it's just creating unaccelerated objects and the "bind to
framebuffer" part is somewhere else, then sure, vgem should probably
just use that. I'm still going to need that to exist as its own device
node not backed by a physical device, but I don't think that's
contentious.
- ajax
[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
[-- Attachment #2: Type: text/plain, Size: 159 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/5] drm/vgem: virtual GEM provider
2012-02-21 16:08 ` Adam Jackson
@ 2012-02-21 17:41 ` Ben Widawsky
0 siblings, 0 replies; 22+ messages in thread
From: Ben Widawsky @ 2012-02-21 17:41 UTC (permalink / raw)
To: Adam Jackson; +Cc: dri-devel
On Tue, 21 Feb 2012 11:08:52 -0500
Adam Jackson <ajax@redhat.com> wrote:
> On Tue, 2012-02-21 at 15:34 +0000, Dave Airlie wrote:
>
> > Not sure what you mean there, those 3 APIs are just to create dumb
> > unaccelerated objects,
> > probably are fine for vgem's use. For scanout we create framebuffer
> > objects from a dumb object
> > then we do shove it back in from above.
> >
> > So if the ioctls are doing the same thing we should just use the
> > generic dumb ioctls for vgem.
>
> That's... not at all obvious from the comments or the ioctl argument.
>
> /* create a dumb scanout buffer */
> struct drm_mode_create_dumb {
> uint32_t height;
> uint32_t width;
> uint32_t bpp;
> uint32_t flags;
> /* handle, pitch, size will be returned */
> uint32_t handle;
> uint32_t pitch;
> uint64_t size;
> };
>
> This doesn't look like "here, kernel, I am handing you a pointer". It
> looks instead like "please, kernel, create this thing for me", and the
> comment above sure does imply the thing being created is explicitly for
> scanout purposes.
>
> If it's just creating unaccelerated objects and the "bind to
> framebuffer" part is somewhere else, then sure, vgem should probably
> just use that. I'm still going to need that to exist as its own device
> node not backed by a physical device, but I don't think that's
> contentious.
I'm on board with the dumb ioctls as well. I also need a device node,
and some test ioctls for the prime stuff, but mmap and create are no
issue to switch over at the moment.
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2012-02-21 17:42 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-02-08 23:19 [RFCv2 PATCH 0/5] virtual GEM provider Ben Widawsky
2012-02-08 23:19 ` [PATCH 1/5] drm/vgem: " Ben Widawsky
2012-02-08 23:28 ` Chris Wilson
2012-02-16 13:52 ` Jakob Bornecrantz
2012-02-20 17:10 ` Ben Widawsky
2012-02-21 15:03 ` Adam Jackson
2012-02-21 15:34 ` Dave Airlie
2012-02-21 16:08 ` Adam Jackson
2012-02-21 17:41 ` Ben Widawsky
2012-02-08 23:19 ` [PATCH 2/5] drm/vgem: fops should be separate and constified Ben Widawsky
2012-02-08 23:19 ` [PATCH 3/5] drm/vgem: Add a drm_vgem_gem_object Ben Widawsky
2012-02-08 23:19 ` [PATCH 4/5] drm/vgem: getparam ioctl Ben Widawsky
2012-02-15 21:22 ` Adam Jackson
2012-02-16 7:23 ` Dave Airlie
2012-02-20 17:12 ` Ben Widawsky
2012-02-08 23:19 ` [PATCH 5/5] drm/vgem: properly implement mmap Ben Widawsky
2012-02-08 23:36 ` Chris Wilson
2012-02-09 9:18 ` Ben Widawsky
2012-02-09 10:20 ` Daniel Vetter
2012-02-14 2:18 ` Eric Anholt
2012-02-14 7:49 ` Dave Airlie
2012-02-14 0:14 ` [RFCv2 PATCH 0/5] virtual GEM provider Adam Jackson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.