Re: [Linaro-mm-sig] [PATCH] dma-buf: add meta data attachment
Hi all, On 2014년 03월 24일 16:35, Daniel Vetter wrote: Hi all, Adding piles more people. For the first case of caching the iommu mapping there's different answers, depening upon the exact case: 1) You need to fix your userspace to not constantly re-establish the sharing. 2) We need to add streaming dma support for real to dma-bufs so that the mapping can be kept while we transfer ownership around. Thus far no one really needed this though since usually you don't actually do cpu access. 3) You need opportunistic caching of imported/exported buffer objects and their mappings. For this you need a) subsystem import/export support which guarantees you to hand out the same dma-buf/native object again upon re-export or re-import (drm has it) b) some opportunistic caching of buffer objects (pretty much are real gpu drm drivers have it). No need of any metadata scheme, and given how much fun I've had implemented this for drm I don't you can make your metadata scheme work in a sane or correct way wrt lifetimes. For caching the iommu mapping if the iommu is the same for multiple devices: 1) We need some way to figure out which iommu serves which devices. 2) The exporter needs to consult this and might just hand out the same sg mapping out again if it wants to. No need for importers to do fancy stuff, or attach any importer-visible metadata to dma-bufs. Of course duplicating this code all over the place is a but uncool, so I expect that eventually we'll have a generic exporter implementation, at least for non-swappable buffers. drm/gem is a bit special here ... In general I don't like the idea of arbitrary metadata at all, sounds prone to abuse with crazy lifetime/refcounting rules for the objects involved. Also I think for a lot of your examples (like debugging) it would be much better if we have a standardized piece of metadata so that all drivers/platforms can use the same tooling. And it feels like I'm writing such a mail every few months ... I posted concept about importer priv of dma-buf, and it seems that this patch is partly from similar requirement - iommu map/unmap. And at that time, Daniel agreed at least the issue, that unnecessary map/unmap can repeatedly called, is also in the drm gem. http://lists.freedesktop.org/archives/dri-devel/2013-June/039469.html So I agree about the necessary of some data of dma-buf for general importer even though the data can be shared between different subsystems. Cheers, Daniel On Mon, Mar 24, 2014 at 7:20 AM, Bin Wang b...@marvell.com wrote: Hi Sumit, On 03/21/2014 07:26 PM, Sumit Semwal wrote: Hi Bin Wang, On 21 March 2014 10:34, Bin Wang b...@marvell.com wrote: we found there'd be varied specific data connected to a dmabuf, like the iommu mapping of buffer, the dma descriptors (that can be shared between several components). Though these info might be able to be generated every time before dma operations, for performance sake, it's better to be kept before really invalid. Thanks for your patch! I think we'd need to have more specific details on what does 'meta data' mean here; more specific comments inline. Change-Id: I89d43dc3fe1ee3da91c42074da5df71b968e6d3c Signed-off-by: Bin Wang b...@marvell.com --- drivers/base/dma-buf.c | 100 +++ include/linux/dma-buf.h | 22 ++ 2 files changed, 122 insertions(+), 0 deletions(-) diff --git a/drivers/base/dma-buf.c b/drivers/base/dma-buf.c index 08fe897..5c82e60 100644 --- a/drivers/base/dma-buf.c +++ b/drivers/base/dma-buf.c @@ -50,6 +50,7 @@ static int dma_buf_release(struct inode *inode, struct file *file) BUG_ON(dmabuf-vmapping_counter); + dma_buf_meta_release(dmabuf); dmabuf-ops-release(dmabuf); mutex_lock(db_list.lock); @@ -138,6 +139,7 @@ struct dma_buf *dma_buf_export_named(void *priv, const struct dma_buf_ops *ops, mutex_init(dmabuf-lock); INIT_LIST_HEAD(dmabuf-attachments); + INIT_LIST_HEAD(dmabuf-metas); I am not sure I understand the relationship of 'meta data' with 'dma-buf', or 'attachments' - do you mean to have a list of some meta-data that is unrelated to the attachments to the dma-buf? I think it'd help to explain whether meta-data is specific to each dma-buf, and there's a list of them, or this meta-data is related to each importer device attachment? If it's related to each importer, it should really be added as part of the dma_buf_attachment, and not separately. This is basic concept of my RFC patch. http://www.kernelhub.org/?p=2msg=268056 Best Regards, - Seung-Woo Kim The meta-data here can be any kind of data related to the dmabuf, , it's specific for each dmabuf. For example, we have iommu for a VPU device, VPU driver has to map the dmabuf to get an IOVA address before it can access the buffer. This dmabuf would be reused many times during the video playback, for
Re: [Linaro-mm-sig] [PATCH] dma-buf: add meta data attachment
On Tue, Mar 25, 2014 at 06:21:34PM +0900, Seung-Woo Kim wrote: Hi all, On 2014년 03월 24일 16:35, Daniel Vetter wrote: Hi all, Adding piles more people. For the first case of caching the iommu mapping there's different answers, depening upon the exact case: 1) You need to fix your userspace to not constantly re-establish the sharing. 2) We need to add streaming dma support for real to dma-bufs so that the mapping can be kept while we transfer ownership around. Thus far no one really needed this though since usually you don't actually do cpu access. 3) You need opportunistic caching of imported/exported buffer objects and their mappings. For this you need a) subsystem import/export support which guarantees you to hand out the same dma-buf/native object again upon re-export or re-import (drm has it) b) some opportunistic caching of buffer objects (pretty much are real gpu drm drivers have it). No need of any metadata scheme, and given how much fun I've had implemented this for drm I don't you can make your metadata scheme work in a sane or correct way wrt lifetimes. For caching the iommu mapping if the iommu is the same for multiple devices: 1) We need some way to figure out which iommu serves which devices. 2) The exporter needs to consult this and might just hand out the same sg mapping out again if it wants to. No need for importers to do fancy stuff, or attach any importer-visible metadata to dma-bufs. Of course duplicating this code all over the place is a but uncool, so I expect that eventually we'll have a generic exporter implementation, at least for non-swappable buffers. drm/gem is a bit special here ... In general I don't like the idea of arbitrary metadata at all, sounds prone to abuse with crazy lifetime/refcounting rules for the objects involved. Also I think for a lot of your examples (like debugging) it would be much better if we have a standardized piece of metadata so that all drivers/platforms can use the same tooling. And it feels like I'm writing such a mail every few months ... I posted concept about importer priv of dma-buf, and it seems that this patch is partly from similar requirement - iommu map/unmap. And at that time, Daniel agreed at least the issue, that unnecessary map/unmap can repeatedly called, is also in the drm gem. http://lists.freedesktop.org/archives/dri-devel/2013-June/039469.html So I agree about the necessary of some data of dma-buf for general importer even though the data can be shared between different subsystems. Oh right, I guess someone has to implement this eventually ;-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 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] [PATCH] dma-buf: add meta data attachment
Hi all, Adding piles more people. For the first case of caching the iommu mapping there's different answers, depening upon the exact case: 1) You need to fix your userspace to not constantly re-establish the sharing. 2) We need to add streaming dma support for real to dma-bufs so that the mapping can be kept while we transfer ownership around. Thus far no one really needed this though since usually you don't actually do cpu access. 3) You need opportunistic caching of imported/exported buffer objects and their mappings. For this you need a) subsystem import/export support which guarantees you to hand out the same dma-buf/native object again upon re-export or re-import (drm has it) b) some opportunistic caching of buffer objects (pretty much are real gpu drm drivers have it). No need of any metadata scheme, and given how much fun I've had implemented this for drm I don't you can make your metadata scheme work in a sane or correct way wrt lifetimes. For caching the iommu mapping if the iommu is the same for multiple devices: 1) We need some way to figure out which iommu serves which devices. 2) The exporter needs to consult this and might just hand out the same sg mapping out again if it wants to. No need for importers to do fancy stuff, or attach any importer-visible metadata to dma-bufs. Of course duplicating this code all over the place is a but uncool, so I expect that eventually we'll have a generic exporter implementation, at least for non-swappable buffers. drm/gem is a bit special here ... In general I don't like the idea of arbitrary metadata at all, sounds prone to abuse with crazy lifetime/refcounting rules for the objects involved. Also I think for a lot of your examples (like debugging) it would be much better if we have a standardized piece of metadata so that all drivers/platforms can use the same tooling. And it feels like I'm writing such a mail every few months ... Cheers, Daniel On Mon, Mar 24, 2014 at 7:20 AM, Bin Wang b...@marvell.com wrote: Hi Sumit, On 03/21/2014 07:26 PM, Sumit Semwal wrote: Hi Bin Wang, On 21 March 2014 10:34, Bin Wang b...@marvell.com wrote: we found there'd be varied specific data connected to a dmabuf, like the iommu mapping of buffer, the dma descriptors (that can be shared between several components). Though these info might be able to be generated every time before dma operations, for performance sake, it's better to be kept before really invalid. Thanks for your patch! I think we'd need to have more specific details on what does 'meta data' mean here; more specific comments inline. Change-Id: I89d43dc3fe1ee3da91c42074da5df71b968e6d3c Signed-off-by: Bin Wang b...@marvell.com --- drivers/base/dma-buf.c | 100 +++ include/linux/dma-buf.h | 22 ++ 2 files changed, 122 insertions(+), 0 deletions(-) diff --git a/drivers/base/dma-buf.c b/drivers/base/dma-buf.c index 08fe897..5c82e60 100644 --- a/drivers/base/dma-buf.c +++ b/drivers/base/dma-buf.c @@ -50,6 +50,7 @@ static int dma_buf_release(struct inode *inode, struct file *file) BUG_ON(dmabuf-vmapping_counter); + dma_buf_meta_release(dmabuf); dmabuf-ops-release(dmabuf); mutex_lock(db_list.lock); @@ -138,6 +139,7 @@ struct dma_buf *dma_buf_export_named(void *priv, const struct dma_buf_ops *ops, mutex_init(dmabuf-lock); INIT_LIST_HEAD(dmabuf-attachments); + INIT_LIST_HEAD(dmabuf-metas); I am not sure I understand the relationship of 'meta data' with 'dma-buf', or 'attachments' - do you mean to have a list of some meta-data that is unrelated to the attachments to the dma-buf? I think it'd help to explain whether meta-data is specific to each dma-buf, and there's a list of them, or this meta-data is related to each importer device attachment? If it's related to each importer, it should really be added as part of the dma_buf_attachment, and not separately. The meta-data here can be any kind of data related to the dmabuf, , it's specific for each dmabuf. For example, we have iommu for a VPU device, VPU driver has to map the dmabuf to get an IOVA address before it can access the buffer. This dmabuf would be reused many times during the video playback, for performance concern we don't want this dmabuf be map/unmaped by iommu driver every time of VPU HW data transfer, since the physical pages not changed and iommu IOVA plenty. Another example, in our SoC, device A and device B use the same dma chain format, and these drivers share dmabuf between each other, thus the dma chain array doesn't need to be generated every timer for each device. So here, with the help from the meta-data, only the first time we need to generate the meta-data from scratch, and later we can just reuse it until the buffer freed. The meta-data is related to the importer, or importers. However, the dma_buf_attachment is per devices, holding
