Re: [PATCH 3/4] videobuf2-dma-streaming: new videobuf2 memory allocator
Hi Jon, On Thu, Sep 13, 2012 at 2:54 PM, Jonathan Corbet wrote: > > Well, there is some documentation here: > > https://lwn.net/Articles/447435/ > I thank you for this. It really helped me getting started on videobuf2. > This reminds me that I've always meant to turn it into something to put > into Documentation/. I'll try to get to that soon. > Yes, that's something many of us will appreciate A LOT. Perhaps we could start with some skeleton documentation and then build from there. I maybe naive, but I think that way other developers might add little contributions. Regards, Ezequiel. PS: I wish I had some time to do it myself. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/4] videobuf2-dma-streaming: new videobuf2 memory allocator
Hi Jon, On Thu, Sep 13, 2012 at 2:54 PM, Jonathan Corbet cor...@lwn.net wrote: Well, there is some documentation here: https://lwn.net/Articles/447435/ I thank you for this. It really helped me getting started on videobuf2. This reminds me that I've always meant to turn it into something to put into Documentation/. I'll try to get to that soon. Yes, that's something many of us will appreciate A LOT. Perhaps we could start with some skeleton documentation and then build from there. I maybe naive, but I think that way other developers might add little contributions. Regards, Ezequiel. PS: I wish I had some time to do it myself. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/4] videobuf2-dma-streaming: new videobuf2 memory allocator
In data giovedì 13 settembre 2012 11:45:31, Jonathan Corbet ha scritto: > On Thu, 13 Sep 2012 17:46:32 +0200 > > Federico Vaga wrote: > > > A few words explaining why this memory handling module is required > > > or > > > beneficial will definitely improve the commit :) > > > > ok, I will write some lines > > In general, all of these patches need *much* better changelogs (i.e. > they need changelogs). What are you doing, and *why* are you doing > it? The future will want to know. I can improve the comment about the ADV7180: it is not clear why I'm removing that functions. (and the memory allocator commit is also in the todo list). The STA2X11_VIP commit, I think is clear, I convert it to use videobu2 and control framework. I can add some extra lines to explain why: because videobuf is obsolete > I'm going to try to look at the actual code, but time isn't something > I have a lot of, still... The actual code is the same of the previous one with yours (plural) suggestions from the RFC submission (few week ago). I did not write anything else. -- Federico Vaga -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/4] videobuf2-dma-streaming: new videobuf2 memory allocator
> Well, there is some documentation here: > > https://lwn.net/Articles/447435/ I know this, I learned from this page :) What I'm saying is that I don't know what to write inside the code to make it clearer than now. I think is clear, because if you know the videobuf2, you know what I'm doing in each vb2_mem_ops. I suppose that this is the reason why there are no comments inside the other memory allocator. Maybe I am wrong. -- Federico Vaga -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/4] videobuf2-dma-streaming: new videobuf2 memory allocator
On Thu, 13 Sep 2012 17:42:33 +0200 Federico Vaga wrote: > > The header and esp. the source could really do with more > > documentation. It is not at all clear from the code what the > > dma-streaming allocator does and how it differs from other > > allocators. > > The other allocators are not documented and to understand them I read > the code. All the memory allocators reflect a kind of DMA interface: SG > for scatter/gather, contig for choerent and (now) streaming for > streaming. So, I'm not sure to understand what do you think is the > correct way to document a memory allocator; I can write one or two lines > of comment to summarize each function. what do you think? Well, there is some documentation here: https://lwn.net/Articles/447435/ This reminds me that I've always meant to turn it into something to put into Documentation/. I'll try to get to that soon. In general, the fact that a subsystem is insufficiently documented is not a good reason to add more poorly-documented code. We are, after all, trying to make the kernel better over time... Thanks, jon -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/4] videobuf2-dma-streaming: new videobuf2 memory allocator
On Thu, 13 Sep 2012 17:46:32 +0200 Federico Vaga wrote: > > A few words explaining why this memory handling module is required or > > beneficial will definitely improve the commit :) > > ok, I will write some lines In general, all of these patches need *much* better changelogs (i.e. they need changelogs). What are you doing, and *why* are you doing it? The future will want to know. I'm going to try to look at the actual code, but time isn't something I have a lot of, still... Thanks, jon -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/4] videobuf2-dma-streaming: new videobuf2 memory allocator
> On Thursday, September 13, 2012 3:53 PM Federico Vaga wrote: > > Signed-off-by: Federico Vaga > > A few words explaining why this memory handling module is required or > beneficial will definitely improve the commit :) ok, I will write some lines > > +static void *vb2_dma_streaming_cookie(void *buf_priv) > > +{ > > + struct vb2_streaming_buf *buf = buf_priv; > > + > > + return (void *)buf->dma_handle; > > +} > > Please change this function to: > > static void *vb2_dma_streaming_cookie(void *buf_priv) > { > struct vb2_streaming_buf *buf = buf_priv; > return >dma_handle; > } > > and add a following static inline to > include/media/videobuf2-dma-streaming.h: > > static inline dma_addr_t > vb2_dma_streaming_plane_paddr(struct vb2_buffer *vb, unsigned int > plane_no) { > dma_addr_t *dma_addr = vb2_plane_cookie(vb, plane_no); > return *dma_addr; > } > > Do not use 'cookie' callback directly in the driver, the driver should > use the above proxy. > > The >dma_handle workaround is required for some possible > configurations with 64bit dma addresses, see commit 472af2b05bdefc. ACK. -- Federico Vaga -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/4] videobuf2-dma-streaming: new videobuf2 memory allocator
> typo: steaming -> streaming :-) fixed > The header and esp. the source could really do with more > documentation. It is not at all clear from the code what the > dma-streaming allocator does and how it differs from other > allocators. The other allocators are not documented and to understand them I read the code. All the memory allocators reflect a kind of DMA interface: SG for scatter/gather, contig for choerent and (now) streaming for streaming. So, I'm not sure to understand what do you think is the correct way to document a memory allocator; I can write one or two lines of comment to summarize each function. what do you think? -- Federico Vaga -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/4] videobuf2-dma-streaming: new videobuf2 memory allocator
On Thu 13 September 2012 15:52:47 Federico Vaga wrote: > Signed-off-by: Federico Vaga > --- > drivers/media/v4l2-core/Kconfig | 5 + > drivers/media/v4l2-core/Makefile | 1 + > drivers/media/v4l2-core/videobuf2-dma-streaming.c | 205 > ++ > include/media/videobuf2-dma-streaming.h | 24 +++ > 4 file modificati, 235 inserzioni(+) > create mode 100644 drivers/media/v4l2-core/videobuf2-dma-streaming.c > create mode 100644 include/media/videobuf2-dma-streaming.h > > diff --git a/drivers/media/v4l2-core/Kconfig b/drivers/media/v4l2-core/Kconfig > index 0c54e19..60548a7 100644 > --- a/drivers/media/v4l2-core/Kconfig > +++ b/drivers/media/v4l2-core/Kconfig > @@ -79,3 +79,8 @@ config VIDEOBUF2_DMA_SG > #depends on HAS_DMA > select VIDEOBUF2_CORE > select VIDEOBUF2_MEMOPS > + > +config VIDEOBUF2_DMA_STREAMING > + select VIDEOBUF2_CORE > + select VIDEOBUF2_MEMOPS > + tristate > diff --git a/drivers/media/v4l2-core/Makefile > b/drivers/media/v4l2-core/Makefile > index c2d61d4..0b2756f 100644 > --- a/drivers/media/v4l2-core/Makefile > +++ b/drivers/media/v4l2-core/Makefile > @@ -28,6 +28,7 @@ obj-$(CONFIG_VIDEOBUF2_MEMOPS) += videobuf2-memops.o > obj-$(CONFIG_VIDEOBUF2_VMALLOC) += videobuf2-vmalloc.o > obj-$(CONFIG_VIDEOBUF2_DMA_CONTIG) += videobuf2-dma-contig.o > obj-$(CONFIG_VIDEOBUF2_DMA_SG) += videobuf2-dma-sg.o > +obj-$(CONFIG_VIDEOBUF2_DMA_STREAMING) += videobuf2-dma-streaming.o > > ccflags-y += -I$(srctree)/drivers/media/dvb-core > ccflags-y += -I$(srctree)/drivers/media/dvb-frontends > diff --git a/drivers/media/v4l2-core/videobuf2-dma-streaming.c > b/drivers/media/v4l2-core/videobuf2-dma-streaming.c > new file mode 100644 > index 000..23475a6 > --- /dev/null > +++ b/drivers/media/v4l2-core/videobuf2-dma-streaming.c > @@ -0,0 +1,205 @@ > +/* > + * videobuf2-dma-streaming.c - DMA streaming memory allocator for videobuf2 > + * > + * Copyright (C) 2012 Federico Vaga > + * * > + * 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. > + */ > + > +#include > +#include > +#include > +#include > + > +#include > +#include > +#include > + > +struct vb2_streaming_conf { > + struct device *dev; > +}; > +struct vb2_streaming_buf { > + struct vb2_streaming_conf *conf; > + void*vaddr; > + > + dma_addr_t dma_handle; > + > + unsigned long size; > + struct vm_area_struct *vma; > + > + atomic_trefcount; > + struct vb2_vmarea_handler handler; > +}; > + > +static void vb2_dma_streaming_put(void *buf_priv) > +{ > + struct vb2_streaming_buf *buf = buf_priv; > + > + if (atomic_dec_and_test(>refcount)) { > + dma_unmap_single(buf->conf->dev, buf->dma_handle, buf->size, > + DMA_FROM_DEVICE); > + free_pages_exact(buf->vaddr, buf->size); > + kfree(buf); > + } > + > +} > + > +static void *vb2_dma_streaming_alloc(void *alloc_ctx, unsigned long size) > +{ > + struct vb2_streaming_conf *conf = alloc_ctx; > + struct vb2_streaming_buf *buf; > + int err; > + > + buf = kzalloc(sizeof *buf, GFP_KERNEL); > + if (!buf) > + return ERR_PTR(-ENOMEM); > + buf->vaddr = alloc_pages_exact(size, GFP_KERNEL | GFP_DMA); > + if (!buf->vaddr) { > + err = -ENOMEM; > + goto out; > + } > + buf->dma_handle = dma_map_single(conf->dev, buf->vaddr, size, > + DMA_FROM_DEVICE); > + err = dma_mapping_error(conf->dev, buf->dma_handle); > + if (err) { > + dev_err(conf->dev, "dma_map_single failed\n"); > + > + free_pages_exact(buf->vaddr, size); > + buf->vaddr = NULL; > + goto out_pages; > + } > + buf->conf = conf; > + buf->size = size; > + buf->handler.refcount = >refcount; > + buf->handler.put = vb2_dma_streaming_put; > + buf->handler.arg = buf; > + > + atomic_inc(>refcount); > + return buf; > + > +out_pages: > + free_pages_exact(buf->vaddr, buf->size); > +out: > + kfree(buf); > + return ERR_PTR(err); > +} > + > +static void *vb2_dma_streaming_cookie(void *buf_priv) > +{ > + struct vb2_streaming_buf *buf = buf_priv; > + > + return (void *)buf->dma_handle; > +} > + > +static void *vb2_dma_streaming_vaddr(void *buf_priv) > +{ > + struct vb2_streaming_buf *buf = buf_priv; > + > + if (!buf) > + return NULL; > + return buf->vaddr; > +} > + > +static unsigned int vb2_dma_streaming_num_users(void *buf_priv) > +{ > + struct vb2_streaming_buf *buf = buf_priv; > + > + return atomic_read(>refcount); > +} > + > +static int
RE: [PATCH 3/4] videobuf2-dma-streaming: new videobuf2 memory allocator
Hello, On Thursday, September 13, 2012 3:53 PM Federico Vaga wrote: > Signed-off-by: Federico Vaga A few words explaining why this memory handling module is required or beneficial will definitely improve the commit :) > --- > drivers/media/v4l2-core/Kconfig | 5 + > drivers/media/v4l2-core/Makefile | 1 + > drivers/media/v4l2-core/videobuf2-dma-streaming.c | 205 > ++ > include/media/videobuf2-dma-streaming.h | 24 +++ > 4 file modificati, 235 inserzioni(+) > create mode 100644 drivers/media/v4l2-core/videobuf2-dma-streaming.c > create mode 100644 include/media/videobuf2-dma-streaming.h > > diff --git a/drivers/media/v4l2-core/Kconfig b/drivers/media/v4l2-core/Kconfig > index 0c54e19..60548a7 100644 > --- a/drivers/media/v4l2-core/Kconfig > +++ b/drivers/media/v4l2-core/Kconfig > @@ -79,3 +79,8 @@ config VIDEOBUF2_DMA_SG > #depends on HAS_DMA > select VIDEOBUF2_CORE > select VIDEOBUF2_MEMOPS > + > +config VIDEOBUF2_DMA_STREAMING > + select VIDEOBUF2_CORE > + select VIDEOBUF2_MEMOPS > + tristate > diff --git a/drivers/media/v4l2-core/Makefile > b/drivers/media/v4l2-core/Makefile > index c2d61d4..0b2756f 100644 > --- a/drivers/media/v4l2-core/Makefile > +++ b/drivers/media/v4l2-core/Makefile > @@ -28,6 +28,7 @@ obj-$(CONFIG_VIDEOBUF2_MEMOPS) += videobuf2-memops.o > obj-$(CONFIG_VIDEOBUF2_VMALLOC) += videobuf2-vmalloc.o > obj-$(CONFIG_VIDEOBUF2_DMA_CONTIG) += videobuf2-dma-contig.o > obj-$(CONFIG_VIDEOBUF2_DMA_SG) += videobuf2-dma-sg.o > +obj-$(CONFIG_VIDEOBUF2_DMA_STREAMING) += videobuf2-dma-streaming.o > > ccflags-y += -I$(srctree)/drivers/media/dvb-core > ccflags-y += -I$(srctree)/drivers/media/dvb-frontends > diff --git a/drivers/media/v4l2-core/videobuf2-dma-streaming.c > b/drivers/media/v4l2- > core/videobuf2-dma-streaming.c > new file mode 100644 > index 000..23475a6 > --- /dev/null > +++ b/drivers/media/v4l2-core/videobuf2-dma-streaming.c > @@ -0,0 +1,205 @@ > +/* > + * videobuf2-dma-streaming.c - DMA streaming memory allocator for videobuf2 > + * > + * Copyright (C) 2012 Federico Vaga > + * * > + * 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. > + */ > + > +#include > +#include > +#include > +#include > + > +#include > +#include > +#include > + > +struct vb2_streaming_conf { > + struct device *dev; > +}; > +struct vb2_streaming_buf { > + struct vb2_streaming_conf *conf; > + void*vaddr; > + > + dma_addr_t dma_handle; > + > + unsigned long size; > + struct vm_area_struct *vma; > + > + atomic_trefcount; > + struct vb2_vmarea_handler handler; > +}; > + > +static void vb2_dma_streaming_put(void *buf_priv) > +{ > + struct vb2_streaming_buf *buf = buf_priv; > + > + if (atomic_dec_and_test(>refcount)) { > + dma_unmap_single(buf->conf->dev, buf->dma_handle, buf->size, > + DMA_FROM_DEVICE); > + free_pages_exact(buf->vaddr, buf->size); > + kfree(buf); > + } > + > +} > + > +static void *vb2_dma_streaming_alloc(void *alloc_ctx, unsigned long size) > +{ > + struct vb2_streaming_conf *conf = alloc_ctx; > + struct vb2_streaming_buf *buf; > + int err; > + > + buf = kzalloc(sizeof *buf, GFP_KERNEL); > + if (!buf) > + return ERR_PTR(-ENOMEM); > + buf->vaddr = alloc_pages_exact(size, GFP_KERNEL | GFP_DMA); > + if (!buf->vaddr) { > + err = -ENOMEM; > + goto out; > + } > + buf->dma_handle = dma_map_single(conf->dev, buf->vaddr, size, > + DMA_FROM_DEVICE); > + err = dma_mapping_error(conf->dev, buf->dma_handle); > + if (err) { > + dev_err(conf->dev, "dma_map_single failed\n"); > + > + free_pages_exact(buf->vaddr, size); > + buf->vaddr = NULL; > + goto out_pages; > + } > + buf->conf = conf; > + buf->size = size; > + buf->handler.refcount = >refcount; > + buf->handler.put = vb2_dma_streaming_put; > + buf->handler.arg = buf; > + > + atomic_inc(>refcount); > + return buf; > + > +out_pages: > + free_pages_exact(buf->vaddr, buf->size); > +out: > + kfree(buf); > + return ERR_PTR(err); > +} > + > +static void *vb2_dma_streaming_cookie(void *buf_priv) > +{ > + struct vb2_streaming_buf *buf = buf_priv; > + > + return (void *)buf->dma_handle; > +} Please change this function to: static void *vb2_dma_streaming_cookie(void *buf_priv) { struct vb2_streaming_buf *buf = buf_priv; return >dma_handle; } and add a following static inline to include/media/videobuf2-dma-streaming.h: static inline
[PATCH 3/4] videobuf2-dma-streaming: new videobuf2 memory allocator
Signed-off-by: Federico Vaga --- drivers/media/v4l2-core/Kconfig | 5 + drivers/media/v4l2-core/Makefile | 1 + drivers/media/v4l2-core/videobuf2-dma-streaming.c | 205 ++ include/media/videobuf2-dma-streaming.h | 24 +++ 4 file modificati, 235 inserzioni(+) create mode 100644 drivers/media/v4l2-core/videobuf2-dma-streaming.c create mode 100644 include/media/videobuf2-dma-streaming.h diff --git a/drivers/media/v4l2-core/Kconfig b/drivers/media/v4l2-core/Kconfig index 0c54e19..60548a7 100644 --- a/drivers/media/v4l2-core/Kconfig +++ b/drivers/media/v4l2-core/Kconfig @@ -79,3 +79,8 @@ config VIDEOBUF2_DMA_SG #depends on HAS_DMA select VIDEOBUF2_CORE select VIDEOBUF2_MEMOPS + +config VIDEOBUF2_DMA_STREAMING + select VIDEOBUF2_CORE + select VIDEOBUF2_MEMOPS + tristate diff --git a/drivers/media/v4l2-core/Makefile b/drivers/media/v4l2-core/Makefile index c2d61d4..0b2756f 100644 --- a/drivers/media/v4l2-core/Makefile +++ b/drivers/media/v4l2-core/Makefile @@ -28,6 +28,7 @@ obj-$(CONFIG_VIDEOBUF2_MEMOPS) += videobuf2-memops.o obj-$(CONFIG_VIDEOBUF2_VMALLOC) += videobuf2-vmalloc.o obj-$(CONFIG_VIDEOBUF2_DMA_CONTIG) += videobuf2-dma-contig.o obj-$(CONFIG_VIDEOBUF2_DMA_SG) += videobuf2-dma-sg.o +obj-$(CONFIG_VIDEOBUF2_DMA_STREAMING) += videobuf2-dma-streaming.o ccflags-y += -I$(srctree)/drivers/media/dvb-core ccflags-y += -I$(srctree)/drivers/media/dvb-frontends diff --git a/drivers/media/v4l2-core/videobuf2-dma-streaming.c b/drivers/media/v4l2-core/videobuf2-dma-streaming.c new file mode 100644 index 000..23475a6 --- /dev/null +++ b/drivers/media/v4l2-core/videobuf2-dma-streaming.c @@ -0,0 +1,205 @@ +/* + * videobuf2-dma-streaming.c - DMA streaming memory allocator for videobuf2 + * + * Copyright (C) 2012 Federico Vaga + * * + * 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. + */ + +#include +#include +#include +#include + +#include +#include +#include + +struct vb2_streaming_conf { + struct device *dev; +}; +struct vb2_streaming_buf { + struct vb2_streaming_conf *conf; + void*vaddr; + + dma_addr_t dma_handle; + + unsigned long size; + struct vm_area_struct *vma; + + atomic_trefcount; + struct vb2_vmarea_handler handler; +}; + +static void vb2_dma_streaming_put(void *buf_priv) +{ + struct vb2_streaming_buf *buf = buf_priv; + + if (atomic_dec_and_test(>refcount)) { + dma_unmap_single(buf->conf->dev, buf->dma_handle, buf->size, +DMA_FROM_DEVICE); + free_pages_exact(buf->vaddr, buf->size); + kfree(buf); + } + +} + +static void *vb2_dma_streaming_alloc(void *alloc_ctx, unsigned long size) +{ + struct vb2_streaming_conf *conf = alloc_ctx; + struct vb2_streaming_buf *buf; + int err; + + buf = kzalloc(sizeof *buf, GFP_KERNEL); + if (!buf) + return ERR_PTR(-ENOMEM); + buf->vaddr = alloc_pages_exact(size, GFP_KERNEL | GFP_DMA); + if (!buf->vaddr) { + err = -ENOMEM; + goto out; + } + buf->dma_handle = dma_map_single(conf->dev, buf->vaddr, size, +DMA_FROM_DEVICE); + err = dma_mapping_error(conf->dev, buf->dma_handle); + if (err) { + dev_err(conf->dev, "dma_map_single failed\n"); + + free_pages_exact(buf->vaddr, size); + buf->vaddr = NULL; + goto out_pages; + } + buf->conf = conf; + buf->size = size; + buf->handler.refcount = >refcount; + buf->handler.put = vb2_dma_streaming_put; + buf->handler.arg = buf; + + atomic_inc(>refcount); + return buf; + +out_pages: + free_pages_exact(buf->vaddr, buf->size); +out: + kfree(buf); + return ERR_PTR(err); +} + +static void *vb2_dma_streaming_cookie(void *buf_priv) +{ + struct vb2_streaming_buf *buf = buf_priv; + + return (void *)buf->dma_handle; +} + +static void *vb2_dma_streaming_vaddr(void *buf_priv) +{ + struct vb2_streaming_buf *buf = buf_priv; + + if (!buf) + return NULL; + return buf->vaddr; +} + +static unsigned int vb2_dma_streaming_num_users(void *buf_priv) +{ + struct vb2_streaming_buf *buf = buf_priv; + + return atomic_read(>refcount); +} + +static int vb2_dma_streaming_mmap(void *buf_priv, struct vm_area_struct *vma) +{ + struct vb2_streaming_buf *buf = buf_priv; + unsigned long pos, start = vma->vm_start; + unsigned long size; + struct page *page; + int err; + + /* Try to remap
[PATCH 3/4] videobuf2-dma-streaming: new videobuf2 memory allocator
Signed-off-by: Federico Vaga federico.v...@gmail.com --- drivers/media/v4l2-core/Kconfig | 5 + drivers/media/v4l2-core/Makefile | 1 + drivers/media/v4l2-core/videobuf2-dma-streaming.c | 205 ++ include/media/videobuf2-dma-streaming.h | 24 +++ 4 file modificati, 235 inserzioni(+) create mode 100644 drivers/media/v4l2-core/videobuf2-dma-streaming.c create mode 100644 include/media/videobuf2-dma-streaming.h diff --git a/drivers/media/v4l2-core/Kconfig b/drivers/media/v4l2-core/Kconfig index 0c54e19..60548a7 100644 --- a/drivers/media/v4l2-core/Kconfig +++ b/drivers/media/v4l2-core/Kconfig @@ -79,3 +79,8 @@ config VIDEOBUF2_DMA_SG #depends on HAS_DMA select VIDEOBUF2_CORE select VIDEOBUF2_MEMOPS + +config VIDEOBUF2_DMA_STREAMING + select VIDEOBUF2_CORE + select VIDEOBUF2_MEMOPS + tristate diff --git a/drivers/media/v4l2-core/Makefile b/drivers/media/v4l2-core/Makefile index c2d61d4..0b2756f 100644 --- a/drivers/media/v4l2-core/Makefile +++ b/drivers/media/v4l2-core/Makefile @@ -28,6 +28,7 @@ obj-$(CONFIG_VIDEOBUF2_MEMOPS) += videobuf2-memops.o obj-$(CONFIG_VIDEOBUF2_VMALLOC) += videobuf2-vmalloc.o obj-$(CONFIG_VIDEOBUF2_DMA_CONTIG) += videobuf2-dma-contig.o obj-$(CONFIG_VIDEOBUF2_DMA_SG) += videobuf2-dma-sg.o +obj-$(CONFIG_VIDEOBUF2_DMA_STREAMING) += videobuf2-dma-streaming.o ccflags-y += -I$(srctree)/drivers/media/dvb-core ccflags-y += -I$(srctree)/drivers/media/dvb-frontends diff --git a/drivers/media/v4l2-core/videobuf2-dma-streaming.c b/drivers/media/v4l2-core/videobuf2-dma-streaming.c new file mode 100644 index 000..23475a6 --- /dev/null +++ b/drivers/media/v4l2-core/videobuf2-dma-streaming.c @@ -0,0 +1,205 @@ +/* + * videobuf2-dma-streaming.c - DMA streaming memory allocator for videobuf2 + * + * Copyright (C) 2012 Federico Vaga federico.v...@gmail.com + * * + * 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. + */ + +#include linux/module.h +#include linux/slab.h +#include linux/pagemap.h +#include linux/dma-mapping.h + +#include media/videobuf2-core.h +#include media/videobuf2-dma-streaming.h +#include media/videobuf2-memops.h + +struct vb2_streaming_conf { + struct device *dev; +}; +struct vb2_streaming_buf { + struct vb2_streaming_conf *conf; + void*vaddr; + + dma_addr_t dma_handle; + + unsigned long size; + struct vm_area_struct *vma; + + atomic_trefcount; + struct vb2_vmarea_handler handler; +}; + +static void vb2_dma_streaming_put(void *buf_priv) +{ + struct vb2_streaming_buf *buf = buf_priv; + + if (atomic_dec_and_test(buf-refcount)) { + dma_unmap_single(buf-conf-dev, buf-dma_handle, buf-size, +DMA_FROM_DEVICE); + free_pages_exact(buf-vaddr, buf-size); + kfree(buf); + } + +} + +static void *vb2_dma_streaming_alloc(void *alloc_ctx, unsigned long size) +{ + struct vb2_streaming_conf *conf = alloc_ctx; + struct vb2_streaming_buf *buf; + int err; + + buf = kzalloc(sizeof *buf, GFP_KERNEL); + if (!buf) + return ERR_PTR(-ENOMEM); + buf-vaddr = alloc_pages_exact(size, GFP_KERNEL | GFP_DMA); + if (!buf-vaddr) { + err = -ENOMEM; + goto out; + } + buf-dma_handle = dma_map_single(conf-dev, buf-vaddr, size, +DMA_FROM_DEVICE); + err = dma_mapping_error(conf-dev, buf-dma_handle); + if (err) { + dev_err(conf-dev, dma_map_single failed\n); + + free_pages_exact(buf-vaddr, size); + buf-vaddr = NULL; + goto out_pages; + } + buf-conf = conf; + buf-size = size; + buf-handler.refcount = buf-refcount; + buf-handler.put = vb2_dma_streaming_put; + buf-handler.arg = buf; + + atomic_inc(buf-refcount); + return buf; + +out_pages: + free_pages_exact(buf-vaddr, buf-size); +out: + kfree(buf); + return ERR_PTR(err); +} + +static void *vb2_dma_streaming_cookie(void *buf_priv) +{ + struct vb2_streaming_buf *buf = buf_priv; + + return (void *)buf-dma_handle; +} + +static void *vb2_dma_streaming_vaddr(void *buf_priv) +{ + struct vb2_streaming_buf *buf = buf_priv; + + if (!buf) + return NULL; + return buf-vaddr; +} + +static unsigned int vb2_dma_streaming_num_users(void *buf_priv) +{ + struct vb2_streaming_buf *buf = buf_priv; + + return atomic_read(buf-refcount); +} + +static int vb2_dma_streaming_mmap(void *buf_priv, struct vm_area_struct *vma) +{ + struct vb2_streaming_buf
RE: [PATCH 3/4] videobuf2-dma-streaming: new videobuf2 memory allocator
Hello, On Thursday, September 13, 2012 3:53 PM Federico Vaga wrote: Signed-off-by: Federico Vaga federico.v...@gmail.com A few words explaining why this memory handling module is required or beneficial will definitely improve the commit :) --- drivers/media/v4l2-core/Kconfig | 5 + drivers/media/v4l2-core/Makefile | 1 + drivers/media/v4l2-core/videobuf2-dma-streaming.c | 205 ++ include/media/videobuf2-dma-streaming.h | 24 +++ 4 file modificati, 235 inserzioni(+) create mode 100644 drivers/media/v4l2-core/videobuf2-dma-streaming.c create mode 100644 include/media/videobuf2-dma-streaming.h diff --git a/drivers/media/v4l2-core/Kconfig b/drivers/media/v4l2-core/Kconfig index 0c54e19..60548a7 100644 --- a/drivers/media/v4l2-core/Kconfig +++ b/drivers/media/v4l2-core/Kconfig @@ -79,3 +79,8 @@ config VIDEOBUF2_DMA_SG #depends on HAS_DMA select VIDEOBUF2_CORE select VIDEOBUF2_MEMOPS + +config VIDEOBUF2_DMA_STREAMING + select VIDEOBUF2_CORE + select VIDEOBUF2_MEMOPS + tristate diff --git a/drivers/media/v4l2-core/Makefile b/drivers/media/v4l2-core/Makefile index c2d61d4..0b2756f 100644 --- a/drivers/media/v4l2-core/Makefile +++ b/drivers/media/v4l2-core/Makefile @@ -28,6 +28,7 @@ obj-$(CONFIG_VIDEOBUF2_MEMOPS) += videobuf2-memops.o obj-$(CONFIG_VIDEOBUF2_VMALLOC) += videobuf2-vmalloc.o obj-$(CONFIG_VIDEOBUF2_DMA_CONTIG) += videobuf2-dma-contig.o obj-$(CONFIG_VIDEOBUF2_DMA_SG) += videobuf2-dma-sg.o +obj-$(CONFIG_VIDEOBUF2_DMA_STREAMING) += videobuf2-dma-streaming.o ccflags-y += -I$(srctree)/drivers/media/dvb-core ccflags-y += -I$(srctree)/drivers/media/dvb-frontends diff --git a/drivers/media/v4l2-core/videobuf2-dma-streaming.c b/drivers/media/v4l2- core/videobuf2-dma-streaming.c new file mode 100644 index 000..23475a6 --- /dev/null +++ b/drivers/media/v4l2-core/videobuf2-dma-streaming.c @@ -0,0 +1,205 @@ +/* + * videobuf2-dma-streaming.c - DMA streaming memory allocator for videobuf2 + * + * Copyright (C) 2012 Federico Vaga federico.v...@gmail.com + * * + * 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. + */ + +#include linux/module.h +#include linux/slab.h +#include linux/pagemap.h +#include linux/dma-mapping.h + +#include media/videobuf2-core.h +#include media/videobuf2-dma-streaming.h +#include media/videobuf2-memops.h + +struct vb2_streaming_conf { + struct device *dev; +}; +struct vb2_streaming_buf { + struct vb2_streaming_conf *conf; + void*vaddr; + + dma_addr_t dma_handle; + + unsigned long size; + struct vm_area_struct *vma; + + atomic_trefcount; + struct vb2_vmarea_handler handler; +}; + +static void vb2_dma_streaming_put(void *buf_priv) +{ + struct vb2_streaming_buf *buf = buf_priv; + + if (atomic_dec_and_test(buf-refcount)) { + dma_unmap_single(buf-conf-dev, buf-dma_handle, buf-size, + DMA_FROM_DEVICE); + free_pages_exact(buf-vaddr, buf-size); + kfree(buf); + } + +} + +static void *vb2_dma_streaming_alloc(void *alloc_ctx, unsigned long size) +{ + struct vb2_streaming_conf *conf = alloc_ctx; + struct vb2_streaming_buf *buf; + int err; + + buf = kzalloc(sizeof *buf, GFP_KERNEL); + if (!buf) + return ERR_PTR(-ENOMEM); + buf-vaddr = alloc_pages_exact(size, GFP_KERNEL | GFP_DMA); + if (!buf-vaddr) { + err = -ENOMEM; + goto out; + } + buf-dma_handle = dma_map_single(conf-dev, buf-vaddr, size, + DMA_FROM_DEVICE); + err = dma_mapping_error(conf-dev, buf-dma_handle); + if (err) { + dev_err(conf-dev, dma_map_single failed\n); + + free_pages_exact(buf-vaddr, size); + buf-vaddr = NULL; + goto out_pages; + } + buf-conf = conf; + buf-size = size; + buf-handler.refcount = buf-refcount; + buf-handler.put = vb2_dma_streaming_put; + buf-handler.arg = buf; + + atomic_inc(buf-refcount); + return buf; + +out_pages: + free_pages_exact(buf-vaddr, buf-size); +out: + kfree(buf); + return ERR_PTR(err); +} + +static void *vb2_dma_streaming_cookie(void *buf_priv) +{ + struct vb2_streaming_buf *buf = buf_priv; + + return (void *)buf-dma_handle; +} Please change this function to: static void *vb2_dma_streaming_cookie(void *buf_priv) { struct vb2_streaming_buf *buf = buf_priv; return buf-dma_handle; } and add a following static inline to
Re: [PATCH 3/4] videobuf2-dma-streaming: new videobuf2 memory allocator
On Thu 13 September 2012 15:52:47 Federico Vaga wrote: Signed-off-by: Federico Vaga federico.v...@gmail.com --- drivers/media/v4l2-core/Kconfig | 5 + drivers/media/v4l2-core/Makefile | 1 + drivers/media/v4l2-core/videobuf2-dma-streaming.c | 205 ++ include/media/videobuf2-dma-streaming.h | 24 +++ 4 file modificati, 235 inserzioni(+) create mode 100644 drivers/media/v4l2-core/videobuf2-dma-streaming.c create mode 100644 include/media/videobuf2-dma-streaming.h diff --git a/drivers/media/v4l2-core/Kconfig b/drivers/media/v4l2-core/Kconfig index 0c54e19..60548a7 100644 --- a/drivers/media/v4l2-core/Kconfig +++ b/drivers/media/v4l2-core/Kconfig @@ -79,3 +79,8 @@ config VIDEOBUF2_DMA_SG #depends on HAS_DMA select VIDEOBUF2_CORE select VIDEOBUF2_MEMOPS + +config VIDEOBUF2_DMA_STREAMING + select VIDEOBUF2_CORE + select VIDEOBUF2_MEMOPS + tristate diff --git a/drivers/media/v4l2-core/Makefile b/drivers/media/v4l2-core/Makefile index c2d61d4..0b2756f 100644 --- a/drivers/media/v4l2-core/Makefile +++ b/drivers/media/v4l2-core/Makefile @@ -28,6 +28,7 @@ obj-$(CONFIG_VIDEOBUF2_MEMOPS) += videobuf2-memops.o obj-$(CONFIG_VIDEOBUF2_VMALLOC) += videobuf2-vmalloc.o obj-$(CONFIG_VIDEOBUF2_DMA_CONTIG) += videobuf2-dma-contig.o obj-$(CONFIG_VIDEOBUF2_DMA_SG) += videobuf2-dma-sg.o +obj-$(CONFIG_VIDEOBUF2_DMA_STREAMING) += videobuf2-dma-streaming.o ccflags-y += -I$(srctree)/drivers/media/dvb-core ccflags-y += -I$(srctree)/drivers/media/dvb-frontends diff --git a/drivers/media/v4l2-core/videobuf2-dma-streaming.c b/drivers/media/v4l2-core/videobuf2-dma-streaming.c new file mode 100644 index 000..23475a6 --- /dev/null +++ b/drivers/media/v4l2-core/videobuf2-dma-streaming.c @@ -0,0 +1,205 @@ +/* + * videobuf2-dma-streaming.c - DMA streaming memory allocator for videobuf2 + * + * Copyright (C) 2012 Federico Vaga federico.v...@gmail.com + * * + * 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. + */ + +#include linux/module.h +#include linux/slab.h +#include linux/pagemap.h +#include linux/dma-mapping.h + +#include media/videobuf2-core.h +#include media/videobuf2-dma-streaming.h +#include media/videobuf2-memops.h + +struct vb2_streaming_conf { + struct device *dev; +}; +struct vb2_streaming_buf { + struct vb2_streaming_conf *conf; + void*vaddr; + + dma_addr_t dma_handle; + + unsigned long size; + struct vm_area_struct *vma; + + atomic_trefcount; + struct vb2_vmarea_handler handler; +}; + +static void vb2_dma_streaming_put(void *buf_priv) +{ + struct vb2_streaming_buf *buf = buf_priv; + + if (atomic_dec_and_test(buf-refcount)) { + dma_unmap_single(buf-conf-dev, buf-dma_handle, buf-size, + DMA_FROM_DEVICE); + free_pages_exact(buf-vaddr, buf-size); + kfree(buf); + } + +} + +static void *vb2_dma_streaming_alloc(void *alloc_ctx, unsigned long size) +{ + struct vb2_streaming_conf *conf = alloc_ctx; + struct vb2_streaming_buf *buf; + int err; + + buf = kzalloc(sizeof *buf, GFP_KERNEL); + if (!buf) + return ERR_PTR(-ENOMEM); + buf-vaddr = alloc_pages_exact(size, GFP_KERNEL | GFP_DMA); + if (!buf-vaddr) { + err = -ENOMEM; + goto out; + } + buf-dma_handle = dma_map_single(conf-dev, buf-vaddr, size, + DMA_FROM_DEVICE); + err = dma_mapping_error(conf-dev, buf-dma_handle); + if (err) { + dev_err(conf-dev, dma_map_single failed\n); + + free_pages_exact(buf-vaddr, size); + buf-vaddr = NULL; + goto out_pages; + } + buf-conf = conf; + buf-size = size; + buf-handler.refcount = buf-refcount; + buf-handler.put = vb2_dma_streaming_put; + buf-handler.arg = buf; + + atomic_inc(buf-refcount); + return buf; + +out_pages: + free_pages_exact(buf-vaddr, buf-size); +out: + kfree(buf); + return ERR_PTR(err); +} + +static void *vb2_dma_streaming_cookie(void *buf_priv) +{ + struct vb2_streaming_buf *buf = buf_priv; + + return (void *)buf-dma_handle; +} + +static void *vb2_dma_streaming_vaddr(void *buf_priv) +{ + struct vb2_streaming_buf *buf = buf_priv; + + if (!buf) + return NULL; + return buf-vaddr; +} + +static unsigned int vb2_dma_streaming_num_users(void *buf_priv) +{ + struct vb2_streaming_buf *buf = buf_priv; + + return atomic_read(buf-refcount); +} + +static int
Re: [PATCH 3/4] videobuf2-dma-streaming: new videobuf2 memory allocator
typo: steaming - streaming :-) fixed The header and esp. the source could really do with more documentation. It is not at all clear from the code what the dma-streaming allocator does and how it differs from other allocators. The other allocators are not documented and to understand them I read the code. All the memory allocators reflect a kind of DMA interface: SG for scatter/gather, contig for choerent and (now) streaming for streaming. So, I'm not sure to understand what do you think is the correct way to document a memory allocator; I can write one or two lines of comment to summarize each function. what do you think? -- Federico Vaga -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/4] videobuf2-dma-streaming: new videobuf2 memory allocator
On Thursday, September 13, 2012 3:53 PM Federico Vaga wrote: Signed-off-by: Federico Vaga federico.v...@gmail.com A few words explaining why this memory handling module is required or beneficial will definitely improve the commit :) ok, I will write some lines +static void *vb2_dma_streaming_cookie(void *buf_priv) +{ + struct vb2_streaming_buf *buf = buf_priv; + + return (void *)buf-dma_handle; +} Please change this function to: static void *vb2_dma_streaming_cookie(void *buf_priv) { struct vb2_streaming_buf *buf = buf_priv; return buf-dma_handle; } and add a following static inline to include/media/videobuf2-dma-streaming.h: static inline dma_addr_t vb2_dma_streaming_plane_paddr(struct vb2_buffer *vb, unsigned int plane_no) { dma_addr_t *dma_addr = vb2_plane_cookie(vb, plane_no); return *dma_addr; } Do not use 'cookie' callback directly in the driver, the driver should use the above proxy. The buf-dma_handle workaround is required for some possible configurations with 64bit dma addresses, see commit 472af2b05bdefc. ACK. -- Federico Vaga -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/4] videobuf2-dma-streaming: new videobuf2 memory allocator
On Thu, 13 Sep 2012 17:46:32 +0200 Federico Vaga federico.v...@gmail.com wrote: A few words explaining why this memory handling module is required or beneficial will definitely improve the commit :) ok, I will write some lines In general, all of these patches need *much* better changelogs (i.e. they need changelogs). What are you doing, and *why* are you doing it? The future will want to know. I'm going to try to look at the actual code, but time isn't something I have a lot of, still... Thanks, jon -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/4] videobuf2-dma-streaming: new videobuf2 memory allocator
On Thu, 13 Sep 2012 17:42:33 +0200 Federico Vaga federico.v...@gmail.com wrote: The header and esp. the source could really do with more documentation. It is not at all clear from the code what the dma-streaming allocator does and how it differs from other allocators. The other allocators are not documented and to understand them I read the code. All the memory allocators reflect a kind of DMA interface: SG for scatter/gather, contig for choerent and (now) streaming for streaming. So, I'm not sure to understand what do you think is the correct way to document a memory allocator; I can write one or two lines of comment to summarize each function. what do you think? Well, there is some documentation here: https://lwn.net/Articles/447435/ This reminds me that I've always meant to turn it into something to put into Documentation/. I'll try to get to that soon. In general, the fact that a subsystem is insufficiently documented is not a good reason to add more poorly-documented code. We are, after all, trying to make the kernel better over time... Thanks, jon -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/4] videobuf2-dma-streaming: new videobuf2 memory allocator
Well, there is some documentation here: https://lwn.net/Articles/447435/ I know this, I learned from this page :) What I'm saying is that I don't know what to write inside the code to make it clearer than now. I think is clear, because if you know the videobuf2, you know what I'm doing in each vb2_mem_ops. I suppose that this is the reason why there are no comments inside the other memory allocator. Maybe I am wrong. -- Federico Vaga -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/4] videobuf2-dma-streaming: new videobuf2 memory allocator
In data giovedì 13 settembre 2012 11:45:31, Jonathan Corbet ha scritto: On Thu, 13 Sep 2012 17:46:32 +0200 Federico Vaga federico.v...@gmail.com wrote: A few words explaining why this memory handling module is required or beneficial will definitely improve the commit :) ok, I will write some lines In general, all of these patches need *much* better changelogs (i.e. they need changelogs). What are you doing, and *why* are you doing it? The future will want to know. I can improve the comment about the ADV7180: it is not clear why I'm removing that functions. (and the memory allocator commit is also in the todo list). The STA2X11_VIP commit, I think is clear, I convert it to use videobu2 and control framework. I can add some extra lines to explain why: because videobuf is obsolete I'm going to try to look at the actual code, but time isn't something I have a lot of, still... The actual code is the same of the previous one with yours (plural) suggestions from the RFC submission (few week ago). I did not write anything else. -- Federico Vaga -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/