[PATCH 1/9] avr32: Provide dma_mmap_coherent() and dma_get_sgtable()

[PATCH 1/9] avr32: Provide dma_mmap_coherent() and dma_get_sgtable()

[Geert Uytterhoeven]

avr32/allmodconfig:

drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_mmap’:
drivers/media/v4l2-core/videobuf2-dma-contig.c:204: error: implicit declaration of function ‘dma_mmap_coherent’
drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_get_base_sgt’:
drivers/media/v4l2-core/videobuf2-dma-contig.c:387: error: implicit declaration of function ‘dma_get_sgtable’

For architectures using dma_map_ops, dma_mmap_coherent() and
dma_get_sgtable() are provided in <asm-generic/dma-mapping-common.h>.

Avr32 does not use dma_map_ops, hence it should implement them as inline
stubs using dma_common_mmap() and dma_common_get_sgtable().

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: Haavard Skinnemoen <hskinnemoen@gmail.com>
Cc: Hans-Christian Egtvedt <egtvedt@samfundet.no>
---
 arch/avr32/include/asm/dma-mapping.h |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/arch/avr32/include/asm/dma-mapping.h b/arch/avr32/include/asm/dma-mapping.h
index aaf5199..b3d18f9 100644
--- a/arch/avr32/include/asm/dma-mapping.h
+++ b/arch/avr32/include/asm/dma-mapping.h
@@ -336,4 +336,14 @@ dma_sync_sg_for_device(struct device *dev, struct scatterlist *sg,
 #define dma_alloc_noncoherent(d, s, h, f) dma_alloc_coherent(d, s, h, f)
 #define dma_free_noncoherent(d, s, v, h) dma_free_coherent(d, s, v, h)
 
+/* drivers/base/dma-mapping.c */
+extern int dma_common_mmap(struct device *dev, struct vm_area_struct *vma,
+			   void *cpu_addr, dma_addr_t dma_addr, size_t size);
+extern int dma_common_get_sgtable(struct device *dev, struct sg_table *sgt,
+				  void *cpu_addr, dma_addr_t dma_addr,
+				  size_t size);
+
+#define dma_mmap_coherent(d, v, c, h, s) dma_common_mmap(d, v, c, h, s)
+#define dma_get_sgtable(d, t, v, h, s) dma_common_get_sgtable(d, t, v, h, s)
+
 #endif /* __ASM_AVR32_DMA_MAPPING_H */
-- 
1.7.0.4

[PATCH 2/9] blackfin: Provide dma_mmap_coherent() and dma_get_sgtable()

[Geert Uytterhoeven]

blackfin/allmodconfig:

drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_mmap’:
drivers/media/v4l2-core/videobuf2-dma-contig.c:204: error: implicit declaration of function ‘dma_mmap_coherent’
drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_get_base_sgt’:
drivers/media/v4l2-core/videobuf2-dma-contig.c:387: error: implicit declaration of function ‘dma_get_sgtable’

For architectures using dma_map_ops, dma_mmap_coherent() and
dma_get_sgtable() are provided in <asm-generic/dma-mapping-common.h>.

Blackfin does not use dma_map_ops, hence it should implement them as inline
stubs using dma_common_mmap() and dma_common_get_sgtable().

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: uclinux-dist-devel@blackfin.uclinux.org
---
 arch/blackfin/include/asm/dma-mapping.h |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/arch/blackfin/include/asm/dma-mapping.h b/arch/blackfin/include/asm/dma-mapping.h
index bbf4610..054d9ec 100644
--- a/arch/blackfin/include/asm/dma-mapping.h
+++ b/arch/blackfin/include/asm/dma-mapping.h
@@ -154,4 +154,14 @@ dma_cache_sync(struct device *dev, void *vaddr, size_t size,
 	_dma_sync((dma_addr_t)vaddr, size, dir);
 }
 
+/* drivers/base/dma-mapping.c */
+extern int dma_common_mmap(struct device *dev, struct vm_area_struct *vma,
+			   void *cpu_addr, dma_addr_t dma_addr, size_t size);
+extern int dma_common_get_sgtable(struct device *dev, struct sg_table *sgt,
+				  void *cpu_addr, dma_addr_t dma_addr,
+				  size_t size);
+
+#define dma_mmap_coherent(d, v, c, h, s) dma_common_mmap(d, v, c, h, s)
+#define dma_get_sgtable(d, t, v, h, s) dma_common_get_sgtable(d, t, v, h, s)
+
 #endif				/* _BLACKFIN_DMA_MAPPING_H */
-- 
1.7.0.4

[PATCH 2/9] blackfin: Provide dma_mmap_coherent() and dma_get_sgtable()

[Geert Uytterhoeven]

blackfin/allmodconfig:

drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_mmap’:
drivers/media/v4l2-core/videobuf2-dma-contig.c:204: error: implicit declaration of function ‘dma_mmap_coherent’
drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_get_base_sgt’:
drivers/media/v4l2-core/videobuf2-dma-contig.c:387: error: implicit declaration of function ‘dma_get_sgtable’

For architectures using dma_map_ops, dma_mmap_coherent() and
dma_get_sgtable() are provided in <asm-generic/dma-mapping-common.h>.

Blackfin does not use dma_map_ops, hence it should implement them as inline
stubs using dma_common_mmap() and dma_common_get_sgtable().

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: uclinux-dist-devel@blackfin.uclinux.org
---
 arch/blackfin/include/asm/dma-mapping.h |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/arch/blackfin/include/asm/dma-mapping.h b/arch/blackfin/include/asm/dma-mapping.h
index bbf4610..054d9ec 100644
--- a/arch/blackfin/include/asm/dma-mapping.h
+++ b/arch/blackfin/include/asm/dma-mapping.h
@@ -154,4 +154,14 @@ dma_cache_sync(struct device *dev, void *vaddr, size_t size,
 	_dma_sync((dma_addr_t)vaddr, size, dir);
 }
 
+/* drivers/base/dma-mapping.c */
+extern int dma_common_mmap(struct device *dev, struct vm_area_struct *vma,
+			   void *cpu_addr, dma_addr_t dma_addr, size_t size);
+extern int dma_common_get_sgtable(struct device *dev, struct sg_table *sgt,
+				  void *cpu_addr, dma_addr_t dma_addr,
+				  size_t size);
+
+#define dma_mmap_coherent(d, v, c, h, s) dma_common_mmap(d, v, c, h, s)
+#define dma_get_sgtable(d, t, v, h, s) dma_common_get_sgtable(d, t, v, h, s)
+
 #endif				/* _BLACKFIN_DMA_MAPPING_H */
-- 
1.7.0.4

_______________________________________________
Uclinux-dist-devel mailing list
Uclinux-dist-devel@blackfin.uclinux.org
https://blackfin.uclinux.org/mailman/listinfo/uclinux-dist-devel

RE: [uclinux-dist-devel] [PATCH 2/9] blackfin: Provide dma_mmap_coherent() and dma_get_sgtable()

[Jiang, Scott]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2470 bytes --]


From: uclinux-dist-devel-bounces@blackfin.uclinux.org [mailto:uclinux-dist-devel-bounces@blackfin.uclinux.org] On Behalf Of Geert Uytterhoeven
Sent: Sunday, January 13, 2013 6:45 PM
To: linux-arch@vger.kernel.org
Cc: uclinux-dist-devel@blackfin.uclinux.org; Geert Uytterhoeven; linux-kernel@vger.kernel.org; Marek Szyprowski
Subject: [uclinux-dist-devel] [PATCH 2/9] blackfin: Provide dma_mmap_coherent() and dma_get_sgtable()

blackfin/allmodconfig:

drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_mmap’:
drivers/media/v4l2-core/videobuf2-dma-contig.c:204: error: implicit declaration of function ‘dma_mmap_coherent’
drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_get_base_sgt’:
drivers/media/v4l2-core/videobuf2-dma-contig.c:387: error: implicit declaration of function ‘dma_get_sgtable’

For architectures using dma_map_ops, dma_mmap_coherent() and
dma_get_sgtable() are provided in <asm-generic/dma-mapping-common.h>.

Blackfin does not use dma_map_ops, hence it should implement them as inline stubs using dma_common_mmap() and dma_common_get_sgtable().

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
Acked-by: Scott Jiang <scott.jiang.linux@gmail.com>

---
 arch/blackfin/include/asm/dma-mapping.h |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/arch/blackfin/include/asm/dma-mapping.h b/arch/blackfin/include/asm/dma-mapping.h
index bbf4610..054d9ec 100644
--- a/arch/blackfin/include/asm/dma-mapping.h
+++ b/arch/blackfin/include/asm/dma-mapping.h
@@ -154,4 +154,14 @@ dma_cache_sync(struct device *dev, void *vaddr, size_t size,
        _dma_sync((dma_addr_t)vaddr, size, dir);  }

+/* drivers/base/dma-mapping.c */
+extern int dma_common_mmap(struct device *dev, struct vm_area_struct *vma,
+                          void *cpu_addr, dma_addr_t dma_addr, size_t size); extern int
+dma_common_get_sgtable(struct device *dev, struct sg_table *sgt,
+                                 void *cpu_addr, dma_addr_t dma_addr,
+                                 size_t size);
+
+#define dma_mmap_coherent(d, v, c, h, s) dma_common_mmap(d, v, c, h, s)
+#define dma_get_sgtable(d, t, v, h, s) dma_common_get_sgtable(d, t, v,
+h, s)
+
 #endif                         /* _BLACKFIN_DMA_MAPPING_H */
--
1.7.0.4

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

RE: [uclinux-dist-devel] [PATCH 2/9] blackfin: Provide dma_mmap_coherent() and dma_get_sgtable()

[Jiang, Scott]

From: uclinux-dist-devel-bounces@blackfin.uclinux.org [mailto:uclinux-dist-devel-bounces@blackfin.uclinux.org] On Behalf Of Geert Uytterhoeven
Sent: Sunday, January 13, 2013 6:45 PM
To: linux-arch@vger.kernel.org
Cc: uclinux-dist-devel@blackfin.uclinux.org; Geert Uytterhoeven; linux-kernel@vger.kernel.org; Marek Szyprowski
Subject: [uclinux-dist-devel] [PATCH 2/9] blackfin: Provide dma_mmap_coherent() and dma_get_sgtable()

blackfin/allmodconfig:

drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_mmap’:
drivers/media/v4l2-core/videobuf2-dma-contig.c:204: error: implicit declaration of function ‘dma_mmap_coherent’
drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_get_base_sgt’:
drivers/media/v4l2-core/videobuf2-dma-contig.c:387: error: implicit declaration of function ‘dma_get_sgtable’

For architectures using dma_map_ops, dma_mmap_coherent() and
dma_get_sgtable() are provided in <asm-generic/dma-mapping-common.h>.

Blackfin does not use dma_map_ops, hence it should implement them as inline stubs using dma_common_mmap() and dma_common_get_sgtable().

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
Acked-by: Scott Jiang <scott.jiang.linux@gmail.com>

---
 arch/blackfin/include/asm/dma-mapping.h |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/arch/blackfin/include/asm/dma-mapping.h b/arch/blackfin/include/asm/dma-mapping.h
index bbf4610..054d9ec 100644
--- a/arch/blackfin/include/asm/dma-mapping.h
+++ b/arch/blackfin/include/asm/dma-mapping.h
@@ -154,4 +154,14 @@ dma_cache_sync(struct device *dev, void *vaddr, size_t size,
        _dma_sync((dma_addr_t)vaddr, size, dir);  }

+/* drivers/base/dma-mapping.c */
+extern int dma_common_mmap(struct device *dev, struct vm_area_struct *vma,
+                          void *cpu_addr, dma_addr_t dma_addr, size_t size); extern int
+dma_common_get_sgtable(struct device *dev, struct sg_table *sgt,
+                                 void *cpu_addr, dma_addr_t dma_addr,
+                                 size_t size);
+
+#define dma_mmap_coherent(d, v, c, h, s) dma_common_mmap(d, v, c, h, s)
+#define dma_get_sgtable(d, t, v, h, s) dma_common_get_sgtable(d, t, v,
+h, s)
+
 #endif                         /* _BLACKFIN_DMA_MAPPING_H */
--
1.7.0.4

[PATCH 3/9] c6x: Provide dma_mmap_coherent() and dma_get_sgtable()

[Geert Uytterhoeven]

c6x/allmodconfig (assumed):

drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_mmap’:
drivers/media/v4l2-core/videobuf2-dma-contig.c:204: error: implicit declaration of function ‘dma_mmap_coherent’
drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_get_base_sgt’:
drivers/media/v4l2-core/videobuf2-dma-contig.c:387: error: implicit declaration of function ‘dma_get_sgtable’

For architectures using dma_map_ops, dma_mmap_coherent() and
dma_get_sgtable() are provided in <asm-generic/dma-mapping-common.h>.

C6x does not use dma_map_ops, hence it should implement them as inline
stubs using dma_common_mmap() and dma_common_get_sgtable().

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: linux-c6x-dev@linux-c6x.org
---
Not compile-tested due to lack of cross-compiler

 arch/c6x/include/asm/dma-mapping.h |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/arch/c6x/include/asm/dma-mapping.h b/arch/c6x/include/asm/dma-mapping.h
index 3c69406..d4cdf85 100644
--- a/arch/c6x/include/asm/dma-mapping.h
+++ b/arch/c6x/include/asm/dma-mapping.h
@@ -89,4 +89,14 @@ extern void dma_free_coherent(struct device *, size_t, void *, dma_addr_t);
 #define dma_alloc_noncoherent(d, s, h, f) dma_alloc_coherent((d), (s), (h), (f))
 #define dma_free_noncoherent(d, s, v, h)  dma_free_coherent((d), (s), (v), (h))
 
+/* drivers/base/dma-mapping.c */
+extern int dma_common_mmap(struct device *dev, struct vm_area_struct *vma,
+			   void *cpu_addr, dma_addr_t dma_addr, size_t size);
+extern int dma_common_get_sgtable(struct device *dev, struct sg_table *sgt,
+				  void *cpu_addr, dma_addr_t dma_addr,
+				  size_t size);
+
+#define dma_mmap_coherent(d, v, c, h, s) dma_common_mmap(d, v, c, h, s)
+#define dma_get_sgtable(d, t, v, h, s) dma_common_get_sgtable(d, t, v, h, s)
+
 #endif	/* _ASM_C6X_DMA_MAPPING_H */
-- 
1.7.0.4

Re: [Linux-c6x-dev] [PATCH 3/9] c6x: Provide dma_mmap_coherent() and dma_get_sgtable()

[Mark Salter]

On Sun, 2013-01-13 at 11:44 +0100, Geert Uytterhoeven wrote:
> c6x/allmodconfig (assumed):
> 
> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_mmap’:
> drivers/media/v4l2-core/videobuf2-dma-contig.c:204: error: implicit declaration of function ‘dma_mmap_coherent’
> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_get_base_sgt’:
> drivers/media/v4l2-core/videobuf2-dma-contig.c:387: error: implicit declaration of function ‘dma_get_sgtable’
> 
> For architectures using dma_map_ops, dma_mmap_coherent() and
> dma_get_sgtable() are provided in <asm-generic/dma-mapping-common.h>.
> 
> C6x does not use dma_map_ops, hence it should implement them as inline
> stubs using dma_common_mmap() and dma_common_get_sgtable().
> 

So are dma_mmap_coherent() and dma_get_sgtable() part of the DMA API
now? I don't them in Documentation/DMA*.txt anywhere.

Why does the default dma_common_mmap() for !CONFIG_MMU return an
error?

Wouldn't it be better to provide default implementations that an arch
could override rather than having to patch all "no dma_map_ops"
architectures?

--Mark

Re: [Linux-c6x-dev] [PATCH 3/9] c6x: Provide dma_mmap_coherent() and dma_get_sgtable()

[Vineet Gupta]

On Monday 14 January 2013 09:07 PM, Mark Salter wrote:
> On Sun, 2013-01-13 at 11:44 +0100, Geert Uytterhoeven wrote:
>> c6x/allmodconfig (assumed):
>>
>> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_mmap’:
>> drivers/media/v4l2-core/videobuf2-dma-contig.c:204: error: implicit declaration of function ‘dma_mmap_coherent’
>> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_get_base_sgt’:
>> drivers/media/v4l2-core/videobuf2-dma-contig.c:387: error: implicit declaration of function ‘dma_get_sgtable’
>>
>> For architectures using dma_map_ops, dma_mmap_coherent() and
>> dma_get_sgtable() are provided in <asm-generic/dma-mapping-common.h>.
>>
>> C6x does not use dma_map_ops, hence it should implement them as inline
>> stubs using dma_common_mmap() and dma_common_get_sgtable().
>>
> 
> So are dma_mmap_coherent() and dma_get_sgtable() part of the DMA API
> now? I don't them in Documentation/DMA*.txt anywhere.
> 
> Why does the default dma_common_mmap() for !CONFIG_MMU return an
> error?
> 
> Wouldn't it be better to provide default implementations that an arch
> could override rather than having to patch all "no dma_map_ops"
> architectures?
> 
> --Mark
> 
> 

Speaking for the still-reviewed ARC Port, I completely agree with Mark.

-Vineet

Re: [Linux-c6x-dev] [PATCH 3/9] c6x: Provide dma_mmap_coherent() and dma_get_sgtable()

[Vineet Gupta]

On Monday 14 January 2013 09:07 PM, Mark Salter wrote:
> On Sun, 2013-01-13 at 11:44 +0100, Geert Uytterhoeven wrote:
>> c6x/allmodconfig (assumed):
>>
>> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_mmap’:
>> drivers/media/v4l2-core/videobuf2-dma-contig.c:204: error: implicit declaration of function ‘dma_mmap_coherent’
>> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_get_base_sgt’:
>> drivers/media/v4l2-core/videobuf2-dma-contig.c:387: error: implicit declaration of function ‘dma_get_sgtable’
>>
>> For architectures using dma_map_ops, dma_mmap_coherent() and
>> dma_get_sgtable() are provided in <asm-generic/dma-mapping-common.h>.
>>
>> C6x does not use dma_map_ops, hence it should implement them as inline
>> stubs using dma_common_mmap() and dma_common_get_sgtable().
>>
> 
> So are dma_mmap_coherent() and dma_get_sgtable() part of the DMA API
> now? I don't them in Documentation/DMA*.txt anywhere.
> 
> Why does the default dma_common_mmap() for !CONFIG_MMU return an
> error?
> 
> Wouldn't it be better to provide default implementations that an arch
> could override rather than having to patch all "no dma_map_ops"
> architectures?
> 
> --Mark
> 
> 

Speaking for the still-reviewed ARC Port, I completely agree with Mark.

-Vineet

Re: [Linux-c6x-dev] [PATCH 3/9] c6x: Provide dma_mmap_coherent() and dma_get_sgtable()

[Geert Uytterhoeven]

Marek?

On Tue, Jan 15, 2013 at 5:16 AM, Vineet Gupta
<Vineet.Gupta1@synopsys.com> wrote:
> On Monday 14 January 2013 09:07 PM, Mark Salter wrote:
>> On Sun, 2013-01-13 at 11:44 +0100, Geert Uytterhoeven wrote:
>>> c6x/allmodconfig (assumed):
>>>
>>> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_mmap’:
>>> drivers/media/v4l2-core/videobuf2-dma-contig.c:204: error: implicit declaration of function ‘dma_mmap_coherent’
>>> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_get_base_sgt’:
>>> drivers/media/v4l2-core/videobuf2-dma-contig.c:387: error: implicit declaration of function ‘dma_get_sgtable’
>>>
>>> For architectures using dma_map_ops, dma_mmap_coherent() and
>>> dma_get_sgtable() are provided in <asm-generic/dma-mapping-common.h>.
>>>
>>> C6x does not use dma_map_ops, hence it should implement them as inline
>>> stubs using dma_common_mmap() and dma_common_get_sgtable().
>>>
>>
>> So are dma_mmap_coherent() and dma_get_sgtable() part of the DMA API
>> now? I don't them in Documentation/DMA*.txt anywhere.
>>
>> Why does the default dma_common_mmap() for !CONFIG_MMU return an
>> error?
>>
>> Wouldn't it be better to provide default implementations that an arch
>> could override rather than having to patch all "no dma_map_ops"
>> architectures?
>>
>> --Mark
>>
>>
>
> Speaking for the still-reviewed ARC Port, I completely agree with Mark.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [Linux-c6x-dev] [PATCH 3/9] c6x: Provide dma_mmap_coherent() and dma_get_sgtable()

[Marek Szyprowski]

Hello,

On 1/15/2013 10:13 AM, Geert Uytterhoeven wrote:
> Marek?
>
> On Tue, Jan 15, 2013 at 5:16 AM, Vineet Gupta
> <Vineet.Gupta1@synopsys.com> wrote:
> > On Monday 14 January 2013 09:07 PM, Mark Salter wrote:
> >> On Sun, 2013-01-13 at 11:44 +0100, Geert Uytterhoeven wrote:
> >>> c6x/allmodconfig (assumed):
> >>>
> >>> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_mmap’:
> >>> drivers/media/v4l2-core/videobuf2-dma-contig.c:204: error: implicit declaration of function ‘dma_mmap_coherent’
> >>> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_get_base_sgt’:
> >>> drivers/media/v4l2-core/videobuf2-dma-contig.c:387: error: implicit declaration of function ‘dma_get_sgtable’
> >>>
> >>> For architectures using dma_map_ops, dma_mmap_coherent() and
> >>> dma_get_sgtable() are provided in <asm-generic/dma-mapping-common.h>.
> >>>
> >>> C6x does not use dma_map_ops, hence it should implement them as inline
> >>> stubs using dma_common_mmap() and dma_common_get_sgtable().
> >>>
> >> So are dma_mmap_coherent() and dma_get_sgtable() part of the DMA API
> >> now? I don't them in Documentation/DMA*.txt anywhere.
> >>
> >> Why does the default dma_common_mmap() for !CONFIG_MMU return an
> >> error?
> >>
> >> Wouldn't it be better to provide default implementations that an arch
> >> could override rather than having to patch all "no dma_map_ops"
> >> architectures?
> >>
> > Speaking for the still-reviewed ARC Port, I completely agree with Mark.

dma_mmap_coherent() was partially in the DMA mapping API for some time, but
it was available only on a few architectures (afair ARM, powerpc and avr32).
This caused significant problems for writing unified device drivers or some
device helper modules which were aimed to work on more than one 
architecture.

dma_get_sgtable() is an extension discussed during the Linaro meetings. It
is required to correctly implement buffer sharing between device driver
without hacks or any assumptions about memory layout in the device drivers.

I have implemented some generic code for both of those two functions, 
keeping
in mind that on some hardware architectures (like already mentioned VIVT)
it might be not possible to provide coherent mapping to userspace. It is
perfectly fine for those functions to return an error in such case.

My common implementation returns error for NOMMU systems because I was
convinced that remapping the pages to userspace is simply not possible
without the MMU.

I'm really sorry for skipping the non dma_map_ops based architectures, I had
a plan to cover them in one of the next updates, but in meantime the v4l2
updates requiring those interfaces has been merged and I got very busy with
other stuff, then Geert was faster fixing it.

I'm also working on updated DMA mapping documentation to cover those new
functions as well as some new DMA attributes introduced recently, I hope to
post them soon.

Best regards
-- 
Marek Szyprowski
Samsung Poland R&D Center

Re: [Linux-c6x-dev] [PATCH 3/9] c6x: Provide dma_mmap_coherent() and dma_get_sgtable()

[James Bottomley]

On Tue, 2013-01-15 at 15:07 +0100, Marek Szyprowski wrote:
> Hello,
> 
> On 1/15/2013 10:13 AM, Geert Uytterhoeven wrote:
> > Marek?
> >
> > On Tue, Jan 15, 2013 at 5:16 AM, Vineet Gupta
> > <Vineet.Gupta1@synopsys.com> wrote:
> > > On Monday 14 January 2013 09:07 PM, Mark Salter wrote:
> > >> On Sun, 2013-01-13 at 11:44 +0100, Geert Uytterhoeven wrote:
> > >>> c6x/allmodconfig (assumed):
> > >>>
> > >>> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_mmap’:
> > >>> drivers/media/v4l2-core/videobuf2-dma-contig.c:204: error: implicit declaration of function ‘dma_mmap_coherent’
> > >>> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_get_base_sgt’:
> > >>> drivers/media/v4l2-core/videobuf2-dma-contig.c:387: error: implicit declaration of function ‘dma_get_sgtable’
> > >>>
> > >>> For architectures using dma_map_ops, dma_mmap_coherent() and
> > >>> dma_get_sgtable() are provided in <asm-generic/dma-mapping-common.h>.
> > >>>
> > >>> C6x does not use dma_map_ops, hence it should implement them as inline
> > >>> stubs using dma_common_mmap() and dma_common_get_sgtable().
> > >>>
> > >> So are dma_mmap_coherent() and dma_get_sgtable() part of the DMA API
> > >> now? I don't them in Documentation/DMA*.txt anywhere.
> > >>
> > >> Why does the default dma_common_mmap() for !CONFIG_MMU return an
> > >> error?
> > >>
> > >> Wouldn't it be better to provide default implementations that an arch
> > >> could override rather than having to patch all "no dma_map_ops"
> > >> architectures?
> > >>
> > > Speaking for the still-reviewed ARC Port, I completely agree with Mark.
> 
> dma_mmap_coherent() was partially in the DMA mapping API for some time, but
> it was available only on a few architectures (afair ARM, powerpc and avr32).
> This caused significant problems for writing unified device drivers or some
> device helper modules which were aimed to work on more than one 
> architecture.
> 
> dma_get_sgtable() is an extension discussed during the Linaro meetings. It
> is required to correctly implement buffer sharing between device driver
> without hacks or any assumptions about memory layout in the device drivers.
> 
> I have implemented some generic code for both of those two functions, 
> keeping
> in mind that on some hardware architectures (like already mentioned VIVT)
> it might be not possible to provide coherent mapping to userspace. It is
> perfectly fine for those functions to return an error in such case.

It's not possible on VIPT either.  This means that the API is unusable
on quite a large number of architectures.  Surely, if we're starting to
write drivers using this, we need to fix the API before more people try
to use it.

For PA-RISC (and all other VIPT, I assume) I need an API which allows me
to remap the virtual address of the kernel component (probably using the
kmap area) so the user space and kernel space addresses are congruent.

James

Re: [Linux-c6x-dev] [PATCH 3/9] c6x: Provide dma_mmap_coherent() and dma_get_sgtable()

[Geert Uytterhoeven]

On Tue, Jan 15, 2013 at 5:56 PM, James Bottomley
<James.Bottomley@hansenpartnership.com> wrote:
> On Tue, 2013-01-15 at 15:07 +0100, Marek Szyprowski wrote:
>> On 1/15/2013 10:13 AM, Geert Uytterhoeven wrote:
>> > Marek?
>> >
>> > On Tue, Jan 15, 2013 at 5:16 AM, Vineet Gupta
>> > <Vineet.Gupta1@synopsys.com> wrote:
>> > > On Monday 14 January 2013 09:07 PM, Mark Salter wrote:
>> > >> On Sun, 2013-01-13 at 11:44 +0100, Geert Uytterhoeven wrote:
>> > >>> c6x/allmodconfig (assumed):
>> > >>>
>> > >>> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_mmap’:
>> > >>> drivers/media/v4l2-core/videobuf2-dma-contig.c:204: error: implicit declaration of function ‘dma_mmap_coherent’
>> > >>> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_get_base_sgt’:
>> > >>> drivers/media/v4l2-core/videobuf2-dma-contig.c:387: error: implicit declaration of function ‘dma_get_sgtable’
>> > >>>
>> > >>> For architectures using dma_map_ops, dma_mmap_coherent() and
>> > >>> dma_get_sgtable() are provided in <asm-generic/dma-mapping-common.h>.
>> > >>>
>> > >>> C6x does not use dma_map_ops, hence it should implement them as inline
>> > >>> stubs using dma_common_mmap() and dma_common_get_sgtable().
>> > >>>
>> > >> So are dma_mmap_coherent() and dma_get_sgtable() part of the DMA API
>> > >> now? I don't them in Documentation/DMA*.txt anywhere.
>> > >>
>> > >> Why does the default dma_common_mmap() for !CONFIG_MMU return an
>> > >> error?
>> > >>
>> > >> Wouldn't it be better to provide default implementations that an arch
>> > >> could override rather than having to patch all "no dma_map_ops"
>> > >> architectures?
>> > >>
>> > > Speaking for the still-reviewed ARC Port, I completely agree with Mark.
>>
>> dma_mmap_coherent() was partially in the DMA mapping API for some time, but
>> it was available only on a few architectures (afair ARM, powerpc and avr32).
>> This caused significant problems for writing unified device drivers or some
>> device helper modules which were aimed to work on more than one
>> architecture.
>>
>> dma_get_sgtable() is an extension discussed during the Linaro meetings. It
>> is required to correctly implement buffer sharing between device driver
>> without hacks or any assumptions about memory layout in the device drivers.
>>
>> I have implemented some generic code for both of those two functions,
>> keeping
>> in mind that on some hardware architectures (like already mentioned VIVT)
>> it might be not possible to provide coherent mapping to userspace. It is
>> perfectly fine for those functions to return an error in such case.
>
> It's not possible on VIPT either.  This means that the API is unusable
> on quite a large number of architectures.  Surely, if we're starting to
> write drivers using this, we need to fix the API before more people try
> to use it.
>
> For PA-RISC (and all other VIPT, I assume) I need an API which allows me
> to remap the virtual address of the kernel component (probably using the
> kmap area) so the user space and kernel space addresses are congruent.

So what are we gonna do for 3.8? I'd like to get my allmodconfig build
green again ;-)

Change the API?

Keep the API but do a best-effort fix to unbreak the builds?
  - Apply my patches that got acks (avr32/blackfin/cris/m68k),
  - Use static inlines that return -EINVAL for the rest
(c6x/frv/mn10300/parisc/xtensa).
I still have a few m68k fixes queued for 3.8, for which I've been postponing the
pull request to get this sorted out, so I could include the above.

Any other solution?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [Linux-c6x-dev] [PATCH 3/9] c6x: Provide dma_mmap_coherent() and dma_get_sgtable()

[James Bottomley]

On Mon, 2013-01-21 at 21:00 +0100, Geert Uytterhoeven wrote:
> On Tue, Jan 15, 2013 at 5:56 PM, James Bottomley
> <James.Bottomley@hansenpartnership.com> wrote:
> > On Tue, 2013-01-15 at 15:07 +0100, Marek Szyprowski wrote:
> >> On 1/15/2013 10:13 AM, Geert Uytterhoeven wrote:
> >> > Marek?
> >> >
> >> > On Tue, Jan 15, 2013 at 5:16 AM, Vineet Gupta
> >> > <Vineet.Gupta1@synopsys.com> wrote:
> >> > > On Monday 14 January 2013 09:07 PM, Mark Salter wrote:
> >> > >> On Sun, 2013-01-13 at 11:44 +0100, Geert Uytterhoeven wrote:
> >> > >>> c6x/allmodconfig (assumed):
> >> > >>>
> >> > >>> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_mmap’:
> >> > >>> drivers/media/v4l2-core/videobuf2-dma-contig.c:204: error: implicit declaration of function ‘dma_mmap_coherent’
> >> > >>> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_get_base_sgt’:
> >> > >>> drivers/media/v4l2-core/videobuf2-dma-contig.c:387: error: implicit declaration of function ‘dma_get_sgtable’
> >> > >>>
> >> > >>> For architectures using dma_map_ops, dma_mmap_coherent() and
> >> > >>> dma_get_sgtable() are provided in <asm-generic/dma-mapping-common.h>.
> >> > >>>
> >> > >>> C6x does not use dma_map_ops, hence it should implement them as inline
> >> > >>> stubs using dma_common_mmap() and dma_common_get_sgtable().
> >> > >>>
> >> > >> So are dma_mmap_coherent() and dma_get_sgtable() part of the DMA API
> >> > >> now? I don't them in Documentation/DMA*.txt anywhere.
> >> > >>
> >> > >> Why does the default dma_common_mmap() for !CONFIG_MMU return an
> >> > >> error?
> >> > >>
> >> > >> Wouldn't it be better to provide default implementations that an arch
> >> > >> could override rather than having to patch all "no dma_map_ops"
> >> > >> architectures?
> >> > >>
> >> > > Speaking for the still-reviewed ARC Port, I completely agree with Mark.
> >>
> >> dma_mmap_coherent() was partially in the DMA mapping API for some time, but
> >> it was available only on a few architectures (afair ARM, powerpc and avr32).
> >> This caused significant problems for writing unified device drivers or some
> >> device helper modules which were aimed to work on more than one
> >> architecture.
> >>
> >> dma_get_sgtable() is an extension discussed during the Linaro meetings. It
> >> is required to correctly implement buffer sharing between device driver
> >> without hacks or any assumptions about memory layout in the device drivers.
> >>
> >> I have implemented some generic code for both of those two functions,
> >> keeping
> >> in mind that on some hardware architectures (like already mentioned VIVT)
> >> it might be not possible to provide coherent mapping to userspace. It is
> >> perfectly fine for those functions to return an error in such case.
> >
> > It's not possible on VIPT either.  This means that the API is unusable
> > on quite a large number of architectures.  Surely, if we're starting to
> > write drivers using this, we need to fix the API before more people try
> > to use it.
> >
> > For PA-RISC (and all other VIPT, I assume) I need an API which allows me
> > to remap the virtual address of the kernel component (probably using the
> > kmap area) so the user space and kernel space addresses are congruent.
> 
> So what are we gonna do for 3.8? I'd like to get my allmodconfig build
> green again ;-)
> 
> Change the API?

Well, if we want the API to work universally, we have to.  As I said,
for VIPT systems, the only coherency mechanism we have is the virtual
address ... we have to fix that in the kernel to be congruent with the
userspace virtual address if we want coherency between the kernel and
userspace.

However, if it only needs to work on ARM and x86, it can stay the way it
is and we could just pull it out of the generic core.

Who actually wants to use this API, and what for?

> Keep the API but do a best-effort fix to unbreak the builds?
>   - Apply my patches that got acks (avr32/blackfin/cris/m68k),
>   - Use static inlines that return -EINVAL for the rest
> (c6x/frv/mn10300/parisc/xtensa).
> I still have a few m68k fixes queued for 3.8, for which I've been postponing the
> pull request to get this sorted out, so I could include the above.
> 
> Any other solution?

If it's an API that only works on ARM and x86, there's not much point
pretending it's universal, so we should remove it from the generic arch
code and allow only those architectures which can use it.

James

Re: [Linux-c6x-dev] [PATCH 3/9] c6x: Provide dma_mmap_coherent() and dma_get_sgtable()

[James Bottomley]

On Mon, 2013-01-21 at 22:59 +0000, James Bottomley wrote:
> On Mon, 2013-01-21 at 21:00 +0100, Geert Uytterhoeven wrote:
> > On Tue, Jan 15, 2013 at 5:56 PM, James Bottomley
> > <James.Bottomley@hansenpartnership.com> wrote:
> > > On Tue, 2013-01-15 at 15:07 +0100, Marek Szyprowski wrote:
> > >> On 1/15/2013 10:13 AM, Geert Uytterhoeven wrote:
> > >> > Marek?
> > >> >
> > >> > On Tue, Jan 15, 2013 at 5:16 AM, Vineet Gupta
> > >> > <Vineet.Gupta1@synopsys.com> wrote:
> > >> > > On Monday 14 January 2013 09:07 PM, Mark Salter wrote:
> > >> > >> On Sun, 2013-01-13 at 11:44 +0100, Geert Uytterhoeven wrote:
> > >> > >>> c6x/allmodconfig (assumed):
> > >> > >>>
> > >> > >>> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_mmap’:
> > >> > >>> drivers/media/v4l2-core/videobuf2-dma-contig.c:204: error: implicit declaration of function ‘dma_mmap_coherent’
> > >> > >>> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_get_base_sgt’:
> > >> > >>> drivers/media/v4l2-core/videobuf2-dma-contig.c:387: error: implicit declaration of function ‘dma_get_sgtable’
> > >> > >>>
> > >> > >>> For architectures using dma_map_ops, dma_mmap_coherent() and
> > >> > >>> dma_get_sgtable() are provided in <asm-generic/dma-mapping-common.h>.
> > >> > >>>
> > >> > >>> C6x does not use dma_map_ops, hence it should implement them as inline
> > >> > >>> stubs using dma_common_mmap() and dma_common_get_sgtable().
> > >> > >>>
> > >> > >> So are dma_mmap_coherent() and dma_get_sgtable() part of the DMA API
> > >> > >> now? I don't them in Documentation/DMA*.txt anywhere.
> > >> > >>
> > >> > >> Why does the default dma_common_mmap() for !CONFIG_MMU return an
> > >> > >> error?
> > >> > >>
> > >> > >> Wouldn't it be better to provide default implementations that an arch
> > >> > >> could override rather than having to patch all "no dma_map_ops"
> > >> > >> architectures?
> > >> > >>
> > >> > > Speaking for the still-reviewed ARC Port, I completely agree with Mark.
> > >>
> > >> dma_mmap_coherent() was partially in the DMA mapping API for some time, but
> > >> it was available only on a few architectures (afair ARM, powerpc and avr32).
> > >> This caused significant problems for writing unified device drivers or some
> > >> device helper modules which were aimed to work on more than one
> > >> architecture.
> > >>
> > >> dma_get_sgtable() is an extension discussed during the Linaro meetings. It
> > >> is required to correctly implement buffer sharing between device driver
> > >> without hacks or any assumptions about memory layout in the device drivers.
> > >>
> > >> I have implemented some generic code for both of those two functions,
> > >> keeping
> > >> in mind that on some hardware architectures (like already mentioned VIVT)
> > >> it might be not possible to provide coherent mapping to userspace. It is
> > >> perfectly fine for those functions to return an error in such case.
> > >
> > > It's not possible on VIPT either.  This means that the API is unusable
> > > on quite a large number of architectures.  Surely, if we're starting to
> > > write drivers using this, we need to fix the API before more people try
> > > to use it.
> > >
> > > For PA-RISC (and all other VIPT, I assume) I need an API which allows me
> > > to remap the virtual address of the kernel component (probably using the
> > > kmap area) so the user space and kernel space addresses are congruent.
> > 
> > So what are we gonna do for 3.8? I'd like to get my allmodconfig build
> > green again ;-)
> > 
> > Change the API?
> 
> Well, if we want the API to work universally, we have to.  As I said,
> for VIPT systems, the only coherency mechanism we have is the virtual
> address ... we have to fix that in the kernel to be congruent with the
> userspace virtual address if we want coherency between the kernel and
> userspace.
> 
> However, if it only needs to work on ARM and x86, it can stay the way it
> is and we could just pull it out of the generic core.
> 
> Who actually wants to use this API, and what for?
> 
> > Keep the API but do a best-effort fix to unbreak the builds?
> >   - Apply my patches that got acks (avr32/blackfin/cris/m68k),
> >   - Use static inlines that return -EINVAL for the rest
> > (c6x/frv/mn10300/parisc/xtensa).
> > I still have a few m68k fixes queued for 3.8, for which I've been postponing the
> > pull request to get this sorted out, so I could include the above.
> > 
> > Any other solution?
> 
> If it's an API that only works on ARM and x86, there's not much point
> pretending it's universal, so we should remove it from the generic arch
> code and allow only those architectures which can use it.

There might be a simple solution:  just replace void *cpu_addr with void
**cpu_addr in the API.  This is a bit nasty since compilers think that
void ** to void * conversion is quite legal, so it would be hard to pick
up misuse of this (uint8_t ** would be better).  That way VIPT could
remap the kernel pages to a coherent address.  This would probably have
to change in the dma_mmap_attr() and dma_ops structures.

All consumers would have to expect cpu_addr might change, but that seems
doable.

James

Re: [Linux-c6x-dev] [PATCH 3/9] c6x: Provide dma_mmap_coherent() and dma_get_sgtable()

[James Bottomley]

[adding Mauro and v4l since they're the only non-arm consumers]
On Tue, 2013-01-22 at 10:13 +0000, James Bottomley wrote:
> On Mon, 2013-01-21 at 22:59 +0000, James Bottomley wrote:
> > On Mon, 2013-01-21 at 21:00 +0100, Geert Uytterhoeven wrote:
> > > On Tue, Jan 15, 2013 at 5:56 PM, James Bottomley
> > > <James.Bottomley@hansenpartnership.com> wrote:
> > > > On Tue, 2013-01-15 at 15:07 +0100, Marek Szyprowski wrote:
> > > >> On 1/15/2013 10:13 AM, Geert Uytterhoeven wrote:
> > > >> > Marek?
> > > >> >
> > > >> > On Tue, Jan 15, 2013 at 5:16 AM, Vineet Gupta
> > > >> > <Vineet.Gupta1@synopsys.com> wrote:
> > > >> > > On Monday 14 January 2013 09:07 PM, Mark Salter wrote:
> > > >> > >> On Sun, 2013-01-13 at 11:44 +0100, Geert Uytterhoeven wrote:
> > > >> > >>> c6x/allmodconfig (assumed):
> > > >> > >>>
> > > >> > >>> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_mmap’:
> > > >> > >>> drivers/media/v4l2-core/videobuf2-dma-contig.c:204: error: implicit declaration of function ‘dma_mmap_coherent’
> > > >> > >>> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_get_base_sgt’:
> > > >> > >>> drivers/media/v4l2-core/videobuf2-dma-contig.c:387: error: implicit declaration of function ‘dma_get_sgtable’
> > > >> > >>>
> > > >> > >>> For architectures using dma_map_ops, dma_mmap_coherent() and
> > > >> > >>> dma_get_sgtable() are provided in <asm-generic/dma-mapping-common.h>.
> > > >> > >>>
> > > >> > >>> C6x does not use dma_map_ops, hence it should implement them as inline
> > > >> > >>> stubs using dma_common_mmap() and dma_common_get_sgtable().
> > > >> > >>>
> > > >> > >> So are dma_mmap_coherent() and dma_get_sgtable() part of the DMA API
> > > >> > >> now? I don't them in Documentation/DMA*.txt anywhere.
> > > >> > >>
> > > >> > >> Why does the default dma_common_mmap() for !CONFIG_MMU return an
> > > >> > >> error?
> > > >> > >>
> > > >> > >> Wouldn't it be better to provide default implementations that an arch
> > > >> > >> could override rather than having to patch all "no dma_map_ops"
> > > >> > >> architectures?
> > > >> > >>
> > > >> > > Speaking for the still-reviewed ARC Port, I completely agree with Mark.
> > > >>
> > > >> dma_mmap_coherent() was partially in the DMA mapping API for some time, but
> > > >> it was available only on a few architectures (afair ARM, powerpc and avr32).
> > > >> This caused significant problems for writing unified device drivers or some
> > > >> device helper modules which were aimed to work on more than one
> > > >> architecture.
> > > >>
> > > >> dma_get_sgtable() is an extension discussed during the Linaro meetings. It
> > > >> is required to correctly implement buffer sharing between device driver
> > > >> without hacks or any assumptions about memory layout in the device drivers.
> > > >>
> > > >> I have implemented some generic code for both of those two functions,
> > > >> keeping
> > > >> in mind that on some hardware architectures (like already mentioned VIVT)
> > > >> it might be not possible to provide coherent mapping to userspace. It is
> > > >> perfectly fine for those functions to return an error in such case.
> > > >
> > > > It's not possible on VIPT either.  This means that the API is unusable
> > > > on quite a large number of architectures.  Surely, if we're starting to
> > > > write drivers using this, we need to fix the API before more people try
> > > > to use it.
> > > >
> > > > For PA-RISC (and all other VIPT, I assume) I need an API which allows me
> > > > to remap the virtual address of the kernel component (probably using the
> > > > kmap area) so the user space and kernel space addresses are congruent.
> > > 
> > > So what are we gonna do for 3.8? I'd like to get my allmodconfig build
> > > green again ;-)
> > > 
> > > Change the API?
> > 
> > Well, if we want the API to work universally, we have to.  As I said,
> > for VIPT systems, the only coherency mechanism we have is the virtual
> > address ... we have to fix that in the kernel to be congruent with the
> > userspace virtual address if we want coherency between the kernel and
> > userspace.
> > 
> > However, if it only needs to work on ARM and x86, it can stay the way it
> > is and we could just pull it out of the generic core.
> > 
> > Who actually wants to use this API, and what for?
> > 
> > > Keep the API but do a best-effort fix to unbreak the builds?
> > >   - Apply my patches that got acks (avr32/blackfin/cris/m68k),
> > >   - Use static inlines that return -EINVAL for the rest
> > > (c6x/frv/mn10300/parisc/xtensa).
> > > I still have a few m68k fixes queued for 3.8, for which I've been postponing the
> > > pull request to get this sorted out, so I could include the above.
> > > 
> > > Any other solution?
> > 
> > If it's an API that only works on ARM and x86, there's not much point
> > pretending it's universal, so we should remove it from the generic arch
> > code and allow only those architectures which can use it.
> 
> There might be a simple solution:  just replace void *cpu_addr with void
> **cpu_addr in the API.  This is a bit nasty since compilers think that
> void ** to void * conversion is quite legal, so it would be hard to pick
> up misuse of this (uint8_t ** would be better).  That way VIPT could
> remap the kernel pages to a coherent address.  This would probably have
> to change in the dma_mmap_attr() and dma_ops structures.
> 
> All consumers would have to expect cpu_addr might change, but that seems
> doable.

Mauro, will this work for you and the v4l guys?  You've got a use
embedded in 

drivers/media/v4l2-core/videobuf2-dma-contig.c

Which I can't tell how extensive it might be.

James

Re: [Linux-c6x-dev] [PATCH 3/9] c6x: Provide dma_mmap_coherent() and dma_get_sgtable()

[James Bottomley]

On Tue, 2013-01-22 at 10:16 +0000, James Bottomley wrote:
> [adding Mauro and v4l since they're the only non-arm consumers]
> On Tue, 2013-01-22 at 10:13 +0000, James Bottomley wrote:
> > On Mon, 2013-01-21 at 22:59 +0000, James Bottomley wrote:
> > > On Mon, 2013-01-21 at 21:00 +0100, Geert Uytterhoeven wrote:
> > > > On Tue, Jan 15, 2013 at 5:56 PM, James Bottomley
> > > > <James.Bottomley@hansenpartnership.com> wrote:
> > > > > On Tue, 2013-01-15 at 15:07 +0100, Marek Szyprowski wrote:
> > > > >> On 1/15/2013 10:13 AM, Geert Uytterhoeven wrote:
> > > > >> > Marek?
> > > > >> >
> > > > >> > On Tue, Jan 15, 2013 at 5:16 AM, Vineet Gupta
> > > > >> > <Vineet.Gupta1@synopsys.com> wrote:
> > > > >> > > On Monday 14 January 2013 09:07 PM, Mark Salter wrote:
> > > > >> > >> On Sun, 2013-01-13 at 11:44 +0100, Geert Uytterhoeven wrote:
> > > > >> > >>> c6x/allmodconfig (assumed):
> > > > >> > >>>
> > > > >> > >>> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_mmap’:
> > > > >> > >>> drivers/media/v4l2-core/videobuf2-dma-contig.c:204: error: implicit declaration of function ‘dma_mmap_coherent’
> > > > >> > >>> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_get_base_sgt’:
> > > > >> > >>> drivers/media/v4l2-core/videobuf2-dma-contig.c:387: error: implicit declaration of function ‘dma_get_sgtable’
> > > > >> > >>>
> > > > >> > >>> For architectures using dma_map_ops, dma_mmap_coherent() and
> > > > >> > >>> dma_get_sgtable() are provided in <asm-generic/dma-mapping-common.h>.
> > > > >> > >>>
> > > > >> > >>> C6x does not use dma_map_ops, hence it should implement them as inline
> > > > >> > >>> stubs using dma_common_mmap() and dma_common_get_sgtable().
> > > > >> > >>>
> > > > >> > >> So are dma_mmap_coherent() and dma_get_sgtable() part of the DMA API
> > > > >> > >> now? I don't them in Documentation/DMA*.txt anywhere.
> > > > >> > >>
> > > > >> > >> Why does the default dma_common_mmap() for !CONFIG_MMU return an
> > > > >> > >> error?
> > > > >> > >>
> > > > >> > >> Wouldn't it be better to provide default implementations that an arch
> > > > >> > >> could override rather than having to patch all "no dma_map_ops"
> > > > >> > >> architectures?
> > > > >> > >>
> > > > >> > > Speaking for the still-reviewed ARC Port, I completely agree with Mark.
> > > > >>
> > > > >> dma_mmap_coherent() was partially in the DMA mapping API for some time, but
> > > > >> it was available only on a few architectures (afair ARM, powerpc and avr32).
> > > > >> This caused significant problems for writing unified device drivers or some
> > > > >> device helper modules which were aimed to work on more than one
> > > > >> architecture.
> > > > >>
> > > > >> dma_get_sgtable() is an extension discussed during the Linaro meetings. It
> > > > >> is required to correctly implement buffer sharing between device driver
> > > > >> without hacks or any assumptions about memory layout in the device drivers.
> > > > >>
> > > > >> I have implemented some generic code for both of those two functions,
> > > > >> keeping
> > > > >> in mind that on some hardware architectures (like already mentioned VIVT)
> > > > >> it might be not possible to provide coherent mapping to userspace. It is
> > > > >> perfectly fine for those functions to return an error in such case.
> > > > >
> > > > > It's not possible on VIPT either.  This means that the API is unusable
> > > > > on quite a large number of architectures.  Surely, if we're starting to
> > > > > write drivers using this, we need to fix the API before more people try
> > > > > to use it.
> > > > >
> > > > > For PA-RISC (and all other VIPT, I assume) I need an API which allows me
> > > > > to remap the virtual address of the kernel component (probably using the
> > > > > kmap area) so the user space and kernel space addresses are congruent.
> > > > 
> > > > So what are we gonna do for 3.8? I'd like to get my allmodconfig build
> > > > green again ;-)
> > > > 
> > > > Change the API?
> > > 
> > > Well, if we want the API to work universally, we have to.  As I said,
> > > for VIPT systems, the only coherency mechanism we have is the virtual
> > > address ... we have to fix that in the kernel to be congruent with the
> > > userspace virtual address if we want coherency between the kernel and
> > > userspace.
> > > 
> > > However, if it only needs to work on ARM and x86, it can stay the way it
> > > is and we could just pull it out of the generic core.
> > > 
> > > Who actually wants to use this API, and what for?
> > > 
> > > > Keep the API but do a best-effort fix to unbreak the builds?
> > > >   - Apply my patches that got acks (avr32/blackfin/cris/m68k),
> > > >   - Use static inlines that return -EINVAL for the rest
> > > > (c6x/frv/mn10300/parisc/xtensa).
> > > > I still have a few m68k fixes queued for 3.8, for which I've been postponing the
> > > > pull request to get this sorted out, so I could include the above.
> > > > 
> > > > Any other solution?
> > > 
> > > If it's an API that only works on ARM and x86, there's not much point
> > > pretending it's universal, so we should remove it from the generic arch
> > > code and allow only those architectures which can use it.
> > 
> > There might be a simple solution:  just replace void *cpu_addr with void
> > **cpu_addr in the API.  This is a bit nasty since compilers think that
> > void ** to void * conversion is quite legal, so it would be hard to pick
> > up misuse of this (uint8_t ** would be better).  That way VIPT could
> > remap the kernel pages to a coherent address.  This would probably have
> > to change in the dma_mmap_attr() and dma_ops structures.
> > 
> > All consumers would have to expect cpu_addr might change, but that seems
> > doable.
> 
> Mauro, will this work for you and the v4l guys?  You've got a use
> embedded in 
> 
> drivers/media/v4l2-core/videobuf2-dma-contig.c
> 
> Which I can't tell how extensive it might be.

Also, this implementation is really not done well.

ppc has this:

arch/powerpc/include/asm/dma-mapping.h:#define ARCH_HAS_DMA_MMAP_COHERENT

But no other architecture does.

Nothing is gated on this except pcm_native.c which has this wonderful
snippet:

#ifndef ARCH_HAS_DMA_MMAP_COHERENT
/* This should be defined / handled globally! */
#ifdef CONFIG_ARM
#define ARCH_HAS_DMA_MMAP_COHERENT
#endif
#endif

Since there's no documentation at all, it's hard to tell what's going
on.

However, I'd suggest

     1. Move to an API that is at least capable of implementation on all
        architectures
     2. Utilize ARCH_HAS_DMA_MMAP_COHERENT to fix the compile problems
        within the architectures
     3. Document whatever is chosen so we don't end up in this mess
        again.

James

Re: [Linux-c6x-dev] [PATCH 3/9] c6x: Provide dma_mmap_coherent() and dma_get_sgtable()

[Mauro Carvalho Chehab]

Em Tue, 22 Jan 2013 10:32:07 +0000
James Bottomley <James.Bottomley@HansenPartnership.com> escreveu:

> On Tue, 2013-01-22 at 10:16 +0000, James Bottomley wrote:
> > [adding Mauro and v4l since they're the only non-arm consumers]
> > On Tue, 2013-01-22 at 10:13 +0000, James Bottomley wrote:
> > > On Mon, 2013-01-21 at 22:59 +0000, James Bottomley wrote:
> > > > On Mon, 2013-01-21 at 21:00 +0100, Geert Uytterhoeven wrote:
> > > > > On Tue, Jan 15, 2013 at 5:56 PM, James Bottomley
> > > > > <James.Bottomley@hansenpartnership.com> wrote:
> > > > > > On Tue, 2013-01-15 at 15:07 +0100, Marek Szyprowski wrote:
> > > > > >> On 1/15/2013 10:13 AM, Geert Uytterhoeven wrote:
> > > > > >> > Marek?
> > > > > >> >
> > > > > >> > On Tue, Jan 15, 2013 at 5:16 AM, Vineet Gupta
> > > > > >> > <Vineet.Gupta1@synopsys.com> wrote:
> > > > > >> > > On Monday 14 January 2013 09:07 PM, Mark Salter wrote:
> > > > > >> > >> On Sun, 2013-01-13 at 11:44 +0100, Geert Uytterhoeven wrote:
> > > > > >> > >>> c6x/allmodconfig (assumed):
> > > > > >> > >>>
> > > > > >> > >>> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_mmap’:
> > > > > >> > >>> drivers/media/v4l2-core/videobuf2-dma-contig.c:204: error: implicit declaration of function ‘dma_mmap_coherent’
> > > > > >> > >>> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_get_base_sgt’:
> > > > > >> > >>> drivers/media/v4l2-core/videobuf2-dma-contig.c:387: error: implicit declaration of function ‘dma_get_sgtable’
> > > > > >> > >>>
> > > > > >> > >>> For architectures using dma_map_ops, dma_mmap_coherent() and
> > > > > >> > >>> dma_get_sgtable() are provided in <asm-generic/dma-mapping-common.h>.
> > > > > >> > >>>
> > > > > >> > >>> C6x does not use dma_map_ops, hence it should implement them as inline
> > > > > >> > >>> stubs using dma_common_mmap() and dma_common_get_sgtable().
> > > > > >> > >>>
> > > > > >> > >> So are dma_mmap_coherent() and dma_get_sgtable() part of the DMA API
> > > > > >> > >> now? I don't them in Documentation/DMA*.txt anywhere.
> > > > > >> > >>
> > > > > >> > >> Why does the default dma_common_mmap() for !CONFIG_MMU return an
> > > > > >> > >> error?
> > > > > >> > >>
> > > > > >> > >> Wouldn't it be better to provide default implementations that an arch
> > > > > >> > >> could override rather than having to patch all "no dma_map_ops"
> > > > > >> > >> architectures?
> > > > > >> > >>
> > > > > >> > > Speaking for the still-reviewed ARC Port, I completely agree with Mark.
> > > > > >>
> > > > > >> dma_mmap_coherent() was partially in the DMA mapping API for some time, but
> > > > > >> it was available only on a few architectures (afair ARM, powerpc and avr32).
> > > > > >> This caused significant problems for writing unified device drivers or some
> > > > > >> device helper modules which were aimed to work on more than one
> > > > > >> architecture.
> > > > > >>
> > > > > >> dma_get_sgtable() is an extension discussed during the Linaro meetings. It
> > > > > >> is required to correctly implement buffer sharing between device driver
> > > > > >> without hacks or any assumptions about memory layout in the device drivers.
> > > > > >>
> > > > > >> I have implemented some generic code for both of those two functions,
> > > > > >> keeping
> > > > > >> in mind that on some hardware architectures (like already mentioned VIVT)
> > > > > >> it might be not possible to provide coherent mapping to userspace. It is
> > > > > >> perfectly fine for those functions to return an error in such case.
> > > > > >
> > > > > > It's not possible on VIPT either.  This means that the API is unusable
> > > > > > on quite a large number of architectures.  Surely, if we're starting to
> > > > > > write drivers using this, we need to fix the API before more people try
> > > > > > to use it.
> > > > > >
> > > > > > For PA-RISC (and all other VIPT, I assume) I need an API which allows me
> > > > > > to remap the virtual address of the kernel component (probably using the
> > > > > > kmap area) so the user space and kernel space addresses are congruent.
> > > > > 
> > > > > So what are we gonna do for 3.8? I'd like to get my allmodconfig build
> > > > > green again ;-)
> > > > > 
> > > > > Change the API?
> > > > 
> > > > Well, if we want the API to work universally, we have to.  As I said,
> > > > for VIPT systems, the only coherency mechanism we have is the virtual
> > > > address ... we have to fix that in the kernel to be congruent with the
> > > > userspace virtual address if we want coherency between the kernel and
> > > > userspace.
> > > > 
> > > > However, if it only needs to work on ARM and x86, it can stay the way it
> > > > is and we could just pull it out of the generic core.
> > > > 
> > > > Who actually wants to use this API, and what for?
> > > > 
> > > > > Keep the API but do a best-effort fix to unbreak the builds?
> > > > >   - Apply my patches that got acks (avr32/blackfin/cris/m68k),
> > > > >   - Use static inlines that return -EINVAL for the rest
> > > > > (c6x/frv/mn10300/parisc/xtensa).
> > > > > I still have a few m68k fixes queued for 3.8, for which I've been postponing the
> > > > > pull request to get this sorted out, so I could include the above.
> > > > > 
> > > > > Any other solution?
> > > > 
> > > > If it's an API that only works on ARM and x86, there's not much point
> > > > pretending it's universal, so we should remove it from the generic arch
> > > > code and allow only those architectures which can use it.
> > > 
> > > There might be a simple solution:  just replace void *cpu_addr with void
> > > **cpu_addr in the API.  This is a bit nasty since compilers think that
> > > void ** to void * conversion is quite legal, so it would be hard to pick
> > > up misuse of this (uint8_t ** would be better).  That way VIPT could
> > > remap the kernel pages to a coherent address.  This would probably have
> > > to change in the dma_mmap_attr() and dma_ops structures.
> > > 
> > > All consumers would have to expect cpu_addr might change, but that seems
> > > doable.
> > 
> > Mauro, will this work for you and the v4l guys?  You've got a use
> > embedded in 
> > 
> > drivers/media/v4l2-core/videobuf2-dma-contig.c
> > 
> > Which I can't tell how extensive it might be.
> 
> Also, this implementation is really not done well.
> 
> ppc has this:
> 
> arch/powerpc/include/asm/dma-mapping.h:#define ARCH_HAS_DMA_MMAP_COHERENT
> 
> But no other architecture does.
> 
> Nothing is gated on this except pcm_native.c which has this wonderful
> snippet:
> 
> #ifndef ARCH_HAS_DMA_MMAP_COHERENT
> /* This should be defined / handled globally! */
> #ifdef CONFIG_ARM
> #define ARCH_HAS_DMA_MMAP_COHERENT
> #endif
> #endif
> 
> Since there's no documentation at all, it's hard to tell what's going
> on.
> 
> However, I'd suggest
> 
>      1. Move to an API that is at least capable of implementation on all
>         architectures
>      2. Utilize ARCH_HAS_DMA_MMAP_COHERENT to fix the compile problems
>         within the architectures
>      3. Document whatever is chosen so we don't end up in this mess
>         again.

Agreed.

I remember some efforts were done in the past years moving toward a common
interface, when DMABUF were introduced (and people explicitly mentioned
ARCH_HAS_DMA_MMAP_COHERENT on such discussions), like on this thread:
	http://www.mail-archive.com/linux-media@vger.kernel.org/msg30487.html

While I followed part of that discussion, I didn't actually track the
arm-specific patches. So, I'm not sure why ARCH_HAS_DMA_MMAP_COHERENT
is not defined on arm.

Maybe Tomasz or Marek may have more details.

Regards,
Mauro

Re: [Linux-c6x-dev] [PATCH 3/9] c6x: Provide dma_mmap_coherent() and dma_get_sgtable()

[Mauro Carvalho Chehab]

Hi James,

Em Tue, 22 Jan 2013 10:16:48 +0000
James Bottomley <James.Bottomley@HansenPartnership.com> escreveu:

> [adding Mauro and v4l since they're the only non-arm consumers]
> On Tue, 2013-01-22 at 10:13 +0000, James Bottomley wrote:
> > On Mon, 2013-01-21 at 22:59 +0000, James Bottomley wrote:
> > > On Mon, 2013-01-21 at 21:00 +0100, Geert Uytterhoeven wrote:
> > > > On Tue, Jan 15, 2013 at 5:56 PM, James Bottomley
> > > > <James.Bottomley@hansenpartnership.com> wrote:
> > > > > On Tue, 2013-01-15 at 15:07 +0100, Marek Szyprowski wrote:
> > > > >> On 1/15/2013 10:13 AM, Geert Uytterhoeven wrote:
> > > > >> > Marek?
> > > > >> >
> > > > >> > On Tue, Jan 15, 2013 at 5:16 AM, Vineet Gupta
> > > > >> > <Vineet.Gupta1@synopsys.com> wrote:
> > > > >> > > On Monday 14 January 2013 09:07 PM, Mark Salter wrote:
> > > > >> > >> On Sun, 2013-01-13 at 11:44 +0100, Geert Uytterhoeven wrote:
> > > > >> > >>> c6x/allmodconfig (assumed):
> > > > >> > >>>
> > > > >> > >>> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_mmap’:
> > > > >> > >>> drivers/media/v4l2-core/videobuf2-dma-contig.c:204: error: implicit declaration of function ‘dma_mmap_coherent’
> > > > >> > >>> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_get_base_sgt’:
> > > > >> > >>> drivers/media/v4l2-core/videobuf2-dma-contig.c:387: error: implicit declaration of function ‘dma_get_sgtable’
> > > > >> > >>>
> > > > >> > >>> For architectures using dma_map_ops, dma_mmap_coherent() and
> > > > >> > >>> dma_get_sgtable() are provided in <asm-generic/dma-mapping-common.h>.
> > > > >> > >>>
> > > > >> > >>> C6x does not use dma_map_ops, hence it should implement them as inline
> > > > >> > >>> stubs using dma_common_mmap() and dma_common_get_sgtable().
> > > > >> > >>>
> > > > >> > >> So are dma_mmap_coherent() and dma_get_sgtable() part of the DMA API
> > > > >> > >> now? I don't them in Documentation/DMA*.txt anywhere.
> > > > >> > >>
> > > > >> > >> Why does the default dma_common_mmap() for !CONFIG_MMU return an
> > > > >> > >> error?
> > > > >> > >>
> > > > >> > >> Wouldn't it be better to provide default implementations that an arch
> > > > >> > >> could override rather than having to patch all "no dma_map_ops"
> > > > >> > >> architectures?
> > > > >> > >>
> > > > >> > > Speaking for the still-reviewed ARC Port, I completely agree with Mark.
> > > > >>
> > > > >> dma_mmap_coherent() was partially in the DMA mapping API for some time, but
> > > > >> it was available only on a few architectures (afair ARM, powerpc and avr32).
> > > > >> This caused significant problems for writing unified device drivers or some
> > > > >> device helper modules which were aimed to work on more than one
> > > > >> architecture.
> > > > >>
> > > > >> dma_get_sgtable() is an extension discussed during the Linaro meetings. It
> > > > >> is required to correctly implement buffer sharing between device driver
> > > > >> without hacks or any assumptions about memory layout in the device drivers.
> > > > >>
> > > > >> I have implemented some generic code for both of those two functions,
> > > > >> keeping
> > > > >> in mind that on some hardware architectures (like already mentioned VIVT)
> > > > >> it might be not possible to provide coherent mapping to userspace. It is
> > > > >> perfectly fine for those functions to return an error in such case.
> > > > >
> > > > > It's not possible on VIPT either.  This means that the API is unusable
> > > > > on quite a large number of architectures.  Surely, if we're starting to
> > > > > write drivers using this, we need to fix the API before more people try
> > > > > to use it.
> > > > >
> > > > > For PA-RISC (and all other VIPT, I assume) I need an API which allows me
> > > > > to remap the virtual address of the kernel component (probably using the
> > > > > kmap area) so the user space and kernel space addresses are congruent.
> > > > 
> > > > So what are we gonna do for 3.8? I'd like to get my allmodconfig build
> > > > green again ;-)
> > > > 
> > > > Change the API?
> > > 
> > > Well, if we want the API to work universally, we have to.  As I said,
> > > for VIPT systems, the only coherency mechanism we have is the virtual
> > > address ... we have to fix that in the kernel to be congruent with the
> > > userspace virtual address if we want coherency between the kernel and
> > > userspace.
> > > 
> > > However, if it only needs to work on ARM and x86, it can stay the way it
> > > is and we could just pull it out of the generic core.
> > > 
> > > Who actually wants to use this API, and what for?
> > > 
> > > > Keep the API but do a best-effort fix to unbreak the builds?
> > > >   - Apply my patches that got acks (avr32/blackfin/cris/m68k),
> > > >   - Use static inlines that return -EINVAL for the rest
> > > > (c6x/frv/mn10300/parisc/xtensa).
> > > > I still have a few m68k fixes queued for 3.8, for which I've been postponing the
> > > > pull request to get this sorted out, so I could include the above.
> > > > 
> > > > Any other solution?
> > > 
> > > If it's an API that only works on ARM and x86, there's not much point
> > > pretending it's universal, so we should remove it from the generic arch
> > > code and allow only those architectures which can use it.
> > 
> > There might be a simple solution:  just replace void *cpu_addr with void
> > **cpu_addr in the API.  This is a bit nasty since compilers think that
> > void ** to void * conversion is quite legal, so it would be hard to pick
> > up misuse of this (uint8_t ** would be better).  That way VIPT could
> > remap the kernel pages to a coherent address.  This would probably have
> > to change in the dma_mmap_attr() and dma_ops structures.
> > 
> > All consumers would have to expect cpu_addr might change, but that seems
> > doable.
> 
> Mauro, will this work for you and the v4l guys?  You've got a use
> embedded in 
> 
> drivers/media/v4l2-core/videobuf2-dma-contig.c
> 
> Which I can't tell how extensive it might be.

Proper handling of video stream is a hard task and can be very tricky.
In order to avoid duplicated stuff, the video streaming buffer handling,
implemented by videobuf2 (aka VB2), is part of the media core, 
and it was written to be as much architecture-independent as possible.
There is a previous implementation of it, and we're gradually changing
the drivers to VB2, as it is more robust and fixes some known issues
with the legacy one.

The VB2 core has one generic driver, completely independent on the memory
model, with provides the interface for the V4L drivers, and 3 memory-specific
ones that match the DMA needs for that architecture and device:

	- one for vmalloc'ed memory - used by USB based devices where double
buffering is needed in order to strip USB protocol packets from the video
payload;
	- one for DMA scatter/gather - most PCI/PCIe devices use DMA
scatter/gather;

	- one for DMA where a continuous memory is required to fit an entire
video image (videobuf2-dma-contig). This is generally required by the 
embedded drivers, as either the video capture IP block and/or the DMA
engine can't handle scatter/gather pages.

So, while the current drivers that use videobuf2-dma-contig are (mainly)
for x86 and arm, the better is for videobuf2-dma-contig to be able of
working with all possible architectures, as I'm sure that sooner or later
a video device will be needed at the other architectures, and this driver
will be needed there.

That's said, changing from *cpu_addr to **cpu_addr sounds ok on my
eyes, but Pawel and Marek knows a lot more about videobuf2 than me,
as they wrote this driver. So, they can contribute more with the
interface specifics.

>From my side, I don't care that much with the needed API changes
for the DMA interfaces to be generic, provided that the v4l2 core
compilation won't depend on any CONFIG_ARCH* Kconfig symbol, and
that all architectures implement the DMA API interfaces at the same
way, so, there will be no need to add #if's inside the videobuf2 code[1].

Regards,
Mauro

[1] Unfortunately, there's currently one such #if's there, in order to
support (sub)architectures where there's no MMU.

Re: [Linux-c6x-dev] [PATCH 3/9] c6x: Provide dma_mmap_coherent() and dma_get_sgtable()

[Mauro Carvalho Chehab]

Hi James,

Em Tue, 22 Jan 2013 10:16:48 +0000
James Bottomley <James.Bottomley@HansenPartnership.com> escreveu:

> [adding Mauro and v4l since they're the only non-arm consumers]
> On Tue, 2013-01-22 at 10:13 +0000, James Bottomley wrote:
> > On Mon, 2013-01-21 at 22:59 +0000, James Bottomley wrote:
> > > On Mon, 2013-01-21 at 21:00 +0100, Geert Uytterhoeven wrote:
> > > > On Tue, Jan 15, 2013 at 5:56 PM, James Bottomley
> > > > <James.Bottomley@hansenpartnership.com> wrote:
> > > > > On Tue, 2013-01-15 at 15:07 +0100, Marek Szyprowski wrote:
> > > > >> On 1/15/2013 10:13 AM, Geert Uytterhoeven wrote:
> > > > >> > Marek?
> > > > >> >
> > > > >> > On Tue, Jan 15, 2013 at 5:16 AM, Vineet Gupta
> > > > >> > <Vineet.Gupta1@synopsys.com> wrote:
> > > > >> > > On Monday 14 January 2013 09:07 PM, Mark Salter wrote:
> > > > >> > >> On Sun, 2013-01-13 at 11:44 +0100, Geert Uytterhoeven wrote:
> > > > >> > >>> c6x/allmodconfig (assumed):
> > > > >> > >>>
> > > > >> > >>> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_mmap’:
> > > > >> > >>> drivers/media/v4l2-core/videobuf2-dma-contig.c:204: error: implicit declaration of function ‘dma_mmap_coherent’
> > > > >> > >>> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_get_base_sgt’:
> > > > >> > >>> drivers/media/v4l2-core/videobuf2-dma-contig.c:387: error: implicit declaration of function ‘dma_get_sgtable’
> > > > >> > >>>
> > > > >> > >>> For architectures using dma_map_ops, dma_mmap_coherent() and
> > > > >> > >>> dma_get_sgtable() are provided in <asm-generic/dma-mapping-common.h>.
> > > > >> > >>>
> > > > >> > >>> C6x does not use dma_map_ops, hence it should implement them as inline
> > > > >> > >>> stubs using dma_common_mmap() and dma_common_get_sgtable().
> > > > >> > >>>
> > > > >> > >> So are dma_mmap_coherent() and dma_get_sgtable() part of the DMA API
> > > > >> > >> now? I don't them in Documentation/DMA*.txt anywhere.
> > > > >> > >>
> > > > >> > >> Why does the default dma_common_mmap() for !CONFIG_MMU return an
> > > > >> > >> error?
> > > > >> > >>
> > > > >> > >> Wouldn't it be better to provide default implementations that an arch
> > > > >> > >> could override rather than having to patch all "no dma_map_ops"
> > > > >> > >> architectures?
> > > > >> > >>
> > > > >> > > Speaking for the still-reviewed ARC Port, I completely agree with Mark.
> > > > >>
> > > > >> dma_mmap_coherent() was partially in the DMA mapping API for some time, but
> > > > >> it was available only on a few architectures (afair ARM, powerpc and avr32).
> > > > >> This caused significant problems for writing unified device drivers or some
> > > > >> device helper modules which were aimed to work on more than one
> > > > >> architecture.
> > > > >>
> > > > >> dma_get_sgtable() is an extension discussed during the Linaro meetings. It
> > > > >> is required to correctly implement buffer sharing between device driver
> > > > >> without hacks or any assumptions about memory layout in the device drivers.
> > > > >>
> > > > >> I have implemented some generic code for both of those two functions,
> > > > >> keeping
> > > > >> in mind that on some hardware architectures (like already mentioned VIVT)
> > > > >> it might be not possible to provide coherent mapping to userspace. It is
> > > > >> perfectly fine for those functions to return an error in such case.
> > > > >
> > > > > It's not possible on VIPT either.  This means that the API is unusable
> > > > > on quite a large number of architectures.  Surely, if we're starting to
> > > > > write drivers using this, we need to fix the API before more people try
> > > > > to use it.
> > > > >
> > > > > For PA-RISC (and all other VIPT, I assume) I need an API which allows me
> > > > > to remap the virtual address of the kernel component (probably using the
> > > > > kmap area) so the user space and kernel space addresses are congruent.
> > > > 
> > > > So what are we gonna do for 3.8? I'd like to get my allmodconfig build
> > > > green again ;-)
> > > > 
> > > > Change the API?
> > > 
> > > Well, if we want the API to work universally, we have to.  As I said,
> > > for VIPT systems, the only coherency mechanism we have is the virtual
> > > address ... we have to fix that in the kernel to be congruent with the
> > > userspace virtual address if we want coherency between the kernel and
> > > userspace.
> > > 
> > > However, if it only needs to work on ARM and x86, it can stay the way it
> > > is and we could just pull it out of the generic core.
> > > 
> > > Who actually wants to use this API, and what for?
> > > 
> > > > Keep the API but do a best-effort fix to unbreak the builds?
> > > >   - Apply my patches that got acks (avr32/blackfin/cris/m68k),
> > > >   - Use static inlines that return -EINVAL for the rest
> > > > (c6x/frv/mn10300/parisc/xtensa).
> > > > I still have a few m68k fixes queued for 3.8, for which I've been postponing the
> > > > pull request to get this sorted out, so I could include the above.
> > > > 
> > > > Any other solution?
> > > 
> > > If it's an API that only works on ARM and x86, there's not much point
> > > pretending it's universal, so we should remove it from the generic arch
> > > code and allow only those architectures which can use it.
> > 
> > There might be a simple solution:  just replace void *cpu_addr with void
> > **cpu_addr in the API.  This is a bit nasty since compilers think that
> > void ** to void * conversion is quite legal, so it would be hard to pick
> > up misuse of this (uint8_t ** would be better).  That way VIPT could
> > remap the kernel pages to a coherent address.  This would probably have
> > to change in the dma_mmap_attr() and dma_ops structures.
> > 
> > All consumers would have to expect cpu_addr might change, but that seems
> > doable.
> 
> Mauro, will this work for you and the v4l guys?  You've got a use
> embedded in 
> 
> drivers/media/v4l2-core/videobuf2-dma-contig.c
> 
> Which I can't tell how extensive it might be.

Proper handling of video stream is a hard task and can be very tricky.
In order to avoid duplicated stuff, the video streaming buffer handling,
implemented by videobuf2 (aka VB2), is part of the media core, 
and it was written to be as much architecture-independent as possible.
There is a previous implementation of it, and we're gradually changing
the drivers to VB2, as it is more robust and fixes some known issues
with the legacy one.

The VB2 core has one generic driver, completely independent on the memory
model, with provides the interface for the V4L drivers, and 3 memory-specific
ones that match the DMA needs for that architecture and device:

	- one for vmalloc'ed memory - used by USB based devices where double
buffering is needed in order to strip USB protocol packets from the video
payload;
	- one for DMA scatter/gather - most PCI/PCIe devices use DMA
scatter/gather;

	- one for DMA where a continuous memory is required to fit an entire
video image (videobuf2-dma-contig). This is generally required by the 
embedded drivers, as either the video capture IP block and/or the DMA
engine can't handle scatter/gather pages.

So, while the current drivers that use videobuf2-dma-contig are (mainly)
for x86 and arm, the better is for videobuf2-dma-contig to be able of
working with all possible architectures, as I'm sure that sooner or later
a video device will be needed at the other architectures, and this driver
will be needed there.

That's said, changing from *cpu_addr to **cpu_addr sounds ok on my
eyes, but Pawel and Marek knows a lot more about videobuf2 than me,
as they wrote this driver. So, they can contribute more with the
interface specifics.

From my side, I don't care that much with the needed API changes
for the DMA interfaces to be generic, provided that the v4l2 core
compilation won't depend on any CONFIG_ARCH* Kconfig symbol, and
that all architectures implement the DMA API interfaces at the same
way, so, there will be no need to add #if's inside the videobuf2 code[1].

Regards,
Mauro

[1] Unfortunately, there's currently one such #if's there, in order to
support (sub)architectures where there's no MMU.

Re: [Linux-c6x-dev] [PATCH 3/9] c6x: Provide dma_mmap_coherent() and dma_get_sgtable()

[Marek Szyprowski]

Hello,

On 1/22/2013 11:13 AM, James Bottomley wrote:
> On Mon, 2013-01-21 at 22:59 +0000, James Bottomley wrote:
> > On Mon, 2013-01-21 at 21:00 +0100, Geert Uytterhoeven wrote:
> > > On Tue, Jan 15, 2013 at 5:56 PM, James Bottomley
> > > <James.Bottomley@hansenpartnership.com> wrote:
> > > > On Tue, 2013-01-15 at 15:07 +0100, Marek Szyprowski wrote:
> > > >> On 1/15/2013 10:13 AM, Geert Uytterhoeven wrote:
> > > >> > Marek?
> > > >> >
> > > >> > On Tue, Jan 15, 2013 at 5:16 AM, Vineet Gupta
> > > >> > <Vineet.Gupta1@synopsys.com> wrote:
> > > >> > > On Monday 14 January 2013 09:07 PM, Mark Salter wrote:
> > > >> > >> On Sun, 2013-01-13 at 11:44 +0100, Geert Uytterhoeven wrote:
> > > >> > >>> c6x/allmodconfig (assumed):
> > > >> > >>>
> > > >> > >>> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_mmap’:
> > > >> > >>> drivers/media/v4l2-core/videobuf2-dma-contig.c:204: error: implicit declaration of function ‘dma_mmap_coherent’
> > > >> > >>> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_get_base_sgt’:
> > > >> > >>> drivers/media/v4l2-core/videobuf2-dma-contig.c:387: error: implicit declaration of function ‘dma_get_sgtable’
> > > >> > >>>
> > > >> > >>> For architectures using dma_map_ops, dma_mmap_coherent() and
> > > >> > >>> dma_get_sgtable() are provided in <asm-generic/dma-mapping-common.h>.
> > > >> > >>>
> > > >> > >>> C6x does not use dma_map_ops, hence it should implement them as inline
> > > >> > >>> stubs using dma_common_mmap() and dma_common_get_sgtable().
> > > >> > >>>
> > > >> > >> So are dma_mmap_coherent() and dma_get_sgtable() part of the DMA API
> > > >> > >> now? I don't them in Documentation/DMA*.txt anywhere.
> > > >> > >>
> > > >> > >> Why does the default dma_common_mmap() for !CONFIG_MMU return an
> > > >> > >> error?
> > > >> > >>
> > > >> > >> Wouldn't it be better to provide default implementations that an arch
> > > >> > >> could override rather than having to patch all "no dma_map_ops"
> > > >> > >> architectures?
> > > >> > >>
> > > >> > > Speaking for the still-reviewed ARC Port, I completely agree with Mark.
> > > >>
> > > >> dma_mmap_coherent() was partially in the DMA mapping API for some time, but
> > > >> it was available only on a few architectures (afair ARM, powerpc and avr32).
> > > >> This caused significant problems for writing unified device drivers or some
> > > >> device helper modules which were aimed to work on more than one
> > > >> architecture.
> > > >>
> > > >> dma_get_sgtable() is an extension discussed during the Linaro meetings. It
> > > >> is required to correctly implement buffer sharing between device driver
> > > >> without hacks or any assumptions about memory layout in the device drivers.
> > > >>
> > > >> I have implemented some generic code for both of those two functions,
> > > >> keeping
> > > >> in mind that on some hardware architectures (like already mentioned VIVT)
> > > >> it might be not possible to provide coherent mapping to userspace. It is
> > > >> perfectly fine for those functions to return an error in such case.
> > > >
> > > > It's not possible on VIPT either.  This means that the API is unusable
> > > > on quite a large number of architectures.  Surely, if we're starting to
> > > > write drivers using this, we need to fix the API before more people try
> > > > to use it.
> > > >
> > > > For PA-RISC (and all other VIPT, I assume) I need an API which allows me
> > > > to remap the virtual address of the kernel component (probably using the
> > > > kmap area) so the user space and kernel space addresses are congruent.
> > >
> > > So what are we gonna do for 3.8? I'd like to get my allmodconfig build
> > > green again ;-)
> > >
> > > Change the API?
> >
> > Well, if we want the API to work universally, we have to.  As I said,
> > for VIPT systems, the only coherency mechanism we have is the virtual
> > address ... we have to fix that in the kernel to be congruent with the
> > userspace virtual address if we want coherency between the kernel and
> > userspace.
> >
> > However, if it only needs to work on ARM and x86, it can stay the way it
> > is and we could just pull it out of the generic core.
> >
> > Who actually wants to use this API, and what for?
> >
> > > Keep the API but do a best-effort fix to unbreak the builds?
> > >   - Apply my patches that got acks (avr32/blackfin/cris/m68k),
> > >   - Use static inlines that return -EINVAL for the rest
> > > (c6x/frv/mn10300/parisc/xtensa).
> > > I still have a few m68k fixes queued for 3.8, for which I've been postponing the
> > > pull request to get this sorted out, so I could include the above.
> > >
> > > Any other solution?
> >
> > If it's an API that only works on ARM and x86, there's not much point
> > pretending it's universal, so we should remove it from the generic arch
> > code and allow only those architectures which can use it.
>
> There might be a simple solution:  just replace void *cpu_addr with void
> **cpu_addr in the API.  This is a bit nasty since compilers think that
> void ** to void * conversion is quite legal, so it would be hard to pick
> up misuse of this (uint8_t ** would be better).  That way VIPT could
> remap the kernel pages to a coherent address.  This would probably have
> to change in the dma_mmap_attr() and dma_ops structures.

Frankly, I don't get this change.

The call sequence for the current api is following:

/* driver initialization */
dma_addr_t dmah;
void *cpu_ptr = dma_alloc_coherent(dev, size, &dmah, GFP_KERNEL);

/* mmap implementation */

return dma_mmap_coherent(dev, vma, cpu_addr, dmah, size);

/* cleanup */
dma_free_coherent(dev, size, cpu_addr, dmah);

I assume you want to let dma_mmap_coherent to change the cpu_addr returned
by dma_alloc_coherent()? I really don't see how this might help.


Best regards
-- 
Marek Szyprowski
Samsung Poland R&D Center

Re: [Linux-c6x-dev] [PATCH 3/9] c6x: Provide dma_mmap_coherent() and dma_get_sgtable()

[James Bottomley]

On Tue, 2013-01-22 at 11:33 +0100, Marek Szyprowski wrote:
> Hello,
> 
> On 1/22/2013 11:13 AM, James Bottomley wrote:
> > On Mon, 2013-01-21 at 22:59 +0000, James Bottomley wrote:
> > > On Mon, 2013-01-21 at 21:00 +0100, Geert Uytterhoeven wrote:
> > > > On Tue, Jan 15, 2013 at 5:56 PM, James Bottomley
> > > > <James.Bottomley@hansenpartnership.com> wrote:
> > > > > On Tue, 2013-01-15 at 15:07 +0100, Marek Szyprowski wrote:
> > > > >> On 1/15/2013 10:13 AM, Geert Uytterhoeven wrote:
> > > > >> > Marek?
> > > > >> >
> > > > >> > On Tue, Jan 15, 2013 at 5:16 AM, Vineet Gupta
> > > > >> > <Vineet.Gupta1@synopsys.com> wrote:
> > > > >> > > On Monday 14 January 2013 09:07 PM, Mark Salter wrote:
> > > > >> > >> On Sun, 2013-01-13 at 11:44 +0100, Geert Uytterhoeven wrote:
> > > > >> > >>> c6x/allmodconfig (assumed):
> > > > >> > >>>
> > > > >> > >>> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_mmap’:
> > > > >> > >>> drivers/media/v4l2-core/videobuf2-dma-contig.c:204: error: implicit declaration of function ‘dma_mmap_coherent’
> > > > >> > >>> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_get_base_sgt’:
> > > > >> > >>> drivers/media/v4l2-core/videobuf2-dma-contig.c:387: error: implicit declaration of function ‘dma_get_sgtable’
> > > > >> > >>>
> > > > >> > >>> For architectures using dma_map_ops, dma_mmap_coherent() and
> > > > >> > >>> dma_get_sgtable() are provided in <asm-generic/dma-mapping-common.h>.
> > > > >> > >>>
> > > > >> > >>> C6x does not use dma_map_ops, hence it should implement them as inline
> > > > >> > >>> stubs using dma_common_mmap() and dma_common_get_sgtable().
> > > > >> > >>>
> > > > >> > >> So are dma_mmap_coherent() and dma_get_sgtable() part of the DMA API
> > > > >> > >> now? I don't them in Documentation/DMA*.txt anywhere.
> > > > >> > >>
> > > > >> > >> Why does the default dma_common_mmap() for !CONFIG_MMU return an
> > > > >> > >> error?
> > > > >> > >>
> > > > >> > >> Wouldn't it be better to provide default implementations that an arch
> > > > >> > >> could override rather than having to patch all "no dma_map_ops"
> > > > >> > >> architectures?
> > > > >> > >>
> > > > >> > > Speaking for the still-reviewed ARC Port, I completely agree with Mark.
> > > > >>
> > > > >> dma_mmap_coherent() was partially in the DMA mapping API for some time, but
> > > > >> it was available only on a few architectures (afair ARM, powerpc and avr32).
> > > > >> This caused significant problems for writing unified device drivers or some
> > > > >> device helper modules which were aimed to work on more than one
> > > > >> architecture.
> > > > >>
> > > > >> dma_get_sgtable() is an extension discussed during the Linaro meetings. It
> > > > >> is required to correctly implement buffer sharing between device driver
> > > > >> without hacks or any assumptions about memory layout in the device drivers.
> > > > >>
> > > > >> I have implemented some generic code for both of those two functions,
> > > > >> keeping
> > > > >> in mind that on some hardware architectures (like already mentioned VIVT)
> > > > >> it might be not possible to provide coherent mapping to userspace. It is
> > > > >> perfectly fine for those functions to return an error in such case.
> > > > >
> > > > > It's not possible on VIPT either.  This means that the API is unusable
> > > > > on quite a large number of architectures.  Surely, if we're starting to
> > > > > write drivers using this, we need to fix the API before more people try
> > > > > to use it.
> > > > >
> > > > > For PA-RISC (and all other VIPT, I assume) I need an API which allows me
> > > > > to remap the virtual address of the kernel component (probably using the
> > > > > kmap area) so the user space and kernel space addresses are congruent.
> > > >
> > > > So what are we gonna do for 3.8? I'd like to get my allmodconfig build
> > > > green again ;-)
> > > >
> > > > Change the API?
> > >
> > > Well, if we want the API to work universally, we have to.  As I said,
> > > for VIPT systems, the only coherency mechanism we have is the virtual
> > > address ... we have to fix that in the kernel to be congruent with the
> > > userspace virtual address if we want coherency between the kernel and
> > > userspace.
> > >
> > > However, if it only needs to work on ARM and x86, it can stay the way it
> > > is and we could just pull it out of the generic core.
> > >
> > > Who actually wants to use this API, and what for?
> > >
> > > > Keep the API but do a best-effort fix to unbreak the builds?
> > > >   - Apply my patches that got acks (avr32/blackfin/cris/m68k),
> > > >   - Use static inlines that return -EINVAL for the rest
> > > > (c6x/frv/mn10300/parisc/xtensa).
> > > > I still have a few m68k fixes queued for 3.8, for which I've been postponing the
> > > > pull request to get this sorted out, so I could include the above.
> > > >
> > > > Any other solution?
> > >
> > > If it's an API that only works on ARM and x86, there's not much point
> > > pretending it's universal, so we should remove it from the generic arch
> > > code and allow only those architectures which can use it.
> >
> > There might be a simple solution:  just replace void *cpu_addr with void
> > **cpu_addr in the API.  This is a bit nasty since compilers think that
> > void ** to void * conversion is quite legal, so it would be hard to pick
> > up misuse of this (uint8_t ** would be better).  That way VIPT could
> > remap the kernel pages to a coherent address.  This would probably have
> > to change in the dma_mmap_attr() and dma_ops structures.
> 
> Frankly, I don't get this change.
> 
> The call sequence for the current api is following:
> 
> /* driver initialization */
> dma_addr_t dmah;
> void *cpu_ptr = dma_alloc_coherent(dev, size, &dmah, GFP_KERNEL);
> 
> /* mmap implementation */
> 
> return dma_mmap_coherent(dev, vma, cpu_addr, dmah, size);
> 
> /* cleanup */
> dma_free_coherent(dev, size, cpu_addr, dmah);
> 
> I assume you want to let dma_mmap_coherent to change the cpu_addr returned
> by dma_alloc_coherent()? I really don't see how this might help.

That's right.

Because on PA-RISC, I can push the current cpu_addr TLB out of the MMU,
do a remap on the physical page to a kernel virtual address which is
congruent with that page in the VMA and we have automatic kernel to user
space coherency without having to take the performance hit of marking
the page as uncacheable and actually getting it to work on both non-U
and no-MMU architectures.  I think we can even update the offset mapping
to generate a fault if someone then tries to address the page via its
old offset address.

I don't quite get the concern: this is what we do today for vmap and
vmalloc.

James

Re: [Linux-c6x-dev] [PATCH 3/9] c6x: Provide dma_mmap_coherent() and dma_get_sgtable()

[Marek Szyprowski]

On 1/22/2013 11:13 AM, James Bottomley wrote:
> On Mon, 2013-01-21 at 22:59 +0000, James Bottomley wrote:
> > On Mon, 2013-01-21 at 21:00 +0100, Geert Uytterhoeven wrote:
> > > On Tue, Jan 15, 2013 at 5:56 PM, James Bottomley
> > > <James.Bottomley@hansenpartnership.com> wrote:
> > > > On Tue, 2013-01-15 at 15:07 +0100, Marek Szyprowski wrote:
> > > >> On 1/15/2013 10:13 AM, Geert Uytterhoeven wrote:
> > > >> > Marek?
> > > >> >
> > > >> > On Tue, Jan 15, 2013 at 5:16 AM, Vineet Gupta
> > > >> > <Vineet.Gupta1@synopsys.com> wrote:
> > > >> > > On Monday 14 January 2013 09:07 PM, Mark Salter wrote:
> > > >> > >> On Sun, 2013-01-13 at 11:44 +0100, Geert Uytterhoeven wrote:
> > > >> > >>> c6x/allmodconfig (assumed):
> > > >> > >>>
> > > >> > >>> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_mmap’:
> > > >> > >>> drivers/media/v4l2-core/videobuf2-dma-contig.c:204: error: implicit declaration of function ‘dma_mmap_coherent’
> > > >> > >>> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_get_base_sgt’:
> > > >> > >>> drivers/media/v4l2-core/videobuf2-dma-contig.c:387: error: implicit declaration of function ‘dma_get_sgtable’
> > > >> > >>>
> > > >> > >>> For architectures using dma_map_ops, dma_mmap_coherent() and
> > > >> > >>> dma_get_sgtable() are provided in <asm-generic/dma-mapping-common.h>.
> > > >> > >>>
> > > >> > >>> C6x does not use dma_map_ops, hence it should implement them as inline
> > > >> > >>> stubs using dma_common_mmap() and dma_common_get_sgtable().
> > > >> > >>>
> > > >> > >> So are dma_mmap_coherent() and dma_get_sgtable() part of the DMA API
> > > >> > >> now? I don't them in Documentation/DMA*.txt anywhere.
> > > >> > >>
> > > >> > >> Why does the default dma_common_mmap() for !CONFIG_MMU return an
> > > >> > >> error?
> > > >> > >>
> > > >> > >> Wouldn't it be better to provide default implementations that an arch
> > > >> > >> could override rather than having to patch all "no dma_map_ops"
> > > >> > >> architectures?
> > > >> > >>
> > > >> > > Speaking for the still-reviewed ARC Port, I completely agree with Mark.
> > > >>
> > > >> dma_mmap_coherent() was partially in the DMA mapping API for some time, but
> > > >> it was available only on a few architectures (afair ARM, powerpc and avr32).
> > > >> This caused significant problems for writing unified device drivers or some
> > > >> device helper modules which were aimed to work on more than one
> > > >> architecture.
> > > >>
> > > >> dma_get_sgtable() is an extension discussed during the Linaro meetings. It
> > > >> is required to correctly implement buffer sharing between device driver
> > > >> without hacks or any assumptions about memory layout in the device drivers.
> > > >>
> > > >> I have implemented some generic code for both of those two functions,
> > > >> keeping
> > > >> in mind that on some hardware architectures (like already mentioned VIVT)
> > > >> it might be not possible to provide coherent mapping to userspace. It is
> > > >> perfectly fine for those functions to return an error in such case.
> > > >
> > > > It's not possible on VIPT either.  This means that the API is unusable
> > > > on quite a large number of architectures.  Surely, if we're starting to
> > > > write drivers using this, we need to fix the API before more people try
> > > > to use it.
> > > >
> > > > For PA-RISC (and all other VIPT, I assume) I need an API which allows me
> > > > to remap the virtual address of the kernel component (probably using the
> > > > kmap area) so the user space and kernel space addresses are congruent.
> > >
> > > So what are we gonna do for 3.8? I'd like to get my allmodconfig build
> > > green again ;-)
> > >
> > > Change the API?
> >
> > Well, if we want the API to work universally, we have to.  As I said,
> > for VIPT systems, the only coherency mechanism we have is the virtual
> > address ... we have to fix that in the kernel to be congruent with the
> > userspace virtual address if we want coherency between the kernel and
> > userspace.
> >
> > However, if it only needs to work on ARM and x86, it can stay the way it
> > is and we could just pull it out of the generic core.
> >
> > Who actually wants to use this API, and what for?
> >
> > > Keep the API but do a best-effort fix to unbreak the builds?
> > >   - Apply my patches that got acks (avr32/blackfin/cris/m68k),
> > >   - Use static inlines that return -EINVAL for the rest
> > > (c6x/frv/mn10300/parisc/xtensa).
> > > I still have a few m68k fixes queued for 3.8, for which I've been postponing the
> > > pull request to get this sorted out, so I could include the above.
> > >
> > > Any other solution?
> >
> > If it's an API that only works on ARM and x86, there's not much point
> > pretending it's universal, so we should remove it from the generic arch
> > code and allow only those architectures which can use it.
>
> There might be a simple solution:  just replace void *cpu_addr with void
> **cpu_addr in the API.  This is a bit nasty since compilers think that
> void ** to void * conversion is quite legal, so it would be hard to pick
> up misuse of this (uint8_t ** would be better).  That way VIPT could
> remap the kernel pages to a coherent address.  This would probably have
> to change in the dma_mmap_attr() and dma_ops structures.
>
> All consumers would have to expect cpu_addr might change, but that seems
> doable.

I still don't get how this can help having a coherent buffer between DMA
(devices) and CPU (kernel and user space mappings). The main purpose of
the dma_mmap_coherent() call is to provide a common API for mapping a
coherent buffer between DMA (device) and userspace. It is not about
creating a coherent mapping for sharing memory between userspace and
kernel space.

Best regards
-- 
Marek Szyprowski
Samsung Poland R&D Center

Re: [Linux-c6x-dev] [PATCH 3/9] c6x: Provide dma_mmap_coherent() and dma_get_sgtable()

[James Bottomley]

On Wed, 2013-01-23 at 10:47 +0100, Marek Szyprowski wrote:
> On 1/22/2013 11:13 AM, James Bottomley wrote:
> > There might be a simple solution:  just replace void *cpu_addr with void
> > **cpu_addr in the API.  This is a bit nasty since compilers think that
> > void ** to void * conversion is quite legal, so it would be hard to pick
> > up misuse of this (uint8_t ** would be better).  That way VIPT could
> > remap the kernel pages to a coherent address.  This would probably have
> > to change in the dma_mmap_attr() and dma_ops structures.
> >
> > All consumers would have to expect cpu_addr might change, but that seems
> > doable.
> 
> I still don't get how this can help having a coherent buffer between DMA
> (devices) and CPU (kernel and user space mappings). The main purpose of
> the dma_mmap_coherent() call is to provide a common API for mapping a
> coherent buffer between DMA (device) and userspace. It is not about
> creating a coherent mapping for sharing memory between userspace and
> kernel space.

OK, so I assume you don't understand how VIPT architectures work?

On a VIPT architecture, the CPU cache is indexed by the virtual address
but tagged by the physical address.  This means that when an address
comes into the CPU, we can do simultaneous lookups in the cache and the
TLB (by the virtual address).  The cache doesn't have the full address
bits of the index, so it usually only looks up the lowest n bits.  The
value of n gives the congruency of the cache (sometimes called the
colour of the cache lines).  The cache usually produces a number of
possible lines depending on its associativity and the TLB lookup
produces the physical address.  We can now sweep through the cache lines
and if a physical address tag matches, return the answer from the cache
instead of having to consult main memory.  This gives a speed advantage
over PIPT (Physically Indexed Physically Tagged) caches because on PIPT
the cache lookup can only occur *after* the TLB lookup instead of
concurrently with.

As an aside, in practise every PIPT CPU actually has a VIPT cache,
purely because of the speed angle.  The trick to making it all work is
to shrink n so that n <= PAGE_SIZE_BITS and increase the associativity.
This means you can get VIPT speed without producing the aliasing
effects.

Coherence is achieved in VIPT CPUs when two mapping addresses for the
same physical page, say v1 and v2 are congruent i.e. (v1 & ((1<<n)-1))
== (v2 & ((1<<n)-1)).  For such mappings, the same cache line is used.
This is why we have an architecture override for
arch_get_unmapped_area(). we use this to ensure all shareable mappings
for all userspace process are at congruent addresses, so we don't get
any aliasing problems in shared VMAs.

Aliasing happens when v1 & ((1<<n)-1)) != (v2 & ((1<<n)-1)) where v1 and
v2 are virtual addresses of the same physical page.  Now that page has
*two* cache copies and you have to be very careful to resolve the
aliases.  This, unfortunately, is the default scenario for kernel
virtual and user virtual addresses because all kernel pages are offset
mapped which is why we have to be annoyingly careful about flushing
kernel pages before we hand control back to userspace.

As you can see, we can only share a mapping between the kernel and user
space without all the aliasing problems if the address is congruent.

dma_mmap_coherent() seeks to be coherent from the device to the kernel
to userspace.  On PARISC, we fix device coherency by loading a coherence
index into our IOMMU (it effectively tells the IOMMU the virtual alias
for the physical memory and allows the CPU cache to be managed as part
of the I/O transaction).  The only way to prevent aliasing between the
kernel and user space is to make the two addresses congruent.  We
already have a fixed user address (given to us by the vma), so our only
choice is to remap the page in the kernel to a congruent address and
bingo we have a buffer shared between device/kernel/user space with no
aliasing problems.

The reason for making the cpu_addr mutable is to allow the
implementation to remap the page and make full congruency happen.  We
can't allow the kernel and userspace addresses to be non-congruent
because kmap will fail horribly (it's required by the dma_buf_ops) and
we also run the risk of getting a machine check if the cache discovers
two dirty aliases of the same page.

James

Re: [Linux-c6x-dev] [PATCH 3/9] c6x: Provide dma_mmap_coherent() and dma_get_sgtable()

[Marek Szyprowski]

On 1/23/2013 11:29 AM, James Bottomley wrote:
> On Wed, 2013-01-23 at 10:47 +0100, Marek Szyprowski wrote:
> > On 1/22/2013 11:13 AM, James Bottomley wrote:
> > > There might be a simple solution:  just replace void *cpu_addr with void
> > > **cpu_addr in the API.  This is a bit nasty since compilers think that
> > > void ** to void * conversion is quite legal, so it would be hard to pick
> > > up misuse of this (uint8_t ** would be better).  That way VIPT could
> > > remap the kernel pages to a coherent address.  This would probably have
> > > to change in the dma_mmap_attr() and dma_ops structures.
> > >
> > > All consumers would have to expect cpu_addr might change, but that seems
> > > doable.
> >
> > I still don't get how this can help having a coherent buffer between DMA
> > (devices) and CPU (kernel and user space mappings). The main purpose of
> > the dma_mmap_coherent() call is to provide a common API for mapping a
> > coherent buffer between DMA (device) and userspace. It is not about
> > creating a coherent mapping for sharing memory between userspace and
> > kernel space.
>
> OK, so I assume you don't understand how VIPT architectures work?
> On a VIPT architecture, the CPU cache is indexed by the virtual address
> but tagged by the physical address.  This means that when an address
> comes into the CPU, we can do simultaneous lookups in the cache and the
> TLB (by the virtual address).  The cache doesn't have the full address
> bits of the index, so it usually only looks up the lowest n bits.  The
> value of n gives the congruency of the cache (sometimes called the
> colour of the cache lines).  The cache usually produces a number of
> possible lines depending on its associativity and the TLB lookup
> produces the physical address.  We can now sweep through the cache lines
> and if a physical address tag matches, return the answer from the cache
> instead of having to consult main memory.  This gives a speed advantage
> over PIPT (Physically Indexed Physically Tagged) caches because on PIPT
> the cache lookup can only occur *after* the TLB lookup instead of
> concurrently with.
>
> As an aside, in practise every PIPT CPU actually has a VIPT cache,
> purely because of the speed angle.  The trick to making it all work is
> to shrink n so that n <= PAGE_SIZE_BITS and increase the associativity.
> This means you can get VIPT speed without producing the aliasing
> effects.
>
> Coherence is achieved in VIPT CPUs when two mapping addresses for the
> same physical page, say v1 and v2 are congruent i.e. (v1 & ((1<<n)-1))
> == (v2 & ((1<<n)-1)).  For such mappings, the same cache line is used.
> This is why we have an architecture override for
> arch_get_unmapped_area(). we use this to ensure all shareable mappings
> for all userspace process are at congruent addresses, so we don't get
> any aliasing problems in shared VMAs.
>
> Aliasing happens when v1 & ((1<<n)-1)) != (v2 & ((1<<n)-1)) where v1 and
> v2 are virtual addresses of the same physical page.  Now that page has
> *two* cache copies and you have to be very careful to resolve the
> aliases.  This, unfortunately, is the default scenario for kernel
> virtual and user virtual addresses because all kernel pages are offset
> mapped which is why we have to be annoyingly careful about flushing
> kernel pages before we hand control back to userspace.
>
> As you can see, we can only share a mapping between the kernel and user
> space without all the aliasing problems if the address is congruent.
>
> dma_mmap_coherent() seeks to be coherent from the device to the kernel
> to userspace.  On PARISC, we fix device coherency by loading a coherence
> index into our IOMMU (it effectively tells the IOMMU the virtual alias
> for the physical memory and allows the CPU cache to be managed as part
> of the I/O transaction).  The only way to prevent aliasing between the
> kernel and user space is to make the two addresses congruent.  We
> already have a fixed user address (given to us by the vma), so our only
> choice is to remap the page in the kernel to a congruent address and
> bingo we have a buffer shared between device/kernel/user space with no
> aliasing problems.
>
> The reason for making the cpu_addr mutable is to allow the
> implementation to remap the page and make full congruency happen.  We
> can't allow the kernel and userspace addresses to be non-congruent
> because kmap will fail horribly (it's required by the dma_buf_ops) and
> we also run the risk of getting a machine check if the cache discovers
> two dirty aliases of the same page.

I know what is VIPT, but I didn't know how device coherency is achieved on
PARISC and I wasn't aware that it is not possible to use non-cached 
mappings.
Now I have a complete image of this platform, thanks for you very detailed
description.

Now we need to find some working solution. This case shows that the current
DMA mapping API is quite limited and one day it will need complete rewrite.

Probably the best approach would be to change the API completely and
introduce some kind of DMA object with the following usage pattern:

dma_obj *buf = dma_alloc(dev, size, DMA_COHERENT | DMA_USERSPACE | 
DMA_KERNELSPACE, gfp);
if (!buf) {
     dma_addr_t dma_addr = dma_get_device_addr(buf);
     void *virt_addr = dma_get_kernel_addr(buf);
     ...

     dma_mmap_buf(buf, vma);
}

This way one can specify if kernel mapping or userspace mapping is required
for the given buffer or not.

This however requires a lot of work and update in all existing drivers. 
I doubt
that this can be done quickly without some period of transition and 
emulation of
the old api on top of the new one. It will also need a lot of discussion 
between
core kernel developers and a good proposal. I cannot promise that I will 
handle
this in any reasonable time frame now.

In meantime we need to live with the current design of dma_alloc_coherent do
something with dma_mmap_coherent. You have mentioned cpu_addr update in
dma_mmap_coherent, but frankly I don't like the '**cpu_addr' approach, as it
looks very hacky and it will end in various misusage in the drivers. API 
should
be clear and simple, but a function which updates the point clearly doesn't
meet this criteria.

I wonder if there will be any clients for the dma_mmap_* call on PARISC
(and other architectures which currently lack good way for mapping coherent
buffers to userspace). Currently there is no such driver. Maybe we can stick
with dummy implementation, which returns -EINVAL for now?

I think that the complete API redesign will be required one day. There 
are too
many limitations of the current API, which even now results in hacks and
workarounds in the drivers. We tried to avoid hacks in the drivers and 
update
the dma-mapping API, but it looks that this was only a top of the iceberg...

Best regards
-- 
Marek Szyprowski
Samsung Poland R&D Center

Re: [Linux-c6x-dev] [PATCH 3/9] c6x: Provide dma_mmap_coherent() and dma_get_sgtable()

[James Bottomley]

On Wed, 2013-01-23 at 18:44 +0100, Marek Szyprowski wrote:
> On 1/23/2013 11:29 AM, James Bottomley wrote:
> > On Wed, 2013-01-23 at 10:47 +0100, Marek Szyprowski wrote:
> > > On 1/22/2013 11:13 AM, James Bottomley wrote:
> > > > There might be a simple solution:  just replace void *cpu_addr with void
> > > > **cpu_addr in the API.  This is a bit nasty since compilers think that
> > > > void ** to void * conversion is quite legal, so it would be hard to pick
> > > > up misuse of this (uint8_t ** would be better).  That way VIPT could
> > > > remap the kernel pages to a coherent address.  This would probably have
> > > > to change in the dma_mmap_attr() and dma_ops structures.
> > > >
> > > > All consumers would have to expect cpu_addr might change, but that seems
> > > > doable.
> > >
> > > I still don't get how this can help having a coherent buffer between DMA
> > > (devices) and CPU (kernel and user space mappings). The main purpose of
> > > the dma_mmap_coherent() call is to provide a common API for mapping a
> > > coherent buffer between DMA (device) and userspace. It is not about
> > > creating a coherent mapping for sharing memory between userspace and
> > > kernel space.
> >
> > OK, so I assume you don't understand how VIPT architectures work?
> > On a VIPT architecture, the CPU cache is indexed by the virtual address
> > but tagged by the physical address.  This means that when an address
> > comes into the CPU, we can do simultaneous lookups in the cache and the
> > TLB (by the virtual address).  The cache doesn't have the full address
> > bits of the index, so it usually only looks up the lowest n bits.  The
> > value of n gives the congruency of the cache (sometimes called the
> > colour of the cache lines).  The cache usually produces a number of
> > possible lines depending on its associativity and the TLB lookup
> > produces the physical address.  We can now sweep through the cache lines
> > and if a physical address tag matches, return the answer from the cache
> > instead of having to consult main memory.  This gives a speed advantage
> > over PIPT (Physically Indexed Physically Tagged) caches because on PIPT
> > the cache lookup can only occur *after* the TLB lookup instead of
> > concurrently with.
> >
> > As an aside, in practise every PIPT CPU actually has a VIPT cache,
> > purely because of the speed angle.  The trick to making it all work is
> > to shrink n so that n <= PAGE_SIZE_BITS and increase the associativity.
> > This means you can get VIPT speed without producing the aliasing
> > effects.
> >
> > Coherence is achieved in VIPT CPUs when two mapping addresses for the
> > same physical page, say v1 and v2 are congruent i.e. (v1 & ((1<<n)-1))
> > == (v2 & ((1<<n)-1)).  For such mappings, the same cache line is used.
> > This is why we have an architecture override for
> > arch_get_unmapped_area(). we use this to ensure all shareable mappings
> > for all userspace process are at congruent addresses, so we don't get
> > any aliasing problems in shared VMAs.
> >
> > Aliasing happens when v1 & ((1<<n)-1)) != (v2 & ((1<<n)-1)) where v1 and
> > v2 are virtual addresses of the same physical page.  Now that page has
> > *two* cache copies and you have to be very careful to resolve the
> > aliases.  This, unfortunately, is the default scenario for kernel
> > virtual and user virtual addresses because all kernel pages are offset
> > mapped which is why we have to be annoyingly careful about flushing
> > kernel pages before we hand control back to userspace.
> >
> > As you can see, we can only share a mapping between the kernel and user
> > space without all the aliasing problems if the address is congruent.
> >
> > dma_mmap_coherent() seeks to be coherent from the device to the kernel
> > to userspace.  On PARISC, we fix device coherency by loading a coherence
> > index into our IOMMU (it effectively tells the IOMMU the virtual alias
> > for the physical memory and allows the CPU cache to be managed as part
> > of the I/O transaction).  The only way to prevent aliasing between the
> > kernel and user space is to make the two addresses congruent.  We
> > already have a fixed user address (given to us by the vma), so our only
> > choice is to remap the page in the kernel to a congruent address and
> > bingo we have a buffer shared between device/kernel/user space with no
> > aliasing problems.
> >
> > The reason for making the cpu_addr mutable is to allow the
> > implementation to remap the page and make full congruency happen.  We
> > can't allow the kernel and userspace addresses to be non-congruent
> > because kmap will fail horribly (it's required by the dma_buf_ops) and
> > we also run the risk of getting a machine check if the cache discovers
> > two dirty aliases of the same page.
> 
> I know what is VIPT, but I didn't know how device coherency is achieved on
> PARISC and I wasn't aware that it is not possible to use non-cached 
> mappings.

It is possible on most ... we have a couple of chips that can't do it
because of some architectural issue.

> Now I have a complete image of this platform, thanks for you very detailed
> description.
> 
> Now we need to find some working solution. This case shows that the current
> DMA mapping API is quite limited and one day it will need complete rewrite.
> 
> Probably the best approach would be to change the API completely and
> introduce some kind of DMA object with the following usage pattern:
> 
> dma_obj *buf = dma_alloc(dev, size, DMA_COHERENT | DMA_USERSPACE | 
> DMA_KERNELSPACE, gfp);

DMA_USERSPACE doesn't really tell us anything useful here.  In order to
force userspace coherence, we need the vma (or the user virtual address)
passed in so we can work out what a coherent address in the kernel
should be.  We could use it as an indicator that the handle should be
removed from the offset map, I suppose.

> if (!buf) {

I think you mean if(buf)

>      dma_addr_t dma_addr = dma_get_device_addr(buf);
>      void *virt_addr = dma_get_kernel_addr(buf);

So this isn't right.  To get congruence we need to remap the kernel page
to and address congruent with the userspace vma, so this would probably
have to take a vma as an argument and have 'kmap' somewhere in the API
prototype

So something like

    dma_addr_t dma_addr = dma_get_device_addr(buf);
    void *virt_addr = dma_kmap_kernel_addr(buf, vma);

We don't explicitly need the vma, we could easily make do with the vma
virtual address, so this could be something like

    void *virt_addr = dma_kmap_kernel_addr(buf, vaddr);

The easiest case for us is doing the alloc and mmap at the same time,
because that way we have all the required information to ascertain the
congruence address and do the remap before the kernel ever sees the
object.

>      ...
> 
>      dma_mmap_buf(buf, vma);
> }
> 
> This way one can specify if kernel mapping or userspace mapping is required
> for the given buffer or not.
> 
> This however requires a lot of work and update in all existing drivers. 
> I doubt
> that this can be done quickly without some period of transition and 
> emulation of
> the old api on top of the new one. It will also need a lot of discussion 
> between
> core kernel developers and a good proposal. I cannot promise that I will 
> handle
> this in any reasonable time frame now.
> 
> In meantime we need to live with the current design of dma_alloc_coherent do
> something with dma_mmap_coherent. You have mentioned cpu_addr update in
> dma_mmap_coherent, but frankly I don't like the '**cpu_addr' approach, as it
> looks very hacky and it will end in various misusage in the drivers. API 
> should
> be clear and simple, but a function which updates the point clearly doesn't
> meet this criteria.
> 
> I wonder if there will be any clients for the dma_mmap_* call on PARISC
> (and other architectures which currently lack good way for mapping coherent
> buffers to userspace). Currently there is no such driver. Maybe we can stick
> with dummy implementation, which returns -EINVAL for now?

Well, that's what lead me to ask the question whether this should be
architecture dependent.  It looks like the only drivers using this are
either SoC or v4l2 SoC type using dma-contig.

I think the best way forward would be to fix ARCH_HAS_DMA_MMAP_COHERENT
and cause a compile break if someone tries to build a driver which
requires this.  Under the current theory that they're all SoC and ARM,
they should not be buildable for PARISC (so allmodconfig and
allyesconfig should be currently safe).  This will allow us to get a
signal if that rule gets broken.

> I think that the complete API redesign will be required one day. There 
> are too
> many limitations of the current API, which even now results in hacks and
> workarounds in the drivers. We tried to avoid hacks in the drivers and 
> update
> the dma-mapping API, but it looks that this was only a top of the iceberg...

Well possibly.  A lot of it comes to us via the separate alloc and map,
which means we have to change kernel virtual address.

James

Re: [Linux-c6x-dev] [PATCH 3/9] c6x: Provide dma_mmap_coherent() and dma_get_sgtable()

[Marek Szyprowski]

On 1/21/2013 9:00 PM, Geert Uytterhoeven wrote:
> On Tue, Jan 15, 2013 at 5:56 PM, James Bottomley
> <James.Bottomley@hansenpartnership.com> wrote:
> > On Tue, 2013-01-15 at 15:07 +0100, Marek Szyprowski wrote:
> >> On 1/15/2013 10:13 AM, Geert Uytterhoeven wrote:
> >> > Marek?
> >> >
> >> > On Tue, Jan 15, 2013 at 5:16 AM, Vineet Gupta
> >> > <Vineet.Gupta1@synopsys.com> wrote:
> >> > > On Monday 14 January 2013 09:07 PM, Mark Salter wrote:
> >> > >> On Sun, 2013-01-13 at 11:44 +0100, Geert Uytterhoeven wrote:
> >> > >>> c6x/allmodconfig (assumed):
> >> > >>>
> >> > >>> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_mmap’:
> >> > >>> drivers/media/v4l2-core/videobuf2-dma-contig.c:204: error: implicit declaration of function ‘dma_mmap_coherent’
> >> > >>> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_get_base_sgt’:
> >> > >>> drivers/media/v4l2-core/videobuf2-dma-contig.c:387: error: implicit declaration of function ‘dma_get_sgtable’
> >> > >>>
> >> > >>> For architectures using dma_map_ops, dma_mmap_coherent() and
> >> > >>> dma_get_sgtable() are provided in <asm-generic/dma-mapping-common.h>.
> >> > >>>
> >> > >>> C6x does not use dma_map_ops, hence it should implement them as inline
> >> > >>> stubs using dma_common_mmap() and dma_common_get_sgtable().
> >> > >>>
> >> > >> So are dma_mmap_coherent() and dma_get_sgtable() part of the DMA API
> >> > >> now? I don't them in Documentation/DMA*.txt anywhere.
> >> > >>
> >> > >> Why does the default dma_common_mmap() for !CONFIG_MMU return an
> >> > >> error?
> >> > >>
> >> > >> Wouldn't it be better to provide default implementations that an arch
> >> > >> could override rather than having to patch all "no dma_map_ops"
> >> > >> architectures?
> >> > >>
> >> > > Speaking for the still-reviewed ARC Port, I completely agree with Mark.
> >>
> >> dma_mmap_coherent() was partially in the DMA mapping API for some time, but
> >> it was available only on a few architectures (afair ARM, powerpc and avr32).
> >> This caused significant problems for writing unified device drivers or some
> >> device helper modules which were aimed to work on more than one
> >> architecture.
> >>
> >> dma_get_sgtable() is an extension discussed during the Linaro meetings. It
> >> is required to correctly implement buffer sharing between device driver
> >> without hacks or any assumptions about memory layout in the device drivers.
> >>
> >> I have implemented some generic code for both of those two functions,
> >> keeping
> >> in mind that on some hardware architectures (like already mentioned VIVT)
> >> it might be not possible to provide coherent mapping to userspace. It is
> >> perfectly fine for those functions to return an error in such case.
> >
> > It's not possible on VIPT either.  This means that the API is unusable
> > on quite a large number of architectures.  Surely, if we're starting to
> > write drivers using this, we need to fix the API before more people try
> > to use it.
> >
> > For PA-RISC (and all other VIPT, I assume) I need an API which allows me
> > to remap the virtual address of the kernel component (probably using the
> > kmap area) so the user space and kernel space addresses are congruent.
>
> So what are we gonna do for 3.8? I'd like to get my allmodconfig build
> green again ;-)
>
> Change the API?
>
> Keep the API but do a best-effort fix to unbreak the builds?
>    - Apply my patches that got acks (avr32/blackfin/cris/m68k),
>    - Use static inlines that return -EINVAL for the rest
> (c6x/frv/mn10300/parisc/xtensa).
> I still have a few m68k fixes queued for 3.8, for which I've been postponing the
> pull request to get this sorted out, so I could include the above.

I think this would be the best solution for fixing v3.8. I would like to put
those patches in linux-next asap and then after a week or so send a pull
request for merging them to v3.8-rc6.

EINVAL approach can be later fixed/extended with real implementation (if
there are any driver candidates, which will use such interface).

Best regards
-- 
Marek Szyprowski
Samsung Poland R&D Center

Re: [Linux-c6x-dev] [PATCH 3/9] c6x: Provide dma_mmap_coherent() and dma_get_sgtable()

[Marek Szyprowski]

Hello,

On 1/15/2013 5:56 PM, James Bottomley wrote:
> On Tue, 2013-01-15 at 15:07 +0100, Marek Szyprowski wrote:
> > Hello,
> >
> > On 1/15/2013 10:13 AM, Geert Uytterhoeven wrote:
> > > Marek?
> > >
> > > On Tue, Jan 15, 2013 at 5:16 AM, Vineet Gupta
> > > <Vineet.Gupta1@synopsys.com> wrote:
> > > > On Monday 14 January 2013 09:07 PM, Mark Salter wrote:
> > > >> On Sun, 2013-01-13 at 11:44 +0100, Geert Uytterhoeven wrote:
> > > >>> c6x/allmodconfig (assumed):
> > > >>>
> > > >>> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_mmap’:
> > > >>> drivers/media/v4l2-core/videobuf2-dma-contig.c:204: error: implicit declaration of function ‘dma_mmap_coherent’
> > > >>> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_get_base_sgt’:
> > > >>> drivers/media/v4l2-core/videobuf2-dma-contig.c:387: error: implicit declaration of function ‘dma_get_sgtable’
> > > >>>
> > > >>> For architectures using dma_map_ops, dma_mmap_coherent() and
> > > >>> dma_get_sgtable() are provided in <asm-generic/dma-mapping-common.h>.
> > > >>>
> > > >>> C6x does not use dma_map_ops, hence it should implement them as inline
> > > >>> stubs using dma_common_mmap() and dma_common_get_sgtable().
> > > >>>
> > > >> So are dma_mmap_coherent() and dma_get_sgtable() part of the DMA API
> > > >> now? I don't them in Documentation/DMA*.txt anywhere.
> > > >>
> > > >> Why does the default dma_common_mmap() for !CONFIG_MMU return an
> > > >> error?
> > > >>
> > > >> Wouldn't it be better to provide default implementations that an arch
> > > >> could override rather than having to patch all "no dma_map_ops"
> > > >> architectures?
> > > >>
> > > > Speaking for the still-reviewed ARC Port, I completely agree with Mark.
> >
> > dma_mmap_coherent() was partially in the DMA mapping API for some time, but
> > it was available only on a few architectures (afair ARM, powerpc and avr32).
> > This caused significant problems for writing unified device drivers or some
> > device helper modules which were aimed to work on more than one
> > architecture.
> >
> > dma_get_sgtable() is an extension discussed during the Linaro meetings. It
> > is required to correctly implement buffer sharing between device driver
> > without hacks or any assumptions about memory layout in the device drivers.
> >
> > I have implemented some generic code for both of those two functions,
> > keeping
> > in mind that on some hardware architectures (like already mentioned VIVT)
> > it might be not possible to provide coherent mapping to userspace. It is
> > perfectly fine for those functions to return an error in such case.
>
> It's not possible on VIPT either.  This means that the API is unusable
> on quite a large number of architectures.  Surely, if we're starting to
> write drivers using this, we need to fix the API before more people try
> to use it.

I don't get this one. On ARM coherent mappings are implemented as 
non-cacheable,
both in userspace and kernel-space, so having a coherent mapping is 
possible on
VIPT architecture.

There are common, already defined and widely used user interfaces (like 
mentioned
fbdev, v4l2, alsa) which rely on mmaping coherent buffers to userspace. 
There are
drivers which use coherent buffers for those interfaces. It is already 
an issue
to implement mmap call for those drivers as it requires some architecture
specific assumptions. We wanted to remove those assumptions from the 
drivers and
move them to some common dma-mapping calls.

I agree that this might not be directly possible on non-mmu systems, 
where mmap
syscall is handled differently. Some architectures might also have some 
hardware
limitations, which prevents implementing such interface. That's ok, as 
probably
there is no drivers which will use this interface on this architecture.

> For PA-RISC (and all other VIPT, I assume) I need an API which allows me
> to remap the virtual address of the kernel component (probably using the
> kmap area) so the user space and kernel space addresses are congruent.

How such API would look like? What syscall might be used for it?

Best regards
-- 
Marek Szyprowski
Samsung Poland R&D Center

Re: [Linux-c6x-dev] [PATCH 3/9] c6x: Provide dma_mmap_coherent() and dma_get_sgtable()

[James Bottomley]

On Tue, 2013-01-22 at 11:15 +0100, Marek Szyprowski wrote:
> Hello,
> 
> On 1/15/2013 5:56 PM, James Bottomley wrote:
> > On Tue, 2013-01-15 at 15:07 +0100, Marek Szyprowski wrote:
> > > Hello,
> > >
> > > On 1/15/2013 10:13 AM, Geert Uytterhoeven wrote:
> > > > Marek?
> > > >
> > > > On Tue, Jan 15, 2013 at 5:16 AM, Vineet Gupta
> > > > <Vineet.Gupta1@synopsys.com> wrote:
> > > > > On Monday 14 January 2013 09:07 PM, Mark Salter wrote:
> > > > >> On Sun, 2013-01-13 at 11:44 +0100, Geert Uytterhoeven wrote:
> > > > >>> c6x/allmodconfig (assumed):
> > > > >>>
> > > > >>> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_mmap’:
> > > > >>> drivers/media/v4l2-core/videobuf2-dma-contig.c:204: error: implicit declaration of function ‘dma_mmap_coherent’
> > > > >>> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_get_base_sgt’:
> > > > >>> drivers/media/v4l2-core/videobuf2-dma-contig.c:387: error: implicit declaration of function ‘dma_get_sgtable’
> > > > >>>
> > > > >>> For architectures using dma_map_ops, dma_mmap_coherent() and
> > > > >>> dma_get_sgtable() are provided in <asm-generic/dma-mapping-common.h>.
> > > > >>>
> > > > >>> C6x does not use dma_map_ops, hence it should implement them as inline
> > > > >>> stubs using dma_common_mmap() and dma_common_get_sgtable().
> > > > >>>
> > > > >> So are dma_mmap_coherent() and dma_get_sgtable() part of the DMA API
> > > > >> now? I don't them in Documentation/DMA*.txt anywhere.
> > > > >>
> > > > >> Why does the default dma_common_mmap() for !CONFIG_MMU return an
> > > > >> error?
> > > > >>
> > > > >> Wouldn't it be better to provide default implementations that an arch
> > > > >> could override rather than having to patch all "no dma_map_ops"
> > > > >> architectures?
> > > > >>
> > > > > Speaking for the still-reviewed ARC Port, I completely agree with Mark.
> > >
> > > dma_mmap_coherent() was partially in the DMA mapping API for some time, but
> > > it was available only on a few architectures (afair ARM, powerpc and avr32).
> > > This caused significant problems for writing unified device drivers or some
> > > device helper modules which were aimed to work on more than one
> > > architecture.
> > >
> > > dma_get_sgtable() is an extension discussed during the Linaro meetings. It
> > > is required to correctly implement buffer sharing between device driver
> > > without hacks or any assumptions about memory layout in the device drivers.
> > >
> > > I have implemented some generic code for both of those two functions,
> > > keeping
> > > in mind that on some hardware architectures (like already mentioned VIVT)
> > > it might be not possible to provide coherent mapping to userspace. It is
> > > perfectly fine for those functions to return an error in such case.
> >
> > It's not possible on VIPT either.  This means that the API is unusable
> > on quite a large number of architectures.  Surely, if we're starting to
> > write drivers using this, we need to fix the API before more people try
> > to use it.
> 
> I don't get this one. On ARM coherent mappings are implemented as 
> non-cacheable,
> both in userspace and kernel-space, so having a coherent mapping is 
> possible on
> VIPT architecture.

Only if you have an uncacheable bit in the architecture page table ...
which some of ours don't.

Regardless, setting pages Uncacheable is really a hack job on shared
buffers because it creates a huge slow down .... like an order of
magnitude more than simply copying the page, which I believe is the
current solution.

> There are common, already defined and widely used user interfaces (like 
> mentioned
> fbdev, v4l2, alsa) which rely on mmaping coherent buffers to userspace. 
> There are
> drivers which use coherent buffers for those interfaces. It is already 
> an issue
> to implement mmap call for those drivers as it requires some architecture
> specific assumptions. We wanted to remove those assumptions from the 
> drivers and
> move them to some common dma-mapping calls.
> 
> I agree that this might not be directly possible on non-mmu systems, 
> where mmap
> syscall is handled differently. Some architectures might also have some 
> hardware
> limitations, which prevents implementing such interface. That's ok, as 
> probably
> there is no drivers which will use this interface on this architecture.
> 
> > For PA-RISC (and all other VIPT, I assume) I need an API which allows me
> > to remap the virtual address of the kernel component (probably using the
> > kmap area) so the user space and kernel space addresses are congruent.
> 
> How such API would look like? What syscall might be used for it?

See next email.

James

Re: [Linux-c6x-dev] [PATCH 3/9] c6x: Provide dma_mmap_coherent() and dma_get_sgtable()

[Vineet Gupta]

On Tuesday 22 January 2013 03:52 PM, James Bottomley wrote:
> On Tue, 2013-01-22 at 11:15 +0100, Marek Szyprowski wrote:
>> Hello,
>>
>> On 1/15/2013 5:56 PM, James Bottomley wrote:
>>> On Tue, 2013-01-15 at 15:07 +0100, Marek Szyprowski wrote:
>>>> Hello,
>>>>
>>>> On 1/15/2013 10:13 AM, Geert Uytterhoeven wrote:
>>>>> Marek?
>>>>>
>>>>> On Tue, Jan 15, 2013 at 5:16 AM, Vineet Gupta
>>>>> <Vineet.Gupta1@synopsys.com> wrote:
>>>>>> On Monday 14 January 2013 09:07 PM, Mark Salter wrote:
>>>>>>> On Sun, 2013-01-13 at 11:44 +0100, Geert Uytterhoeven wrote:
>>>>>>>> c6x/allmodconfig (assumed):
>>>>>>>>
>>>>>>>> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_mmap’:
>>>>>>>> drivers/media/v4l2-core/videobuf2-dma-contig.c:204: error: implicit declaration of function ‘dma_mmap_coherent’
>>>>>>>> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_get_base_sgt’:
>>>>>>>> drivers/media/v4l2-core/videobuf2-dma-contig.c:387: error: implicit declaration of function ‘dma_get_sgtable’
>>>>>>>>
>>>>>>>> For architectures using dma_map_ops, dma_mmap_coherent() and
>>>>>>>> dma_get_sgtable() are provided in <asm-generic/dma-mapping-common.h>.
>>>>>>>>
>>>>>>>> C6x does not use dma_map_ops, hence it should implement them as inline
>>>>>>>> stubs using dma_common_mmap() and dma_common_get_sgtable().
>>>>>>>>
>>>>>>> So are dma_mmap_coherent() and dma_get_sgtable() part of the DMA API
>>>>>>> now? I don't them in Documentation/DMA*.txt anywhere.
>>>>>>>
>>>>>>> Why does the default dma_common_mmap() for !CONFIG_MMU return an
>>>>>>> error?
>>>>>>>
>>>>>>> Wouldn't it be better to provide default implementations that an arch
>>>>>>> could override rather than having to patch all "no dma_map_ops"
>>>>>>> architectures?
>>>>>>>
>>>>>> Speaking for the still-reviewed ARC Port, I completely agree with Mark.
>>>>
>>>> dma_mmap_coherent() was partially in the DMA mapping API for some time, but
>>>> it was available only on a few architectures (afair ARM, powerpc and avr32).
>>>> This caused significant problems for writing unified device drivers or some
>>>> device helper modules which were aimed to work on more than one
>>>> architecture.
>>>>
>>>> dma_get_sgtable() is an extension discussed during the Linaro meetings. It
>>>> is required to correctly implement buffer sharing between device driver
>>>> without hacks or any assumptions about memory layout in the device drivers.
>>>>
>>>> I have implemented some generic code for both of those two functions,
>>>> keeping
>>>> in mind that on some hardware architectures (like already mentioned VIVT)
>>>> it might be not possible to provide coherent mapping to userspace. It is
>>>> perfectly fine for those functions to return an error in such case.
>>>
>>> It's not possible on VIPT either.  This means that the API is unusable
>>> on quite a large number of architectures.  Surely, if we're starting to
>>> write drivers using this, we need to fix the API before more people try
>>> to use it.
>>
>> I don't get this one. On ARM coherent mappings are implemented as 
>> non-cacheable,
>> both in userspace and kernel-space, so having a coherent mapping is 
>> possible on
>> VIPT architecture.
> 
> Only if you have an uncacheable bit in the architecture page table ...
> which some of ours don't.
> 
> Regardless, setting pages Uncacheable is really a hack job on shared
> buffers because it creates a huge slow down .... like an order of
> magnitude more than simply copying the page, which I believe is the
> current solution.

IMHO, setting the page uncached, even for shared buffer, is the only option for
arches with non snooping DMA - independent of VIPT issue.

-Vineet

Re: [Linux-c6x-dev] [PATCH 3/9] c6x: Provide dma_mmap_coherent() and dma_get_sgtable()

[Vineet Gupta]

On Tuesday 22 January 2013 03:52 PM, James Bottomley wrote:
> On Tue, 2013-01-22 at 11:15 +0100, Marek Szyprowski wrote:
>> Hello,
>>
>> On 1/15/2013 5:56 PM, James Bottomley wrote:
>>> On Tue, 2013-01-15 at 15:07 +0100, Marek Szyprowski wrote:
>>>> Hello,
>>>>
>>>> On 1/15/2013 10:13 AM, Geert Uytterhoeven wrote:
>>>>> Marek?
>>>>>
>>>>> On Tue, Jan 15, 2013 at 5:16 AM, Vineet Gupta
>>>>> <Vineet.Gupta1@synopsys.com> wrote:
>>>>>> On Monday 14 January 2013 09:07 PM, Mark Salter wrote:
>>>>>>> On Sun, 2013-01-13 at 11:44 +0100, Geert Uytterhoeven wrote:
>>>>>>>> c6x/allmodconfig (assumed):
>>>>>>>>
>>>>>>>> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_mmap’:
>>>>>>>> drivers/media/v4l2-core/videobuf2-dma-contig.c:204: error: implicit declaration of function ‘dma_mmap_coherent’
>>>>>>>> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_get_base_sgt’:
>>>>>>>> drivers/media/v4l2-core/videobuf2-dma-contig.c:387: error: implicit declaration of function ‘dma_get_sgtable’
>>>>>>>>
>>>>>>>> For architectures using dma_map_ops, dma_mmap_coherent() and
>>>>>>>> dma_get_sgtable() are provided in <asm-generic/dma-mapping-common.h>.
>>>>>>>>
>>>>>>>> C6x does not use dma_map_ops, hence it should implement them as inline
>>>>>>>> stubs using dma_common_mmap() and dma_common_get_sgtable().
>>>>>>>>
>>>>>>> So are dma_mmap_coherent() and dma_get_sgtable() part of the DMA API
>>>>>>> now? I don't them in Documentation/DMA*.txt anywhere.
>>>>>>>
>>>>>>> Why does the default dma_common_mmap() for !CONFIG_MMU return an
>>>>>>> error?
>>>>>>>
>>>>>>> Wouldn't it be better to provide default implementations that an arch
>>>>>>> could override rather than having to patch all "no dma_map_ops"
>>>>>>> architectures?
>>>>>>>
>>>>>> Speaking for the still-reviewed ARC Port, I completely agree with Mark.
>>>>
>>>> dma_mmap_coherent() was partially in the DMA mapping API for some time, but
>>>> it was available only on a few architectures (afair ARM, powerpc and avr32).
>>>> This caused significant problems for writing unified device drivers or some
>>>> device helper modules which were aimed to work on more than one
>>>> architecture.
>>>>
>>>> dma_get_sgtable() is an extension discussed during the Linaro meetings. It
>>>> is required to correctly implement buffer sharing between device driver
>>>> without hacks or any assumptions about memory layout in the device drivers.
>>>>
>>>> I have implemented some generic code for both of those two functions,
>>>> keeping
>>>> in mind that on some hardware architectures (like already mentioned VIVT)
>>>> it might be not possible to provide coherent mapping to userspace. It is
>>>> perfectly fine for those functions to return an error in such case.
>>>
>>> It's not possible on VIPT either.  This means that the API is unusable
>>> on quite a large number of architectures.  Surely, if we're starting to
>>> write drivers using this, we need to fix the API before more people try
>>> to use it.
>>
>> I don't get this one. On ARM coherent mappings are implemented as 
>> non-cacheable,
>> both in userspace and kernel-space, so having a coherent mapping is 
>> possible on
>> VIPT architecture.
> 
> Only if you have an uncacheable bit in the architecture page table ...
> which some of ours don't.
> 
> Regardless, setting pages Uncacheable is really a hack job on shared
> buffers because it creates a huge slow down .... like an order of
> magnitude more than simply copying the page, which I believe is the
> current solution.

IMHO, setting the page uncached, even for shared buffer, is the only option for
arches with non snooping DMA - independent of VIPT issue.

-Vineet

Re: [Linux-c6x-dev] [PATCH 3/9] c6x: Provide dma_mmap_coherent() and dma_get_sgtable()

[James Bottomley]

On Wed, 2013-01-23 at 12:53 +0530, Vineet Gupta wrote:
> On Tuesday 22 January 2013 03:52 PM, James Bottomley wrote:
> > On Tue, 2013-01-22 at 11:15 +0100, Marek Szyprowski wrote:
> >> Hello,
> >>
> >> On 1/15/2013 5:56 PM, James Bottomley wrote:
> >>> On Tue, 2013-01-15 at 15:07 +0100, Marek Szyprowski wrote:
> >>>> Hello,
> >>>>
> >>>> On 1/15/2013 10:13 AM, Geert Uytterhoeven wrote:
> >>>>> Marek?
> >>>>>
> >>>>> On Tue, Jan 15, 2013 at 5:16 AM, Vineet Gupta
> >>>>> <Vineet.Gupta1@synopsys.com> wrote:
> >>>>>> On Monday 14 January 2013 09:07 PM, Mark Salter wrote:
> >>>>>>> On Sun, 2013-01-13 at 11:44 +0100, Geert Uytterhoeven wrote:
> >>>>>>>> c6x/allmodconfig (assumed):
> >>>>>>>>
> >>>>>>>> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_mmap’:
> >>>>>>>> drivers/media/v4l2-core/videobuf2-dma-contig.c:204: error: implicit declaration of function ‘dma_mmap_coherent’
> >>>>>>>> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_get_base_sgt’:
> >>>>>>>> drivers/media/v4l2-core/videobuf2-dma-contig.c:387: error: implicit declaration of function ‘dma_get_sgtable’
> >>>>>>>>
> >>>>>>>> For architectures using dma_map_ops, dma_mmap_coherent() and
> >>>>>>>> dma_get_sgtable() are provided in <asm-generic/dma-mapping-common.h>.
> >>>>>>>>
> >>>>>>>> C6x does not use dma_map_ops, hence it should implement them as inline
> >>>>>>>> stubs using dma_common_mmap() and dma_common_get_sgtable().
> >>>>>>>>
> >>>>>>> So are dma_mmap_coherent() and dma_get_sgtable() part of the DMA API
> >>>>>>> now? I don't them in Documentation/DMA*.txt anywhere.
> >>>>>>>
> >>>>>>> Why does the default dma_common_mmap() for !CONFIG_MMU return an
> >>>>>>> error?
> >>>>>>>
> >>>>>>> Wouldn't it be better to provide default implementations that an arch
> >>>>>>> could override rather than having to patch all "no dma_map_ops"
> >>>>>>> architectures?
> >>>>>>>
> >>>>>> Speaking for the still-reviewed ARC Port, I completely agree with Mark.
> >>>>
> >>>> dma_mmap_coherent() was partially in the DMA mapping API for some time, but
> >>>> it was available only on a few architectures (afair ARM, powerpc and avr32).
> >>>> This caused significant problems for writing unified device drivers or some
> >>>> device helper modules which were aimed to work on more than one
> >>>> architecture.
> >>>>
> >>>> dma_get_sgtable() is an extension discussed during the Linaro meetings. It
> >>>> is required to correctly implement buffer sharing between device driver
> >>>> without hacks or any assumptions about memory layout in the device drivers.
> >>>>
> >>>> I have implemented some generic code for both of those two functions,
> >>>> keeping
> >>>> in mind that on some hardware architectures (like already mentioned VIVT)
> >>>> it might be not possible to provide coherent mapping to userspace. It is
> >>>> perfectly fine for those functions to return an error in such case.
> >>>
> >>> It's not possible on VIPT either.  This means that the API is unusable
> >>> on quite a large number of architectures.  Surely, if we're starting to
> >>> write drivers using this, we need to fix the API before more people try
> >>> to use it.
> >>
> >> I don't get this one. On ARM coherent mappings are implemented as 
> >> non-cacheable,
> >> both in userspace and kernel-space, so having a coherent mapping is 
> >> possible on
> >> VIPT architecture.
> > 
> > Only if you have an uncacheable bit in the architecture page table ...
> > which some of ours don't.
> > 
> > Regardless, setting pages Uncacheable is really a hack job on shared
> > buffers because it creates a huge slow down .... like an order of
> > magnitude more than simply copying the page, which I believe is the
> > current solution.
> 
> IMHO, setting the page uncached, even for shared buffer, is the only option for
> arches with non snooping DMA - independent of VIPT issue.

I didn't say architectures *shouldn't* be able to do this.  I did say
that for architectures where it's not possible (some PARISC and nommu)
or for which it tanks performance, there should still be a functional
API.

changing the cpu_addr to be mutable means that current uncached
implementations pass it through unchanged.  However if we need to make
it coherent by changing it, we now have that option.

The point is that the current implementation can't work on some
architectures and is badly performing on others.  The new proposal will,
I think, work on every architecture and it can improve performance
should the architecture wish to take advantage of it.

James

[PATCH 4/9] cris: Provide dma_mmap_coherent() and dma_get_sgtable()

[Geert Uytterhoeven]

cris/allmodconfig:

drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_mmap’:
drivers/media/v4l2-core/videobuf2-dma-contig.c:204: error: implicit declaration of function ‘dma_mmap_coherent’
drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_get_base_sgt’:
drivers/media/v4l2-core/videobuf2-dma-contig.c:387: error: implicit declaration of function ‘dma_get_sgtable’

For architectures using dma_map_ops, dma_mmap_coherent() and
dma_get_sgtable() are provided in <asm-generic/dma-mapping-common.h>.

Cris does not use dma_map_ops, hence it should implement them as inline
stubs using dma_common_mmap() and dma_common_get_sgtable().

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: linux-cris-kernel@axis.com
---
 arch/cris/include/asm/dma-mapping.h |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/arch/cris/include/asm/dma-mapping.h b/arch/cris/include/asm/dma-mapping.h
index 8588b2c..2f0f654 100644
--- a/arch/cris/include/asm/dma-mapping.h
+++ b/arch/cris/include/asm/dma-mapping.h
@@ -158,5 +158,15 @@ dma_cache_sync(struct device *dev, void *vaddr, size_t size,
 {
 }
 
+/* drivers/base/dma-mapping.c */
+extern int dma_common_mmap(struct device *dev, struct vm_area_struct *vma,
+			   void *cpu_addr, dma_addr_t dma_addr, size_t size);
+extern int dma_common_get_sgtable(struct device *dev, struct sg_table *sgt,
+				  void *cpu_addr, dma_addr_t dma_addr,
+				  size_t size);
+
+#define dma_mmap_coherent(d, v, c, h, s) dma_common_mmap(d, v, c, h, s)
+#define dma_get_sgtable(d, t, v, h, s) dma_common_get_sgtable(d, t, v, h, s)
+
 
 #endif
-- 
1.7.0.4

Re: [PATCH 4/9] cris: Provide dma_mmap_coherent() and dma_get_sgtable()

[Jesper Nilsson]

On Sun, Jan 13, 2013 at 11:44:45AM +0100, Geert Uytterhoeven wrote:
> cris/allmodconfig:
> 
> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_mmap’:
> drivers/media/v4l2-core/videobuf2-dma-contig.c:204: error: implicit declaration of function ‘dma_mmap_coherent’
> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_get_base_sgt’:
> drivers/media/v4l2-core/videobuf2-dma-contig.c:387: error: implicit declaration of function ‘dma_get_sgtable’
> 
> For architectures using dma_map_ops, dma_mmap_coherent() and
> dma_get_sgtable() are provided in <asm-generic/dma-mapping-common.h>.
> 
> Cris does not use dma_map_ops, hence it should implement them as inline
> stubs using dma_common_mmap() and dma_common_get_sgtable().
> 
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> Cc: linux-cris-kernel@axis.com

Acked-by: Jesper Nilsson <jesper.nilsson@axis.com>

/^JN - Jesper Nilsson
-- 
               Jesper Nilsson -- jesper.nilsson@axis.com

[PATCH 5/9] frv: Provide dma_mmap_coherent() and dma_get_sgtable()

[Geert Uytterhoeven]

frv/allmodconfig:

drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_mmap’:
drivers/media/v4l2-core/videobuf2-dma-contig.c:204: error: implicit declaration of function ‘dma_mmap_coherent’
drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_get_base_sgt’:
drivers/media/v4l2-core/videobuf2-dma-contig.c:387: error: implicit declaration of function ‘dma_get_sgtable’

For architectures using dma_map_ops, dma_mmap_coherent() and
dma_get_sgtable() are provided in <asm-generic/dma-mapping-common.h>.

Frv does not use dma_map_ops, hence it should implement them as inline
stubs using dma_common_mmap() and dma_common_get_sgtable().

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: David Howells <dhowells@redhat.com>
---
 arch/frv/include/asm/dma-mapping.h |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/arch/frv/include/asm/dma-mapping.h b/arch/frv/include/asm/dma-mapping.h
index dfb8110..e8142b0 100644
--- a/arch/frv/include/asm/dma-mapping.h
+++ b/arch/frv/include/asm/dma-mapping.h
@@ -132,4 +132,14 @@ void dma_cache_sync(struct device *dev, void *vaddr, size_t size,
 	flush_write_buffers();
 }
 
+/* drivers/base/dma-mapping.c */
+extern int dma_common_mmap(struct device *dev, struct vm_area_struct *vma,
+			   void *cpu_addr, dma_addr_t dma_addr, size_t size);
+extern int dma_common_get_sgtable(struct device *dev, struct sg_table *sgt,
+				  void *cpu_addr, dma_addr_t dma_addr,
+				  size_t size);
+
+#define dma_mmap_coherent(d, v, c, h, s) dma_common_mmap(d, v, c, h, s)
+#define dma_get_sgtable(d, t, v, h, s) dma_common_get_sgtable(d, t, v, h, s)
+
 #endif  /* _ASM_DMA_MAPPING_H */
-- 
1.7.0.4

[PATCH 6/9] m68k: Provide dma_mmap_coherent() and dma_get_sgtable()

[Geert Uytterhoeven]

m68k/allmodconfig:

drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_mmap’:
drivers/media/v4l2-core/videobuf2-dma-contig.c:204: error: implicit declaration of function ‘dma_mmap_coherent’
drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_get_base_sgt’:
drivers/media/v4l2-core/videobuf2-dma-contig.c:387: error: implicit declaration of function ‘dma_get_sgtable’

For architectures using dma_map_ops, dma_mmap_coherent() and
dma_get_sgtable() are provided in <asm-generic/dma-mapping-common.h>.

M68k does not use dma_map_ops, hence it should implement them as inline
stubs using dma_common_mmap() and dma_common_get_sgtable().

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: linux-m68k@lists.linux-m68k.org
---
 arch/m68k/include/asm/dma-mapping.h |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/arch/m68k/include/asm/dma-mapping.h b/arch/m68k/include/asm/dma-mapping.h
index c68cdb4..05aa535 100644
--- a/arch/m68k/include/asm/dma-mapping.h
+++ b/arch/m68k/include/asm/dma-mapping.h
@@ -110,4 +110,14 @@ static inline int dma_mapping_error(struct device *dev, dma_addr_t handle)
 	return 0;
 }
 
+/* drivers/base/dma-mapping.c */
+extern int dma_common_mmap(struct device *dev, struct vm_area_struct *vma,
+			   void *cpu_addr, dma_addr_t dma_addr, size_t size);
+extern int dma_common_get_sgtable(struct device *dev, struct sg_table *sgt,
+				  void *cpu_addr, dma_addr_t dma_addr,
+				  size_t size);
+
+#define dma_mmap_coherent(d, v, c, h, s) dma_common_mmap(d, v, c, h, s)
+#define dma_get_sgtable(d, t, v, h, s) dma_common_get_sgtable(d, t, v, h, s)
+
 #endif  /* _M68K_DMA_MAPPING_H */
-- 
1.7.0.4

[PATCH 6/9] m68k: Provide dma_mmap_coherent() and dma_get_sgtable()

[Geert Uytterhoeven]

m68k/allmodconfig:

drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_mmap’:
drivers/media/v4l2-core/videobuf2-dma-contig.c:204: error: implicit declaration of function ‘dma_mmap_coherent’
drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_get_base_sgt’:
drivers/media/v4l2-core/videobuf2-dma-contig.c:387: error: implicit declaration of function ‘dma_get_sgtable’

For architectures using dma_map_ops, dma_mmap_coherent() and
dma_get_sgtable() are provided in <asm-generic/dma-mapping-common.h>.

M68k does not use dma_map_ops, hence it should implement them as inline
stubs using dma_common_mmap() and dma_common_get_sgtable().

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: linux-m68k@lists.linux-m68k.org
---
 arch/m68k/include/asm/dma-mapping.h |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/arch/m68k/include/asm/dma-mapping.h b/arch/m68k/include/asm/dma-mapping.h
index c68cdb4..05aa535 100644
--- a/arch/m68k/include/asm/dma-mapping.h
+++ b/arch/m68k/include/asm/dma-mapping.h
@@ -110,4 +110,14 @@ static inline int dma_mapping_error(struct device *dev, dma_addr_t handle)
 	return 0;
 }
 
+/* drivers/base/dma-mapping.c */
+extern int dma_common_mmap(struct device *dev, struct vm_area_struct *vma,
+			   void *cpu_addr, dma_addr_t dma_addr, size_t size);
+extern int dma_common_get_sgtable(struct device *dev, struct sg_table *sgt,
+				  void *cpu_addr, dma_addr_t dma_addr,
+				  size_t size);
+
+#define dma_mmap_coherent(d, v, c, h, s) dma_common_mmap(d, v, c, h, s)
+#define dma_get_sgtable(d, t, v, h, s) dma_common_get_sgtable(d, t, v, h, s)
+
 #endif  /* _M68K_DMA_MAPPING_H */
-- 
1.7.0.4

[PATCH 7/9] mn10300: Provide dma_mmap_coherent() and dma_get_sgtable()

[Geert Uytterhoeven]

mn10300/allmodconfig:

drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_mmap’:
drivers/media/v4l2-core/videobuf2-dma-contig.c:204: error: implicit declaration of function ‘dma_mmap_coherent’
drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_get_base_sgt’:
drivers/media/v4l2-core/videobuf2-dma-contig.c:387: error: implicit declaration of function ‘dma_get_sgtable’

For architectures using dma_map_ops, dma_mmap_coherent() and
dma_get_sgtable() are provided in <asm-generic/dma-mapping-common.h>.

Mn10300 does not use dma_map_ops, hence it should implement them as inline
stubs using dma_common_mmap() and dma_common_get_sgtable().

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: linux-am33-list@redhat.com
---
 arch/mn10300/include/asm/dma-mapping.h |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/arch/mn10300/include/asm/dma-mapping.h b/arch/mn10300/include/asm/dma-mapping.h
index c1be439..f7a4311 100644
--- a/arch/mn10300/include/asm/dma-mapping.h
+++ b/arch/mn10300/include/asm/dma-mapping.h
@@ -168,4 +168,14 @@ void dma_cache_sync(void *vaddr, size_t size,
 	mn10300_dcache_flush_inv();
 }
 
+/* drivers/base/dma-mapping.c */
+extern int dma_common_mmap(struct device *dev, struct vm_area_struct *vma,
+			   void *cpu_addr, dma_addr_t dma_addr, size_t size);
+extern int dma_common_get_sgtable(struct device *dev, struct sg_table *sgt,
+				  void *cpu_addr, dma_addr_t dma_addr,
+				  size_t size);
+
+#define dma_mmap_coherent(d, v, c, h, s) dma_common_mmap(d, v, c, h, s)
+#define dma_get_sgtable(d, t, v, h, s) dma_common_get_sgtable(d, t, v, h, s)
+
 #endif
-- 
1.7.0.4

[PATCH 8/9] parisc: Provide dma_mmap_coherent() and dma_get_sgtable()

[Geert Uytterhoeven]

parisc/allmodconfig:

drivers/media/v4l2-core/videobuf2-dma-contig.c: In function =E2=80=98vb=
2_dc_mmap=E2=80=99:
drivers/media/v4l2-core/videobuf2-dma-contig.c:204: error: implicit dec=
laration of function =E2=80=98dma_mmap_coherent=E2=80=99
drivers/media/v4l2-core/videobuf2-dma-contig.c: In function =E2=80=98vb=
2_dc_get_base_sgt=E2=80=99:
drivers/media/v4l2-core/videobuf2-dma-contig.c:387: error: implicit dec=
laration of function =E2=80=98dma_get_sgtable=E2=80=99

=46or architectures using dma_map_ops, dma_mmap_coherent() and
dma_get_sgtable() are provided in <asm-generic/dma-mapping-common.h>.

Parisc does not use dma_map_ops, hence it should implement them as inli=
ne
stubs using dma_common_mmap() and dma_common_get_sgtable().

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: linux-parisc@vger.kernel.org
---
 arch/parisc/include/asm/dma-mapping.h |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/arch/parisc/include/asm/dma-mapping.h b/arch/parisc/includ=
e/asm/dma-mapping.h
index 467bbd5..1fac0bf 100644
--- a/arch/parisc/include/asm/dma-mapping.h
+++ b/arch/parisc/include/asm/dma-mapping.h
@@ -238,4 +238,14 @@ void * sba_get_iommu(struct parisc_device *dev);
 /* At the moment, we panic on error for IOMMU resource exaustion */
 #define dma_mapping_error(dev, x)	0
=20
+/* drivers/base/dma-mapping.c */
+extern int dma_common_mmap(struct device *dev, struct vm_area_struct *=
vma,
+			   void *cpu_addr, dma_addr_t dma_addr, size_t size);
+extern int dma_common_get_sgtable(struct device *dev, struct sg_table =
*sgt,
+				  void *cpu_addr, dma_addr_t dma_addr,
+				  size_t size);
+
+#define dma_mmap_coherent(d, v, c, h, s) dma_common_mmap(d, v, c, h, s=
)
+#define dma_get_sgtable(d, t, v, h, s) dma_common_get_sgtable(d, t, v,=
 h, s)
+
 #endif
--=20
1.7.0.4

[PATCH 8/9] parisc: Provide dma_mmap_coherent() and dma_get_sgtable()

[Geert Uytterhoeven]

parisc/allmodconfig:

drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_mmap’:
drivers/media/v4l2-core/videobuf2-dma-contig.c:204: error: implicit declaration of function ‘dma_mmap_coherent’
drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_get_base_sgt’:
drivers/media/v4l2-core/videobuf2-dma-contig.c:387: error: implicit declaration of function ‘dma_get_sgtable’

For architectures using dma_map_ops, dma_mmap_coherent() and
dma_get_sgtable() are provided in <asm-generic/dma-mapping-common.h>.

Parisc does not use dma_map_ops, hence it should implement them as inline
stubs using dma_common_mmap() and dma_common_get_sgtable().

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: linux-parisc@vger.kernel.org
---
 arch/parisc/include/asm/dma-mapping.h |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/arch/parisc/include/asm/dma-mapping.h b/arch/parisc/include/asm/dma-mapping.h
index 467bbd5..1fac0bf 100644
--- a/arch/parisc/include/asm/dma-mapping.h
+++ b/arch/parisc/include/asm/dma-mapping.h
@@ -238,4 +238,14 @@ void * sba_get_iommu(struct parisc_device *dev);
 /* At the moment, we panic on error for IOMMU resource exaustion */
 #define dma_mapping_error(dev, x)	0
 
+/* drivers/base/dma-mapping.c */
+extern int dma_common_mmap(struct device *dev, struct vm_area_struct *vma,
+			   void *cpu_addr, dma_addr_t dma_addr, size_t size);
+extern int dma_common_get_sgtable(struct device *dev, struct sg_table *sgt,
+				  void *cpu_addr, dma_addr_t dma_addr,
+				  size_t size);
+
+#define dma_mmap_coherent(d, v, c, h, s) dma_common_mmap(d, v, c, h, s)
+#define dma_get_sgtable(d, t, v, h, s) dma_common_get_sgtable(d, t, v, h, s)
+
 #endif
-- 
1.7.0.4

Re: [PATCH 8/9] parisc: Provide dma_mmap_coherent() and dma_get_sgtable()

[James Bottomley]

On Sun, 2013-01-13 at 11:44 +0100, Geert Uytterhoeven wrote:
> parisc/allmodconfig:
>=20
> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function =E2=80=98=
vb2_dc_mmap=E2=80=99:
> drivers/media/v4l2-core/videobuf2-dma-contig.c:204: error: implicit d=
eclaration of function =E2=80=98dma_mmap_coherent=E2=80=99
> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function =E2=80=98=
vb2_dc_get_base_sgt=E2=80=99:
> drivers/media/v4l2-core/videobuf2-dma-contig.c:387: error: implicit d=
eclaration of function =E2=80=98dma_get_sgtable=E2=80=99
>=20
> For architectures using dma_map_ops, dma_mmap_coherent() and
> dma_get_sgtable() are provided in <asm-generic/dma-mapping-common.h>.
>=20
> Parisc does not use dma_map_ops, hence it should implement them as in=
line
> stubs using dma_common_mmap() and dma_common_get_sgtable().
>=20
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> Cc: linux-parisc@vger.kernel.org
> ---
>  arch/parisc/include/asm/dma-mapping.h |   10 ++++++++++
>  1 files changed, 10 insertions(+), 0 deletions(-)
>=20
> diff --git a/arch/parisc/include/asm/dma-mapping.h b/arch/parisc/incl=
ude/asm/dma-mapping.h
> index 467bbd5..1fac0bf 100644
> --- a/arch/parisc/include/asm/dma-mapping.h
> +++ b/arch/parisc/include/asm/dma-mapping.h
> @@ -238,4 +238,14 @@ void * sba_get_iommu(struct parisc_device *dev);
>  /* At the moment, we panic on error for IOMMU resource exaustion */
>  #define dma_mapping_error(dev, x)	0
> =20
> +/* drivers/base/dma-mapping.c */
> +extern int dma_common_mmap(struct device *dev, struct vm_area_struct=
 *vma,
> +			   void *cpu_addr, dma_addr_t dma_addr, size_t size);
> +extern int dma_common_get_sgtable(struct device *dev, struct sg_tabl=
e *sgt,
> +				  void *cpu_addr, dma_addr_t dma_addr,
> +				  size_t size);
> +
> +#define dma_mmap_coherent(d, v, c, h, s) dma_common_mmap(d, v, c, h,=
 s)
> +#define dma_get_sgtable(d, t, v, h, s) dma_common_get_sgtable(d, t, =
v, h, s)

What is the documentation around these functions?  The name sounds
suspiciously like you want a mapping of a buffer coherent between kerne=
l
and user space, which doesn't seem possible for us given the API.  We'r=
e
a VIPT architecture, so the only way we can do this is to have the
actual vma user space address be congruent with cpu_addr.  How do we do
that if the vma and kernel addresses are already fixed?

In other words, either the interface is unusable by parisc, or the
common code definitely won't work for us.

James

Re: [PATCH 8/9] parisc: Provide dma_mmap_coherent() and dma_get_sgtable()

[James Bottomley]

On Sun, 2013-01-13 at 11:44 +0100, Geert Uytterhoeven wrote:
> parisc/allmodconfig:
> 
> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_mmap’:
> drivers/media/v4l2-core/videobuf2-dma-contig.c:204: error: implicit declaration of function ‘dma_mmap_coherent’
> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_get_base_sgt’:
> drivers/media/v4l2-core/videobuf2-dma-contig.c:387: error: implicit declaration of function ‘dma_get_sgtable’
> 
> For architectures using dma_map_ops, dma_mmap_coherent() and
> dma_get_sgtable() are provided in <asm-generic/dma-mapping-common.h>.
> 
> Parisc does not use dma_map_ops, hence it should implement them as inline
> stubs using dma_common_mmap() and dma_common_get_sgtable().
> 
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> Cc: linux-parisc@vger.kernel.org
> ---
>  arch/parisc/include/asm/dma-mapping.h |   10 ++++++++++
>  1 files changed, 10 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/parisc/include/asm/dma-mapping.h b/arch/parisc/include/asm/dma-mapping.h
> index 467bbd5..1fac0bf 100644
> --- a/arch/parisc/include/asm/dma-mapping.h
> +++ b/arch/parisc/include/asm/dma-mapping.h
> @@ -238,4 +238,14 @@ void * sba_get_iommu(struct parisc_device *dev);
>  /* At the moment, we panic on error for IOMMU resource exaustion */
>  #define dma_mapping_error(dev, x)	0
>  
> +/* drivers/base/dma-mapping.c */
> +extern int dma_common_mmap(struct device *dev, struct vm_area_struct *vma,
> +			   void *cpu_addr, dma_addr_t dma_addr, size_t size);
> +extern int dma_common_get_sgtable(struct device *dev, struct sg_table *sgt,
> +				  void *cpu_addr, dma_addr_t dma_addr,
> +				  size_t size);
> +
> +#define dma_mmap_coherent(d, v, c, h, s) dma_common_mmap(d, v, c, h, s)
> +#define dma_get_sgtable(d, t, v, h, s) dma_common_get_sgtable(d, t, v, h, s)

What is the documentation around these functions?  The name sounds
suspiciously like you want a mapping of a buffer coherent between kernel
and user space, which doesn't seem possible for us given the API.  We're
a VIPT architecture, so the only way we can do this is to have the
actual vma user space address be congruent with cpu_addr.  How do we do
that if the vma and kernel addresses are already fixed?

In other words, either the interface is unusable by parisc, or the
common code definitely won't work for us.

James

Re: [PATCH 8/9] parisc: Provide dma_mmap_coherent() and dma_get_sgtable()

[Geert Uytterhoeven]

On Sun, Jan 13, 2013 at 12:36 PM, James Bottomley
<James.Bottomley@hansenpartnership.com> wrote:
> On Sun, 2013-01-13 at 11:44 +0100, Geert Uytterhoeven wrote:
>> parisc/allmodconfig:
>>
>> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function =E2=80=98=
vb2_dc_mmap=E2=80=99:
>> drivers/media/v4l2-core/videobuf2-dma-contig.c:204: error: implicit =
declaration of function =E2=80=98dma_mmap_coherent=E2=80=99
>> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function =E2=80=98=
vb2_dc_get_base_sgt=E2=80=99:
>> drivers/media/v4l2-core/videobuf2-dma-contig.c:387: error: implicit =
declaration of function =E2=80=98dma_get_sgtable=E2=80=99
>>
>> For architectures using dma_map_ops, dma_mmap_coherent() and
>> dma_get_sgtable() are provided in <asm-generic/dma-mapping-common.h>=
=2E
>>
>> Parisc does not use dma_map_ops, hence it should implement them as i=
nline
>> stubs using dma_common_mmap() and dma_common_get_sgtable().
>>
>> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
>> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
>> Cc: linux-parisc@vger.kernel.org
>> ---
>>  arch/parisc/include/asm/dma-mapping.h |   10 ++++++++++
>>  1 files changed, 10 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/parisc/include/asm/dma-mapping.h b/arch/parisc/inc=
lude/asm/dma-mapping.h
>> index 467bbd5..1fac0bf 100644
>> --- a/arch/parisc/include/asm/dma-mapping.h
>> +++ b/arch/parisc/include/asm/dma-mapping.h
>> @@ -238,4 +238,14 @@ void * sba_get_iommu(struct parisc_device *dev)=
;
>>  /* At the moment, we panic on error for IOMMU resource exaustion */
>>  #define dma_mapping_error(dev, x)    0
>>
>> +/* drivers/base/dma-mapping.c */
>> +extern int dma_common_mmap(struct device *dev, struct vm_area_struc=
t *vma,
>> +                        void *cpu_addr, dma_addr_t dma_addr, size_t=
 size);
>> +extern int dma_common_get_sgtable(struct device *dev, struct sg_tab=
le *sgt,
>> +                               void *cpu_addr, dma_addr_t dma_addr,
>> +                               size_t size);
>> +
>> +#define dma_mmap_coherent(d, v, c, h, s) dma_common_mmap(d, v, c, h=
, s)
>> +#define dma_get_sgtable(d, t, v, h, s) dma_common_get_sgtable(d, t,=
 v, h, s)
>
> What is the documentation around these functions?  The name sounds
> suspiciously like you want a mapping of a buffer coherent between ker=
nel
> and user space, which doesn't seem possible for us given the API.  We=
're
> a VIPT architecture, so the only way we can do this is to have the
> actual vma user space address be congruent with cpu_addr.  How do we =
do
> that if the vma and kernel addresses are already fixed?

I was already afraid of this when seeing the "coherent" in the naming..=
=2E

> In other words, either the interface is unusable by parisc, or the
> common code definitely won't work for us.

So you probably want a static inline function that returns -EINVAL for =
now?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-=
m68k.org

In personal conversations with technical people, I call myself a hacker=
=2E But
when I'm talking to journalists I just say "programmer" or something li=
ke that.
                                -- Linus Torvalds

Re: [PATCH 8/9] parisc: Provide dma_mmap_coherent() and dma_get_sgtable()

[Geert Uytterhoeven]

On Sun, Jan 13, 2013 at 12:36 PM, James Bottomley
<James.Bottomley@hansenpartnership.com> wrote:
> On Sun, 2013-01-13 at 11:44 +0100, Geert Uytterhoeven wrote:
>> parisc/allmodconfig:
>>
>> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_mmap’:
>> drivers/media/v4l2-core/videobuf2-dma-contig.c:204: error: implicit declaration of function ‘dma_mmap_coherent’
>> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_get_base_sgt’:
>> drivers/media/v4l2-core/videobuf2-dma-contig.c:387: error: implicit declaration of function ‘dma_get_sgtable’
>>
>> For architectures using dma_map_ops, dma_mmap_coherent() and
>> dma_get_sgtable() are provided in <asm-generic/dma-mapping-common.h>.
>>
>> Parisc does not use dma_map_ops, hence it should implement them as inline
>> stubs using dma_common_mmap() and dma_common_get_sgtable().
>>
>> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
>> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
>> Cc: linux-parisc@vger.kernel.org
>> ---
>>  arch/parisc/include/asm/dma-mapping.h |   10 ++++++++++
>>  1 files changed, 10 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/parisc/include/asm/dma-mapping.h b/arch/parisc/include/asm/dma-mapping.h
>> index 467bbd5..1fac0bf 100644
>> --- a/arch/parisc/include/asm/dma-mapping.h
>> +++ b/arch/parisc/include/asm/dma-mapping.h
>> @@ -238,4 +238,14 @@ void * sba_get_iommu(struct parisc_device *dev);
>>  /* At the moment, we panic on error for IOMMU resource exaustion */
>>  #define dma_mapping_error(dev, x)    0
>>
>> +/* drivers/base/dma-mapping.c */
>> +extern int dma_common_mmap(struct device *dev, struct vm_area_struct *vma,
>> +                        void *cpu_addr, dma_addr_t dma_addr, size_t size);
>> +extern int dma_common_get_sgtable(struct device *dev, struct sg_table *sgt,
>> +                               void *cpu_addr, dma_addr_t dma_addr,
>> +                               size_t size);
>> +
>> +#define dma_mmap_coherent(d, v, c, h, s) dma_common_mmap(d, v, c, h, s)
>> +#define dma_get_sgtable(d, t, v, h, s) dma_common_get_sgtable(d, t, v, h, s)
>
> What is the documentation around these functions?  The name sounds
> suspiciously like you want a mapping of a buffer coherent between kernel
> and user space, which doesn't seem possible for us given the API.  We're
> a VIPT architecture, so the only way we can do this is to have the
> actual vma user space address be congruent with cpu_addr.  How do we do
> that if the vma and kernel addresses are already fixed?

I was already afraid of this when seeing the "coherent" in the naming...

> In other words, either the interface is unusable by parisc, or the
> common code definitely won't work for us.

So you probably want a static inline function that returns -EINVAL for now?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 8/9] parisc: Provide dma_mmap_coherent() and dma_get_sgtable()

[James Bottomley]

On Sun, 2013-01-13 at 14:12 +0100, Geert Uytterhoeven wrote:
> On Sun, Jan 13, 2013 at 12:36 PM, James Bottomley
> <James.Bottomley@hansenpartnership.com> wrote:
> > On Sun, 2013-01-13 at 11:44 +0100, Geert Uytterhoeven wrote:
> >> parisc/allmodconfig:
> >>
> >> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function =E2=80=
=98vb2_dc_mmap=E2=80=99:
> >> drivers/media/v4l2-core/videobuf2-dma-contig.c:204: error: implici=
t declaration of function =E2=80=98dma_mmap_coherent=E2=80=99
> >> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function =E2=80=
=98vb2_dc_get_base_sgt=E2=80=99:
> >> drivers/media/v4l2-core/videobuf2-dma-contig.c:387: error: implici=
t declaration of function =E2=80=98dma_get_sgtable=E2=80=99
> >>
> >> For architectures using dma_map_ops, dma_mmap_coherent() and
> >> dma_get_sgtable() are provided in <asm-generic/dma-mapping-common.=
h>.
> >>
> >> Parisc does not use dma_map_ops, hence it should implement them as=
 inline
> >> stubs using dma_common_mmap() and dma_common_get_sgtable().
> >>
> >> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> >> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> >> Cc: linux-parisc@vger.kernel.org
> >> ---
> >>  arch/parisc/include/asm/dma-mapping.h |   10 ++++++++++
> >>  1 files changed, 10 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/arch/parisc/include/asm/dma-mapping.h b/arch/parisc/i=
nclude/asm/dma-mapping.h
> >> index 467bbd5..1fac0bf 100644
> >> --- a/arch/parisc/include/asm/dma-mapping.h
> >> +++ b/arch/parisc/include/asm/dma-mapping.h
> >> @@ -238,4 +238,14 @@ void * sba_get_iommu(struct parisc_device *de=
v);
> >>  /* At the moment, we panic on error for IOMMU resource exaustion =
*/
> >>  #define dma_mapping_error(dev, x)    0
> >>
> >> +/* drivers/base/dma-mapping.c */
> >> +extern int dma_common_mmap(struct device *dev, struct vm_area_str=
uct *vma,
> >> +                        void *cpu_addr, dma_addr_t dma_addr, size=
_t size);
> >> +extern int dma_common_get_sgtable(struct device *dev, struct sg_t=
able *sgt,
> >> +                               void *cpu_addr, dma_addr_t dma_add=
r,
> >> +                               size_t size);
> >> +
> >> +#define dma_mmap_coherent(d, v, c, h, s) dma_common_mmap(d, v, c,=
 h, s)
> >> +#define dma_get_sgtable(d, t, v, h, s) dma_common_get_sgtable(d, =
t, v, h, s)
> >
> > What is the documentation around these functions?  The name sounds
> > suspiciously like you want a mapping of a buffer coherent between k=
ernel
> > and user space, which doesn't seem possible for us given the API.  =
We're
> > a VIPT architecture, so the only way we can do this is to have the
> > actual vma user space address be congruent with cpu_addr.  How do w=
e do
> > that if the vma and kernel addresses are already fixed?
>=20
> I was already afraid of this when seeing the "coherent" in the naming=
=2E..

Afraid of what, that the interface won't actually work for VIPT or that
we need special functions?

We won't be the only ones: MIPS at least will have this issue.

But I would like to clear up whether we can actually use this on PA or
not.

> > In other words, either the interface is unusable by parisc, or the
> > common code definitely won't work for us.
>=20
> So you probably want a static inline function that returns -EINVAL fo=
r now?

Yes, that should work (provided the consumers handle the return code
correctly).

James

Re: [PATCH 8/9] parisc: Provide dma_mmap_coherent() and dma_get_sgtable()

[James Bottomley]

On Sun, 2013-01-13 at 14:12 +0100, Geert Uytterhoeven wrote:
> On Sun, Jan 13, 2013 at 12:36 PM, James Bottomley
> <James.Bottomley@hansenpartnership.com> wrote:
> > On Sun, 2013-01-13 at 11:44 +0100, Geert Uytterhoeven wrote:
> >> parisc/allmodconfig:
> >>
> >> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_mmap’:
> >> drivers/media/v4l2-core/videobuf2-dma-contig.c:204: error: implicit declaration of function ‘dma_mmap_coherent’
> >> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_get_base_sgt’:
> >> drivers/media/v4l2-core/videobuf2-dma-contig.c:387: error: implicit declaration of function ‘dma_get_sgtable’
> >>
> >> For architectures using dma_map_ops, dma_mmap_coherent() and
> >> dma_get_sgtable() are provided in <asm-generic/dma-mapping-common.h>.
> >>
> >> Parisc does not use dma_map_ops, hence it should implement them as inline
> >> stubs using dma_common_mmap() and dma_common_get_sgtable().
> >>
> >> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> >> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> >> Cc: linux-parisc@vger.kernel.org
> >> ---
> >>  arch/parisc/include/asm/dma-mapping.h |   10 ++++++++++
> >>  1 files changed, 10 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/arch/parisc/include/asm/dma-mapping.h b/arch/parisc/include/asm/dma-mapping.h
> >> index 467bbd5..1fac0bf 100644
> >> --- a/arch/parisc/include/asm/dma-mapping.h
> >> +++ b/arch/parisc/include/asm/dma-mapping.h
> >> @@ -238,4 +238,14 @@ void * sba_get_iommu(struct parisc_device *dev);
> >>  /* At the moment, we panic on error for IOMMU resource exaustion */
> >>  #define dma_mapping_error(dev, x)    0
> >>
> >> +/* drivers/base/dma-mapping.c */
> >> +extern int dma_common_mmap(struct device *dev, struct vm_area_struct *vma,
> >> +                        void *cpu_addr, dma_addr_t dma_addr, size_t size);
> >> +extern int dma_common_get_sgtable(struct device *dev, struct sg_table *sgt,
> >> +                               void *cpu_addr, dma_addr_t dma_addr,
> >> +                               size_t size);
> >> +
> >> +#define dma_mmap_coherent(d, v, c, h, s) dma_common_mmap(d, v, c, h, s)
> >> +#define dma_get_sgtable(d, t, v, h, s) dma_common_get_sgtable(d, t, v, h, s)
> >
> > What is the documentation around these functions?  The name sounds
> > suspiciously like you want a mapping of a buffer coherent between kernel
> > and user space, which doesn't seem possible for us given the API.  We're
> > a VIPT architecture, so the only way we can do this is to have the
> > actual vma user space address be congruent with cpu_addr.  How do we do
> > that if the vma and kernel addresses are already fixed?
> 
> I was already afraid of this when seeing the "coherent" in the naming...

Afraid of what, that the interface won't actually work for VIPT or that
we need special functions?

We won't be the only ones: MIPS at least will have this issue.

But I would like to clear up whether we can actually use this on PA or
not.

> > In other words, either the interface is unusable by parisc, or the
> > common code definitely won't work for us.
> 
> So you probably want a static inline function that returns -EINVAL for now?

Yes, that should work (provided the consumers handle the return code
correctly).

James

Re: [PATCH 8/9] parisc: Provide dma_mmap_coherent() and dma_get_sgtable()

[Geert Uytterhoeven]

On Sun, Jan 13, 2013 at 2:49 PM, James Bottomley
<James.Bottomley@hansenpartnership.com> wrote:
> On Sun, 2013-01-13 at 14:12 +0100, Geert Uytterhoeven wrote:
>> On Sun, Jan 13, 2013 at 12:36 PM, James Bottomley
>> <James.Bottomley@hansenpartnership.com> wrote:
>> > On Sun, 2013-01-13 at 11:44 +0100, Geert Uytterhoeven wrote:
>> >> parisc/allmodconfig:
>> >>
>> >> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function =E2=80=
=98vb2_dc_mmap=E2=80=99:
>> >> drivers/media/v4l2-core/videobuf2-dma-contig.c:204: error: implic=
it declaration of function =E2=80=98dma_mmap_coherent=E2=80=99
>> >> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function =E2=80=
=98vb2_dc_get_base_sgt=E2=80=99:
>> >> drivers/media/v4l2-core/videobuf2-dma-contig.c:387: error: implic=
it declaration of function =E2=80=98dma_get_sgtable=E2=80=99
>> >>
>> >> For architectures using dma_map_ops, dma_mmap_coherent() and
>> >> dma_get_sgtable() are provided in <asm-generic/dma-mapping-common=
=2Eh>.
>> >>
>> >> Parisc does not use dma_map_ops, hence it should implement them a=
s inline
>> >> stubs using dma_common_mmap() and dma_common_get_sgtable().
>> >>
>> >> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
>> >> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
>> >> Cc: linux-parisc@vger.kernel.org
>> >> ---
>> >>  arch/parisc/include/asm/dma-mapping.h |   10 ++++++++++
>> >>  1 files changed, 10 insertions(+), 0 deletions(-)
>> >>
>> >> diff --git a/arch/parisc/include/asm/dma-mapping.h b/arch/parisc/=
include/asm/dma-mapping.h
>> >> index 467bbd5..1fac0bf 100644
>> >> --- a/arch/parisc/include/asm/dma-mapping.h
>> >> +++ b/arch/parisc/include/asm/dma-mapping.h
>> >> @@ -238,4 +238,14 @@ void * sba_get_iommu(struct parisc_device *d=
ev);
>> >>  /* At the moment, we panic on error for IOMMU resource exaustion=
 */
>> >>  #define dma_mapping_error(dev, x)    0
>> >>
>> >> +/* drivers/base/dma-mapping.c */
>> >> +extern int dma_common_mmap(struct device *dev, struct vm_area_st=
ruct *vma,
>> >> +                        void *cpu_addr, dma_addr_t dma_addr, siz=
e_t size);
>> >> +extern int dma_common_get_sgtable(struct device *dev, struct sg_=
table *sgt,
>> >> +                               void *cpu_addr, dma_addr_t dma_ad=
dr,
>> >> +                               size_t size);
>> >> +
>> >> +#define dma_mmap_coherent(d, v, c, h, s) dma_common_mmap(d, v, c=
, h, s)
>> >> +#define dma_get_sgtable(d, t, v, h, s) dma_common_get_sgtable(d,=
 t, v, h, s)
>> >
>> > What is the documentation around these functions?  The name sounds
>> > suspiciously like you want a mapping of a buffer coherent between =
kernel
>> > and user space, which doesn't seem possible for us given the API. =
 We're
>> > a VIPT architecture, so the only way we can do this is to have the
>> > actual vma user space address be congruent with cpu_addr.  How do =
we do
>> > that if the vma and kernel addresses are already fixed?
>>
>> I was already afraid of this when seeing the "coherent" in the namin=
g...
>
> Afraid of what, that the interface won't actually work for VIPT or th=
at
> we need special functions?

That it wouldn't work on parisc.

> We won't be the only ones: MIPS at least will have this issue.

MIPS uses <asm-generic/dma-mapping-common.h>, BUT has some ugly
#ifdefery in sound/core/pcm_native.c:snd_pcm_lib_default_mmap().

BTW, does current ALSA mmap() work on parisc?

> But I would like to clear up whether we can actually use this on PA o=
r
> not.
>
>> > In other words, either the interface is unusable by parisc, or the
>> > common code definitely won't work for us.
>>
>> So you probably want a static inline function that returns -EINVAL f=
or now?
>
> Yes, that should work (provided the consumers handle the return code
> correctly).

OK. Let's wait (for Marek's documentation) and see whether we can fix i=
t
for real in 3.8, or have to resort to -EINVAL.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-=
m68k.org

In personal conversations with technical people, I call myself a hacker=
=2E But
when I'm talking to journalists I just say "programmer" or something li=
ke that.
                                -- Linus Torvalds

Re: [PATCH 8/9] parisc: Provide dma_mmap_coherent() and dma_get_sgtable()

[Geert Uytterhoeven]

On Sun, Jan 13, 2013 at 2:49 PM, James Bottomley
<James.Bottomley@hansenpartnership.com> wrote:
> On Sun, 2013-01-13 at 14:12 +0100, Geert Uytterhoeven wrote:
>> On Sun, Jan 13, 2013 at 12:36 PM, James Bottomley
>> <James.Bottomley@hansenpartnership.com> wrote:
>> > On Sun, 2013-01-13 at 11:44 +0100, Geert Uytterhoeven wrote:
>> >> parisc/allmodconfig:
>> >>
>> >> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_mmap’:
>> >> drivers/media/v4l2-core/videobuf2-dma-contig.c:204: error: implicit declaration of function ‘dma_mmap_coherent’
>> >> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_get_base_sgt’:
>> >> drivers/media/v4l2-core/videobuf2-dma-contig.c:387: error: implicit declaration of function ‘dma_get_sgtable’
>> >>
>> >> For architectures using dma_map_ops, dma_mmap_coherent() and
>> >> dma_get_sgtable() are provided in <asm-generic/dma-mapping-common.h>.
>> >>
>> >> Parisc does not use dma_map_ops, hence it should implement them as inline
>> >> stubs using dma_common_mmap() and dma_common_get_sgtable().
>> >>
>> >> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
>> >> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
>> >> Cc: linux-parisc@vger.kernel.org
>> >> ---
>> >>  arch/parisc/include/asm/dma-mapping.h |   10 ++++++++++
>> >>  1 files changed, 10 insertions(+), 0 deletions(-)
>> >>
>> >> diff --git a/arch/parisc/include/asm/dma-mapping.h b/arch/parisc/include/asm/dma-mapping.h
>> >> index 467bbd5..1fac0bf 100644
>> >> --- a/arch/parisc/include/asm/dma-mapping.h
>> >> +++ b/arch/parisc/include/asm/dma-mapping.h
>> >> @@ -238,4 +238,14 @@ void * sba_get_iommu(struct parisc_device *dev);
>> >>  /* At the moment, we panic on error for IOMMU resource exaustion */
>> >>  #define dma_mapping_error(dev, x)    0
>> >>
>> >> +/* drivers/base/dma-mapping.c */
>> >> +extern int dma_common_mmap(struct device *dev, struct vm_area_struct *vma,
>> >> +                        void *cpu_addr, dma_addr_t dma_addr, size_t size);
>> >> +extern int dma_common_get_sgtable(struct device *dev, struct sg_table *sgt,
>> >> +                               void *cpu_addr, dma_addr_t dma_addr,
>> >> +                               size_t size);
>> >> +
>> >> +#define dma_mmap_coherent(d, v, c, h, s) dma_common_mmap(d, v, c, h, s)
>> >> +#define dma_get_sgtable(d, t, v, h, s) dma_common_get_sgtable(d, t, v, h, s)
>> >
>> > What is the documentation around these functions?  The name sounds
>> > suspiciously like you want a mapping of a buffer coherent between kernel
>> > and user space, which doesn't seem possible for us given the API.  We're
>> > a VIPT architecture, so the only way we can do this is to have the
>> > actual vma user space address be congruent with cpu_addr.  How do we do
>> > that if the vma and kernel addresses are already fixed?
>>
>> I was already afraid of this when seeing the "coherent" in the naming...
>
> Afraid of what, that the interface won't actually work for VIPT or that
> we need special functions?

That it wouldn't work on parisc.

> We won't be the only ones: MIPS at least will have this issue.

MIPS uses <asm-generic/dma-mapping-common.h>, BUT has some ugly
#ifdefery in sound/core/pcm_native.c:snd_pcm_lib_default_mmap().

BTW, does current ALSA mmap() work on parisc?

> But I would like to clear up whether we can actually use this on PA or
> not.
>
>> > In other words, either the interface is unusable by parisc, or the
>> > common code definitely won't work for us.
>>
>> So you probably want a static inline function that returns -EINVAL for now?
>
> Yes, that should work (provided the consumers handle the return code
> correctly).

OK. Let's wait (for Marek's documentation) and see whether we can fix it
for real in 3.8, or have to resort to -EINVAL.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 8/9] parisc: Provide dma_mmap_coherent() and dma_get_sgtable()

[James Bottomley]

On Sun, 2013-01-13 at 15:52 +0100, Geert Uytterhoeven wrote:
> On Sun, Jan 13, 2013 at 2:49 PM, James Bottomley
> <James.Bottomley@hansenpartnership.com> wrote:
> > On Sun, 2013-01-13 at 14:12 +0100, Geert Uytterhoeven wrote:
> >> On Sun, Jan 13, 2013 at 12:36 PM, James Bottomley
> >> <James.Bottomley@hansenpartnership.com> wrote:
> >> > On Sun, 2013-01-13 at 11:44 +0100, Geert Uytterhoeven wrote:
> >> >> parisc/allmodconfig:
> >> >>
> >> >> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function =E2=
=80=98vb2_dc_mmap=E2=80=99:
> >> >> drivers/media/v4l2-core/videobuf2-dma-contig.c:204: error: impl=
icit declaration of function =E2=80=98dma_mmap_coherent=E2=80=99
> >> >> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function =E2=
=80=98vb2_dc_get_base_sgt=E2=80=99:
> >> >> drivers/media/v4l2-core/videobuf2-dma-contig.c:387: error: impl=
icit declaration of function =E2=80=98dma_get_sgtable=E2=80=99
> >> >>
> >> >> For architectures using dma_map_ops, dma_mmap_coherent() and
> >> >> dma_get_sgtable() are provided in <asm-generic/dma-mapping-comm=
on.h>.
> >> >>
> >> >> Parisc does not use dma_map_ops, hence it should implement them=
 as inline
> >> >> stubs using dma_common_mmap() and dma_common_get_sgtable().
> >> >>
> >> >> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> >> >> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> >> >> Cc: linux-parisc@vger.kernel.org
> >> >> ---
> >> >>  arch/parisc/include/asm/dma-mapping.h |   10 ++++++++++
> >> >>  1 files changed, 10 insertions(+), 0 deletions(-)
> >> >>
> >> >> diff --git a/arch/parisc/include/asm/dma-mapping.h b/arch/paris=
c/include/asm/dma-mapping.h
> >> >> index 467bbd5..1fac0bf 100644
> >> >> --- a/arch/parisc/include/asm/dma-mapping.h
> >> >> +++ b/arch/parisc/include/asm/dma-mapping.h
> >> >> @@ -238,4 +238,14 @@ void * sba_get_iommu(struct parisc_device =
*dev);
> >> >>  /* At the moment, we panic on error for IOMMU resource exausti=
on */
> >> >>  #define dma_mapping_error(dev, x)    0
> >> >>
> >> >> +/* drivers/base/dma-mapping.c */
> >> >> +extern int dma_common_mmap(struct device *dev, struct vm_area_=
struct *vma,
> >> >> +                        void *cpu_addr, dma_addr_t dma_addr, s=
ize_t size);
> >> >> +extern int dma_common_get_sgtable(struct device *dev, struct s=
g_table *sgt,
> >> >> +                               void *cpu_addr, dma_addr_t dma_=
addr,
> >> >> +                               size_t size);
> >> >> +
> >> >> +#define dma_mmap_coherent(d, v, c, h, s) dma_common_mmap(d, v,=
 c, h, s)
> >> >> +#define dma_get_sgtable(d, t, v, h, s) dma_common_get_sgtable(=
d, t, v, h, s)
> >> >
> >> > What is the documentation around these functions?  The name soun=
ds
> >> > suspiciously like you want a mapping of a buffer coherent betwee=
n kernel
> >> > and user space, which doesn't seem possible for us given the API=
=2E  We're
> >> > a VIPT architecture, so the only way we can do this is to have t=
he
> >> > actual vma user space address be congruent with cpu_addr.  How d=
o we do
> >> > that if the vma and kernel addresses are already fixed?
> >>
> >> I was already afraid of this when seeing the "coherent" in the nam=
ing...
> >
> > Afraid of what, that the interface won't actually work for VIPT or =
that
> > we need special functions?
>=20
> That it wouldn't work on parisc.

Hmm, so that would seem to be a serious deficiency with the API then.

> > We won't be the only ones: MIPS at least will have this issue.
>=20
> MIPS uses <asm-generic/dma-mapping-common.h>, BUT has some ugly
> #ifdefery in sound/core/pcm_native.c:snd_pcm_lib_default_mmap().
>=20
> BTW, does current ALSA mmap() work on parisc?

Yes, we used to have the same #ifdeffery, but in the alsa core ... I
can't remember how we fixed it for our two sound drivers (harmony and
AD1889).

> > But I would like to clear up whether we can actually use this on PA=
 or
> > not.
> >
> >> > In other words, either the interface is unusable by parisc, or t=
he
> >> > common code definitely won't work for us.
> >>
> >> So you probably want a static inline function that returns -EINVAL=
 for now?
> >
> > Yes, that should work (provided the consumers handle the return cod=
e
> > correctly).
>=20
> OK. Let's wait (for Marek's documentation) and see whether we can fix=
 it
> for real in 3.8, or have to resort to -EINVAL.

The way we could fix this is by altering the kernel CPU address, using =
a
kmap area, so we have to change the virtual address by which the kernel
refers to this page to be coherent with user space, but the API is two
narrow to allow that, so perhaps we need to redesign how the API works.

James

Re: [PATCH 8/9] parisc: Provide dma_mmap_coherent() and dma_get_sgtable()

[James Bottomley]

On Sun, 2013-01-13 at 15:52 +0100, Geert Uytterhoeven wrote:
> On Sun, Jan 13, 2013 at 2:49 PM, James Bottomley
> <James.Bottomley@hansenpartnership.com> wrote:
> > On Sun, 2013-01-13 at 14:12 +0100, Geert Uytterhoeven wrote:
> >> On Sun, Jan 13, 2013 at 12:36 PM, James Bottomley
> >> <James.Bottomley@hansenpartnership.com> wrote:
> >> > On Sun, 2013-01-13 at 11:44 +0100, Geert Uytterhoeven wrote:
> >> >> parisc/allmodconfig:
> >> >>
> >> >> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_mmap’:
> >> >> drivers/media/v4l2-core/videobuf2-dma-contig.c:204: error: implicit declaration of function ‘dma_mmap_coherent’
> >> >> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_get_base_sgt’:
> >> >> drivers/media/v4l2-core/videobuf2-dma-contig.c:387: error: implicit declaration of function ‘dma_get_sgtable’
> >> >>
> >> >> For architectures using dma_map_ops, dma_mmap_coherent() and
> >> >> dma_get_sgtable() are provided in <asm-generic/dma-mapping-common.h>.
> >> >>
> >> >> Parisc does not use dma_map_ops, hence it should implement them as inline
> >> >> stubs using dma_common_mmap() and dma_common_get_sgtable().
> >> >>
> >> >> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> >> >> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> >> >> Cc: linux-parisc@vger.kernel.org
> >> >> ---
> >> >>  arch/parisc/include/asm/dma-mapping.h |   10 ++++++++++
> >> >>  1 files changed, 10 insertions(+), 0 deletions(-)
> >> >>
> >> >> diff --git a/arch/parisc/include/asm/dma-mapping.h b/arch/parisc/include/asm/dma-mapping.h
> >> >> index 467bbd5..1fac0bf 100644
> >> >> --- a/arch/parisc/include/asm/dma-mapping.h
> >> >> +++ b/arch/parisc/include/asm/dma-mapping.h
> >> >> @@ -238,4 +238,14 @@ void * sba_get_iommu(struct parisc_device *dev);
> >> >>  /* At the moment, we panic on error for IOMMU resource exaustion */
> >> >>  #define dma_mapping_error(dev, x)    0
> >> >>
> >> >> +/* drivers/base/dma-mapping.c */
> >> >> +extern int dma_common_mmap(struct device *dev, struct vm_area_struct *vma,
> >> >> +                        void *cpu_addr, dma_addr_t dma_addr, size_t size);
> >> >> +extern int dma_common_get_sgtable(struct device *dev, struct sg_table *sgt,
> >> >> +                               void *cpu_addr, dma_addr_t dma_addr,
> >> >> +                               size_t size);
> >> >> +
> >> >> +#define dma_mmap_coherent(d, v, c, h, s) dma_common_mmap(d, v, c, h, s)
> >> >> +#define dma_get_sgtable(d, t, v, h, s) dma_common_get_sgtable(d, t, v, h, s)
> >> >
> >> > What is the documentation around these functions?  The name sounds
> >> > suspiciously like you want a mapping of a buffer coherent between kernel
> >> > and user space, which doesn't seem possible for us given the API.  We're
> >> > a VIPT architecture, so the only way we can do this is to have the
> >> > actual vma user space address be congruent with cpu_addr.  How do we do
> >> > that if the vma and kernel addresses are already fixed?
> >>
> >> I was already afraid of this when seeing the "coherent" in the naming...
> >
> > Afraid of what, that the interface won't actually work for VIPT or that
> > we need special functions?
> 
> That it wouldn't work on parisc.

Hmm, so that would seem to be a serious deficiency with the API then.

> > We won't be the only ones: MIPS at least will have this issue.
> 
> MIPS uses <asm-generic/dma-mapping-common.h>, BUT has some ugly
> #ifdefery in sound/core/pcm_native.c:snd_pcm_lib_default_mmap().
> 
> BTW, does current ALSA mmap() work on parisc?

Yes, we used to have the same #ifdeffery, but in the alsa core ... I
can't remember how we fixed it for our two sound drivers (harmony and
AD1889).

> > But I would like to clear up whether we can actually use this on PA or
> > not.
> >
> >> > In other words, either the interface is unusable by parisc, or the
> >> > common code definitely won't work for us.
> >>
> >> So you probably want a static inline function that returns -EINVAL for now?
> >
> > Yes, that should work (provided the consumers handle the return code
> > correctly).
> 
> OK. Let's wait (for Marek's documentation) and see whether we can fix it
> for real in 3.8, or have to resort to -EINVAL.

The way we could fix this is by altering the kernel CPU address, using a
kmap area, so we have to change the virtual address by which the kernel
refers to this page to be coherent with user space, but the API is two
narrow to allow that, so perhaps we need to redesign how the API works.

James

[PATCH 9/9] xtensa: Provide dma_mmap_coherent() and dma_get_sgtable()

[Geert Uytterhoeven]

xtensa/allmodconfig:

drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_mmap’:
drivers/media/v4l2-core/videobuf2-dma-contig.c:204: error: implicit declaration of function ‘dma_mmap_coherent’
drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_get_base_sgt’:
drivers/media/v4l2-core/videobuf2-dma-contig.c:387: error: implicit declaration of function ‘dma_get_sgtable’

For architectures using dma_map_ops, dma_mmap_coherent() and
dma_get_sgtable() are provided in <asm-generic/dma-mapping-common.h>.

Xtensa does not use dma_map_ops, hence it should implement them as inline
stubs using dma_common_mmap() and dma_common_get_sgtable().

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: linux-xtensa@linux-xtensa.org
---
 arch/xtensa/include/asm/dma-mapping.h |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/arch/xtensa/include/asm/dma-mapping.h b/arch/xtensa/include/asm/dma-mapping.h
index 4acb5fe..9b778fc 100644
--- a/arch/xtensa/include/asm/dma-mapping.h
+++ b/arch/xtensa/include/asm/dma-mapping.h
@@ -170,4 +170,14 @@ dma_cache_sync(struct device *dev, void *vaddr, size_t size,
 	consistent_sync(vaddr, size, direction);
 }
 
+/* drivers/base/dma-mapping.c */
+extern int dma_common_mmap(struct device *dev, struct vm_area_struct *vma,
+			   void *cpu_addr, dma_addr_t dma_addr, size_t size);
+extern int dma_common_get_sgtable(struct device *dev, struct sg_table *sgt,
+				  void *cpu_addr, dma_addr_t dma_addr,
+				  size_t size);
+
+#define dma_mmap_coherent(d, v, c, h, s) dma_common_mmap(d, v, c, h, s)
+#define dma_get_sgtable(d, t, v, h, s) dma_common_get_sgtable(d, t, v, h, s)
+
 #endif	/* _XTENSA_DMA_MAPPING_H */
-- 
1.7.0.4

Re: [PATCH 1/9] avr32: Provide dma_mmap_coherent() and dma_get_sgtable()

[Hans-Christian Egtvedt]

Around Sun 13 Jan 2013 11:44:42 +0100 or thereabout, Geert Uytterhoeven wrote:
> avr32/allmodconfig:
> 
> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_mmap’:
> drivers/media/v4l2-core/videobuf2-dma-contig.c:204: error: implicit declaration of function ‘dma_mmap_coherent’
> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_get_base_sgt’:
> drivers/media/v4l2-core/videobuf2-dma-contig.c:387: error: implicit declaration of function ‘dma_get_sgtable’
> 
> For architectures using dma_map_ops, dma_mmap_coherent() and
> dma_get_sgtable() are provided in <asm-generic/dma-mapping-common.h>.
> 
> Avr32 does not use dma_map_ops, hence it should implement them as inline
> stubs using dma_common_mmap() and dma_common_get_sgtable().

Thanks for fixing.

> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> Cc: Haavard Skinnemoen <hskinnemoen@gmail.com>
> Cc: Hans-Christian Egtvedt <egtvedt@samfundet.no>

Acked-by: Hans-Christian Egtvedt <egtvedt@samfundet.no>

<snipp diff>

-- 
mvh
Hans-Christian Egtvedt