[PATCH v3 0/4] Use dma_mmap_coherent to support IOMMU mapper

There is a dma_mmap_coherent() API in some architectures.  This API
provides a mmap function for memory allocated by dma_alloc_coherent().
Some drivers mmap a dma_addr_t returned by dma_alloc_coherent() as a
physical address.  But such drivers do not work correctly when IOMMU
mapper is used.

v3:
- Remove an unnecessary line which sets page protection bits.
v2:
- Rebase on fbdev-next branch of
  git://github.com/schandinat/linux-2.6.git.
- Initialize .fb_mmap in both sh_mobile_lcdc_overlay_ops and
  sh_mobile_lcdc_ops.
- Add Laurent's clean up patch.

Hideki EIRAKU (3):
  ARM: dma-mapping: define ARCH_HAS_DMA_MMAP_COHERENT
  media: videobuf2-dma-contig: use dma_mmap_coherent if available
  fbdev: sh_mobile_lcdc: use dma_mmap_coherent if available

Laurent Pinchart (1):
  ALSA: pcm - Don't define ARCH_HAS_DMA_MMAP_COHERENT privately for ARM

 arch/arm/include/asm/dma-mapping.h         |    1 +
 drivers/media/video/videobuf2-dma-contig.c |   17 +++++++++++++++++
 drivers/video/sh_mobile_lcdcfb.c           |   28 ++++++++++++++++++++++++++++
 sound/core/pcm_native.c                    |    7 -------
 4 files changed, 46 insertions(+), 7 deletions(-)

ARCH_HAS_DMA_MMAP_COHERENT indicates that there is dma_mmap_coherent() API
in this architecture.  The name is already defined in PowerPC.

Signed-off-by: Hideki EIRAKU <redacted>
---
 arch/arm/include/asm/dma-mapping.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/arm/include/asm/dma-mapping.h b/arch/arm/include/asm/dma-mapping.h
index bbef15d..f41cd30 100644
--- a/arch/arm/include/asm/dma-mapping.h
+++ b/arch/arm/include/asm/dma-mapping.h

@@ -187,6 +187,7 @@ extern int arm_dma_mmap(struct device *dev, struct vm_area_struct *vma,
 			struct dma_attrs *attrs);
 
 #define dma_mmap_coherent(d, v, c, h, s) dma_mmap_attrs(d, v, c, h, s, NULL)
+#define ARCH_HAS_DMA_MMAP_COHERENT
 
 static inline int dma_mmap_attrs(struct device *dev, struct vm_area_struct *vma,
 				  void *cpu_addr, dma_addr_t dma_addr,

-- 
1.7.0.4

From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

The ARM architecture now defines ARCH_HAS_DMA_MMAP_COHERENT, there's no
need to define it privately anymore.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 sound/core/pcm_native.c |    7 -------
 1 files changed, 0 insertions(+), 7 deletions(-)

diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index 53b5ada..84ead60 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c

@@ -3156,13 +3156,6 @@ static const struct vm_operations_struct snd_pcm_vm_ops_data_fault = {
 	.fault =	snd_pcm_mmap_data_fault,
 };
 
-#ifndef ARCH_HAS_DMA_MMAP_COHERENT
-/* This should be defined / handled globally! */
-#ifdef CONFIG_ARM
-#define ARCH_HAS_DMA_MMAP_COHERENT
-#endif
-#endif
-
 /*
  * mmap the DMA buffer on RAM
  */

-- 
1.7.0.4

Previously the vb2_dma_contig_mmap() function was using a dma_addr_t as a
physical address.  The two addressses are not necessarily the same.
For example, when using the IOMMU funtion on certain platforms, dma_addr_t
addresses are not directly mappable physical address.
dma_mmap_coherent() maps the address correctly.
It is available on ARM platforms.

Signed-off-by: Hideki EIRAKU <redacted>
---
 drivers/media/video/videobuf2-dma-contig.c |   17 +++++++++++++++++
 1 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/drivers/media/video/videobuf2-dma-contig.c b/drivers/media/video/videobuf2-dma-contig.c
index 4b71326..7eee9c5 100644
--- a/drivers/media/video/videobuf2-dma-contig.c
+++ b/drivers/media/video/videobuf2-dma-contig.c

@@ -101,14 +101,31 @@ static unsigned int vb2_dma_contig_num_users(void *buf_priv)
 static int vb2_dma_contig_mmap(void *buf_priv, struct vm_area_struct *vma)
 {
 	struct vb2_dc_buf *buf = buf_priv;
+#ifdef ARCH_HAS_DMA_MMAP_COHERENT
+	int ret;
+#endif
 
 	if (!buf) {
 		printk(KERN_ERR "No buffer to map\n");
 		return -EINVAL;
 	}
 
+#ifdef ARCH_HAS_DMA_MMAP_COHERENT
+	ret = dma_mmap_coherent(buf->conf->dev, vma, buf->vaddr, buf->dma_addr,
+				buf->size);
+	if (ret) {
+		pr_err("Remapping memory failed, error: %d\n", ret);
+		return ret;
+	}
+	vma->vm_flags |= VM_DONTEXPAND | VM_RESERVED;
+	vma->vm_private_data = &buf->handler;
+	vma->vm_ops = &vb2_common_vm_ops;
+	vma->vm_ops->open(vma);
+	return 0;
+#else
 	return vb2_mmap_pfn_range(vma, buf->dma_addr, buf->size,
 				  &vb2_common_vm_ops, &buf->handler);
+#endif
 }
 
 static void *vb2_dma_contig_get_userptr(void *alloc_ctx, unsigned long vaddr,

-- 
1.7.0.4

fb_mmap() implemented in fbmem.c uses smem_start as the physical
address of the frame buffer.  In the sh_mobile_lcdc driver, the
smem_start is a dma_addr_t that is not a physical address when IOMMU is
enabled.  dma_mmap_coherent() maps the address correctly.  It is
available on ARM platforms.

Signed-off-by: Hideki EIRAKU <redacted>
---
 drivers/video/sh_mobile_lcdcfb.c |   28 ++++++++++++++++++++++++++++
 1 files changed, 28 insertions(+), 0 deletions(-)

diff --git a/drivers/video/sh_mobile_lcdcfb.c b/drivers/video/sh_mobile_lcdcfb.c
index 8cb653b..c8cba7a 100644
--- a/drivers/video/sh_mobile_lcdcfb.c
+++ b/drivers/video/sh_mobile_lcdcfb.c

@@ -1614,6 +1614,17 @@ static int sh_mobile_lcdc_overlay_blank(int blank, struct fb_info *info)
 	return 1;
 }
 
+#ifdef ARCH_HAS_DMA_MMAP_COHERENT
+static int
+sh_mobile_lcdc_overlay_mmap(struct fb_info *info, struct vm_area_struct *vma)
+{
+	struct sh_mobile_lcdc_overlay *ovl = info->par;
+
+	return dma_mmap_coherent(ovl->channel->lcdc->dev, vma, ovl->fb_mem,
+				 ovl->dma_handle, ovl->fb_size);
+}
+#endif
+
 static struct fb_ops sh_mobile_lcdc_overlay_ops = {
 	.owner          = THIS_MODULE,
 	.fb_read        = fb_sys_read,

@@ -1626,6 +1637,9 @@ static struct fb_ops sh_mobile_lcdc_overlay_ops = {
 	.fb_ioctl       = sh_mobile_lcdc_overlay_ioctl,
 	.fb_check_var	= sh_mobile_lcdc_overlay_check_var,
 	.fb_set_par	= sh_mobile_lcdc_overlay_set_par,
+#ifdef ARCH_HAS_DMA_MMAP_COHERENT
+	.fb_mmap	= sh_mobile_lcdc_overlay_mmap,
+#endif
 };
 
 static void

@@ -2093,6 +2107,17 @@ static int sh_mobile_lcdc_blank(int blank, struct fb_info *info)
 	return 0;
 }
 
+#ifdef ARCH_HAS_DMA_MMAP_COHERENT
+static int
+sh_mobile_lcdc_mmap(struct fb_info *info, struct vm_area_struct *vma)
+{
+	struct sh_mobile_lcdc_chan *ch = info->par;
+
+	return dma_mmap_coherent(ch->lcdc->dev, vma, ch->fb_mem,
+				 ch->dma_handle, ch->fb_size);
+}
+#endif
+
 static struct fb_ops sh_mobile_lcdc_ops = {
 	.owner          = THIS_MODULE,
 	.fb_setcolreg	= sh_mobile_lcdc_setcolreg,

@@ -2108,6 +2133,9 @@ static struct fb_ops sh_mobile_lcdc_ops = {
 	.fb_release	= sh_mobile_lcdc_release,
 	.fb_check_var	= sh_mobile_lcdc_check_var,
 	.fb_set_par	= sh_mobile_lcdc_set_par,
+#ifdef ARCH_HAS_DMA_MMAP_COHERENT
+	.fb_mmap	= sh_mobile_lcdc_mmap,
+#endif
 };
 
 static void

-- 
1.7.0.4

Hi Eiraku-san,

On Monday 06 August 2012 18:55:24 Hideki EIRAKU wrote:

fb_mmap() implemented in fbmem.c uses smem_start as the physical
address of the frame buffer.  In the sh_mobile_lcdc driver, the
smem_start is a dma_addr_t that is not a physical address when IOMMU is
enabled.  dma_mmap_coherent() maps the address correctly.  It is
available on ARM platforms.

Signed-off-by: Hideki EIRAKU <redacted>

Acked-by: Hideki EIRAKU <redacted>

As this patch doesn't depend on any other patch in your series 
(ARCH_HAS_DMA_MMAP_COHERENT will not be defined without 1/4, so this patch 
will be a no-op until then), I've applied it to my tree and will push it to 
avoid merge conflicts, unless you would prefer to push it yourself.

-- 
Regards,

Laurent Pinchart

On Tuesday 07 August 2012 14:01:43 Laurent Pinchart wrote:

Hi Eiraku-san,

On Monday 06 August 2012 18:55:24 Hideki EIRAKU wrote:

quoted

fb_mmap() implemented in fbmem.c uses smem_start as the physical
address of the frame buffer.  In the sh_mobile_lcdc driver, the
smem_start is a dma_addr_t that is not a physical address when IOMMU is
enabled.  dma_mmap_coherent() maps the address correctly.  It is
available on ARM platforms.

Signed-off-by: Hideki EIRAKU <redacted>

Acked-by: Hideki EIRAKU <redacted>

I obviously meant

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

As this patch doesn't depend on any other patch in your series
(ARCH_HAS_DMA_MMAP_COHERENT will not be defined without 1/4, so this patch
will be a no-op until then), I've applied it to my tree and will push it to
avoid merge conflicts, unless you would prefer to push it yourself.

quoted

---

 drivers/video/sh_mobile_lcdcfb.c |   28 ++++++++++++++++++++++++++++
 1 files changed, 28 insertions(+), 0 deletions(-)

diff --git a/drivers/video/sh_mobile_lcdcfb.c

b/drivers/video/sh_mobile_lcdcfb.c index 8cb653b..c8cba7a 100644

--- a/drivers/video/sh_mobile_lcdcfb.c
+++ b/drivers/video/sh_mobile_lcdcfb.c

@@ -1614,6 +1614,17 @@ static int sh_mobile_lcdc_overlay_blank(int blank,

struct fb_info *info) return 1;

 }

+#ifdef ARCH_HAS_DMA_MMAP_COHERENT
+static int
+sh_mobile_lcdc_overlay_mmap(struct fb_info *info, struct vm_area_struct
*vma) +{
+	struct sh_mobile_lcdc_overlay *ovl = info->par;
+
+	return dma_mmap_coherent(ovl->channel->lcdc->dev, vma, ovl->fb_mem,
+				 ovl->dma_handle, ovl->fb_size);
+}
+#endif
+

 static struct fb_ops sh_mobile_lcdc_overlay_ops = {
 
 	.owner          = THIS_MODULE,
 	.fb_read        = fb_sys_read,

@@ -1626,6 +1637,9 @@ static struct fb_ops sh_mobile_lcdc_overlay_ops = {

 	.fb_ioctl       = sh_mobile_lcdc_overlay_ioctl,
 	.fb_check_var	= sh_mobile_lcdc_overlay_check_var,
 	.fb_set_par	= sh_mobile_lcdc_overlay_set_par,

+#ifdef ARCH_HAS_DMA_MMAP_COHERENT
+	.fb_mmap	= sh_mobile_lcdc_overlay_mmap,
+#endif

 };
 
 static void

@@ -2093,6 +2107,17 @@ static int sh_mobile_lcdc_blank(int blank, struct

fb_info *info) return 0;

 }

+#ifdef ARCH_HAS_DMA_MMAP_COHERENT
+static int
+sh_mobile_lcdc_mmap(struct fb_info *info, struct vm_area_struct *vma)
+{
+	struct sh_mobile_lcdc_chan *ch = info->par;
+
+	return dma_mmap_coherent(ch->lcdc->dev, vma, ch->fb_mem,
+				 ch->dma_handle, ch->fb_size);
+}
+#endif
+

 static struct fb_ops sh_mobile_lcdc_ops = {
 
 	.owner          = THIS_MODULE,
 	.fb_setcolreg	= sh_mobile_lcdc_setcolreg,

@@ -2108,6 +2133,9 @@ static struct fb_ops sh_mobile_lcdc_ops = {

 	.fb_release	= sh_mobile_lcdc_release,
 	.fb_check_var	= sh_mobile_lcdc_check_var,
 	.fb_set_par	= sh_mobile_lcdc_set_par,

+#ifdef ARCH_HAS_DMA_MMAP_COHERENT
+	.fb_mmap	= sh_mobile_lcdc_mmap,
+#endif

 };
 
 static void

-- 
Regards,

Laurent Pinchart

Hello,

On Monday, August 06, 2012 11:55 AM Hideki EIRAKU wrote:

Previously the vb2_dma_contig_mmap() function was using a dma_addr_t as a
physical address.  The two addressses are not necessarily the same.
For example, when using the IOMMU funtion on certain platforms, dma_addr_t
addresses are not directly mappable physical address.
dma_mmap_coherent() maps the address correctly.
It is available on ARM platforms.

Signed-off-by: Hideki EIRAKU <redacted>

I'm sorry for bringing this issue now, once you have already created v3 of your
patches, but similar patch has been already proposed some time ago. It is already
processed together with general videobuf2-dma-contig redesign and dma-buf extensions
by Tomasz Stanislawski.

See post http://thread.gmane.org/gmane.comp.video.dri.devel/70402/focusI461 and
http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/49438 

It doesn't use conditional code inside videobuf2 allocator and rely entirely on 
dma-mapping subsystem to provide a working dma_mmap_coherent/writecombine/attrs() 
function. When it was posted, it relied on the dma-mapping extensions, which now
have been finally merged to v3.6-rc1. Now I wonder if there are any architectures, 
which don't use dma_map_ops based dma-mapping framework, which might use 
videobuf2-dma-conting module. 

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

Hi Hideki,

On Monday, August 06, 2012 11:55 AM Hideki EIRAKU wrote:

I will take this patch to my dma-mapping kernel tree, to the fixes branch.

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

Hello,

From: Marek Szyprowski <m.szyprowski@samsung.com>
Subject: RE: [PATCH v3 3/4] media: videobuf2-dma-contig: use dma_mmap_coherent if available
Date: Tue, 07 Aug 2012 16:53:25 +0200

I'm sorry for bringing this issue now, once you have already created v3 of your
patches, but similar patch has been already proposed some time ago. It is already
processed together with general videobuf2-dma-contig redesign and dma-buf extensions
by Tomasz Stanislawski.

See post http://thread.gmane.org/gmane.comp.video.dri.devel/70402/focusI461 and
http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/49438 

It doesn't use conditional code inside videobuf2 allocator and rely entirely on 
dma-mapping subsystem to provide a working dma_mmap_coherent/writecombine/attrs() 
function. When it was posted, it relied on the dma-mapping extensions, which now
have been finally merged to v3.6-rc1. Now I wonder if there are any architectures, 
which don't use dma_map_ops based dma-mapping framework, which might use 
videobuf2-dma-conting module. 

Thank you for telling me about videobuf2-dma-contig and v3.6-rc1.  The
videobuf2-dma-contig patch I sent is now unnecessary.  So I will
remove the patch.  I will remove the patch defining
ARCH_HAS_DMA_MMAP_COHERENT too because the v3.6-rc1 kernel has generic
dma_mmap_coherent() API for every architecture.

I will also remove the Laurent's patch I sent because it was related
to ARCH_HAS_DMA_MMAP_COHERENT.

The remaining patch is sh_mobile_lcdc.  I will remove ifdefs from the
patch and re-post it as a patch v4.

-- 
Hideki EIRAKU [off-list ref]

Hello,

On Thursday, August 16, 2012 12:14 PM Hideki EIRAKU wrote:

From: Marek Szyprowski <m.szyprowski@samsung.com>
Subject: RE: [PATCH v3 3/4] media: videobuf2-dma-contig: use dma_mmap_coherent if available
Date: Tue, 07 Aug 2012 16:53:25 +0200

quoted

I'm sorry for bringing this issue now, once you have already created v3 of your
patches, but similar patch has been already proposed some time ago. It is already
processed together with general videobuf2-dma-contig redesign and dma-buf extensions
by Tomasz Stanislawski.

See post http://thread.gmane.org/gmane.comp.video.dri.devel/70402/focusI461 and
http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/49438

It doesn't use conditional code inside videobuf2 allocator and rely entirely on
dma-mapping subsystem to provide a working dma_mmap_coherent/writecombine/attrs()
function. When it was posted, it relied on the dma-mapping extensions, which now
have been finally merged to v3.6-rc1. Now I wonder if there are any architectures,
which don't use dma_map_ops based dma-mapping framework, which might use
videobuf2-dma-conting module.

Thank you for telling me about videobuf2-dma-contig and v3.6-rc1.  The
videobuf2-dma-contig patch I sent is now unnecessary.  So I will
remove the patch.  I will remove the patch defining
ARCH_HAS_DMA_MMAP_COHERENT too because the v3.6-rc1 kernel has generic
dma_mmap_coherent() API for every architecture.

Just to let you know - Tomasz has posted an updated version of the dma-buf/vb2-dma-contig
patches:

http://www.spinics.net/lists/linux-media/msg51768.html

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