[Linaro-mm-sig] [RFC v2] dma-buf: Add buffer sharing framework
Daniel Vetter
daniel at ffwll.ch
Fri Sep 23 17:41:26 UTC 2011
On Tue, Sep 13, 2011 at 07:59:09PM +0530, Sumit Semwal wrote:
> This is the first step in defining a buffer sharing framework.
> A new dma_buf buffer object is added, with hooks to allow for easy sharing of
> this buffer object across devices.
>
> The framework allows:
> - a new buffer-object to be created with fixed size.
> - different devices to 'attach' themselves to this buffer, to facilitate
> backing storage negotiation, using dma_buf_attach() API.
> - association of a file pointer with each user-buffer and associated
> allocator-defined operations on that buffer. This operation is called the
> 'export' operation.
> - this exported buffer-object to be shared with the other entity by asking for
> its 'file-descriptor (fd)', and sharing the fd across.
> - a received fd to get the buffer object back, where it can be accessed using
> the associated exporter-defined operations.
> - the exporter and importer to share the scatterlist using get_scatterlist and
> put_scatterlist operations.
>
> Atleast one 'attach()' call is required to be made prior to calling the
> buffer_map() callback.
>
> Couple of building blocks in get_scatterlist() are added to ease introduction
> of sync'ing across exporter and importers, and late allocation by the exporter.
>
> Optionally, mmap() file operation is provided for the associated 'fd', as
> wrapper over the allocator defined mmap()[optional], to be used by devices that
> might need one.
>
> The idea was first mooted at the Linaro memory management mini-summit in
> Budapest in May 2011, as part of multiple things needed for a 'unified memory
> management framework'. It took a more concrete shape at Linaro memory-management
> mini-summit in Cambridge, Aug 2011.
>
> This is based on design suggestions from many people at both the mini-summits,
> most notably from Arnd Bergmann <arnd at arndb.de>, Rob Clark <rob at ti.com> and
> Daniel Vetter <daniel at ffwll.ch>.
>
> The implementation is inspired from proof-of-concept patch-set from
> Tomasz Stanislawski <t.stanislaws at samsung.com>, who demonstrated buffer sharing
> between two v4l2 devices.
>
> ------
> v1: initial RFC.
> v2:
> - added attach() / detach() dma_buf_ops, and dma_buf_attach(),dma_buf_detach().
> - added handling of list of attachment in the dma_buf central API itself.
> - corrected copyright information.
>
> Signed-off-by: Sumit Semwal <sumit.semwal at linaro.org>
> Signed-off-by: Sumit Semwal <sumit.semwal at ti.com>
Hi Summit,
Sorry for the late review, I've been a bit busy. Some nitpicks below, but
most of it has already been brought up by Rob and Subash.
> ---
> drivers/base/Kconfig | 10 ++
> drivers/base/Makefile | 1 +
> drivers/base/dma-buf.c | 224 +++++++++++++++++++++++++++++++++++++++++++++++
> include/linux/dma-buf.h | 179 +++++++++++++++++++++++++++++++++++++
> 4 files changed, 414 insertions(+), 0 deletions(-)
> create mode 100644 drivers/base/dma-buf.c
> create mode 100644 include/linux/dma-buf.h
>
> diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
> index 21cf46f..07d8095 100644
> --- a/drivers/base/Kconfig
> +++ b/drivers/base/Kconfig
> @@ -174,4 +174,14 @@ config SYS_HYPERVISOR
>
> source "drivers/base/regmap/Kconfig"
>
> +config DMA_SHARED_BUFFER
> + bool "Buffer framework to be shared between drivers"
> + default n
> + depends on ANON_INODES
> + help
> + This option enables the framework for buffer-sharing between
> + multiple drivers. A buffer is associated with a file using driver
> + APIs extension; the file's descriptor can then be passed on to other
> + driver.
> +
> endmenu
> diff --git a/drivers/base/Makefile b/drivers/base/Makefile
> index 99a375a..d0df046 100644
> --- a/drivers/base/Makefile
> +++ b/drivers/base/Makefile
> @@ -8,6 +8,7 @@ obj-$(CONFIG_DEVTMPFS) += devtmpfs.o
> obj-y += power/
> obj-$(CONFIG_HAS_DMA) += dma-mapping.o
> obj-$(CONFIG_HAVE_GENERIC_DMA_COHERENT) += dma-coherent.o
> +obj-$(CONFIG_DMA_SHARED_BUFFER) += dma-buf.o
> obj-$(CONFIG_ISA) += isa.o
> obj-$(CONFIG_FW_LOADER) += firmware_class.o
> obj-$(CONFIG_NUMA) += node.o
> diff --git a/drivers/base/dma-buf.c b/drivers/base/dma-buf.c
> new file mode 100644
> index 0000000..8936da7
> --- /dev/null
> +++ b/drivers/base/dma-buf.c
> @@ -0,0 +1,224 @@
> +/*
> + * Framework for buffer objects that can be shared across devices/subsystems.
> + *
> + * Copyright(C) 2011 Linaro Limited. All rights reserved.
> + * Author: Sumit Semwal <sumit.semwal at ti.com>
> + *
> + * Many thanks to linaro-mm-sig list, and specially
> + * Arnd Bergmann <arnd at arndb.de>, Rob Clark <rob at ti.com> and
> + * Daniel Vetter <daniel at ffwll.ch> for their support in creation and
> + * refining of this idea.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published by
> + * the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <linux/fs.h>
> +#include <linux/slab.h>
> +#include <linux/dma-buf.h>
> +#include <linux/anon_inodes.h>
> +
> +static inline int is_dma_buf_file(struct file *);
> +
> +static int dma_buf_mmap(struct file *file, struct vm_area_struct *vma)
> +{
> + struct dma_buf *dmabuf;
> +
> + if (!is_dma_buf_file(file))
> + return -EINVAL;
> +
> + dmabuf = file->private_data;
> +
> + if (!dmabuf->ops->mmap)
> + return -EINVAL;
> +
> + return dmabuf->ops->mmap(dmabuf, vma);
> +}
> +
> +static int dma_buf_release(struct inode *inode, struct file *file)
> +{
> + struct dma_buf *dmabuf;
> +
> + if (!is_dma_buf_file(file))
> + return -EINVAL;
> +
> + dmabuf = file->private_data;
> +
> + dmabuf->ops->release(dmabuf);
> + kfree(dmabuf);
> + return 0;
> +}
> +
> +static const struct file_operations dma_buf_fops = {
> + .mmap = dma_buf_mmap,
> + .release = dma_buf_release,
> +};
> +
> +/*
> + * is_dma_buf_file - Check if struct file* is associated with dma_buf
> + */
> +static inline int is_dma_buf_file(struct file *file)
> +{
> + return file->f_op == &dma_buf_fops;
> +}
> +
> +/**
> + * dma_buf_export - Creates a new dma_buf, and associates an anon file
> + * with this buffer,so it can be exported.
> + * Also connect the allocator specific data and ops to the buffer.
> + *
> + * @priv: [in] Attach private data of allocator to this buffer
> + * @ops: [in] Attach allocator-defined dma buf ops to the new buffer.
> + * @flags: [in] mode flags for the file.
> + *
> + * Returns, on success, a newly created dma_buf object, which wraps the
> + * supplied private data and operations for dma_buf_ops. On failure to
> + * allocate the dma_buf object, it can return NULL.
> + *
> + */
> +struct dma_buf *dma_buf_export(void *priv, struct dma_buf_ops *ops,
> + int flags)
> +{
> + struct dma_buf *dmabuf;
> + struct file *file;
> +
> + BUG_ON(!priv || !ops);
> +
> + dmabuf = kzalloc(sizeof(struct dma_buf), GFP_KERNEL);
> + if (dmabuf == NULL)
> + return dmabuf;
> +
> + dmabuf->priv = priv;
> + dmabuf->ops = ops;
> +
> + file = anon_inode_getfile("dmabuf", &dma_buf_fops, dmabuf, flags);
> +
> + dmabuf->file = file;
> +
> + INIT_LIST_HEAD(&dmabuf->attachments);
> +
> + return dmabuf;
> +}
> +EXPORT_SYMBOL(dma_buf_export);
> +
> +
> +/**
> + * dma_buf_fd - returns a file descriptor for the given dma_buf
> + * @dmabuf: [in] pointer to dma_buf for which fd is required.
> + *
> + * On success, returns an associated 'fd'. Else, returns error.
> + */
> +int dma_buf_fd(struct dma_buf *dmabuf)
> +{
> + int error, fd;
> +
> + if (!dmabuf->file)
> + return -EINVAL;
> +
> + error = get_unused_fd_flags(0);
> + if (error < 0)
> + return error;
> + fd = error;
> +
> + fd_install(fd, dmabuf->file);
> +
> + return fd;
> +}
> +EXPORT_SYMBOL(dma_buf_fd);
> +
> +/**
> + * dma_buf_get - returns the dma_buf structure related to an fd
> + * @fd: [in] fd associated with the dma_buf to be returned
> + *
> + * On success, returns the dma_buf structure associated with an fd; uses
> + * file's refcounting done by fget to increase refcount. returns ERR_PTR
> + * otherwise.
> + */
> +struct dma_buf *dma_buf_get(int fd)
> +{
> + struct file *file;
> +
> + file = fget(fd);
> +
> + if (!file)
> + return ERR_PTR(-EBADF);
> +
> + if (!is_dma_buf_file(file)) {
> + fput(file);
> + return ERR_PTR(-EINVAL);
> + }
> +
> + return file->private_data;
> +}
> +EXPORT_SYMBOL(dma_buf_get);
> +
> +/**
> + * dma_buf_put - decreases refcount of the buffer
> + * @dmabuf: [in] buffer to reduce refcount of
> + *
> + * Uses file's refcounting done implicitly by fput()
> + */
> +void dma_buf_put(struct dma_buf *dmabuf)
> +{
> + BUG_ON(!dmabuf->file);
> +
> + fput(dmabuf->file);
> +
> + return;
> +}
> +EXPORT_SYMBOL(dma_buf_put);
> +
> +/**
> + * dma_buf_attach - Add the device to dma_buf's attachments list; calls the
> + * attach() of dma_buf_ops to allow device-specific attach functionality
> + * @dmabuf: [in] buffer to attach device to.
> + * @dev: [in] device to be attached.
> + *
> + * Returns struct dma_buf_attachment * for this attachment; may return NULL.
> + *
> + */
> +struct dma_buf_attachment *dma_buf_attach(struct dma_buf *dmabuf,
> + struct device *dev)
> +{
> + struct dma_buf_attachment *attach;
> +
> + BUG_ON(!dmabuf || !dev);
> +
> + attach = kzalloc(sizeof(struct dma_buf_attachment), GFP_KERNEL);
> + if (attach == NULL)
> + return attach;
> +
> + dmabuf->ops->attach(dmabuf, dev, attach);
> + list_add(&attach->node, &dmabuf->attachments);
> +
> + return attach;
> +}
> +EXPORT_SYMBOL(dma_buf_attach);
I agree with Subash that we need a look here to protect the list
manipulation. I think for simplicity just add a mutex and wrap it around
both the list manipulation and the call to attach/detach.
Also, I think the common code shoudl set dma_buf_attachment->dev and
ops->attach/detach should be optional (for simpler cases) and attach needs
to be able to return an error (which should then be converted into an
ERR_PTR by this function) in case the allocation is already fixed and the
device can't access it at its current position.
> +/**
> + * dma_buf_detach - Remove the given attachment from dmabuf's attachments list;
> + * calls detach() of dma_buf_ops for device-specific detach
> + * @dmabuf: [in] buffer to detach from.
> + * @attach: [in] attachment to be detached; is free'd after this call.
> + *
> + */
> +void dma_buf_detach(struct dma_buf *dmabuf, struct dma_buf_attachment *attach)
> +{
> + BUG_ON(!dmabuf || !attach);
> +
> + list_del(&attach->node);
> + dmabuf->ops->detach(dmabuf, attach);
> +
> + kfree(attach);
> + return;
> +}
> +EXPORT_SYMBOL(dma_buf_detach);
> diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
> new file mode 100644
> index 0000000..2894b45
> --- /dev/null
> +++ b/include/linux/dma-buf.h
> @@ -0,0 +1,179 @@
> +/*
> + * Header file for dma buffer sharing framework.
> + *
> + * Copyright(C) 2011 Linaro Limited. All rights reserved.
> + * Author: Sumit Semwal <sumit.semwal at ti.com>
> + *
> + * Many thanks to linaro-mm-sig list, and specially
> + * Arnd Bergmann <arnd at arndb.de>, Rob Clark <rob at ti.com> and
> + * Daniel Vetter <daniel at ffwll.ch> for their support in creation and
> + * refining of this idea.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published by
> + * the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program. If not, see <http://www.gnu.org/licenses/>.
> + */
> +#ifndef __DMA_BUF_H__
> +#define __DMA_BUF_H__
> +
> +#include <linux/file.h>
> +#include <linux/err.h>
> +#include <linux/device.h>
> +#include <linux/scatterlist.h>
> +#include <linux/list.h>
> +
> +struct dma_buf;
> +
> +/**
> + * enum dma_buf_optype - Used to denote the operation for which
> + * get_scatterlist() is called. This will help in implementing a wait(op)
> + * for sync'ing.
> + * @DMA_BUF_OP_READ: read operation will be done on this scatterlist
> + * @DMA_BUF_OP_WRITE: write operation will be done on this scatterlist
> + */
> +enum dma_buf_optype {
> + DMA_BUF_OP_READ = (1 << 0),
> + DMA_BUF_OP_WRITE = (1 << 1),
> + DMA_BUF_OP_MAX,
> +};
Can we just use DMA_TO_DEVICE, DMA_FROM_DEVICE, DMA_BIDIRECTIONAL from the
dma api? I think that would be much better for 2 reasons:
- consistency between dma api and dma_buf api
- on sane machines (x86) the dma_buf allocator could just grab an sg_list
and directly map the get_scatterlist to the dma_map_sg call.
> +/**
> + * enum dma_buf_attr_flags - Defines the attributes for this buffer. This
> + * can allow 'late backing of buffer' based on first get_scatterlist() call.
> + * @DMA_BUF_ATTR_CONTIG: Contiguous buffer
> + * @DMA_BUF_ATTR_DISCONTIG: Discontiguous buffer
> + * @DMA_BUF_ATTR_CUSTOM: Platform specific buffer; should evolve into some
> + * attributes that define buffers on given specific platform
> + */
> +enum dma_buf_attr_flags {
> + DMA_BUF_ATTR_CONTIG,
> + DMA_BUF_ATTR_DISCONTIG,
> + DMA_BUF_ATTR_CUSTOM,
> + DMA_BUF_ATTR_MAX,
> +};
I don't like this for essentially the same reasons as Rob. Also, if we
really need this I think it should be passed in at attach time, not at
get_scatterlist (where it might be too late). Also, for the same reasons
as with the direction flags above, I want this to be consistent with the
solution the normal dma mapping api provides.
Furthermore adding flags for extendability always souns great until you
actually want to extend your api: Usually you then notice gazillions of
ad-hoc extension and the associated mess, making extensions harder than
easier. So for dma_buf, first try I'd aim for the simplest possible
solution that can get the job done. No strange flags, please ;-)
> +/**
> + * struct dma_buf_attachment - holds device-buffer attachment data
> + * @node: list_head to allow manipulation of list of dma_buf_attachment.
> + * @dev: device attached to the buffer.
> + * @priv: exporter-specific attachment data.
> + */
> +struct dma_buf_attachment {
> + struct list_head node;
> + struct device *dev;
> + void *priv;
> +};
> +
> +/**
> + * struct dma_buf_ops - operations possible on struct dma_buf
> + * @create: creates a struct dma_buf of a fixed size. Actual allocation
> + * does not happen here.
> + * @attach: allows different devices to 'attach' themselves to the given
> + * buffer. It might return -EBUSY to signal that backing storage
> + * is already allocated and incompatible with the requirements
> + * of requesting device.
> + * @detach: detach a given device from this buffer.
> + * @get_scatterlist: returns list of scatter pages allocated, increases
> + * usecount of the buffer. Requires atleast one attach to be
> + * called before.
I think the important bit to spec here is that the returned sg list is
already mapped into the _device_ address space. This would support the
use case of a special remmapper (like ompas TILER) that a group of devices
could access to e.g. see tiled buffers as linear buffers. That would
obviously require some driver/platform specific magic, but I think we
should design the interface to allow this.
Also, I think this will give us a cleaner interface, because about the
only thing the driver can do with that sg list is to map it. So better
fold that into the get_scatterlist.
I'd still like to see a name for the get/put_scatterlist that matches more
the dma api (i.e. a wraper dma_buf_map or something), but this is a bit
bikeshedding, so I don't care much.
> + * @put_scatterlist: decreases usecount of buffer, might deallocate scatter
> + * pages.
> + * @mmap: memory map this buffer - optional.
> + * @release: release this buffer; to be called after the last dma_buf_put.
> + * @sync_sg_for_cpu: sync the sg list for cpu.
> + * @sync_sg_for_device: synch the sg list for device.
> + */
> +struct dma_buf_ops {
> + void (*attach)(struct dma_buf *, struct device *,
> + struct dma_buf_attachment *);
> +
> + void (*detach)(struct dma_buf *, struct dma_buf_attachment *);
> +
> + struct scatterlist * (*get_scatterlist)(struct dma_buf *,
> + struct dma_buf_attachment *,
> + enum dma_buf_optype,
> + enum dma_buf_attr_flags);
> + void (*put_scatterlist)(struct dma_buf *, struct dma_buf_attachment *,
> + struct scatterlist *);
> +
> + /* allow mmap optionally for devices that need it */
> + int (*mmap)(struct dma_buf *, struct vm_area_struct *);
> + /* after final dma_buf_put() */
> + void (*release)(struct dma_buf *);
> +
> + /* allow allocator to take care of cache ops */
> + void (*sync_sg_for_cpu) (struct dma_buf *, struct device *);
> + void (*sync_sg_for_device)(struct dma_buf *, struct device *);
> +};
> +
> +/**
> + * struct dma_buf - shared buffer object
> + * @file: file pointer used for sharing buffers across, and for refcounting.
> + * @attachments: list of dma_buf_attachment that denotes all devices attached.
> + * @ops: dma_buf_ops associated with this buffer object
> + * @priv: user specific private data
> + */
> +struct dma_buf {
> + size_t size;
> + struct file *file;
> + struct list_head attachments;
> + struct dma_buf_ops *ops;
> + void *priv;
> +};
> +
> +#ifdef CONFIG_DMA_SHARED_BUFFER
> +struct dma_buf_attachment *dma_buf_attach(struct dma_buf *dmabuf,
> + struct device *dev);
> +void dma_buf_detach(struct dma_buf *dmabuf,
> + struct dma_buf_attachment *dmabuf_attach);
> +struct dma_buf *dma_buf_export(void *priv, struct dma_buf_ops *ops, int flags);
> +int dma_buf_fd(struct dma_buf *dmabuf);
> +struct dma_buf *dma_buf_get(int fd);
> +void dma_buf_put(struct dma_buf *dmabuf);
> +
> +#else
> +
> +static inline struct dma_buf_attachment *dma_buf_attach(struct dma_buf *dmabuf,
> + struct device *dev)
> +{
> + return ERR_PTR(-ENODEV);
> +}
> +
> +static inline void dma_buf_detach(struct dma_buf *dmabuf,
> + struct dma_buf_attachment *dmabuf_attach)
> +{
> + return;
> +}
> +
> +static inline struct dma_buf *dma_buf_export(void *priv,
> + struct dma_buf_ops *ops,
> + int flags)
> +{
> + return ERR_PTR(-ENODEV);
> +}
> +
> +static inline int dma_buf_fd(struct dma_buf *dmabuf)
> +{
> + return -ENODEV;
> +}
> +
> +static inline struct dma_buf *dma_buf_get(int fd)
> +{
> + return ERR_PTR(-ENODEV);
> +}
> +
> +static inline void dma_buf_put(struct dma_buf *dmabuf)
> +{
> + return;
> +}
> +#endif /* CONFIG_DMA_SHARED_BUFFER */
> +
> +#endif /* __DMA_BUF_H__ */
> --
> 1.7.4.1
>
--
Daniel Vetter
Mail: daniel at ffwll.ch
Mobile: +41 (0)79 365 57 48
More information about the Linaro-mm-sig
mailing list