[PATCH/RFC 0/7] Samsung IOMMU videobuf2 allocator and s5p-fimc update
Kukjin Kim
kgene.kim at samsung.com
Tue Mar 8 02:28:34 EST 2011
Hello,
There are comments for your System MMU driver below.
It's good that System MMU has functionality of mapping but System MMU have
to use other mapping of virtual memory allocator.
And would be better to change sysmmu_list to use array of defined in
s5p_sysmmu_ip enumeration, so that can get enhancement of memory space
usage, speed, and readability of codes.
TLB replacement policy does not need to use LRU. Of course, current System
MMU also needs it. I think, the round robin is enough, because to access
memory has no temporal locality and to make LRU need to access to System
MMU register one more. The reset value is round robin.
In the setting of SHARED page table in s5p_sysmmu_control_locked, get the
page table base address of ARM core from cp15 register now. But current->mm-
>pgd is better for more compatibility.
When it make page table with PRIVATE page table methods, the size of the
structure to manage the second page table is quite big. It is much better
rather that to make slab with cache size of 1KB.
Besides, the page mapping implementation is not safe in your System MMU
driver. Because only first one confirms primary page table entry, when it
assigns four second page tables consecutively at a time.
The System MMU driver cannot apply runtime pm by oneself with calling
pm_runtime_put_sync(). The reason is because a device with System MMU can
on/off power. I think just clock gating is enough. However, I can't find
clock enable/disable in your driver.
By PRIVATE page table method, each system MMU comes to have a page table
only for oneself. In this case, the problem is that each MFC System MMU L
and R having another page table.
In your System MMU driver, the page size is always 4KB crucially. This says
TLB thrashing and produces a result to lose a TLB hit rate. It is a big
problem with the device such as rotator which does not do sequential access
especially.
And the IRQ handler just outputs only a message. It should be implemented
in call back function to be able to handle from each device driver.
When it sets System MMU in SHARED page table, kernel virtual memory is
broken by a method such as s5p_sysmmu_map_area() :(
Thanks.
Best regards,
Kgene.
--
Kukjin Kim <kgene.kim at samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.
Marek Szyprowski wrote:
>
> Hello,
>
> This patch series introduces new type of videbuf2 memory allocator -
> vb2-s5p-iommu. This allocator can be used only on Samsung SoCs that have
> IOMMU module. Currently only Samsung EXYNOS4 (former S5PV310) platform
> has SYSMMU modules. The allocator is then used by s5p-fimc driver. To
> make it possible some additional changes are required. Mainly platform
> support for s5p-fimc for EXYNOS4 machines need to be defined. The
> proposed solution has been tested on Universal C210 board (Samsung
> S5PC210/EXYNOS4 based).
>
> We decided to use driver private address space mode of the iommu driver.
> This way each vb2-s5p-iommu client gets it's own address space for
> memory buffers. This reduces kernel virtual memory fragmentation as well
> as solves some non-trivial page table updates issues. The drawback is
> the fact that the interface for s5p-sysmmu has been changed.
>
> This IOMMU allocator has no dependences on other subsystems besides
> Samsung platfrom core. We also ported s5p-mfc and s5p-tv drivers to this
> allocator, they will be posted in separate patch series. This will
> enable to get them working on EXYNOS4 (S5PV310) platform. Support for
> S5PV210/S5PC110 platform still depends on CMA allocator that needs more
> discussion on memory management mailing list and development. The
> patches with updated s5p-mfc and s5p-tv drivers will follow.
>
> To get FIMC module working on EXYNOS4/UniversalC210 board we also added
> support for power domains and power gating.
>
> This patch series contains a collection of patches for various platform
> subsystems. Here is a detailed list:
>
> [PATCH 1/7] ARM: S5PV310: Add platform definitions for FIMC
> - adds basic platform resources for FIMC modules (for s5p-fimc driver)
>
> [PATCH 2/7] ARM: S5PV310: power domains: fixes and code cleanup
> - adds support for block gating in Samsung power domain driver and
> performs some cleanup
>
> [PATCH 3/7] ARM: Samsung: update/rewrite Samsung SYSMMU (IOMMU) driver
> - a complete rewrite of sysmmu driver for Samsung platform:
> - the new version introduces device private page tables (address space)
> mode
> - simplified the resource management (no more horrible single platform
> device with 32 resources is needed)
> - some other API chages required by upcoming videobuf2 allocator
>
> [PATCH 4/7] v4l: videobuf2: add Samsung SYSMMU (IOMMU) based allocator
> - introduces new memory allocator for videobuf2, it uses s5p-sysmmu
> iommu driver, memory for video buffers is acuired by alloc_page() kernel
> function
>
> [PATCH 5/7] s5p-fimc: add pm_runtime support
> - adds support for pm_runtime in s5p-fimc driver
>
> [PATCH 6/7] s5p-fimc: Add support for vb2-s5p-iommu allocator
> - adds support for the newly introduces videbuf2-s5p-iommu allocator
> on EXYNOS4 platform
>
> [PATCH 7/7] ARM: S5PC210: enable FIMC on Universal_C210
> - adds all required machine definitions to get FIMC modules working
> on Universal C210 boards
>
>
> The patch series is based on git://linuxtv.org/media_tree.git tree,
> staging/for_v2.6.39 branch with the following Samsung platform patches:
> 1. [PATCH] ARM: Samsung: change suspend/resume code to depend on
> CONFIG_SUSPEND
> http://www.mail-archive.com/linux-samsung-
soc at vger.kernel.org/msg04025.html
> 2. [PATCH v2] ARM: S5PC210: add support for i2c PMICs on Universal_C210
board
> http://www.mail-archive.com/linux-samsung-
soc at vger.kernel.org/msg04029.html
>
> This series has not been rebased onto the latest changes (S5PV310
> renamed to EXYNOS4) in
> git://git.kernel.org/pub/scm/linux/kernel/git/kgene/linux-samsung.git,
> for-next branch. We will rebase them soon, but first we want to get
> feedback and comments on the s5p-iommu videobuf2 allocator idea.
>
> Best regards
> --
> Marek Szyprowski
> Samsung Poland R&D Center
>
>
> Complete patch summary:
>
> Andrzej Pietrasiewicz (2):
> ARM: Samsung: update/rewrite Samsung SYSMMU (IOMMU) driver
> v4l: videobuf2: add Samsung SYSMMU (IOMMU) based allocator
>
> Marek Szyprowski (3):
> s5p-fimc: add pm_runtime support
> s5p-fimc: Add support for vb2-s5p-iommu allocator
> ARM: S5PC210: enable FIMC on Universal_C210
>
> Sylwester Nawrocki (1):
> ARM: S5PV310: Add platform definitions for FIMC
>
> Tomasz Stanislawski (1):
> ARM: S5PV310: power domains: fixes and code cleanup
>
> arch/arm/mach-s5pv310/Kconfig | 6 +
> arch/arm/mach-s5pv310/clock.c | 91 ++
> arch/arm/mach-s5pv310/cpu.c | 7 +
> arch/arm/mach-s5pv310/dev-pd.c | 93 ++-
> arch/arm/mach-s5pv310/dev-sysmmu.c | 582 +++++++++----
> arch/arm/mach-s5pv310/include/mach/irqs.h | 40 +-
> arch/arm/mach-s5pv310/include/mach/map.h | 8 +
> arch/arm/mach-s5pv310/include/mach/regs-clock.h | 12 +
> arch/arm/mach-s5pv310/include/mach/regs-sysmmu.h | 23 +-
> arch/arm/mach-s5pv310/include/mach/sysmmu.h | 122 ---
> arch/arm/mach-s5pv310/mach-universal_c210.c | 20 +
> arch/arm/plat-s5p/Kconfig | 22 +-
> arch/arm/plat-s5p/Makefile | 1 +
> arch/arm/plat-s5p/dev-fimc3.c | 43 +
> arch/arm/plat-s5p/include/plat/sysmmu.h | 127 +++
> arch/arm/plat-s5p/sysmmu.c | 988 +++++++++++++++--
---
> --
> arch/arm/plat-samsung/include/plat/devs.h | 3 +-
> arch/arm/plat-samsung/include/plat/fimc-core.h | 5 +
> arch/arm/plat-samsung/include/plat/pd.h | 1 +
> drivers/media/video/Kconfig | 11 +-
> drivers/media/video/Makefile | 1 +
> drivers/media/video/s5p-fimc/fimc-capture.c | 9 +-
> drivers/media/video/s5p-fimc/fimc-core.c | 36 +-
> drivers/media/video/s5p-fimc/fimc-mem.h | 87 ++
> drivers/media/video/videobuf2-s5p-iommu.c | 444 ++++++++++
> include/media/videobuf2-s5p-iommu.h | 50 ++
> 26 files changed, 2129 insertions(+), 703 deletions(-)
> rewrite arch/arm/mach-s5pv310/dev-sysmmu.c (86%)
> delete mode 100644 arch/arm/mach-s5pv310/include/mach/sysmmu.h
> create mode 100644 arch/arm/plat-s5p/dev-fimc3.c
> create mode 100644 arch/arm/plat-s5p/include/plat/sysmmu.h
> rewrite arch/arm/plat-s5p/sysmmu.c (85%)
> create mode 100644 drivers/media/video/s5p-fimc/fimc-mem.h
> create mode 100644 drivers/media/video/videobuf2-s5p-iommu.c
> create mode 100644 include/media/videobuf2-s5p-iommu.h
>
> --
> 1.7.1.569.g6f426
More information about the linux-arm-kernel
mailing list