Re: [Linaro-mm-sig] [RFC 1/2] dma-buf: Introduce dma buffer sharing mechanism
Hi Dave, Daniel, Rob, On Sun, Nov 27, 2011 at 12:29 PM, Rob Clark robdcl...@gmail.com wrote: On Sat, Nov 26, 2011 at 8:00 AM, Daniel Vetter dan...@ffwll.ch wrote: On Fri, Nov 25, 2011 at 17:28, Dave Airlie airl...@gmail.com wrote: I've rebuilt my PRIME interface on top of dmabuf to see how it would work, I've got primed gears running again on top, but I expect all my object lifetime and memory ownership rules need fixing up (i.e. leaks like a sieve). http://cgit.freedesktop.org/~airlied/linux/log/?h=drm-prime-dmabuf has the i915/nouveau patches for the kernel to produce the prime interface. I've noticed that your implementations for get_scatterlist (at least for the i915 driver) doesn't return the sg table mapped into the device address space. I've checked and the documentation makes it clear that this should be the case (and we really need this to support certain insane hw), but the get/put_scatterlist names are a bit misleading. Proposal: - use struct sg_table instead of scatterlist like you've already done in you branch. Simply more consistent with the dma api. yup - rename get/put_scatterlist into map/unmap for consistency with all the map/unmap dma api functions. The attachement would then serve as the abstract cookie to the backing storage, similar to how struct page * works as an abstract cookie for dma_map/unmap_page. The only special thing is that struct device * parameter because that's already part of the attachment. yup - add new wrapper functions dma_buf_map_attachment and dma_buf_unmap_attachement to hide all the pointer/vtable-chasing that we currently expose to users of this interface. I thought that was one of the earlier comments on the initial dmabuf patch, but either way: yup Thanks for your comments; I will incorporate all of these in the next version I'll send out. BR, -R BR, Sumit. Comments? Cheers, Daniel -- Daniel Vetter daniel.vet...@ffwll.ch - +41 (0) 79 364 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Linaro-mm-sig] [RFC 1/2] dma-buf: Introduce dma buffer sharing mechanism
On Fri, Nov 25, 2011 at 17:28, Dave Airlie airl...@gmail.com wrote: I've rebuilt my PRIME interface on top of dmabuf to see how it would work, I've got primed gears running again on top, but I expect all my object lifetime and memory ownership rules need fixing up (i.e. leaks like a sieve). http://cgit.freedesktop.org/~airlied/linux/log/?h=drm-prime-dmabuf has the i915/nouveau patches for the kernel to produce the prime interface. I've noticed that your implementations for get_scatterlist (at least for the i915 driver) doesn't return the sg table mapped into the device address space. I've checked and the documentation makes it clear that this should be the case (and we really need this to support certain insane hw), but the get/put_scatterlist names are a bit misleading. Proposal: - use struct sg_table instead of scatterlist like you've already done in you branch. Simply more consistent with the dma api. - rename get/put_scatterlist into map/unmap for consistency with all the map/unmap dma api functions. The attachement would then serve as the abstract cookie to the backing storage, similar to how struct page * works as an abstract cookie for dma_map/unmap_page. The only special thing is that struct device * parameter because that's already part of the attachment. - add new wrapper functions dma_buf_map_attachment and dma_buf_unmap_attachement to hide all the pointer/vtable-chasing that we currently expose to users of this interface. Comments? Cheers, Daniel -- Daniel Vetter daniel.vet...@ffwll.ch - +41 (0) 79 364 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Linaro-mm-sig] [RFC 1/2] dma-buf: Introduce dma buffer sharing mechanism
On Sat, Nov 26, 2011 at 8:00 AM, Daniel Vetter dan...@ffwll.ch wrote: On Fri, Nov 25, 2011 at 17:28, Dave Airlie airl...@gmail.com wrote: I've rebuilt my PRIME interface on top of dmabuf to see how it would work, I've got primed gears running again on top, but I expect all my object lifetime and memory ownership rules need fixing up (i.e. leaks like a sieve). http://cgit.freedesktop.org/~airlied/linux/log/?h=drm-prime-dmabuf has the i915/nouveau patches for the kernel to produce the prime interface. I've noticed that your implementations for get_scatterlist (at least for the i915 driver) doesn't return the sg table mapped into the device address space. I've checked and the documentation makes it clear that this should be the case (and we really need this to support certain insane hw), but the get/put_scatterlist names are a bit misleading. Proposal: - use struct sg_table instead of scatterlist like you've already done in you branch. Simply more consistent with the dma api. yup - rename get/put_scatterlist into map/unmap for consistency with all the map/unmap dma api functions. The attachement would then serve as the abstract cookie to the backing storage, similar to how struct page * works as an abstract cookie for dma_map/unmap_page. The only special thing is that struct device * parameter because that's already part of the attachment. yup - add new wrapper functions dma_buf_map_attachment and dma_buf_unmap_attachement to hide all the pointer/vtable-chasing that we currently expose to users of this interface. I thought that was one of the earlier comments on the initial dmabuf patch, but either way: yup BR, -R Comments? Cheers, Daniel -- Daniel Vetter daniel.vet...@ffwll.ch - +41 (0) 79 364 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Linaro-mm-sig] [RFC 1/2] dma-buf: Introduce dma buffer sharing mechanism
On Fri, Nov 25, 2011 at 02:13:22PM +, Dave Airlie wrote: On Tue, Oct 11, 2011 at 10:23 AM, Sumit Semwal sumit.sem...@ti.com wrote: This is the first step in defining a dma buffer sharing mechanism. A new buffer object dma_buf is added, with operations and API to allow 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 user to share the scatterlist using get_scatterlist and put_scatterlist operations. Atleast one 'attach()' call is required to be made prior to calling the get_scatterlist() operation. Couple of building blocks in get_scatterlist() are added to ease introduction of sync'ing across exporter and users, and late allocation by the exporter. mmap() file operation is provided for the associated 'fd', as wrapper over the optional allocator defined mmap(), to be used by devices that might need one. More details are there in the documentation patch. Some questions, I've started playing around with using this framework to do buffer sharing between DRM devices, Why struct scatterlist and not struct sg_table? it seems like I really want to use an sg_table, No reason at all besides that intel-gtt is using scatterlist internally (and only kludges the sg_table together in an ad-hoc fashion) and so I haven't noticed. sg_table for more consistency with the dma api sounds good. I'm not convinced fd's are really useful over just some idr allocated handle, so far I'm just returning the fd to userspace as a handle, and passing it back in the other side, so I'm not really sure what an fd wins us here, apart from the mmap thing which I think shouldn't be here anyways. (if fd's do win us more we should probably record that in the docs patch). Imo fds are nice because their known and there's already all the preexisting infrastructure for them around. And if we ever get fancy with e.g. sync objects we can easily add poll support (or some insane ioctls). But I agree that we can mmap is bust as a reason and should just die. -Daniel -- Daniel Vetter Mail: dan...@ffwll.ch Mobile: +41 (0)79 365 57 48 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Linaro-mm-sig] [RFC 1/2] dma-buf: Introduce dma buffer sharing mechanism
+struct dma_buf_attachment *dma_buf_attach(struct dma_buf *dmabuf,
+ struct device *dev)
+{
+ struct dma_buf_attachment *attach;
+ int ret;
+
+ BUG_ON(!dmabuf || !dev);
+
+ mutex_lock(dmabuf-lock);
+
+ attach = kzalloc(sizeof(struct dma_buf_attachment), GFP_KERNEL);
+ if (attach == NULL)
+ goto err_alloc;
+
+ attach-dev = dev;
+ if (dmabuf-ops-attach) {
+ ret = dmabuf-ops-attach(dmabuf, dev, attach);
+ if (!ret)
+ goto err_attach;
+ }
+ list_add(attach-node, dmabuf-attachments);
+
I would assume at some point this needed at
attach-dmabuf = dmabuf;
added.
Dave.
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Linaro-mm-sig] [RFC 1/2] dma-buf: Introduce dma buffer sharing mechanism
I've rebuilt my PRIME interface on top of dmabuf to see how it would work, I've got primed gears running again on top, but I expect all my object lifetime and memory ownership rules need fixing up (i.e. leaks like a sieve). http://cgit.freedesktop.org/~airlied/linux/log/?h=drm-prime-dmabuf has the i915/nouveau patches for the kernel to produce the prime interface. Dave. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 1/2] dma-buf: Introduce dma buffer sharing mechanism
On Thu, Nov 3, 2011 at 3:04 AM, Marek Szyprowski m.szyprow...@samsung.com wrote: Hello, I'm sorry for a late reply, but after Kernel Summit/ELC I have some comments. On Friday, October 14, 2011 5:35 PM Daniel Vetter wrote: On Fri, Oct 14, 2011 at 12:00:58PM +0200, Tomasz Stanislawski wrote: +/** + * 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. The 'create' ops is not present in dma_buf_ops. + * @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. [optional] + * @detach: detach a given device from this buffer. [optional] + * @get_scatterlist: returns list of scatter pages allocated, increases + * usecount of the buffer. Requires atleast one attach to be + * called before. Returned sg list should already be mapped + * into _device_ address space. You must add a comment that this call 'may sleep'. I like the get_scatterlist idea. It allows the exported to create a valid scatterlist for a client in a elegant way. I do not like this whole attachment idea. The problem is that currently there is no support in DMA framework for allocation for multiple devices. As long as no such a support exists, there is no generic way to handle attribute negotiations and buffer allocations that involve multiple devices. So the exporter drivers would have to implement more or less hacky solutions to handle memory requirements and choosing the device that allocated memory. Currently, AFAIK there is even no generic way for a driver to acquire its own DMA memory requirements. Therefore all logic hidden beneath 'attachment' is pointless. I think that support for attach/detach (and related stuff) should be postponed until support for multi-device allocation is added to DMA framework. Imo we clearly need this to make the multi-device-driver with insane dma requirements work on arm. And rewriting the buffer handling in participating subsystem twice isn't really a great plan. I envision that on platforms where we need this madness, the driver must call back to the dma subsytem to create a dma_buf. The dma subsytem should be already aware of all the requirements and hence should be able to handle them.. I don't say the attachment list idea is wrong but adding attachment stuff creates an illusion that problem of multi-device allocations is somehow magically solved. We should not force the developers of exporter drivers to solve the problem that is not solvable yet. Well, this is why we need to create a decent support infrastructure for platforms (= arm madness) that needs this, so that device drivers and subsystem don't need to invent that wheel on their own. Which as you point out, they actually can't. The real question is whether it is possible to create any generic support infrastructure. I really doubt. IMHO this is something that will be hacked for each 'product release' and will never read the mainline... The other problem are the APIs. For example, the V4L2 subsystem assumes that memory is allocated after successful VIDIOC_REQBUFS with V4L2_MEMORY_MMAP memory type. Therefore attach would be automatically followed by get_scatterlist, blocking possibility of any buffer migrations in future. Well, pardon to break the news, but v4l needs to rework the buffer handling. If you want to share buffers with a gpu driver, you _have_ to life with the fact that gpus do fully dynamic buffer management, meaning: - buffers get allocated and destroyed on the fly, meaning static reqbuf just went out the window (we obviously cache buffer objects and reuse them for performance, as long as the processing pipeline doesn't really change). - buffers get moved around in memory, meaning you either need full-blown sync-objects with a callback to drivers to tear-down mappings on-demand, or every driver needs to guarnatee to call put_scatterlist in a reasonable short time. The latter is probably the more natural thing for v4l devices. I'm really not convinced if it is possible to go for the completely dynamic buffer management, especially if we are implementing a proof-of-concept solution. Please notice the following facts: 1. all v4l2 drivers do the 'static' buffer management - memory is being allocated on REQBUF() call and then mapped permanently into both userspace and dma (io) address space. Is this strictly true if we are introducing a new 'enum v4l2_memory' for dmabuf's? Shouldn't that give us some flexibility, especially if the v4l2 device is only the importer, not the allocator, of the memory. and a couple
RE: [RFC 1/2] dma-buf: Introduce dma buffer sharing mechanism
Hello, I'm sorry for a late reply, but after Kernel Summit/ELC I have some comments. On Friday, October 14, 2011 5:35 PM Daniel Vetter wrote: On Fri, Oct 14, 2011 at 12:00:58PM +0200, Tomasz Stanislawski wrote: +/** + * 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. The 'create' ops is not present in dma_buf_ops. + * @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. [optional] + * @detach: detach a given device from this buffer. [optional] + * @get_scatterlist: returns list of scatter pages allocated, increases + * usecount of the buffer. Requires atleast one attach to be + * called before. Returned sg list should already be mapped + * into _device_ address space. You must add a comment that this call 'may sleep'. I like the get_scatterlist idea. It allows the exported to create a valid scatterlist for a client in a elegant way. I do not like this whole attachment idea. The problem is that currently there is no support in DMA framework for allocation for multiple devices. As long as no such a support exists, there is no generic way to handle attribute negotiations and buffer allocations that involve multiple devices. So the exporter drivers would have to implement more or less hacky solutions to handle memory requirements and choosing the device that allocated memory. Currently, AFAIK there is even no generic way for a driver to acquire its own DMA memory requirements. Therefore all logic hidden beneath 'attachment' is pointless. I think that support for attach/detach (and related stuff) should be postponed until support for multi-device allocation is added to DMA framework. Imo we clearly need this to make the multi-device-driver with insane dma requirements work on arm. And rewriting the buffer handling in participating subsystem twice isn't really a great plan. I envision that on platforms where we need this madness, the driver must call back to the dma subsytem to create a dma_buf. The dma subsytem should be already aware of all the requirements and hence should be able to handle them.. I don't say the attachment list idea is wrong but adding attachment stuff creates an illusion that problem of multi-device allocations is somehow magically solved. We should not force the developers of exporter drivers to solve the problem that is not solvable yet. Well, this is why we need to create a decent support infrastructure for platforms (= arm madness) that needs this, so that device drivers and subsystem don't need to invent that wheel on their own. Which as you point out, they actually can't. The real question is whether it is possible to create any generic support infrastructure. I really doubt. IMHO this is something that will be hacked for each 'product release' and will never read the mainline... The other problem are the APIs. For example, the V4L2 subsystem assumes that memory is allocated after successful VIDIOC_REQBUFS with V4L2_MEMORY_MMAP memory type. Therefore attach would be automatically followed by get_scatterlist, blocking possibility of any buffer migrations in future. Well, pardon to break the news, but v4l needs to rework the buffer handling. If you want to share buffers with a gpu driver, you _have_ to life with the fact that gpus do fully dynamic buffer management, meaning: - buffers get allocated and destroyed on the fly, meaning static reqbuf just went out the window (we obviously cache buffer objects and reuse them for performance, as long as the processing pipeline doesn't really change). - buffers get moved around in memory, meaning you either need full-blown sync-objects with a callback to drivers to tear-down mappings on-demand, or every driver needs to guarnatee to call put_scatterlist in a reasonable short time. The latter is probably the more natural thing for v4l devices. I'm really not convinced if it is possible to go for the completely dynamic buffer management, especially if we are implementing a proof-of-concept solution. Please notice the following facts: 1. all v4l2 drivers do the 'static' buffer management - memory is being allocated on REQBUF() call and then mapped permanently into both userspace and dma (io) address space. 2. dma-mapping api is very limited in the area of the dynamic buffer management, this API has been designed definitely for static buffer allocation and mapping. It looks that fully dynamic buffer management requires a complete change of v4l2 api principles (V4L3?) and a completely new DMA API interface.
Re: [RFC 1/2] dma-buf: Introduce dma buffer sharing mechanism
Hi Mr. Sumit Semwal, Thank you for taking care of the framework for buffer sharing. The support of buffer sharing in V4L2, both exporting and importing was posted in shrbuf proof-of-concept patch. It should be easy to port it to dmabuf. http://lists.linaro.org/pipermail/linaro-mm-sig/2011-August/000485.html Please refer to the comments below: On 10/11/2011 11:23 AM, Sumit Semwal wrote: This is the first step in defining a dma buffer sharing mechanism. A new buffer object dma_buf is added, with operations and API to allow 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 user to share the scatterlist using get_scatterlist and put_scatterlist operations. Atleast one 'attach()' call is required to be made prior to calling the get_scatterlist() operation. Couple of building blocks in get_scatterlist() are added to ease introduction of sync'ing across exporter and users, and late allocation by the exporter. mmap() file operation is provided for the associated 'fd', as wrapper over the optional allocator defined mmap(), to be used by devices that might need one. More details are there in the documentation patch. This is based on design suggestions from many people at the mini-summits[1], most notably from Arnd Bergmanna...@arndb.de, Rob Clarkr...@ti.com and Daniel Vetterdan...@ffwll.ch. The implementation is inspired from proof-of-concept patch-set from Tomasz Stanislawskit.stanisl...@samsung.com, who demonstrated buffer sharing between two v4l2 devices. [2] [1]: https://wiki.linaro.org/OfficeofCTO/MemoryManagement [2]: http://lwn.net/Articles/454389 Signed-off-by: Sumit Semwalsumit.sem...@linaro.org Signed-off-by: Sumit Semwalsumit.sem...@ti.com --- drivers/base/Kconfig| 10 ++ drivers/base/Makefile |1 + drivers/base/dma-buf.c | 242 +++ include/linux/dma-buf.h | 162 +++ 4 files changed, 415 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 000..58c51a0 --- /dev/null +++ b/drivers/base/dma-buf.c @@ -0,0 +1,242 @@ +/* + * Framework for buffer objects that can be shared across devices/subsystems. + * + * Copyright(C) 2011 Linaro Limited. All rights reserved. + * Author: Sumit Semwalsumit.sem...@ti.com + * + * Many thanks to linaro-mm-sig list, and specially + * Arnd Bergmanna...@arndb.de, Rob Clarkr...@ti.com and + * Daniel Vetterdan...@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,
Re: [RFC 1/2] dma-buf: Introduce dma buffer sharing mechanism
On 14 October 2011 15:30, Tomasz Stanislawski t.stanisl...@samsung.com wrote:
Hi Mr. Sumit Semwal,
Hello Mr. Tomasz Stanislawski :),
Thank you for taking care of the framework for buffer sharing.
The support of buffer sharing in V4L2, both exporting and importing was
posted in shrbuf proof-of-concept patch. It should be easy to port it to
dmabuf.
http://lists.linaro.org/pipermail/linaro-mm-sig/2011-August/000485.html
I should thank you for the wonderful proof-of-concept patch, and the
idea behind it! I am currently working on the V4L2 side patch for it,
and would send out the RFC soon.
Also, thanks for a good review and some pertinent points; replies inline.
Please refer to the comments below:
On 10/11/2011 11:23 AM, Sumit Semwal wrote:
snip
+/**
+ * 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.
What is the purpose of these flags? The file is not visible to any process
by any file system, is it?
These are the standard file mode flags, which can be used to do
file-level access-type control by the exporter, so for example
write-access can be denied for a buffer exported as a read-only
buffer.
+ *
+ * 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;
+
why priv is not allowed to be NULL?
priv will be used by the exporter to attach its own context to the dma
buf; I couldn't think of any case where it could be NULL?
+ BUG_ON(!priv || !ops);
+
snip
+ BUG_ON(!dmabuf-file);
+
+ fput(dmabuf-file);
+
return is not needed
Right; will correct this.
+ return;
+}
+EXPORT_SYMBOL(dma_buf_put);
+
+/**
+ * dma_buf_attach - Add the device to dma_buf's attachments list;
optionally,
+ * calls 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;
+ int ret;
+
+ BUG_ON(!dmabuf || !dev);
+
+ mutex_lock(dmabuf-lock);
+
There is no need to call kzalloc inside critical section protected by
dmabuf-lock. The code would be simpler if the allocation is moved outside
the section.
Yes, you're right; will correct it.
+ attach = kzalloc(sizeof(struct dma_buf_attachment), GFP_KERNEL);
+ if (attach == NULL)
+ goto err_alloc;
+
+ attach-dev = dev;
+ if (dmabuf-ops-attach) {
+ ret = dmabuf-ops-attach(dmabuf, dev, attach);
+ if (!ret)
+ goto err_attach;
+ }
+ list_add(attach-node,dmabuf-attachments);
+
+err_alloc:
+ mutex_unlock(dmabuf-lock);
+ return attach;
+err_attach:
+ kfree(attach);
+ mutex_unlock(dmabuf-lock);
+ return ERR_PTR(ret);
+}
+EXPORT_SYMBOL(dma_buf_attach);
+
+/**
+ * dma_buf_detach - Remove the given attachment from dmabuf's attachments
list;
+ * optionally 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);
+
+ mutex_lock(dmabuf-lock);
+ list_del(attach-node);
+ if (dmabuf-ops-detach)
+ dmabuf-ops-detach(dmabuf, attach);
+
[as above]
Ok.
+ kfree(attach);
+ mutex_unlock(dmabuf-lock);
+ 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 000..5bdf16a
--- /dev/null
+++ b/include/linux/dma-buf.h
@@ -0,0 +1,162 @@
+/*
+ * Header file for dma buffer sharing framework.
+ *
+ * Copyright(C) 2011 Linaro Limited. All rights reserved.
+ * Author: Sumit Semwalsumit.sem...@ti.com
+ *
+ * Many thanks to linaro-mm-sig list, and specially
+ * Arnd Bergmanna...@arndb.de, Rob Clarkr...@ti.com and
+ * Daniel Vetterdan...@ffwll.ch for their support in creation and
+ * refining of this idea.
+ *
+ * This program
Re: [RFC 1/2] dma-buf: Introduce dma buffer sharing mechanism
On Fri, Oct 14, 2011 at 12:00:58PM +0200, Tomasz Stanislawski wrote: +/** + * 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. The 'create' ops is not present in dma_buf_ops. + * @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. [optional] + * @detach: detach a given device from this buffer. [optional] + * @get_scatterlist: returns list of scatter pages allocated, increases + * usecount of the buffer. Requires atleast one attach to be + * called before. Returned sg list should already be mapped + * into _device_ address space. You must add a comment that this call 'may sleep'. I like the get_scatterlist idea. It allows the exported to create a valid scatterlist for a client in a elegant way. I do not like this whole attachment idea. The problem is that currently there is no support in DMA framework for allocation for multiple devices. As long as no such a support exists, there is no generic way to handle attribute negotiations and buffer allocations that involve multiple devices. So the exporter drivers would have to implement more or less hacky solutions to handle memory requirements and choosing the device that allocated memory. Currently, AFAIK there is even no generic way for a driver to acquire its own DMA memory requirements. Therefore all logic hidden beneath 'attachment' is pointless. I think that support for attach/detach (and related stuff) should be postponed until support for multi-device allocation is added to DMA framework. Imo we clearly need this to make the multi-device-driver with insane dma requirements work on arm. And rewriting the buffer handling in participating subsystem twice isn't really a great plan. I envision that on platforms where we need this madness, the driver must call back to the dma subsytem to create a dma_buf. The dma subsytem should be already aware of all the requirements and hence should be able to handle them.. I don't say the attachment list idea is wrong but adding attachment stuff creates an illusion that problem of multi-device allocations is somehow magically solved. We should not force the developers of exporter drivers to solve the problem that is not solvable yet. Well, this is why we need to create a decent support infrastructure for platforms (= arm madness) that needs this, so that device drivers and subsystem don't need to invent that wheel on their own. Which as you point out, they actually can't. The other problem are the APIs. For example, the V4L2 subsystem assumes that memory is allocated after successful VIDIOC_REQBUFS with V4L2_MEMORY_MMAP memory type. Therefore attach would be automatically followed by get_scatterlist, blocking possibility of any buffer migrations in future. Well, pardon to break the news, but v4l needs to rework the buffer handling. If you want to share buffers with a gpu driver, you _have_ to life with the fact that gpus do fully dynamic buffer management, meaning: - buffers get allocated and destroyed on the fly, meaning static reqbuf just went out the window (we obviously cache buffer objects and reuse them for performance, as long as the processing pipeline doesn't really change). - buffers get moved around in memory, meaning you either need full-blown sync-objects with a callback to drivers to tear-down mappings on-demand, or every driver needs to guarnatee to call put_scatterlist in a reasonable short time. The latter is probably the more natural thing for v4l devices. The same situation happens if buffer sharing is added to framebuffer API. You can fix that by using the gem/ttm infrastructure of drm (or whatever the blob gpu drivers are using). Which is why I think fb should just die, please. The buffer sharing mechanism is dedicated to improve cooperation between multiple APIs. Therefore the common denominator strategy should be applied that is buffer-creation == buffer-allocation. No. Really, there's just no way gpu's will be moving back to static buffer management. And I know, for many use-cases we could get away with a bunch of static buffers (e.g. a video processing pipe). But in drm-land even scanout-buffers can get moved around - currently only when they're not being used, but strictly speaking nothing prevents us from copying the scanout to a new location and issueing a pageflip and so even move the buffer around even when it's in use. But let's look quickly at an OpenCL usecase, moving Gb's of date per second around between the cpu and a bunch of add-on gpus (or other special purpose processing units). We'd also need to extend dma_buf with sync objects to make this work well, but there's
Re: [RFC 1/2] dma-buf: Introduce dma buffer sharing mechanism
On Fri, Oct 14, 2011 at 5:00 AM, Tomasz Stanislawski
t.stanisl...@samsung.com wrote:
+ * @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. [optional]
+ * @detach: detach a given device from this buffer. [optional]
+ * @get_scatterlist: returns list of scatter pages allocated, increases
+ * usecount of the buffer. Requires atleast one attach
to be
+ * called before. Returned sg list should already be
mapped
+ * into _device_ address space.
You must add a comment that this call 'may sleep'.
I like the get_scatterlist idea. It allows the exported to create a valid
scatterlist for a client in a elegant way.
I do not like this whole attachment idea. The problem is that currently
there is no support in DMA framework for allocation for multiple devices. As
long as no such a support exists, there is no generic way to handle
attribute negotiations and buffer allocations that involve multiple devices.
So the exporter drivers would have to implement more or less hacky solutions
to handle memory requirements and choosing the device that allocated memory.
Currently, AFAIK there is even no generic way for a driver to acquire its
own DMA memory requirements.
dev-dma_params (struct device_dma_parameters).. for example
it would need to be expanded a bit to have a way to say it needs to
be physically contiguous..
Therefore all logic hidden beneath 'attachment' is pointless. I think that
support for attach/detach (and related stuff) should be postponed until
support for multi-device allocation is added to DMA framework.
I don't say the attachment list idea is wrong but adding attachment stuff
creates an illusion that problem of multi-device allocations is somehow
magically solved. We should not force the developers of exporter drivers to
solve the problem that is not solvable yet.
The other problem are the APIs. For example, the V4L2 subsystem assumes that
memory is allocated after successful VIDIOC_REQBUFS with V4L2_MEMORY_MMAP
memory type. Therefore attach would be automatically followed by
get_scatterlist, blocking possibility of any buffer migrations in future.
But this problem really only applies if v4l is your buffer allocator.
I don't think a v4l limitation is a valid argument to remove the
attachment stuff.
The same situation happens if buffer sharing is added to framebuffer API.
The buffer sharing mechanism is dedicated to improve cooperation between
multiple APIs. Therefore the common denominator strategy should be applied
that is buffer-creation == buffer-allocation.
I think it would be sufficient if buffer creators that cannot defer
the allocation just take a worst-case approach and allocate physically
contiguous buffers. No need to penalize other potential buffer
allocators. This allows buffer creators with more flexibility the
option for deferring the allocation until it knows whether the buffer
really needs to be contiguous.
+ * @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 {
+ int (*attach)(struct dma_buf *, struct device *,
+ struct dma_buf_attachment *);
+
+ void (*detach)(struct dma_buf *, struct dma_buf_attachment *);
+
+ /* For {get,put}_scatterlist below, any specific buffer attributes
+ * required should get added to device_dma_parameters accessible
+ * via dev-dma_params.
+ */
+ struct scatterlist * (*get_scatterlist)(struct dma_buf_attachment
*,
+ enum dma_data_direction,
+ int *nents);
+ void (*put_scatterlist)(struct dma_buf_attachment *,
+ struct scatterlist *,
+ int nents);
+ /* TODO: Add interruptible and interruptible_timeout versions */
I don't agree the interruptible and interruptible_timeout versions are
needed. I think that get_scatterlist should alway be interruptible. You can
add try_get_scatterlist callback that returns ERR_PTR(-EBUSY) if the call
would be blocking.
+
+ /* allow mmap optionally for devices that need it */
+ int (*mmap)(struct dma_buf *, struct vm_area_struct *);
The mmap is not needed for inital version. It could be added at any time in
the future. The dmabuf client should not be allowed to create mapping of the
dmabuf from the scatterlist.
fwiw, this wasn't
Re: [Linaro-mm-sig] [RFC 1/2] dma-buf: Introduce dma buffer sharing mechanism
On Tue, Oct 11, 2011 at 10:23 AM, Sumit Semwal sumit.sem...@ti.com wrote: This is the first step in defining a dma buffer sharing mechanism. A new buffer object dma_buf is added, with operations and API to allow 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 user to share the scatterlist using get_scatterlist and put_scatterlist operations. Atleast one 'attach()' call is required to be made prior to calling the get_scatterlist() operation. Couple of building blocks in get_scatterlist() are added to ease introduction of sync'ing across exporter and users, and late allocation by the exporter. mmap() file operation is provided for the associated 'fd', as wrapper over the optional allocator defined mmap(), to be used by devices that might need one. Why is this needed? it really doesn't make sense to be mmaping objects independent of some front-end like drm or v4l. how will you know what contents are in them, how will you synchronise access. Unless someone has a hard use-case for this I'd say we drop it until someone does. Dave. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Linaro-mm-sig] [RFC 1/2] dma-buf: Introduce dma buffer sharing mechanism
On Wed, Oct 12, 2011 at 7:41 AM, Dave Airlie airl...@gmail.com wrote: On Tue, Oct 11, 2011 at 10:23 AM, Sumit Semwal sumit.sem...@ti.com wrote: This is the first step in defining a dma buffer sharing mechanism. A new buffer object dma_buf is added, with operations and API to allow 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 user to share the scatterlist using get_scatterlist and put_scatterlist operations. Atleast one 'attach()' call is required to be made prior to calling the get_scatterlist() operation. Couple of building blocks in get_scatterlist() are added to ease introduction of sync'ing across exporter and users, and late allocation by the exporter. mmap() file operation is provided for the associated 'fd', as wrapper over the optional allocator defined mmap(), to be used by devices that might need one. Why is this needed? it really doesn't make sense to be mmaping objects independent of some front-end like drm or v4l. well, the mmap is actually implemented by the buffer allocator (v4l/drm).. although not sure if this was the point how will you know what contents are in them, how will you synchronise access. Unless someone has a hard use-case for this I'd say we drop it until someone does. The intent was that this is for well defined formats.. ie. it would need to be a format that both v4l and drm understood in the first place for sharing to make sense at all.. Anyways, the basic reason is to handle random edge cases where you need sw access to the buffer. For example, you are decoding video and pull out a frame to generate a thumbnail w/ a sw jpeg encoder.. On gstreamer 0.11 branch, for example, there is already a map/unmap virtual method on the gst buffer for sw access (ie. same purpose as PrepareAccess/FinishAccess in EXA). The idea w/ dmabuf mmap() support is that we could implement support to mmap()/munmap() before/after sw access. With this current scheme, synchronization could be handled in dmabufops-mmap() and vm_ops-close().. it is perhaps a bit heavy to require mmap/munmap for each sw access, but I suppose this isn't really for the high-performance use case. It is just so that some random bit of sw that gets passed a dmabuf handle without knowing who allocated it can have sw access if really needed. BR, -R Dave. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Linaro-mm-sig] [RFC 1/2] dma-buf: Introduce dma buffer sharing mechanism
well, the mmap is actually implemented by the buffer allocator (v4l/drm).. although not sure if this was the point Then why not use the correct interface? doing some sort of not-quite generic interface isn't really helping anyone except adding an ABI that we have to support. If someone wants to bypass the current kernel APIs we should add a new API for them not shove it into this generic buffer sharing layer. The intent was that this is for well defined formats.. ie. it would need to be a format that both v4l and drm understood in the first place for sharing to make sense at all.. How will you know the stride to take a simple example? The userspace had to create this buffer somehow and wants to share it with something, you sound like you really needs another API that is a simple accessor API that can handle mmaps. Anyways, the basic reason is to handle random edge cases where you need sw access to the buffer. For example, you are decoding video and pull out a frame to generate a thumbnail w/ a sw jpeg encoder.. Again, doesn't sound like it should be part of this API, and also sounds like the sw jpeg encoder will need more info about the buffer anyways like stride and format. With this current scheme, synchronization could be handled in dmabufops-mmap() and vm_ops-close().. it is perhaps a bit heavy to require mmap/munmap for each sw access, but I suppose this isn't really for the high-performance use case. It is just so that some random bit of sw that gets passed a dmabuf handle without knowing who allocated it can have sw access if really needed. So I think thats fine, write a sw accessor providers, don't go overloading the buffer sharing code. This API will limit what people can use this buffer sharing for with pure hw accessors, you might say, oh buts its okay to fail the mmap then, but the chances of sw handling that I'm not so sure off. Dave. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Linaro-mm-sig] [RFC 1/2] dma-buf: Introduce dma buffer sharing mechanism
On Wed, Oct 12, 2011 at 8:35 AM, Dave Airlie airl...@gmail.com wrote: well, the mmap is actually implemented by the buffer allocator (v4l/drm).. although not sure if this was the point Then why not use the correct interface? doing some sort of not-quite generic interface isn't really helping anyone except adding an ABI that we have to support. But what if you don't know who allocated the buffer? How do you know what interface to use to mmap? If someone wants to bypass the current kernel APIs we should add a new API for them not shove it into this generic buffer sharing layer. The intent was that this is for well defined formats.. ie. it would need to be a format that both v4l and drm understood in the first place for sharing to make sense at all.. How will you know the stride to take a simple example? The userspace had to create this buffer somehow and wants to share it with something, you sound like you really needs another API that is a simple accessor API that can handle mmaps. Well, things like stride, width, height, color format, userspace needs to know all this already, even for malloc()'d sw buffers. The assumption is userspace already has a way to pass this information around so it was not required to be duplicated by dmabuf. Anyways, the basic reason is to handle random edge cases where you need sw access to the buffer. For example, you are decoding video and pull out a frame to generate a thumbnail w/ a sw jpeg encoder.. Again, doesn't sound like it should be part of this API, and also sounds like the sw jpeg encoder will need more info about the buffer anyways like stride and format. With this current scheme, synchronization could be handled in dmabufops-mmap() and vm_ops-close().. it is perhaps a bit heavy to require mmap/munmap for each sw access, but I suppose this isn't really for the high-performance use case. It is just so that some random bit of sw that gets passed a dmabuf handle without knowing who allocated it can have sw access if really needed. So I think thats fine, write a sw accessor providers, don't go overloading the buffer sharing code. But then we'd need a different set of accessors for every different drm/v4l/etc driver, wouldn't we? This API will limit what people can use this buffer sharing for with pure hw accessors, you might say, oh buts its okay to fail the mmap then, but the chances of sw handling that I'm not so sure off. I'm not entirely sure the case you are worried about.. sharing buffers between multiple GPU's that understand same tiled formats? I guess that is a bit different from a case like a jpeg encoder that is passed a dmabuf handle without any idea where it came from.. I guess if sharing a buffer between multiple drm devices, there is nothing stopping you from having some NOT_DMABUF_MMAPABLE flag you pass when the buffer is allocated, then you don't have to support dmabuf-mmap(), and instead mmap via device and use some sort of DRM_CPU_PREP/FINI ioctls for synchronization.. BR, -R Dave. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Linaro-mm-sig] [RFC 1/2] dma-buf: Introduce dma buffer sharing mechanism
But then we'd need a different set of accessors for every different drm/v4l/etc driver, wouldn't we? Not any more different than you need for this, you just have a new interface that you request a sw object from, then mmap that object, and underneath it knows who owns it in the kernel. mmap just feels wrong in this API, which is a buffer sharing API not a buffer mapping API. I guess if sharing a buffer between multiple drm devices, there is nothing stopping you from having some NOT_DMABUF_MMAPABLE flag you pass when the buffer is allocated, then you don't have to support dmabuf-mmap(), and instead mmap via device and use some sort of DRM_CPU_PREP/FINI ioctls for synchronization.. Or we could make a generic CPU accessor that we don't have to worry about. Dave. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Linaro-mm-sig] [RFC 1/2] dma-buf: Introduce dma buffer sharing mechanism
On Wed, Oct 12, 2011 at 9:01 AM, Dave Airlie airl...@gmail.com wrote: But then we'd need a different set of accessors for every different drm/v4l/etc driver, wouldn't we? Not any more different than you need for this, you just have a new interface that you request a sw object from, then mmap that object, and underneath it knows who owns it in the kernel. oh, ok, so you are talking about a kernel level interface, rather than userspace.. but I guess in this case I don't quite see the difference. It amounts to which fd you call mmap (or ioctl[*]) on.. If you use the dmabuf fd directly then you don't have to pass around a 2nd fd. [*] there is nothing stopping defining some dmabuf ioctls (such as for synchronization).. although the thinking was to keep it simple for first version of dmabuf BR, -R mmap just feels wrong in this API, which is a buffer sharing API not a buffer mapping API. I guess if sharing a buffer between multiple drm devices, there is nothing stopping you from having some NOT_DMABUF_MMAPABLE flag you pass when the buffer is allocated, then you don't have to support dmabuf-mmap(), and instead mmap via device and use some sort of DRM_CPU_PREP/FINI ioctls for synchronization.. Or we could make a generic CPU accessor that we don't have to worry about. Dave. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Linaro-mm-sig] [RFC 1/2] dma-buf: Introduce dma buffer sharing mechanism
On Wed, Oct 12, 2011 at 3:24 PM, Rob Clark robdcl...@gmail.com wrote: On Wed, Oct 12, 2011 at 9:01 AM, Dave Airlie airl...@gmail.com wrote: But then we'd need a different set of accessors for every different drm/v4l/etc driver, wouldn't we? Not any more different than you need for this, you just have a new interface that you request a sw object from, then mmap that object, and underneath it knows who owns it in the kernel. oh, ok, so you are talking about a kernel level interface, rather than userspace.. but I guess in this case I don't quite see the difference. It amounts to which fd you call mmap (or ioctl[*]) on.. If you use the dmabuf fd directly then you don't have to pass around a 2nd fd. [*] there is nothing stopping defining some dmabuf ioctls (such as for synchronization).. although the thinking was to keep it simple for first version of dmabuf Yes a separate kernel level interface. Well I'd like to keep it even simpler. dmabuf is a buffer sharing API, shoehorning in a sw mapping API isn't making it simpler. The problem I have with implementing mmap on the sharing fd, is that nothing says this should be purely optional and userspace shouldn't rely on it. In the Intel GEM space alone you have two types of mapping, one direct to shmem one via GTT, the GTT could be even be a linear view. The intel guys initially did GEM mmaps direct to the shmem pages because it seemed simple, up until they had to do step two which was do mmaps on the GTT copy and ended up having two separate mmap methods. I think the problem here is it seems deceptively simple to add this to the API now because the API is simple, however I think in the future it'll become a burden that we'll have to workaround. Dave. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Linaro-mm-sig] [RFC 1/2] dma-buf: Introduce dma buffer sharing mechanism
On Wed, Oct 12, 2011 at 03:34:54PM +0100, Dave Airlie wrote: On Wed, Oct 12, 2011 at 3:24 PM, Rob Clark robdcl...@gmail.com wrote: On Wed, Oct 12, 2011 at 9:01 AM, Dave Airlie airl...@gmail.com wrote: But then we'd need a different set of accessors for every different drm/v4l/etc driver, wouldn't we? Not any more different than you need for this, you just have a new interface that you request a sw object from, then mmap that object, and underneath it knows who owns it in the kernel. oh, ok, so you are talking about a kernel level interface, rather than userspace.. but I guess in this case I don't quite see the difference. It amounts to which fd you call mmap (or ioctl[*]) on.. If you use the dmabuf fd directly then you don't have to pass around a 2nd fd. [*] there is nothing stopping defining some dmabuf ioctls (such as for synchronization).. although the thinking was to keep it simple for first version of dmabuf Yes a separate kernel level interface. Well I'd like to keep it even simpler. dmabuf is a buffer sharing API, shoehorning in a sw mapping API isn't making it simpler. The problem I have with implementing mmap on the sharing fd, is that nothing says this should be purely optional and userspace shouldn't rely on it. In the Intel GEM space alone you have two types of mapping, one direct to shmem one via GTT, the GTT could be even be a linear view. The intel guys initially did GEM mmaps direct to the shmem pages because it seemed simple, up until they had to do step two which was do mmaps on the GTT copy and ended up having two separate mmap methods. I think the problem here is it seems deceptively simple to add this to the API now because the API is simple, however I think in the future it'll become a burden that we'll have to workaround. Yeah, that's my feeling, too. Adding mmap sounds like a neat, simple idea, that could simplify things for simple devices like v4l. But as soon as you're dealing with a real gpu, nothing is simple. Those who don't believe this, just take a look at the data upload/download paths in the open-source i915,nouveau,radeon drivers. Making this fast (and for gpus, it needs to be fast) requires tons of tricks, special-cases and jumping through loops. You absolutely want the device-specific ioctls to do that. Adding a generic mmap just makes matters worse, especially if userspace expects this to work synchronized with everything else that is going on. Cheers, Daniel -- Daniel Vetter Mail: dan...@ffwll.ch Mobile: +41 (0)79 365 57 48 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Linaro-mm-sig] [RFC 1/2] dma-buf: Introduce dma buffer sharing mechanism
On Wed, Oct 12, 2011 at 9:34 AM, Dave Airlie airl...@gmail.com wrote: On Wed, Oct 12, 2011 at 3:24 PM, Rob Clark robdcl...@gmail.com wrote: On Wed, Oct 12, 2011 at 9:01 AM, Dave Airlie airl...@gmail.com wrote: But then we'd need a different set of accessors for every different drm/v4l/etc driver, wouldn't we? Not any more different than you need for this, you just have a new interface that you request a sw object from, then mmap that object, and underneath it knows who owns it in the kernel. oh, ok, so you are talking about a kernel level interface, rather than userspace.. but I guess in this case I don't quite see the difference. It amounts to which fd you call mmap (or ioctl[*]) on.. If you use the dmabuf fd directly then you don't have to pass around a 2nd fd. [*] there is nothing stopping defining some dmabuf ioctls (such as for synchronization).. although the thinking was to keep it simple for first version of dmabuf Yes a separate kernel level interface. I'm not against it, but if it is a device-independent interface, it just seems like six of one, half-dozen of the other.. Ie. how does it differ if the dmabuf fd is the fd used for ioctl/mmap, vs if some other /dev/buffer-sharer file that you open? But I think maybe I'm misunderstanding what you have in mind? BR, -R Well I'd like to keep it even simpler. dmabuf is a buffer sharing API, shoehorning in a sw mapping API isn't making it simpler. The problem I have with implementing mmap on the sharing fd, is that nothing says this should be purely optional and userspace shouldn't rely on it. In the Intel GEM space alone you have two types of mapping, one direct to shmem one via GTT, the GTT could be even be a linear view. The intel guys initially did GEM mmaps direct to the shmem pages because it seemed simple, up until they had to do step two which was do mmaps on the GTT copy and ended up having two separate mmap methods. I think the problem here is it seems deceptively simple to add this to the API now because the API is simple, however I think in the future it'll become a burden that we'll have to workaround. Dave. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC 1/2] dma-buf: Introduce dma buffer sharing mechanism
This is the first step in defining a dma buffer sharing mechanism.
A new buffer object dma_buf is added, with operations and API to allow 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 user to share the scatterlist using get_scatterlist and
put_scatterlist operations.
Atleast one 'attach()' call is required to be made prior to calling the
get_scatterlist() operation.
Couple of building blocks in get_scatterlist() are added to ease introduction
of sync'ing across exporter and users, and late allocation by the exporter.
mmap() file operation is provided for the associated 'fd', as wrapper over the
optional allocator defined mmap(), to be used by devices that might need one.
More details are there in the documentation patch.
This is based on design suggestions from many people at the mini-summits[1],
most notably from Arnd Bergmann a...@arndb.de, Rob Clark r...@ti.com and
Daniel Vetter dan...@ffwll.ch.
The implementation is inspired from proof-of-concept patch-set from
Tomasz Stanislawski t.stanisl...@samsung.com, who demonstrated buffer sharing
between two v4l2 devices. [2]
[1]: https://wiki.linaro.org/OfficeofCTO/MemoryManagement
[2]: http://lwn.net/Articles/454389
Signed-off-by: Sumit Semwal sumit.sem...@linaro.org
Signed-off-by: Sumit Semwal sumit.sem...@ti.com
---
drivers/base/Kconfig| 10 ++
drivers/base/Makefile |1 +
drivers/base/dma-buf.c | 242 +++
include/linux/dma-buf.h | 162 +++
4 files changed, 415 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 000..58c51a0
--- /dev/null
+++ b/drivers/base/dma-buf.c
@@ -0,0 +1,242 @@
+/*
+ * Framework for buffer objects that can be shared across devices/subsystems.
+ *
+ * Copyright(C) 2011 Linaro Limited. All rights reserved.
+ * Author: Sumit Semwal sumit.sem...@ti.com
+ *
+ * Many thanks to linaro-mm-sig list, and specially
+ * Arnd Bergmann a...@arndb.de, Rob Clark r...@ti.com and
+ * Daniel Vetter dan...@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
