[PATCH v4 0/7] Exynos: MFC driver: reserved memory cleanup and IOMMU support

[PATCH v4 0/7] Exynos: MFC driver: reserved memory cleanup and IOMMU support

[Marek Szyprowski]

Hello,

This is another version of the patchset, which performs cleanup of
custom code for reserved memory handling in s5p-mfc codec driver. The
first part is removal of custom, driver specific code for intializing
and handling of reserved memory. Instead, a generic code for reserved
memory regions is used. Then, the driver is extended with IOMMU support.
The additional code is needed because of specific requirements of MFC
device firmware (see patch 3 for more details). When no IOMMU is
available, the code fallbacks to use generic reserved memory regions.

After applying this patchset, MFC device works correctly either when
IOMMU is either enabled or disabled (assuming that board provides
required reserved memory regions).

Patches have been tested on top of linux-next from 20160524 with some
additional patches:
- "[PATCH 0/3] [media] s5p-mfc: Fixes for issues when module is removed":
  https://lkml.org/lkml/2016/5/3/782
- "[PATCH v5 0/2] vb2-dma-contig: configure DMA max segment size properly":
  https://www.mail-archive.com/linux-media@vger.kernel.org/msg97625.html

I've prepared a git branch with all needed patches:
https://git.linaro.org/people/marek.szyprowski/linux-srpol.git v4.6-next20160524-mfc

I would prefer to merge patches 1-4 via media tree and patches 5-7 via
Samsung SoC tree (there are no compile-time dependencies between those
two sets). Patches have been tested on Odroid U3 (Exynos 4412 based) and
Odroid XU3 (Exynos 5422 based) boards.

Best regards
Marek Szyprowski
Samsung R&D Institute Poland


Changelog:
v4:
- moved videobuf2-dma-contig/dma max segment size patches to separate thread:
  https://www.mail-archive.com/linux-media@vger.kernel.org/msg97625.html
- removed 'named' support for reserved memory regions, now regions are selected
 by index (requested by Rob Herring in v3)
- splitted 'ARM: Exynos: convert MFC device to generic reserved memory bindings'
  patch into 3 separate changes (bindings, dts, code removal), rewrote commit
  messages to better describe the changes
- rebased onto latest linux-next kernel tree

v3: http://comments.gmane.org/gmane.linux.drivers.video-input-infrastructure/97316
- fixed issues pointed by Laurent Pinchart:
  - added documentation to memory-region-names property,
  - changed devm_kzalloc to kzalloc in vb2_dma_contig_set_max_seg_size() to
    avoid access to freed memory after reloading driver module
- unified odroid mfc reserved memory configuration with other Exynos4 boards

v2: http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/97025
- reworked of_reserved_mem_init* functions on request from Rob Herring,
  added separate index and name based selection of reserved region
- adapted for of_reserved_mem_init* related changes

v1: https://www.mail-archive.com/linux-media@vger.kernel.org/msg94100.html
- initial version of another approach for this problem, rewrote driver code
  for new reserved memory bindings, which finally have been merged some
  time ago

v0: http://lists.infradead.org/pipermail/linux-arm-kernel/2013-August/189259.html
- old patchset solving the same problem, abandoned due to other tasks
  and long time of merging reserved memory bindings and support code for
  it

Patch summary:


Marek Szyprowski (7):
  of: reserved_mem: add support for using more than one region for given
    device
  media: s5p-mfc: use generic reserved memory bindings
  media: s5p-mfc: replace custom reserved memory handling code with
    generic one
  media: s5p-mfc: add iommu support
  ARM: Exynos: remove code for MFC custom reserved memory handling
  ARM: dts: exynos: convert MFC device to generic reserved memory
    bindings
  ARM: dts: exynos4412-odroid*: enable MFC device

 .../devicetree/bindings/media/s5p-mfc.txt          |  39 +++--
 arch/arm/boot/dts/exynos-mfc-reserved-memory.dtsi  |  27 ++++
 arch/arm/boot/dts/exynos4210-origen.dts            |   4 +-
 arch/arm/boot/dts/exynos4210-smdkv310.dts          |   4 +-
 arch/arm/boot/dts/exynos4412-odroid-common.dtsi    |   6 +
 arch/arm/boot/dts/exynos4412-origen.dts            |   4 +-
 arch/arm/boot/dts/exynos4412-smdk4412.dts          |   4 +-
 arch/arm/boot/dts/exynos5250-arndale.dts           |   4 +-
 arch/arm/boot/dts/exynos5250-smdk5250.dts          |   4 +-
 arch/arm/boot/dts/exynos5250-spring.dts            |   4 +-
 arch/arm/boot/dts/exynos5420-arndale-octa.dts      |   4 +-
 arch/arm/boot/dts/exynos5420-peach-pit.dts         |   4 +-
 arch/arm/boot/dts/exynos5420-smdk5420.dts          |   4 +-
 arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi |   4 +-
 arch/arm/boot/dts/exynos5800-peach-pi.dts          |   4 +-
 arch/arm/mach-exynos/Makefile                      |   2 -
 arch/arm/mach-exynos/exynos.c                      |  19 ---
 arch/arm/mach-exynos/mfc.h                         |  16 ---
 arch/arm/mach-exynos/s5p-dev-mfc.c                 |  93 ------------
 drivers/media/platform/s5p-mfc/s5p_mfc.c           | 158 +++++++++++----------
 drivers/media/platform/s5p-mfc/s5p_mfc_iommu.h     |  79 +++++++++++
 drivers/of/of_reserved_mem.c                       |  85 ++++++++---
 include/linux/of_reserved_mem.h                    |  25 +++-
 23 files changed, 338 insertions(+), 259 deletions(-)
 create mode 100644 arch/arm/boot/dts/exynos-mfc-reserved-memory.dtsi
 delete mode 100644 arch/arm/mach-exynos/mfc.h
 delete mode 100644 arch/arm/mach-exynos/s5p-dev-mfc.c
 create mode 100644 drivers/media/platform/s5p-mfc/s5p_mfc_iommu.h

-- 
1.9.2

[PATCH v4 1/7] of: reserved_mem: add support for using more than one region for given device

[Marek Szyprowski]

This patch allows device drivers to initialize more than one reserved
memory region assigned to given device. When driver needs to use more
than one reserved memory region, it should allocate child devices and
initialize regions by index for each of its child devices.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 drivers/of/of_reserved_mem.c    | 85 +++++++++++++++++++++++++++++++----------
 include/linux/of_reserved_mem.h | 25 ++++++++++--
 2 files changed, 86 insertions(+), 24 deletions(-)

diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
index ed01c01..04e4fe5 100644
--- a/drivers/of/of_reserved_mem.c
+++ b/drivers/of/of_reserved_mem.c
@@ -21,6 +21,7 @@
 #include <linux/sizes.h>
 #include <linux/of_reserved_mem.h>
 #include <linux/sort.h>
+#include <linux/slab.h>
 
 #define MAX_RESERVED_REGIONS	16
 static struct reserved_mem reserved_mem[MAX_RESERVED_REGIONS];
@@ -289,53 +290,95 @@ static inline struct reserved_mem *__find_rmem(struct device_node *node)
 	return NULL;
 }
 
+struct rmem_assigned_device {
+	struct device *dev;
+	struct reserved_mem *rmem;
+	struct list_head list;
+};
+
+static LIST_HEAD(of_rmem_assigned_device_list);
+static DEFINE_MUTEX(of_rmem_assigned_device_mutex);
+
 /**
- * of_reserved_mem_device_init() - assign reserved memory region to given device
+ * of_reserved_mem_device_init_by_idx() - assign reserved memory region to
+ *					  given device
+ * @dev:	Pointer to the device to configure
+ * @np:		Pointer to the device_node with 'reserved-memory' property
+ * @idx:	Index of selected region
  *
- * This function assign memory region pointed by "memory-region" device tree
- * property to the given device.
+ * This function assigns respective DMA-mapping operations based on reserved
+ * memory region specified by 'memory-region' property in @np node to the @dev
+ * device. When driver needs to use more than one reserved memory region, it
+ * should allocate child devices and initialize regions by name for each of
+ * child device.
+ *
+ * Returns error code or zero on success.
  */
-int of_reserved_mem_device_init(struct device *dev)
+int of_reserved_mem_device_init_by_idx(struct device *dev,
+				       struct device_node *np, int idx)
 {
+	struct rmem_assigned_device *rd;
+	struct device_node *target;
 	struct reserved_mem *rmem;
-	struct device_node *np;
 	int ret;
 
-	np = of_parse_phandle(dev->of_node, "memory-region", 0);
-	if (!np)
-		return -ENODEV;
+	if (!np || !dev)
+		return -EINVAL;
+
+	target = of_parse_phandle(np, "memory-region", idx);
+	if (!target)
+		return -EINVAL;
 
-	rmem = __find_rmem(np);
-	of_node_put(np);
+	rmem = __find_rmem(target);
+	of_node_put(target);
 
 	if (!rmem || !rmem->ops || !rmem->ops->device_init)
 		return -EINVAL;
 
+	rd = kmalloc(sizeof(struct rmem_assigned_device), GFP_KERNEL);
+	if (!rd)
+		return -ENOMEM;
+
 	ret = rmem->ops->device_init(rmem, dev);
-	if (ret == 0)
+	if (ret == 0) {
+		rd->dev = dev;
+		rd->rmem = rmem;
+
+		mutex_lock(&of_rmem_assigned_device_mutex);
+		list_add(&rd->list, &of_rmem_assigned_device_list);
+		mutex_unlock(&of_rmem_assigned_device_mutex);
+
 		dev_info(dev, "assigned reserved memory node %s\n", rmem->name);
+	} else {
+		kfree(rd);
+	}
 
 	return ret;
 }
-EXPORT_SYMBOL_GPL(of_reserved_mem_device_init);
+EXPORT_SYMBOL_GPL(of_reserved_mem_device_init_by_idx);
 
 /**
  * of_reserved_mem_device_release() - release reserved memory device structures
+ * @dev:	Pointer to the device to deconfigure
  *
  * This function releases structures allocated for memory region handling for
  * the given device.
  */
 void of_reserved_mem_device_release(struct device *dev)
 {
-	struct reserved_mem *rmem;
-	struct device_node *np;
-
-	np = of_parse_phandle(dev->of_node, "memory-region", 0);
-	if (!np)
-		return;
-
-	rmem = __find_rmem(np);
-	of_node_put(np);
+	struct rmem_assigned_device *rd;
+	struct reserved_mem *rmem = NULL;
+
+	mutex_lock(&of_rmem_assigned_device_mutex);
+	list_for_each_entry(rd, &of_rmem_assigned_device_list, list) {
+		if (rd->dev == dev) {
+			rmem = rd->rmem;
+			list_del(&rd->list);
+			kfree(rd);
+			break;
+		}
+	}
+	mutex_unlock(&of_rmem_assigned_device_mutex);
 
 	if (!rmem || !rmem->ops || !rmem->ops->device_release)
 		return;
diff --git a/include/linux/of_reserved_mem.h b/include/linux/of_reserved_mem.h
index ad2f670..1779cda 100644
--- a/include/linux/of_reserved_mem.h
+++ b/include/linux/of_reserved_mem.h
@@ -1,7 +1,8 @@
 #ifndef __OF_RESERVED_MEM_H
 #define __OF_RESERVED_MEM_H
 
-struct device;
+#include <linux/device.h>
+
 struct of_phandle_args;
 struct reserved_mem_ops;
 
@@ -28,14 +29,17 @@ typedef int (*reservedmem_of_init_fn)(struct reserved_mem *rmem);
 	_OF_DECLARE(reservedmem, name, compat, init, reservedmem_of_init_fn)
 
 #ifdef CONFIG_OF_RESERVED_MEM
-int of_reserved_mem_device_init(struct device *dev);
+
+int of_reserved_mem_device_init_by_idx(struct device *dev,
+				       struct device_node *np, int idx);
 void of_reserved_mem_device_release(struct device *dev);
 
 void fdt_init_reserved_mem(void);
 void fdt_reserved_mem_save_node(unsigned long node, const char *uname,
 			       phys_addr_t base, phys_addr_t size);
 #else
-static inline int of_reserved_mem_device_init(struct device *dev)
+static inline int of_reserved_mem_device_init_by_idx(struct device *dev,
+					struct device_node *np, int idx)
 {
 	return -ENOSYS;
 }
@@ -46,4 +50,19 @@ static inline void fdt_reserved_mem_save_node(unsigned long node,
 		const char *uname, phys_addr_t base, phys_addr_t size) { }
 #endif
 
+/**
+ * of_reserved_mem_device_init() - assign reserved memory region to given device
+ * @dev:	Pointer to the device to configure
+ *
+ * This function assigns respective DMA-mapping operations based on the first
+ * reserved memory region specified by 'memory-region' property in device tree
+ * node of the given device.
+ *
+ * Returns error code or zero on success.
+ */
+static inline int of_reserved_mem_device_init(struct device *dev)
+{
+	return of_reserved_mem_device_init_by_idx(dev, dev->of_node, 0);
+}
+
 #endif /* __OF_RESERVED_MEM_H */
-- 
1.9.2

[PATCH v4 2/7] media: s5p-mfc: use generic reserved memory bindings

[Marek Szyprowski]

Use generic reserved memory bindings and mark old, custom properties
as obsoleted.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 .../devicetree/bindings/media/s5p-mfc.txt          | 39 +++++++++++++++++-----
 1 file changed, 31 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/s5p-mfc.txt b/Documentation/devicetree/bindings/media/s5p-mfc.txt
index 2d5787e..92c94f5 100644
--- a/Documentation/devicetree/bindings/media/s5p-mfc.txt
+++ b/Documentation/devicetree/bindings/media/s5p-mfc.txt
@@ -21,15 +21,18 @@ Required properties:
   - clock-names : from common clock binding: must contain "mfc",
 		  corresponding to entry in the clocks property.
 
-  - samsung,mfc-r : Base address of the first memory bank used by MFC
-		    for DMA contiguous memory allocation and its size.
-
-  - samsung,mfc-l : Base address of the second memory bank used by MFC
-		    for DMA contiguous memory allocation and its size.
-
 Optional properties:
   - power-domains : power-domain property defined with a phandle
 			   to respective power domain.
+  - memory-region : from reserved memory binding: phandles to two reserved
+	memory regions, first is for "left" mfc memory bus interfaces,
+	second if for the "right" mfc memory bus, used when no SYSMMU
+	support is available
+
+Obsolete properties:
+  - samsung,mfc-r, samsung,mfc-l : support removed, please use memory-region
+	property instead
+
 
 Example:
 SoC specific DT entry:
@@ -43,9 +46,29 @@ mfc: codec@13400000 {
 	clock-names = "mfc";
 };
 
+Reserved memory specific DT entry for given board (see reserved memory binding
+for more information):
+
+reserved-memory {
+	#address-cells = <1>;
+	#size-cells = <1>;
+	ranges;
+
+	mfc_left: region@51000000 {
+		compatible = "shared-dma-pool";
+		no-map;
+		reg = <0x51000000 0x800000>;
+	};
+
+	mfc_right: region@43000000 {
+		compatible = "shared-dma-pool";
+		no-map;
+		reg = <0x43000000 0x800000>;
+	};
+};
+
 Board specific DT entry:
 
 codec@13400000 {
-	samsung,mfc-r = <0x43000000 0x800000>;
-	samsung,mfc-l = <0x51000000 0x800000>;
+	memory-region = <&mfc_left>, <&mfc_right>;
 };
-- 
1.9.2

[PATCH v4 3/7] media: s5p-mfc: replace custom reserved memory handling code with generic one

[Marek Szyprowski]

This patch removes custom code for initialization and handling of
reserved memory regions in s5p-mfc driver and replaces it with generic
reserved memory regions api.

s5p-mfc driver now handles two reserved memory regions defined by
generic reserved memory bindings. Support for non-dt platform has been
removed, because all supported platforms have been already converted to
device tree.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 drivers/media/platform/s5p-mfc/s5p_mfc.c | 138 ++++++++++++++-----------------
 1 file changed, 63 insertions(+), 75 deletions(-)

diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c b/drivers/media/platform/s5p-mfc/s5p_mfc.c
index d1d9d388..fff5f43 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
@@ -22,6 +22,7 @@
 #include <media/v4l2-event.h>
 #include <linux/workqueue.h>
 #include <linux/of.h>
+#include <linux/of_reserved_mem.h>
 #include <media/videobuf2-v4l2.h>
 #include "s5p_mfc_common.h"
 #include "s5p_mfc_ctrl.h"
@@ -1043,66 +1044,71 @@ static const struct v4l2_file_operations s5p_mfc_fops = {
 	.mmap = s5p_mfc_mmap,
 };
 
-static int match_child(struct device *dev, void *data)
-{
-	if (!dev_name(dev))
-		return 0;
-	return !strcmp(dev_name(dev), (char *)data);
-}
-
+/* DMA memory related helper functions */
 static void s5p_mfc_memdev_release(struct device *dev)
 {
-	dma_release_declared_memory(dev);
+	of_reserved_mem_device_release(dev);
 }
 
-static void *mfc_get_drv_data(struct platform_device *pdev);
-
-static int s5p_mfc_alloc_memdevs(struct s5p_mfc_dev *dev)
+static struct device *s5p_mfc_alloc_memdev(struct device *dev,
+					   const char *name, unsigned int idx)
 {
-	unsigned int mem_info[2] = { };
+	struct device *child;
+	int ret;
 
-	dev->mem_dev_l = devm_kzalloc(&dev->plat_dev->dev,
-			sizeof(struct device), GFP_KERNEL);
-	if (!dev->mem_dev_l) {
-		mfc_err("Not enough memory\n");
-		return -ENOMEM;
+	child = devm_kzalloc(dev, sizeof(struct device), GFP_KERNEL);
+	if (!child)
+		return NULL;
+
+	device_initialize(child);
+	dev_set_name(child, "%s:%s", dev_name(dev), name);
+	child->parent = dev;
+	child->bus = dev->bus;
+	child->coherent_dma_mask = dev->coherent_dma_mask;
+	child->dma_mask = dev->dma_mask;
+	child->release = s5p_mfc_memdev_release;
+
+	if (device_add(child) == 0) {
+		ret = of_reserved_mem_device_init_by_idx(child, dev->of_node,
+							 idx);
+		if (ret == 0)
+			return child;
 	}
 
-	dev_set_name(dev->mem_dev_l, "%s", "s5p-mfc-l");
-	dev->mem_dev_l->release = s5p_mfc_memdev_release;
-	device_initialize(dev->mem_dev_l);
-	of_property_read_u32_array(dev->plat_dev->dev.of_node,
-			"samsung,mfc-l", mem_info, 2);
-	if (dma_declare_coherent_memory(dev->mem_dev_l, mem_info[0],
-				mem_info[0], mem_info[1],
-				DMA_MEMORY_MAP | DMA_MEMORY_EXCLUSIVE) == 0) {
-		mfc_err("Failed to declare coherent memory for\n"
-		"MFC device\n");
-		return -ENOMEM;
-	}
+	put_device(child);
+	return NULL;
+}
 
-	dev->mem_dev_r = devm_kzalloc(&dev->plat_dev->dev,
-			sizeof(struct device), GFP_KERNEL);
-	if (!dev->mem_dev_r) {
-		mfc_err("Not enough memory\n");
-		return -ENOMEM;
-	}
+static int s5p_mfc_configure_dma_memory(struct s5p_mfc_dev *mfc_dev)
+{
+	struct device *dev = &mfc_dev->plat_dev->dev;
 
-	dev_set_name(dev->mem_dev_r, "%s", "s5p-mfc-r");
-	dev->mem_dev_r->release = s5p_mfc_memdev_release;
-	device_initialize(dev->mem_dev_r);
-	of_property_read_u32_array(dev->plat_dev->dev.of_node,
-			"samsung,mfc-r", mem_info, 2);
-	if (dma_declare_coherent_memory(dev->mem_dev_r, mem_info[0],
-				mem_info[0], mem_info[1],
-				DMA_MEMORY_MAP | DMA_MEMORY_EXCLUSIVE) == 0) {
-		pr_err("Failed to declare coherent memory for\n"
-		"MFC device\n");
-		return -ENOMEM;
+	/*
+	 * Create and initialize virtual devices for accessing
+	 * reserved memory regions.
+	 */
+	mfc_dev->mem_dev_l = s5p_mfc_alloc_memdev(dev, "left",
+						  MFC_BANK1_ALLOC_CTX);
+	if (!mfc_dev->mem_dev_l)
+		return -ENODEV;
+	mfc_dev->mem_dev_r = s5p_mfc_alloc_memdev(dev, "right",
+						  MFC_BANK2_ALLOC_CTX);
+	if (!mfc_dev->mem_dev_r) {
+		device_unregister(mfc_dev->mem_dev_l);
+		return -ENODEV;
 	}
+
 	return 0;
 }
 
+static void s5p_mfc_unconfigure_dma_memory(struct s5p_mfc_dev *mfc_dev)
+{
+	device_unregister(mfc_dev->mem_dev_l);
+	device_unregister(mfc_dev->mem_dev_r);
+}
+
+static void *mfc_get_drv_data(struct platform_device *pdev);
+
 /* MFC probe function */
 static int s5p_mfc_probe(struct platform_device *pdev)
 {
@@ -1128,12 +1134,6 @@ static int s5p_mfc_probe(struct platform_device *pdev)
 
 	dev->variant = mfc_get_drv_data(pdev);
 
-	ret = s5p_mfc_init_pm(dev);
-	if (ret < 0) {
-		dev_err(&pdev->dev, "failed to get mfc clock source\n");
-		return ret;
-	}
-
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 
 	dev->regs_base = devm_ioremap_resource(&pdev->dev, res);
@@ -1154,25 +1154,16 @@ static int s5p_mfc_probe(struct platform_device *pdev)
 		goto err_res;
 	}
 
-	if (pdev->dev.of_node) {
-		ret = s5p_mfc_alloc_memdevs(dev);
-		if (ret < 0)
-			goto err_res;
-	} else {
-		dev->mem_dev_l = device_find_child(&dev->plat_dev->dev,
-				"s5p-mfc-l", match_child);
-		if (!dev->mem_dev_l) {
-			mfc_err("Mem child (L) device get failed\n");
-			ret = -ENODEV;
-			goto err_res;
-		}
-		dev->mem_dev_r = device_find_child(&dev->plat_dev->dev,
-				"s5p-mfc-r", match_child);
-		if (!dev->mem_dev_r) {
-			mfc_err("Mem child (R) device get failed\n");
-			ret = -ENODEV;
-			goto err_res;
-		}
+	ret = s5p_mfc_configure_dma_memory(dev);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "failed to configure DMA memory\n");
+		return ret;
+	}
+
+	ret = s5p_mfc_init_pm(dev);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "failed to get mfc clock source\n");
+		return ret;
 	}
 
 	vb2_dma_contig_set_max_seg_size(dev->mem_dev_l, DMA_BIT_MASK(32));
@@ -1309,12 +1300,9 @@ static int s5p_mfc_remove(struct platform_device *pdev)
 	s5p_mfc_release_firmware(dev);
 	vb2_dma_contig_cleanup_ctx(dev->alloc_ctx[0]);
 	vb2_dma_contig_cleanup_ctx(dev->alloc_ctx[1]);
+	s5p_mfc_unconfigure_dma_memory(dev);
 	vb2_dma_contig_clear_max_seg_size(dev->mem_dev_l);
 	vb2_dma_contig_clear_max_seg_size(dev->mem_dev_r);
-	if (pdev->dev.of_node) {
-		put_device(dev->mem_dev_l);
-		put_device(dev->mem_dev_r);
-	}
 
 	s5p_mfc_final_pm(dev);
 	return 0;
-- 
1.9.2

[PATCH v4 4/7] media: s5p-mfc: add iommu support

[Marek Szyprowski]

This patch adds support for IOMMU to s5p-mfc device driver. MFC firmware
is limited and it cannot use the default configuration. If IOMMU is
available, the patch disables the default DMA address space
configuration and creates a new address space of size limited to 256M
and base address set to 0x20000000.

For now the same address space is shared by both 'left' and 'right'
memory channels, because the DMA/IOMMU frameworks do not support
configuring them separately. This is not optimal, but besides limiting
total address space available has no other drawbacks (MFC firmware
supports 256M of address space per each channel).

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 drivers/media/platform/s5p-mfc/s5p_mfc.c       | 24 ++++++++
 drivers/media/platform/s5p-mfc/s5p_mfc_iommu.h | 79 ++++++++++++++++++++++++++
 2 files changed, 103 insertions(+)
 create mode 100644 drivers/media/platform/s5p-mfc/s5p_mfc_iommu.h

diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c b/drivers/media/platform/s5p-mfc/s5p_mfc.c
index fff5f43..6ee620e 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
@@ -30,6 +30,7 @@
 #include "s5p_mfc_dec.h"
 #include "s5p_mfc_enc.h"
 #include "s5p_mfc_intr.h"
+#include "s5p_mfc_iommu.h"
 #include "s5p_mfc_opr.h"
 #include "s5p_mfc_cmd.h"
 #include "s5p_mfc_pm.h"
@@ -1084,6 +1085,22 @@ static int s5p_mfc_configure_dma_memory(struct s5p_mfc_dev *mfc_dev)
 	struct device *dev = &mfc_dev->plat_dev->dev;
 
 	/*
+	 * When IOMMU is available, we cannot use the default configuration,
+	 * because of MFC firmware requirements: address space limited to
+	 * 256M and non-zero default start address.
+	 * This is still simplified, not optimal configuration, but for now
+	 * IOMMU core doesn't allow to configure device's IOMMUs channel
+	 * separately.
+	 */
+	if (exynos_is_iommu_available(dev)) {
+		int ret = exynos_configure_iommu(dev, S5P_MFC_IOMMU_DMA_BASE,
+						 S5P_MFC_IOMMU_DMA_SIZE);
+		if (ret == 0)
+			mfc_dev->mem_dev_l = mfc_dev->mem_dev_r = dev;
+		return ret;
+	}
+
+	/*
 	 * Create and initialize virtual devices for accessing
 	 * reserved memory regions.
 	 */
@@ -1103,6 +1120,13 @@ static int s5p_mfc_configure_dma_memory(struct s5p_mfc_dev *mfc_dev)
 
 static void s5p_mfc_unconfigure_dma_memory(struct s5p_mfc_dev *mfc_dev)
 {
+	struct device *dev = &mfc_dev->plat_dev->dev;
+
+	if (exynos_is_iommu_available(dev)) {
+		exynos_unconfigure_iommu(dev);
+		return;
+	}
+
 	device_unregister(mfc_dev->mem_dev_l);
 	device_unregister(mfc_dev->mem_dev_r);
 }
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_iommu.h b/drivers/media/platform/s5p-mfc/s5p_mfc_iommu.h
new file mode 100644
index 0000000..5d1d1c2
--- /dev/null
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_iommu.h
@@ -0,0 +1,79 @@
+/*
+ * Copyright (C) 2015 Samsung Electronics Co.Ltd
+ * Authors: Marek Szyprowski <m.szyprowski@samsung.com>
+ *
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ */
+
+#ifndef S5P_MFC_IOMMU_H_
+#define S5P_MFC_IOMMU_H_
+
+#define S5P_MFC_IOMMU_DMA_BASE	0x20000000lu
+#define S5P_MFC_IOMMU_DMA_SIZE	SZ_256M
+
+#ifdef CONFIG_EXYNOS_IOMMU
+
+#include <asm/dma-iommu.h>
+
+static inline bool exynos_is_iommu_available(struct device *dev)
+{
+	return dev->archdata.iommu != NULL;
+}
+
+static inline void exynos_unconfigure_iommu(struct device *dev)
+{
+	struct dma_iommu_mapping *mapping = to_dma_iommu_mapping(dev);
+
+	arm_iommu_detach_device(dev);
+	arm_iommu_release_mapping(mapping);
+}
+
+static inline int exynos_configure_iommu(struct device *dev,
+					 unsigned int base, unsigned int size)
+{
+	struct dma_iommu_mapping *mapping = NULL;
+	int ret;
+
+	/* Disable the default mapping created by device core */
+	if (to_dma_iommu_mapping(dev))
+		exynos_unconfigure_iommu(dev);
+
+	mapping = arm_iommu_create_mapping(dev->bus, base, size);
+	if (IS_ERR(mapping)) {
+		pr_warn("Failed to create IOMMU mapping for device %s\n",
+			dev_name(dev));
+		return PTR_ERR(mapping);
+	}
+
+	ret = arm_iommu_attach_device(dev, mapping);
+	if (ret) {
+		pr_warn("Failed to attached device %s to IOMMU_mapping\n",
+				dev_name(dev));
+		arm_iommu_release_mapping(mapping);
+		return ret;
+	}
+
+	return 0;
+}
+
+#else
+
+static inline bool exynos_is_iommu_available(struct device *dev)
+{
+	return false;
+}
+
+static inline int exynos_configure_iommu(struct device *dev,
+					 unsigned int base, unsigned int size)
+{
+	return -ENOSYS;
+}
+
+static inline void exynos_unconfigure_iommu(struct device *dev) { }
+
+#endif
+
+#endif /* S5P_MFC_IOMMU_H_ */
-- 
1.9.2

[PATCH v4 5/7] ARM: Exynos: remove code for MFC custom reserved memory handling

[Marek Szyprowski]

Once MFC driver has been converted to generic reserved memory bindings,
there is no need for custom memory reservation code.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 arch/arm/mach-exynos/Makefile      |  2 -
 arch/arm/mach-exynos/exynos.c      | 19 --------
 arch/arm/mach-exynos/mfc.h         | 16 -------
 arch/arm/mach-exynos/s5p-dev-mfc.c | 93 --------------------------------------
 4 files changed, 130 deletions(-)
 delete mode 100644 arch/arm/mach-exynos/mfc.h
 delete mode 100644 arch/arm/mach-exynos/s5p-dev-mfc.c

diff --git a/arch/arm/mach-exynos/Makefile b/arch/arm/mach-exynos/Makefile
index 34d29df..b91b382 100644
--- a/arch/arm/mach-exynos/Makefile
+++ b/arch/arm/mach-exynos/Makefile
@@ -23,5 +23,3 @@ AFLAGS_sleep.o			:=-Wa,-march=armv7-a$(plus_sec)
 
 obj-$(CONFIG_EXYNOS5420_MCPM)	+= mcpm-exynos.o
 CFLAGS_mcpm-exynos.o		+= -march=armv7-a
-
-obj-$(CONFIG_S5P_DEV_MFC)	+= s5p-dev-mfc.o
diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c
index 52ccf24..a8620c6 100644
--- a/arch/arm/mach-exynos/exynos.c
+++ b/arch/arm/mach-exynos/exynos.c
@@ -27,7 +27,6 @@
 #include <mach/map.h>
 
 #include "common.h"
-#include "mfc.h"
 
 static struct map_desc exynos4_iodesc[] __initdata = {
 	{
@@ -237,23 +236,6 @@ static char const *const exynos_dt_compat[] __initconst = {
 	NULL
 };
 
-static void __init exynos_reserve(void)
-{
-#ifdef CONFIG_S5P_DEV_MFC
-	int i;
-	char *mfc_mem[] = {
-		"samsung,mfc-v5",
-		"samsung,mfc-v6",
-		"samsung,mfc-v7",
-		"samsung,mfc-v8",
-	};
-
-	for (i = 0; i < ARRAY_SIZE(mfc_mem); i++)
-		if (of_scan_flat_dt(s5p_fdt_alloc_mfc_mem, mfc_mem[i]))
-			break;
-#endif
-}
-
 static void __init exynos_dt_fixup(void)
 {
 	/*
@@ -275,6 +257,5 @@ DT_MACHINE_START(EXYNOS_DT, "SAMSUNG EXYNOS (Flattened Device Tree)")
 	.init_machine	= exynos_dt_machine_init,
 	.init_late	= exynos_init_late,
 	.dt_compat	= exynos_dt_compat,
-	.reserve	= exynos_reserve,
 	.dt_fixup	= exynos_dt_fixup,
 MACHINE_END
diff --git a/arch/arm/mach-exynos/mfc.h b/arch/arm/mach-exynos/mfc.h
deleted file mode 100644
index dec93cd..0000000
--- a/arch/arm/mach-exynos/mfc.h
+++ /dev/null
@@ -1,16 +0,0 @@
-/*
- * Copyright (C) 2013 Samsung Electronics Co.Ltd
- *
- * This program is free software; you can redistribute  it and/or modify it
- * under  the terms of  the GNU General  Public License as published by the
- * Free Software Foundation;  either version 2 of the  License, or (at your
- * option) any later version.
- */
-
-#ifndef __MACH_EXYNOS_MFC_H
-#define __MACH_EXYNOS_MFC_H __FILE__
-
-int __init s5p_fdt_alloc_mfc_mem(unsigned long node, const char *uname,
-				int depth, void *data);
-
-#endif /* __MACH_EXYNOS_MFC_H */
diff --git a/arch/arm/mach-exynos/s5p-dev-mfc.c b/arch/arm/mach-exynos/s5p-dev-mfc.c
deleted file mode 100644
index 8ef1f3e..0000000
--- a/arch/arm/mach-exynos/s5p-dev-mfc.c
+++ /dev/null
@@ -1,93 +0,0 @@
-/*
- * Copyright (C) 2010-2011 Samsung Electronics Co.Ltd
- *
- * Base S5P MFC resource and device definitions
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- */
-
-#include <linux/kernel.h>
-#include <linux/platform_device.h>
-#include <linux/dma-mapping.h>
-#include <linux/memblock.h>
-#include <linux/ioport.h>
-#include <linux/of_fdt.h>
-#include <linux/of.h>
-
-static struct platform_device s5p_device_mfc_l;
-static struct platform_device s5p_device_mfc_r;
-
-struct s5p_mfc_dt_meminfo {
-	unsigned long	loff;
-	unsigned long	lsize;
-	unsigned long	roff;
-	unsigned long	rsize;
-	char		*compatible;
-};
-
-struct s5p_mfc_reserved_mem {
-	phys_addr_t	base;
-	unsigned long	size;
-	struct device	*dev;
-};
-
-static struct s5p_mfc_reserved_mem s5p_mfc_mem[2] __initdata;
-
-
-static void __init s5p_mfc_reserve_mem(phys_addr_t rbase, unsigned int rsize,
-				phys_addr_t lbase, unsigned int lsize)
-{
-	int i;
-
-	s5p_mfc_mem[0].dev = &s5p_device_mfc_r.dev;
-	s5p_mfc_mem[0].base = rbase;
-	s5p_mfc_mem[0].size = rsize;
-
-	s5p_mfc_mem[1].dev = &s5p_device_mfc_l.dev;
-	s5p_mfc_mem[1].base = lbase;
-	s5p_mfc_mem[1].size = lsize;
-
-	for (i = 0; i < ARRAY_SIZE(s5p_mfc_mem); i++) {
-		struct s5p_mfc_reserved_mem *area = &s5p_mfc_mem[i];
-		if (memblock_remove(area->base, area->size)) {
-			printk(KERN_ERR "Failed to reserve memory for MFC device (%ld bytes at 0x%08lx)\n",
-			       area->size, (unsigned long) area->base);
-			area->base = 0;
-		}
-	}
-}
-
-int __init s5p_fdt_alloc_mfc_mem(unsigned long node, const char *uname,
-				int depth, void *data)
-{
-	const __be32 *prop;
-	int len;
-	struct s5p_mfc_dt_meminfo mfc_mem;
-
-	if (!data)
-		return 0;
-
-	if (!of_flat_dt_is_compatible(node, data))
-		return 0;
-
-	prop = of_get_flat_dt_prop(node, "samsung,mfc-l", &len);
-	if (!prop || (len != 2 * sizeof(unsigned long)))
-		return 0;
-
-	mfc_mem.loff = be32_to_cpu(prop[0]);
-	mfc_mem.lsize = be32_to_cpu(prop[1]);
-
-	prop = of_get_flat_dt_prop(node, "samsung,mfc-r", &len);
-	if (!prop || (len != 2 * sizeof(unsigned long)))
-		return 0;
-
-	mfc_mem.roff = be32_to_cpu(prop[0]);
-	mfc_mem.rsize = be32_to_cpu(prop[1]);
-
-	s5p_mfc_reserve_mem(mfc_mem.roff, mfc_mem.rsize,
-			mfc_mem.loff, mfc_mem.lsize);
-
-	return 1;
-}
-- 
1.9.2

[PATCH v4 6/7] ARM: dts: exynos: convert MFC device to generic reserved memory bindings

[Marek Szyprowski]

This patch replaces custom properties for defining reserved memory
regions with generic reserved memory bindings for MFC video codec
device.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 arch/arm/boot/dts/exynos-mfc-reserved-memory.dtsi  | 27 ++++++++++++++++++++++
 arch/arm/boot/dts/exynos4210-origen.dts            |  4 ++--
 arch/arm/boot/dts/exynos4210-smdkv310.dts          |  4 ++--
 arch/arm/boot/dts/exynos4412-origen.dts            |  4 ++--
 arch/arm/boot/dts/exynos4412-smdk4412.dts          |  4 ++--
 arch/arm/boot/dts/exynos5250-arndale.dts           |  4 ++--
 arch/arm/boot/dts/exynos5250-smdk5250.dts          |  4 ++--
 arch/arm/boot/dts/exynos5250-spring.dts            |  4 ++--
 arch/arm/boot/dts/exynos5420-arndale-octa.dts      |  4 ++--
 arch/arm/boot/dts/exynos5420-peach-pit.dts         |  4 ++--
 arch/arm/boot/dts/exynos5420-smdk5420.dts          |  4 ++--
 arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi |  4 ++--
 arch/arm/boot/dts/exynos5800-peach-pi.dts          |  4 ++--
 13 files changed, 51 insertions(+), 24 deletions(-)
 create mode 100644 arch/arm/boot/dts/exynos-mfc-reserved-memory.dtsi

diff --git a/arch/arm/boot/dts/exynos-mfc-reserved-memory.dtsi b/arch/arm/boot/dts/exynos-mfc-reserved-memory.dtsi
new file mode 100644
index 0000000..e7445c9
--- /dev/null
+++ b/arch/arm/boot/dts/exynos-mfc-reserved-memory.dtsi
@@ -0,0 +1,27 @@
+/*
+ * Samsung's Exynos SoC MFC (Video Codec) reserved memory common definition.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+/ {
+	reserved-memory {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges;
+
+		mfc_left: region@51000000 {
+			compatible = "shared-dma-pool";
+			no-map;
+			reg = <0x51000000 0x800000>;
+		};
+
+		mfc_right: region@43000000 {
+			compatible = "shared-dma-pool";
+			no-map;
+			reg = <0x43000000 0x800000>;
+		};
+	};
+};
diff --git a/arch/arm/boot/dts/exynos4210-origen.dts b/arch/arm/boot/dts/exynos4210-origen.dts
index ad7394c..f5e4eb2 100644
--- a/arch/arm/boot/dts/exynos4210-origen.dts
+++ b/arch/arm/boot/dts/exynos4210-origen.dts
@@ -18,6 +18,7 @@
 #include "exynos4210.dtsi"
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/input/input.h>
+#include "exynos-mfc-reserved-memory.dtsi"
 
 / {
 	model = "Insignal Origen evaluation board based on Exynos4210";
@@ -288,8 +289,7 @@
 };
 
 &mfc {
-	samsung,mfc-r = <0x43000000 0x800000>;
-	samsung,mfc-l = <0x51000000 0x800000>;
+	memory-region = <&mfc_left>, <&mfc_right>;
 	status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/exynos4210-smdkv310.dts b/arch/arm/boot/dts/exynos4210-smdkv310.dts
index 94ca7d3..de917f0 100644
--- a/arch/arm/boot/dts/exynos4210-smdkv310.dts
+++ b/arch/arm/boot/dts/exynos4210-smdkv310.dts
@@ -17,6 +17,7 @@
 /dts-v1/;
 #include "exynos4210.dtsi"
 #include <dt-bindings/gpio/gpio.h>
+#include "exynos-mfc-reserved-memory.dtsi"
 
 / {
 	model = "Samsung smdkv310 evaluation board based on Exynos4210";
@@ -133,8 +134,7 @@
 };
 
 &mfc {
-	samsung,mfc-r = <0x43000000 0x800000>;
-	samsung,mfc-l = <0x51000000 0x800000>;
+	memory-region = <&mfc_left>, <&mfc_right>;
 	status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/exynos4412-origen.dts b/arch/arm/boot/dts/exynos4412-origen.dts
index 8bca699..cd363d7 100644
--- a/arch/arm/boot/dts/exynos4412-origen.dts
+++ b/arch/arm/boot/dts/exynos4412-origen.dts
@@ -16,6 +16,7 @@
 #include "exynos4412.dtsi"
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/input/input.h>
+#include "exynos-mfc-reserved-memory.dtsi"
 
 / {
 	model = "Insignal Origen evaluation board based on Exynos4412";
@@ -466,8 +467,7 @@
 };
 
 &mfc {
-	samsung,mfc-r = <0x43000000 0x800000>;
-	samsung,mfc-l = <0x51000000 0x800000>;
+	memory-region = <&mfc_left>, <&mfc_right>;
 	status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/exynos4412-smdk4412.dts b/arch/arm/boot/dts/exynos4412-smdk4412.dts
index a51069f..9b6d561 100644
--- a/arch/arm/boot/dts/exynos4412-smdk4412.dts
+++ b/arch/arm/boot/dts/exynos4412-smdk4412.dts
@@ -14,6 +14,7 @@
 
 /dts-v1/;
 #include "exynos4412.dtsi"
+#include "exynos-mfc-reserved-memory.dtsi"
 
 / {
 	model = "Samsung SMDK evaluation board based on Exynos4412";
@@ -112,8 +113,7 @@
 };
 
 &mfc {
-	samsung,mfc-r = <0x43000000 0x800000>;
-	samsung,mfc-l = <0x51000000 0x800000>;
+	memory-region = <&mfc_left>, <&mfc_right>;
 	status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/exynos5250-arndale.dts b/arch/arm/boot/dts/exynos5250-arndale.dts
index 85dad29..39940f4 100644
--- a/arch/arm/boot/dts/exynos5250-arndale.dts
+++ b/arch/arm/boot/dts/exynos5250-arndale.dts
@@ -14,6 +14,7 @@
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/input/input.h>
 #include "exynos5250.dtsi"
+#include "exynos-mfc-reserved-memory.dtsi"
 
 / {
 	model = "Insignal Arndale evaluation board based on EXYNOS5250";
@@ -516,8 +517,7 @@
 };
 
 &mfc {
-	samsung,mfc-r = <0x43000000 0x800000>;
-	samsung,mfc-l = <0x51000000 0x800000>;
+	memory-region = <&mfc_left>, <&mfc_right>;
 };
 
 &mmc_0 {
diff --git a/arch/arm/boot/dts/exynos5250-smdk5250.dts b/arch/arm/boot/dts/exynos5250-smdk5250.dts
index b7992b1..9fac874 100644
--- a/arch/arm/boot/dts/exynos5250-smdk5250.dts
+++ b/arch/arm/boot/dts/exynos5250-smdk5250.dts
@@ -13,6 +13,7 @@
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/interrupt-controller/irq.h>
 #include "exynos5250.dtsi"
+#include "exynos-mfc-reserved-memory.dtsi"
 
 / {
 	model = "SAMSUNG SMDK5250 board based on EXYNOS5250";
@@ -344,8 +345,7 @@
 };
 
 &mfc {
-	samsung,mfc-r = <0x43000000 0x800000>;
-	samsung,mfc-l = <0x51000000 0x800000>;
+	memory-region = <&mfc_left>, <&mfc_right>;
 };
 
 &mmc_0 {
diff --git a/arch/arm/boot/dts/exynos5250-spring.dts b/arch/arm/boot/dts/exynos5250-spring.dts
index ac291f5..784130b 100644
--- a/arch/arm/boot/dts/exynos5250-spring.dts
+++ b/arch/arm/boot/dts/exynos5250-spring.dts
@@ -14,6 +14,7 @@
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/input/input.h>
 #include "exynos5250.dtsi"
+#include "exynos-mfc-reserved-memory.dtsi"
 
 / {
 	model = "Google Spring";
@@ -425,8 +426,7 @@
 };
 
 &mfc {
-	samsung,mfc-r = <0x43000000 0x800000>;
-	samsung,mfc-l = <0x51000000 0x800000>;
+	memory-region = <&mfc_left>, <&mfc_right>;
 };
 
 &mmc_0 {
diff --git a/arch/arm/boot/dts/exynos5420-arndale-octa.dts b/arch/arm/boot/dts/exynos5420-arndale-octa.dts
index 60bc861..b8b5f3a 100644
--- a/arch/arm/boot/dts/exynos5420-arndale-octa.dts
+++ b/arch/arm/boot/dts/exynos5420-arndale-octa.dts
@@ -16,6 +16,7 @@
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/input/input.h>
 #include <dt-bindings/clock/samsung,s2mps11.h>
+#include "exynos-mfc-reserved-memory.dtsi"
 
 / {
 	model = "Insignal Arndale Octa evaluation board based on EXYNOS5420";
@@ -347,8 +348,7 @@
 };
 
 &mfc {
-	samsung,mfc-r = <0x43000000 0x800000>;
-	samsung,mfc-l = <0x51000000 0x800000>;
+	memory-region = <&mfc_left>, <&mfc_right>;
 };
 
 &mmc_0 {
diff --git a/arch/arm/boot/dts/exynos5420-peach-pit.dts b/arch/arm/boot/dts/exynos5420-peach-pit.dts
index f9d2e4f..d530b4f 100644
--- a/arch/arm/boot/dts/exynos5420-peach-pit.dts
+++ b/arch/arm/boot/dts/exynos5420-peach-pit.dts
@@ -16,6 +16,7 @@
 #include <dt-bindings/regulator/maxim,max77802.h>
 #include "exynos5420.dtsi"
 #include "exynos5420-cpus.dtsi"
+#include "exynos-mfc-reserved-memory.dtsi"
 
 / {
 	model = "Google Peach Pit Rev 6+";
@@ -695,8 +696,7 @@
 };
 
 &mfc {
-	samsung,mfc-r = <0x43000000 0x800000>;
-	samsung,mfc-l = <0x51000000 0x800000>;
+	memory-region = <&mfc_left>, <&mfc_right>;
 };
 
 &mmc_0 {
diff --git a/arch/arm/boot/dts/exynos5420-smdk5420.dts b/arch/arm/boot/dts/exynos5420-smdk5420.dts
index 2e748d1..5206f41 100644
--- a/arch/arm/boot/dts/exynos5420-smdk5420.dts
+++ b/arch/arm/boot/dts/exynos5420-smdk5420.dts
@@ -13,6 +13,7 @@
 #include "exynos5420.dtsi"
 #include "exynos5420-cpus.dtsi"
 #include <dt-bindings/gpio/gpio.h>
+#include "exynos-mfc-reserved-memory.dtsi"
 
 / {
 	model = "Samsung SMDK5420 board based on EXYNOS5420";
@@ -355,8 +356,7 @@
 };
 
 &mfc {
-	samsung,mfc-r = <0x43000000 0x800000>;
-	samsung,mfc-l = <0x51000000 0x800000>;
+	memory-region = <&mfc_left>, <&mfc_right>;
 };
 
 &mmc_0 {
diff --git a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
index 2a4e10b..7c2335f 100644
--- a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
+++ b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
@@ -17,6 +17,7 @@
 #include "exynos5800.dtsi"
 #include "exynos5422-cpus.dtsi"
 #include "exynos5422-cpu-thermal.dtsi"
+#include "exynos-mfc-reserved-memory.dtsi"
 
 / {
 	memory {
@@ -406,8 +407,7 @@
 };
 
 &mfc {
-	samsung,mfc-r = <0x43000000 0x800000>;
-	samsung,mfc-l = <0x51000000 0x800000>;
+	memory-region = <&mfc_left>, <&mfc_right>;
 };
 
 &mmc_0 {
diff --git a/arch/arm/boot/dts/exynos5800-peach-pi.dts b/arch/arm/boot/dts/exynos5800-peach-pi.dts
index 62ceb89..1f73596 100644
--- a/arch/arm/boot/dts/exynos5800-peach-pi.dts
+++ b/arch/arm/boot/dts/exynos5800-peach-pi.dts
@@ -16,6 +16,7 @@
 #include <dt-bindings/regulator/maxim,max77802.h>
 #include "exynos5800.dtsi"
 #include "exynos5420-cpus.dtsi"
+#include "exynos-mfc-reserved-memory.dtsi"
 
 / {
 	model = "Google Peach Pi Rev 10+";
@@ -670,8 +671,7 @@
 };
 
 &mfc {
-	samsung,mfc-r = <0x43000000 0x800000>;
-	samsung,mfc-l = <0x51000000 0x800000>;
+	memory-region = <&mfc_left>, <&mfc_right>;
 };
 
 &mmc_0 {
-- 
1.9.2

[PATCH v4 7/7] ARM: dts: exynos4412-odroid*: enable MFC device

[Marek Szyprowski]

Enable support for Multimedia Codec (MFC) device for all Exynos4412-based
Odroid boards.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 arch/arm/boot/dts/exynos4412-odroid-common.dtsi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
index ec7619a..276ac9a 100644
--- a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
+++ b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
@@ -13,6 +13,7 @@
 #include "exynos4412.dtsi"
 #include "exynos4412-ppmu-common.dtsi"
 #include <dt-bindings/gpio/gpio.h>
+#include "exynos-mfc-reserved-memory.dtsi"
 
 / {
 	chosen {
@@ -499,6 +500,11 @@
 	clock-names = "iis", "i2s_opclk0", "i2s_opclk1";
 };
 
+&mfc {
+	memory-region = <&mfc_left>, <&mfc_right>;
+	status = "okay";
+};
+
 &mixer {
 	status = "okay";
 };
-- 
1.9.2

Re: [PATCH v4 6/7] ARM: dts: exynos: convert MFC device to generic reserved memory bindings

[Krzysztof Kozlowski]

On 05/24/2016 03:31 PM, Marek Szyprowski wrote:
> This patch replaces custom properties for defining reserved memory
> regions with generic reserved memory bindings for MFC video codec
> device.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  arch/arm/boot/dts/exynos-mfc-reserved-memory.dtsi  | 27 ++++++++++++++++++++++
>  arch/arm/boot/dts/exynos4210-origen.dts            |  4 ++--
>  arch/arm/boot/dts/exynos4210-smdkv310.dts          |  4 ++--
>  arch/arm/boot/dts/exynos4412-origen.dts            |  4 ++--
>  arch/arm/boot/dts/exynos4412-smdk4412.dts          |  4 ++--
>  arch/arm/boot/dts/exynos5250-arndale.dts           |  4 ++--
>  arch/arm/boot/dts/exynos5250-smdk5250.dts          |  4 ++--
>  arch/arm/boot/dts/exynos5250-spring.dts            |  4 ++--
>  arch/arm/boot/dts/exynos5420-arndale-octa.dts      |  4 ++--
>  arch/arm/boot/dts/exynos5420-peach-pit.dts         |  4 ++--
>  arch/arm/boot/dts/exynos5420-smdk5420.dts          |  4 ++--
>  arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi |  4 ++--
>  arch/arm/boot/dts/exynos5800-peach-pi.dts          |  4 ++--
>  13 files changed, 51 insertions(+), 24 deletions(-)
>  create mode 100644 arch/arm/boot/dts/exynos-mfc-reserved-memory.dtsi
> 
> diff --git a/arch/arm/boot/dts/exynos-mfc-reserved-memory.dtsi b/arch/arm/boot/dts/exynos-mfc-reserved-memory.dtsi
> new file mode 100644
> index 0000000..e7445c9
> --- /dev/null
> +++ b/arch/arm/boot/dts/exynos-mfc-reserved-memory.dtsi
> @@ -0,0 +1,27 @@
> +/*
> + * Samsung's Exynos SoC MFC (Video Codec) reserved memory common definition.

Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>

One nit - our copyright would be welcomed:
	Copyright (c) 2016 Samsung Electronics Co., Ltd

However if there are no objections I will add it when applying.

Best regards,
Krzysztof

Re: [PATCH v4 2/7] media: s5p-mfc: use generic reserved memory bindings

[Javier Martinez Canillas]

Hello Marek,

On 05/24/2016 09:31 AM, Marek Szyprowski wrote:
> Use generic reserved memory bindings and mark old, custom properties
> as obsoleted.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  .../devicetree/bindings/media/s5p-mfc.txt          | 39 +++++++++++++++++-----
>  1 file changed, 31 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/s5p-mfc.txt b/Documentation/devicetree/bindings/media/s5p-mfc.txt
> index 2d5787e..92c94f5 100644
> --- a/Documentation/devicetree/bindings/media/s5p-mfc.txt
> +++ b/Documentation/devicetree/bindings/media/s5p-mfc.txt
> @@ -21,15 +21,18 @@ Required properties:
>    - clock-names : from common clock binding: must contain "mfc",
>  		  corresponding to entry in the clocks property.
>  
> -  - samsung,mfc-r : Base address of the first memory bank used by MFC
> -		    for DMA contiguous memory allocation and its size.
> -
> -  - samsung,mfc-l : Base address of the second memory bank used by MFC
> -		    for DMA contiguous memory allocation and its size.
> -
>  Optional properties:
>    - power-domains : power-domain property defined with a phandle
>  			   to respective power domain.
> +  - memory-region : from reserved memory binding: phandles to two reserved
> +	memory regions, first is for "left" mfc memory bus interfaces,
> +	second if for the "right" mfc memory bus, used when no SYSMMU
> +	support is available
> +
> +Obsolete properties:
> +  - samsung,mfc-r, samsung,mfc-l : support removed, please use memory-region
> +	property instead
> +
> 

I wonder if we should maintain backward compatibility for this driver
since s5p-mfc memory allocation won't work with an old FDT if support
for the old properties are removed.

Although I'm not a big fan of keeping backward compatibility just to
add dead code that will never be used and I don't know of any Exynos
machine where the DTB and kernel are not updated together so I agree
with this patch:

Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH v4 3/7] media: s5p-mfc: replace custom reserved memory handling code with generic one

[Javier Martinez Canillas]

Hello Marek,

On 05/24/2016 09:31 AM, Marek Szyprowski wrote:
> This patch removes custom code for initialization and handling of
> reserved memory regions in s5p-mfc driver and replaces it with generic
> reserved memory regions api.
> 
> s5p-mfc driver now handles two reserved memory regions defined by
> generic reserved memory bindings. Support for non-dt platform has been
> removed, because all supported platforms have been already converted to
> device tree.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---

Similar question than for patch #2, shouldn't we include the in-kernel DTS
changes with this patch to keep bisectability? Otherwise the mfc-{l,r} mem
allocation will fail after this patch.

The patch looks good to me though:

Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>

I couldn't test this exact patch because my Odroid XU4 died but I've tested
the previous version and the only difference is that the memory reserve is
made by index now instead of name, so I think you can add:

Tested-by: Javier Martinez Canillas <javier@osg.samsung.com>

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH v4 4/7] media: s5p-mfc: add iommu support

[Javier Martinez Canillas]

Hello Marek,

On 05/24/2016 09:31 AM, Marek Szyprowski wrote:
> This patch adds support for IOMMU to s5p-mfc device driver. MFC firmware
> is limited and it cannot use the default configuration. If IOMMU is
> available, the patch disables the default DMA address space
> configuration and creates a new address space of size limited to 256M
> and base address set to 0x20000000.
> 
> For now the same address space is shared by both 'left' and 'right'
> memory channels, because the DMA/IOMMU frameworks do not support
> configuring them separately. This is not optimal, but besides limiting
> total address space available has no other drawbacks (MFC firmware
> supports 256M of address space per each channel).
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---

Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>
Tested-by: Javier Martinez Canillas <javier@osg.samsung.com>

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH v4 5/7] ARM: Exynos: remove code for MFC custom reserved memory handling

[Javier Martinez Canillas]

Hello Marek,

On 05/24/2016 09:31 AM, Marek Szyprowski wrote:
> Once MFC driver has been converted to generic reserved memory bindings,
> there is no need for custom memory reservation code.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---

Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH v4 3/7] media: s5p-mfc: replace custom reserved memory handling code with generic one

[Javier Martinez Canillas]

Hello Marek,

On 05/25/2016 11:42 AM, Javier Martinez Canillas wrote:

[snip]

> 
> I couldn't test this exact patch because my Odroid XU4 died but I've tested
> the previous version and the only difference is that the memory reserve is
> made by index now instead of name, so I think you can add:
> 
> Tested-by: Javier Martinez Canillas <javier@osg.samsung.com>
> 

And now I also could test v4 of this patch on an Exynos5800 Peach Chromebook.

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH v4 6/7] ARM: dts: exynos: convert MFC device to generic reserved memory bindings

[Javier Martinez Canillas]

Hello Marek,

On 05/24/2016 09:31 AM, Marek Szyprowski wrote:
> This patch replaces custom properties for defining reserved memory
> regions with generic reserved memory bindings for MFC video codec
> device.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---

[snip]

> +
> +/ {
> +	reserved-memory {
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		ranges;
> +
> +		mfc_left: region@51000000 {
> +			compatible = "shared-dma-pool";
> +			no-map;
> +			reg = <0x51000000 0x800000>;
> +		};
> +
> +		mfc_right: region@43000000 {
> +			compatible = "shared-dma-pool";
> +			no-map;
> +			reg = <0x43000000 0x800000>;
> +		};
> +	};

I've a question probably for a follow up patch, but do you know what's a
sane default size for these? I needed to bump the mfc_left size from 8 MiB
to 16 MiB in order to decode a 480p H264 video using GStramer. So clearly
the default sizes are not that useful.

> +};
> diff --git a/arch/arm/boot/dts/exynos4210-origen.dts b/arch/arm/boot/dts/exynos4210-origen.dts
> index ad7394c..f5e4eb2 100644
> --- a/arch/arm/boot/dts/exynos4210-origen.dts
> +++ b/arch/arm/boot/dts/exynos4210-origen.dts
> @@ -18,6 +18,7 @@
>  #include "exynos4210.dtsi"
>  #include <dt-bindings/gpio/gpio.h>
>  #include <dt-bindings/input/input.h>
> +#include "exynos-mfc-reserved-memory.dtsi"
>  
>  / {
>  	model = "Insignal Origen evaluation board based on Exynos4210";
> @@ -288,8 +289,7 @@
>  };
>  
>  &mfc {
> -	samsung,mfc-r = <0x43000000 0x800000>;
> -	samsung,mfc-l = <0x51000000 0x800000>;
> +	memory-region = <&mfc_left>, <&mfc_right>;
>  	status = "okay";

I wonder if shouldn't be better to include the exynos-mfc-reserved-memory.dtsi
on each SoC dtsi and set the memory-regions in the MFC node instead of doing
it on each DTS, and let DTS to just replace with its own memory regions if the
default sizes are not suitable for them.

Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>
Tested-by: Javier Martinez Canillas <javier@osg.samsung.com>

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH v4 7/7] ARM: dts: exynos4412-odroid*: enable MFC device

[Javier Martinez Canillas]

Hello Marek,

On 05/24/2016 09:31 AM, Marek Szyprowski wrote:
> Enable support for Multimedia Codec (MFC) device for all Exynos4412-based
> Odroid boards.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---

Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH v4 2/7] media: s5p-mfc: use generic reserved memory bindings

[Rob Herring]

On Wed, May 25, 2016 at 11:18:59AM -0400, Javier Martinez Canillas wrote:
> Hello Marek,
> 
> On 05/24/2016 09:31 AM, Marek Szyprowski wrote:
> > Use generic reserved memory bindings and mark old, custom properties
> > as obsoleted.
> > 
> > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> > ---
> >  .../devicetree/bindings/media/s5p-mfc.txt          | 39 +++++++++++++++++-----
> >  1 file changed, 31 insertions(+), 8 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/media/s5p-mfc.txt b/Documentation/devicetree/bindings/media/s5p-mfc.txt
> > index 2d5787e..92c94f5 100644
> > --- a/Documentation/devicetree/bindings/media/s5p-mfc.txt
> > +++ b/Documentation/devicetree/bindings/media/s5p-mfc.txt
> > @@ -21,15 +21,18 @@ Required properties:
> >    - clock-names : from common clock binding: must contain "mfc",
> >  		  corresponding to entry in the clocks property.
> >  
> > -  - samsung,mfc-r : Base address of the first memory bank used by MFC
> > -		    for DMA contiguous memory allocation and its size.
> > -
> > -  - samsung,mfc-l : Base address of the second memory bank used by MFC
> > -		    for DMA contiguous memory allocation and its size.
> > -
> >  Optional properties:
> >    - power-domains : power-domain property defined with a phandle
> >  			   to respective power domain.
> > +  - memory-region : from reserved memory binding: phandles to two reserved
> > +	memory regions, first is for "left" mfc memory bus interfaces,
> > +	second if for the "right" mfc memory bus, used when no SYSMMU
> > +	support is available
> > +
> > +Obsolete properties:
> > +  - samsung,mfc-r, samsung,mfc-l : support removed, please use memory-region
> > +	property instead
> > +
> > 
> 
> I wonder if we should maintain backward compatibility for this driver
> since s5p-mfc memory allocation won't work with an old FDT if support
> for the old properties are removed.

Well, minimally the commit log should indicate that compatibility is 
being broken.

Rob

Re: [PATCH v4 2/7] media: s5p-mfc: use generic reserved memory bindings

[Rob Herring]

On Tue, May 24, 2016 at 03:31:25PM +0200, Marek Szyprowski wrote:
> Use generic reserved memory bindings and mark old, custom properties
> as obsoleted.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  .../devicetree/bindings/media/s5p-mfc.txt          | 39 +++++++++++++++++-----
>  1 file changed, 31 insertions(+), 8 deletions(-)

Acked-by: Rob Herring <robh@kernel.org>

Re: [PATCH v4 1/7] of: reserved_mem: add support for using more than one region for given device

[Rob Herring]

On Tue, May 24, 2016 at 8:31 AM, Marek Szyprowski
<m.szyprowski@samsung.com> wrote:
> This patch allows device drivers to initialize more than one reserved
> memory region assigned to given device. When driver needs to use more
> than one reserved memory region, it should allocate child devices and
> initialize regions by index for each of its child devices.
>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  drivers/of/of_reserved_mem.c    | 85 +++++++++++++++++++++++++++++++----------
>  include/linux/of_reserved_mem.h | 25 ++++++++++--
>  2 files changed, 86 insertions(+), 24 deletions(-)

Acked-by: Rob Herring <robh@kernel.org>

Re: [PATCH v4 2/7] media: s5p-mfc: use generic reserved memory bindings

[Krzysztof Kozlowski]

On 05/24/2016 03:31 PM, Marek Szyprowski wrote:
> Use generic reserved memory bindings and mark old, custom properties
> as obsoleted.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  .../devicetree/bindings/media/s5p-mfc.txt          | 39 +++++++++++++++++-----
>  1 file changed, 31 insertions(+), 8 deletions(-)
>

Acked-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>


Best regards,
Krzysztof

Re: [PATCH v4 2/7] media: s5p-mfc: use generic reserved memory bindings

[Marek Szyprowski]

Hello,


On 2016-05-25 19:36, Rob Herring wrote:
> On Wed, May 25, 2016 at 11:18:59AM -0400, Javier Martinez Canillas wrote:
>> Hello Marek,
>>
>> On 05/24/2016 09:31 AM, Marek Szyprowski wrote:
>>> Use generic reserved memory bindings and mark old, custom properties
>>> as obsoleted.
>>>
>>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>> ---
>>>   .../devicetree/bindings/media/s5p-mfc.txt          | 39 +++++++++++++++++-----
>>>   1 file changed, 31 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/media/s5p-mfc.txt b/Documentation/devicetree/bindings/media/s5p-mfc.txt
>>> index 2d5787e..92c94f5 100644
>>> --- a/Documentation/devicetree/bindings/media/s5p-mfc.txt
>>> +++ b/Documentation/devicetree/bindings/media/s5p-mfc.txt
>>> @@ -21,15 +21,18 @@ Required properties:
>>>     - clock-names : from common clock binding: must contain "mfc",
>>>   		  corresponding to entry in the clocks property.
>>>   
>>> -  - samsung,mfc-r : Base address of the first memory bank used by MFC
>>> -		    for DMA contiguous memory allocation and its size.
>>> -
>>> -  - samsung,mfc-l : Base address of the second memory bank used by MFC
>>> -		    for DMA contiguous memory allocation and its size.
>>> -
>>>   Optional properties:
>>>     - power-domains : power-domain property defined with a phandle
>>>   			   to respective power domain.
>>> +  - memory-region : from reserved memory binding: phandles to two reserved
>>> +	memory regions, first is for "left" mfc memory bus interfaces,
>>> +	second if for the "right" mfc memory bus, used when no SYSMMU
>>> +	support is available
>>> +
>>> +Obsolete properties:
>>> +  - samsung,mfc-r, samsung,mfc-l : support removed, please use memory-region
>>> +	property instead
>>> +
>>>
>> I wonder if we should maintain backward compatibility for this driver
>> since s5p-mfc memory allocation won't work with an old FDT if support
>> for the old properties are removed.
> Well, minimally the commit log should indicate that compatibility is
> being broken.

Compatibility is only partially broken. I add this to the commit 
message. Old
bindings will still work with the new driver when IOMMU is enabled - in 
such case reserved
memory regions are ignored so this should not be a big issue. Using 
IOMMU also increases
total memory space for the video buffers without wasting it as 
'reserved'. Hope that
once those patches are merged, the IOMMU can be finally enabled in the 
exynos_defconfig.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH v4 6/7] ARM: dts: exynos: convert MFC device to generic reserved memory bindings

[Marek Szyprowski]

Hello,


On 2016-05-25 19:11, Javier Martinez Canillas wrote:
> Hello Marek,
>
> On 05/24/2016 09:31 AM, Marek Szyprowski wrote:
>> This patch replaces custom properties for defining reserved memory
>> regions with generic reserved memory bindings for MFC video codec
>> device.
>>
>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> ---
> [snip]
>
>> +
>> +/ {
>> +	reserved-memory {
>> +		#address-cells = <1>;
>> +		#size-cells = <1>;
>> +		ranges;
>> +
>> +		mfc_left: region@51000000 {
>> +			compatible = "shared-dma-pool";
>> +			no-map;
>> +			reg = <0x51000000 0x800000>;
>> +		};
>> +
>> +		mfc_right: region@43000000 {
>> +			compatible = "shared-dma-pool";
>> +			no-map;
>> +			reg = <0x43000000 0x800000>;
>> +		};
>> +	};
> I've a question probably for a follow up patch, but do you know what's a
> sane default size for these? I needed to bump the mfc_left size from 8 MiB
> to 16 MiB in order to decode a 480p H264 video using GStramer. So clearly
> the default sizes are not that useful.

Right, the default size for the 'left' region can be increased. Frankly, 
those
values (8MiB/0x43000000+ 8MiB/0x51000000) comes from my initial patches
prepared for some demo and don't have much with any real requirements. They
were copied (blindly...) by various developers without any deeper 
understanding.
Probably the most sane would be to use something like this:

mfc_left: region_mfc_left {
          compatible = "shared-dma-pool";
          no-map;
          size = <0x1000000>;
          alignment = <0x100000>;
};

mfc_right: region_mfc_right {
          compatible = "shared-dma-pool";
          no-map;
          size = <0x800000>;
          alignment = <0x100000>;
};

So the region will be allocated automatically from the available memory. 
This way
another nice feature of the generic reserved memory regions can be used.

The only platform which really requires MFC regions to be placed at 
certain memory
offsets is Samsung S5PV210/S5PC110 (sometimes called exynos3), where 
there is no
memory address interleaving and MFC device has limited memory interface, 
which cannot
do 2 transactions to the same physical memory bank. However 
S5PV210/S5PC110 machine
code lost support for MFC during conversion to device tree, so it is not 
a problem
here.

All newer platforms (Exynos4, Exynos3250, Exynos5+) use memory 
interleaving, so the
actual offset of memory bank has no influence on the physical memory bank.

>> +};
>> diff --git a/arch/arm/boot/dts/exynos4210-origen.dts b/arch/arm/boot/dts/exynos4210-origen.dts
>> index ad7394c..f5e4eb2 100644
>> --- a/arch/arm/boot/dts/exynos4210-origen.dts
>> +++ b/arch/arm/boot/dts/exynos4210-origen.dts
>> @@ -18,6 +18,7 @@
>>   #include "exynos4210.dtsi"
>>   #include <dt-bindings/gpio/gpio.h>
>>   #include <dt-bindings/input/input.h>
>> +#include "exynos-mfc-reserved-memory.dtsi"
>>   
>>   / {
>>   	model = "Insignal Origen evaluation board based on Exynos4210";
>> @@ -288,8 +289,7 @@
>>   };
>>   
>>   &mfc {
>> -	samsung,mfc-r = <0x43000000 0x800000>;
>> -	samsung,mfc-l = <0x51000000 0x800000>;
>> +	memory-region = <&mfc_left>, <&mfc_right>;
>>   	status = "okay";
> I wonder if shouldn't be better to include the exynos-mfc-reserved-memory.dtsi
> on each SoC dtsi and set the memory-regions in the MFC node instead of doing
> it on each DTS, and let DTS to just replace with its own memory regions if the
> default sizes are not suitable for them.

I don't have strong opinion on this. Maybe it would make more sense to 
move the
following entry:

&mfc {
         memory-region = <&mfc_left>, <&mfc_right>;
};

also to the exynos-mfc-reserved-memory.dtsi ? So board will just include 
it if
it want to use MFC device with reserved memory regions.

> Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>
> Tested-by: Javier Martinez Canillas <javier@osg.samsung.com>

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH v4 2/7] media: s5p-mfc: use generic reserved memory bindings

[Javier Martinez Canillas]

Hello Marek,

On 05/27/2016 02:37 AM, Marek Szyprowski wrote:
> Hello,
> 
> 
> On 2016-05-25 19:36, Rob Herring wrote:
>> On Wed, May 25, 2016 at 11:18:59AM -0400, Javier Martinez Canillas wrote:
>>> Hello Marek,
>>>
>>> On 05/24/2016 09:31 AM, Marek Szyprowski wrote:
>>>> Use generic reserved memory bindings and mark old, custom properties
>>>> as obsoleted.
>>>>
>>>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>>> ---
>>>>   .../devicetree/bindings/media/s5p-mfc.txt          | 39 +++++++++++++++++-----
>>>>   1 file changed, 31 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/media/s5p-mfc.txt b/Documentation/devicetree/bindings/media/s5p-mfc.txt
>>>> index 2d5787e..92c94f5 100644
>>>> --- a/Documentation/devicetree/bindings/media/s5p-mfc.txt
>>>> +++ b/Documentation/devicetree/bindings/media/s5p-mfc.txt
>>>> @@ -21,15 +21,18 @@ Required properties:
>>>>     - clock-names : from common clock binding: must contain "mfc",
>>>>             corresponding to entry in the clocks property.
>>>>   -  - samsung,mfc-r : Base address of the first memory bank used by MFC
>>>> -            for DMA contiguous memory allocation and its size.
>>>> -
>>>> -  - samsung,mfc-l : Base address of the second memory bank used by MFC
>>>> -            for DMA contiguous memory allocation and its size.
>>>> -
>>>>   Optional properties:
>>>>     - power-domains : power-domain property defined with a phandle
>>>>                  to respective power domain.
>>>> +  - memory-region : from reserved memory binding: phandles to two reserved
>>>> +    memory regions, first is for "left" mfc memory bus interfaces,
>>>> +    second if for the "right" mfc memory bus, used when no SYSMMU
>>>> +    support is available
>>>> +
>>>> +Obsolete properties:
>>>> +  - samsung,mfc-r, samsung,mfc-l : support removed, please use memory-region
>>>> +    property instead
>>>> +
>>>>
>>> I wonder if we should maintain backward compatibility for this driver
>>> since s5p-mfc memory allocation won't work with an old FDT if support
>>> for the old properties are removed.
>> Well, minimally the commit log should indicate that compatibility is
>> being broken.
> 
> Compatibility is only partially broken. I add this to the commit message. Old
> bindings will still work with the new driver when IOMMU is enabled - in such case reserved
> memory regions are ignored so this should not be a big issue. Using IOMMU also increases
> total memory space for the video buffers without wasting it as 'reserved'. Hope that
> once those patches are merged, the IOMMU can be finally enabled in the exynos_defconfig.
>

Yes, a problem is that some Exynos machines (for example the Snow and Peach
Chromebooks) fail to boot when IOMMU is enabled due the bootloader leaving
the FIMD enabled doing DMA operations automatically as you found before.

You proposed to add a "iommu-reserved-mapping" property [0] but that never
landed due not having an agreement on how should be fixed properly IIUC [1].

So that has to be fixed before enabling the Exynos IOMMU support by default.

[0]: http://www.spinics.net/lists/arm-kernel/msg415501.html
[1]: http://www.spinics.net/lists/arm-kernel/msg419747.html 

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH v4 6/7] ARM: dts: exynos: convert MFC device to generic reserved memory bindings

[Javier Martinez Canillas]

Hello Marek,

On 05/27/2016 07:32 AM, Marek Szyprowski wrote:
> Hello,
> 
> 
> On 2016-05-25 19:11, Javier Martinez Canillas wrote:
>> Hello Marek,
>>
>> On 05/24/2016 09:31 AM, Marek Szyprowski wrote:
>>> This patch replaces custom properties for defining reserved memory
>>> regions with generic reserved memory bindings for MFC video codec
>>> device.
>>>
>>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>> ---
>> [snip]
>>
>>> +
>>> +/ {
>>> +    reserved-memory {
>>> +        #address-cells = <1>;
>>> +        #size-cells = <1>;
>>> +        ranges;
>>> +
>>> +        mfc_left: region@51000000 {
>>> +            compatible = "shared-dma-pool";
>>> +            no-map;
>>> +            reg = <0x51000000 0x800000>;
>>> +        };
>>> +
>>> +        mfc_right: region@43000000 {
>>> +            compatible = "shared-dma-pool";
>>> +            no-map;
>>> +            reg = <0x43000000 0x800000>;
>>> +        };
>>> +    };
>> I've a question probably for a follow up patch, but do you know what's a
>> sane default size for these? I needed to bump the mfc_left size from 8 MiB
>> to 16 MiB in order to decode a 480p H264 video using GStramer. So clearly
>> the default sizes are not that useful.
> 
> Right, the default size for the 'left' region can be increased. Frankly, those
> values (8MiB/0x43000000+ 8MiB/0x51000000) comes from my initial patches
> prepared for some demo and don't have much with any real requirements. They
> were copied (blindly...) by various developers without any deeper understanding.

Yes, I've to admit that I was one of those when added the MFC regions to the
Peach Chromebooks but worked because I tested with small videos. When trying
to decode bigger videos, then had to increase the 'left' region as mentioned.

> Probably the most sane would be to use something like this:
> 
> mfc_left: region_mfc_left {
>          compatible = "shared-dma-pool";
>          no-map;
>          size = <0x1000000>;
>          alignment = <0x100000>;
> };
> 
> mfc_right: region_mfc_right {
>          compatible = "shared-dma-pool";
>          no-map;
>          size = <0x800000>;
>          alignment = <0x100000>;
> };
> 
> So the region will be allocated automatically from the available memory. This way
> another nice feature of the generic reserved memory regions can be used.
>

That sounds better indeed. Not requiring a certain memory offset will also have the
nice side effect to prevent conflicts like the one Pankaj had with his initramfs [0].
 
> The only platform which really requires MFC regions to be placed at certain memory
> offsets is Samsung S5PV210/S5PC110 (sometimes called exynos3), where there is no
> memory address interleaving and MFC device has limited memory interface, which cannot
> do 2 transactions to the same physical memory bank. However S5PV210/S5PC110 machine
> code lost support for MFC during conversion to device tree, so it is not a problem
> here.
>
> All newer platforms (Exynos4, Exynos3250, Exynos5+) use memory interleaving, so the
> actual offset of memory bank has no influence on the physical memory bank.
>

I see, thanks a lot for the explanation.
 
>>> +};
>>> diff --git a/arch/arm/boot/dts/exynos4210-origen.dts b/arch/arm/boot/dts/exynos4210-origen.dts
>>> index ad7394c..f5e4eb2 100644
>>> --- a/arch/arm/boot/dts/exynos4210-origen.dts
>>> +++ b/arch/arm/boot/dts/exynos4210-origen.dts
>>> @@ -18,6 +18,7 @@
>>>   #include "exynos4210.dtsi"
>>>   #include <dt-bindings/gpio/gpio.h>
>>>   #include <dt-bindings/input/input.h>
>>> +#include "exynos-mfc-reserved-memory.dtsi"
>>>     / {
>>>       model = "Insignal Origen evaluation board based on Exynos4210";
>>> @@ -288,8 +289,7 @@
>>>   };
>>>     &mfc {
>>> -    samsung,mfc-r = <0x43000000 0x800000>;
>>> -    samsung,mfc-l = <0x51000000 0x800000>;
>>> +    memory-region = <&mfc_left>, <&mfc_right>;
>>>       status = "okay";
>> I wonder if shouldn't be better to include the exynos-mfc-reserved-memory.dtsi
>> on each SoC dtsi and set the memory-regions in the MFC node instead of doing
>> it on each DTS, and let DTS to just replace with its own memory regions if the
>> default sizes are not suitable for them.
> 
> I don't have strong opinion on this. Maybe it would make more sense to move the
> following entry:
> 
> &mfc {
>         memory-region = <&mfc_left>, <&mfc_right>;
> };
> 
> also to the exynos-mfc-reserved-memory.dtsi ? So board will just include it if
> it want to use MFC device with reserved memory regions.
>

Ok, that also sounds like a good option for me.
 
>> Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>
>> Tested-by: Javier Martinez Canillas <javier@osg.samsung.com>
> 
> Best regards

[0]: https://lkml.org/lkml/2016/5/26/98

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH v4 5/7] ARM: Exynos: remove code for MFC custom reserved memory handling

[Krzysztof Kozlowski]

On 05/24/2016 03:31 PM, Marek Szyprowski wrote:
> Once MFC driver has been converted to generic reserved memory bindings,
> there is no need for custom memory reservation code.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  arch/arm/mach-exynos/Makefile      |  2 -
>  arch/arm/mach-exynos/exynos.c      | 19 --------
>  arch/arm/mach-exynos/mfc.h         | 16 -------
>  arch/arm/mach-exynos/s5p-dev-mfc.c | 93 --------------------------------------
>  4 files changed, 130 deletions(-)
>  delete mode 100644 arch/arm/mach-exynos/mfc.h
>  delete mode 100644 arch/arm/mach-exynos/s5p-dev-mfc.c

Thanks, applied.

What is your final decision for DTS patches? Which approach for MFC
reserved memory node?

Krzysztof

Re: [PATCH v4 5/7] ARM: Exynos: remove code for MFC custom reserved memory handling

[Javier Martinez Canillas]

Hello Krzysztof,

On 05/30/2016 03:28 AM, Krzysztof Kozlowski wrote:
> On 05/24/2016 03:31 PM, Marek Szyprowski wrote:
>> Once MFC driver has been converted to generic reserved memory bindings,
>> there is no need for custom memory reservation code.
>>
>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> ---
>>  arch/arm/mach-exynos/Makefile      |  2 -
>>  arch/arm/mach-exynos/exynos.c      | 19 --------
>>  arch/arm/mach-exynos/mfc.h         | 16 -------
>>  arch/arm/mach-exynos/s5p-dev-mfc.c | 93 --------------------------------------
>>  4 files changed, 130 deletions(-)
>>  delete mode 100644 arch/arm/mach-exynos/mfc.h
>>  delete mode 100644 arch/arm/mach-exynos/s5p-dev-mfc.c
> 
> Thanks, applied.
>

This patch can't be applied before patches 2/5 and 3/5, or the custom
memory regions reservation will break with the current s5p-mfc driver.
 
> What is your final decision for DTS patches? Which approach for MFC
> reserved memory node?
> 
> Krzysztof
> 
 
Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH v4 5/7] ARM: Exynos: remove code for MFC custom reserved memory handling

[Krzysztof Kozlowski]

On 06/02/2016 05:20 PM, Javier Martinez Canillas wrote:
> Hello Krzysztof,
> 
> On 05/30/2016 03:28 AM, Krzysztof Kozlowski wrote:
>> On 05/24/2016 03:31 PM, Marek Szyprowski wrote:
>>> Once MFC driver has been converted to generic reserved memory bindings,
>>> there is no need for custom memory reservation code.
>>>
>>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>> ---
>>>  arch/arm/mach-exynos/Makefile      |  2 -
>>>  arch/arm/mach-exynos/exynos.c      | 19 --------
>>>  arch/arm/mach-exynos/mfc.h         | 16 -------
>>>  arch/arm/mach-exynos/s5p-dev-mfc.c | 93 --------------------------------------
>>>  4 files changed, 130 deletions(-)
>>>  delete mode 100644 arch/arm/mach-exynos/mfc.h
>>>  delete mode 100644 arch/arm/mach-exynos/s5p-dev-mfc.c
>>
>> Thanks, applied.
>>
> 
> This patch can't be applied before patches 2/5 and 3/5, or the custom
> memory regions reservation will break with the current s5p-mfc driver.

Yes, I know. As I understood from talk with Marek, the driver is broken
now so continuous work was not chosen. If it is not correct and full
bisectability is required, then entire patchset requires special
handling - I need a stable tag from media tree. Without this everything
will be broken anyway.

Best regards,
Krzysztof

Re: [PATCH v4 5/7] ARM: Exynos: remove code for MFC custom reserved memory handling

[Javier Martinez Canillas]

Hello Krzysztof,

On 06/02/2016 12:31 PM, Krzysztof Kozlowski wrote:
> On 06/02/2016 05:20 PM, Javier Martinez Canillas wrote:
>> Hello Krzysztof,
>>
>> On 05/30/2016 03:28 AM, Krzysztof Kozlowski wrote:
>>> On 05/24/2016 03:31 PM, Marek Szyprowski wrote:
>>>> Once MFC driver has been converted to generic reserved memory bindings,
>>>> there is no need for custom memory reservation code.
>>>>
>>>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>>> ---
>>>>  arch/arm/mach-exynos/Makefile      |  2 -
>>>>  arch/arm/mach-exynos/exynos.c      | 19 --------
>>>>  arch/arm/mach-exynos/mfc.h         | 16 -------
>>>>  arch/arm/mach-exynos/s5p-dev-mfc.c | 93 --------------------------------------
>>>>  4 files changed, 130 deletions(-)
>>>>  delete mode 100644 arch/arm/mach-exynos/mfc.h
>>>>  delete mode 100644 arch/arm/mach-exynos/s5p-dev-mfc.c
>>>
>>> Thanks, applied.
>>>
>>
>> This patch can't be applied before patches 2/5 and 3/5, or the custom
>> memory regions reservation will break with the current s5p-mfc driver.
> 
> Yes, I know. As I understood from talk with Marek, the driver is broken
> now so continuous work was not chosen. If it is not correct and full

It's true that the driven is currently broken in mainline and is not really
stable, I posted fixes for all the issues I found (mostly in module removal
and insert paths).

But with just the following patch from Ayaka on top of mainline, I'm able to
have video decoding working: https://lkml.org/lkml/2016/5/6/577

Marek mentioned that bisectability is only partially broken because the old
binding will still work after this series if IOMMU is enabled (because the
properties are ignored in this case). But will break if IOMMU isn't enabled
which will be the case for some boards that fails to boot with IOMMU due the
bootloader leaving the FIMD enabled doing DMA operations automatically AFAIU. 

Now, I'm OK with not keeping backwards compatibility for the MFC dt bindings
since arguably the driver has been broken for a long time and nobody cared
and also I don't think anyone in practice boots a new kernel with an old DTB
for Exynos.

But I don't think is correct to introduce a new issue as is the case if this
patch is applied before the previous patches in the series since this causes
the driver to probe to fail and the following warn on boot (while it used to
at least probe correctly in mainline):

[   17.190165] WARNING: CPU: 0 PID: 224 at kernel/memremap.c:111 memremap+0x1a8/0x1b0
[   17.193127] memremap attempted on ram 0x51000000 size: 0x800000
[   17.196247] Modules linked in: s5p_mfc(+) uvcvideo s5p_jpeg videobuf2_vmalloc v4l2_mem2mem
[   17.199569] CPU: 0 PID: 224 Comm: systemd-udevd Not tainted 4.7.0-rc1-next-20160531-00006-g569df5b983f3 #68
[   17.202556] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
[   17.205534] [<c010e1ac>] (unwind_backtrace) from [<c010af38>] (show_stack+0x10/0x14)
[   17.208535] [<c010af38>] (show_stack) from [<c0323b9c>] (dump_stack+0x88/0x9c)
[   17.211530] [<c0323b9c>] (dump_stack) from [<c011a828>] (__warn+0xe8/0x100)
[   17.214492] [<c011a828>] (__warn) from [<c011a878>] (warn_slowpath_fmt+0x38/0x48)
[   17.217479] [<c011a878>] (warn_slowpath_fmt) from [<c0194ee0>] (memremap+0x1a8/0x1b0)
[   17.220476] [<c0194ee0>] (memremap) from [<c040d92c>] (dma_init_coherent_memory+0xf8/0x130)
[   17.223489] [<c040d92c>] (dma_init_coherent_memory) from [<c040d990>] (dma_declare_coherent_memory+0x2c/0x6c)
[   17.226570] [<c040d990>] (dma_declare_coherent_memory) from [<bf1768c4>] (s5p_mfc_probe+0x170/0x650 [s5p_mfc])
[   17.229648] [<bf1768c4>] (s5p_mfc_probe [s5p_mfc]) from [<c03fb704>] (platform_drv_probe+0x4c/0xb0)
[   17.232718] [<c03fb704>] (platform_drv_probe) from [<c03f9e58>] (driver_probe_device+0x214/0x2c0)
[   17.235800] [<c03f9e58>] (driver_probe_device) from [<c03f9fb0>] (__driver_attach+0xac/0xb0)
[   17.238906] [<c03f9fb0>] (__driver_attach) from [<c03f81d0>] (bus_for_each_dev+0x68/0x9c)
[   17.242031] [<c03f81d0>] (bus_for_each_dev) from [<c03f944c>] (bus_add_driver+0x1a0/0x218)
[   17.245160] [<c03f944c>] (bus_add_driver) from [<c03fa7c8>] (driver_register+0x78/0xf8)
[   17.248300] [<c03fa7c8>] (driver_register) from [<c01017c0>] (do_one_initcall+0x40/0x170)
[   17.251475] [<c01017c0>] (do_one_initcall) from [<c01956e4>] (do_init_module+0x60/0x1b0)
[   17.254656] [<c01956e4>] (do_init_module) from [<c0188dc0>] (load_module+0x17ec/0x1dd0)
[   17.257844] [<c0188dc0>] (load_module) from [<c0189570>] (SyS_finit_module+0x8c/0x9c)
[   17.261037] [<c0189570>] (SyS_finit_module) from [<c01078c0>] (ret_fast_syscall+0x0/0x3c)
[   17.265210] ---[ end trace 33de2b5daf697e0f ]---
[   17.269300] s5p_mfc_alloc_memdevs:1072: Failed to declare coherent memory for
               MFC device
[   17.277593] ------------[ cut here ]------------

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH v4 5/7] ARM: Exynos: remove code for MFC custom reserved memory handling

[Krzysztof Kozlowski]

On 06/02/2016 07:25 PM, Javier Martinez Canillas wrote:
> Hello Krzysztof,
> 
> On 06/02/2016 12:31 PM, Krzysztof Kozlowski wrote:
>> On 06/02/2016 05:20 PM, Javier Martinez Canillas wrote:
>>> Hello Krzysztof,
>>>
>>> On 05/30/2016 03:28 AM, Krzysztof Kozlowski wrote:
>>>> On 05/24/2016 03:31 PM, Marek Szyprowski wrote:
>>>>> Once MFC driver has been converted to generic reserved memory bindings,
>>>>> there is no need for custom memory reservation code.
>>>>>
>>>>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>>>> ---
>>>>>  arch/arm/mach-exynos/Makefile      |  2 -
>>>>>  arch/arm/mach-exynos/exynos.c      | 19 --------
>>>>>  arch/arm/mach-exynos/mfc.h         | 16 -------
>>>>>  arch/arm/mach-exynos/s5p-dev-mfc.c | 93 --------------------------------------
>>>>>  4 files changed, 130 deletions(-)
>>>>>  delete mode 100644 arch/arm/mach-exynos/mfc.h
>>>>>  delete mode 100644 arch/arm/mach-exynos/s5p-dev-mfc.c
>>>>
>>>> Thanks, applied.
>>>>
>>>
>>> This patch can't be applied before patches 2/5 and 3/5, or the custom
>>> memory regions reservation will break with the current s5p-mfc driver.
>>
>> Yes, I know. As I understood from talk with Marek, the driver is broken
>> now so continuous work was not chosen. If it is not correct and full
> 
> It's true that the driven is currently broken in mainline and is not really
> stable, I posted fixes for all the issues I found (mostly in module removal
> and insert paths).
> 
> But with just the following patch from Ayaka on top of mainline, I'm able to
> have video decoding working: https://lkml.org/lkml/2016/5/6/577

Which is still a "future" patch, not current state...
> 
> Marek mentioned that bisectability is only partially broken because the old
> binding will still work after this series if IOMMU is enabled (because the
> properties are ignored in this case). But will break if IOMMU isn't enabled
> which will be the case for some boards that fails to boot with IOMMU due the
> bootloader leaving the FIMD enabled doing DMA operations automatically AFAIU. 
> 
> Now, I'm OK with not keeping backwards compatibility for the MFC dt bindings
> since arguably the driver has been broken for a long time and nobody cared
> and also I don't think anyone in practice boots a new kernel with an old DTB
> for Exynos.
> 
> But I don't think is correct to introduce a new issue as is the case if this
> patch is applied before the previous patches in the series since this causes
> the driver to probe to fail and the following warn on boot (while it used to
> at least probe correctly in mainline):

Okay but the patches will go through separate tree. This is not a
problem, as I said, I just need a stable tag from media tree with first
four patches (Mauro?).

Best regards,
Krzysztof

[ATTN] Re: [PATCH v4 5/7] ARM: Exynos: remove code for MFC custom reserved memory handling

[Sylwester Nawrocki]

On 06/03/2016 08:57 AM, Krzysztof Kozlowski wrote:
> On 06/02/2016 07:25 PM, Javier Martinez Canillas wrote:
>> On 06/02/2016 12:31 PM, Krzysztof Kozlowski wrote:
>>> On 06/02/2016 05:20 PM, Javier Martinez Canillas wrote:
>>>> On 05/30/2016 03:28 AM, Krzysztof Kozlowski wrote:
>>>>> On 05/24/2016 03:31 PM, Marek Szyprowski wrote:
>>>>>> Once MFC driver has been converted to generic reserved memory bindings,
>>>>>> there is no need for custom memory reservation code.
>>>>>>
>>>>>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>>>>> ---
>>>>>>  arch/arm/mach-exynos/Makefile      |  2 -
>>>>>>  arch/arm/mach-exynos/exynos.c      | 19 --------
>>>>>>  arch/arm/mach-exynos/mfc.h         | 16 -------
>>>>>>  arch/arm/mach-exynos/s5p-dev-mfc.c | 93 -----------------------------
>>>>>>  4 files changed, 130 deletions(-)
>>>>>>  delete mode 100644 arch/arm/mach-exynos/mfc.h
>>>>>>  delete mode 100644 arch/arm/mach-exynos/s5p-dev-mfc.c
>>>>>
>>>>> Thanks, applied.
>>>>
>>>> This patch can't be applied before patches 2/5 and 3/5, or the custom
>>>> memory regions reservation will break with the current s5p-mfc driver.
>>>
>>> Yes, I know. As I understood from talk with Marek, the driver is broken
>>> now so continuous work was not chosen. If it is not correct and full
>>
>> It's true that the driven is currently broken in mainline and is not really
>> stable, I posted fixes for all the issues I found (mostly in module removal
>> and insert paths).
>>
>> But with just the following patch from Ayaka on top of mainline, I'm able to
>> have video decoding working: https://lkml.org/lkml/2016/5/6/577
> 
> Which is still a "future" patch, not current state...
>>
>> Marek mentioned that bisectability is only partially broken because the old
>> binding will still work after this series if IOMMU is enabled (because the
>> properties are ignored in this case). But will break if IOMMU isn't enabled
>> which will be the case for some boards that fails to boot with IOMMU due the
>> bootloader leaving the FIMD enabled doing DMA operations automatically AFAIU. 
>>
>> Now, I'm OK with not keeping backwards compatibility for the MFC dt bindings
>> since arguably the driver has been broken for a long time and nobody cared
>> and also I don't think anyone in practice boots a new kernel with an old DTB
>> for Exynos.
>>
>> But I don't think is correct to introduce a new issue as is the case if this
>> patch is applied before the previous patches in the series since this causes
>> the driver to probe to fail and the following warn on boot (while it used to
>> at least probe correctly in mainline):
> 
> Okay but the patches will go through separate tree. This is not a
> problem, as I said, I just need a stable tag from media tree with first
> four patches (Mauro?).

I have prepared a topic branch including media patches from this patch
series and the dependency fix patches from Javier and Marek.
So this could be used as a topic branch to pull into media master branch
and a dependency topic branch for Krzysztof's samsung-soc tree.
Mauro, can we do it this way? I already talked to Kamil about this.

---8<----
The following changes since commit 1a695a905c18548062509178b98bc91e67510864:

  Linux 4.7-rc1 (2016-05-29 09:29:24 -0700)

are available in the git repository at:

  git://linuxtv.org/snawrocki/samsung.git for-v4.8/media/exynos-mfc

for you to fetch changes up to 04f776734c4e03e33111d3d5a994b589870df623:

  media: s5p-mfc: add iommu support (2016-06-03 11:13:45 +0200)

----------------------------------------------------------------
Javier Martinez Canillas (3):
      s5p-mfc: Set device name for reserved memory region devs
      s5p-mfc: Add release callback for memory region devs
      s5p-mfc: Fix race between s5p_mfc_probe() and s5p_mfc_open()

Marek Szyprowski (6):
      media: vb2-dma-contig: add helper for setting dma max seg size
      media: set proper max seg size for devices on Exynos SoCs
      of: reserved_mem: add support for using more than one region for given device
      media: s5p-mfc: use generic reserved memory bindings
      media: s5p-mfc: replace custom reserved memory handling code with generic one
      media: s5p-mfc: add iommu support

 Documentation/devicetree/bindings/media/s5p-mfc.txt |  39 ++++-
 drivers/media/platform/exynos-gsc/gsc-core.c        |   2 +
 drivers/media/platform/exynos4-is/fimc-core.c       |   2 +
 drivers/media/platform/exynos4-is/fimc-is.c         |   2 +
 drivers/media/platform/exynos4-is/fimc-lite.c       |   2 +
 drivers/media/platform/s5p-g2d/g2d.c                |   2 +
 drivers/media/platform/s5p-jpeg/jpeg-core.c         |   2 +
 drivers/media/platform/s5p-mfc/s5p_mfc.c            | 198 ++++++++++++++-----------
 drivers/media/platform/s5p-mfc/s5p_mfc_iommu.h      |  79 ++++++++++
 drivers/media/platform/s5p-tv/mixer_video.c         |   2 +
 drivers/media/v4l2-core/videobuf2-dma-contig.c      |  53 +++++++
 drivers/of/of_reserved_mem.c                        |  85 ++++++++---
 include/linux/of_reserved_mem.h                     |  25 +++-
 include/media/videobuf2-dma-contig.h                |   2 +
 14 files changed, 379 insertions(+), 116 deletions(-)
 create mode 100644 drivers/media/platform/s5p-mfc/s5p_mfc_iommu.h
---8<----

-- 
Regards,
Sylwester

Re: [ATTN] Re: [PATCH v4 5/7] ARM: Exynos: remove code for MFC custom reserved memory handling

[Krzysztof Kozlowski]

On 06/03/2016 11:59 AM, Sylwester Nawrocki wrote:
> On 06/03/2016 08:57 AM, Krzysztof Kozlowski wrote:
>> On 06/02/2016 07:25 PM, Javier Martinez Canillas wrote:
>>> On 06/02/2016 12:31 PM, Krzysztof Kozlowski wrote:
>>>> On 06/02/2016 05:20 PM, Javier Martinez Canillas wrote:
>>>>> On 05/30/2016 03:28 AM, Krzysztof Kozlowski wrote:
>>>>>> On 05/24/2016 03:31 PM, Marek Szyprowski wrote:
>>>>>>> Once MFC driver has been converted to generic reserved memory bindings,
>>>>>>> there is no need for custom memory reservation code.
>>>>>>>
>>>>>>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>>>>>> ---
>>>>>>>  arch/arm/mach-exynos/Makefile      |  2 -
>>>>>>>  arch/arm/mach-exynos/exynos.c      | 19 --------
>>>>>>>  arch/arm/mach-exynos/mfc.h         | 16 -------
>>>>>>>  arch/arm/mach-exynos/s5p-dev-mfc.c | 93 -----------------------------
>>>>>>>  4 files changed, 130 deletions(-)
>>>>>>>  delete mode 100644 arch/arm/mach-exynos/mfc.h
>>>>>>>  delete mode 100644 arch/arm/mach-exynos/s5p-dev-mfc.c
>>>>>>
>>>>>> Thanks, applied.
>>>>>
>>>>> This patch can't be applied before patches 2/5 and 3/5, or the custom
>>>>> memory regions reservation will break with the current s5p-mfc driver.
>>>>
>>>> Yes, I know. As I understood from talk with Marek, the driver is broken
>>>> now so continuous work was not chosen. If it is not correct and full
>>>
>>> It's true that the driven is currently broken in mainline and is not really
>>> stable, I posted fixes for all the issues I found (mostly in module removal
>>> and insert paths).
>>>
>>> But with just the following patch from Ayaka on top of mainline, I'm able to
>>> have video decoding working: https://lkml.org/lkml/2016/5/6/577
>>
>> Which is still a "future" patch, not current state...
>>>
>>> Marek mentioned that bisectability is only partially broken because the old
>>> binding will still work after this series if IOMMU is enabled (because the
>>> properties are ignored in this case). But will break if IOMMU isn't enabled
>>> which will be the case for some boards that fails to boot with IOMMU due the
>>> bootloader leaving the FIMD enabled doing DMA operations automatically AFAIU. 
>>>
>>> Now, I'm OK with not keeping backwards compatibility for the MFC dt bindings
>>> since arguably the driver has been broken for a long time and nobody cared
>>> and also I don't think anyone in practice boots a new kernel with an old DTB
>>> for Exynos.
>>>
>>> But I don't think is correct to introduce a new issue as is the case if this
>>> patch is applied before the previous patches in the series since this causes
>>> the driver to probe to fail and the following warn on boot (while it used to
>>> at least probe correctly in mainline):
>>
>> Okay but the patches will go through separate tree. This is not a
>> problem, as I said, I just need a stable tag from media tree with first
>> four patches (Mauro?).
> 
> I have prepared a topic branch including media patches from this patch
> series and the dependency fix patches from Javier and Marek.
> So this could be used as a topic branch to pull into media master branch
> and a dependency topic branch for Krzysztof's samsung-soc tree.
> Mauro, can we do it this way? I already talked to Kamil about this.
> 
> ---8<----
> The following changes since commit 1a695a905c18548062509178b98bc91e67510864:
> 
>   Linux 4.7-rc1 (2016-05-29 09:29:24 -0700)
> 
> are available in the git repository at:
> 
>   git://linuxtv.org/snawrocki/samsung.git for-v4.8/media/exynos-mfc
> 
> for you to fetch changes up to 04f776734c4e03e33111d3d5a994b589870df623:
> 
>   media: s5p-mfc: add iommu support (2016-06-03 11:13:45 +0200)

Thanks Sylwester! Although this is branch not a tag but I believe it
will stay stable. Pulled. I will apply Exynos-specific patches on top of it.

Best regards,
Krzysztof

Re: [PATCH v4 5/7] ARM: Exynos: remove code for MFC custom reserved memory handling

[Krzysztof Kozlowski]

On 06/03/2016 08:57 AM, Krzysztof Kozlowski wrote:
> On 06/02/2016 07:25 PM, Javier Martinez Canillas wrote:
>> Marek mentioned that bisectability is only partially broken because the old
>> binding will still work after this series if IOMMU is enabled (because the
>> properties are ignored in this case). But will break if IOMMU isn't enabled
>> which will be the case for some boards that fails to boot with IOMMU due the
>> bootloader leaving the FIMD enabled doing DMA operations automatically AFAIU. 
>>
>> Now, I'm OK with not keeping backwards compatibility for the MFC dt bindings
>> since arguably the driver has been broken for a long time and nobody cared
>> and also I don't think anyone in practice boots a new kernel with an old DTB
>> for Exynos.
>>
>> But I don't think is correct to introduce a new issue as is the case if this
>> patch is applied before the previous patches in the series since this causes
>> the driver to probe to fail and the following warn on boot (while it used to
>> at least probe correctly in mainline):
> 
> Okay but the patches will go through separate tree. This is not a
> problem, as I said, I just need a stable tag from media tree with first
> four patches (Mauro?).

Applied again this and DTS changes (remaining two patches) for v4.8 on
top of branch provided by Sylwester:
https://git.kernel.org/cgit/linux/kernel/git/krzk/linux.git/log/?h=for-v4.8/exynos-mfc

Best regards,
Krzysztof

[PATCH 1/3] ARM: dts: exynos: replace hardcoded reserved memory ranges with auto-allocated ones

[Marek Szyprowski]

Generic reserved memory regions bindings allow to automatically allocate
region of given parameters (alignment and size), so use this feature
instead of the hardcoded values, which had no dependency on the real
hardware. This patch also increases "left" region from 8MiB to 16MiB to
make the codec really usable with nowadays steams (with 8MiB reserved
region it was not even possible to decode 480p H264 video).

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 arch/arm/boot/dts/exynos-mfc-reserved-memory.dtsi | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/dts/exynos-mfc-reserved-memory.dtsi b/arch/arm/boot/dts/exynos-mfc-reserved-memory.dtsi
index c4d063a..da3ced9 100644
--- a/arch/arm/boot/dts/exynos-mfc-reserved-memory.dtsi
+++ b/arch/arm/boot/dts/exynos-mfc-reserved-memory.dtsi
@@ -14,16 +14,18 @@
 		#size-cells = <1>;
 		ranges;
 
-		mfc_left: region@51000000 {
+		mfc_left: region_mfc_left {
 			compatible = "shared-dma-pool";
 			no-map;
-			reg = <0x51000000 0x800000>;
+			size = <0x1000000>;
+			alignment = <0x100000>;
 		};
 
-		mfc_right: region@43000000 {
+		mfc_right: region_mfc_right {
 			compatible = "shared-dma-pool";
 			no-map;
-			reg = <0x43000000 0x800000>;
+			size = <0x800000>;
+			alignment = <0x100000>;
 		};
 	};
 };
-- 
1.9.2

[PATCH 2/3] ARM: dts: exynos: move MFC reserved memory regions from boards to .dtsi

[Marek Szyprowski]

This patch moves assigning reserved memory regions from each board dts
to common exynos-mfc-reserved-memory.dtsi file, where those regions are
defined.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 arch/arm/boot/dts/exynos-mfc-reserved-memory.dtsi  | 4 ++++
 arch/arm/boot/dts/exynos4210-origen.dts            | 1 -
 arch/arm/boot/dts/exynos4210-smdkv310.dts          | 1 -
 arch/arm/boot/dts/exynos4412-odroid-common.dtsi    | 1 -
 arch/arm/boot/dts/exynos4412-origen.dts            | 1 -
 arch/arm/boot/dts/exynos4412-smdk4412.dts          | 1 -
 arch/arm/boot/dts/exynos5250-arndale.dts           | 4 ----
 arch/arm/boot/dts/exynos5250-smdk5250.dts          | 4 ----
 arch/arm/boot/dts/exynos5250-spring.dts            | 4 ----
 arch/arm/boot/dts/exynos5420-arndale-octa.dts      | 4 ----
 arch/arm/boot/dts/exynos5420-peach-pit.dts         | 4 ----
 arch/arm/boot/dts/exynos5420-smdk5420.dts          | 4 ----
 arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi | 4 ----
 arch/arm/boot/dts/exynos5800-peach-pi.dts          | 4 ----
 14 files changed, 4 insertions(+), 37 deletions(-)

diff --git a/arch/arm/boot/dts/exynos-mfc-reserved-memory.dtsi b/arch/arm/boot/dts/exynos-mfc-reserved-memory.dtsi
index da3ced9..f78c14c 100644
--- a/arch/arm/boot/dts/exynos-mfc-reserved-memory.dtsi
+++ b/arch/arm/boot/dts/exynos-mfc-reserved-memory.dtsi
@@ -29,3 +29,7 @@
 		};
 	};
 };
+
+&mfc {
+	memory-region = <&mfc_left>, <&mfc_right>;
+};
diff --git a/arch/arm/boot/dts/exynos4210-origen.dts b/arch/arm/boot/dts/exynos4210-origen.dts
index f5e4eb2..07a00dd 100644
--- a/arch/arm/boot/dts/exynos4210-origen.dts
+++ b/arch/arm/boot/dts/exynos4210-origen.dts
@@ -289,7 +289,6 @@
 };
 
 &mfc {
-	memory-region = <&mfc_left>, <&mfc_right>;
 	status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/exynos4210-smdkv310.dts b/arch/arm/boot/dts/exynos4210-smdkv310.dts
index de917f0..2fab072 100644
--- a/arch/arm/boot/dts/exynos4210-smdkv310.dts
+++ b/arch/arm/boot/dts/exynos4210-smdkv310.dts
@@ -134,7 +134,6 @@
 };
 
 &mfc {
-	memory-region = <&mfc_left>, <&mfc_right>;
 	status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
index 13972ca..b3c95d2 100644
--- a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
+++ b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
@@ -510,7 +510,6 @@
 };
 
 &mfc {
-	memory-region = <&mfc_left>, <&mfc_right>;
 	status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/exynos4412-origen.dts b/arch/arm/boot/dts/exynos4412-origen.dts
index 2959fc8..547ae04 100644
--- a/arch/arm/boot/dts/exynos4412-origen.dts
+++ b/arch/arm/boot/dts/exynos4412-origen.dts
@@ -483,7 +483,6 @@
 };
 
 &mfc {
-	memory-region = <&mfc_left>, <&mfc_right>;
 	status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/exynos4412-smdk4412.dts b/arch/arm/boot/dts/exynos4412-smdk4412.dts
index 9b6d561..d4f9383 100644
--- a/arch/arm/boot/dts/exynos4412-smdk4412.dts
+++ b/arch/arm/boot/dts/exynos4412-smdk4412.dts
@@ -113,7 +113,6 @@
 };
 
 &mfc {
-	memory-region = <&mfc_left>, <&mfc_right>;
 	status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/exynos5250-arndale.dts b/arch/arm/boot/dts/exynos5250-arndale.dts
index 39940f4..ea70603 100644
--- a/arch/arm/boot/dts/exynos5250-arndale.dts
+++ b/arch/arm/boot/dts/exynos5250-arndale.dts
@@ -516,10 +516,6 @@
 	status = "okay";
 };
 
-&mfc {
-	memory-region = <&mfc_left>, <&mfc_right>;
-};
-
 &mmc_0 {
 	status = "okay";
 	num-slots = <1>;
diff --git a/arch/arm/boot/dts/exynos5250-smdk5250.dts b/arch/arm/boot/dts/exynos5250-smdk5250.dts
index 9fac874..381af13 100644
--- a/arch/arm/boot/dts/exynos5250-smdk5250.dts
+++ b/arch/arm/boot/dts/exynos5250-smdk5250.dts
@@ -344,10 +344,6 @@
 	status = "okay";
 };
 
-&mfc {
-	memory-region = <&mfc_left>, <&mfc_right>;
-};
-
 &mmc_0 {
 	status = "okay";
 	num-slots = <1>;
diff --git a/arch/arm/boot/dts/exynos5250-spring.dts b/arch/arm/boot/dts/exynos5250-spring.dts
index 784130b..44f4292 100644
--- a/arch/arm/boot/dts/exynos5250-spring.dts
+++ b/arch/arm/boot/dts/exynos5250-spring.dts
@@ -425,10 +425,6 @@
 	status = "okay";
 };
 
-&mfc {
-	memory-region = <&mfc_left>, <&mfc_right>;
-};
-
 &mmc_0 {
 	status = "okay";
 	num-slots = <1>;
diff --git a/arch/arm/boot/dts/exynos5420-arndale-octa.dts b/arch/arm/boot/dts/exynos5420-arndale-octa.dts
index b8b5f3a..39a3b81 100644
--- a/arch/arm/boot/dts/exynos5420-arndale-octa.dts
+++ b/arch/arm/boot/dts/exynos5420-arndale-octa.dts
@@ -347,10 +347,6 @@
 	};
 };
 
-&mfc {
-	memory-region = <&mfc_left>, <&mfc_right>;
-};
-
 &mmc_0 {
 	status = "okay";
 	broken-cd;
diff --git a/arch/arm/boot/dts/exynos5420-peach-pit.dts b/arch/arm/boot/dts/exynos5420-peach-pit.dts
index 5bd3f07..fe57423 100644
--- a/arch/arm/boot/dts/exynos5420-peach-pit.dts
+++ b/arch/arm/boot/dts/exynos5420-peach-pit.dts
@@ -689,10 +689,6 @@
 	status = "okay";
 };
 
-&mfc {
-	memory-region = <&mfc_left>, <&mfc_right>;
-};
-
 &mmc_0 {
 	status = "okay";
 	num-slots = <1>;
diff --git a/arch/arm/boot/dts/exynos5420-smdk5420.dts b/arch/arm/boot/dts/exynos5420-smdk5420.dts
index 5206f41..ed8f342 100644
--- a/arch/arm/boot/dts/exynos5420-smdk5420.dts
+++ b/arch/arm/boot/dts/exynos5420-smdk5420.dts
@@ -355,10 +355,6 @@
 	};
 };
 
-&mfc {
-	memory-region = <&mfc_left>, <&mfc_right>;
-};
-
 &mmc_0 {
 	status = "okay";
 	broken-cd;
diff --git a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
index d52a80f..d562530 100644
--- a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
+++ b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
@@ -495,10 +495,6 @@
 	};
 };
 
-&mfc {
-	memory-region = <&mfc_left>, <&mfc_right>;
-};
-
 &mmc_0 {
 	status = "okay";
 	mmc-pwrseq = <&emmc_pwrseq>;
diff --git a/arch/arm/boot/dts/exynos5800-peach-pi.dts b/arch/arm/boot/dts/exynos5800-peach-pi.dts
index 65482b7..5ec71e2 100644
--- a/arch/arm/boot/dts/exynos5800-peach-pi.dts
+++ b/arch/arm/boot/dts/exynos5800-peach-pi.dts
@@ -664,10 +664,6 @@
 	status = "okay";
 };
 
-&mfc {
-	memory-region = <&mfc_left>, <&mfc_right>;
-};
-
 &mmc_0 {
 	status = "okay";
 	num-slots = <1>;
-- 
1.9.2

[PATCH 3/3] ARM: dts: exynos: enable MFC device for all boards

[Marek Szyprowski]

MFC device can be used without any external hardware dependencies (when
IOMMU is enabled), so it can be enabled by default (like it has been
already done for Exynos 542x platforms). This unifies handling of this
device for Exynos3250, Exynos4 and Exynos542x platforms. Board can still
include exynos-mfc-reserved-memory.dtsi to enable using this device
without IOMMU being enabled.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 arch/arm/boot/dts/exynos3250-rinato.dts         | 4 ----
 arch/arm/boot/dts/exynos3250.dtsi               | 1 -
 arch/arm/boot/dts/exynos4.dtsi                  | 1 -
 arch/arm/boot/dts/exynos4210-origen.dts         | 4 ----
 arch/arm/boot/dts/exynos4210-smdkv310.dts       | 4 ----
 arch/arm/boot/dts/exynos4412-odroid-common.dtsi | 4 ----
 arch/arm/boot/dts/exynos4412-origen.dts         | 4 ----
 arch/arm/boot/dts/exynos4412-smdk4412.dts       | 4 ----
 8 files changed, 26 deletions(-)

diff --git a/arch/arm/boot/dts/exynos3250-rinato.dts b/arch/arm/boot/dts/exynos3250-rinato.dts
index e4228195..a921813 100644
--- a/arch/arm/boot/dts/exynos3250-rinato.dts
+++ b/arch/arm/boot/dts/exynos3250-rinato.dts
@@ -632,10 +632,6 @@
 	status = "okay";
 };
 
-&mfc {
-	status = "okay";
-};
-
 &jpeg {
 	status = "okay";
 };
diff --git a/arch/arm/boot/dts/exynos3250.dtsi b/arch/arm/boot/dts/exynos3250.dtsi
index 62f3dcd..70e3ace 100644
--- a/arch/arm/boot/dts/exynos3250.dtsi
+++ b/arch/arm/boot/dts/exynos3250.dtsi
@@ -431,7 +431,6 @@
 			clocks = <&cmu CLK_MFC>, <&cmu CLK_SCLK_MFC>;
 			power-domains = <&pd_mfc>;
 			iommus = <&sysmmu_mfc>;
-			status = "disabled";
 		};
 
 		sysmmu_mfc: sysmmu@13620000 {
diff --git a/arch/arm/boot/dts/exynos4.dtsi b/arch/arm/boot/dts/exynos4.dtsi
index ca8f3e3..32f22e1 100644
--- a/arch/arm/boot/dts/exynos4.dtsi
+++ b/arch/arm/boot/dts/exynos4.dtsi
@@ -428,7 +428,6 @@
 		clock-names = "mfc", "sclk_mfc";
 		iommus = <&sysmmu_mfc_l>, <&sysmmu_mfc_r>;
 		iommu-names = "left", "right";
-		status = "disabled";
 	};
 
 	serial_0: serial@13800000 {
diff --git a/arch/arm/boot/dts/exynos4210-origen.dts b/arch/arm/boot/dts/exynos4210-origen.dts
index 07a00dd..be2751e 100644
--- a/arch/arm/boot/dts/exynos4210-origen.dts
+++ b/arch/arm/boot/dts/exynos4210-origen.dts
@@ -288,10 +288,6 @@
 	};
 };
 
-&mfc {
-	status = "okay";
-};
-
 &sdhci_0 {
 	bus-width = <4>;
 	pinctrl-0 = <&sd0_clk &sd0_cmd &sd0_bus4 &sd0_cd>;
diff --git a/arch/arm/boot/dts/exynos4210-smdkv310.dts b/arch/arm/boot/dts/exynos4210-smdkv310.dts
index 2fab072..847fae3 100644
--- a/arch/arm/boot/dts/exynos4210-smdkv310.dts
+++ b/arch/arm/boot/dts/exynos4210-smdkv310.dts
@@ -133,10 +133,6 @@
 	};
 };
 
-&mfc {
-	status = "okay";
-};
-
 &pinctrl_1 {
 	keypad_rows: keypad-rows {
 		samsung,pins = "gpx2-0", "gpx2-1";
diff --git a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
index b3c95d2..58ad48e7 100644
--- a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
+++ b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
@@ -509,10 +509,6 @@
 	clock-names = "iis", "i2s_opclk0", "i2s_opclk1";
 };
 
-&mfc {
-	status = "okay";
-};
-
 &mixer {
 	status = "okay";
 };
diff --git a/arch/arm/boot/dts/exynos4412-origen.dts b/arch/arm/boot/dts/exynos4412-origen.dts
index 547ae04..26a36fe 100644
--- a/arch/arm/boot/dts/exynos4412-origen.dts
+++ b/arch/arm/boot/dts/exynos4412-origen.dts
@@ -482,10 +482,6 @@
 	};
 };
 
-&mfc {
-	status = "okay";
-};
-
 &mshc_0 {
 	pinctrl-0 = <&sd4_clk &sd4_cmd &sd4_bus4 &sd4_bus8>;
 	pinctrl-names = "default";
diff --git a/arch/arm/boot/dts/exynos4412-smdk4412.dts b/arch/arm/boot/dts/exynos4412-smdk4412.dts
index d4f9383..231ffbd 100644
--- a/arch/arm/boot/dts/exynos4412-smdk4412.dts
+++ b/arch/arm/boot/dts/exynos4412-smdk4412.dts
@@ -112,10 +112,6 @@
 	};
 };
 
-&mfc {
-	status = "okay";
-};
-
 &pinctrl_1 {
 	keypad_rows: keypad-rows {
 		samsung,pins = "gpx2-0", "gpx2-1", "gpx2-2";
-- 
1.9.2

Re: [PATCH 1/3] ARM: dts: exynos: replace hardcoded reserved memory ranges with auto-allocated ones

[Javier Martinez Canillas]

Hello Marek,

On 06/07/2016 08:03 AM, Marek Szyprowski wrote:
> Generic reserved memory regions bindings allow to automatically allocate
> region of given parameters (alignment and size), so use this feature
> instead of the hardcoded values, which had no dependency on the real
> hardware. This patch also increases "left" region from 8MiB to 16MiB to
> make the codec really usable with nowadays steams (with 8MiB reserved
> region it was not even possible to decode 480p H264 video).
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---

The patch looks good to me and I've also tested it on an Exynos5800 Peach
Pi Chromebook and an Exynos5420 Odroid XU4 board.

Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>
Tested-by: Javier Martinez Canillas <javier@osg.samsung.com>

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH 2/3] ARM: dts: exynos: move MFC reserved memory regions from boards to .dtsi

[Javier Martinez Canillas]

Hello Marek,

On 06/07/2016 08:03 AM, Marek Szyprowski wrote:
> This patch moves assigning reserved memory regions from each board dts
> to common exynos-mfc-reserved-memory.dtsi file, where those regions are
> defined.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---

Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>
Tested-by: Javier Martinez Canillas <javier@osg.samsung.com>

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH 3/3] ARM: dts: exynos: enable MFC device for all boards

[Javier Martinez Canillas]

Hello Marek,

On 06/07/2016 08:03 AM, Marek Szyprowski wrote:
> MFC device can be used without any external hardware dependencies (when
> IOMMU is enabled), so it can be enabled by default (like it has been
> already done for Exynos 542x platforms). This unifies handling of this
> device for Exynos3250, Exynos4 and Exynos542x platforms. Board can still
> include exynos-mfc-reserved-memory.dtsi to enable using this device
> without IOMMU being enabled.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---

Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH 1/3] ARM: dts: exynos: replace hardcoded reserved memory ranges with auto-allocated ones

[Krzysztof Kozlowski]

On 06/07/2016 02:03 PM, Marek Szyprowski wrote:
> Generic reserved memory regions bindings allow to automatically allocate
> region of given parameters (alignment and size), so use this feature
> instead of the hardcoded values, which had no dependency on the real
> hardware. This patch also increases "left" region from 8MiB to 16MiB to
> make the codec really usable with nowadays steams (with 8MiB reserved
> region it was not even possible to decode 480p H264 video).
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  arch/arm/boot/dts/exynos-mfc-reserved-memory.dtsi | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)

Applied all three on top of topic branch.

Thanks,
Krzysztof

Re: [PATCH v4 3/7] media: s5p-mfc: replace custom reserved memory handling code with generic one

[Liviu Dudau]

On Tue, May 24, 2016 at 03:31:26PM +0200, Marek Szyprowski wrote:
> This patch removes custom code for initialization and handling of
> reserved memory regions in s5p-mfc driver and replaces it with generic
> reserved memory regions api.
> 
> s5p-mfc driver now handles two reserved memory regions defined by
> generic reserved memory bindings. Support for non-dt platform has been
> removed, because all supported platforms have been already converted to
> device tree.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  drivers/media/platform/s5p-mfc/s5p_mfc.c | 138 ++++++++++++++-----------------
>  1 file changed, 63 insertions(+), 75 deletions(-)
> 
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c b/drivers/media/platform/s5p-mfc/s5p_mfc.c
> index d1d9d388..fff5f43 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
> @@ -22,6 +22,7 @@
>  #include <media/v4l2-event.h>
>  #include <linux/workqueue.h>
>  #include <linux/of.h>
> +#include <linux/of_reserved_mem.h>
>  #include <media/videobuf2-v4l2.h>
>  #include "s5p_mfc_common.h"
>  #include "s5p_mfc_ctrl.h"
> @@ -1043,66 +1044,71 @@ static const struct v4l2_file_operations s5p_mfc_fops = {
>  	.mmap = s5p_mfc_mmap,
>  };
>  
> -static int match_child(struct device *dev, void *data)
> -{
> -	if (!dev_name(dev))
> -		return 0;
> -	return !strcmp(dev_name(dev), (char *)data);
> -}
> -
> +/* DMA memory related helper functions */
>  static void s5p_mfc_memdev_release(struct device *dev)
>  {
> -	dma_release_declared_memory(dev);
> +	of_reserved_mem_device_release(dev);
>  }
>  
> -static void *mfc_get_drv_data(struct platform_device *pdev);
> -
> -static int s5p_mfc_alloc_memdevs(struct s5p_mfc_dev *dev)
> +static struct device *s5p_mfc_alloc_memdev(struct device *dev,
> +					   const char *name, unsigned int idx)
>  {
> -	unsigned int mem_info[2] = { };
> +	struct device *child;
> +	int ret;
>  
> -	dev->mem_dev_l = devm_kzalloc(&dev->plat_dev->dev,
> -			sizeof(struct device), GFP_KERNEL);
> -	if (!dev->mem_dev_l) {
> -		mfc_err("Not enough memory\n");
> -		return -ENOMEM;
> +	child = devm_kzalloc(dev, sizeof(struct device), GFP_KERNEL);
> +	if (!child)
> +		return NULL;
> +
> +	device_initialize(child);
> +	dev_set_name(child, "%s:%s", dev_name(dev), name);
> +	child->parent = dev;
> +	child->bus = dev->bus;
> +	child->coherent_dma_mask = dev->coherent_dma_mask;
> +	child->dma_mask = dev->dma_mask;
> +	child->release = s5p_mfc_memdev_release;
> +
> +	if (device_add(child) == 0) {
> +		ret = of_reserved_mem_device_init_by_idx(child, dev->of_node,
> +							 idx);
> +		if (ret == 0)
> +			return child;
>  	}
>  
> -	dev_set_name(dev->mem_dev_l, "%s", "s5p-mfc-l");
> -	dev->mem_dev_l->release = s5p_mfc_memdev_release;
> -	device_initialize(dev->mem_dev_l);
> -	of_property_read_u32_array(dev->plat_dev->dev.of_node,
> -			"samsung,mfc-l", mem_info, 2);
> -	if (dma_declare_coherent_memory(dev->mem_dev_l, mem_info[0],
> -				mem_info[0], mem_info[1],
> -				DMA_MEMORY_MAP | DMA_MEMORY_EXCLUSIVE) == 0) {
> -		mfc_err("Failed to declare coherent memory for\n"
> -		"MFC device\n");
> -		return -ENOMEM;
> -	}
> +	put_device(child);
> +	return NULL;
> +}
>  
> -	dev->mem_dev_r = devm_kzalloc(&dev->plat_dev->dev,
> -			sizeof(struct device), GFP_KERNEL);
> -	if (!dev->mem_dev_r) {
> -		mfc_err("Not enough memory\n");
> -		return -ENOMEM;
> -	}
> +static int s5p_mfc_configure_dma_memory(struct s5p_mfc_dev *mfc_dev)
> +{
> +	struct device *dev = &mfc_dev->plat_dev->dev;
>  
> -	dev_set_name(dev->mem_dev_r, "%s", "s5p-mfc-r");
> -	dev->mem_dev_r->release = s5p_mfc_memdev_release;
> -	device_initialize(dev->mem_dev_r);
> -	of_property_read_u32_array(dev->plat_dev->dev.of_node,
> -			"samsung,mfc-r", mem_info, 2);
> -	if (dma_declare_coherent_memory(dev->mem_dev_r, mem_info[0],
> -				mem_info[0], mem_info[1],
> -				DMA_MEMORY_MAP | DMA_MEMORY_EXCLUSIVE) == 0) {
> -		pr_err("Failed to declare coherent memory for\n"
> -		"MFC device\n");
> -		return -ENOMEM;
> +	/*
> +	 * Create and initialize virtual devices for accessing
> +	 * reserved memory regions.
> +	 */
> +	mfc_dev->mem_dev_l = s5p_mfc_alloc_memdev(dev, "left",
> +						  MFC_BANK1_ALLOC_CTX);
> +	if (!mfc_dev->mem_dev_l)
> +		return -ENODEV;
> +	mfc_dev->mem_dev_r = s5p_mfc_alloc_memdev(dev, "right",
> +						  MFC_BANK2_ALLOC_CTX);
> +	if (!mfc_dev->mem_dev_r) {
> +		device_unregister(mfc_dev->mem_dev_l);
> +		return -ENODEV;
>  	}
> +
>  	return 0;
>  }
>  
> +static void s5p_mfc_unconfigure_dma_memory(struct s5p_mfc_dev *mfc_dev)
> +{
> +	device_unregister(mfc_dev->mem_dev_l);
> +	device_unregister(mfc_dev->mem_dev_r);
> +}
> +
> +static void *mfc_get_drv_data(struct platform_device *pdev);
> +
>  /* MFC probe function */
>  static int s5p_mfc_probe(struct platform_device *pdev)
>  {
> @@ -1128,12 +1134,6 @@ static int s5p_mfc_probe(struct platform_device *pdev)
>  
>  	dev->variant = mfc_get_drv_data(pdev);
>  
> -	ret = s5p_mfc_init_pm(dev);
> -	if (ret < 0) {
> -		dev_err(&pdev->dev, "failed to get mfc clock source\n");
> -		return ret;
> -	}
> -
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  
>  	dev->regs_base = devm_ioremap_resource(&pdev->dev, res);
> @@ -1154,25 +1154,16 @@ static int s5p_mfc_probe(struct platform_device *pdev)
>  		goto err_res;
>  	}
>  
> -	if (pdev->dev.of_node) {
> -		ret = s5p_mfc_alloc_memdevs(dev);
> -		if (ret < 0)
> -			goto err_res;
> -	} else {
> -		dev->mem_dev_l = device_find_child(&dev->plat_dev->dev,
> -				"s5p-mfc-l", match_child);
> -		if (!dev->mem_dev_l) {
> -			mfc_err("Mem child (L) device get failed\n");
> -			ret = -ENODEV;
> -			goto err_res;
> -		}
> -		dev->mem_dev_r = device_find_child(&dev->plat_dev->dev,
> -				"s5p-mfc-r", match_child);
> -		if (!dev->mem_dev_r) {
> -			mfc_err("Mem child (R) device get failed\n");
> -			ret = -ENODEV;
> -			goto err_res;
> -		}
> +	ret = s5p_mfc_configure_dma_memory(dev);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "failed to configure DMA memory\n");
> +		return ret;
> +	}
> +
> +	ret = s5p_mfc_init_pm(dev);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "failed to get mfc clock source\n");

Should you not be calling s5p_mfc_unconfigure_dma_memory(dev) here? It looks
like you are leaking memory.

Best regards,
Liviu


> +		return ret;
>  	}
>  
>  	vb2_dma_contig_set_max_seg_size(dev->mem_dev_l, DMA_BIT_MASK(32));
> @@ -1309,12 +1300,9 @@ static int s5p_mfc_remove(struct platform_device *pdev)
>  	s5p_mfc_release_firmware(dev);
>  	vb2_dma_contig_cleanup_ctx(dev->alloc_ctx[0]);
>  	vb2_dma_contig_cleanup_ctx(dev->alloc_ctx[1]);
> +	s5p_mfc_unconfigure_dma_memory(dev);
>  	vb2_dma_contig_clear_max_seg_size(dev->mem_dev_l);
>  	vb2_dma_contig_clear_max_seg_size(dev->mem_dev_r);
> -	if (pdev->dev.of_node) {
> -		put_device(dev->mem_dev_l);
> -		put_device(dev->mem_dev_r);
> -	}
>  
>  	s5p_mfc_final_pm(dev);
>  	return 0;
> -- 
> 1.9.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] media: s5p-mfc: fix error path in driver probe

[Marek Szyprowski]

This patch fixes the error path in the driver probe, so in case of
any failure, the resources are not leaked.

Reported-by: Liviu Dudau <liviu.dudau@arm.com>
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 drivers/media/platform/s5p-mfc/s5p_mfc.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c b/drivers/media/platform/s5p-mfc/s5p_mfc.c
index 6ee620ee8cd5..1f3a7ee753db 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
@@ -1159,7 +1159,10 @@ static int s5p_mfc_probe(struct platform_device *pdev)
 	dev->variant = mfc_get_drv_data(pdev);
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-
+	if (res == NULL) {
+		dev_err(&pdev->dev, "failed to get io resource\n");
+		return -ENOENT;
+	}
 	dev->regs_base = devm_ioremap_resource(&pdev->dev, res);
 	if (IS_ERR(dev->regs_base))
 		return PTR_ERR(dev->regs_base);
@@ -1167,15 +1170,14 @@ static int s5p_mfc_probe(struct platform_device *pdev)
 	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
 	if (res == NULL) {
 		dev_err(&pdev->dev, "failed to get irq resource\n");
-		ret = -ENOENT;
-		goto err_res;
+		return -ENOENT;
 	}
 	dev->irq = res->start;
 	ret = devm_request_irq(&pdev->dev, dev->irq, s5p_mfc_irq,
 					0, pdev->name, dev);
 	if (ret) {
 		dev_err(&pdev->dev, "Failed to install irq (%d)\n", ret);
-		goto err_res;
+		return ret;
 	}
 
 	ret = s5p_mfc_configure_dma_memory(dev);
@@ -1187,7 +1189,7 @@ static int s5p_mfc_probe(struct platform_device *pdev)
 	ret = s5p_mfc_init_pm(dev);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "failed to get mfc clock source\n");
-		return ret;
+		goto err_dma;
 	}
 
 	vb2_dma_contig_set_max_seg_size(dev->mem_dev_l, DMA_BIT_MASK(32));
@@ -1301,6 +1303,8 @@ err_mem_init_ctx_1:
 	vb2_dma_contig_cleanup_ctx(dev->alloc_ctx[0]);
 err_res:
 	s5p_mfc_final_pm(dev);
+err_dma:
+	s5p_mfc_unconfigure_dma_memory(dev);
 
 	pr_debug("%s-- with error\n", __func__);
 	return ret;
-- 
1.9.2

Re: [PATCH] media: s5p-mfc: fix error path in driver probe

[Liviu Dudau]

On Wed, Jun 08, 2016 at 01:33:40PM +0200, Marek Szyprowski wrote:
> This patch fixes the error path in the driver probe, so in case of
> any failure, the resources are not leaked.
> 
> Reported-by: Liviu Dudau <liviu.dudau@arm.com>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>

Looks good to me now! If it is useful:

Acked-by: Liviu Dudau <Liviu.Dudau@arm.com>

> ---
>  drivers/media/platform/s5p-mfc/s5p_mfc.c | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c b/drivers/media/platform/s5p-mfc/s5p_mfc.c
> index 6ee620ee8cd5..1f3a7ee753db 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
> @@ -1159,7 +1159,10 @@ static int s5p_mfc_probe(struct platform_device *pdev)
>  	dev->variant = mfc_get_drv_data(pdev);
>  
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -
> +	if (res == NULL) {
> +		dev_err(&pdev->dev, "failed to get io resource\n");
> +		return -ENOENT;
> +	}
>  	dev->regs_base = devm_ioremap_resource(&pdev->dev, res);
>  	if (IS_ERR(dev->regs_base))
>  		return PTR_ERR(dev->regs_base);
> @@ -1167,15 +1170,14 @@ static int s5p_mfc_probe(struct platform_device *pdev)
>  	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
>  	if (res == NULL) {
>  		dev_err(&pdev->dev, "failed to get irq resource\n");
> -		ret = -ENOENT;
> -		goto err_res;
> +		return -ENOENT;
>  	}
>  	dev->irq = res->start;
>  	ret = devm_request_irq(&pdev->dev, dev->irq, s5p_mfc_irq,
>  					0, pdev->name, dev);
>  	if (ret) {
>  		dev_err(&pdev->dev, "Failed to install irq (%d)\n", ret);
> -		goto err_res;
> +		return ret;
>  	}
>  
>  	ret = s5p_mfc_configure_dma_memory(dev);
> @@ -1187,7 +1189,7 @@ static int s5p_mfc_probe(struct platform_device *pdev)
>  	ret = s5p_mfc_init_pm(dev);
>  	if (ret < 0) {
>  		dev_err(&pdev->dev, "failed to get mfc clock source\n");
> -		return ret;
> +		goto err_dma;
>  	}
>  
>  	vb2_dma_contig_set_max_seg_size(dev->mem_dev_l, DMA_BIT_MASK(32));
> @@ -1301,6 +1303,8 @@ err_mem_init_ctx_1:
>  	vb2_dma_contig_cleanup_ctx(dev->alloc_ctx[0]);
>  err_res:
>  	s5p_mfc_final_pm(dev);
> +err_dma:
> +	s5p_mfc_unconfigure_dma_memory(dev);
>  
>  	pr_debug("%s-- with error\n", __func__);
>  	return ret;
> -- 
> 1.9.2
> 

-- 
-------------------
   .oooO
   (   )
    \ (  Oooo.
     \_) (   )
          ) /
         (_/

 One small step
   for me ...

Re: [PATCH] media: s5p-mfc: fix error path in driver probe

[Shuah Khan]

Hi Marek,

On 06/08/2016 08:35 AM, Liviu Dudau wrote:
> On Wed, Jun 08, 2016 at 01:33:40PM +0200, Marek Szyprowski wrote:
>> This patch fixes the error path in the driver probe, so in case of
>> any failure, the resources are not leaked.
>>
>> Reported-by: Liviu Dudau <liviu.dudau@arm.com>
>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> 
> Looks good to me now! If it is useful:
> 
> Acked-by: Liviu Dudau <Liviu.Dudau@arm.com>
> 
>> ---
>>  drivers/media/platform/s5p-mfc/s5p_mfc.c | 14 +++++++++-----
>>  1 file changed, 9 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c b/drivers/media/platform/s5p-mfc/s5p_mfc.c
>> index 6ee620ee8cd5..1f3a7ee753db 100644
>> --- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
>> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
>> @@ -1159,7 +1159,10 @@ static int s5p_mfc_probe(struct platform_device *pdev)
>>  	dev->variant = mfc_get_drv_data(pdev);
>>  
>>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> -
>> +	if (res == NULL) {
>> +		dev_err(&pdev->dev, "failed to get io resource\n");
>> +		return -ENOENT;
>> +	}
>>  	dev->regs_base = devm_ioremap_resource(&pdev->dev, res);
>>  	if (IS_ERR(dev->regs_base))
>>  		return PTR_ERR(dev->regs_base);
>> @@ -1167,15 +1170,14 @@ static int s5p_mfc_probe(struct platform_device *pdev)
>>  	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
>>  	if (res == NULL) {
>>  		dev_err(&pdev->dev, "failed to get irq resource\n");
>> -		ret = -ENOENT;
>> -		goto err_res;
>> +		return -ENOENT;
>>  	}
>>  	dev->irq = res->start;
>>  	ret = devm_request_irq(&pdev->dev, dev->irq, s5p_mfc_irq,
>>  					0, pdev->name, dev);
>>  	if (ret) {
>>  		dev_err(&pdev->dev, "Failed to install irq (%d)\n", ret);
>> -		goto err_res;
>> +		return ret;
>>  	}
>>  
>>  	ret = s5p_mfc_configure_dma_memory(dev);
>> @@ -1187,7 +1189,7 @@ static int s5p_mfc_probe(struct platform_device *pdev)
>>  	ret = s5p_mfc_init_pm(dev);
>>  	if (ret < 0) {
>>  		dev_err(&pdev->dev, "failed to get mfc clock source\n");
>> -		return ret;
>> +		goto err_dma;
>>  	}
>>  
>>  	vb2_dma_contig_set_max_seg_size(dev->mem_dev_l, DMA_BIT_MASK(32));
>> @@ -1301,6 +1303,8 @@ err_mem_init_ctx_1:
>>  	vb2_dma_contig_cleanup_ctx(dev->alloc_ctx[0]);
>>  err_res:

While you are at it, could you change this to err_pm. err_res doesn't
match the actual failure.

thanks,
-- Shuah

>>  	s5p_mfc_final_pm(dev);
>> +err_dma:
>> +	s5p_mfc_unconfigure_dma_memory(dev);
>>  
>>  	pr_debug("%s-- with error\n", __func__);
>>  	return ret;
>> -- 
>> 1.9.2
>>
>