Re: [PATCH V2 1/5] drm/vkms: Add dumb operations

On Thu, Jun 21, 2018 at 09:16:13AM -0300, Rodrigo Siqueira wrote:

VKMS currently does not handle dumb data, and as a consequence, it does not provide mechanisms for handling gem. This commit adds the necessary support for gem object/handler and the dumb functions.

Signed-off-by: Rodrigo Siqueira rodrigosiqueiramelo@gmail.com

This looks good to me except with missing gem_free_object_unlocked callback, which causes warning: Memory manager not clean during takedown.

Maybe it will be easier if we add this in another patch instead of creating v3 of this patchset?

drivers/gpu/drm/vkms/Makefile | 2 +- drivers/gpu/drm/vkms/vkms_drv.c | 9 ++ drivers/gpu/drm/vkms/vkms_drv.h | 21 ++++ drivers/gpu/drm/vkms/vkms_gem.c | 168 ++++++++++++++++++++++++++++++++ 4 files changed, 199 insertions(+), 1 deletion(-) create mode 100644 drivers/gpu/drm/vkms/vkms_gem.c

diff --git a/drivers/gpu/drm/vkms/Makefile b/drivers/gpu/drm/vkms/Makefile index 3f774a6a9c58..986297da51bf 100644 --- a/drivers/gpu/drm/vkms/Makefile +++ b/drivers/gpu/drm/vkms/Makefile @@ -1,3 +1,3 @@ -vkms-y := vkms_drv.o vkms_plane.o vkms_output.o vkms_crtc.o +vkms-y := vkms_drv.o vkms_plane.o vkms_output.o vkms_crtc.o vkms_gem.o

obj-$(CONFIG_DRM_VKMS) += vkms.o diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c index 740a4cbfed91..638bab9083b5 100644 --- a/drivers/gpu/drm/vkms/vkms_drv.c +++ b/drivers/gpu/drm/vkms/vkms_drv.c @@ -37,6 +37,12 @@ static const struct file_operations vkms_driver_fops = { .release = drm_release, };

+static const struct vm_operations_struct vkms_gem_vm_ops = {

• .fault = vkms_gem_fault,
• .open = drm_gem_vm_open,
• .close = drm_gem_vm_close,

+};

static void vkms_release(struct drm_device *dev) { struct vkms_device *vkms = container_of(dev, struct vkms_device, drm); @@ -50,6 +56,9 @@ static struct drm_driver vkms_driver = { .driver_features = DRIVER_MODESET | DRIVER_ATOMIC | DRIVER_GEM, .release = vkms_release, .fops = &vkms_driver_fops,

• .dumb_create = vkms_dumb_create,

• .dumb_map_offset = vkms_dumb_map,

• .gem_vm_ops = &vkms_gem_vm_ops,

  .name = DRIVER_NAME, .desc = DRIVER_DESC,

diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h index b0f9d2e61a42..54bb3dd2b2c1 100644 --- a/drivers/gpu/drm/vkms/vkms_drv.h +++ b/drivers/gpu/drm/vkms/vkms_drv.h @@ -3,6 +3,7 @@

#include <drm/drmP.h> #include <drm/drm.h> +#include <drm/drm_gem.h> #include <drm/drm_encoder.h>

static const u32 vkms_formats[] = { @@ -21,6 +22,12 @@ struct vkms_device { struct vkms_output output; };

+struct vkms_gem_object {

• struct drm_gem_object gem;
• struct mutex pages_lock; /* Page lock used in page fault handler */
• struct page **pages;

+};

int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc, struct drm_plane *primary, struct drm_plane *cursor);

@@ -28,4 +35,18 @@ int vkms_output_init(struct vkms_device *vkmsdev);

struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev);

+/* Gem stuff */ +struct drm_gem_object *vkms_gem_create(struct drm_device *dev,

• 		       struct drm_file *file,
  

• 		       u32 *handle,
  

• 		       u64 size);
  

+int vkms_gem_fault(struct vm_fault *vmf);

+int vkms_dumb_create(struct drm_file *file, struct drm_device *dev,

•      struct drm_mode_create_dumb *args);
  

+int vkms_dumb_map(struct drm_file *file, struct drm_device *dev,

•   u32 handle, u64 *offset);
  

#endif /* _VKMS_DRV_H_ */ diff --git a/drivers/gpu/drm/vkms/vkms_gem.c b/drivers/gpu/drm/vkms/vkms_gem.c new file mode 100644 index 000000000000..9f820f56b9e0 --- /dev/null +++ b/drivers/gpu/drm/vkms/vkms_gem.c @@ -0,0 +1,168 @@ +// SPDX-License-Identifier: GPL-2.0 +/*

• • This program is free software; you can redistribute it and/or modify
• • it under the terms of the GNU General Public License as published by
• • the Free Software Foundation; either version 2 of the License, or
• • (at your option) any later version.
• */

+#include <linux/shmem_fs.h>

+#include "vkms_drv.h"

+static struct vkms_gem_object *__vkms_gem_create(struct drm_device *dev,

• 				 u64 size)
  

+{

• struct vkms_gem_object *obj;
• int ret;
• obj = kzalloc(sizeof(*obj), GFP_KERNEL);
• if (!obj)

• return ERR_PTR(-ENOMEM);
  

• size = roundup(size, PAGE_SIZE);
• ret = drm_gem_object_init(dev, &obj->gem, size);
• if (ret) {

• kfree(obj);
  

• return ERR_PTR(ret);
  

• }
• mutex_init(&obj->pages_lock);
• return obj;

+}

+int vkms_gem_fault(struct vm_fault *vmf) +{

• struct vm_area_struct *vma = vmf->vma;
• struct vkms_gem_object *obj = vma->vm_private_data;
• unsigned long vaddr = vmf->address;
• pgoff_t page_offset;
• loff_t num_pages;
• int ret;
• page_offset = (vaddr - vma->vm_start) >> PAGE_SHIFT;
• num_pages = DIV_ROUND_UP(obj->gem.size, PAGE_SIZE);
• if (page_offset > num_pages)

• return VM_FAULT_SIGBUS;
  

• ret = -ENOENT;
• mutex_lock(&obj->pages_lock);
• if (obj->pages) {

• get_page(obj->pages[page_offset]);
  

• vmf->page = obj->pages[page_offset];
  

• ret = 0;
  

• }
• mutex_unlock(&obj->pages_lock);
• if (ret) {

• struct page *page;
  

• struct address_space *mapping;
  

• mapping = file_inode(obj->gem.filp)->i_mapping;
  

• page = shmem_read_mapping_page(mapping, page_offset);
  

• if (!IS_ERR(page)) {
  

• 	vmf->page = page;
  

• 	ret = 0;
  

• } else {
  

• 	switch (PTR_ERR(page)) {
  

• 	case -ENOSPC:
  

• 	case -ENOMEM:
  

• 		ret = VM_FAULT_OOM;
  

• 		break;
  

• 	case -EBUSY:
  

• 		ret = VM_FAULT_RETRY;
  

• 		break;
  

• 	case -EFAULT:
  

• 	case -EINVAL:
  

• 		ret = VM_FAULT_SIGBUS;
  

• 		break;
  

• 	default:
  

• 		WARN_ON(PTR_ERR(page));
  

• 		ret = VM_FAULT_SIGBUS;
  

• 		break;
  

• 	}
  

• }
  

• }
• return ret;

+}

+struct drm_gem_object *vkms_gem_create(struct drm_device *dev,

• 		       struct drm_file *file,
  

• 		       u32 *handle,
  

• 		       u64 size)
  

+{

• struct vkms_gem_object *obj;
• int ret;
• if (!file || !dev || !handle)

• return ERR_PTR(-EINVAL);
  

• obj = __vkms_gem_create(dev, size);
• if (IS_ERR(obj))

• return ERR_CAST(obj);
  

• ret = drm_gem_handle_create(file, &obj->gem, handle);
• drm_gem_object_put_unlocked(&obj->gem);
• if (ret) {

• drm_gem_object_release(&obj->gem);
  

• kfree(obj);
  

• return ERR_PTR(ret);
  

• }
• return &obj->gem;

+}

+int vkms_dumb_create(struct drm_file *file, struct drm_device *dev,

•      struct drm_mode_create_dumb *args)
  

+{

• struct drm_gem_object *gem_obj;
• u64 pitch, size;
• if (!args || !dev || !file)

• return -EINVAL;
  

• pitch = args->width * DIV_ROUND_UP(args->bpp, 8);
• size = pitch * args->height;
• if (!size)

• return -EINVAL;
  

• gem_obj = vkms_gem_create(dev, file, &args->handle, size);
• if (IS_ERR(gem_obj))

• return PTR_ERR(gem_obj);
  

• args->size = gem_obj->size;
• args->pitch = pitch;
• DRM_DEBUG_DRIVER("Created object of size %lld\n", size);
• return 0;

+}

+int vkms_dumb_map(struct drm_file *file, struct drm_device *dev,

•   u32 handle, u64 *offset)
  

+{

• struct drm_gem_object *obj;
• int ret;
• obj = drm_gem_object_lookup(file, handle);
• if (!obj)

• return -ENOENT;
  

• if (!obj->filp) {

• ret = -EINVAL;
  

• goto unref;
  

• }
• ret = drm_gem_create_mmap_offset(obj);
• if (ret)

• goto unref;
  

• *offset = drm_vma_node_offset_addr(&obj->vma_node);

+unref:

• drm_gem_object_put_unlocked(obj);
• return ret;

+}

2.17.1