This reverts commit 502e95c6678505474f1056480310cd9382bacbac.
Originally vgem was just to have a drm driver for software-only renders like llvmpipe so that they integrate more nicely with the existing compositor protocols like DRI2/3.
Later on Ben extended it to serve as a dma-buf import/export test vehicle for debugging. Both of these use-cases imo make sense and are on the level of just yet another (virtual) drm driver.
But only just I realized that Google's CrOS plans with vgem are much more pervasive - the plan seems to be to use it as the generic software mmap path on top of any kind of hw driver buffer objects from the underlying SoC gfx hw. That means vgem is now common infrastructre which should work for all drivers and properly interact with them, and from that point-of-view it definitely falls short on three accounts:
- It directly forwards the current dma-buf ->mmap model, and the consensus for that has always been that it's a toy one at best useful for debugging or super-unlikely fallbacks. Originally it was added mostly as a concession to get people on top of the dma-buf train since Ion has this too.
For proper mmap support we really need some software begin/end signals, probably best in the form of a set of ioctls on the dma-buf fd that forward to the already existing ->begin/end_cpu_access hooks.
Not exposing this and not requiring userspace to use them puts a serious burden on any kind of driver that needs to occasionally handle a bit of coherency troubles. Which includes anything on ARM and even more so more fancy stuff like virtual hardware.
Furthermore the vgem mmap interface doesn't expose any begin/end ioctls either, which means we can't even use the existing dma-buf hooks properly and fix this up in vgem.
- vgem uses the dumb buffer api to expose the the mmap support. Generally I don't care about what kind of horror-shows of abi abuse drivers do (and there's lots of dumb buffer abused out there). But vgem as used now isn't just a driver, but core infrastructure. And the dumb mmap interface really is just for uploading the occasional splash screen, and definitely not for more elaborate buffer object usage.
Furthermore the dumb buffer api is explictily for kms drivers to be used as framebuffers. Which vgem just isn't.
- The userspace demonstration vehicle seems missing. Yes there's Ajax's old llvmpipe-on-vgem stuff, and there's Ben's old patches to use vgem to put dma-buf on i915 through it's paces. But vgem as-is has long eclipsed these use-cases, but the corresponding CrOS code hasn't been floated around. Which means it's pretty much impossible to gauge whether the proposed ABI is a good fit for the stated goals.
Worse still I tried to raise this with Stéphane on irc and wanted to discuss my concerns, but he just shrugged them off that intel hw issues aren't his concern (and a few other denials). While intel hw is one of the few used by CrOS which actually has a fully coherent gpu (well at least the ones with LLC).
Given all that I think vgem just isn't ready yet. I'm definitely not opposed to the overall idea, but I do think it needs to be fleshed out a bit more. And since the release of 4.1 is fairly close the best immediate action seems to be to temporarily revert the vgem driver for now.
Cc: Thomas Hellstrom thellstrom@vmware.com Cc: Dave Airlie airlied@redhat.com Cc: Stéphane Marchesin marcheu@chromium.org Cc: Rob Clark robdclark@gmail.com Cc: Adam Jackson ajax@redhat.com Cc: Zach Reizner zachr@google.com Signed-off-by: Daniel Vetter daniel.vetter@intel.com --- drivers/gpu/drm/Kconfig | 9 - drivers/gpu/drm/Makefile | 1 - drivers/gpu/drm/vgem/Makefile | 4 - drivers/gpu/drm/vgem/vgem_dma_buf.c | 94 ---------- drivers/gpu/drm/vgem/vgem_drv.c | 364 ------------------------------------ drivers/gpu/drm/vgem/vgem_drv.h | 57 ------ 6 files changed, 529 deletions(-) delete mode 100644 drivers/gpu/drm/vgem/Makefile delete mode 100644 drivers/gpu/drm/vgem/vgem_dma_buf.c delete mode 100644 drivers/gpu/drm/vgem/vgem_drv.c delete mode 100644 drivers/gpu/drm/vgem/vgem_drv.h
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index 47f2ce81b412..151a050129e7 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -165,15 +165,6 @@ 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/rockchip/Kconfig" diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index 4de8d9b006ec..44caabf13041 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -46,7 +46,6 @@ 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_ROCKCHIP) +=rockchip/ diff --git a/drivers/gpu/drm/vgem/Makefile b/drivers/gpu/drm/vgem/Makefile deleted file mode 100644 index 1055cb79096c..000000000000 --- a/drivers/gpu/drm/vgem/Makefile +++ /dev/null @@ -1,4 +0,0 @@ -ccflags-y := -Iinclude/drm -vgem-y := vgem_drv.o vgem_dma_buf.o - -obj-$(CONFIG_DRM_VGEM) += vgem.o diff --git a/drivers/gpu/drm/vgem/vgem_dma_buf.c b/drivers/gpu/drm/vgem/vgem_dma_buf.c deleted file mode 100644 index 0254438ad1a6..000000000000 --- a/drivers/gpu/drm/vgem/vgem_dma_buf.c +++ /dev/null @@ -1,94 +0,0 @@ -/* - * Copyright © 2012 Intel Corporation - * Copyright © 2014 The Chromium OS Authors - * - * 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 - * the rights to use, copy, modify, merge, publish, distribute, sublicense, - * and/or sell copies of the Software, and to permit persons to whom the - * 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 MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL - * THE AUTHORS OR COPYRIGHT HOLDERS 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: - * Ben Widawsky ben@bwidawsk.net - * - */ - -#include <linux/dma-buf.h> -#include "vgem_drv.h" - -struct sg_table *vgem_gem_prime_get_sg_table(struct drm_gem_object *gobj) -{ - struct drm_vgem_gem_object *obj = to_vgem_bo(gobj); - BUG_ON(obj->pages == NULL); - - return drm_prime_pages_to_sg(obj->pages, obj->base.size / PAGE_SIZE); -} - -int vgem_gem_prime_pin(struct drm_gem_object *gobj) -{ - struct drm_vgem_gem_object *obj = to_vgem_bo(gobj); - return vgem_gem_get_pages(obj); -} - -void vgem_gem_prime_unpin(struct drm_gem_object *gobj) -{ - struct drm_vgem_gem_object *obj = to_vgem_bo(gobj); - vgem_gem_put_pages(obj); -} - -void *vgem_gem_prime_vmap(struct drm_gem_object *gobj) -{ - struct drm_vgem_gem_object *obj = to_vgem_bo(gobj); - BUG_ON(obj->pages == NULL); - - return vmap(obj->pages, obj->base.size / PAGE_SIZE, 0, PAGE_KERNEL); -} - -void vgem_gem_prime_vunmap(struct drm_gem_object *obj, void *vaddr) -{ - vunmap(vaddr); -} - -struct drm_gem_object *vgem_gem_prime_import(struct drm_device *dev, - struct dma_buf *dma_buf) -{ - struct drm_vgem_gem_object *obj = NULL; - int ret; - - obj = kzalloc(sizeof(*obj), GFP_KERNEL); - if (obj == NULL) { - ret = -ENOMEM; - goto fail; - } - - ret = drm_gem_object_init(dev, &obj->base, dma_buf->size); - if (ret) { - ret = -ENOMEM; - goto fail_free; - } - - get_dma_buf(dma_buf); - - obj->base.dma_buf = dma_buf; - obj->use_dma_buf = true; - - return &obj->base; - -fail_free: - kfree(obj); -fail: - return ERR_PTR(ret); -} diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c deleted file mode 100644 index cb3b43525b2d..000000000000 --- a/drivers/gpu/drm/vgem/vgem_drv.c +++ /dev/null @@ -1,364 +0,0 @@ -/* - * Copyright 2011 Red Hat, Inc. - * Copyright © 2014 The Chromium OS Authors - * - * 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 - * Ben Widawsky ben@bwidawsk.net - */ - -/** - * 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 <linux/module.h> -#include <linux/ramfs.h> -#include <linux/shmem_fs.h> -#include <linux/dma-buf.h> -#include "vgem_drv.h" - -#define DRIVER_NAME "vgem" -#define DRIVER_DESC "Virtual GEM provider" -#define DRIVER_DATE "20120112" -#define DRIVER_MAJOR 1 -#define DRIVER_MINOR 0 - -void vgem_gem_put_pages(struct drm_vgem_gem_object *obj) -{ - drm_gem_put_pages(&obj->base, obj->pages, false, false); - obj->pages = NULL; -} - -static void vgem_gem_free_object(struct drm_gem_object *obj) -{ - struct drm_vgem_gem_object *vgem_obj = to_vgem_bo(obj); - - drm_gem_free_mmap_offset(obj); - - if (vgem_obj->use_dma_buf && obj->dma_buf) { - dma_buf_put(obj->dma_buf); - obj->dma_buf = NULL; - } - - drm_gem_object_release(obj); - - if (vgem_obj->pages) - vgem_gem_put_pages(vgem_obj); - - vgem_obj->pages = NULL; - - kfree(vgem_obj); -} - -int vgem_gem_get_pages(struct drm_vgem_gem_object *obj) -{ - struct page **pages; - - if (obj->pages || obj->use_dma_buf) - return 0; - - pages = drm_gem_get_pages(&obj->base); - if (IS_ERR(pages)) { - return PTR_ERR(pages); - } - - obj->pages = pages; - - return 0; -} - -static int vgem_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf) -{ - struct drm_vgem_gem_object *obj = vma->vm_private_data; - struct drm_device *dev = obj->base.dev; - 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 = DIV_ROUND_UP(obj->base.size, PAGE_SIZE); - - if (page_offset > num_pages) - return VM_FAULT_SIGBUS; - - mutex_lock(&dev->struct_mutex); - - ret = vm_insert_page(vma, (unsigned long)vmf->virtual_address, - obj->pages[page_offset]); - - mutex_unlock(&dev->struct_mutex); - switch (ret) { - case 0: - return VM_FAULT_NOPAGE; - case -ENOMEM: - return VM_FAULT_OOM; - case -EBUSY: - return VM_FAULT_RETRY; - case -EFAULT: - case -EINVAL: - return VM_FAULT_SIGBUS; - default: - WARN_ON(1); - return VM_FAULT_SIGBUS; - } -} - -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) -{ - struct drm_vgem_gem_object *obj; - struct drm_gem_object *gem_object; - int err; - - size = roundup(size, PAGE_SIZE); - - obj = kzalloc(sizeof(*obj), GFP_KERNEL); - if (!obj) - return ERR_PTR(-ENOMEM); - - gem_object = &obj->base; - - err = drm_gem_object_init(dev, gem_object, size); - if (err) - goto out; - - err = drm_gem_handle_create(file, gem_object, handle); - if (err) - goto handle_out; - - drm_gem_object_unreference_unlocked(gem_object); - - return gem_object; - -handle_out: - drm_gem_object_release(gem_object); -out: - kfree(obj); - return ERR_PTR(err); -} - -static int vgem_gem_dumb_create(struct drm_file *file, struct drm_device *dev, - struct drm_mode_create_dumb *args) -{ - struct drm_gem_object *gem_object; - uint64_t size; - uint64_t pitch = args->width * DIV_ROUND_UP(args->bpp, 8); - - size = args->height * pitch; - if (size == 0) - return -EINVAL; - - gem_object = vgem_gem_create(dev, file, &args->handle, size); - - if (IS_ERR(gem_object)) { - DRM_DEBUG_DRIVER("object creation failed\n"); - return PTR_ERR(gem_object); - } - - args->size = gem_object->size; - args->pitch = pitch; - - DRM_DEBUG_DRIVER("Created object of size %lld\n", size); - - return 0; -} - -int vgem_gem_dumb_map(struct drm_file *file, struct drm_device *dev, - uint32_t handle, uint64_t *offset) -{ - int ret = 0; - struct drm_gem_object *obj; - - mutex_lock(&dev->struct_mutex); - obj = drm_gem_object_lookup(dev, file, handle); - if (!obj) { - ret = -ENOENT; - goto unlock; - } - - if (!drm_vma_node_has_offset(&obj->vma_node)) { - ret = drm_gem_create_mmap_offset(obj); - if (ret) - goto unref; - } - - BUG_ON(!obj->filp); - - obj->filp->private_data = obj; - - ret = vgem_gem_get_pages(to_vgem_bo(obj)); - if (ret) - goto fail_get_pages; - - *offset = drm_vma_node_offset_addr(&obj->vma_node); - - goto unref; - -fail_get_pages: - drm_gem_free_mmap_offset(obj); -unref: - drm_gem_object_unreference(obj); -unlock: - mutex_unlock(&dev->struct_mutex); - return ret; -} - -int vgem_drm_gem_mmap(struct file *filp, struct vm_area_struct *vma) -{ - struct drm_file *priv = filp->private_data; - struct drm_device *dev = priv->minor->dev; - struct drm_vma_offset_node *node; - struct drm_gem_object *obj; - struct drm_vgem_gem_object *vgem_obj; - int ret = 0; - - mutex_lock(&dev->struct_mutex); - - node = drm_vma_offset_exact_lookup(dev->vma_offset_manager, - vma->vm_pgoff, - vma_pages(vma)); - if (!node) { - ret = -EINVAL; - goto out_unlock; - } else if (!drm_vma_node_is_allowed(node, filp)) { - ret = -EACCES; - goto out_unlock; - } - - obj = container_of(node, struct drm_gem_object, vma_node); - - vgem_obj = to_vgem_bo(obj); - - if (obj->dma_buf && vgem_obj->use_dma_buf) { - ret = dma_buf_mmap(obj->dma_buf, vma, 0); - goto out_unlock; - } - - if (!obj->dev->driver->gem_vm_ops) { - ret = -EINVAL; - goto out_unlock; - } - - vma->vm_flags |= VM_IO | VM_MIXEDMAP | VM_DONTEXPAND | VM_DONTDUMP; - vma->vm_ops = obj->dev->driver->gem_vm_ops; - vma->vm_private_data = vgem_obj; - vma->vm_page_prot = - pgprot_writecombine(vm_get_page_prot(vma->vm_flags)); - - mutex_unlock(&dev->struct_mutex); - drm_gem_vm_open(vma); - return ret; - -out_unlock: - mutex_unlock(&dev->struct_mutex); - - return ret; -} - - -static struct drm_ioctl_desc vgem_ioctls[] = { -}; - -static const struct file_operations vgem_driver_fops = { - .owner = THIS_MODULE, - .open = drm_open, - .mmap = vgem_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_GEM | DRIVER_PRIME, - .gem_free_object = vgem_gem_free_object, - .gem_vm_ops = &vgem_gem_vm_ops, - .ioctls = vgem_ioctls, - .fops = &vgem_driver_fops, - .dumb_create = vgem_gem_dumb_create, - .dumb_map_offset = vgem_gem_dumb_map, - .prime_handle_to_fd = drm_gem_prime_handle_to_fd, - .prime_fd_to_handle = drm_gem_prime_fd_to_handle, - .gem_prime_export = drm_gem_prime_export, - .gem_prime_import = vgem_gem_prime_import, - .gem_prime_pin = vgem_gem_prime_pin, - .gem_prime_unpin = vgem_gem_prime_unpin, - .gem_prime_get_sg_table = vgem_gem_prime_get_sg_table, - .gem_prime_vmap = vgem_gem_prime_vmap, - .gem_prime_vunmap = vgem_gem_prime_vunmap, - .name = DRIVER_NAME, - .desc = DRIVER_DESC, - .date = DRIVER_DATE, - .major = DRIVER_MAJOR, - .minor = DRIVER_MINOR, -}; - -struct drm_device *vgem_device; - -static int __init vgem_init(void) -{ - int ret; - - vgem_device = drm_dev_alloc(&vgem_driver, NULL); - if (!vgem_device) { - ret = -ENOMEM; - goto out; - } - - ret = drm_dev_register(vgem_device, 0); - - if (ret) - goto out_unref; - - return 0; - -out_unref: - drm_dev_unref(vgem_device); -out: - return ret; -} - -static void __exit vgem_exit(void) -{ - drm_dev_unregister(vgem_device); - drm_dev_unref(vgem_device); -} - -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/drivers/gpu/drm/vgem/vgem_drv.h b/drivers/gpu/drm/vgem/vgem_drv.h deleted file mode 100644 index 57ab4d8f41f9..000000000000 --- a/drivers/gpu/drm/vgem/vgem_drv.h +++ /dev/null @@ -1,57 +0,0 @@ -/* - * Copyright © 2012 Intel Corporation - * Copyright © 2014 The Chromium OS Authors - * - * 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 - * the rights to use, copy, modify, merge, publish, distribute, sublicense, - * and/or sell copies of the Software, and to permit persons to whom the - * 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 MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL - * THE AUTHORS OR COPYRIGHT HOLDERS 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: - * Ben Widawsky ben@bwidawsk.net - * - */ - -#ifndef _VGEM_DRV_H_ -#define _VGEM_DRV_H_ - -#include <drm/drmP.h> -#include <drm/drm_gem.h> - -#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; - bool use_dma_buf; -}; - -/* vgem_drv.c */ -extern void vgem_gem_put_pages(struct drm_vgem_gem_object *obj); -extern int vgem_gem_get_pages(struct drm_vgem_gem_object *obj); - -/* vgem_dma_buf.c */ -extern struct sg_table *vgem_gem_prime_get_sg_table( - struct drm_gem_object *gobj); -extern int vgem_gem_prime_pin(struct drm_gem_object *gobj); -extern void vgem_gem_prime_unpin(struct drm_gem_object *gobj); -extern void *vgem_gem_prime_vmap(struct drm_gem_object *gobj); -extern void vgem_gem_prime_vunmap(struct drm_gem_object *obj, void *vaddr); -extern struct drm_gem_object *vgem_gem_prime_import(struct drm_device *dev, - struct dma_buf *dma_buf); - - -#endif