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
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 000..b91287f
> --- /dev/null
> +++ b/drivers/staging/android/ion/ion-arm.c
> @@ -0,0 +1,83 @@
> +/*
> + * Copyright (C) 2016 Laura Abbott
> + *
> + * 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
> +#include
> +#include
> +#include
> +
> +#include
> +
> +#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);
> +