[PATCH v2 6/8] drm: Add vmalloc BO helper

Daniel Vetter daniel at ffwll.ch
Thu Oct 19 06:48:31 UTC 2017


On Wed, Oct 18, 2017 at 09:44:43PM +0200, Noralf Trønnes wrote:
> Den 17.10.2017 14.51, skrev Daniel Vetter:
> > On Sun, Oct 15, 2017 at 06:30:40PM +0200, Noralf Trønnes wrote:
> > > Add vmalloc buffer object helper that can be useful for modesetting
> > > drivers, particularly the framebuffer flushing kind.
> > > 
> > > Signed-off-by: Noralf Trønnes <noralf at tronnes.org>
> > Why can't we extend the shmem stuff to provide a simple vmalloc helper?
> > Doing a new flavour of gem for every special use-case seems like not a
> > good idea ...
> > 
> > This could then also be used by stuff like udl, with fewer changes.
> 
> This is the rationale (from the coverletter):
> 
> The reason I want to move away from the cma helper, is that it restricts
> which drivers to PRIME import from, since cma requires the buffer to be
> physically continuous. Initially I looked at udl and decided to use
> shmem, but have later decided against it since shmem doesn't work with
> fbdev deferred I/O, they both use page->lru and page->mapping. This
> meant adding a shadow buffer for fbdev with increased complexity, so I
> fell back to the KISS principle.

Hm, I missed that. Can you pls add the reason for why we can't just use
shmem to the overview kerneldoc comment?

> I'm trying to make tinydrm support drivers like simpledrm and udl, so if
> using vmalloc blocks those use cases, please let me know.

It shouldn't, I just wanted to know why we need something really similar
to shmem (since that's also an sg list that can be vmapped). One remote
concern I have is that drivers which in general want to use shmem, but
also need fbdev defio support might need to do some acrobatics, and use
the vmalloc gem type only for fbdev. But I think that should work out too.

I'm travelling next week and will probably not get around to do a full
review of this patch here, would be good if you can find someone else
meanwhile.

Thanks, Daniel

> 
> 
> Noralf.
> 
> > -Daniel
> > 
> > > ---
> > >   Documentation/gpu/drm-kms-helpers.rst   |  12 ++
> > >   drivers/gpu/drm/Kconfig                 |   7 +
> > >   drivers/gpu/drm/Makefile                |   1 +
> > >   drivers/gpu/drm/drm_vmalloc_bo_helper.c | 305 ++++++++++++++++++++++++++++++++
> > >   include/drm/drm_vmalloc_bo_helper.h     |  88 +++++++++
> > >   5 files changed, 413 insertions(+)
> > >   create mode 100644 drivers/gpu/drm/drm_vmalloc_bo_helper.c
> > >   create mode 100644 include/drm/drm_vmalloc_bo_helper.h
> > > 
> > > diff --git a/Documentation/gpu/drm-kms-helpers.rst b/Documentation/gpu/drm-kms-helpers.rst
> > > index 13dd237418cc..fd1ca10f6611 100644
> > > --- a/Documentation/gpu/drm-kms-helpers.rst
> > > +++ b/Documentation/gpu/drm-kms-helpers.rst
> > > @@ -305,3 +305,15 @@ Framebuffer GEM Helper Reference
> > >   .. kernel-doc:: drivers/gpu/drm/drm_gem_framebuffer_helper.c
> > >      :export:
> > > +
> > > +vmalloc buffer object helper
> > > +============================
> > > +
> > > +.. kernel-doc:: drivers/gpu/drm/drm_vmalloc_bo_helper.c
> > > +   :doc: overview
> > > +
> > > +.. kernel-doc:: include/drm/drm_vmalloc_bo_helper.h
> > > +   :internal:
> > > +
> > > +.. kernel-doc:: drivers/gpu/drm/drm_vmalloc_bo_helper.c
> > > +   :export:
> > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> > > index 4d9f21831741..5d580440a259 100644
> > > --- a/drivers/gpu/drm/Kconfig
> > > +++ b/drivers/gpu/drm/Kconfig
> > > @@ -145,6 +145,13 @@ config DRM_KMS_CMA_HELPER
> > >   	help
> > >   	  Choose this if you need the KMS CMA helper functions
> > > +config DRM_VMALLOC_BO_HELPER
> > > +	bool
> > > +	depends on DRM
> > > +	select DRM_KMS_HELPER
> > > +	help
> > > +	  Choose this if you need the vmalloc buffer object helper functions
> > > +
> > >   config DRM_VM
> > >   	bool
> > >   	depends on DRM && MMU
> > > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> > > index a3fdc5a68dff..ed3eafa97a69 100644
> > > --- a/drivers/gpu/drm/Makefile
> > > +++ b/drivers/gpu/drm/Makefile
> > > @@ -39,6 +39,7 @@ drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o drm_probe_helper.o \
> > >   drm_kms_helper-$(CONFIG_DRM_PANEL_BRIDGE) += bridge/panel.o
> > >   drm_kms_helper-$(CONFIG_DRM_FBDEV_EMULATION) += drm_fb_helper.o
> > >   drm_kms_helper-$(CONFIG_DRM_KMS_CMA_HELPER) += drm_fb_cma_helper.o
> > > +drm_kms_helper-$(CONFIG_DRM_VMALLOC_BO_HELPER) += drm_vmalloc_bo_helper.o
> > >   drm_kms_helper-$(CONFIG_DRM_DP_AUX_CHARDEV) += drm_dp_aux_dev.o
> > >   obj-$(CONFIG_DRM_KMS_HELPER) += drm_kms_helper.o
> > > diff --git a/drivers/gpu/drm/drm_vmalloc_bo_helper.c b/drivers/gpu/drm/drm_vmalloc_bo_helper.c
> > > new file mode 100644
> > > index 000000000000..4015b9d1d671
> > > --- /dev/null
> > > +++ b/drivers/gpu/drm/drm_vmalloc_bo_helper.c
> > > @@ -0,0 +1,305 @@
> > > +/*
> > > + * DRM vmalloc buffer object helper functions
> > > + *
> > > + * Copyright (C) 2017 Noralf Trønnes
> > > + *
> > > + * 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/dma-buf.h>
> > > +#include <linux/export.h>
> > > +#include <linux/slab.h>
> > > +#include <linux/vmalloc.h>
> > > +
> > > +#include <drm/drmP.h>
> > > +#include <drm/drm_fb_helper.h>
> > > +#include <drm/drm_gem_framebuffer_helper.h>
> > > +#include <drm/drm_vmalloc_bo_helper.h>
> > > +
> > > +/**
> > > + * DOC: overview
> > > + *
> > > + * This helper provides a simple GEM based buffer object with buffers allocated
> > > + * using vmalloc(). This is useful for modesetting drivers that do framebuffer
> > > + * flushing. It supports dumb buffers and PRIME import which can be setup using
> > > + * the DEFINE_DRM_VMALLOC_BO_FOPS() and DRM_VMALLOC_BO_DRIVER_OPS() macros.
> > > + *
> > > + * fbdev emulation can be setup using the drm_vmalloc_bo_fbdev_probe() function.
> > > + */
> > > +
> > > +static struct drm_vmalloc_bo *
> > > +drm_vmalloc_bo_create(struct drm_device *dev, size_t size, bool backing)
> > > +{
> > > +	struct drm_vmalloc_bo *bo;
> > > +	int ret;
> > > +
> > > +	size = PAGE_ALIGN(size);
> > > +
> > > +	bo = kzalloc(sizeof(*bo), GFP_KERNEL);
> > > +	if (!bo)
> > > +		return ERR_PTR(-ENOMEM);
> > > +
> > > +	if (backing) {
> > > +		bo->vaddr = vmalloc_user(size);
> > > +		if (!bo->vaddr) {
> > > +			ret = -ENOMEM;
> > > +			goto error_free;
> > > +		}
> > > +	}
> > > +
> > > +	drm_gem_private_object_init(dev, &bo->base, size);
> > > +
> > > +	return bo;
> > > +
> > > +error_free:
> > > +	kfree(bo);
> > > +
> > > +	return ERR_PTR(ret);
> > > +}
> > > +
> > > +/**
> > > + * drm_vmalloc_bo_free_object() - Free resources associated with a vmalloc BO
> > > + *                                object
> > > + * @obj: GEM object to free
> > > + *
> > > + * This function frees the backing memory of the vmalloc BO object, cleans up
> > > + * the GEM object state and frees the memory used to store the object itself.
> > > + * Drivers using the vmalloc BO helpers should set this as their
> > > + * &drm_driver.gem_free_object callback.
> > > + */
> > > +void drm_vmalloc_bo_free_object(struct drm_gem_object *obj)
> > > +{
> > > +	struct drm_vmalloc_bo *bo = to_drm_vmalloc_bo(obj);
> > > +
> > > +	if (obj->import_attach)
> > > +		dma_buf_vunmap(obj->import_attach->dmabuf, bo->vaddr);
> > > +	else
> > > +		vfree(bo->vaddr);
> > > +
> > > +	drm_gem_object_release(obj);
> > > +	kfree(bo);
> > > +}
> > > +EXPORT_SYMBOL(drm_vmalloc_bo_free_object);
> > > +
> > > +/**
> > > + * drm_vmalloc_bo_dumb_create() - Create a dumb vmalloc BO
> > > + * @file: DRM file structure to create the dumb buffer for
> > > + * @dev: DRM device
> > > + * @args: IOCTL data
> > > + *
> > > + * This function computes the pitch of the dumb buffer and rounds it up to an
> > > + * integer number of bytes per pixel. Drivers for hardware that doesn't have
> > > + * any additional restrictions on the pitch can directly use this function as
> > > + * their &drm_driver.dumb_create callback.
> > > + *
> > > + * For hardware with additional restrictions, drivers can adjust the fields
> > > + * set up by userspace before calling into this function. Also
> > > + * &drm_mode_create_dumb.pitch and &drm_mode_create_dumb.size can be set.
> > > + *
> > > + * Returns:
> > > + * Zero on success or a negative error code on failure.
> > > + */
> > > +int drm_vmalloc_bo_dumb_create(struct drm_file *file, struct drm_device *dev,
> > > +			       struct drm_mode_create_dumb *args)
> > > +{
> > > +	struct drm_vmalloc_bo *bo;
> > > +	int ret;
> > > +
> > > +	if (!args->pitch)
> > > +		args->pitch = DIV_ROUND_UP(args->width * args->bpp, 8);
> > > +	else
> > > +		args->pitch = max(args->pitch,
> > > +				  DIV_ROUND_UP(args->width * args->bpp, 8));
> > > +	if (!args->size)
> > > +		args->size = args->pitch * args->height;
> > > +	else
> > > +		args->size = max_t(typeof(args->size), args->size,
> > > +				   args->pitch * args->height);
> > > +
> > > +	bo = drm_vmalloc_bo_create(dev, args->size, true);
> > > +	if (IS_ERR(bo))
> > > +		return PTR_ERR(bo);
> > > +
> > > +	ret = drm_gem_handle_create(file, &bo->base, &args->handle);
> > > +	/* drop reference from allocate - handle holds it now. */
> > > +	drm_gem_object_put_unlocked(&bo->base);
> > > +
> > > +	return ret;
> > > +}
> > > +EXPORT_SYMBOL(drm_vmalloc_bo_dumb_create);
> > > +
> > > +const struct vm_operations_struct drm_vmalloc_bo_vm_ops = {
> > > +	.open = drm_gem_vm_open,
> > > +	.close = drm_gem_vm_close,
> > > +};
> > > +EXPORT_SYMBOL_GPL(drm_vmalloc_bo_vm_ops);
> > > +
> > > +/**
> > > + * drm_vmalloc_bo_mmap() - Memory-map a vmalloc BO
> > > + * @filp: File object
> > > + * @vma: VMA for the area to be mapped
> > > + *
> > > + * This function implements an augmented version of the GEM DRM file mmap
> > > + * operation for vmalloc buffer objects. Drivers should use this function as
> > > + * their ->mmap handler in the DRM device file's file_operations structure.
> > > + *
> > > + * Instead of directly referencing this function, drivers should use the
> > > + * DEFINE_DRM_VMALLOC_BO_FOPS() macro.
> > > + *
> > > + * Returns:
> > > + * Zero on success or a negative error code on failure.
> > > + */
> > > +int drm_vmalloc_bo_mmap(struct file *filp, struct vm_area_struct *vma)
> > > +{
> > > +	struct drm_vmalloc_bo *bo;
> > > +	int ret;
> > > +
> > > +	ret = drm_gem_mmap(filp, vma);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	/* Set by drm_gem_mmap() */
> > > +	vma->vm_flags &= ~VM_IO;
> > > +	vma->vm_flags &= ~VM_PFNMAP;
> > > +
> > > +	bo = to_drm_vmalloc_bo(vma->vm_private_data);
> > > +
> > > +	return remap_vmalloc_range(vma, bo->vaddr, 0);
> > > +}
> > > +EXPORT_SYMBOL(drm_vmalloc_bo_mmap);
> > > +
> > > +/**
> > > + * drm_vmalloc_bo_prime_import_sg_table() - Produce a vmalloc BO from a dma-buf
> > > + * @dev: DRM device to import into
> > > + * @attach: dmabuf attachment
> > > + * @sgt: Scatter/gather table of pinned pages
> > > + *
> > > + * This function creates a vmalloc buffer object using the virtual address on
> > > + * the dma-buf exported by another driver. Drivers using the vmalloc BO helpers
> > > + * should set this as their &drm_driver.gem_prime_import_sg_table callback.
> > > + *
> > > + * Returns:
> > > + * A pointer to a newly created GEM object or an ERR_PTR-encoded negative
> > > + * error code on failure.
> > > + */
> > > +struct drm_gem_object *
> > > +drm_vmalloc_bo_prime_import_sg_table(struct drm_device *dev,
> > > +				     struct dma_buf_attachment *attach,
> > > +				     struct sg_table *sgt)
> > > +{
> > > +	struct drm_vmalloc_bo *bo;
> > > +	void *vaddr;
> > > +
> > > +	vaddr = dma_buf_vmap(attach->dmabuf);
> > > +	if (!vaddr) {
> > > +		DRM_ERROR("Failed to vmap PRIME buffer\n");
> > > +		return ERR_PTR(-ENOMEM);
> > > +	}
> > > +
> > > +	bo = drm_vmalloc_bo_create(dev, attach->dmabuf->size, false);
> > > +	if (IS_ERR(bo)) {
> > > +		dma_buf_vunmap(attach->dmabuf, vaddr);
> > > +		return ERR_CAST(bo);
> > > +	}
> > > +
> > > +	bo->vaddr = vaddr;
> > > +
> > > +	DRM_DEBUG_PRIME("size = %zu\n", attach->dmabuf->size);
> > > +
> > > +	return &bo->base;
> > > +}
> > > +EXPORT_SYMBOL(drm_vmalloc_bo_prime_import_sg_table);
> > > +
> > > +static struct fb_ops drm_vmalloc_bo_fbdev_ops = {
> > > +	.owner		= THIS_MODULE,
> > > +	DRM_FB_HELPER_DEFAULT_OPS,
> > > +	.fb_read	= drm_fb_helper_sys_read,
> > > +	.fb_write	= drm_fb_helper_sys_write,
> > > +	.fb_fillrect	= drm_fb_helper_sys_fillrect,
> > > +	.fb_copyarea	= drm_fb_helper_sys_copyarea,
> > > +	.fb_imageblit	= drm_fb_helper_sys_imageblit,
> > > +};
> > > +
> > > +/**
> > > + * drm_vmalloc_bo_fbdev_probe - fbdev emulation \.fb_probe helper
> > > + * @fb_helper: fbdev emulation helper structure
> > > + * @sizes: Describes fbdev size and scanout surface size
> > > + * @fb_funcs: DRM framebuffer functions
> > > + *
> > > + * This function can be used in the &drm_fb_helper_funcs.fb_probe callback to
> > > + * setup fbdev emulation. If @fb_funcs->dirty is set, fbdev deferred I/O is
> > > + * initialized. drm_fb_helper_simple_init() can be used to initialize the fbdev
> > > + * emulation.
> > > + *
> > > + * Returns:
> > > + * Zero on success, negative error code on failure.
> > > + */
> > > +int drm_vmalloc_bo_fbdev_probe(struct drm_fb_helper *fb_helper,
> > > +			       struct drm_fb_helper_surface_size *sizes,
> > > +			       const struct drm_framebuffer_funcs *fb_funcs)
> > > +{
> > > +	struct drm_device *dev = fb_helper->dev;
> > > +	struct drm_framebuffer *fb;
> > > +	struct drm_vmalloc_bo *bo;
> > > +	struct fb_info *fbi;
> > > +	size_t size;
> > > +	int ret;
> > > +
> > > +	DRM_DEBUG_KMS("surface width(%d), height(%d) and bpp(%d)\n",
> > > +		      sizes->surface_width, sizes->surface_height,
> > > +		      sizes->surface_bpp);
> > > +
> > > +	size = sizes->surface_width * sizes->surface_height *
> > > +	       DIV_ROUND_UP(sizes->surface_bpp, 8);
> > > +
> > > +	bo = drm_vmalloc_bo_create(dev, size, true);
> > > +	if (IS_ERR(bo))
> > > +		return -ENOMEM;
> > > +
> > > +	fb = drm_gem_fbdev_fb_create(dev, sizes, 0, &bo->base, fb_funcs);
> > > +	if (IS_ERR(fb)) {
> > > +		dev_err(dev->dev, "Failed to allocate DRM framebuffer.\n");
> > > +		ret = PTR_ERR(fb);
> > > +		goto err_bo_free;
> > > +	}
> > > +
> > > +	fb_helper->fb = fb;
> > > +
> > > +	fbi = drm_fb_helper_alloc_fbi(fb_helper);
> > > +	if (IS_ERR(fbi)) {
> > > +		ret = PTR_ERR(fbi);
> > > +		goto err_fb_free;
> > > +	}
> > > +
> > > +	fbi->par = fb_helper;
> > > +	fbi->flags = FBINFO_VIRTFB;
> > > +	fbi->fbops = &drm_vmalloc_bo_fbdev_ops;
> > > +
> > > +	drm_fb_helper_fill_fix(fbi, fb->pitches[0], fb->format->depth);
> > > +	drm_fb_helper_fill_var(fbi, fb_helper, sizes->fb_width, sizes->fb_height);
> > > +
> > > +	fbi->screen_base = bo->vaddr;
> > > +	fbi->screen_size = size;
> > > +	fbi->fix.smem_len = size;
> > > +
> > > +	if (fb_funcs->dirty) {
> > > +		ret = drm_fb_helper_defio_init(fb_helper);
> > > +		if (ret)
> > > +			goto err_fb_info_destroy;
> > > +	}
> > > +
> > > +	return 0;
> > > +
> > > +err_fb_info_destroy:
> > > +	drm_fb_helper_fini(fb_helper);
> > > +err_fb_free:
> > > +	drm_framebuffer_remove(fb);
> > > +err_bo_free:
> > > +	drm_gem_object_put_unlocked(&bo->base);
> > > +
> > > +	return ret;
> > > +}
> > > +EXPORT_SYMBOL(drm_vmalloc_bo_fbdev_probe);
> > > diff --git a/include/drm/drm_vmalloc_bo_helper.h b/include/drm/drm_vmalloc_bo_helper.h
> > > new file mode 100644
> > > index 000000000000..0df3d15e2e4a
> > > --- /dev/null
> > > +++ b/include/drm/drm_vmalloc_bo_helper.h
> > > @@ -0,0 +1,88 @@
> > > +#ifndef __DRM_VMALLOC_BO_HELPER_H__
> > > +#define __DRM_VMALLOC_BO_HELPER_H__
> > > +
> > > +#include <drm/drmP.h>
> > > +#include <drm/drm_gem.h>
> > > +
> > > +struct drm_fb_helper_surface_size;
> > > +
> > > +/**
> > > + * struct drm_vmalloc_bo - vmalloc buffer object
> > > + */
> > > +struct drm_vmalloc_bo {
> > > +	/**
> > > +	 * @base:
> > > +	 *
> > > +	 * Base GEM object.
> > > +	 */
> > > +	struct drm_gem_object base;
> > > +
> > > +	/**
> > > +	 * @vaddr:
> > > +	 *
> > > +	 * Kernel virtual address of the buffer.
> > > +	 */
> > > +	void *vaddr;
> > > +};
> > > +
> > > +static inline struct drm_vmalloc_bo *
> > > +to_drm_vmalloc_bo(struct drm_gem_object *obj)
> > > +{
> > > +	return container_of(obj, struct drm_vmalloc_bo, base);
> > > +}
> > > +
> > > +static inline void *drm_vmalloc_bo_fb_vaddr(struct drm_framebuffer *fb)
> > > +{
> > > +	return to_drm_vmalloc_bo(fb->obj[0])->vaddr;
> > > +}
> > > +
> > > +void drm_vmalloc_bo_free_object(struct drm_gem_object *obj);
> > > +int drm_vmalloc_bo_dumb_create(struct drm_file *file, struct drm_device *dev,
> > > +			       struct drm_mode_create_dumb *args);
> > > +int drm_vmalloc_bo_mmap(struct file *filp, struct vm_area_struct *vma);
> > > +struct drm_gem_object *
> > > +drm_vmalloc_bo_prime_import_sg_table(struct drm_device *dev,
> > > +				     struct dma_buf_attachment *attach,
> > > +				     struct sg_table *sgt);
> > > +
> > > +int drm_vmalloc_bo_fbdev_probe(struct drm_fb_helper *fb_helper,
> > > +			       struct drm_fb_helper_surface_size *sizes,
> > > +			       const struct drm_framebuffer_funcs *fb_funcs);
> > > +
> > > +/**
> > > + * DEFINE_DRM_VMALLOC_BO_FOPS() - Macro to generate file operations for drivers
> > > + * @name: Name for the generated structure
> > > + *
> > > + * This macro autogenerates a suitable &struct file_operations which can be
> > > + * assigned to &drm_driver.fops for vmalloc BO drivers.
> > > + */
> > > +#define DEFINE_DRM_VMALLOC_BO_FOPS(name) \
> > > +	static const struct file_operations name = {\
> > > +		.owner		= THIS_MODULE,\
> > > +		.open		= drm_open,\
> > > +		.release	= drm_release,\
> > > +		.unlocked_ioctl	= drm_ioctl,\
> > > +		.compat_ioctl	= drm_compat_ioctl,\
> > > +		.poll		= drm_poll,\
> > > +		.read		= drm_read,\
> > > +		.llseek		= noop_llseek,\
> > > +		.mmap		= drm_vmalloc_bo_mmap,\
> > > +	}
> > > +
> > > +extern const struct vm_operations_struct drm_vmalloc_bo_vm_ops;
> > > +
> > > +/**
> > > + * DRM_VMALLOC_BO_DRIVER_OPS - Default GEM and PRIME operations
> > > + *
> > > + * This macro provides a shortcut for setting the GEM and PRIME operations in
> > > + * the &drm_driver structure for vmalloc BO drivers.
> > > + */
> > > +#define DRM_VMALLOC_BO_DRIVER_OPS \
> > > +	.gem_free_object	= drm_vmalloc_bo_free_object, \
> > > +	.gem_vm_ops		= &drm_vmalloc_bo_vm_ops, \
> > > +	.prime_fd_to_handle	= drm_gem_prime_fd_to_handle, \
> > > +	.gem_prime_import	= drm_gem_prime_import, \
> > > +	.gem_prime_import_sg_table = drm_vmalloc_bo_prime_import_sg_table, \
> > > +	.dumb_create		= drm_vmalloc_bo_dumb_create
> > > +
> > > +#endif /* __DRM_VMALLOC_BO_HELPER_H__ */
> > > -- 
> > > 2.14.2
> > > 
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch


More information about the dri-devel mailing list