* [PATCH] media: vb2: dma-sg allocator: change scatterlist allocation method
@ 2011-08-10 8:23 Marek Szyprowski
2011-08-12 21:54 ` Laurent Pinchart
0 siblings, 1 reply; 9+ messages in thread
From: Marek Szyprowski @ 2011-08-10 8:23 UTC (permalink / raw)
To: linux-media
Cc: Marek Szyprowski, Kyungmin Park, Pawel Osciak, Andrzej Pietrasiewicz
From: Andrzej Pietrasiewicz <andrzej.p@samsung.com>
Scatter-gather lib provides a helper functions to allocate scatter list,
so there is no need to use vmalloc for it. sg_alloc_table() splits
allocation into page size chunks and links them together into a chain.
Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
CC: Pawel Osciak <pawel@osciak.com>
---
drivers/media/video/videobuf2-dma-sg.c | 54 +++++++++++++++++++------------
1 files changed, 33 insertions(+), 21 deletions(-)
diff --git a/drivers/media/video/videobuf2-dma-sg.c b/drivers/media/video/videobuf2-dma-sg.c
index 065f468..e1158f9 100644
--- a/drivers/media/video/videobuf2-dma-sg.c
+++ b/drivers/media/video/videobuf2-dma-sg.c
@@ -36,6 +36,8 @@ static void vb2_dma_sg_put(void *buf_priv);
static void *vb2_dma_sg_alloc(void *alloc_ctx, unsigned long size)
{
struct vb2_dma_sg_buf *buf;
+ struct sg_table sgt;
+ struct scatterlist *sl;
int i;
buf = kzalloc(sizeof *buf, GFP_KERNEL);
@@ -48,23 +50,21 @@ static void *vb2_dma_sg_alloc(void *alloc_ctx, unsigned long size)
buf->sg_desc.size = size;
buf->sg_desc.num_pages = (size + PAGE_SIZE - 1) >> PAGE_SHIFT;
- buf->sg_desc.sglist = vzalloc(buf->sg_desc.num_pages *
- sizeof(*buf->sg_desc.sglist));
- if (!buf->sg_desc.sglist)
+ if (sg_alloc_table(&sgt, buf->sg_desc.num_pages, GFP_KERNEL))
goto fail_sglist_alloc;
- sg_init_table(buf->sg_desc.sglist, buf->sg_desc.num_pages);
+ buf->sg_desc.sglist = sgt.sgl;
buf->pages = kzalloc(buf->sg_desc.num_pages * sizeof(struct page *),
GFP_KERNEL);
if (!buf->pages)
goto fail_pages_array_alloc;
- for (i = 0; i < buf->sg_desc.num_pages; ++i) {
- buf->pages[i] = alloc_page(GFP_KERNEL | __GFP_ZERO | __GFP_NOWARN);
+ for_each_sg(buf->sg_desc.sglist, sl, buf->sg_desc.num_pages, i) {
+ buf->pages[i] = alloc_page(GFP_KERNEL | __GFP_ZERO |
+ __GFP_NOWARN);
if (NULL == buf->pages[i])
goto fail_pages_alloc;
- sg_set_page(&buf->sg_desc.sglist[i],
- buf->pages[i], PAGE_SIZE, 0);
+ sg_set_page(sl, buf->pages[i], PAGE_SIZE, 0);
}
buf->handler.refcount = &buf->refcount;
@@ -89,7 +89,7 @@ fail_pages_alloc:
kfree(buf->pages);
fail_pages_array_alloc:
- vfree(buf->sg_desc.sglist);
+ sg_free_table(&sgt);
fail_sglist_alloc:
kfree(buf);
@@ -99,6 +99,7 @@ fail_sglist_alloc:
static void vb2_dma_sg_put(void *buf_priv)
{
struct vb2_dma_sg_buf *buf = buf_priv;
+ struct sg_table sgt;
int i = buf->sg_desc.num_pages;
if (atomic_dec_and_test(&buf->refcount)) {
@@ -106,7 +107,9 @@ static void vb2_dma_sg_put(void *buf_priv)
buf->sg_desc.num_pages);
if (buf->vaddr)
vm_unmap_ram(buf->vaddr, buf->sg_desc.num_pages);
- vfree(buf->sg_desc.sglist);
+ sgt.sgl = buf->sg_desc.sglist;
+ sgt.orig_nents = sgt.nents = i;
+ sg_free_table(&sgt);
while (--i >= 0)
__free_page(buf->pages[i]);
kfree(buf->pages);
@@ -118,6 +121,8 @@ static void *vb2_dma_sg_get_userptr(void *alloc_ctx, unsigned long vaddr,
unsigned long size, int write)
{
struct vb2_dma_sg_buf *buf;
+ struct sg_table sgt;
+ struct scatterlist *sl;
unsigned long first, last;
int num_pages_from_user, i;
@@ -134,12 +139,9 @@ static void *vb2_dma_sg_get_userptr(void *alloc_ctx, unsigned long vaddr,
last = ((vaddr + size - 1) & PAGE_MASK) >> PAGE_SHIFT;
buf->sg_desc.num_pages = last - first + 1;
- buf->sg_desc.sglist = vzalloc(
- buf->sg_desc.num_pages * sizeof(*buf->sg_desc.sglist));
- if (!buf->sg_desc.sglist)
+ if (sg_alloc_table(&sgt, buf->sg_desc.num_pages, GFP_KERNEL))
goto userptr_fail_sglist_alloc;
-
- sg_init_table(buf->sg_desc.sglist, buf->sg_desc.num_pages);
+ buf->sg_desc.sglist = sgt.sgl;
buf->pages = kzalloc(buf->sg_desc.num_pages * sizeof(struct page *),
GFP_KERNEL);
@@ -158,12 +160,12 @@ static void *vb2_dma_sg_get_userptr(void *alloc_ctx, unsigned long vaddr,
if (num_pages_from_user != buf->sg_desc.num_pages)
goto userptr_fail_get_user_pages;
- sg_set_page(&buf->sg_desc.sglist[0], buf->pages[0],
- PAGE_SIZE - buf->offset, buf->offset);
+ sl = buf->sg_desc.sglist;
+ sg_set_page(sl, buf->pages[0], PAGE_SIZE - buf->offset, buf->offset);
size -= PAGE_SIZE - buf->offset;
for (i = 1; i < buf->sg_desc.num_pages; ++i) {
- sg_set_page(&buf->sg_desc.sglist[i], buf->pages[i],
- min_t(size_t, PAGE_SIZE, size), 0);
+ sl = sg_next(sl);
+ sg_set_page(sl, buf->pages[i], min_t(size_t, PAGE_SIZE, size), 0);
size -= min_t(size_t, PAGE_SIZE, size);
}
return buf;
@@ -176,7 +178,7 @@ userptr_fail_get_user_pages:
kfree(buf->pages);
userptr_fail_pages_array_alloc:
- vfree(buf->sg_desc.sglist);
+ sg_free_table(&sgt);
userptr_fail_sglist_alloc:
kfree(buf);
@@ -190,6 +192,8 @@ userptr_fail_sglist_alloc:
static void vb2_dma_sg_put_userptr(void *buf_priv)
{
struct vb2_dma_sg_buf *buf = buf_priv;
+ struct sg_table sgt;
+
int i = buf->sg_desc.num_pages;
printk(KERN_DEBUG "%s: Releasing userspace buffer of %d pages\n",
@@ -201,7 +205,9 @@ static void vb2_dma_sg_put_userptr(void *buf_priv)
set_page_dirty_lock(buf->pages[i]);
put_page(buf->pages[i]);
}
- vfree(buf->sg_desc.sglist);
+ sgt.sgl = buf->sg_desc.sglist;
+ sgt.orig_nents = sgt.nents = buf->sg_desc.num_pages;
+ sg_free_table(&sgt);
kfree(buf->pages);
kfree(buf);
}
@@ -218,6 +224,12 @@ static void *vb2_dma_sg_vaddr(void *buf_priv)
-1,
PAGE_KERNEL);
+ if (!buf->vaddr) {
+ printk(KERN_ERR "Cannot map buffer memory "
+ "into kernel address space\n");
+ return NULL;
+ }
+
/* add offset in case userptr is not page-aligned */
return buf->vaddr + buf->offset;
}
--
1.7.1.569.g6f426
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] media: vb2: dma-sg allocator: change scatterlist allocation method
2011-08-10 8:23 [PATCH] media: vb2: dma-sg allocator: change scatterlist allocation method Marek Szyprowski
@ 2011-08-12 21:54 ` Laurent Pinchart
2011-08-16 5:35 ` Marek Szyprowski
0 siblings, 1 reply; 9+ messages in thread
From: Laurent Pinchart @ 2011-08-12 21:54 UTC (permalink / raw)
To: Marek Szyprowski
Cc: linux-media, Kyungmin Park, Pawel Osciak, Andrzej Pietrasiewicz
Hi,
On Wednesday 10 August 2011 10:23:37 Marek Szyprowski wrote:
> From: Andrzej Pietrasiewicz <andrzej.p@samsung.com>
>
> Scatter-gather lib provides a helper functions to allocate scatter list,
> so there is no need to use vmalloc for it. sg_alloc_table() splits
> allocation into page size chunks and links them together into a chain.
Last time I check ARM platforms didn't support SG list chaining. Has that been
fixed ?
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> CC: Pawel Osciak <pawel@osciak.com>
> ---
> drivers/media/video/videobuf2-dma-sg.c | 54
> +++++++++++++++++++------------ 1 files changed, 33 insertions(+), 21
> deletions(-)
>
> diff --git a/drivers/media/video/videobuf2-dma-sg.c
> b/drivers/media/video/videobuf2-dma-sg.c index 065f468..e1158f9 100644
> --- a/drivers/media/video/videobuf2-dma-sg.c
> +++ b/drivers/media/video/videobuf2-dma-sg.c
> @@ -36,6 +36,8 @@ static void vb2_dma_sg_put(void *buf_priv);
> static void *vb2_dma_sg_alloc(void *alloc_ctx, unsigned long size)
> {
> struct vb2_dma_sg_buf *buf;
> + struct sg_table sgt;
> + struct scatterlist *sl;
> int i;
>
> buf = kzalloc(sizeof *buf, GFP_KERNEL);
> @@ -48,23 +50,21 @@ static void *vb2_dma_sg_alloc(void *alloc_ctx, unsigned
> long size) buf->sg_desc.size = size;
> buf->sg_desc.num_pages = (size + PAGE_SIZE - 1) >> PAGE_SHIFT;
>
> - buf->sg_desc.sglist = vzalloc(buf->sg_desc.num_pages *
> - sizeof(*buf->sg_desc.sglist));
> - if (!buf->sg_desc.sglist)
> + if (sg_alloc_table(&sgt, buf->sg_desc.num_pages, GFP_KERNEL))
> goto fail_sglist_alloc;
> - sg_init_table(buf->sg_desc.sglist, buf->sg_desc.num_pages);
> + buf->sg_desc.sglist = sgt.sgl;
>
> buf->pages = kzalloc(buf->sg_desc.num_pages * sizeof(struct page *),
> GFP_KERNEL);
> if (!buf->pages)
> goto fail_pages_array_alloc;
>
> - for (i = 0; i < buf->sg_desc.num_pages; ++i) {
> - buf->pages[i] = alloc_page(GFP_KERNEL | __GFP_ZERO | __GFP_NOWARN);
> + for_each_sg(buf->sg_desc.sglist, sl, buf->sg_desc.num_pages, i) {
> + buf->pages[i] = alloc_page(GFP_KERNEL | __GFP_ZERO |
> + __GFP_NOWARN);
> if (NULL == buf->pages[i])
> goto fail_pages_alloc;
> - sg_set_page(&buf->sg_desc.sglist[i],
> - buf->pages[i], PAGE_SIZE, 0);
> + sg_set_page(sl, buf->pages[i], PAGE_SIZE, 0);
> }
>
> buf->handler.refcount = &buf->refcount;
> @@ -89,7 +89,7 @@ fail_pages_alloc:
> kfree(buf->pages);
>
> fail_pages_array_alloc:
> - vfree(buf->sg_desc.sglist);
> + sg_free_table(&sgt);
>
> fail_sglist_alloc:
> kfree(buf);
> @@ -99,6 +99,7 @@ fail_sglist_alloc:
> static void vb2_dma_sg_put(void *buf_priv)
> {
> struct vb2_dma_sg_buf *buf = buf_priv;
> + struct sg_table sgt;
> int i = buf->sg_desc.num_pages;
>
> if (atomic_dec_and_test(&buf->refcount)) {
> @@ -106,7 +107,9 @@ static void vb2_dma_sg_put(void *buf_priv)
> buf->sg_desc.num_pages);
> if (buf->vaddr)
> vm_unmap_ram(buf->vaddr, buf->sg_desc.num_pages);
> - vfree(buf->sg_desc.sglist);
> + sgt.sgl = buf->sg_desc.sglist;
> + sgt.orig_nents = sgt.nents = i;
> + sg_free_table(&sgt);
> while (--i >= 0)
> __free_page(buf->pages[i]);
> kfree(buf->pages);
> @@ -118,6 +121,8 @@ static void *vb2_dma_sg_get_userptr(void *alloc_ctx,
> unsigned long vaddr, unsigned long size, int write)
> {
> struct vb2_dma_sg_buf *buf;
> + struct sg_table sgt;
> + struct scatterlist *sl;
> unsigned long first, last;
> int num_pages_from_user, i;
>
> @@ -134,12 +139,9 @@ static void *vb2_dma_sg_get_userptr(void *alloc_ctx,
> unsigned long vaddr, last = ((vaddr + size - 1) & PAGE_MASK) >>
> PAGE_SHIFT;
> buf->sg_desc.num_pages = last - first + 1;
>
> - buf->sg_desc.sglist = vzalloc(
> - buf->sg_desc.num_pages * sizeof(*buf->sg_desc.sglist));
> - if (!buf->sg_desc.sglist)
> + if (sg_alloc_table(&sgt, buf->sg_desc.num_pages, GFP_KERNEL))
> goto userptr_fail_sglist_alloc;
> -
> - sg_init_table(buf->sg_desc.sglist, buf->sg_desc.num_pages);
> + buf->sg_desc.sglist = sgt.sgl;
>
> buf->pages = kzalloc(buf->sg_desc.num_pages * sizeof(struct page *),
> GFP_KERNEL);
> @@ -158,12 +160,12 @@ static void *vb2_dma_sg_get_userptr(void *alloc_ctx,
> unsigned long vaddr, if (num_pages_from_user != buf->sg_desc.num_pages)
> goto userptr_fail_get_user_pages;
>
> - sg_set_page(&buf->sg_desc.sglist[0], buf->pages[0],
> - PAGE_SIZE - buf->offset, buf->offset);
> + sl = buf->sg_desc.sglist;
> + sg_set_page(sl, buf->pages[0], PAGE_SIZE - buf->offset, buf->offset);
> size -= PAGE_SIZE - buf->offset;
> for (i = 1; i < buf->sg_desc.num_pages; ++i) {
> - sg_set_page(&buf->sg_desc.sglist[i], buf->pages[i],
> - min_t(size_t, PAGE_SIZE, size), 0);
> + sl = sg_next(sl);
> + sg_set_page(sl, buf->pages[i], min_t(size_t, PAGE_SIZE, size), 0);
> size -= min_t(size_t, PAGE_SIZE, size);
> }
> return buf;
> @@ -176,7 +178,7 @@ userptr_fail_get_user_pages:
> kfree(buf->pages);
>
> userptr_fail_pages_array_alloc:
> - vfree(buf->sg_desc.sglist);
> + sg_free_table(&sgt);
>
> userptr_fail_sglist_alloc:
> kfree(buf);
> @@ -190,6 +192,8 @@ userptr_fail_sglist_alloc:
> static void vb2_dma_sg_put_userptr(void *buf_priv)
> {
> struct vb2_dma_sg_buf *buf = buf_priv;
> + struct sg_table sgt;
> +
> int i = buf->sg_desc.num_pages;
>
> printk(KERN_DEBUG "%s: Releasing userspace buffer of %d pages\n",
> @@ -201,7 +205,9 @@ static void vb2_dma_sg_put_userptr(void *buf_priv)
> set_page_dirty_lock(buf->pages[i]);
> put_page(buf->pages[i]);
> }
> - vfree(buf->sg_desc.sglist);
> + sgt.sgl = buf->sg_desc.sglist;
> + sgt.orig_nents = sgt.nents = buf->sg_desc.num_pages;
> + sg_free_table(&sgt);
> kfree(buf->pages);
> kfree(buf);
> }
> @@ -218,6 +224,12 @@ static void *vb2_dma_sg_vaddr(void *buf_priv)
> -1,
> PAGE_KERNEL);
>
> + if (!buf->vaddr) {
> + printk(KERN_ERR "Cannot map buffer memory "
> + "into kernel address space\n");
> + return NULL;
> + }
> +
> /* add offset in case userptr is not page-aligned */
> return buf->vaddr + buf->offset;
> }
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH] media: vb2: dma-sg allocator: change scatterlist allocation method
2011-08-12 21:54 ` Laurent Pinchart
@ 2011-08-16 5:35 ` Marek Szyprowski
2011-08-16 8:41 ` Laurent Pinchart
0 siblings, 1 reply; 9+ messages in thread
From: Marek Szyprowski @ 2011-08-16 5:35 UTC (permalink / raw)
To: 'Laurent Pinchart'
Cc: linux-media, 'Kyungmin Park', 'Pawel Osciak',
Andrzej Pietrasiewicz
Hello,
On Friday, August 12, 2011 11:55 PM Laurent Pinchart wrote:
> On Wednesday 10 August 2011 10:23:37 Marek Szyprowski wrote:
> > From: Andrzej Pietrasiewicz <andrzej.p@samsung.com>
> >
> > Scatter-gather lib provides a helper functions to allocate scatter list,
> > so there is no need to use vmalloc for it. sg_alloc_table() splits
> > allocation into page size chunks and links them together into a chain.
>
> Last time I check ARM platforms didn't support SG list chaining. Has that been
> fixed ?
DMA-mapping code for ARM platform use for_each_sg() macro which has no problems
with chained SG lists.
(snipped)
Best regards
--
Marek Szyprowski
Samsung Poland R&D Center
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] media: vb2: dma-sg allocator: change scatterlist allocation method
2011-08-16 5:35 ` Marek Szyprowski
@ 2011-08-16 8:41 ` Laurent Pinchart
2011-08-16 10:34 ` Marek Szyprowski
0 siblings, 1 reply; 9+ messages in thread
From: Laurent Pinchart @ 2011-08-16 8:41 UTC (permalink / raw)
To: Marek Szyprowski
Cc: linux-media, 'Kyungmin Park', 'Pawel Osciak',
Andrzej Pietrasiewicz
Hi Marek,
On Tuesday 16 August 2011 07:35:05 Marek Szyprowski wrote:
> On Friday, August 12, 2011 11:55 PM Laurent Pinchart wrote:
> > On Wednesday 10 August 2011 10:23:37 Marek Szyprowski wrote:
> > > From: Andrzej Pietrasiewicz <andrzej.p@samsung.com>
> > >
> > > Scatter-gather lib provides a helper functions to allocate scatter
> > > list, so there is no need to use vmalloc for it. sg_alloc_table()
> > > splits allocation into page size chunks and links them together into a
> > > chain.
> >
> > Last time I check ARM platforms didn't support SG list chaining. Has that
> > been fixed ?
>
> DMA-mapping code for ARM platform use for_each_sg() macro which has no
> problems with chained SG lists.
for_each_sg() is fine, but sg_alloc_table() doesn't seem to be.
__sg_alloc_table(), called from sg_alloc_table(), starts with
#ifndef ARCH_HAS_SG_CHAIN
BUG_ON(nents > max_ents);
#endif
It also calls sg_chain() internally, which starts with
#ifndef ARCH_HAS_SG_CHAIN
BUG();
#endif
ARCH_HAS_SG_CHAIN is defined on ARM if CONFIG_ARM_HAS_SG_CHAIN is set. That's
a boolean Kconfig option that is currently never set.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH] media: vb2: dma-sg allocator: change scatterlist allocation method
2011-08-16 8:41 ` Laurent Pinchart
@ 2011-08-16 10:34 ` Marek Szyprowski
2011-08-16 11:01 ` Laurent Pinchart
2011-08-17 0:03 ` Jonathan Corbet
0 siblings, 2 replies; 9+ messages in thread
From: Marek Szyprowski @ 2011-08-16 10:34 UTC (permalink / raw)
To: 'Laurent Pinchart'
Cc: linux-media, 'Kyungmin Park', 'Pawel Osciak',
Andrzej Pietrasiewicz
Hello,
On Tuesday, August 16, 2011 10:42 AM Laurent Pinchart wrote:
> On Tuesday 16 August 2011 07:35:05 Marek Szyprowski wrote:
> > On Friday, August 12, 2011 11:55 PM Laurent Pinchart wrote:
> > > On Wednesday 10 August 2011 10:23:37 Marek Szyprowski wrote:
> > > > From: Andrzej Pietrasiewicz <andrzej.p@samsung.com>
> > > >
> > > > Scatter-gather lib provides a helper functions to allocate scatter
> > > > list, so there is no need to use vmalloc for it. sg_alloc_table()
> > > > splits allocation into page size chunks and links them together into a
> > > > chain.
> > >
> > > Last time I check ARM platforms didn't support SG list chaining. Has that
> > > been fixed ?
> >
> > DMA-mapping code for ARM platform use for_each_sg() macro which has no
> > problems with chained SG lists.
>
> for_each_sg() is fine, but sg_alloc_table() doesn't seem to be.
> __sg_alloc_table(), called from sg_alloc_table(), starts with
>
> #ifndef ARCH_HAS_SG_CHAIN
> BUG_ON(nents > max_ents);
> #endif
>
> It also calls sg_chain() internally, which starts with
>
> #ifndef ARCH_HAS_SG_CHAIN
> BUG();
> #endif
>
> ARCH_HAS_SG_CHAIN is defined on ARM if CONFIG_ARM_HAS_SG_CHAIN is set. That's
> a boolean Kconfig option that is currently never set.
Right, I wasn't aware of that, but it still doesn't look like an issue. The only
client of dma-sg allocator is marvell-ccic, which is used on x86 systems. If one
needs dma-sg allocator on ARM, he should follow the suggestion from the
74facffeca3795ffb5cf8898f5859fbb822e4c5d commit message.
Best regards
--
Marek Szyprowski
Samsung Poland R&D Center
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] media: vb2: dma-sg allocator: change scatterlist allocation method
2011-08-16 10:34 ` Marek Szyprowski
@ 2011-08-16 11:01 ` Laurent Pinchart
2011-08-16 11:48 ` Marek Szyprowski
2011-08-17 0:03 ` Jonathan Corbet
1 sibling, 1 reply; 9+ messages in thread
From: Laurent Pinchart @ 2011-08-16 11:01 UTC (permalink / raw)
To: Marek Szyprowski
Cc: linux-media, 'Kyungmin Park', 'Pawel Osciak',
Andrzej Pietrasiewicz
Hi Marek,
On Tuesday 16 August 2011 12:34:56 Marek Szyprowski wrote:
> On Tuesday, August 16, 2011 10:42 AM Laurent Pinchart wrote:
> > On Tuesday 16 August 2011 07:35:05 Marek Szyprowski wrote:
> > > On Friday, August 12, 2011 11:55 PM Laurent Pinchart wrote:
> > > > On Wednesday 10 August 2011 10:23:37 Marek Szyprowski wrote:
> > > > > From: Andrzej Pietrasiewicz <andrzej.p@samsung.com>
> > > > >
> > > > > Scatter-gather lib provides a helper functions to allocate scatter
> > > > > list, so there is no need to use vmalloc for it. sg_alloc_table()
> > > > > splits allocation into page size chunks and links them together
> > > > > into a chain.
> > > >
> > > > Last time I check ARM platforms didn't support SG list chaining. Has
> > > > that been fixed ?
> > >
> > > DMA-mapping code for ARM platform use for_each_sg() macro which has no
> > > problems with chained SG lists.
> >
> > for_each_sg() is fine, but sg_alloc_table() doesn't seem to be.
> > __sg_alloc_table(), called from sg_alloc_table(), starts with
> >
> > #ifndef ARCH_HAS_SG_CHAIN
> >
> > BUG_ON(nents > max_ents);
> >
> > #endif
> >
> > It also calls sg_chain() internally, which starts with
> >
> > #ifndef ARCH_HAS_SG_CHAIN
> >
> > BUG();
> >
> > #endif
> >
> > ARCH_HAS_SG_CHAIN is defined on ARM if CONFIG_ARM_HAS_SG_CHAIN is set.
> > That's a boolean Kconfig option that is currently never set.
>
> Right, I wasn't aware of that, but it still doesn't look like an issue. The
> only client of dma-sg allocator is marvell-ccic, which is used on x86
> systems. If one needs dma-sg allocator on ARM, he should follow the
> suggestion from the 74facffeca3795ffb5cf8898f5859fbb822e4c5d commit message.
Won't the dma-sg allocator be the right one for systems with an IOMMU ? If so
we'll soon run into this issue. I'd like to port the OMAP3 ISP driver to
videobuf2.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH] media: vb2: dma-sg allocator: change scatterlist allocation method
2011-08-16 11:01 ` Laurent Pinchart
@ 2011-08-16 11:48 ` Marek Szyprowski
0 siblings, 0 replies; 9+ messages in thread
From: Marek Szyprowski @ 2011-08-16 11:48 UTC (permalink / raw)
To: 'Laurent Pinchart'
Cc: linux-media, 'Kyungmin Park', 'Pawel Osciak',
Andrzej Pietrasiewicz
Hello,
On Tuesday, August 16, 2011 1:02 PM Laurent Pinchart wrote:
> On Tuesday 16 August 2011 12:34:56 Marek Szyprowski wrote:
> > On Tuesday, August 16, 2011 10:42 AM Laurent Pinchart wrote:
> > > On Tuesday 16 August 2011 07:35:05 Marek Szyprowski wrote:
> > > > On Friday, August 12, 2011 11:55 PM Laurent Pinchart wrote:
> > > > > On Wednesday 10 August 2011 10:23:37 Marek Szyprowski wrote:
> > > > > > From: Andrzej Pietrasiewicz <andrzej.p@samsung.com>
> > > > > >
> > > > > > Scatter-gather lib provides a helper functions to allocate scatter
> > > > > > list, so there is no need to use vmalloc for it. sg_alloc_table()
> > > > > > splits allocation into page size chunks and links them together
> > > > > > into a chain.
> > > > >
> > > > > Last time I check ARM platforms didn't support SG list chaining. Has
> > > > > that been fixed ?
> > > >
> > > > DMA-mapping code for ARM platform use for_each_sg() macro which has no
> > > > problems with chained SG lists.
> > >
> > > for_each_sg() is fine, but sg_alloc_table() doesn't seem to be.
> > > __sg_alloc_table(), called from sg_alloc_table(), starts with
> > >
> > > #ifndef ARCH_HAS_SG_CHAIN
> > >
> > > BUG_ON(nents > max_ents);
> > >
> > > #endif
> > >
> > > It also calls sg_chain() internally, which starts with
> > >
> > > #ifndef ARCH_HAS_SG_CHAIN
> > >
> > > BUG();
> > >
> > > #endif
> > >
> > > ARCH_HAS_SG_CHAIN is defined on ARM if CONFIG_ARM_HAS_SG_CHAIN is set.
> > > That's a boolean Kconfig option that is currently never set.
> >
> > Right, I wasn't aware of that, but it still doesn't look like an issue. The
> > only client of dma-sg allocator is marvell-ccic, which is used on x86
> > systems. If one needs dma-sg allocator on ARM, he should follow the
> > suggestion from the 74facffeca3795ffb5cf8898f5859fbb822e4c5d commit message.
>
> Won't the dma-sg allocator be the right one for systems with an IOMMU ? If so
> we'll soon run into this issue. I'd like to port the OMAP3 ISP driver to
> videobuf2.
Nope. The correct place for IOMMU is dma-contig allocator. In theory DMA-mapping
functions SHOULD hide the presence of the IOMMU from the driver. The driver
would
only need to get the 'dma_address' of the buffer and write it to the respective
multimedia device register.
The proof-of-concept of such solution for kernel allocated buffer has been
presented in my first ARM dma-mapping patches:
http://git.infradead.org/users/kmpark/linux-2.6-samsung/shortlog/refs/heads/dma-
mapping
User pointer buffers will need a bit more work to get them working correctly for
both iommu and non-iommu cases. For IOMMU dma_map_sg() will just fill the
scatter
list with just one dma_addr entry and it will contain the address of the buffer
in driver's io address space. Non-IOMMU needs some more tweaking (also in the
DMA-mapping)
IMHO the best way to porting OMAP3 ISP to videobuf2 is to first create a custom
memory allocator that working with OMAP3 IOMMU directly and then
adapt/merge/port
it to DMA-mapping based approach.
You can also refer to our first approaches with videobuf2-dma-iommu allocator:
http://lwn.net/Articles/439175/
Especially the discussion in that thread is really important for understanding
how the IOMMU should be integrated in the Linux kernel.
Best regards
--
Marek Szyprowski
Samsung Poland R&D Center
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] media: vb2: dma-sg allocator: change scatterlist allocation method
2011-08-16 10:34 ` Marek Szyprowski
2011-08-16 11:01 ` Laurent Pinchart
@ 2011-08-17 0:03 ` Jonathan Corbet
2011-08-17 8:51 ` Marek Szyprowski
1 sibling, 1 reply; 9+ messages in thread
From: Jonathan Corbet @ 2011-08-17 0:03 UTC (permalink / raw)
To: Marek Szyprowski
Cc: 'Laurent Pinchart', linux-media, 'Kyungmin Park',
'Pawel Osciak',
Andrzej Pietrasiewicz
On Tue, 16 Aug 2011 12:34:56 +0200
Marek Szyprowski <m.szyprowski@samsung.com> wrote:
> Right, I wasn't aware of that, but it still doesn't look like an issue. The only
>
> client of dma-sg allocator is marvell-ccic, which is used on x86 systems. If one
> needs dma-sg allocator on ARM, he should follow the suggestion from the
> 74facffeca3795ffb5cf8898f5859fbb822e4c5d commit message.
Um...that driver runs on ARM, actually - the controller is part of the
ARMADA 610 SoC...
For the OLPC 1.75 there will never be a scatterlist longer than one
page, I don't think. That controller can do HD, though, so longer
lists are possible in the future.
jon
^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH] media: vb2: dma-sg allocator: change scatterlist allocation method
2011-08-17 0:03 ` Jonathan Corbet
@ 2011-08-17 8:51 ` Marek Szyprowski
0 siblings, 0 replies; 9+ messages in thread
From: Marek Szyprowski @ 2011-08-17 8:51 UTC (permalink / raw)
To: 'Jonathan Corbet'
Cc: 'Laurent Pinchart', linux-media, 'Kyungmin Park',
'Pawel Osciak',
Andrzej Pietrasiewicz
Hello,
On Wednesday, August 17, 2011 2:03 AM Jonathan Corbet wrote:
> On Tue, 16 Aug 2011 12:34:56 +0200
> Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>
> > Right, I wasn't aware of that, but it still doesn't look like an issue. The
only
> >
> > client of dma-sg allocator is marvell-ccic, which is used on x86 systems. If
one
> > needs dma-sg allocator on ARM, he should follow the suggestion from the
> > 74facffeca3795ffb5cf8898f5859fbb822e4c5d commit message.
>
> Um...that driver runs on ARM, actually - the controller is part of the
> ARMADA 610 SoC...
Ups, I'm really sorry, it looks that I mixed something. I thought that OLPCs are
only x86 based.
> For the OLPC 1.75 there will never be a scatterlist longer than one
> page, I don't think. That controller can do HD, though, so longer
> lists are possible in the future.
Ok, I see. Do you think it would be possible to ask PXA/MMP platform developers
to
review all the drivers that can be used on that platform and enable scatter-list
chaining for this arm sub-arch? If this is a problem then we will have to delay
this
sg chaining patch.
Best regards
--
Marek Szyprowski
Samsung Poland R&D Center
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2011-08-17 8:52 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-08-10 8:23 [PATCH] media: vb2: dma-sg allocator: change scatterlist allocation method Marek Szyprowski
2011-08-12 21:54 ` Laurent Pinchart
2011-08-16 5:35 ` Marek Szyprowski
2011-08-16 8:41 ` Laurent Pinchart
2011-08-16 10:34 ` Marek Szyprowski
2011-08-16 11:01 ` Laurent Pinchart
2011-08-16 11:48 ` Marek Szyprowski
2011-08-17 0:03 ` Jonathan Corbet
2011-08-17 8:51 ` Marek Szyprowski
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.