Re: [PATCH 3/4] videobuf2-dma-streaming: new videobuf2 memory allocator

2012-09-14 Thread Ezequiel Garcia
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

2012-09-14 Thread Ezequiel Garcia
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

2012-09-13 Thread Federico Vaga
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

2012-09-13 Thread Federico Vaga
> 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

2012-09-13 Thread Jonathan Corbet
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

2012-09-13 Thread Jonathan Corbet
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

2012-09-13 Thread Federico Vaga
> 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

2012-09-13 Thread Federico Vaga
> 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

2012-09-13 Thread Hans Verkuil
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

2012-09-13 Thread Marek Szyprowski
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

2012-09-13 Thread Federico Vaga
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

2012-09-13 Thread Federico Vaga
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

2012-09-13 Thread Marek Szyprowski
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

2012-09-13 Thread Hans Verkuil
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

2012-09-13 Thread Federico Vaga
 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

2012-09-13 Thread Federico Vaga
 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

2012-09-13 Thread Jonathan Corbet
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

2012-09-13 Thread Jonathan Corbet
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

2012-09-13 Thread Federico Vaga
 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

2012-09-13 Thread Federico Vaga
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/