[RFC][PATCH 0/3] Clean up Ion mapping/caching

[RFC][PATCH 0/3] Clean up Ion mapping/caching

[Laura Abbott]

Hi,

This series cleans up Ion a bit to be more in line with existing standards for
caching and dma mapping.

The most controversial part of this is probably going to be the first patch.
Ion takes quite a few liberties with how the DMA APIs are used for cache
syncing. dma_sync_sg is used without calling dma_map first. There isn't a
good way to get the cache synchronization with the DMA APIs. The behavior of
Ion is closer to the DRM framework which uses its own private cache APIs for
synchronization so I took that approach.

Assuming the approach of the first patch is appropriate, the next two patches
are fairly simple. dma_buf added support for a sync ioctl. Ion has a similar
ioctl already so this fixes the Ion APIs to be compatible with the dma_buf
ioctl. My plan would be to put a timeline on deprecation for the old Ion
sync ioctl. The map_dma_buf calls were also missing calls to the underlying
DMA APIs so the final patch in the series adds the appropriate calls.

Feedback is appreciated.

Thanks,
Laura

Laura Abbott (3):
  staging: ion: Move away from the DMA APIs for cache flushing
  staging: ion: Add support for syncing with DMA_BUF_IOCTL_SYNC
  staging: ion: Add dma_map/dma_unmap calls to dma_buf calls

 drivers/staging/android/ion/Kconfig             | 14 ++++-
 drivers/staging/android/ion/Makefile            |  3 +
 drivers/staging/android/ion/ion-arm.c           | 83 ++++++++++++++++++++++++
 drivers/staging/android/ion/ion-arm64.c         | 46 ++++++++++++++
 drivers/staging/android/ion/ion-x86.c           | 34 ++++++++++
 drivers/staging/android/ion/ion.c               | 84 +++++++++++++------------
 drivers/staging/android/ion/ion_carveout_heap.c |  5 +-
 drivers/staging/android/ion/ion_chunk_heap.c    |  7 +--
 drivers/staging/android/ion/ion_page_pool.c     |  3 +-
 drivers/staging/android/ion/ion_priv.h          | 14 ++---
 drivers/staging/android/ion/ion_system_heap.c   |  5 +-
 11 files changed, 235 insertions(+), 63 deletions(-)
 create mode 100644 drivers/staging/android/ion/ion-arm.c
 create mode 100644 drivers/staging/android/ion/ion-arm64.c
 create mode 100644 drivers/staging/android/ion/ion-x86.c

-- 
2.5.5

[RFC][PATCH 1/3] staging: ion: Move away from the DMA APIs for cache flushing

[Laura Abbott]

Ion is currently using the DMA APIs in non-compliant ways for cache
maintaince. The issue is Ion needs to do cache operations outside of
the regular DMA model. The Ion model matches more closely with the
DRM model which calls cache APIs directly. Add an appropriate
abstraction layer for Ion to call cache operations outside of the
DMA API.

Signed-off-by: Laura Abbott <labbott@redhat.com>
---
 drivers/staging/android/ion/Kconfig             | 14 ++++-
 drivers/staging/android/ion/Makefile            |  3 +
 drivers/staging/android/ion/ion-arm.c           | 83 +++++++++++++++++++++++++
 drivers/staging/android/ion/ion-arm64.c         | 46 ++++++++++++++
 drivers/staging/android/ion/ion-x86.c           | 34 ++++++++++
 drivers/staging/android/ion/ion.c               | 59 ++++++------------
 drivers/staging/android/ion/ion_carveout_heap.c |  5 +-
 drivers/staging/android/ion/ion_chunk_heap.c    |  7 +--
 drivers/staging/android/ion/ion_page_pool.c     |  3 +-
 drivers/staging/android/ion/ion_priv.h          | 14 ++---
 drivers/staging/android/ion/ion_system_heap.c   |  5 +-
 11 files changed, 210 insertions(+), 63 deletions(-)
 create mode 100644 drivers/staging/android/ion/ion-arm.c
 create mode 100644 drivers/staging/android/ion/ion-arm64.c
 create mode 100644 drivers/staging/android/ion/ion-x86.c

diff --git a/drivers/staging/android/ion/Kconfig b/drivers/staging/android/ion/Kconfig
index 19c1572..c1b1813 100644
--- a/drivers/staging/android/ion/Kconfig
+++ b/drivers/staging/android/ion/Kconfig
@@ -1,6 +1,6 @@
 menuconfig ION
 	bool "Ion Memory Manager"
-	depends on HAVE_MEMBLOCK && HAS_DMA && MMU
+	depends on HAVE_MEMBLOCK && HAS_DMA && MMU && (X86 || ARM || ARM64)
 	select GENERIC_ALLOCATOR
 	select DMA_SHARED_BUFFER
 	---help---
@@ -10,6 +10,18 @@ menuconfig ION
 	  If you're not using Android its probably safe to
 	  say N here.
 
+config ION_ARM
+	depends on ION && ARM
+	def_bool y
+
+config ION_ARM64
+	depends on ION && ARM64
+	def_bool y
+
+config ION_X86
+	depends on ION && X86
+	def_bool y
+
 config ION_TEST
 	tristate "Ion Test Device"
 	depends on ION
diff --git a/drivers/staging/android/ion/Makefile b/drivers/staging/android/ion/Makefile
index 18cc2aa..1b82bb5 100644
--- a/drivers/staging/android/ion/Makefile
+++ b/drivers/staging/android/ion/Makefile
@@ -1,5 +1,8 @@
 obj-$(CONFIG_ION) +=	ion.o ion_heap.o ion_page_pool.o ion_system_heap.o \
 			ion_carveout_heap.o ion_chunk_heap.o ion_cma_heap.o
+obj-$(CONFIG_ION_X86) += ion-x86.o
+obj-$(CONFIG_ION_ARM) += ion-arm.o
+obj-$(CONFIG_ION_ARM64) += ion-arm64.o
 obj-$(CONFIG_ION_TEST) += ion_test.o
 ifdef CONFIG_COMPAT
 obj-$(CONFIG_ION) += compat_ion.o
diff --git a/drivers/staging/android/ion/ion-arm.c b/drivers/staging/android/ion/ion-arm.c
new file mode 100644
index 0000000..b91287f
--- /dev/null
+++ b/drivers/staging/android/ion/ion-arm.c
@@ -0,0 +1,83 @@
+/*
+ * Copyright (C) 2016 Laura Abbott <laura@labbott.name>
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <linux/types.h>
+#include <linux/scatterlist.h>
+#include <linux/dma-mapping.h>
+#include <linux/highmem.h>
+
+#include <asm/cacheflush.h>
+
+#include "ion_priv.h"
+
+
+void ion_clean_page(struct page *page, size_t size)
+{
+	unsigned long pfn;
+	size_t left = size;
+
+	pfn = page_to_pfn(page);
+
+	/*
+	 * A single sg entry may refer to multiple physically contiguous
+	 * pages.  But we still need to process highmem pages individually.
+	 * If highmem is not configured then the bulk of this loop gets
+	 * optimized out.
+	 */
+	do {
+		size_t len = left;
+		void *vaddr;
+
+		page = pfn_to_page(pfn);
+
+		if (PageHighMem(page)) {
+			if (len > PAGE_SIZE)
+				len = PAGE_SIZE;
+
+			if (cache_is_vipt_nonaliasing()) {
+				vaddr = kmap_atomic(page);
+				__cpuc_flush_dcache_area(vaddr, len);
+				kunmap_atomic(vaddr);
+			} else {
+				vaddr = kmap_high_get(page);
+				if (vaddr) {
+					__cpuc_flush_dcache_area(vaddr, len);
+					kunmap_high(page);
+				}
+			}
+		} else {
+			vaddr = page_address(page);
+			__cpuc_flush_dcache_area(vaddr, len);
+		}
+		pfn++;
+		left -= len;
+	} while (left);
+}
+
+/*
+ * ARM has highmem and a bunch of other 'fun' features. It's so much easier just
+ * to do the ISA DMA and call things that way
+ */
+
+void ion_invalidate_buffer(struct ion_buffer *buffer)
+{
+	dma_unmap_sg(NULL, buffer->sg_table->sgl, buffer->sg_table->orig_nents,
+			DMA_BIDIRECTIONAL);
+}
+
+void ion_clean_buffer(struct ion_buffer *buffer)
+{
+	dma_map_sg(NULL, buffer->sg_table->sgl, buffer->sg_table->orig_nents,
+			DMA_BIDIRECTIONAL);
+}
diff --git a/drivers/staging/android/ion/ion-arm64.c b/drivers/staging/android/ion/ion-arm64.c
new file mode 100644
index 0000000..afd387a
--- /dev/null
+++ b/drivers/staging/android/ion/ion-arm64.c
@@ -0,0 +1,46 @@
+/*
+ * Copyright (C) 2016 Laura Abbott <laura@labbott.name>
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <linux/types.h>
+#include <linux/scatterlist.h>
+#include <linux/dma-mapping.h>
+
+#include <asm/cacheflush.h>
+
+#include "ion_priv.h"
+
+void ion_clean_page(struct page *page, size_t size)
+{
+	__flush_dcache_area(page_address(page), size);
+}
+
+void ion_invalidate_buffer(struct ion_buffer *buffer)
+{
+	struct scatterlist *sg;
+	int i;
+
+	for_each_sg(buffer->sg_table->sgl, sg, buffer->sg_table->orig_nents, i)
+		__dma_unmap_area(page_address(sg_page(sg)), sg->length,
+					DMA_BIDIRECTIONAL);
+}
+
+void ion_clean_buffer(struct ion_buffer *buffer)
+{
+	struct scatterlist *sg;
+	int i;
+
+	for_each_sg(buffer->sg_table->sgl, sg, buffer->sg_table->orig_nents, i)
+		__dma_map_area(page_address(sg_page(sg)), sg->length,
+					DMA_BIDIRECTIONAL);
+}
diff --git a/drivers/staging/android/ion/ion-x86.c b/drivers/staging/android/ion/ion-x86.c
new file mode 100644
index 0000000..05fd684
--- /dev/null
+++ b/drivers/staging/android/ion/ion-x86.c
@@ -0,0 +1,34 @@
+/*
+ * Copyright (C) 2016 Laura Abbott <laura@labbott.name>
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <linux/types.h>
+#include <linux/scatterlist.h>
+#include <linux/dma-mapping.h>
+
+#include "ion_priv.h"
+
+void ion_clean_page(struct page *page, size_t size)
+{
+	/* Do nothing right now */
+}
+
+void ion_invalidate_buffer(struct ion_buffer *buffer)
+{
+	/* Do nothing right now */
+}
+
+void ion_clean_buffer(struct ion_buffer *buffer)
+{
+	/* Do nothing right now */
+}
diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
index a2cf93b..9282d96a 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -937,42 +937,6 @@ struct sg_table *ion_sg_table(struct ion_client *client,
 }
 EXPORT_SYMBOL(ion_sg_table);
 
-static void ion_buffer_sync_for_device(struct ion_buffer *buffer,
-				       struct device *dev,
-				       enum dma_data_direction direction);
-
-static struct sg_table *ion_map_dma_buf(struct dma_buf_attachment *attachment,
-					enum dma_data_direction direction)
-{
-	struct dma_buf *dmabuf = attachment->dmabuf;
-	struct ion_buffer *buffer = dmabuf->priv;
-
-	ion_buffer_sync_for_device(buffer, attachment->dev, direction);
-	return buffer->sg_table;
-}
-
-static void ion_unmap_dma_buf(struct dma_buf_attachment *attachment,
-			      struct sg_table *table,
-			      enum dma_data_direction direction)
-{
-}
-
-void ion_pages_sync_for_device(struct device *dev, struct page *page,
-		size_t size, enum dma_data_direction dir)
-{
-	struct scatterlist sg;
-
-	sg_init_table(&sg, 1);
-	sg_set_page(&sg, page, size, 0);
-	/*
-	 * This is not correct - sg_dma_address needs a dma_addr_t that is valid
-	 * for the targeted device, but this works on the currently targeted
-	 * hardware.
-	 */
-	sg_dma_address(&sg) = page_to_phys(page);
-	dma_sync_sg_for_device(dev, &sg, 1, dir);
-}
-
 struct ion_vma_list {
 	struct list_head list;
 	struct vm_area_struct *vma;
@@ -997,8 +961,7 @@ static void ion_buffer_sync_for_device(struct ion_buffer *buffer,
 		struct page *page = buffer->pages[i];
 
 		if (ion_buffer_page_is_dirty(page))
-			ion_pages_sync_for_device(dev, ion_buffer_page(page),
-							PAGE_SIZE, dir);
+			ion_clean_page(ion_buffer_page(page), PAGE_SIZE);
 
 		ion_buffer_page_clean(buffer->pages + i);
 	}
@@ -1011,6 +974,22 @@ static void ion_buffer_sync_for_device(struct ion_buffer *buffer,
 	mutex_unlock(&buffer->lock);
 }
 
+static struct sg_table *ion_map_dma_buf(struct dma_buf_attachment *attachment,
+					enum dma_data_direction direction)
+{
+	struct dma_buf *dmabuf = attachment->dmabuf;
+	struct ion_buffer *buffer = dmabuf->priv;
+
+	ion_buffer_sync_for_device(buffer, attachment->dev, direction);
+	return buffer->sg_table;
+}
+
+static void ion_unmap_dma_buf(struct dma_buf_attachment *attachment,
+			      struct sg_table *table,
+			      enum dma_data_direction direction)
+{
+}
+
 static int ion_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
 {
 	struct ion_buffer *buffer = vma->vm_private_data;
@@ -1293,8 +1272,8 @@ static int ion_sync_for_device(struct ion_client *client, int fd)
 	}
 	buffer = dmabuf->priv;
 
-	dma_sync_sg_for_device(NULL, buffer->sg_table->sgl,
-			       buffer->sg_table->nents, DMA_BIDIRECTIONAL);
+	ion_clean_buffer(buffer);
+
 	dma_buf_put(dmabuf);
 	return 0;
 }
diff --git a/drivers/staging/android/ion/ion_carveout_heap.c b/drivers/staging/android/ion/ion_carveout_heap.c
index 1fb0d81..d213cbf 100644
--- a/drivers/staging/android/ion/ion_carveout_heap.c
+++ b/drivers/staging/android/ion/ion_carveout_heap.c
@@ -116,8 +116,7 @@ static void ion_carveout_heap_free(struct ion_buffer *buffer)
 	ion_heap_buffer_zero(buffer);
 
 	if (ion_buffer_cached(buffer))
-		dma_sync_sg_for_device(NULL, table->sgl, table->nents,
-				       DMA_BIDIRECTIONAL);
+		ion_clean_page(page, buffer->size);
 
 	ion_carveout_free(heap, paddr, buffer->size);
 	sg_free_table(table);
@@ -157,7 +156,7 @@ struct ion_heap *ion_carveout_heap_create(struct ion_platform_heap *heap_data)
 	page = pfn_to_page(PFN_DOWN(heap_data->base));
 	size = heap_data->size;
 
-	ion_pages_sync_for_device(NULL, page, size, DMA_BIDIRECTIONAL);
+	ion_clean_page(page, size);
 
 	ret = ion_heap_pages_zero(page, size, pgprot_writecombine(PAGE_KERNEL));
 	if (ret)
diff --git a/drivers/staging/android/ion/ion_chunk_heap.c b/drivers/staging/android/ion/ion_chunk_heap.c
index e0553fe..0666529 100644
--- a/drivers/staging/android/ion/ion_chunk_heap.c
+++ b/drivers/staging/android/ion/ion_chunk_heap.c
@@ -104,11 +104,10 @@ static void ion_chunk_heap_free(struct ion_buffer *buffer)
 
 	ion_heap_buffer_zero(buffer);
 
-	if (ion_buffer_cached(buffer))
-		dma_sync_sg_for_device(NULL, table->sgl, table->nents,
-							DMA_BIDIRECTIONAL);
 
 	for_each_sg(table->sgl, sg, table->nents, i) {
+		if (ion_buffer_cached(buffer))
+			ion_clean_page(sg_page(table->sgl), sg->length);
 		gen_pool_free(chunk_heap->pool, page_to_phys(sg_page(sg)),
 			      sg->length);
 	}
@@ -148,7 +147,7 @@ struct ion_heap *ion_chunk_heap_create(struct ion_platform_heap *heap_data)
 	page = pfn_to_page(PFN_DOWN(heap_data->base));
 	size = heap_data->size;
 
-	ion_pages_sync_for_device(NULL, page, size, DMA_BIDIRECTIONAL);
+	ion_clean_page(page, size);
 
 	ret = ion_heap_pages_zero(page, size, pgprot_writecombine(PAGE_KERNEL));
 	if (ret)
diff --git a/drivers/staging/android/ion/ion_page_pool.c b/drivers/staging/android/ion/ion_page_pool.c
index 1fe8016..b854f91 100644
--- a/drivers/staging/android/ion/ion_page_pool.c
+++ b/drivers/staging/android/ion/ion_page_pool.c
@@ -30,8 +30,7 @@ static void *ion_page_pool_alloc_pages(struct ion_page_pool *pool)
 
 	if (!page)
 		return NULL;
-	ion_pages_sync_for_device(NULL, page, PAGE_SIZE << pool->order,
-						DMA_BIDIRECTIONAL);
+	ion_clean_page(page, PAGE_SIZE << pool->order);
 	return page;
 }
 
diff --git a/drivers/staging/android/ion/ion_priv.h b/drivers/staging/android/ion/ion_priv.h
index 0239883..b368338 100644
--- a/drivers/staging/android/ion/ion_priv.h
+++ b/drivers/staging/android/ion/ion_priv.h
@@ -392,15 +392,9 @@ void ion_page_pool_free(struct ion_page_pool *, struct page *);
 int ion_page_pool_shrink(struct ion_page_pool *pool, gfp_t gfp_mask,
 			  int nr_to_scan);
 
-/**
- * ion_pages_sync_for_device - cache flush pages for use with the specified
- *                             device
- * @dev:		the device the pages will be used with
- * @page:		the first page to be flushed
- * @size:		size in bytes of region to be flushed
- * @dir:		direction of dma transfer
- */
-void ion_pages_sync_for_device(struct device *dev, struct page *page,
-		size_t size, enum dma_data_direction dir);
+void ion_clean_page(struct page *page, size_t size);
+
+void ion_clean_buffer(struct ion_buffer *buffer);
 
+void ion_invalidate_buffer(struct ion_buffer *buffer);
 #endif /* _ION_PRIV_H */
diff --git a/drivers/staging/android/ion/ion_system_heap.c b/drivers/staging/android/ion/ion_system_heap.c
index b69dfc7..aa39660 100644
--- a/drivers/staging/android/ion/ion_system_heap.c
+++ b/drivers/staging/android/ion/ion_system_heap.c
@@ -70,8 +70,7 @@ static struct page *alloc_buffer_page(struct ion_system_heap *heap,
 		page = alloc_pages(gfp_flags | __GFP_COMP, order);
 		if (!page)
 			return NULL;
-		ion_pages_sync_for_device(NULL, page, PAGE_SIZE << order,
-						DMA_BIDIRECTIONAL);
+		ion_clean_page(page, PAGE_SIZE << order);
 	}
 
 	return page;
@@ -360,7 +359,7 @@ static int ion_system_contig_heap_allocate(struct ion_heap *heap,
 
 	buffer->priv_virt = table;
 
-	ion_pages_sync_for_device(NULL, page, len, DMA_BIDIRECTIONAL);
+	ion_clean_page(page, len);
 
 	return 0;
 
-- 
2.5.5

[RFC][PATCH 1/3] staging: ion: Move away from the DMA APIs for cache flushing

[Liviu Dudau]

On Wed, May 25, 2016 at 12:48:02PM -0700, Laura Abbott wrote:
> 
> Ion is currently using the DMA APIs in non-compliant ways for cache
> maintaince. The issue is Ion needs to do cache operations outside of
> the regular DMA model. The Ion model matches more closely with the
> DRM model which calls cache APIs directly. Add an appropriate
> abstraction layer for Ion to call cache operations outside of the
> DMA API.
> 
> Signed-off-by: Laura Abbott <labbott@redhat.com>

Hi Laura,

By no means a full review, just some comments:

> ---
>  drivers/staging/android/ion/Kconfig             | 14 ++++-
>  drivers/staging/android/ion/Makefile            |  3 +
>  drivers/staging/android/ion/ion-arm.c           | 83 +++++++++++++++++++++++++
>  drivers/staging/android/ion/ion-arm64.c         | 46 ++++++++++++++
>  drivers/staging/android/ion/ion-x86.c           | 34 ++++++++++
>  drivers/staging/android/ion/ion.c               | 59 ++++++------------
>  drivers/staging/android/ion/ion_carveout_heap.c |  5 +-
>  drivers/staging/android/ion/ion_chunk_heap.c    |  7 +--
>  drivers/staging/android/ion/ion_page_pool.c     |  3 +-
>  drivers/staging/android/ion/ion_priv.h          | 14 ++---
>  drivers/staging/android/ion/ion_system_heap.c   |  5 +-
>  11 files changed, 210 insertions(+), 63 deletions(-)
>  create mode 100644 drivers/staging/android/ion/ion-arm.c
>  create mode 100644 drivers/staging/android/ion/ion-arm64.c
>  create mode 100644 drivers/staging/android/ion/ion-x86.c
> 
> diff --git a/drivers/staging/android/ion/Kconfig b/drivers/staging/android/ion/Kconfig
> index 19c1572..c1b1813 100644
> --- a/drivers/staging/android/ion/Kconfig
> +++ b/drivers/staging/android/ion/Kconfig
> @@ -1,6 +1,6 @@
>  menuconfig ION
>  	bool "Ion Memory Manager"
> -	depends on HAVE_MEMBLOCK && HAS_DMA && MMU
> +	depends on HAVE_MEMBLOCK && HAS_DMA && MMU && (X86 || ARM || ARM64)
>  	select GENERIC_ALLOCATOR
>  	select DMA_SHARED_BUFFER
>  	---help---
> @@ -10,6 +10,18 @@ menuconfig ION
>  	  If you're not using Android its probably safe to
>  	  say N here.
>  
> +config ION_ARM
> +	depends on ION && ARM
> +	def_bool y
> +
> +config ION_ARM64
> +	depends on ION && ARM64
> +	def_bool y
> +
> +config ION_X86
> +	depends on ION && X86
> +	def_bool y

If you are going to make this arch specific, can I suggest that you make CONFIG_ION
depend on CONFIG_ARCH_ION (or any name you like) and then in each arch select it?
I don't particularly like the enumeration of all ION enabled arches above 
(is the list exhaustive?)

> +
>  config ION_TEST
>  	tristate "Ion Test Device"
>  	depends on ION
> diff --git a/drivers/staging/android/ion/Makefile b/drivers/staging/android/ion/Makefile
> index 18cc2aa..1b82bb5 100644
> --- a/drivers/staging/android/ion/Makefile
> +++ b/drivers/staging/android/ion/Makefile
> @@ -1,5 +1,8 @@
>  obj-$(CONFIG_ION) +=	ion.o ion_heap.o ion_page_pool.o ion_system_heap.o \
>  			ion_carveout_heap.o ion_chunk_heap.o ion_cma_heap.o
> +obj-$(CONFIG_ION_X86) += ion-x86.o
> +obj-$(CONFIG_ION_ARM) += ion-arm.o
> +obj-$(CONFIG_ION_ARM64) += ion-arm64.o
>  obj-$(CONFIG_ION_TEST) += ion_test.o
>  ifdef CONFIG_COMPAT
>  obj-$(CONFIG_ION) += compat_ion.o
> diff --git a/drivers/staging/android/ion/ion-arm.c b/drivers/staging/android/ion/ion-arm.c
> new file mode 100644
> index 0000000..b91287f
> --- /dev/null
> +++ b/drivers/staging/android/ion/ion-arm.c
> @@ -0,0 +1,83 @@
> +/*
> + * Copyright (C) 2016 Laura Abbott <laura@labbott.name>
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include <linux/types.h>
> +#include <linux/scatterlist.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/highmem.h>
> +
> +#include <asm/cacheflush.h>
> +
> +#include "ion_priv.h"
> +
> +
> +void ion_clean_page(struct page *page, size_t size)
> +{
> +	unsigned long pfn;
> +	size_t left = size;
> +
> +	pfn = page_to_pfn(page);
> +
> +	/*
> +	 * A single sg entry may refer to multiple physically contiguous
> +	 * pages.  But we still need to process highmem pages individually.
> +	 * If highmem is not configured then the bulk of this loop gets
> +	 * optimized out.
> +	 */
> +	do {
> +		size_t len = left;
> +		void *vaddr;
> +
> +		page = pfn_to_page(pfn);
> +
> +		if (PageHighMem(page)) {
> +			if (len > PAGE_SIZE)
> +				len = PAGE_SIZE;
> +
> +			if (cache_is_vipt_nonaliasing()) {
> +				vaddr = kmap_atomic(page);
> +				__cpuc_flush_dcache_area(vaddr, len);
> +				kunmap_atomic(vaddr);
> +			} else {
> +				vaddr = kmap_high_get(page);
> +				if (vaddr) {
> +					__cpuc_flush_dcache_area(vaddr, len);
> +					kunmap_high(page);
> +				}
> +			}
> +		} else {
> +			vaddr = page_address(page);
> +			__cpuc_flush_dcache_area(vaddr, len);
> +		}
> +		pfn++;
> +		left -= len;
> +	} while (left);
> +}
> +
> +/*
> + * ARM has highmem and a bunch of other 'fun' features. It's so much easier just
> + * to do the ISA DMA and call things that way
> + */
> +
> +void ion_invalidate_buffer(struct ion_buffer *buffer)
> +{
> +	dma_unmap_sg(NULL, buffer->sg_table->sgl, buffer->sg_table->orig_nents,

arm_dma_unmap_sg?

> +			DMA_BIDIRECTIONAL);
> +}
> +
> +void ion_clean_buffer(struct ion_buffer *buffer)
> +{
> +	dma_map_sg(NULL, buffer->sg_table->sgl, buffer->sg_table->orig_nents,

arm_dma_map_sg?

Otherwise, these two functions looks quite generic. Can they be the default implementation?

Best regards,
Liviu

> +			DMA_BIDIRECTIONAL);
> +}
> diff --git a/drivers/staging/android/ion/ion-arm64.c b/drivers/staging/android/ion/ion-arm64.c
> new file mode 100644
> index 0000000..afd387a
> --- /dev/null
> +++ b/drivers/staging/android/ion/ion-arm64.c
> @@ -0,0 +1,46 @@
> +/*
> + * Copyright (C) 2016 Laura Abbott <laura@labbott.name>
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include <linux/types.h>
> +#include <linux/scatterlist.h>
> +#include <linux/dma-mapping.h>
> +
> +#include <asm/cacheflush.h>
> +
> +#include "ion_priv.h"
> +
> +void ion_clean_page(struct page *page, size_t size)
> +{
> +	__flush_dcache_area(page_address(page), size);
> +}
> +
> +void ion_invalidate_buffer(struct ion_buffer *buffer)
> +{
> +	struct scatterlist *sg;
> +	int i;
> +
> +	for_each_sg(buffer->sg_table->sgl, sg, buffer->sg_table->orig_nents, i)
> +		__dma_unmap_area(page_address(sg_page(sg)), sg->length,
> +					DMA_BIDIRECTIONAL);
> +}
> +
> +void ion_clean_buffer(struct ion_buffer *buffer)
> +{
> +	struct scatterlist *sg;
> +	int i;
> +
> +	for_each_sg(buffer->sg_table->sgl, sg, buffer->sg_table->orig_nents, i)
> +		__dma_map_area(page_address(sg_page(sg)), sg->length,
> +					DMA_BIDIRECTIONAL);
> +}
> diff --git a/drivers/staging/android/ion/ion-x86.c b/drivers/staging/android/ion/ion-x86.c
> new file mode 100644
> index 0000000..05fd684
> --- /dev/null
> +++ b/drivers/staging/android/ion/ion-x86.c
> @@ -0,0 +1,34 @@
> +/*
> + * Copyright (C) 2016 Laura Abbott <laura@labbott.name>
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include <linux/types.h>
> +#include <linux/scatterlist.h>
> +#include <linux/dma-mapping.h>
> +
> +#include "ion_priv.h"
> +
> +void ion_clean_page(struct page *page, size_t size)
> +{
> +	/* Do nothing right now */
> +}
> +
> +void ion_invalidate_buffer(struct ion_buffer *buffer)
> +{
> +	/* Do nothing right now */
> +}
> +
> +void ion_clean_buffer(struct ion_buffer *buffer)
> +{
> +	/* Do nothing right now */
> +}
> diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
> index a2cf93b..9282d96a 100644
> --- a/drivers/staging/android/ion/ion.c
> +++ b/drivers/staging/android/ion/ion.c
> @@ -937,42 +937,6 @@ struct sg_table *ion_sg_table(struct ion_client *client,
>  }
>  EXPORT_SYMBOL(ion_sg_table);
>  
> -static void ion_buffer_sync_for_device(struct ion_buffer *buffer,
> -				       struct device *dev,
> -				       enum dma_data_direction direction);
> -
> -static struct sg_table *ion_map_dma_buf(struct dma_buf_attachment *attachment,
> -					enum dma_data_direction direction)
> -{
> -	struct dma_buf *dmabuf = attachment->dmabuf;
> -	struct ion_buffer *buffer = dmabuf->priv;
> -
> -	ion_buffer_sync_for_device(buffer, attachment->dev, direction);
> -	return buffer->sg_table;
> -}
> -
> -static void ion_unmap_dma_buf(struct dma_buf_attachment *attachment,
> -			      struct sg_table *table,
> -			      enum dma_data_direction direction)
> -{
> -}
> -
> -void ion_pages_sync_for_device(struct device *dev, struct page *page,
> -		size_t size, enum dma_data_direction dir)
> -{
> -	struct scatterlist sg;
> -
> -	sg_init_table(&sg, 1);
> -	sg_set_page(&sg, page, size, 0);
> -	/*
> -	 * This is not correct - sg_dma_address needs a dma_addr_t that is valid
> -	 * for the targeted device, but this works on the currently targeted
> -	 * hardware.
> -	 */
> -	sg_dma_address(&sg) = page_to_phys(page);
> -	dma_sync_sg_for_device(dev, &sg, 1, dir);
> -}
> -
>  struct ion_vma_list {
>  	struct list_head list;
>  	struct vm_area_struct *vma;
> @@ -997,8 +961,7 @@ static void ion_buffer_sync_for_device(struct ion_buffer *buffer,
>  		struct page *page = buffer->pages[i];
>  
>  		if (ion_buffer_page_is_dirty(page))
> -			ion_pages_sync_for_device(dev, ion_buffer_page(page),
> -							PAGE_SIZE, dir);
> +			ion_clean_page(ion_buffer_page(page), PAGE_SIZE);
>  
>  		ion_buffer_page_clean(buffer->pages + i);
>  	}
> @@ -1011,6 +974,22 @@ static void ion_buffer_sync_for_device(struct ion_buffer *buffer,
>  	mutex_unlock(&buffer->lock);
>  }
>  
> +static struct sg_table *ion_map_dma_buf(struct dma_buf_attachment *attachment,
> +					enum dma_data_direction direction)
> +{
> +	struct dma_buf *dmabuf = attachment->dmabuf;
> +	struct ion_buffer *buffer = dmabuf->priv;
> +
> +	ion_buffer_sync_for_device(buffer, attachment->dev, direction);
> +	return buffer->sg_table;
> +}
> +
> +static void ion_unmap_dma_buf(struct dma_buf_attachment *attachment,
> +			      struct sg_table *table,
> +			      enum dma_data_direction direction)
> +{
> +}
> +
>  static int ion_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
>  {
>  	struct ion_buffer *buffer = vma->vm_private_data;
> @@ -1293,8 +1272,8 @@ static int ion_sync_for_device(struct ion_client *client, int fd)
>  	}
>  	buffer = dmabuf->priv;
>  
> -	dma_sync_sg_for_device(NULL, buffer->sg_table->sgl,
> -			       buffer->sg_table->nents, DMA_BIDIRECTIONAL);
> +	ion_clean_buffer(buffer);
> +
>  	dma_buf_put(dmabuf);
>  	return 0;
>  }
> diff --git a/drivers/staging/android/ion/ion_carveout_heap.c b/drivers/staging/android/ion/ion_carveout_heap.c
> index 1fb0d81..d213cbf 100644
> --- a/drivers/staging/android/ion/ion_carveout_heap.c
> +++ b/drivers/staging/android/ion/ion_carveout_heap.c
> @@ -116,8 +116,7 @@ static void ion_carveout_heap_free(struct ion_buffer *buffer)
>  	ion_heap_buffer_zero(buffer);
>  
>  	if (ion_buffer_cached(buffer))
> -		dma_sync_sg_for_device(NULL, table->sgl, table->nents,
> -				       DMA_BIDIRECTIONAL);
> +		ion_clean_page(page, buffer->size);
>  
>  	ion_carveout_free(heap, paddr, buffer->size);
>  	sg_free_table(table);
> @@ -157,7 +156,7 @@ struct ion_heap *ion_carveout_heap_create(struct ion_platform_heap *heap_data)
>  	page = pfn_to_page(PFN_DOWN(heap_data->base));
>  	size = heap_data->size;
>  
> -	ion_pages_sync_for_device(NULL, page, size, DMA_BIDIRECTIONAL);
> +	ion_clean_page(page, size);
>  
>  	ret = ion_heap_pages_zero(page, size, pgprot_writecombine(PAGE_KERNEL));
>  	if (ret)
> diff --git a/drivers/staging/android/ion/ion_chunk_heap.c b/drivers/staging/android/ion/ion_chunk_heap.c
> index e0553fe..0666529 100644
> --- a/drivers/staging/android/ion/ion_chunk_heap.c
> +++ b/drivers/staging/android/ion/ion_chunk_heap.c
> @@ -104,11 +104,10 @@ static void ion_chunk_heap_free(struct ion_buffer *buffer)
>  
>  	ion_heap_buffer_zero(buffer);
>  
> -	if (ion_buffer_cached(buffer))
> -		dma_sync_sg_for_device(NULL, table->sgl, table->nents,
> -							DMA_BIDIRECTIONAL);
>  
>  	for_each_sg(table->sgl, sg, table->nents, i) {
> +		if (ion_buffer_cached(buffer))
> +			ion_clean_page(sg_page(table->sgl), sg->length);
>  		gen_pool_free(chunk_heap->pool, page_to_phys(sg_page(sg)),
>  			      sg->length);
>  	}
> @@ -148,7 +147,7 @@ struct ion_heap *ion_chunk_heap_create(struct ion_platform_heap *heap_data)
>  	page = pfn_to_page(PFN_DOWN(heap_data->base));
>  	size = heap_data->size;
>  
> -	ion_pages_sync_for_device(NULL, page, size, DMA_BIDIRECTIONAL);
> +	ion_clean_page(page, size);
>  
>  	ret = ion_heap_pages_zero(page, size, pgprot_writecombine(PAGE_KERNEL));
>  	if (ret)
> diff --git a/drivers/staging/android/ion/ion_page_pool.c b/drivers/staging/android/ion/ion_page_pool.c
> index 1fe8016..b854f91 100644
> --- a/drivers/staging/android/ion/ion_page_pool.c
> +++ b/drivers/staging/android/ion/ion_page_pool.c
> @@ -30,8 +30,7 @@ static void *ion_page_pool_alloc_pages(struct ion_page_pool *pool)
>  
>  	if (!page)
>  		return NULL;
> -	ion_pages_sync_for_device(NULL, page, PAGE_SIZE << pool->order,
> -						DMA_BIDIRECTIONAL);
> +	ion_clean_page(page, PAGE_SIZE << pool->order);
>  	return page;
>  }
>  
> diff --git a/drivers/staging/android/ion/ion_priv.h b/drivers/staging/android/ion/ion_priv.h
> index 0239883..b368338 100644
> --- a/drivers/staging/android/ion/ion_priv.h
> +++ b/drivers/staging/android/ion/ion_priv.h
> @@ -392,15 +392,9 @@ void ion_page_pool_free(struct ion_page_pool *, struct page *);
>  int ion_page_pool_shrink(struct ion_page_pool *pool, gfp_t gfp_mask,
>  			  int nr_to_scan);
>  
> -/**
> - * ion_pages_sync_for_device - cache flush pages for use with the specified
> - *                             device
> - * @dev:		the device the pages will be used with
> - * @page:		the first page to be flushed
> - * @size:		size in bytes of region to be flushed
> - * @dir:		direction of dma transfer
> - */
> -void ion_pages_sync_for_device(struct device *dev, struct page *page,
> -		size_t size, enum dma_data_direction dir);
> +void ion_clean_page(struct page *page, size_t size);
> +
> +void ion_clean_buffer(struct ion_buffer *buffer);
>  
> +void ion_invalidate_buffer(struct ion_buffer *buffer);
>  #endif /* _ION_PRIV_H */
> diff --git a/drivers/staging/android/ion/ion_system_heap.c b/drivers/staging/android/ion/ion_system_heap.c
> index b69dfc7..aa39660 100644
> --- a/drivers/staging/android/ion/ion_system_heap.c
> +++ b/drivers/staging/android/ion/ion_system_heap.c
> @@ -70,8 +70,7 @@ static struct page *alloc_buffer_page(struct ion_system_heap *heap,
>  		page = alloc_pages(gfp_flags | __GFP_COMP, order);
>  		if (!page)
>  			return NULL;
> -		ion_pages_sync_for_device(NULL, page, PAGE_SIZE << order,
> -						DMA_BIDIRECTIONAL);
> +		ion_clean_page(page, PAGE_SIZE << order);
>  	}
>  
>  	return page;
> @@ -360,7 +359,7 @@ static int ion_system_contig_heap_allocate(struct ion_heap *heap,
>  
>  	buffer->priv_virt = table;
>  
> -	ion_pages_sync_for_device(NULL, page, len, DMA_BIDIRECTIONAL);
> +	ion_clean_page(page, len);
>  
>  	return 0;
>  
> -- 
> 2.5.5
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ?\_(?)_/?

[RFC][PATCH 1/3] staging: ion: Move away from the DMA APIs for cache flushing

[Russell King - ARM Linux]

On Thu, May 26, 2016 at 10:58:35AM +0100, Liviu Dudau wrote:
> On Wed, May 25, 2016 at 12:48:02PM -0700, Laura Abbott wrote:
> > 
> > Ion is currently using the DMA APIs in non-compliant ways for cache
> > maintaince. The issue is Ion needs to do cache operations outside of
> > the regular DMA model. The Ion model matches more closely with the
> > DRM model which calls cache APIs directly. Add an appropriate
> > abstraction layer for Ion to call cache operations outside of the
> > DMA API.

I _really_ hate seeing architecture internal functions being abused in
drivers - architecture internal functions are there to implement the
official kernel APIs and are not for drivers to poke about with.

I've had this happen several times, and each time it makes maintanence
of architecture code harder than it should be.

In any case, the functions you are using are probably not appropriate -
the way I've defined the architecture internal functions is for each
to have a specific purpose.  Eg, if caches need flushing when page tables
change, then the function gets implemented, otherwise it's a no-op.  Using
a function which _seems_ to do the right thing today in a way which is
against its purpose is a recipe for your code breaking.

If you need something from the architecture which isn't already provided,
then you need to talk to architecture people about proposing an official
interface to that functionality, rather than trying to bolt per-
architecture shims into drivers.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[RFC][PATCH 1/3] staging: ion: Move away from the DMA APIs for cache flushing

[Christoph Hellwig]

On Thu, May 26, 2016 at 11:59:22AM +0100, Russell King - ARM Linux wrote:
> I _really_ hate seeing architecture internal functions being abused in
> drivers - architecture internal functions are there to implement the
> official kernel APIs and are not for drivers to poke about with.

Exactly - if drivers like drm and ion want to do something out side the
current DMA API we need to add a proper API, not hack around the lack of
one.

[Linaro-mm-sig] [RFC][PATCH 1/3] staging: ion: Move away from the DMA APIs for cache flushing

[Li, Xiaoquan]

Hi Laura,

It seems that outer cache is needed to be handled after __cpuc_flush_dcache_area() for ARM platform.

Please correct me if I am wrong.

Thanks

Xiaoquan



-----Original Message-----
From: Linaro-mm-sig [mailto:linaro-mm-sig-bounces at lists.linaro.org] On Behalf Of Laura Abbott
Sent: 2016?5?26? 3:48
To: Sumit Semwal; John Stultz; Arve Hj?nnev?g; Riley Andrews
Cc: devel at driverdev.osuosl.org; Jon Medhurst; Android Kernel Team; Greg Kroah-Hartman; Will Deacon; Russell King; linux-kernel at vger.kernel.org; linaro-mm-sig at lists.linaro.org; Rohit kumar; Jeremy Gebben; Eun Taik Lee; Catalin Marinas; Liviu Dudau; Laura Abbott; linux-arm-kernel at lists.infradead.org
Subject: [Linaro-mm-sig] [RFC][PATCH 1/3] staging: ion: Move away from the DMA APIs for cache flushing


Ion is currently using the DMA APIs in non-compliant ways for cache
maintaince. The issue is Ion needs to do cache operations outside of
the regular DMA model. The Ion model matches more closely with the
DRM model which calls cache APIs directly. Add an appropriate
abstraction layer for Ion to call cache operations outside of the
DMA API.

Signed-off-by: Laura Abbott <labbott@redhat.com>
---
 drivers/staging/android/ion/Kconfig             | 14 ++++-
 drivers/staging/android/ion/Makefile            |  3 +
 drivers/staging/android/ion/ion-arm.c           | 83 +++++++++++++++++++++++++
 drivers/staging/android/ion/ion-arm64.c         | 46 ++++++++++++++
 drivers/staging/android/ion/ion-x86.c           | 34 ++++++++++
 drivers/staging/android/ion/ion.c               | 59 ++++++------------
 drivers/staging/android/ion/ion_carveout_heap.c |  5 +-
 drivers/staging/android/ion/ion_chunk_heap.c    |  7 +--
 drivers/staging/android/ion/ion_page_pool.c     |  3 +-
 drivers/staging/android/ion/ion_priv.h          | 14 ++---
 drivers/staging/android/ion/ion_system_heap.c   |  5 +-
 11 files changed, 210 insertions(+), 63 deletions(-)
 create mode 100644 drivers/staging/android/ion/ion-arm.c
 create mode 100644 drivers/staging/android/ion/ion-arm64.c
 create mode 100644 drivers/staging/android/ion/ion-x86.c

diff --git a/drivers/staging/android/ion/Kconfig b/drivers/staging/android/ion/Kconfig
index 19c1572..c1b1813 100644
--- a/drivers/staging/android/ion/Kconfig
+++ b/drivers/staging/android/ion/Kconfig
@@ -1,6 +1,6 @@
 menuconfig ION
 	bool "Ion Memory Manager"
-	depends on HAVE_MEMBLOCK && HAS_DMA && MMU
+	depends on HAVE_MEMBLOCK && HAS_DMA && MMU && (X86 || ARM || ARM64)
 	select GENERIC_ALLOCATOR
 	select DMA_SHARED_BUFFER
 	---help---
@@ -10,6 +10,18 @@ menuconfig ION
 	  If you're not using Android its probably safe to
 	  say N here.
 
+config ION_ARM
+	depends on ION && ARM
+	def_bool y
+
+config ION_ARM64
+	depends on ION && ARM64
+	def_bool y
+
+config ION_X86
+	depends on ION && X86
+	def_bool y
+
 config ION_TEST
 	tristate "Ion Test Device"
 	depends on ION
diff --git a/drivers/staging/android/ion/Makefile b/drivers/staging/android/ion/Makefile
index 18cc2aa..1b82bb5 100644
--- a/drivers/staging/android/ion/Makefile
+++ b/drivers/staging/android/ion/Makefile
@@ -1,5 +1,8 @@
 obj-$(CONFIG_ION) +=	ion.o ion_heap.o ion_page_pool.o ion_system_heap.o \
 			ion_carveout_heap.o ion_chunk_heap.o ion_cma_heap.o
+obj-$(CONFIG_ION_X86) += ion-x86.o
+obj-$(CONFIG_ION_ARM) += ion-arm.o
+obj-$(CONFIG_ION_ARM64) += ion-arm64.o
 obj-$(CONFIG_ION_TEST) += ion_test.o
 ifdef CONFIG_COMPAT
 obj-$(CONFIG_ION) += compat_ion.o
diff --git a/drivers/staging/android/ion/ion-arm.c b/drivers/staging/android/ion/ion-arm.c
new file mode 100644
index 0000000..b91287f
--- /dev/null
+++ b/drivers/staging/android/ion/ion-arm.c
@@ -0,0 +1,83 @@
+/*
+ * Copyright (C) 2016 Laura Abbott <laura@labbott.name>
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <linux/types.h>
+#include <linux/scatterlist.h>
+#include <linux/dma-mapping.h>
+#include <linux/highmem.h>
+
+#include <asm/cacheflush.h>
+
+#include "ion_priv.h"
+
+
+void ion_clean_page(struct page *page, size_t size)
+{
+	unsigned long pfn;
+	size_t left = size;
+
+	pfn = page_to_pfn(page);
+
+	/*
+	 * A single sg entry may refer to multiple physically contiguous
+	 * pages.  But we still need to process highmem pages individually.
+	 * If highmem is not configured then the bulk of this loop gets
+	 * optimized out.
+	 */
+	do {
+		size_t len = left;
+		void *vaddr;
+
+		page = pfn_to_page(pfn);
+
+		if (PageHighMem(page)) {
+			if (len > PAGE_SIZE)
+				len = PAGE_SIZE;
+
+			if (cache_is_vipt_nonaliasing()) {
+				vaddr = kmap_atomic(page);
+				__cpuc_flush_dcache_area(vaddr, len);
+				kunmap_atomic(vaddr);
+			} else {
+				vaddr = kmap_high_get(page);
+				if (vaddr) {
+					__cpuc_flush_dcache_area(vaddr, len);
+					kunmap_high(page);
+				}
+			}
+		} else {
+			vaddr = page_address(page);
+			__cpuc_flush_dcache_area(vaddr, len);
+		}
+		pfn++;
+		left -= len;
+	} while (left);
+}
+
+/*
+ * ARM has highmem and a bunch of other 'fun' features. It's so much easier just
+ * to do the ISA DMA and call things that way
+ */
+
+void ion_invalidate_buffer(struct ion_buffer *buffer)
+{
+	dma_unmap_sg(NULL, buffer->sg_table->sgl, buffer->sg_table->orig_nents,
+			DMA_BIDIRECTIONAL);
+}
+
+void ion_clean_buffer(struct ion_buffer *buffer)
+{
+	dma_map_sg(NULL, buffer->sg_table->sgl, buffer->sg_table->orig_nents,
+			DMA_BIDIRECTIONAL);
+}
diff --git a/drivers/staging/android/ion/ion-arm64.c b/drivers/staging/android/ion/ion-arm64.c
new file mode 100644
index 0000000..afd387a
--- /dev/null
+++ b/drivers/staging/android/ion/ion-arm64.c
@@ -0,0 +1,46 @@
+/*
+ * Copyright (C) 2016 Laura Abbott <laura@labbott.name>
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <linux/types.h>
+#include <linux/scatterlist.h>
+#include <linux/dma-mapping.h>
+
+#include <asm/cacheflush.h>
+
+#include "ion_priv.h"
+
+void ion_clean_page(struct page *page, size_t size)
+{
+	__flush_dcache_area(page_address(page), size);
+}
+
+void ion_invalidate_buffer(struct ion_buffer *buffer)
+{
+	struct scatterlist *sg;
+	int i;
+
+	for_each_sg(buffer->sg_table->sgl, sg, buffer->sg_table->orig_nents, i)
+		__dma_unmap_area(page_address(sg_page(sg)), sg->length,
+					DMA_BIDIRECTIONAL);
+}
+
+void ion_clean_buffer(struct ion_buffer *buffer)
+{
+	struct scatterlist *sg;
+	int i;
+
+	for_each_sg(buffer->sg_table->sgl, sg, buffer->sg_table->orig_nents, i)
+		__dma_map_area(page_address(sg_page(sg)), sg->length,
+					DMA_BIDIRECTIONAL);
+}
diff --git a/drivers/staging/android/ion/ion-x86.c b/drivers/staging/android/ion/ion-x86.c
new file mode 100644
index 0000000..05fd684
--- /dev/null
+++ b/drivers/staging/android/ion/ion-x86.c
@@ -0,0 +1,34 @@
+/*
+ * Copyright (C) 2016 Laura Abbott <laura@labbott.name>
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <linux/types.h>
+#include <linux/scatterlist.h>
+#include <linux/dma-mapping.h>
+
+#include "ion_priv.h"
+
+void ion_clean_page(struct page *page, size_t size)
+{
+	/* Do nothing right now */
+}
+
+void ion_invalidate_buffer(struct ion_buffer *buffer)
+{
+	/* Do nothing right now */
+}
+
+void ion_clean_buffer(struct ion_buffer *buffer)
+{
+	/* Do nothing right now */
+}
diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
index a2cf93b..9282d96a 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -937,42 +937,6 @@ struct sg_table *ion_sg_table(struct ion_client *client,
 }
 EXPORT_SYMBOL(ion_sg_table);
 
-static void ion_buffer_sync_for_device(struct ion_buffer *buffer,
-				       struct device *dev,
-				       enum dma_data_direction direction);
-
-static struct sg_table *ion_map_dma_buf(struct dma_buf_attachment *attachment,
-					enum dma_data_direction direction)
-{
-	struct dma_buf *dmabuf = attachment->dmabuf;
-	struct ion_buffer *buffer = dmabuf->priv;
-
-	ion_buffer_sync_for_device(buffer, attachment->dev, direction);
-	return buffer->sg_table;
-}
-
-static void ion_unmap_dma_buf(struct dma_buf_attachment *attachment,
-			      struct sg_table *table,
-			      enum dma_data_direction direction)
-{
-}
-
-void ion_pages_sync_for_device(struct device *dev, struct page *page,
-		size_t size, enum dma_data_direction dir)
-{
-	struct scatterlist sg;
-
-	sg_init_table(&sg, 1);
-	sg_set_page(&sg, page, size, 0);
-	/*
-	 * This is not correct - sg_dma_address needs a dma_addr_t that is valid
-	 * for the targeted device, but this works on the currently targeted
-	 * hardware.
-	 */
-	sg_dma_address(&sg) = page_to_phys(page);
-	dma_sync_sg_for_device(dev, &sg, 1, dir);
-}
-
 struct ion_vma_list {
 	struct list_head list;
 	struct vm_area_struct *vma;
@@ -997,8 +961,7 @@ static void ion_buffer_sync_for_device(struct ion_buffer *buffer,
 		struct page *page = buffer->pages[i];
 
 		if (ion_buffer_page_is_dirty(page))
-			ion_pages_sync_for_device(dev, ion_buffer_page(page),
-							PAGE_SIZE, dir);
+			ion_clean_page(ion_buffer_page(page), PAGE_SIZE);
 
 		ion_buffer_page_clean(buffer->pages + i);
 	}
@@ -1011,6 +974,22 @@ static void ion_buffer_sync_for_device(struct ion_buffer *buffer,
 	mutex_unlock(&buffer->lock);
 }
 
+static struct sg_table *ion_map_dma_buf(struct dma_buf_attachment *attachment,
+					enum dma_data_direction direction)
+{
+	struct dma_buf *dmabuf = attachment->dmabuf;
+	struct ion_buffer *buffer = dmabuf->priv;
+
+	ion_buffer_sync_for_device(buffer, attachment->dev, direction);
+	return buffer->sg_table;
+}
+
+static void ion_unmap_dma_buf(struct dma_buf_attachment *attachment,
+			      struct sg_table *table,
+			      enum dma_data_direction direction)
+{
+}
+
 static int ion_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
 {
 	struct ion_buffer *buffer = vma->vm_private_data;
@@ -1293,8 +1272,8 @@ static int ion_sync_for_device(struct ion_client *client, int fd)
 	}
 	buffer = dmabuf->priv;
 
-	dma_sync_sg_for_device(NULL, buffer->sg_table->sgl,
-			       buffer->sg_table->nents, DMA_BIDIRECTIONAL);
+	ion_clean_buffer(buffer);
+
 	dma_buf_put(dmabuf);
 	return 0;
 }
diff --git a/drivers/staging/android/ion/ion_carveout_heap.c b/drivers/staging/android/ion/ion_carveout_heap.c
index 1fb0d81..d213cbf 100644
--- a/drivers/staging/android/ion/ion_carveout_heap.c
+++ b/drivers/staging/android/ion/ion_carveout_heap.c
@@ -116,8 +116,7 @@ static void ion_carveout_heap_free(struct ion_buffer *buffer)
 	ion_heap_buffer_zero(buffer);
 
 	if (ion_buffer_cached(buffer))
-		dma_sync_sg_for_device(NULL, table->sgl, table->nents,
-				       DMA_BIDIRECTIONAL);
+		ion_clean_page(page, buffer->size);
 
 	ion_carveout_free(heap, paddr, buffer->size);
 	sg_free_table(table);
@@ -157,7 +156,7 @@ struct ion_heap *ion_carveout_heap_create(struct ion_platform_heap *heap_data)
 	page = pfn_to_page(PFN_DOWN(heap_data->base));
 	size = heap_data->size;
 
-	ion_pages_sync_for_device(NULL, page, size, DMA_BIDIRECTIONAL);
+	ion_clean_page(page, size);
 
 	ret = ion_heap_pages_zero(page, size, pgprot_writecombine(PAGE_KERNEL));
 	if (ret)
diff --git a/drivers/staging/android/ion/ion_chunk_heap.c b/drivers/staging/android/ion/ion_chunk_heap.c
index e0553fe..0666529 100644
--- a/drivers/staging/android/ion/ion_chunk_heap.c
+++ b/drivers/staging/android/ion/ion_chunk_heap.c
@@ -104,11 +104,10 @@ static void ion_chunk_heap_free(struct ion_buffer *buffer)
 
 	ion_heap_buffer_zero(buffer);
 
-	if (ion_buffer_cached(buffer))
-		dma_sync_sg_for_device(NULL, table->sgl, table->nents,
-							DMA_BIDIRECTIONAL);
 
 	for_each_sg(table->sgl, sg, table->nents, i) {
+		if (ion_buffer_cached(buffer))
+			ion_clean_page(sg_page(table->sgl), sg->length);
 		gen_pool_free(chunk_heap->pool, page_to_phys(sg_page(sg)),
 			      sg->length);
 	}
@@ -148,7 +147,7 @@ struct ion_heap *ion_chunk_heap_create(struct ion_platform_heap *heap_data)
 	page = pfn_to_page(PFN_DOWN(heap_data->base));
 	size = heap_data->size;
 
-	ion_pages_sync_for_device(NULL, page, size, DMA_BIDIRECTIONAL);
+	ion_clean_page(page, size);
 
 	ret = ion_heap_pages_zero(page, size, pgprot_writecombine(PAGE_KERNEL));
 	if (ret)
diff --git a/drivers/staging/android/ion/ion_page_pool.c b/drivers/staging/android/ion/ion_page_pool.c
index 1fe8016..b854f91 100644
--- a/drivers/staging/android/ion/ion_page_pool.c
+++ b/drivers/staging/android/ion/ion_page_pool.c
@@ -30,8 +30,7 @@ static void *ion_page_pool_alloc_pages(struct ion_page_pool *pool)
 
 	if (!page)
 		return NULL;
-	ion_pages_sync_for_device(NULL, page, PAGE_SIZE << pool->order,
-						DMA_BIDIRECTIONAL);
+	ion_clean_page(page, PAGE_SIZE << pool->order);
 	return page;
 }
 
diff --git a/drivers/staging/android/ion/ion_priv.h b/drivers/staging/android/ion/ion_priv.h
index 0239883..b368338 100644
--- a/drivers/staging/android/ion/ion_priv.h
+++ b/drivers/staging/android/ion/ion_priv.h
@@ -392,15 +392,9 @@ void ion_page_pool_free(struct ion_page_pool *, struct page *);
 int ion_page_pool_shrink(struct ion_page_pool *pool, gfp_t gfp_mask,
 			  int nr_to_scan);
 
-/**
- * ion_pages_sync_for_device - cache flush pages for use with the specified
- *                             device
- * @dev:		the device the pages will be used with
- * @page:		the first page to be flushed
- * @size:		size in bytes of region to be flushed
- * @dir:		direction of dma transfer
- */
-void ion_pages_sync_for_device(struct device *dev, struct page *page,
-		size_t size, enum dma_data_direction dir);
+void ion_clean_page(struct page *page, size_t size);
+
+void ion_clean_buffer(struct ion_buffer *buffer);
 
+void ion_invalidate_buffer(struct ion_buffer *buffer);
 #endif /* _ION_PRIV_H */
diff --git a/drivers/staging/android/ion/ion_system_heap.c b/drivers/staging/android/ion/ion_system_heap.c
index b69dfc7..aa39660 100644
--- a/drivers/staging/android/ion/ion_system_heap.c
+++ b/drivers/staging/android/ion/ion_system_heap.c
@@ -70,8 +70,7 @@ static struct page *alloc_buffer_page(struct ion_system_heap *heap,
 		page = alloc_pages(gfp_flags | __GFP_COMP, order);
 		if (!page)
 			return NULL;
-		ion_pages_sync_for_device(NULL, page, PAGE_SIZE << order,
-						DMA_BIDIRECTIONAL);
+		ion_clean_page(page, PAGE_SIZE << order);
 	}
 
 	return page;
@@ -360,7 +359,7 @@ static int ion_system_contig_heap_allocate(struct ion_heap *heap,
 
 	buffer->priv_virt = table;
 
-	ion_pages_sync_for_device(NULL, page, len, DMA_BIDIRECTIONAL);
+	ion_clean_page(page, len);
 
 	return 0;
 
-- 
2.5.5

_______________________________________________
Linaro-mm-sig mailing list
Linaro-mm-sig at lists.linaro.org
https://lists.linaro.org/mailman/listinfo/linaro-mm-sig

[RFC][PATCH 2/3] staging: ion: Add support for syncing with DMA_BUF_IOCTL_SYNC

[Laura Abbott]

dma_buf added support for a userspace syncing ioctl. It is implemented
by calling dma_buf_begin_cpu_access and dma_buf_end_cpu_access. Ion
currently lacks cache operations on this code path. Add them for
compatibility with the dma_buf ioctl.

Signed-off-by: Laura Abbott <labbott@redhat.com>
---
 drivers/staging/android/ion/ion.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
index 9282d96a..2096592 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -1117,6 +1117,11 @@ static int ion_dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
 	mutex_lock(&buffer->lock);
 	vaddr = ion_buffer_kmap_get(buffer);
 	mutex_unlock(&buffer->lock);
+
+	if (direction != DMA_TO_DEVICE) {
+		ion_invalidate_buffer(buffer);
+	}
+
 	return PTR_ERR_OR_ZERO(vaddr);
 }
 
@@ -1129,6 +1134,12 @@ static int ion_dma_buf_end_cpu_access(struct dma_buf *dmabuf,
 	ion_buffer_kmap_put(buffer);
 	mutex_unlock(&buffer->lock);
 
+	if (direction == DMA_FROM_DEVICE) {
+		ion_invalidate_buffer(buffer);
+	} else {
+		ion_clean_buffer(buffer);
+	}
+
 	return 0;
 }
 
@@ -1259,6 +1270,8 @@ static int ion_sync_for_device(struct ion_client *client, int fd)
 	struct dma_buf *dmabuf;
 	struct ion_buffer *buffer;
 
+	WARN_ONCE(1, "This API is deprecated in favor of the dma_buf ioctl\n");
+
 	dmabuf = dma_buf_get(fd);
 	if (IS_ERR(dmabuf))
 		return PTR_ERR(dmabuf);
-- 
2.5.5

[RFC][PATCH 3/3] staging: ion: Add dma_map/dma_unmap calls to dma_buf calls

[Laura Abbott]

The .map_dma_buf/.unmap_dma_buf function calls are designed for buffer
users to perform DMA accesses. Ion doesn't call into the lower DMA
layer currently. This may leave mapping incomplete for devices with
more complex topologies (e.g. IOMMUs). Add the appropriate dma_map
calls to the dma_buf call backs.

Signed-off-by: Laura Abbott <labbott@redhat.com>
---
 drivers/staging/android/ion/ion.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
index 2096592..52dbea4 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -980,7 +980,18 @@ static struct sg_table *ion_map_dma_buf(struct dma_buf_attachment *attachment,
 	struct dma_buf *dmabuf = attachment->dmabuf;
 	struct ion_buffer *buffer = dmabuf->priv;
 
+	/* Necessary but not sufficient: This is for userspace */
 	ion_buffer_sync_for_device(buffer, attachment->dev, direction);
+
+	/* The rest of this is for the standard mappings */
+	buffer->sg_table->nents = dma_map_sg(attachment->dev,
+					buffer->sg_table->sgl,
+					buffer->sg_table->orig_nents,
+					direction);
+
+	if (!buffer->sg_table->nents)
+		return ERR_PTR(-EIO);
+
 	return buffer->sg_table;
 }
 
@@ -988,6 +999,7 @@ static void ion_unmap_dma_buf(struct dma_buf_attachment *attachment,
 			      struct sg_table *table,
 			      enum dma_data_direction direction)
 {
+	dma_unmap_sg(attachment->dev, table->sgl, table->nents, direction);
 }
 
 static int ion_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
-- 
2.5.5