[PATCH v2 00/15] Exynos MFC v6+ - remove the need for the reserved memory

[PATCH v2 00/15] Exynos MFC v6+ - remove the need for the reserved memory

[Marek Szyprowski]

Dear All,

This patchset is a result of my work on enabling full support for MFC device
(multimedia codec) on Exynos 5433 on ARM64 architecture. Initially I thought
that to let it working on ARM64 architecture with IOMMU, I would need to
solve the issue related to the fact that s5p-mfc driver was depending on the
first-fit allocation method in the DMA-mapping / IOMMU glue code (ARM64 use
different algorithm). It turned out, that there is a much simpler way.

During my research I found that some of the requirements for the memory
buffers for MFC v6+ devices were blindly copied from the previous
hardware (v5) version and simply turned out to be excessive. It turned out
that there is no strict requirement for ALL buffers to be allocated on
the higher addresses than the firmware base. This requirement is true only
for the device and per-context buffers. All video data buffers can be
allocated anywhere for all MFC v6+ versions. This heavily simplifies
memory management in the driver.

Such relaxed requirements for the memory buffers can be easily fulfilled
by allocating firmware, device and per-context buffers from the probe-time
preallocated larger buffer. There is no need to create special reserved
memory regions. The only case, when those memory regions are needed is an
oldest Exynos series - Exynos4210 or Exyno4412, which both have MFC v5
hardware, and only when IOMMU is disabled.

This patchset has been tested on Odroid U3 (Exynos4412 with MFC v5), Google
Snow (Exynos5250 with MFC v6), Odroid XU3 (Exynos5422 with MFC v8) and
TM2 (Exynos5433 with MFC v8, ARM64) boards.

To get it working on TM2/Exynos5433 with IOMMU enabled, the 'architectural
clock gating' in SYSMMU has to be disabled. Fixing this will be handled
separately. As a temporary solution, one need to clear CFG_ACGEN bit in
REG_MMU_CFG of the SYSMMU, see __sysmmu_init_config function in
drivers/iommu/exynos-iommu.c.

Patches are based on linux-next from 20th February 2017 with "media:
s5p-mfc: Fix initialization of internal structures" patch applied:
https://patchwork.linuxtv.org/patch/39198/

I've tried to split changes into small pieces to make it easier to review
the code. I've also did a bit of cleanup while touching the driver.

Best regards
Marek Szyprowski
Samsung R&D Institute Poland


Changelog:

v2:
- fixed issues pointed by Javier Martinez Canillas: code compiles now
  after applying each patch, added missing cleanup
- added tags

v1: https://www.spinics.net/lists/linux-media/msg111156.html
- initial version


Patch summary:

Marek Szyprowski (15):
  media: s5p-mfc: Remove unused structures and dead code
  media: s5p-mfc: Use generic of_device_get_match_data helper
  media: s5p-mfc: Replace mem_dev_* entries with an array
  media: s5p-mfc: Replace bank1/bank2 entries with an array
  media: s5p-mfc: Simplify alloc/release private buffer functions
  media: s5p-mfc: Move setting DMA max segment size to DMA configure
    function
  media: s5p-mfc: Put firmware to private buffer structure
  media: s5p-mfc: Move firmware allocation to DMA configure function
  media: s5p-mfc: Allocate firmware with internal private buffer alloc
    function
  media: s5p-mfc: Reduce firmware buffer size for MFC v6+ variants
  media: s5p-mfc: Split variant DMA memory configuration into separate
    functions
  media: s5p-mfc: Add support for probe-time preallocated block based
    allocator
  media: s5p-mfc: Remove special configuration of IOMMU domain
  media: s5p-mfc: Use preallocated block allocator always for MFC v6+
  ARM: dts: exynos: Remove MFC reserved buffers

 .../devicetree/bindings/media/s5p-mfc.txt          |   2 +-
 arch/arm/boot/dts/exynos5250-arndale.dts           |   1 -
 arch/arm/boot/dts/exynos5250-smdk5250.dts          |   1 -
 arch/arm/boot/dts/exynos5250-spring.dts            |   1 -
 arch/arm/boot/dts/exynos5420-arndale-octa.dts      |   1 -
 arch/arm/boot/dts/exynos5420-peach-pit.dts         |   1 -
 arch/arm/boot/dts/exynos5420-smdk5420.dts          |   1 -
 arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi |   1 -
 arch/arm/boot/dts/exynos5800-peach-pi.dts          |   1 -
 drivers/media/platform/s5p-mfc/regs-mfc-v6.h       |   2 +-
 drivers/media/platform/s5p-mfc/regs-mfc-v7.h       |   2 +-
 drivers/media/platform/s5p-mfc/regs-mfc-v8.h       |   2 +-
 drivers/media/platform/s5p-mfc/s5p_mfc.c           | 214 +++++++++++++--------
 drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v5.c    |   2 +-
 drivers/media/platform/s5p-mfc/s5p_mfc_common.h    |  43 ++---
 drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c      |  71 ++-----
 drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.h      |   1 -
 drivers/media/platform/s5p-mfc/s5p_mfc_dec.c       |   8 +-
 drivers/media/platform/s5p-mfc/s5p_mfc_enc.c       |  10 +-
 drivers/media/platform/s5p-mfc/s5p_mfc_iommu.h     |  51 +----
 drivers/media/platform/s5p-mfc/s5p_mfc_opr.c       |  65 +++++--
 drivers/media/platform/s5p-mfc/s5p_mfc_opr.h       |   8 +-
 drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c    |  48 ++---
 drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c    |  14 +-
 24 files changed, 268 insertions(+), 283 deletions(-)

-- 
1.9.1

[PATCH v2 01/15] media: s5p-mfc: Remove unused structures and dead code

[Marek Szyprowski]

Remove unused structures, definitions and functions that are no longer
called from the driver code.

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>
---
 drivers/media/platform/s5p-mfc/s5p_mfc.c        | 21 ---------------------
 drivers/media/platform/s5p-mfc/s5p_mfc_common.h | 13 -------------
 drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.h   |  1 -
 3 files changed, 35 deletions(-)

diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c b/drivers/media/platform/s5p-mfc/s5p_mfc.c
index 05fe82be6584..3e1f22eb4339 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
@@ -1422,16 +1422,11 @@ static int s5p_mfc_resume(struct device *dev)
 	.priv	= &mfc_buf_size_v5,
 };
 
-static struct s5p_mfc_buf_align mfc_buf_align_v5 = {
-	.base = MFC_BASE_ALIGN_ORDER,
-};
-
 static struct s5p_mfc_variant mfc_drvdata_v5 = {
 	.version	= MFC_VERSION,
 	.version_bit	= MFC_V5_BIT,
 	.port_num	= MFC_NUM_PORTS,
 	.buf_size	= &buf_size_v5,
-	.buf_align	= &mfc_buf_align_v5,
 	.fw_name[0]	= "s5p-mfc.fw",
 	.clk_names	= {"mfc", "sclk_mfc"},
 	.num_clocks	= 2,
@@ -1452,16 +1447,11 @@ static int s5p_mfc_resume(struct device *dev)
 	.priv	= &mfc_buf_size_v6,
 };
 
-static struct s5p_mfc_buf_align mfc_buf_align_v6 = {
-	.base = 0,
-};
-
 static struct s5p_mfc_variant mfc_drvdata_v6 = {
 	.version	= MFC_VERSION_V6,
 	.version_bit	= MFC_V6_BIT,
 	.port_num	= MFC_NUM_PORTS_V6,
 	.buf_size	= &buf_size_v6,
-	.buf_align	= &mfc_buf_align_v6,
 	.fw_name[0]     = "s5p-mfc-v6.fw",
 	/*
 	 * v6-v2 firmware contains bug fixes and interface change
@@ -1486,16 +1476,11 @@ static int s5p_mfc_resume(struct device *dev)
 	.priv	= &mfc_buf_size_v7,
 };
 
-static struct s5p_mfc_buf_align mfc_buf_align_v7 = {
-	.base = 0,
-};
-
 static struct s5p_mfc_variant mfc_drvdata_v7 = {
 	.version	= MFC_VERSION_V7,
 	.version_bit	= MFC_V7_BIT,
 	.port_num	= MFC_NUM_PORTS_V7,
 	.buf_size	= &buf_size_v7,
-	.buf_align	= &mfc_buf_align_v7,
 	.fw_name[0]     = "s5p-mfc-v7.fw",
 	.clk_names	= {"mfc", "sclk_mfc"},
 	.num_clocks	= 2,
@@ -1515,16 +1500,11 @@ static int s5p_mfc_resume(struct device *dev)
 	.priv	= &mfc_buf_size_v8,
 };
 
-static struct s5p_mfc_buf_align mfc_buf_align_v8 = {
-	.base = 0,
-};
-
 static struct s5p_mfc_variant mfc_drvdata_v8 = {
 	.version	= MFC_VERSION_V8,
 	.version_bit	= MFC_V8_BIT,
 	.port_num	= MFC_NUM_PORTS_V8,
 	.buf_size	= &buf_size_v8,
-	.buf_align	= &mfc_buf_align_v8,
 	.fw_name[0]     = "s5p-mfc-v8.fw",
 	.clk_names	= {"mfc"},
 	.num_clocks	= 1,
@@ -1535,7 +1515,6 @@ static int s5p_mfc_resume(struct device *dev)
 	.version_bit	= MFC_V8_BIT,
 	.port_num	= MFC_NUM_PORTS_V8,
 	.buf_size	= &buf_size_v8,
-	.buf_align	= &mfc_buf_align_v8,
 	.fw_name[0]     = "s5p-mfc-v8.fw",
 	.clk_names	= {"pclk", "aclk", "aclk_xiu"},
 	.num_clocks	= 3,
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
index ab23236aa942..3e0e8eaf8bfe 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
@@ -44,14 +44,6 @@
 
 #include <media/videobuf2-dma-contig.h>
 
-static inline dma_addr_t s5p_mfc_mem_cookie(void *a, void *b)
-{
-	/* Same functionality as the vb2_dma_contig_plane_paddr */
-	dma_addr_t *paddr = vb2_dma_contig_memops.cookie(b);
-
-	return *paddr;
-}
-
 /* MFC definitions */
 #define MFC_MAX_EXTRA_DPB       5
 #define MFC_MAX_BUFFERS		32
@@ -229,16 +221,11 @@ struct s5p_mfc_buf_size {
 	void *priv;
 };
 
-struct s5p_mfc_buf_align {
-	unsigned int base;
-};
-
 struct s5p_mfc_variant {
 	unsigned int version;
 	unsigned int port_num;
 	u32 version_bit;
 	struct s5p_mfc_buf_size *buf_size;
-	struct s5p_mfc_buf_align *buf_align;
 	char	*fw_name[MFC_FW_MAX_VERSIONS];
 	const char	*clk_names[MFC_MAX_CLOCKS];
 	int		num_clocks;
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.h b/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.h
index 8e5df041edf7..45c807bf19cc 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.h
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.h
@@ -18,7 +18,6 @@
 int s5p_mfc_release_firmware(struct s5p_mfc_dev *dev);
 int s5p_mfc_alloc_firmware(struct s5p_mfc_dev *dev);
 int s5p_mfc_load_firmware(struct s5p_mfc_dev *dev);
-int s5p_mfc_reload_firmware(struct s5p_mfc_dev *dev);
 
 int s5p_mfc_init_hw(struct s5p_mfc_dev *dev);
 void s5p_mfc_deinit_hw(struct s5p_mfc_dev *dev);
-- 
1.9.1

Re: [PATCH v2 01/15] media: s5p-mfc: Remove unused structures and dead code

[Andrzej Hajda]

On 20.02.2017 14:38, Marek Szyprowski wrote:
> Remove unused structures, definitions and functions that are no longer
> called from the driver code.
>
> 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>

For the whole patchset:

Acked-by: Andrzej Hajda <a.hajda@samsung.com>
--
Regards
Andrzej


> ---
>  drivers/media/platform/s5p-mfc/s5p_mfc.c        | 21 ---------------------
>  drivers/media/platform/s5p-mfc/s5p_mfc_common.h | 13 -------------
>  drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.h   |  1 -
>  3 files changed, 35 deletions(-)
>
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c b/drivers/media/platform/s5p-mfc/s5p_mfc.c
> index 05fe82be6584..3e1f22eb4339 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
> @@ -1422,16 +1422,11 @@ static int s5p_mfc_resume(struct device *dev)
>  	.priv	= &mfc_buf_size_v5,
>  };
>  
> -static struct s5p_mfc_buf_align mfc_buf_align_v5 = {
> -	.base = MFC_BASE_ALIGN_ORDER,
> -};
> -
>  static struct s5p_mfc_variant mfc_drvdata_v5 = {
>  	.version	= MFC_VERSION,
>  	.version_bit	= MFC_V5_BIT,
>  	.port_num	= MFC_NUM_PORTS,
>  	.buf_size	= &buf_size_v5,
> -	.buf_align	= &mfc_buf_align_v5,
>  	.fw_name[0]	= "s5p-mfc.fw",
>  	.clk_names	= {"mfc", "sclk_mfc"},
>  	.num_clocks	= 2,
> @@ -1452,16 +1447,11 @@ static int s5p_mfc_resume(struct device *dev)
>  	.priv	= &mfc_buf_size_v6,
>  };
>  
> -static struct s5p_mfc_buf_align mfc_buf_align_v6 = {
> -	.base = 0,
> -};
> -
>  static struct s5p_mfc_variant mfc_drvdata_v6 = {
>  	.version	= MFC_VERSION_V6,
>  	.version_bit	= MFC_V6_BIT,
>  	.port_num	= MFC_NUM_PORTS_V6,
>  	.buf_size	= &buf_size_v6,
> -	.buf_align	= &mfc_buf_align_v6,
>  	.fw_name[0]     = "s5p-mfc-v6.fw",
>  	/*
>  	 * v6-v2 firmware contains bug fixes and interface change
> @@ -1486,16 +1476,11 @@ static int s5p_mfc_resume(struct device *dev)
>  	.priv	= &mfc_buf_size_v7,
>  };
>  
> -static struct s5p_mfc_buf_align mfc_buf_align_v7 = {
> -	.base = 0,
> -};
> -
>  static struct s5p_mfc_variant mfc_drvdata_v7 = {
>  	.version	= MFC_VERSION_V7,
>  	.version_bit	= MFC_V7_BIT,
>  	.port_num	= MFC_NUM_PORTS_V7,
>  	.buf_size	= &buf_size_v7,
> -	.buf_align	= &mfc_buf_align_v7,
>  	.fw_name[0]     = "s5p-mfc-v7.fw",
>  	.clk_names	= {"mfc", "sclk_mfc"},
>  	.num_clocks	= 2,
> @@ -1515,16 +1500,11 @@ static int s5p_mfc_resume(struct device *dev)
>  	.priv	= &mfc_buf_size_v8,
>  };
>  
> -static struct s5p_mfc_buf_align mfc_buf_align_v8 = {
> -	.base = 0,
> -};
> -
>  static struct s5p_mfc_variant mfc_drvdata_v8 = {
>  	.version	= MFC_VERSION_V8,
>  	.version_bit	= MFC_V8_BIT,
>  	.port_num	= MFC_NUM_PORTS_V8,
>  	.buf_size	= &buf_size_v8,
> -	.buf_align	= &mfc_buf_align_v8,
>  	.fw_name[0]     = "s5p-mfc-v8.fw",
>  	.clk_names	= {"mfc"},
>  	.num_clocks	= 1,
> @@ -1535,7 +1515,6 @@ static int s5p_mfc_resume(struct device *dev)
>  	.version_bit	= MFC_V8_BIT,
>  	.port_num	= MFC_NUM_PORTS_V8,
>  	.buf_size	= &buf_size_v8,
> -	.buf_align	= &mfc_buf_align_v8,
>  	.fw_name[0]     = "s5p-mfc-v8.fw",
>  	.clk_names	= {"pclk", "aclk", "aclk_xiu"},
>  	.num_clocks	= 3,
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
> index ab23236aa942..3e0e8eaf8bfe 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
> @@ -44,14 +44,6 @@
>  
>  #include <media/videobuf2-dma-contig.h>
>  
> -static inline dma_addr_t s5p_mfc_mem_cookie(void *a, void *b)
> -{
> -	/* Same functionality as the vb2_dma_contig_plane_paddr */
> -	dma_addr_t *paddr = vb2_dma_contig_memops.cookie(b);
> -
> -	return *paddr;
> -}
> -
>  /* MFC definitions */
>  #define MFC_MAX_EXTRA_DPB       5
>  #define MFC_MAX_BUFFERS		32
> @@ -229,16 +221,11 @@ struct s5p_mfc_buf_size {
>  	void *priv;
>  };
>  
> -struct s5p_mfc_buf_align {
> -	unsigned int base;
> -};
> -
>  struct s5p_mfc_variant {
>  	unsigned int version;
>  	unsigned int port_num;
>  	u32 version_bit;
>  	struct s5p_mfc_buf_size *buf_size;
> -	struct s5p_mfc_buf_align *buf_align;
>  	char	*fw_name[MFC_FW_MAX_VERSIONS];
>  	const char	*clk_names[MFC_MAX_CLOCKS];
>  	int		num_clocks;
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.h b/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.h
> index 8e5df041edf7..45c807bf19cc 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.h
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.h
> @@ -18,7 +18,6 @@
>  int s5p_mfc_release_firmware(struct s5p_mfc_dev *dev);
>  int s5p_mfc_alloc_firmware(struct s5p_mfc_dev *dev);
>  int s5p_mfc_load_firmware(struct s5p_mfc_dev *dev);
> -int s5p_mfc_reload_firmware(struct s5p_mfc_dev *dev);
>  
>  int s5p_mfc_init_hw(struct s5p_mfc_dev *dev);
>  void s5p_mfc_deinit_hw(struct s5p_mfc_dev *dev);

Re: [v2,01/15] media: s5p-mfc: Remove unused structures and dead code

[Smitha T Murthy]

On Mon, 2017-02-20 at 14:38 +0100, Marek Szyprowski wrote:
> Remove unused structures, definitions and functions that are no longer
> called from the driver code.
> 
> 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>
> Acked-by: Andrzej Hajda <a.hajda@samsung.com>

Hi Marek,
I tested this complete series on Exynos7880, along with my MFC v10.10
patchset v2 [1] and it's working fine.

Please review MFC v10.10 series. 

Tested-by: Smitha T Murthy <smitha.t@samsung.com>

[1]: https://www.spinics.net/lists/arm-kernel/msg566019.html

Thanks,
Smitha T Murthy

Re: [v2,01/15] media: s5p-mfc: Remove unused structures and dead code

[Smitha T Murthy]

On Mon, 2017-02-20 at 14:38 +0100, Marek Szyprowski wrote:
> Remove unused structures, definitions and functions that are no longer
> called from the driver code.
> 
> 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>
> Acked-by: Andrzej Hajda <a.hajda@samsung.com>

Reviewed-by: Smitha T Murthy <smitha.t@samsung.com>

Regards,
Smitha T Murthy
> ---
>  drivers/media/platform/s5p-mfc/s5p_mfc.c        | 21 ---------------------
>  drivers/media/platform/s5p-mfc/s5p_mfc_common.h | 13 -------------
>  drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.h   |  1 -
>  3 files changed, 35 deletions(-)
> 
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c b/drivers/media/platform/s5p-mfc/s5p_mfc.c
> index 05fe82be6584..3e1f22eb4339 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
> @@ -1422,16 +1422,11 @@ static int s5p_mfc_resume(struct device *dev)
>  	.priv	= &mfc_buf_size_v5,
>  };
>  
> -static struct s5p_mfc_buf_align mfc_buf_align_v5 = {
> -	.base = MFC_BASE_ALIGN_ORDER,
> -};
> -
>  static struct s5p_mfc_variant mfc_drvdata_v5 = {
>  	.version	= MFC_VERSION,
>  	.version_bit	= MFC_V5_BIT,
>  	.port_num	= MFC_NUM_PORTS,
>  	.buf_size	= &buf_size_v5,
> -	.buf_align	= &mfc_buf_align_v5,
>  	.fw_name[0]	= "s5p-mfc.fw",
>  	.clk_names	= {"mfc", "sclk_mfc"},
>  	.num_clocks	= 2,
> @@ -1452,16 +1447,11 @@ static int s5p_mfc_resume(struct device *dev)
>  	.priv	= &mfc_buf_size_v6,
>  };
>  
> -static struct s5p_mfc_buf_align mfc_buf_align_v6 = {
> -	.base = 0,
> -};
> -
>  static struct s5p_mfc_variant mfc_drvdata_v6 = {
>  	.version	= MFC_VERSION_V6,
>  	.version_bit	= MFC_V6_BIT,
>  	.port_num	= MFC_NUM_PORTS_V6,
>  	.buf_size	= &buf_size_v6,
> -	.buf_align	= &mfc_buf_align_v6,
>  	.fw_name[0]     = "s5p-mfc-v6.fw",
>  	/*
>  	 * v6-v2 firmware contains bug fixes and interface change
> @@ -1486,16 +1476,11 @@ static int s5p_mfc_resume(struct device *dev)
>  	.priv	= &mfc_buf_size_v7,
>  };
>  
> -static struct s5p_mfc_buf_align mfc_buf_align_v7 = {
> -	.base = 0,
> -};
> -
>  static struct s5p_mfc_variant mfc_drvdata_v7 = {
>  	.version	= MFC_VERSION_V7,
>  	.version_bit	= MFC_V7_BIT,
>  	.port_num	= MFC_NUM_PORTS_V7,
>  	.buf_size	= &buf_size_v7,
> -	.buf_align	= &mfc_buf_align_v7,
>  	.fw_name[0]     = "s5p-mfc-v7.fw",
>  	.clk_names	= {"mfc", "sclk_mfc"},
>  	.num_clocks	= 2,
> @@ -1515,16 +1500,11 @@ static int s5p_mfc_resume(struct device *dev)
>  	.priv	= &mfc_buf_size_v8,
>  };
>  
> -static struct s5p_mfc_buf_align mfc_buf_align_v8 = {
> -	.base = 0,
> -};
> -
>  static struct s5p_mfc_variant mfc_drvdata_v8 = {
>  	.version	= MFC_VERSION_V8,
>  	.version_bit	= MFC_V8_BIT,
>  	.port_num	= MFC_NUM_PORTS_V8,
>  	.buf_size	= &buf_size_v8,
> -	.buf_align	= &mfc_buf_align_v8,
>  	.fw_name[0]     = "s5p-mfc-v8.fw",
>  	.clk_names	= {"mfc"},
>  	.num_clocks	= 1,
> @@ -1535,7 +1515,6 @@ static int s5p_mfc_resume(struct device *dev)
>  	.version_bit	= MFC_V8_BIT,
>  	.port_num	= MFC_NUM_PORTS_V8,
>  	.buf_size	= &buf_size_v8,
> -	.buf_align	= &mfc_buf_align_v8,
>  	.fw_name[0]     = "s5p-mfc-v8.fw",
>  	.clk_names	= {"pclk", "aclk", "aclk_xiu"},
>  	.num_clocks	= 3,
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
> index ab23236aa942..3e0e8eaf8bfe 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
> @@ -44,14 +44,6 @@
>  
>  #include <media/videobuf2-dma-contig.h>
>  
> -static inline dma_addr_t s5p_mfc_mem_cookie(void *a, void *b)
> -{
> -	/* Same functionality as the vb2_dma_contig_plane_paddr */
> -	dma_addr_t *paddr = vb2_dma_contig_memops.cookie(b);
> -
> -	return *paddr;
> -}
> -
>  /* MFC definitions */
>  #define MFC_MAX_EXTRA_DPB       5
>  #define MFC_MAX_BUFFERS		32
> @@ -229,16 +221,11 @@ struct s5p_mfc_buf_size {
>  	void *priv;
>  };
>  
> -struct s5p_mfc_buf_align {
> -	unsigned int base;
> -};
> -
>  struct s5p_mfc_variant {
>  	unsigned int version;
>  	unsigned int port_num;
>  	u32 version_bit;
>  	struct s5p_mfc_buf_size *buf_size;
> -	struct s5p_mfc_buf_align *buf_align;
>  	char	*fw_name[MFC_FW_MAX_VERSIONS];
>  	const char	*clk_names[MFC_MAX_CLOCKS];
>  	int		num_clocks;
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.h b/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.h
> index 8e5df041edf7..45c807bf19cc 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.h
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.h
> @@ -18,7 +18,6 @@
>  int s5p_mfc_release_firmware(struct s5p_mfc_dev *dev);
>  int s5p_mfc_alloc_firmware(struct s5p_mfc_dev *dev);
>  int s5p_mfc_load_firmware(struct s5p_mfc_dev *dev);
> -int s5p_mfc_reload_firmware(struct s5p_mfc_dev *dev);
>  
>  int s5p_mfc_init_hw(struct s5p_mfc_dev *dev);
>  void s5p_mfc_deinit_hw(struct s5p_mfc_dev *dev);

[PATCH v2 02/15] media: s5p-mfc: Use generic of_device_get_match_data helper

[Marek Szyprowski]

Replace custom code with generic helper to retrieve driver data.

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>
---
 drivers/media/platform/s5p-mfc/s5p_mfc.c        | 17 ++---------------
 drivers/media/platform/s5p-mfc/s5p_mfc_common.h |  4 ++--
 2 files changed, 4 insertions(+), 17 deletions(-)

diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c b/drivers/media/platform/s5p-mfc/s5p_mfc.c
index 3e1f22eb4339..ad3d7377f40d 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_device.h>
 #include <linux/of_reserved_mem.h>
 #include <media/videobuf2-v4l2.h>
 #include "s5p_mfc_common.h"
@@ -1157,8 +1158,6 @@ static void s5p_mfc_unconfigure_dma_memory(struct s5p_mfc_dev *mfc_dev)
 	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)
 {
@@ -1182,7 +1181,7 @@ static int s5p_mfc_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
-	dev->variant = mfc_get_drv_data(pdev);
+	dev->variant = of_device_get_match_data(&pdev->dev);
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	dev->regs_base = devm_ioremap_resource(&pdev->dev, res);
@@ -1541,18 +1540,6 @@ static int s5p_mfc_resume(struct device *dev)
 };
 MODULE_DEVICE_TABLE(of, exynos_mfc_match);
 
-static void *mfc_get_drv_data(struct platform_device *pdev)
-{
-	struct s5p_mfc_variant *driver_data = NULL;
-	const struct of_device_id *match;
-
-	match = of_match_node(exynos_mfc_match, pdev->dev.of_node);
-	if (match)
-		driver_data = (struct s5p_mfc_variant *)match->data;
-
-	return driver_data;
-}
-
 static struct platform_driver s5p_mfc_driver = {
 	.probe		= s5p_mfc_probe,
 	.remove		= s5p_mfc_remove,
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
index 3e0e8eaf8bfe..2f1387a4c386 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
@@ -192,7 +192,7 @@ struct s5p_mfc_buf {
  */
 struct s5p_mfc_pm {
 	struct clk	*clock_gate;
-	const char	**clk_names;
+	const char * const *clk_names;
 	struct clk	*clocks[MFC_MAX_CLOCKS];
 	int		num_clocks;
 	bool		use_clock_gating;
@@ -304,7 +304,7 @@ struct s5p_mfc_dev {
 	struct v4l2_ctrl_handler dec_ctrl_handler;
 	struct v4l2_ctrl_handler enc_ctrl_handler;
 	struct s5p_mfc_pm	pm;
-	struct s5p_mfc_variant	*variant;
+	const struct s5p_mfc_variant	*variant;
 	int num_inst;
 	spinlock_t irqlock;	/* lock when operating on context */
 	spinlock_t condlock;	/* lock when changing/checking if a context is
-- 
1.9.1

Re: [v2, 02/15] media: s5p-mfc: Use generic of_device_get_match_data helper

[Smitha T Murthy]

On Mon, 2017-02-20 at 14:38 +0100, Marek Szyprowski wrote: 
> Replace custom code with generic helper to retrieve driver data.
> 
> 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>

Reviewed-by: Smitha T Murthy <smitha.t@samsung.com>

Regards,
Smitha T Murthy

> ---
>  drivers/media/platform/s5p-mfc/s5p_mfc.c        | 17 ++---------------
>  drivers/media/platform/s5p-mfc/s5p_mfc_common.h |  4 ++--
>  2 files changed, 4 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c b/drivers/media/platform/s5p-mfc/s5p_mfc.c
> index 3e1f22eb4339..ad3d7377f40d 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_device.h>
>  #include <linux/of_reserved_mem.h>
>  #include <media/videobuf2-v4l2.h>
>  #include "s5p_mfc_common.h"
> @@ -1157,8 +1158,6 @@ static void s5p_mfc_unconfigure_dma_memory(struct s5p_mfc_dev *mfc_dev)
>  	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)
>  {
> @@ -1182,7 +1181,7 @@ static int s5p_mfc_probe(struct platform_device *pdev)
>  		return -ENODEV;
>  	}
>  
> -	dev->variant = mfc_get_drv_data(pdev);
> +	dev->variant = of_device_get_match_data(&pdev->dev);
>  
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	dev->regs_base = devm_ioremap_resource(&pdev->dev, res);
> @@ -1541,18 +1540,6 @@ static int s5p_mfc_resume(struct device *dev)
>  };
>  MODULE_DEVICE_TABLE(of, exynos_mfc_match);
>  
> -static void *mfc_get_drv_data(struct platform_device *pdev)
> -{
> -	struct s5p_mfc_variant *driver_data = NULL;
> -	const struct of_device_id *match;
> -
> -	match = of_match_node(exynos_mfc_match, pdev->dev.of_node);
> -	if (match)
> -		driver_data = (struct s5p_mfc_variant *)match->data;
> -
> -	return driver_data;
> -}
> -
>  static struct platform_driver s5p_mfc_driver = {
>  	.probe		= s5p_mfc_probe,
>  	.remove		= s5p_mfc_remove,
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
> index 3e0e8eaf8bfe..2f1387a4c386 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
> @@ -192,7 +192,7 @@ struct s5p_mfc_buf {
>   */
>  struct s5p_mfc_pm {
>  	struct clk	*clock_gate;
> -	const char	**clk_names;
> +	const char * const *clk_names;
>  	struct clk	*clocks[MFC_MAX_CLOCKS];
>  	int		num_clocks;
>  	bool		use_clock_gating;
> @@ -304,7 +304,7 @@ struct s5p_mfc_dev {
>  	struct v4l2_ctrl_handler dec_ctrl_handler;
>  	struct v4l2_ctrl_handler enc_ctrl_handler;
>  	struct s5p_mfc_pm	pm;
> -	struct s5p_mfc_variant	*variant;
> +	const struct s5p_mfc_variant	*variant;
>  	int num_inst;
>  	spinlock_t irqlock;	/* lock when operating on context */
>  	spinlock_t condlock;	/* lock when changing/checking if a context is

[PATCH v2 03/15] media: s5p-mfc: Replace mem_dev_* entries with an array

[Marek Szyprowski]

Internal MFC driver device structure contains two pointers to devices used
for DMA memory allocation: mem_dev_l and mem_dev_r. Replace them with the
mem_dev[] array and use defines for accessing particular banks. This will
help to simplify code in the next patches.

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>
---
 drivers/media/platform/s5p-mfc/s5p_mfc.c        | 31 +++++++++++++-----------
 drivers/media/platform/s5p-mfc/s5p_mfc_common.h | 11 ++++-----
 drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c   | 23 +++++++++---------
 drivers/media/platform/s5p-mfc/s5p_mfc_dec.c    |  8 +++----
 drivers/media/platform/s5p-mfc/s5p_mfc_enc.c    | 10 ++++----
 drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c | 32 ++++++++++++++-----------
 drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c | 15 ++++++------
 7 files changed, 69 insertions(+), 61 deletions(-)

diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c b/drivers/media/platform/s5p-mfc/s5p_mfc.c
index ad3d7377f40d..f7664910f12c 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
@@ -1123,7 +1123,8 @@ static int s5p_mfc_configure_dma_memory(struct s5p_mfc_dev *mfc_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;
+			mfc_dev->mem_dev[BANK1_CTX] =
+				mfc_dev->mem_dev[BANK2_CTX] = dev;
 		return ret;
 	}
 
@@ -1131,14 +1132,14 @@ static int s5p_mfc_configure_dma_memory(struct s5p_mfc_dev *mfc_dev)
 	 * 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)
+	mfc_dev->mem_dev[BANK1_CTX] = s5p_mfc_alloc_memdev(dev, "left",
+							   BANK1_CTX);
+	if (!mfc_dev->mem_dev[BANK1_CTX])
 		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);
+	mfc_dev->mem_dev[BANK2_CTX] = s5p_mfc_alloc_memdev(dev, "right",
+							   BANK2_CTX);
+	if (!mfc_dev->mem_dev[BANK2_CTX]) {
+		device_unregister(mfc_dev->mem_dev[BANK1_CTX]);
 		return -ENODEV;
 	}
 
@@ -1154,8 +1155,8 @@ static void s5p_mfc_unconfigure_dma_memory(struct s5p_mfc_dev *mfc_dev)
 		return;
 	}
 
-	device_unregister(mfc_dev->mem_dev_l);
-	device_unregister(mfc_dev->mem_dev_r);
+	device_unregister(mfc_dev->mem_dev[BANK1_CTX]);
+	device_unregister(mfc_dev->mem_dev[BANK2_CTX]);
 }
 
 /* MFC probe function */
@@ -1213,8 +1214,10 @@ static int s5p_mfc_probe(struct platform_device *pdev)
 		goto err_dma;
 	}
 
-	vb2_dma_contig_set_max_seg_size(dev->mem_dev_l, DMA_BIT_MASK(32));
-	vb2_dma_contig_set_max_seg_size(dev->mem_dev_r, DMA_BIT_MASK(32));
+	vb2_dma_contig_set_max_seg_size(dev->mem_dev[BANK1_CTX],
+					DMA_BIT_MASK(32));
+	vb2_dma_contig_set_max_seg_size(dev->mem_dev[BANK2_CTX],
+					DMA_BIT_MASK(32));
 
 	mutex_init(&dev->mfc_mutex);
 	init_waitqueue_head(&dev->queue);
@@ -1348,8 +1351,8 @@ static int s5p_mfc_remove(struct platform_device *pdev)
 	v4l2_device_unregister(&dev->v4l2_dev);
 	s5p_mfc_release_firmware(dev);
 	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);
+	vb2_dma_contig_clear_max_seg_size(dev->mem_dev[BANK1_CTX]);
+	vb2_dma_contig_clear_max_seg_size(dev->mem_dev[BANK2_CTX]);
 
 	s5p_mfc_final_pm(dev);
 	return 0;
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
index 2f1387a4c386..27d4c864e06e 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
@@ -33,8 +33,9 @@
 *  while mmaping */
 #define DST_QUEUE_OFF_BASE	(1 << 30)
 
-#define MFC_BANK1_ALLOC_CTX	0
-#define MFC_BANK2_ALLOC_CTX	1
+#define BANK1_CTX	0
+#define BANK2_CTX	1
+#define BANK_CTX_NUM	2
 
 #define MFC_BANK1_ALIGN_ORDER	13
 #define MFC_BANK2_ALIGN_ORDER	13
@@ -254,8 +255,7 @@ struct s5p_mfc_priv_buf {
  * @vfd_dec:		video device for decoding
  * @vfd_enc:		video device for encoding
  * @plat_dev:		platform device
- * @mem_dev_l:		child device of the left memory bank (0)
- * @mem_dev_r:		child device of the right memory bank (1)
+ * @mem_dev[]:		child devices of the memory banks
  * @regs_base:		base address of the MFC hw registers
  * @irq:		irq resource
  * @dec_ctrl_handler:	control framework handler for decoding
@@ -297,8 +297,7 @@ struct s5p_mfc_dev {
 	struct video_device	*vfd_dec;
 	struct video_device	*vfd_enc;
 	struct platform_device	*plat_dev;
-	struct device		*mem_dev_l;
-	struct device		*mem_dev_r;
+	struct device		*mem_dev[BANK_CTX_NUM];
 	void __iomem		*regs_base;
 	int			irq;
 	struct v4l2_ctrl_handler dec_ctrl_handler;
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c b/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c
index cc888713b3b6..cd1406c75d9a 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c
@@ -28,6 +28,7 @@ int s5p_mfc_alloc_firmware(struct s5p_mfc_dev *dev)
 {
 	void *bank2_virt;
 	dma_addr_t bank2_dma_addr;
+	unsigned int align_size = 1 << MFC_BASE_ALIGN_ORDER;
 
 	dev->fw_size = dev->variant->buf_size->fw;
 
@@ -36,8 +37,8 @@ int s5p_mfc_alloc_firmware(struct s5p_mfc_dev *dev)
 		return -ENOMEM;
 	}
 
-	dev->fw_virt_addr = dma_alloc_coherent(dev->mem_dev_l, dev->fw_size,
-					&dev->bank1, GFP_KERNEL);
+	dev->fw_virt_addr = dma_alloc_coherent(dev->mem_dev[BANK1_CTX],
+					dev->fw_size, &dev->bank1, GFP_KERNEL);
 
 	if (!dev->fw_virt_addr) {
 		mfc_err("Allocating bitprocessor buffer failed\n");
@@ -45,13 +46,13 @@ int s5p_mfc_alloc_firmware(struct s5p_mfc_dev *dev)
 	}
 
 	if (HAS_PORTNUM(dev) && IS_TWOPORT(dev)) {
-		bank2_virt = dma_alloc_coherent(dev->mem_dev_r, 1 << MFC_BASE_ALIGN_ORDER,
-					&bank2_dma_addr, GFP_KERNEL);
+		bank2_virt = dma_alloc_coherent(dev->mem_dev[BANK2_CTX],
+				       align_size, &bank2_dma_addr, GFP_KERNEL);
 
 		if (!bank2_virt) {
 			mfc_err("Allocating bank2 base failed\n");
-			dma_free_coherent(dev->mem_dev_l, dev->fw_size,
-				dev->fw_virt_addr, dev->bank1);
+			dma_free_coherent(dev->mem_dev[BANK1_CTX], dev->fw_size,
+					  dev->fw_virt_addr, dev->bank1);
 			dev->fw_virt_addr = NULL;
 			return -ENOMEM;
 		}
@@ -60,10 +61,10 @@ int s5p_mfc_alloc_firmware(struct s5p_mfc_dev *dev)
 		 * should not have address of bank2 - MFC will treat it as a null frame.
 		 * To avoid such situation we set bank2 address below the pool address.
 		 */
-		dev->bank2 = bank2_dma_addr - (1 << MFC_BASE_ALIGN_ORDER);
+		dev->bank2 = bank2_dma_addr - align_size;
 
-		dma_free_coherent(dev->mem_dev_r, 1 << MFC_BASE_ALIGN_ORDER,
-			bank2_virt, bank2_dma_addr);
+		dma_free_coherent(dev->mem_dev[BANK2_CTX], align_size,
+				  bank2_virt, bank2_dma_addr);
 
 	} else {
 		/* In this case bank2 can point to the same address as bank1.
@@ -123,8 +124,8 @@ int s5p_mfc_release_firmware(struct s5p_mfc_dev *dev)
 	 * that MFC is no longer processing */
 	if (!dev->fw_virt_addr)
 		return -EINVAL;
-	dma_free_coherent(dev->mem_dev_l, dev->fw_size, dev->fw_virt_addr,
-						dev->bank1);
+	dma_free_coherent(dev->mem_dev[BANK1_CTX], dev->fw_size,
+			  dev->fw_virt_addr, dev->bank1);
 	dev->fw_virt_addr = NULL;
 	return 0;
 }
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c b/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
index 367ef8e8dbf0..f17062f9070b 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
@@ -931,14 +931,14 @@ static int s5p_mfc_queue_setup(struct vb2_queue *vq,
 		psize[1] = ctx->chroma_size;
 
 		if (IS_MFCV6_PLUS(dev))
-			alloc_devs[0] = ctx->dev->mem_dev_l;
+			alloc_devs[0] = ctx->dev->mem_dev[BANK1_CTX];
 		else
-			alloc_devs[0] = ctx->dev->mem_dev_r;
-		alloc_devs[1] = ctx->dev->mem_dev_l;
+			alloc_devs[0] = ctx->dev->mem_dev[BANK2_CTX];
+		alloc_devs[1] = ctx->dev->mem_dev[BANK1_CTX];
 	} else if (vq->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE &&
 		   ctx->state == MFCINST_INIT) {
 		psize[0] = ctx->dec_src_buf_size;
-		alloc_devs[0] = ctx->dev->mem_dev_l;
+		alloc_devs[0] = ctx->dev->mem_dev[BANK1_CTX];
 	} else {
 		mfc_err("This video node is dedicated to decoding. Decoding not initialized\n");
 		return -EINVAL;
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c b/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
index e39d9e06e299..2eea21f06d7e 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
@@ -1832,7 +1832,7 @@ static int s5p_mfc_queue_setup(struct vb2_queue *vq,
 		if (*buf_count > MFC_MAX_BUFFERS)
 			*buf_count = MFC_MAX_BUFFERS;
 		psize[0] = ctx->enc_dst_buf_size;
-		alloc_devs[0] = ctx->dev->mem_dev_l;
+		alloc_devs[0] = ctx->dev->mem_dev[BANK1_CTX];
 	} else if (vq->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) {
 		if (ctx->src_fmt)
 			*plane_count = ctx->src_fmt->num_planes;
@@ -1848,11 +1848,11 @@ static int s5p_mfc_queue_setup(struct vb2_queue *vq,
 		psize[1] = ctx->chroma_size;
 
 		if (IS_MFCV6_PLUS(dev)) {
-			alloc_devs[0] = ctx->dev->mem_dev_l;
-			alloc_devs[1] = ctx->dev->mem_dev_l;
+			alloc_devs[0] = ctx->dev->mem_dev[BANK1_CTX];
+			alloc_devs[1] = ctx->dev->mem_dev[BANK1_CTX];
 		} else {
-			alloc_devs[0] = ctx->dev->mem_dev_r;
-			alloc_devs[1] = ctx->dev->mem_dev_r;
+			alloc_devs[0] = ctx->dev->mem_dev[BANK2_CTX];
+			alloc_devs[1] = ctx->dev->mem_dev[BANK2_CTX];
 		}
 	} else {
 		mfc_err("invalid queue type: %d\n", vq->type);
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c
index f4301d5bbd32..65dd3e64b4db 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c
@@ -41,7 +41,8 @@ static int s5p_mfc_alloc_dec_temp_buffers_v5(struct s5p_mfc_ctx *ctx)
 	int ret;
 
 	ctx->dsc.size = buf_size->dsc;
-	ret =  s5p_mfc_alloc_priv_buf(dev->mem_dev_l, dev->bank1, &ctx->dsc);
+	ret =  s5p_mfc_alloc_priv_buf(dev->mem_dev[BANK1_CTX], dev->bank1,
+				      &ctx->dsc);
 	if (ret) {
 		mfc_err("Failed to allocate temporary buffer\n");
 		return ret;
@@ -57,7 +58,7 @@ static int s5p_mfc_alloc_dec_temp_buffers_v5(struct s5p_mfc_ctx *ctx)
 /* Release temporary buffers for decoding */
 static void s5p_mfc_release_dec_desc_buffer_v5(struct s5p_mfc_ctx *ctx)
 {
-	s5p_mfc_release_priv_buf(ctx->dev->mem_dev_l, &ctx->dsc);
+	s5p_mfc_release_priv_buf(ctx->dev->mem_dev[BANK1_CTX], &ctx->dsc);
 }
 
 /* Allocate codec buffers */
@@ -172,8 +173,8 @@ static int s5p_mfc_alloc_codec_buffers_v5(struct s5p_mfc_ctx *ctx)
 	/* Allocate only if memory from bank 1 is necessary */
 	if (ctx->bank1.size > 0) {
 
-		ret = s5p_mfc_alloc_priv_buf(dev->mem_dev_l, dev->bank1,
-					     &ctx->bank1);
+		ret = s5p_mfc_alloc_priv_buf(dev->mem_dev[BANK1_CTX],
+					     dev->bank1, &ctx->bank1);
 		if (ret) {
 			mfc_err("Failed to allocate Bank1 temporary buffer\n");
 			return ret;
@@ -182,11 +183,12 @@ static int s5p_mfc_alloc_codec_buffers_v5(struct s5p_mfc_ctx *ctx)
 	}
 	/* Allocate only if memory from bank 2 is necessary */
 	if (ctx->bank2.size > 0) {
-		ret = s5p_mfc_alloc_priv_buf(dev->mem_dev_r, dev->bank2,
-					     &ctx->bank2);
+		ret = s5p_mfc_alloc_priv_buf(dev->mem_dev[BANK2_CTX],
+					     dev->bank2, &ctx->bank2);
 		if (ret) {
 			mfc_err("Failed to allocate Bank2 temporary buffer\n");
-			s5p_mfc_release_priv_buf(ctx->dev->mem_dev_l, &ctx->bank1);
+			s5p_mfc_release_priv_buf(ctx->dev->mem_dev[BANK1_CTX],
+						 &ctx->bank1);
 			return ret;
 		}
 		BUG_ON(ctx->bank2.dma & ((1 << MFC_BANK2_ALIGN_ORDER) - 1));
@@ -197,8 +199,8 @@ static int s5p_mfc_alloc_codec_buffers_v5(struct s5p_mfc_ctx *ctx)
 /* Release buffers allocated for codec */
 static void s5p_mfc_release_codec_buffers_v5(struct s5p_mfc_ctx *ctx)
 {
-	s5p_mfc_release_priv_buf(ctx->dev->mem_dev_l, &ctx->bank1);
-	s5p_mfc_release_priv_buf(ctx->dev->mem_dev_r, &ctx->bank2);
+	s5p_mfc_release_priv_buf(ctx->dev->mem_dev[BANK1_CTX], &ctx->bank1);
+	s5p_mfc_release_priv_buf(ctx->dev->mem_dev[BANK2_CTX], &ctx->bank2);
 }
 
 /* Allocate memory for instance data buffer */
@@ -214,7 +216,8 @@ static int s5p_mfc_alloc_instance_buffer_v5(struct s5p_mfc_ctx *ctx)
 	else
 		ctx->ctx.size = buf_size->non_h264_ctx;
 
-	ret = s5p_mfc_alloc_priv_buf(dev->mem_dev_l, dev->bank1, &ctx->ctx);
+	ret = s5p_mfc_alloc_priv_buf(dev->mem_dev[BANK1_CTX], dev->bank1,
+				     &ctx->ctx);
 	if (ret) {
 		mfc_err("Failed to allocate instance buffer\n");
 		return ret;
@@ -227,10 +230,11 @@ static int s5p_mfc_alloc_instance_buffer_v5(struct s5p_mfc_ctx *ctx)
 
 	/* Initialize shared memory */
 	ctx->shm.size = buf_size->shm;
-	ret = s5p_mfc_alloc_priv_buf(dev->mem_dev_l, dev->bank1, &ctx->shm);
+	ret = s5p_mfc_alloc_priv_buf(dev->mem_dev[BANK1_CTX], dev->bank1,
+				     &ctx->shm);
 	if (ret) {
 		mfc_err("Failed to allocate shared memory buffer\n");
-		s5p_mfc_release_priv_buf(dev->mem_dev_l, &ctx->ctx);
+		s5p_mfc_release_priv_buf(dev->mem_dev[BANK1_CTX], &ctx->ctx);
 		return ret;
 	}
 
@@ -246,8 +250,8 @@ static int s5p_mfc_alloc_instance_buffer_v5(struct s5p_mfc_ctx *ctx)
 /* Release instance buffer */
 static void s5p_mfc_release_instance_buffer_v5(struct s5p_mfc_ctx *ctx)
 {
-	s5p_mfc_release_priv_buf(ctx->dev->mem_dev_l, &ctx->ctx);
-	s5p_mfc_release_priv_buf(ctx->dev->mem_dev_l, &ctx->shm);
+	s5p_mfc_release_priv_buf(ctx->dev->mem_dev[BANK1_CTX], &ctx->ctx);
+	s5p_mfc_release_priv_buf(ctx->dev->mem_dev[BANK1_CTX], &ctx->shm);
 }
 
 static int s5p_mfc_alloc_dev_context_buffer_v5(struct s5p_mfc_dev *dev)
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
index d6f207e859ab..d2bc938253bc 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
@@ -239,8 +239,8 @@ static int s5p_mfc_alloc_codec_buffers_v6(struct s5p_mfc_ctx *ctx)
 
 	/* Allocate only if memory from bank 1 is necessary */
 	if (ctx->bank1.size > 0) {
-		ret = s5p_mfc_alloc_priv_buf(dev->mem_dev_l, dev->bank1,
-					     &ctx->bank1);
+		ret = s5p_mfc_alloc_priv_buf(dev->mem_dev[BANK1_CTX],
+					     dev->bank1, &ctx->bank1);
 		if (ret) {
 			mfc_err("Failed to allocate Bank1 memory\n");
 			return ret;
@@ -253,7 +253,7 @@ static int s5p_mfc_alloc_codec_buffers_v6(struct s5p_mfc_ctx *ctx)
 /* Release buffers allocated for codec */
 static void s5p_mfc_release_codec_buffers_v6(struct s5p_mfc_ctx *ctx)
 {
-	s5p_mfc_release_priv_buf(ctx->dev->mem_dev_l, &ctx->bank1);
+	s5p_mfc_release_priv_buf(ctx->dev->mem_dev[BANK1_CTX], &ctx->bank1);
 }
 
 /* Allocate memory for instance data buffer */
@@ -292,7 +292,8 @@ static int s5p_mfc_alloc_instance_buffer_v6(struct s5p_mfc_ctx *ctx)
 		break;
 	}
 
-	ret = s5p_mfc_alloc_priv_buf(dev->mem_dev_l, dev->bank1, &ctx->ctx);
+	ret = s5p_mfc_alloc_priv_buf(dev->mem_dev[BANK1_CTX], dev->bank1,
+				     &ctx->ctx);
 	if (ret) {
 		mfc_err("Failed to allocate instance buffer\n");
 		return ret;
@@ -309,7 +310,7 @@ static int s5p_mfc_alloc_instance_buffer_v6(struct s5p_mfc_ctx *ctx)
 /* Release instance buffer */
 static void s5p_mfc_release_instance_buffer_v6(struct s5p_mfc_ctx *ctx)
 {
-	s5p_mfc_release_priv_buf(ctx->dev->mem_dev_l, &ctx->ctx);
+	s5p_mfc_release_priv_buf(ctx->dev->mem_dev[BANK1_CTX], &ctx->ctx);
 }
 
 /* Allocate context buffers for SYS_INIT */
@@ -321,7 +322,7 @@ static int s5p_mfc_alloc_dev_context_buffer_v6(struct s5p_mfc_dev *dev)
 	mfc_debug_enter();
 
 	dev->ctx_buf.size = buf_size->dev_ctx;
-	ret = s5p_mfc_alloc_priv_buf(dev->mem_dev_l, dev->bank1,
+	ret = s5p_mfc_alloc_priv_buf(dev->mem_dev[BANK1_CTX], dev->bank1,
 				     &dev->ctx_buf);
 	if (ret) {
 		mfc_err("Failed to allocate device context buffer\n");
@@ -339,7 +340,7 @@ static int s5p_mfc_alloc_dev_context_buffer_v6(struct s5p_mfc_dev *dev)
 /* Release context buffers for SYS_INIT */
 static void s5p_mfc_release_dev_context_buffer_v6(struct s5p_mfc_dev *dev)
 {
-	s5p_mfc_release_priv_buf(dev->mem_dev_l, &dev->ctx_buf);
+	s5p_mfc_release_priv_buf(dev->mem_dev[BANK1_CTX], &dev->ctx_buf);
 }
 
 static int calc_plane(int width, int height)
-- 
1.9.1

Re: [PATCH v2 03/15] media: s5p-mfc: Replace mem_dev_* entries with an array

[Shuah Khan]

On Mon, Feb 20, 2017 at 6:38 AM, Marek Szyprowski
<m.szyprowski@samsung.com> wrote:
> Internal MFC driver device structure contains two pointers to devices used
> for DMA memory allocation: mem_dev_l and mem_dev_r. Replace them with the
> mem_dev[] array and use defines for accessing particular banks. This will
> help to simplify code in the next patches.

Hi Marek,

The change looks good to me. One comment thought that it would be
good to keep the left and right banks in the driver code to be in sync
with the DT nomenclature.

BANK_L_CTX instead of BANK1_CTX
BANK_R_CTX instead of BANK2_CTX

>
> 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>
> ---
>  drivers/media/platform/s5p-mfc/s5p_mfc.c        | 31 +++++++++++++-----------
>  drivers/media/platform/s5p-mfc/s5p_mfc_common.h | 11 ++++-----
>  drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c   | 23 +++++++++---------
>  drivers/media/platform/s5p-mfc/s5p_mfc_dec.c    |  8 +++----
>  drivers/media/platform/s5p-mfc/s5p_mfc_enc.c    | 10 ++++----
>  drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c | 32 ++++++++++++++-----------
>  drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c | 15 ++++++------
>  7 files changed, 69 insertions(+), 61 deletions(-)
>
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c b/drivers/media/platform/s5p-mfc/s5p_mfc.c
> index ad3d7377f40d..f7664910f12c 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
> @@ -1123,7 +1123,8 @@ static int s5p_mfc_configure_dma_memory(struct s5p_mfc_dev *mfc_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;
> +                       mfc_dev->mem_dev[BANK1_CTX] =
> +                               mfc_dev->mem_dev[BANK2_CTX] = dev;
>                 return ret;
>         }
>
> @@ -1131,14 +1132,14 @@ static int s5p_mfc_configure_dma_memory(struct s5p_mfc_dev *mfc_dev)
>          * 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)
> +       mfc_dev->mem_dev[BANK1_CTX] = s5p_mfc_alloc_memdev(dev, "left",
> +                                                          BANK1_CTX);
> +       if (!mfc_dev->mem_dev[BANK1_CTX])
>                 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);
> +       mfc_dev->mem_dev[BANK2_CTX] = s5p_mfc_alloc_memdev(dev, "right",
> +                                                          BANK2_CTX);
> +       if (!mfc_dev->mem_dev[BANK2_CTX]) {
> +               device_unregister(mfc_dev->mem_dev[BANK1_CTX]);
>                 return -ENODEV;
>         }
>
> @@ -1154,8 +1155,8 @@ static void s5p_mfc_unconfigure_dma_memory(struct s5p_mfc_dev *mfc_dev)
>                 return;
>         }
>
> -       device_unregister(mfc_dev->mem_dev_l);
> -       device_unregister(mfc_dev->mem_dev_r);
> +       device_unregister(mfc_dev->mem_dev[BANK1_CTX]);
> +       device_unregister(mfc_dev->mem_dev[BANK2_CTX]);
>  }
>
>  /* MFC probe function */
> @@ -1213,8 +1214,10 @@ static int s5p_mfc_probe(struct platform_device *pdev)
>                 goto err_dma;
>         }
>
> -       vb2_dma_contig_set_max_seg_size(dev->mem_dev_l, DMA_BIT_MASK(32));
> -       vb2_dma_contig_set_max_seg_size(dev->mem_dev_r, DMA_BIT_MASK(32));
> +       vb2_dma_contig_set_max_seg_size(dev->mem_dev[BANK1_CTX],
> +                                       DMA_BIT_MASK(32));
> +       vb2_dma_contig_set_max_seg_size(dev->mem_dev[BANK2_CTX],
> +                                       DMA_BIT_MASK(32));
>
>         mutex_init(&dev->mfc_mutex);
>         init_waitqueue_head(&dev->queue);
> @@ -1348,8 +1351,8 @@ static int s5p_mfc_remove(struct platform_device *pdev)
>         v4l2_device_unregister(&dev->v4l2_dev);
>         s5p_mfc_release_firmware(dev);
>         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);
> +       vb2_dma_contig_clear_max_seg_size(dev->mem_dev[BANK1_CTX]);
> +       vb2_dma_contig_clear_max_seg_size(dev->mem_dev[BANK2_CTX]);
>
>         s5p_mfc_final_pm(dev);
>         return 0;
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
> index 2f1387a4c386..27d4c864e06e 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
> @@ -33,8 +33,9 @@
>  *  while mmaping */
>  #define DST_QUEUE_OFF_BASE     (1 << 30)
>
> -#define MFC_BANK1_ALLOC_CTX    0
> -#define MFC_BANK2_ALLOC_CTX    1
> +#define BANK1_CTX      0
> +#define BANK2_CTX      1
> +#define BANK_CTX_NUM   2
>
>  #define MFC_BANK1_ALIGN_ORDER  13
>  #define MFC_BANK2_ALIGN_ORDER  13
> @@ -254,8 +255,7 @@ struct s5p_mfc_priv_buf {
>   * @vfd_dec:           video device for decoding
>   * @vfd_enc:           video device for encoding
>   * @plat_dev:          platform device
> - * @mem_dev_l:         child device of the left memory bank (0)
> - * @mem_dev_r:         child device of the right memory bank (1)
> + * @mem_dev[]:         child devices of the memory banks
>   * @regs_base:         base address of the MFC hw registers
>   * @irq:               irq resource
>   * @dec_ctrl_handler:  control framework handler for decoding
> @@ -297,8 +297,7 @@ struct s5p_mfc_dev {
>         struct video_device     *vfd_dec;
>         struct video_device     *vfd_enc;
>         struct platform_device  *plat_dev;
> -       struct device           *mem_dev_l;
> -       struct device           *mem_dev_r;
> +       struct device           *mem_dev[BANK_CTX_NUM];
>         void __iomem            *regs_base;
>         int                     irq;
>         struct v4l2_ctrl_handler dec_ctrl_handler;
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c b/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c
> index cc888713b3b6..cd1406c75d9a 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c
> @@ -28,6 +28,7 @@ int s5p_mfc_alloc_firmware(struct s5p_mfc_dev *dev)
>  {
>         void *bank2_virt;
>         dma_addr_t bank2_dma_addr;
> +       unsigned int align_size = 1 << MFC_BASE_ALIGN_ORDER;
>
>         dev->fw_size = dev->variant->buf_size->fw;
>
> @@ -36,8 +37,8 @@ int s5p_mfc_alloc_firmware(struct s5p_mfc_dev *dev)
>                 return -ENOMEM;
>         }
>
> -       dev->fw_virt_addr = dma_alloc_coherent(dev->mem_dev_l, dev->fw_size,
> -                                       &dev->bank1, GFP_KERNEL);
> +       dev->fw_virt_addr = dma_alloc_coherent(dev->mem_dev[BANK1_CTX],
> +                                       dev->fw_size, &dev->bank1, GFP_KERNEL);
>
>         if (!dev->fw_virt_addr) {
>                 mfc_err("Allocating bitprocessor buffer failed\n");
> @@ -45,13 +46,13 @@ int s5p_mfc_alloc_firmware(struct s5p_mfc_dev *dev)
>         }
>
>         if (HAS_PORTNUM(dev) && IS_TWOPORT(dev)) {
> -               bank2_virt = dma_alloc_coherent(dev->mem_dev_r, 1 << MFC_BASE_ALIGN_ORDER,
> -                                       &bank2_dma_addr, GFP_KERNEL);
> +               bank2_virt = dma_alloc_coherent(dev->mem_dev[BANK2_CTX],
> +                                      align_size, &bank2_dma_addr, GFP_KERNEL);
>
>                 if (!bank2_virt) {
>                         mfc_err("Allocating bank2 base failed\n");
> -                       dma_free_coherent(dev->mem_dev_l, dev->fw_size,
> -                               dev->fw_virt_addr, dev->bank1);
> +                       dma_free_coherent(dev->mem_dev[BANK1_CTX], dev->fw_size,
> +                                         dev->fw_virt_addr, dev->bank1);
>                         dev->fw_virt_addr = NULL;
>                         return -ENOMEM;
>                 }
> @@ -60,10 +61,10 @@ int s5p_mfc_alloc_firmware(struct s5p_mfc_dev *dev)
>                  * should not have address of bank2 - MFC will treat it as a null frame.
>                  * To avoid such situation we set bank2 address below the pool address.
>                  */
> -               dev->bank2 = bank2_dma_addr - (1 << MFC_BASE_ALIGN_ORDER);
> +               dev->bank2 = bank2_dma_addr - align_size;
>
> -               dma_free_coherent(dev->mem_dev_r, 1 << MFC_BASE_ALIGN_ORDER,
> -                       bank2_virt, bank2_dma_addr);
> +               dma_free_coherent(dev->mem_dev[BANK2_CTX], align_size,
> +                                 bank2_virt, bank2_dma_addr);
>
>         } else {
>                 /* In this case bank2 can point to the same address as bank1.
> @@ -123,8 +124,8 @@ int s5p_mfc_release_firmware(struct s5p_mfc_dev *dev)
>          * that MFC is no longer processing */
>         if (!dev->fw_virt_addr)
>                 return -EINVAL;
> -       dma_free_coherent(dev->mem_dev_l, dev->fw_size, dev->fw_virt_addr,
> -                                               dev->bank1);
> +       dma_free_coherent(dev->mem_dev[BANK1_CTX], dev->fw_size,
> +                         dev->fw_virt_addr, dev->bank1);
>         dev->fw_virt_addr = NULL;
>         return 0;
>  }
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c b/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
> index 367ef8e8dbf0..f17062f9070b 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
> @@ -931,14 +931,14 @@ static int s5p_mfc_queue_setup(struct vb2_queue *vq,
>                 psize[1] = ctx->chroma_size;
>
>                 if (IS_MFCV6_PLUS(dev))
> -                       alloc_devs[0] = ctx->dev->mem_dev_l;
> +                       alloc_devs[0] = ctx->dev->mem_dev[BANK1_CTX];
>                 else
> -                       alloc_devs[0] = ctx->dev->mem_dev_r;
> -               alloc_devs[1] = ctx->dev->mem_dev_l;
> +                       alloc_devs[0] = ctx->dev->mem_dev[BANK2_CTX];
> +               alloc_devs[1] = ctx->dev->mem_dev[BANK1_CTX];
>         } else if (vq->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE &&
>                    ctx->state == MFCINST_INIT) {
>                 psize[0] = ctx->dec_src_buf_size;
> -               alloc_devs[0] = ctx->dev->mem_dev_l;
> +               alloc_devs[0] = ctx->dev->mem_dev[BANK1_CTX];
>         } else {
>                 mfc_err("This video node is dedicated to decoding. Decoding not initialized\n");
>                 return -EINVAL;
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c b/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
> index e39d9e06e299..2eea21f06d7e 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
> @@ -1832,7 +1832,7 @@ static int s5p_mfc_queue_setup(struct vb2_queue *vq,
>                 if (*buf_count > MFC_MAX_BUFFERS)
>                         *buf_count = MFC_MAX_BUFFERS;
>                 psize[0] = ctx->enc_dst_buf_size;
> -               alloc_devs[0] = ctx->dev->mem_dev_l;
> +               alloc_devs[0] = ctx->dev->mem_dev[BANK1_CTX];
>         } else if (vq->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) {
>                 if (ctx->src_fmt)
>                         *plane_count = ctx->src_fmt->num_planes;
> @@ -1848,11 +1848,11 @@ static int s5p_mfc_queue_setup(struct vb2_queue *vq,
>                 psize[1] = ctx->chroma_size;
>
>                 if (IS_MFCV6_PLUS(dev)) {
> -                       alloc_devs[0] = ctx->dev->mem_dev_l;
> -                       alloc_devs[1] = ctx->dev->mem_dev_l;
> +                       alloc_devs[0] = ctx->dev->mem_dev[BANK1_CTX];
> +                       alloc_devs[1] = ctx->dev->mem_dev[BANK1_CTX];
>                 } else {
> -                       alloc_devs[0] = ctx->dev->mem_dev_r;
> -                       alloc_devs[1] = ctx->dev->mem_dev_r;
> +                       alloc_devs[0] = ctx->dev->mem_dev[BANK2_CTX];
> +                       alloc_devs[1] = ctx->dev->mem_dev[BANK2_CTX];
>                 }
>         } else {
>                 mfc_err("invalid queue type: %d\n", vq->type);
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c
> index f4301d5bbd32..65dd3e64b4db 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c
> @@ -41,7 +41,8 @@ static int s5p_mfc_alloc_dec_temp_buffers_v5(struct s5p_mfc_ctx *ctx)
>         int ret;
>
>         ctx->dsc.size = buf_size->dsc;
> -       ret =  s5p_mfc_alloc_priv_buf(dev->mem_dev_l, dev->bank1, &ctx->dsc);
> +       ret =  s5p_mfc_alloc_priv_buf(dev->mem_dev[BANK1_CTX], dev->bank1,
> +                                     &ctx->dsc);
>         if (ret) {
>                 mfc_err("Failed to allocate temporary buffer\n");
>                 return ret;
> @@ -57,7 +58,7 @@ static int s5p_mfc_alloc_dec_temp_buffers_v5(struct s5p_mfc_ctx *ctx)
>  /* Release temporary buffers for decoding */
>  static void s5p_mfc_release_dec_desc_buffer_v5(struct s5p_mfc_ctx *ctx)
>  {
> -       s5p_mfc_release_priv_buf(ctx->dev->mem_dev_l, &ctx->dsc);
> +       s5p_mfc_release_priv_buf(ctx->dev->mem_dev[BANK1_CTX], &ctx->dsc);
>  }
>
>  /* Allocate codec buffers */
> @@ -172,8 +173,8 @@ static int s5p_mfc_alloc_codec_buffers_v5(struct s5p_mfc_ctx *ctx)
>         /* Allocate only if memory from bank 1 is necessary */
>         if (ctx->bank1.size > 0) {
>
> -               ret = s5p_mfc_alloc_priv_buf(dev->mem_dev_l, dev->bank1,
> -                                            &ctx->bank1);
> +               ret = s5p_mfc_alloc_priv_buf(dev->mem_dev[BANK1_CTX],
> +                                            dev->bank1, &ctx->bank1);
>                 if (ret) {
>                         mfc_err("Failed to allocate Bank1 temporary buffer\n");
>                         return ret;
> @@ -182,11 +183,12 @@ static int s5p_mfc_alloc_codec_buffers_v5(struct s5p_mfc_ctx *ctx)
>         }
>         /* Allocate only if memory from bank 2 is necessary */
>         if (ctx->bank2.size > 0) {
> -               ret = s5p_mfc_alloc_priv_buf(dev->mem_dev_r, dev->bank2,
> -                                            &ctx->bank2);
> +               ret = s5p_mfc_alloc_priv_buf(dev->mem_dev[BANK2_CTX],
> +                                            dev->bank2, &ctx->bank2);
>                 if (ret) {
>                         mfc_err("Failed to allocate Bank2 temporary buffer\n");
> -                       s5p_mfc_release_priv_buf(ctx->dev->mem_dev_l, &ctx->bank1);
> +                       s5p_mfc_release_priv_buf(ctx->dev->mem_dev[BANK1_CTX],
> +                                                &ctx->bank1);
>                         return ret;
>                 }
>                 BUG_ON(ctx->bank2.dma & ((1 << MFC_BANK2_ALIGN_ORDER) - 1));
> @@ -197,8 +199,8 @@ static int s5p_mfc_alloc_codec_buffers_v5(struct s5p_mfc_ctx *ctx)
>  /* Release buffers allocated for codec */
>  static void s5p_mfc_release_codec_buffers_v5(struct s5p_mfc_ctx *ctx)
>  {
> -       s5p_mfc_release_priv_buf(ctx->dev->mem_dev_l, &ctx->bank1);
> -       s5p_mfc_release_priv_buf(ctx->dev->mem_dev_r, &ctx->bank2);
> +       s5p_mfc_release_priv_buf(ctx->dev->mem_dev[BANK1_CTX], &ctx->bank1);
> +       s5p_mfc_release_priv_buf(ctx->dev->mem_dev[BANK2_CTX], &ctx->bank2);
>  }
>
>  /* Allocate memory for instance data buffer */
> @@ -214,7 +216,8 @@ static int s5p_mfc_alloc_instance_buffer_v5(struct s5p_mfc_ctx *ctx)
>         else
>                 ctx->ctx.size = buf_size->non_h264_ctx;
>
> -       ret = s5p_mfc_alloc_priv_buf(dev->mem_dev_l, dev->bank1, &ctx->ctx);
> +       ret = s5p_mfc_alloc_priv_buf(dev->mem_dev[BANK1_CTX], dev->bank1,
> +                                    &ctx->ctx);
>         if (ret) {
>                 mfc_err("Failed to allocate instance buffer\n");
>                 return ret;
> @@ -227,10 +230,11 @@ static int s5p_mfc_alloc_instance_buffer_v5(struct s5p_mfc_ctx *ctx)
>
>         /* Initialize shared memory */
>         ctx->shm.size = buf_size->shm;
> -       ret = s5p_mfc_alloc_priv_buf(dev->mem_dev_l, dev->bank1, &ctx->shm);
> +       ret = s5p_mfc_alloc_priv_buf(dev->mem_dev[BANK1_CTX], dev->bank1,
> +                                    &ctx->shm);
>         if (ret) {
>                 mfc_err("Failed to allocate shared memory buffer\n");
> -               s5p_mfc_release_priv_buf(dev->mem_dev_l, &ctx->ctx);
> +               s5p_mfc_release_priv_buf(dev->mem_dev[BANK1_CTX], &ctx->ctx);
>                 return ret;
>         }
>
> @@ -246,8 +250,8 @@ static int s5p_mfc_alloc_instance_buffer_v5(struct s5p_mfc_ctx *ctx)
>  /* Release instance buffer */
>  static void s5p_mfc_release_instance_buffer_v5(struct s5p_mfc_ctx *ctx)
>  {
> -       s5p_mfc_release_priv_buf(ctx->dev->mem_dev_l, &ctx->ctx);
> -       s5p_mfc_release_priv_buf(ctx->dev->mem_dev_l, &ctx->shm);
> +       s5p_mfc_release_priv_buf(ctx->dev->mem_dev[BANK1_CTX], &ctx->ctx);
> +       s5p_mfc_release_priv_buf(ctx->dev->mem_dev[BANK1_CTX], &ctx->shm);
>  }
>
>  static int s5p_mfc_alloc_dev_context_buffer_v5(struct s5p_mfc_dev *dev)
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
> index d6f207e859ab..d2bc938253bc 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
> @@ -239,8 +239,8 @@ static int s5p_mfc_alloc_codec_buffers_v6(struct s5p_mfc_ctx *ctx)
>
>         /* Allocate only if memory from bank 1 is necessary */
>         if (ctx->bank1.size > 0) {
> -               ret = s5p_mfc_alloc_priv_buf(dev->mem_dev_l, dev->bank1,
> -                                            &ctx->bank1);
> +               ret = s5p_mfc_alloc_priv_buf(dev->mem_dev[BANK1_CTX],
> +                                            dev->bank1, &ctx->bank1);
>                 if (ret) {
>                         mfc_err("Failed to allocate Bank1 memory\n");
>                         return ret;
> @@ -253,7 +253,7 @@ static int s5p_mfc_alloc_codec_buffers_v6(struct s5p_mfc_ctx *ctx)
>  /* Release buffers allocated for codec */
>  static void s5p_mfc_release_codec_buffers_v6(struct s5p_mfc_ctx *ctx)
>  {
> -       s5p_mfc_release_priv_buf(ctx->dev->mem_dev_l, &ctx->bank1);
> +       s5p_mfc_release_priv_buf(ctx->dev->mem_dev[BANK1_CTX], &ctx->bank1);
>  }
>
>  /* Allocate memory for instance data buffer */
> @@ -292,7 +292,8 @@ static int s5p_mfc_alloc_instance_buffer_v6(struct s5p_mfc_ctx *ctx)
>                 break;
>         }
>
> -       ret = s5p_mfc_alloc_priv_buf(dev->mem_dev_l, dev->bank1, &ctx->ctx);
> +       ret = s5p_mfc_alloc_priv_buf(dev->mem_dev[BANK1_CTX], dev->bank1,
> +                                    &ctx->ctx);
>         if (ret) {
>                 mfc_err("Failed to allocate instance buffer\n");
>                 return ret;
> @@ -309,7 +310,7 @@ static int s5p_mfc_alloc_instance_buffer_v6(struct s5p_mfc_ctx *ctx)
>  /* Release instance buffer */
>  static void s5p_mfc_release_instance_buffer_v6(struct s5p_mfc_ctx *ctx)
>  {
> -       s5p_mfc_release_priv_buf(ctx->dev->mem_dev_l, &ctx->ctx);
> +       s5p_mfc_release_priv_buf(ctx->dev->mem_dev[BANK1_CTX], &ctx->ctx);
>  }
>
>  /* Allocate context buffers for SYS_INIT */
> @@ -321,7 +322,7 @@ static int s5p_mfc_alloc_dev_context_buffer_v6(struct s5p_mfc_dev *dev)
>         mfc_debug_enter();
>
>         dev->ctx_buf.size = buf_size->dev_ctx;
> -       ret = s5p_mfc_alloc_priv_buf(dev->mem_dev_l, dev->bank1,
> +       ret = s5p_mfc_alloc_priv_buf(dev->mem_dev[BANK1_CTX], dev->bank1,
>                                      &dev->ctx_buf);
>         if (ret) {
>                 mfc_err("Failed to allocate device context buffer\n");
> @@ -339,7 +340,7 @@ static int s5p_mfc_alloc_dev_context_buffer_v6(struct s5p_mfc_dev *dev)
>  /* Release context buffers for SYS_INIT */
>  static void s5p_mfc_release_dev_context_buffer_v6(struct s5p_mfc_dev *dev)
>  {
> -       s5p_mfc_release_priv_buf(dev->mem_dev_l, &dev->ctx_buf);
> +       s5p_mfc_release_priv_buf(dev->mem_dev[BANK1_CTX], &dev->ctx_buf);
>  }
>
>  static int calc_plane(int width, int height)
> --
> 1.9.1
>

thanks,
-- Shuah

Re: [PATCH v2 03/15] media: s5p-mfc: Replace mem_dev_* entries with an array

[Sylwester Nawrocki]

On 02/22/2017 05:22 PM, Shuah Khan wrote:
> On Mon, Feb 20, 2017 at 6:38 AM, Marek Szyprowski
> <m.szyprowski@samsung.com> wrote:
>> Internal MFC driver device structure contains two pointers to devices used
>> for DMA memory allocation: mem_dev_l and mem_dev_r. Replace them with the
>> mem_dev[] array and use defines for accessing particular banks. This will
>> help to simplify code in the next patches.
> Hi Marek,
>
> The change looks good to me. One comment thought that it would be
> good to keep the left and right banks in the driver code to be in sync
> with the DT nomenclature.
>
> BANK_L_CTX instead of BANK1_CTX
> BANK_R_CTX instead of BANK2_CTX

This patch doesn't apply cleanly, could you please check and resend
rebased onto my for-v4.12/media/next branch?

[PATCH v2 04/15] media: s5p-mfc: Replace bank1/bank2 entries with an array

[Marek Szyprowski]

Internal MFC driver device structure contains two entries for keeping
addresses of the DMA memory banks. Replace them with the dma_base[] array
and use defines for accessing particular banks. This will help to simplify
code in the next patches.

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>
---
 drivers/media/platform/s5p-mfc/s5p_mfc_common.h |  6 ++--
 drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c   | 27 +++++++++++-------
 drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c | 38 +++++++++++++------------
 drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c | 10 +++----
 4 files changed, 43 insertions(+), 38 deletions(-)

diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
index 27d4c864e06e..da601a2dba2f 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
@@ -273,8 +273,7 @@ struct s5p_mfc_priv_buf {
  * @queue:		waitqueue for waiting for completion of device commands
  * @fw_size:		size of firmware
  * @fw_virt_addr:	virtual firmware address
- * @bank1:		address of the beginning of bank 1 memory
- * @bank2:		address of the beginning of bank 2 memory
+ * @dma_base[]:		address of the beginning of memory banks
  * @hw_lock:		used for hardware locking
  * @ctx:		array of driver contexts
  * @curr_ctx:		number of the currently running context
@@ -315,8 +314,7 @@ struct s5p_mfc_dev {
 	wait_queue_head_t queue;
 	size_t fw_size;
 	void *fw_virt_addr;
-	dma_addr_t bank1;
-	dma_addr_t bank2;
+	dma_addr_t dma_base[BANK_CTX_NUM];
 	unsigned long hw_lock;
 	struct s5p_mfc_ctx *ctx[MFC_NUM_CONTEXTS];
 	int curr_ctx;
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c b/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c
index cd1406c75d9a..c9bff3d0655f 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c
@@ -38,8 +38,8 @@ int s5p_mfc_alloc_firmware(struct s5p_mfc_dev *dev)
 	}
 
 	dev->fw_virt_addr = dma_alloc_coherent(dev->mem_dev[BANK1_CTX],
-					dev->fw_size, &dev->bank1, GFP_KERNEL);
-
+					dev->fw_size, &dev->dma_base[BANK1_CTX],
+					GFP_KERNEL);
 	if (!dev->fw_virt_addr) {
 		mfc_err("Allocating bitprocessor buffer failed\n");
 		return -ENOMEM;
@@ -52,7 +52,8 @@ int s5p_mfc_alloc_firmware(struct s5p_mfc_dev *dev)
 		if (!bank2_virt) {
 			mfc_err("Allocating bank2 base failed\n");
 			dma_free_coherent(dev->mem_dev[BANK1_CTX], dev->fw_size,
-					  dev->fw_virt_addr, dev->bank1);
+					  dev->fw_virt_addr,
+					  dev->dma_base[BANK1_CTX]);
 			dev->fw_virt_addr = NULL;
 			return -ENOMEM;
 		}
@@ -61,7 +62,7 @@ int s5p_mfc_alloc_firmware(struct s5p_mfc_dev *dev)
 		 * should not have address of bank2 - MFC will treat it as a null frame.
 		 * To avoid such situation we set bank2 address below the pool address.
 		 */
-		dev->bank2 = bank2_dma_addr - align_size;
+		dev->dma_base[BANK2_CTX] = bank2_dma_addr - align_size;
 
 		dma_free_coherent(dev->mem_dev[BANK2_CTX], align_size,
 				  bank2_virt, bank2_dma_addr);
@@ -70,7 +71,7 @@ int s5p_mfc_alloc_firmware(struct s5p_mfc_dev *dev)
 		/* In this case bank2 can point to the same address as bank1.
 		 * Firmware will always occupy the beginning of this area so it is
 		 * impossible having a video frame buffer with zero address. */
-		dev->bank2 = dev->bank1;
+		dev->dma_base[BANK2_CTX] = dev->dma_base[BANK1_CTX];
 	}
 	return 0;
 }
@@ -125,7 +126,7 @@ int s5p_mfc_release_firmware(struct s5p_mfc_dev *dev)
 	if (!dev->fw_virt_addr)
 		return -EINVAL;
 	dma_free_coherent(dev->mem_dev[BANK1_CTX], dev->fw_size,
-			  dev->fw_virt_addr, dev->bank1);
+			  dev->fw_virt_addr, dev->dma_base[BANK1_CTX]);
 	dev->fw_virt_addr = NULL;
 	return 0;
 }
@@ -211,13 +212,17 @@ int s5p_mfc_reset(struct s5p_mfc_dev *dev)
 static inline void s5p_mfc_init_memctrl(struct s5p_mfc_dev *dev)
 {
 	if (IS_MFCV6_PLUS(dev)) {
-		mfc_write(dev, dev->bank1, S5P_FIMV_RISC_BASE_ADDRESS_V6);
-		mfc_debug(2, "Base Address : %pad\n", &dev->bank1);
+		mfc_write(dev, dev->dma_base[BANK1_CTX],
+			  S5P_FIMV_RISC_BASE_ADDRESS_V6);
+		mfc_debug(2, "Base Address : %pad\n",
+			  &dev->dma_base[BANK1_CTX]);
 	} else {
-		mfc_write(dev, dev->bank1, S5P_FIMV_MC_DRAMBASE_ADR_A);
-		mfc_write(dev, dev->bank2, S5P_FIMV_MC_DRAMBASE_ADR_B);
+		mfc_write(dev, dev->dma_base[BANK1_CTX],
+			  S5P_FIMV_MC_DRAMBASE_ADR_A);
+		mfc_write(dev, dev->dma_base[BANK2_CTX],
+			  S5P_FIMV_MC_DRAMBASE_ADR_B);
 		mfc_debug(2, "Bank1: %pad, Bank2: %pad\n",
-				&dev->bank1, &dev->bank2);
+			  &dev->dma_base[BANK1_CTX], &dev->dma_base[BANK2_CTX]);
 	}
 }
 
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c
index 65dd3e64b4db..32ce9ade2edb 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c
@@ -30,8 +30,8 @@
 #include <linux/mm.h>
 #include <linux/sched.h>
 
-#define OFFSETA(x)		(((x) - dev->bank1) >> MFC_OFFSET_SHIFT)
-#define OFFSETB(x)		(((x) - dev->bank2) >> MFC_OFFSET_SHIFT)
+#define OFFSETA(x)		(((x) - dev->dma_base[BANK1_CTX]) >> MFC_OFFSET_SHIFT)
+#define OFFSETB(x)		(((x) - dev->dma_base[BANK2_CTX]) >> MFC_OFFSET_SHIFT)
 
 /* Allocate temporary buffers for decoding */
 static int s5p_mfc_alloc_dec_temp_buffers_v5(struct s5p_mfc_ctx *ctx)
@@ -41,8 +41,8 @@ static int s5p_mfc_alloc_dec_temp_buffers_v5(struct s5p_mfc_ctx *ctx)
 	int ret;
 
 	ctx->dsc.size = buf_size->dsc;
-	ret =  s5p_mfc_alloc_priv_buf(dev->mem_dev[BANK1_CTX], dev->bank1,
-				      &ctx->dsc);
+	ret =  s5p_mfc_alloc_priv_buf(dev->mem_dev[BANK1_CTX],
+				      dev->dma_base[BANK1_CTX], &ctx->dsc);
 	if (ret) {
 		mfc_err("Failed to allocate temporary buffer\n");
 		return ret;
@@ -174,7 +174,7 @@ static int s5p_mfc_alloc_codec_buffers_v5(struct s5p_mfc_ctx *ctx)
 	if (ctx->bank1.size > 0) {
 
 		ret = s5p_mfc_alloc_priv_buf(dev->mem_dev[BANK1_CTX],
-					     dev->bank1, &ctx->bank1);
+				     dev->dma_base[BANK1_CTX], &ctx->bank1);
 		if (ret) {
 			mfc_err("Failed to allocate Bank1 temporary buffer\n");
 			return ret;
@@ -184,7 +184,7 @@ static int s5p_mfc_alloc_codec_buffers_v5(struct s5p_mfc_ctx *ctx)
 	/* Allocate only if memory from bank 2 is necessary */
 	if (ctx->bank2.size > 0) {
 		ret = s5p_mfc_alloc_priv_buf(dev->mem_dev[BANK2_CTX],
-					     dev->bank2, &ctx->bank2);
+				     dev->dma_base[BANK2_CTX], &ctx->bank2);
 		if (ret) {
 			mfc_err("Failed to allocate Bank2 temporary buffer\n");
 			s5p_mfc_release_priv_buf(ctx->dev->mem_dev[BANK1_CTX],
@@ -216,8 +216,8 @@ static int s5p_mfc_alloc_instance_buffer_v5(struct s5p_mfc_ctx *ctx)
 	else
 		ctx->ctx.size = buf_size->non_h264_ctx;
 
-	ret = s5p_mfc_alloc_priv_buf(dev->mem_dev[BANK1_CTX], dev->bank1,
-				     &ctx->ctx);
+	ret = s5p_mfc_alloc_priv_buf(dev->mem_dev[BANK1_CTX],
+				     dev->dma_base[BANK1_CTX], &ctx->ctx);
 	if (ret) {
 		mfc_err("Failed to allocate instance buffer\n");
 		return ret;
@@ -230,8 +230,8 @@ static int s5p_mfc_alloc_instance_buffer_v5(struct s5p_mfc_ctx *ctx)
 
 	/* Initialize shared memory */
 	ctx->shm.size = buf_size->shm;
-	ret = s5p_mfc_alloc_priv_buf(dev->mem_dev[BANK1_CTX], dev->bank1,
-				     &ctx->shm);
+	ret = s5p_mfc_alloc_priv_buf(dev->mem_dev[BANK1_CTX],
+				     dev->dma_base[BANK1_CTX], &ctx->shm);
 	if (ret) {
 		mfc_err("Failed to allocate shared memory buffer\n");
 		s5p_mfc_release_priv_buf(dev->mem_dev[BANK1_CTX], &ctx->ctx);
@@ -239,7 +239,7 @@ static int s5p_mfc_alloc_instance_buffer_v5(struct s5p_mfc_ctx *ctx)
 	}
 
 	/* shared memory offset only keeps the offset from base (port a) */
-	ctx->shm.ofs = ctx->shm.dma - dev->bank1;
+	ctx->shm.ofs = ctx->shm.dma - dev->dma_base[BANK1_CTX];
 	BUG_ON(ctx->shm.ofs & ((1 << MFC_BANK1_ALIGN_ORDER) - 1));
 
 	memset(ctx->shm.virt, 0, buf_size->shm);
@@ -538,10 +538,10 @@ static void s5p_mfc_get_enc_frame_buffer_v5(struct s5p_mfc_ctx *ctx,
 {
 	struct s5p_mfc_dev *dev = ctx->dev;
 
-	*y_addr = dev->bank2 + (mfc_read(dev, S5P_FIMV_ENCODED_Y_ADDR)
-							<< MFC_OFFSET_SHIFT);
-	*c_addr = dev->bank2 + (mfc_read(dev, S5P_FIMV_ENCODED_C_ADDR)
-							<< MFC_OFFSET_SHIFT);
+	*y_addr = dev->dma_base[BANK2_CTX] +
+		  (mfc_read(dev, S5P_FIMV_ENCODED_Y_ADDR) << MFC_OFFSET_SHIFT);
+	*c_addr = dev->dma_base[BANK2_CTX] +
+		  (mfc_read(dev, S5P_FIMV_ENCODED_C_ADDR) << MFC_OFFSET_SHIFT);
 }
 
 /* Set encoding ref & codec buffer */
@@ -1218,7 +1218,8 @@ static int s5p_mfc_run_enc_frame(struct s5p_mfc_ctx *ctx)
 	}
 	if (list_empty(&ctx->src_queue)) {
 		/* send null frame */
-		s5p_mfc_set_enc_frame_buffer_v5(ctx, dev->bank2, dev->bank2);
+		s5p_mfc_set_enc_frame_buffer_v5(ctx, dev->dma_base[BANK2_CTX],
+						dev->dma_base[BANK2_CTX]);
 		src_mb = NULL;
 	} else {
 		src_mb = list_entry(ctx->src_queue.next, struct s5p_mfc_buf,
@@ -1226,8 +1227,9 @@ static int s5p_mfc_run_enc_frame(struct s5p_mfc_ctx *ctx)
 		src_mb->flags |= MFC_BUF_FLAG_USED;
 		if (src_mb->b->vb2_buf.planes[0].bytesused == 0) {
 			/* send null frame */
-			s5p_mfc_set_enc_frame_buffer_v5(ctx, dev->bank2,
-								dev->bank2);
+			s5p_mfc_set_enc_frame_buffer_v5(ctx,
+						dev->dma_base[BANK2_CTX],
+						dev->dma_base[BANK2_CTX]);
 			ctx->state = MFCINST_FINISHING;
 		} else {
 			src_y_addr = vb2_dma_contig_plane_dma_addr(
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
index d2bc938253bc..51053ed68741 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
@@ -240,7 +240,7 @@ static int s5p_mfc_alloc_codec_buffers_v6(struct s5p_mfc_ctx *ctx)
 	/* Allocate only if memory from bank 1 is necessary */
 	if (ctx->bank1.size > 0) {
 		ret = s5p_mfc_alloc_priv_buf(dev->mem_dev[BANK1_CTX],
-					     dev->bank1, &ctx->bank1);
+					dev->dma_base[BANK1_CTX], &ctx->bank1);
 		if (ret) {
 			mfc_err("Failed to allocate Bank1 memory\n");
 			return ret;
@@ -292,8 +292,8 @@ static int s5p_mfc_alloc_instance_buffer_v6(struct s5p_mfc_ctx *ctx)
 		break;
 	}
 
-	ret = s5p_mfc_alloc_priv_buf(dev->mem_dev[BANK1_CTX], dev->bank1,
-				     &ctx->ctx);
+	ret = s5p_mfc_alloc_priv_buf(dev->mem_dev[BANK1_CTX],
+				     dev->dma_base[BANK1_CTX], &ctx->ctx);
 	if (ret) {
 		mfc_err("Failed to allocate instance buffer\n");
 		return ret;
@@ -322,8 +322,8 @@ static int s5p_mfc_alloc_dev_context_buffer_v6(struct s5p_mfc_dev *dev)
 	mfc_debug_enter();
 
 	dev->ctx_buf.size = buf_size->dev_ctx;
-	ret = s5p_mfc_alloc_priv_buf(dev->mem_dev[BANK1_CTX], dev->bank1,
-				     &dev->ctx_buf);
+	ret = s5p_mfc_alloc_priv_buf(dev->mem_dev[BANK1_CTX],
+				     dev->dma_base[BANK1_CTX], &dev->ctx_buf);
 	if (ret) {
 		mfc_err("Failed to allocate device context buffer\n");
 		return ret;
-- 
1.9.1

[PATCH v2 05/15] media: s5p-mfc: Simplify alloc/release private buffer functions

[Marek Szyprowski]

Change parameters for s5p_mfc_alloc_priv_buf() and s5p_mfc_release_priv_buf()
functions. Instead of DMA device pointer and a base, provide common MFC
device structure and memory bank context identifier.

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>
---
 drivers/media/platform/s5p-mfc/s5p_mfc_common.h |  2 ++
 drivers/media/platform/s5p-mfc/s5p_mfc_opr.c    | 20 +++++++++++------
 drivers/media/platform/s5p-mfc/s5p_mfc_opr.h    |  8 +++----
 drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c | 30 ++++++++++---------------
 drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c | 15 +++++--------
 5 files changed, 37 insertions(+), 38 deletions(-)

diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
index da601a2dba2f..9cf860f34c71 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
@@ -240,12 +240,14 @@ struct s5p_mfc_variant {
  *			buffer accessed by driver
  * @dma:		DMA address, only valid when kernel DMA API used
  * @size:		size of the buffer
+ * @ctx:		memory context (bank) used for this allocation
  */
 struct s5p_mfc_priv_buf {
 	unsigned long	ofs;
 	void		*virt;
 	dma_addr_t	dma;
 	size_t		size;
+	unsigned int	ctx;
 };
 
 /**
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr.c b/drivers/media/platform/s5p-mfc/s5p_mfc_opr.c
index 99f65a92a6be..9294ee124661 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr.c
@@ -37,12 +37,16 @@ void s5p_mfc_init_regs(struct s5p_mfc_dev *dev)
 		dev->mfc_regs = s5p_mfc_init_regs_v6_plus(dev);
 }
 
-int s5p_mfc_alloc_priv_buf(struct device *dev, dma_addr_t base,
-					struct s5p_mfc_priv_buf *b)
+int s5p_mfc_alloc_priv_buf(struct s5p_mfc_dev *dev, unsigned int mem_ctx,
+			   struct s5p_mfc_priv_buf *b)
 {
+	struct device *mem_dev = dev->mem_dev[mem_ctx];
+	dma_addr_t base = dev->dma_base[mem_ctx];
+
 	mfc_debug(3, "Allocating priv: %zu\n", b->size);
 
-	b->virt = dma_alloc_coherent(dev, b->size, &b->dma, GFP_KERNEL);
+	b->ctx = mem_ctx;
+	b->virt = dma_alloc_coherent(mem_dev, b->size, &b->dma, GFP_KERNEL);
 
 	if (!b->virt) {
 		mfc_err("Allocating private buffer of size %zu failed\n",
@@ -53,7 +57,7 @@ int s5p_mfc_alloc_priv_buf(struct device *dev, dma_addr_t base,
 	if (b->dma < base) {
 		mfc_err("Invalid memory configuration - buffer (%pad) is below base memory address(%pad)\n",
 			&b->dma, &base);
-		dma_free_coherent(dev, b->size, b->virt, b->dma);
+		dma_free_coherent(mem_dev, b->size, b->virt, b->dma);
 		return -ENOMEM;
 	}
 
@@ -61,11 +65,13 @@ int s5p_mfc_alloc_priv_buf(struct device *dev, dma_addr_t base,
 	return 0;
 }
 
-void s5p_mfc_release_priv_buf(struct device *dev,
-						struct s5p_mfc_priv_buf *b)
+void s5p_mfc_release_priv_buf(struct s5p_mfc_dev *dev,
+			      struct s5p_mfc_priv_buf *b)
 {
+	struct device *mem_dev = dev->mem_dev[b->ctx];
+
 	if (b->virt) {
-		dma_free_coherent(dev, b->size, b->virt, b->dma);
+		dma_free_coherent(mem_dev, b->size, b->virt, b->dma);
 		b->virt = NULL;
 		b->dma = 0;
 		b->size = 0;
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr.h b/drivers/media/platform/s5p-mfc/s5p_mfc_opr.h
index b6ac417ab63e..108e59382e0c 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr.h
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr.h
@@ -315,10 +315,10 @@ struct s5p_mfc_hw_ops {
 
 void s5p_mfc_init_hw_ops(struct s5p_mfc_dev *dev);
 void s5p_mfc_init_regs(struct s5p_mfc_dev *dev);
-int s5p_mfc_alloc_priv_buf(struct device *dev, dma_addr_t base,
-					struct s5p_mfc_priv_buf *b);
-void s5p_mfc_release_priv_buf(struct device *dev,
-					struct s5p_mfc_priv_buf *b);
+int s5p_mfc_alloc_priv_buf(struct s5p_mfc_dev *dev, unsigned int mem_ctx,
+			   struct s5p_mfc_priv_buf *b);
+void s5p_mfc_release_priv_buf(struct s5p_mfc_dev *dev,
+			      struct s5p_mfc_priv_buf *b);
 
 
 #endif /* S5P_MFC_OPR_H_ */
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c
index 32ce9ade2edb..20e8a1bdc984 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c
@@ -41,8 +41,7 @@ static int s5p_mfc_alloc_dec_temp_buffers_v5(struct s5p_mfc_ctx *ctx)
 	int ret;
 
 	ctx->dsc.size = buf_size->dsc;
-	ret =  s5p_mfc_alloc_priv_buf(dev->mem_dev[BANK1_CTX],
-				      dev->dma_base[BANK1_CTX], &ctx->dsc);
+	ret =  s5p_mfc_alloc_priv_buf(dev, BANK1_CTX, &ctx->dsc);
 	if (ret) {
 		mfc_err("Failed to allocate temporary buffer\n");
 		return ret;
@@ -58,7 +57,7 @@ static int s5p_mfc_alloc_dec_temp_buffers_v5(struct s5p_mfc_ctx *ctx)
 /* Release temporary buffers for decoding */
 static void s5p_mfc_release_dec_desc_buffer_v5(struct s5p_mfc_ctx *ctx)
 {
-	s5p_mfc_release_priv_buf(ctx->dev->mem_dev[BANK1_CTX], &ctx->dsc);
+	s5p_mfc_release_priv_buf(ctx->dev, &ctx->dsc);
 }
 
 /* Allocate codec buffers */
@@ -173,8 +172,7 @@ static int s5p_mfc_alloc_codec_buffers_v5(struct s5p_mfc_ctx *ctx)
 	/* Allocate only if memory from bank 1 is necessary */
 	if (ctx->bank1.size > 0) {
 
-		ret = s5p_mfc_alloc_priv_buf(dev->mem_dev[BANK1_CTX],
-				     dev->dma_base[BANK1_CTX], &ctx->bank1);
+		ret = s5p_mfc_alloc_priv_buf(dev, BANK1_CTX, &ctx->bank1);
 		if (ret) {
 			mfc_err("Failed to allocate Bank1 temporary buffer\n");
 			return ret;
@@ -183,12 +181,10 @@ static int s5p_mfc_alloc_codec_buffers_v5(struct s5p_mfc_ctx *ctx)
 	}
 	/* Allocate only if memory from bank 2 is necessary */
 	if (ctx->bank2.size > 0) {
-		ret = s5p_mfc_alloc_priv_buf(dev->mem_dev[BANK2_CTX],
-				     dev->dma_base[BANK2_CTX], &ctx->bank2);
+		ret = s5p_mfc_alloc_priv_buf(dev, BANK2_CTX, &ctx->bank2);
 		if (ret) {
 			mfc_err("Failed to allocate Bank2 temporary buffer\n");
-			s5p_mfc_release_priv_buf(ctx->dev->mem_dev[BANK1_CTX],
-						 &ctx->bank1);
+			s5p_mfc_release_priv_buf(ctx->dev, &ctx->bank1);
 			return ret;
 		}
 		BUG_ON(ctx->bank2.dma & ((1 << MFC_BANK2_ALIGN_ORDER) - 1));
@@ -199,8 +195,8 @@ static int s5p_mfc_alloc_codec_buffers_v5(struct s5p_mfc_ctx *ctx)
 /* Release buffers allocated for codec */
 static void s5p_mfc_release_codec_buffers_v5(struct s5p_mfc_ctx *ctx)
 {
-	s5p_mfc_release_priv_buf(ctx->dev->mem_dev[BANK1_CTX], &ctx->bank1);
-	s5p_mfc_release_priv_buf(ctx->dev->mem_dev[BANK2_CTX], &ctx->bank2);
+	s5p_mfc_release_priv_buf(ctx->dev, &ctx->bank1);
+	s5p_mfc_release_priv_buf(ctx->dev, &ctx->bank2);
 }
 
 /* Allocate memory for instance data buffer */
@@ -216,8 +212,7 @@ static int s5p_mfc_alloc_instance_buffer_v5(struct s5p_mfc_ctx *ctx)
 	else
 		ctx->ctx.size = buf_size->non_h264_ctx;
 
-	ret = s5p_mfc_alloc_priv_buf(dev->mem_dev[BANK1_CTX],
-				     dev->dma_base[BANK1_CTX], &ctx->ctx);
+	ret = s5p_mfc_alloc_priv_buf(dev, BANK1_CTX, &ctx->ctx);
 	if (ret) {
 		mfc_err("Failed to allocate instance buffer\n");
 		return ret;
@@ -230,11 +225,10 @@ static int s5p_mfc_alloc_instance_buffer_v5(struct s5p_mfc_ctx *ctx)
 
 	/* Initialize shared memory */
 	ctx->shm.size = buf_size->shm;
-	ret = s5p_mfc_alloc_priv_buf(dev->mem_dev[BANK1_CTX],
-				     dev->dma_base[BANK1_CTX], &ctx->shm);
+	ret = s5p_mfc_alloc_priv_buf(dev, BANK1_CTX, &ctx->shm);
 	if (ret) {
 		mfc_err("Failed to allocate shared memory buffer\n");
-		s5p_mfc_release_priv_buf(dev->mem_dev[BANK1_CTX], &ctx->ctx);
+		s5p_mfc_release_priv_buf(dev, &ctx->ctx);
 		return ret;
 	}
 
@@ -250,8 +244,8 @@ static int s5p_mfc_alloc_instance_buffer_v5(struct s5p_mfc_ctx *ctx)
 /* Release instance buffer */
 static void s5p_mfc_release_instance_buffer_v5(struct s5p_mfc_ctx *ctx)
 {
-	s5p_mfc_release_priv_buf(ctx->dev->mem_dev[BANK1_CTX], &ctx->ctx);
-	s5p_mfc_release_priv_buf(ctx->dev->mem_dev[BANK1_CTX], &ctx->shm);
+	s5p_mfc_release_priv_buf(ctx->dev, &ctx->ctx);
+	s5p_mfc_release_priv_buf(ctx->dev, &ctx->shm);
 }
 
 static int s5p_mfc_alloc_dev_context_buffer_v5(struct s5p_mfc_dev *dev)
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
index 51053ed68741..4e5ead236455 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
@@ -239,8 +239,7 @@ static int s5p_mfc_alloc_codec_buffers_v6(struct s5p_mfc_ctx *ctx)
 
 	/* Allocate only if memory from bank 1 is necessary */
 	if (ctx->bank1.size > 0) {
-		ret = s5p_mfc_alloc_priv_buf(dev->mem_dev[BANK1_CTX],
-					dev->dma_base[BANK1_CTX], &ctx->bank1);
+		ret = s5p_mfc_alloc_priv_buf(dev, BANK1_CTX, &ctx->bank1);
 		if (ret) {
 			mfc_err("Failed to allocate Bank1 memory\n");
 			return ret;
@@ -253,7 +252,7 @@ static int s5p_mfc_alloc_codec_buffers_v6(struct s5p_mfc_ctx *ctx)
 /* Release buffers allocated for codec */
 static void s5p_mfc_release_codec_buffers_v6(struct s5p_mfc_ctx *ctx)
 {
-	s5p_mfc_release_priv_buf(ctx->dev->mem_dev[BANK1_CTX], &ctx->bank1);
+	s5p_mfc_release_priv_buf(ctx->dev, &ctx->bank1);
 }
 
 /* Allocate memory for instance data buffer */
@@ -292,8 +291,7 @@ static int s5p_mfc_alloc_instance_buffer_v6(struct s5p_mfc_ctx *ctx)
 		break;
 	}
 
-	ret = s5p_mfc_alloc_priv_buf(dev->mem_dev[BANK1_CTX],
-				     dev->dma_base[BANK1_CTX], &ctx->ctx);
+	ret = s5p_mfc_alloc_priv_buf(dev, BANK1_CTX, &ctx->ctx);
 	if (ret) {
 		mfc_err("Failed to allocate instance buffer\n");
 		return ret;
@@ -310,7 +308,7 @@ static int s5p_mfc_alloc_instance_buffer_v6(struct s5p_mfc_ctx *ctx)
 /* Release instance buffer */
 static void s5p_mfc_release_instance_buffer_v6(struct s5p_mfc_ctx *ctx)
 {
-	s5p_mfc_release_priv_buf(ctx->dev->mem_dev[BANK1_CTX], &ctx->ctx);
+	s5p_mfc_release_priv_buf(ctx->dev, &ctx->ctx);
 }
 
 /* Allocate context buffers for SYS_INIT */
@@ -322,8 +320,7 @@ static int s5p_mfc_alloc_dev_context_buffer_v6(struct s5p_mfc_dev *dev)
 	mfc_debug_enter();
 
 	dev->ctx_buf.size = buf_size->dev_ctx;
-	ret = s5p_mfc_alloc_priv_buf(dev->mem_dev[BANK1_CTX],
-				     dev->dma_base[BANK1_CTX], &dev->ctx_buf);
+	ret = s5p_mfc_alloc_priv_buf(dev, BANK1_CTX, &dev->ctx_buf);
 	if (ret) {
 		mfc_err("Failed to allocate device context buffer\n");
 		return ret;
@@ -340,7 +337,7 @@ static int s5p_mfc_alloc_dev_context_buffer_v6(struct s5p_mfc_dev *dev)
 /* Release context buffers for SYS_INIT */
 static void s5p_mfc_release_dev_context_buffer_v6(struct s5p_mfc_dev *dev)
 {
-	s5p_mfc_release_priv_buf(dev->mem_dev[BANK1_CTX], &dev->ctx_buf);
+	s5p_mfc_release_priv_buf(dev, &dev->ctx_buf);
 }
 
 static int calc_plane(int width, int height)
-- 
1.9.1

[PATCH v2 06/15] media: s5p-mfc: Move setting DMA max segment size to DMA configure function

[Marek Szyprowski]

Setting DMA max segment size to 32 bit mask is a part of DMA memory
configuration, so move those calls to s5p_mfc_configure_dma_memory()
function.

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>
---
 drivers/media/platform/s5p-mfc/s5p_mfc.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c b/drivers/media/platform/s5p-mfc/s5p_mfc.c
index f7664910f12c..bc1aeb25ebeb 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
@@ -1122,9 +1122,13 @@ static int s5p_mfc_configure_dma_memory(struct s5p_mfc_dev *mfc_dev)
 	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)
+		if (ret == 0) {
 			mfc_dev->mem_dev[BANK1_CTX] =
 				mfc_dev->mem_dev[BANK2_CTX] = dev;
+			vb2_dma_contig_set_max_seg_size(dev,
+							DMA_BIT_MASK(32));
+		}
+
 		return ret;
 	}
 
@@ -1143,6 +1147,11 @@ static int s5p_mfc_configure_dma_memory(struct s5p_mfc_dev *mfc_dev)
 		return -ENODEV;
 	}
 
+	vb2_dma_contig_set_max_seg_size(mfc_dev->mem_dev[BANK1_CTX],
+					DMA_BIT_MASK(32));
+	vb2_dma_contig_set_max_seg_size(mfc_dev->mem_dev[BANK2_CTX],
+					DMA_BIT_MASK(32));
+
 	return 0;
 }
 
@@ -1152,11 +1161,14 @@ static void s5p_mfc_unconfigure_dma_memory(struct s5p_mfc_dev *mfc_dev)
 
 	if (exynos_is_iommu_available(dev)) {
 		exynos_unconfigure_iommu(dev);
+		vb2_dma_contig_clear_max_seg_size(dev);
 		return;
 	}
 
 	device_unregister(mfc_dev->mem_dev[BANK1_CTX]);
 	device_unregister(mfc_dev->mem_dev[BANK2_CTX]);
+	vb2_dma_contig_clear_max_seg_size(mfc_dev->mem_dev[BANK1_CTX]);
+	vb2_dma_contig_clear_max_seg_size(mfc_dev->mem_dev[BANK2_CTX]);
 }
 
 /* MFC probe function */
@@ -1214,11 +1226,6 @@ static int s5p_mfc_probe(struct platform_device *pdev)
 		goto err_dma;
 	}
 
-	vb2_dma_contig_set_max_seg_size(dev->mem_dev[BANK1_CTX],
-					DMA_BIT_MASK(32));
-	vb2_dma_contig_set_max_seg_size(dev->mem_dev[BANK2_CTX],
-					DMA_BIT_MASK(32));
-
 	mutex_init(&dev->mfc_mutex);
 	init_waitqueue_head(&dev->queue);
 	dev->hw_lock = 0;
@@ -1351,8 +1358,6 @@ static int s5p_mfc_remove(struct platform_device *pdev)
 	v4l2_device_unregister(&dev->v4l2_dev);
 	s5p_mfc_release_firmware(dev);
 	s5p_mfc_unconfigure_dma_memory(dev);
-	vb2_dma_contig_clear_max_seg_size(dev->mem_dev[BANK1_CTX]);
-	vb2_dma_contig_clear_max_seg_size(dev->mem_dev[BANK2_CTX]);
 
 	s5p_mfc_final_pm(dev);
 	return 0;
-- 
1.9.1

[PATCH v2 07/15] media: s5p-mfc: Put firmware to private buffer structure

[Marek Szyprowski]

Use s5p_mfc_priv_buf structure for keeping the firmware image. This will
help handling of firmware buffer allocation in the next patches.

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>
---
 drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v5.c |  2 +-
 drivers/media/platform/s5p-mfc/s5p_mfc_common.h |  3 +--
 drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c   | 36 ++++++++++++-------------
 3 files changed, 20 insertions(+), 21 deletions(-)

diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v5.c b/drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v5.c
index 8c4739ca16d6..4c80bb4243be 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v5.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v5.c
@@ -47,7 +47,7 @@ static int s5p_mfc_sys_init_cmd_v5(struct s5p_mfc_dev *dev)
 	struct s5p_mfc_cmd_args h2r_args;
 
 	memset(&h2r_args, 0, sizeof(struct s5p_mfc_cmd_args));
-	h2r_args.arg[0] = dev->fw_size;
+	h2r_args.arg[0] = dev->fw_buf.size;
 	return s5p_mfc_cmd_host2risc_v5(dev, S5P_FIMV_H2R_CMD_SYS_INIT,
 			&h2r_args);
 }
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
index 9cf860f34c71..cea17a737ef7 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
@@ -314,8 +314,7 @@ struct s5p_mfc_dev {
 	int int_type;
 	unsigned int int_err;
 	wait_queue_head_t queue;
-	size_t fw_size;
-	void *fw_virt_addr;
+	struct s5p_mfc_priv_buf fw_buf;
 	dma_addr_t dma_base[BANK_CTX_NUM];
 	unsigned long hw_lock;
 	struct s5p_mfc_ctx *ctx[MFC_NUM_CONTEXTS];
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c b/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c
index c9bff3d0655f..50d698968049 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c
@@ -29,21 +29,22 @@ int s5p_mfc_alloc_firmware(struct s5p_mfc_dev *dev)
 	void *bank2_virt;
 	dma_addr_t bank2_dma_addr;
 	unsigned int align_size = 1 << MFC_BASE_ALIGN_ORDER;
+	struct s5p_mfc_priv_buf *fw_buf = &dev->fw_buf;
 
-	dev->fw_size = dev->variant->buf_size->fw;
+	fw_buf->size = dev->variant->buf_size->fw;
 
-	if (dev->fw_virt_addr) {
+	if (fw_buf->virt) {
 		mfc_err("Attempting to allocate firmware when it seems that it is already loaded\n");
 		return -ENOMEM;
 	}
 
-	dev->fw_virt_addr = dma_alloc_coherent(dev->mem_dev[BANK1_CTX],
-					dev->fw_size, &dev->dma_base[BANK1_CTX],
-					GFP_KERNEL);
-	if (!dev->fw_virt_addr) {
+	fw_buf->virt = dma_alloc_coherent(dev->mem_dev[BANK1_CTX], fw_buf->size,
+					 &fw_buf->dma, GFP_KERNEL);
+	if (!fw_buf->virt) {
 		mfc_err("Allocating bitprocessor buffer failed\n");
 		return -ENOMEM;
 	}
+	dev->dma_base[BANK1_CTX] = fw_buf->dma;
 
 	if (HAS_PORTNUM(dev) && IS_TWOPORT(dev)) {
 		bank2_virt = dma_alloc_coherent(dev->mem_dev[BANK2_CTX],
@@ -51,10 +52,9 @@ int s5p_mfc_alloc_firmware(struct s5p_mfc_dev *dev)
 
 		if (!bank2_virt) {
 			mfc_err("Allocating bank2 base failed\n");
-			dma_free_coherent(dev->mem_dev[BANK1_CTX], dev->fw_size,
-					  dev->fw_virt_addr,
-					  dev->dma_base[BANK1_CTX]);
-			dev->fw_virt_addr = NULL;
+			dma_free_coherent(dev->mem_dev[BANK1_CTX], fw_buf->size,
+					  fw_buf->virt, fw_buf->dma);
+			fw_buf->virt = NULL;
 			return -ENOMEM;
 		}
 
@@ -101,17 +101,17 @@ int s5p_mfc_load_firmware(struct s5p_mfc_dev *dev)
 		mfc_err("Firmware is not present in the /lib/firmware directory nor compiled in kernel\n");
 		return -EINVAL;
 	}
-	if (fw_blob->size > dev->fw_size) {
+	if (fw_blob->size > dev->fw_buf.size) {
 		mfc_err("MFC firmware is too big to be loaded\n");
 		release_firmware(fw_blob);
 		return -ENOMEM;
 	}
-	if (!dev->fw_virt_addr) {
+	if (!dev->fw_buf.virt) {
 		mfc_err("MFC firmware is not allocated\n");
 		release_firmware(fw_blob);
 		return -EINVAL;
 	}
-	memcpy(dev->fw_virt_addr, fw_blob->data, fw_blob->size);
+	memcpy(dev->fw_buf.virt, fw_blob->data, fw_blob->size);
 	wmb();
 	release_firmware(fw_blob);
 	mfc_debug_leave();
@@ -123,11 +123,11 @@ int s5p_mfc_release_firmware(struct s5p_mfc_dev *dev)
 {
 	/* Before calling this function one has to make sure
 	 * that MFC is no longer processing */
-	if (!dev->fw_virt_addr)
+	if (!dev->fw_buf.virt)
 		return -EINVAL;
-	dma_free_coherent(dev->mem_dev[BANK1_CTX], dev->fw_size,
-			  dev->fw_virt_addr, dev->dma_base[BANK1_CTX]);
-	dev->fw_virt_addr = NULL;
+	dma_free_coherent(dev->mem_dev[BANK1_CTX], dev->fw_buf.size,
+			  dev->fw_buf.virt, dev->fw_buf.dma);
+	dev->fw_buf.virt = NULL;
 	return 0;
 }
 
@@ -246,7 +246,7 @@ int s5p_mfc_init_hw(struct s5p_mfc_dev *dev)
 	int ret;
 
 	mfc_debug_enter();
-	if (!dev->fw_virt_addr) {
+	if (!dev->fw_buf.virt) {
 		mfc_err("Firmware memory is not allocated.\n");
 		return -EINVAL;
 	}
-- 
1.9.1

[PATCH v2 08/15] media: s5p-mfc: Move firmware allocation to DMA configure function

[Marek Szyprowski]

To complete DMA memory configuration for MFC device, allocation of the
firmware buffer is needed, because some parameters are dependant on its base
address. Till now, this has been handled in the s5p_mfc_alloc_firmware()
function. This patch moves that logic to s5p_mfc_configure_dma_memory() to
keep DMA memory related operations in a single place. This way
s5p_mfc_alloc_firmware() is simplified and does what it name says. The
other consequence of this change is moving s5p_mfc_alloc_firmware() call
from the s5p_mfc_probe() function to the s5p_mfc_configure_dma_memory().

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 drivers/media/platform/s5p-mfc/s5p_mfc.c      | 62 +++++++++++++++++++++------
 drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c | 31 --------------
 2 files changed, 49 insertions(+), 44 deletions(-)

diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c b/drivers/media/platform/s5p-mfc/s5p_mfc.c
index bc1aeb25ebeb..4403487a494a 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
@@ -1110,6 +1110,11 @@ static struct device *s5p_mfc_alloc_memdev(struct device *dev,
 static int s5p_mfc_configure_dma_memory(struct s5p_mfc_dev *mfc_dev)
 {
 	struct device *dev = &mfc_dev->plat_dev->dev;
+	void *bank2_virt;
+	dma_addr_t bank2_dma_addr;
+	unsigned long align_size = 1 << MFC_BASE_ALIGN_ORDER;
+	struct s5p_mfc_priv_buf *fw_buf = &mfc_dev->fw_buf;
+	int ret;
 
 	/*
 	 * When IOMMU is available, we cannot use the default configuration,
@@ -1122,14 +1127,21 @@ static int s5p_mfc_configure_dma_memory(struct s5p_mfc_dev *mfc_dev)
 	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[BANK1_CTX] =
-				mfc_dev->mem_dev[BANK2_CTX] = dev;
-			vb2_dma_contig_set_max_seg_size(dev,
-							DMA_BIT_MASK(32));
+		if (ret)
+			return ret;
+
+		mfc_dev->mem_dev[BANK1_CTX] = mfc_dev->mem_dev[BANK2_CTX] = dev;
+		ret = s5p_mfc_alloc_firmware(mfc_dev);
+		if (ret) {
+			exynos_unconfigure_iommu(dev);
+			return ret;
 		}
 
-		return ret;
+		mfc_dev->dma_base[BANK1_CTX] = fw_buf->dma;
+		mfc_dev->dma_base[BANK2_CTX] = fw_buf->dma;
+		vb2_dma_contig_set_max_seg_size(dev, DMA_BIT_MASK(32));
+
+		return 0;
 	}
 
 	/*
@@ -1147,6 +1159,35 @@ static int s5p_mfc_configure_dma_memory(struct s5p_mfc_dev *mfc_dev)
 		return -ENODEV;
 	}
 
+	/* Allocate memory for firmware and initialize both banks addresses */
+	ret = s5p_mfc_alloc_firmware(mfc_dev);
+	if (ret) {
+		device_unregister(mfc_dev->mem_dev[BANK2_CTX]);
+		device_unregister(mfc_dev->mem_dev[BANK1_CTX]);
+		return ret;
+	}
+
+	mfc_dev->dma_base[BANK1_CTX] = fw_buf->dma;
+
+	bank2_virt = dma_alloc_coherent(mfc_dev->mem_dev[BANK2_CTX], align_size,
+					&bank2_dma_addr, GFP_KERNEL);
+	if (!bank2_virt) {
+		mfc_err("Allocating bank2 base failed\n");
+		s5p_mfc_release_firmware(mfc_dev);
+		device_unregister(mfc_dev->mem_dev[BANK2_CTX]);
+		device_unregister(mfc_dev->mem_dev[BANK1_CTX]);
+		return -ENOMEM;
+	}
+
+	/* Valid buffers passed to MFC encoder with LAST_FRAME command
+	 * should not have address of bank2 - MFC will treat it as a null frame.
+	 * To avoid such situation we set bank2 address below the pool address.
+	 */
+	mfc_dev->dma_base[BANK2_CTX] = bank2_dma_addr - align_size;
+
+	dma_free_coherent(mfc_dev->mem_dev[BANK2_CTX], align_size, bank2_virt,
+			  bank2_dma_addr);
+
 	vb2_dma_contig_set_max_seg_size(mfc_dev->mem_dev[BANK1_CTX],
 					DMA_BIT_MASK(32));
 	vb2_dma_contig_set_max_seg_size(mfc_dev->mem_dev[BANK2_CTX],
@@ -1159,6 +1200,8 @@ static void s5p_mfc_unconfigure_dma_memory(struct s5p_mfc_dev *mfc_dev)
 {
 	struct device *dev = &mfc_dev->plat_dev->dev;
 
+	s5p_mfc_release_firmware(mfc_dev);
+
 	if (exynos_is_iommu_available(dev)) {
 		exynos_unconfigure_iommu(dev);
 		vb2_dma_contig_clear_max_seg_size(dev);
@@ -1235,10 +1278,6 @@ static int s5p_mfc_probe(struct platform_device *pdev)
 	dev->watchdog_timer.data = (unsigned long)dev;
 	dev->watchdog_timer.function = s5p_mfc_watchdog;
 
-	ret = s5p_mfc_alloc_firmware(dev);
-	if (ret)
-		goto err_res;
-
 	ret = v4l2_device_register(&pdev->dev, &dev->v4l2_dev);
 	if (ret)
 		goto err_v4l2_dev_reg;
@@ -1313,8 +1352,6 @@ static int s5p_mfc_probe(struct platform_device *pdev)
 err_dec_alloc:
 	v4l2_device_unregister(&dev->v4l2_dev);
 err_v4l2_dev_reg:
-	s5p_mfc_release_firmware(dev);
-err_res:
 	s5p_mfc_final_pm(dev);
 err_dma:
 	s5p_mfc_unconfigure_dma_memory(dev);
@@ -1356,7 +1393,6 @@ static int s5p_mfc_remove(struct platform_device *pdev)
 	video_device_release(dev->vfd_enc);
 	video_device_release(dev->vfd_dec);
 	v4l2_device_unregister(&dev->v4l2_dev);
-	s5p_mfc_release_firmware(dev);
 	s5p_mfc_unconfigure_dma_memory(dev);
 
 	s5p_mfc_final_pm(dev);
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c b/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c
index 50d698968049..b0cf3970117a 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c
@@ -26,9 +26,6 @@
 /* Allocate memory for firmware */
 int s5p_mfc_alloc_firmware(struct s5p_mfc_dev *dev)
 {
-	void *bank2_virt;
-	dma_addr_t bank2_dma_addr;
-	unsigned int align_size = 1 << MFC_BASE_ALIGN_ORDER;
 	struct s5p_mfc_priv_buf *fw_buf = &dev->fw_buf;
 
 	fw_buf->size = dev->variant->buf_size->fw;
@@ -44,35 +41,7 @@ int s5p_mfc_alloc_firmware(struct s5p_mfc_dev *dev)
 		mfc_err("Allocating bitprocessor buffer failed\n");
 		return -ENOMEM;
 	}
-	dev->dma_base[BANK1_CTX] = fw_buf->dma;
-
-	if (HAS_PORTNUM(dev) && IS_TWOPORT(dev)) {
-		bank2_virt = dma_alloc_coherent(dev->mem_dev[BANK2_CTX],
-				       align_size, &bank2_dma_addr, GFP_KERNEL);
-
-		if (!bank2_virt) {
-			mfc_err("Allocating bank2 base failed\n");
-			dma_free_coherent(dev->mem_dev[BANK1_CTX], fw_buf->size,
-					  fw_buf->virt, fw_buf->dma);
-			fw_buf->virt = NULL;
-			return -ENOMEM;
-		}
-
-		/* Valid buffers passed to MFC encoder with LAST_FRAME command
-		 * should not have address of bank2 - MFC will treat it as a null frame.
-		 * To avoid such situation we set bank2 address below the pool address.
-		 */
-		dev->dma_base[BANK2_CTX] = bank2_dma_addr - align_size;
 
-		dma_free_coherent(dev->mem_dev[BANK2_CTX], align_size,
-				  bank2_virt, bank2_dma_addr);
-
-	} else {
-		/* In this case bank2 can point to the same address as bank1.
-		 * Firmware will always occupy the beginning of this area so it is
-		 * impossible having a video frame buffer with zero address. */
-		dev->dma_base[BANK2_CTX] = dev->dma_base[BANK1_CTX];
-	}
 	return 0;
 }
 
-- 
1.9.1

Re: [PATCH v2 08/15] media: s5p-mfc: Move firmware allocation to DMA configure function

[Javier Martinez Canillas]

Hello Marek,

On 02/20/2017 10:38 AM, Marek Szyprowski wrote:
> To complete DMA memory configuration for MFC device, allocation of the
> firmware buffer is needed, because some parameters are dependant on its base
> address. Till now, this has been handled in the s5p_mfc_alloc_firmware()
> function. This patch moves that logic to s5p_mfc_configure_dma_memory() to
> keep DMA memory related operations in a single place. This way
> s5p_mfc_alloc_firmware() is simplified and does what it name says. The
> other consequence of this change is moving s5p_mfc_alloc_firmware() call
> from the s5p_mfc_probe() function to the s5p_mfc_configure_dma_memory().
> 
> 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 v2 08/15] media: s5p-mfc: Move firmware allocation to DMA configure function

[Shuah Khan]

On Mon, Feb 20, 2017 at 6:38 AM, Marek Szyprowski
<m.szyprowski@samsung.com> wrote:
> To complete DMA memory configuration for MFC device, allocation of the
> firmware buffer is needed, because some parameters are dependant on its base
> address. Till now, this has been handled in the s5p_mfc_alloc_firmware()
> function. This patch moves that logic to s5p_mfc_configure_dma_memory() to
> keep DMA memory related operations in a single place. This way
> s5p_mfc_alloc_firmware() is simplified and does what it name says. The
> other consequence of this change is moving s5p_mfc_alloc_firmware() call
> from the s5p_mfc_probe() function to the s5p_mfc_configure_dma_memory().

Overall looks good. This patch makes subtle change in the dma and firwmare
initialization sequence. Might be okay, but wanted to call out just in case,

Before this change:
vb2_dma_contig_set_max_seg_size() is done for both iommu and non-iommu
case before s5p_mfc_alloc_firmware(). With this change setting
dma_contig max size happens after s5p_mfc_alloc_firmware(). From what
I can tell this might not be an issue.
vb2_dma_contig_clear_max_seg_size() still happens after
s5p_mfc_release_firmware(), so that part hasn't changed.

Do any of the dma_* calls made from s5p_mfc_alloc_firmware() and later
during the dma congiguration sequence depend on dmap_parms being
allocated? Doesn't looks like it from what I can tell, but safe to
ask.

thanks,
-- Shuah

>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  drivers/media/platform/s5p-mfc/s5p_mfc.c      | 62 +++++++++++++++++++++------
>  drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c | 31 --------------
>  2 files changed, 49 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c b/drivers/media/platform/s5p-mfc/s5p_mfc.c
> index bc1aeb25ebeb..4403487a494a 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
> @@ -1110,6 +1110,11 @@ static struct device *s5p_mfc_alloc_memdev(struct device *dev,
>  static int s5p_mfc_configure_dma_memory(struct s5p_mfc_dev *mfc_dev)
>  {
>         struct device *dev = &mfc_dev->plat_dev->dev;
> +       void *bank2_virt;
> +       dma_addr_t bank2_dma_addr;
> +       unsigned long align_size = 1 << MFC_BASE_ALIGN_ORDER;
> +       struct s5p_mfc_priv_buf *fw_buf = &mfc_dev->fw_buf;
> +       int ret;
>
>         /*
>          * When IOMMU is available, we cannot use the default configuration,
> @@ -1122,14 +1127,21 @@ static int s5p_mfc_configure_dma_memory(struct s5p_mfc_dev *mfc_dev)
>         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[BANK1_CTX] =
> -                               mfc_dev->mem_dev[BANK2_CTX] = dev;
> -                       vb2_dma_contig_set_max_seg_size(dev,
> -                                                       DMA_BIT_MASK(32));
> +               if (ret)
> +                       return ret;
> +
> +               mfc_dev->mem_dev[BANK1_CTX] = mfc_dev->mem_dev[BANK2_CTX] = dev;
> +               ret = s5p_mfc_alloc_firmware(mfc_dev);
> +               if (ret) {
> +                       exynos_unconfigure_iommu(dev);
> +                       return ret;
>                 }
>
> -               return ret;
> +               mfc_dev->dma_base[BANK1_CTX] = fw_buf->dma;
> +               mfc_dev->dma_base[BANK2_CTX] = fw_buf->dma;
> +               vb2_dma_contig_set_max_seg_size(dev, DMA_BIT_MASK(32));
> +
> +               return 0;
>         }
>
>         /*
> @@ -1147,6 +1159,35 @@ static int s5p_mfc_configure_dma_memory(struct s5p_mfc_dev *mfc_dev)
>                 return -ENODEV;
>         }
>
> +       /* Allocate memory for firmware and initialize both banks addresses */
> +       ret = s5p_mfc_alloc_firmware(mfc_dev);
> +       if (ret) {
> +               device_unregister(mfc_dev->mem_dev[BANK2_CTX]);
> +               device_unregister(mfc_dev->mem_dev[BANK1_CTX]);
> +               return ret;
> +       }
> +
> +       mfc_dev->dma_base[BANK1_CTX] = fw_buf->dma;
> +
> +       bank2_virt = dma_alloc_coherent(mfc_dev->mem_dev[BANK2_CTX], align_size,
> +                                       &bank2_dma_addr, GFP_KERNEL);
> +       if (!bank2_virt) {
> +               mfc_err("Allocating bank2 base failed\n");
> +               s5p_mfc_release_firmware(mfc_dev);
> +               device_unregister(mfc_dev->mem_dev[BANK2_CTX]);
> +               device_unregister(mfc_dev->mem_dev[BANK1_CTX]);
> +               return -ENOMEM;
> +       }
> +
> +       /* Valid buffers passed to MFC encoder with LAST_FRAME command
> +        * should not have address of bank2 - MFC will treat it as a null frame.
> +        * To avoid such situation we set bank2 address below the pool address.
> +        */
> +       mfc_dev->dma_base[BANK2_CTX] = bank2_dma_addr - align_size;
> +
> +       dma_free_coherent(mfc_dev->mem_dev[BANK2_CTX], align_size, bank2_virt,
> +                         bank2_dma_addr);
> +
>         vb2_dma_contig_set_max_seg_size(mfc_dev->mem_dev[BANK1_CTX],
>                                         DMA_BIT_MASK(32));
>         vb2_dma_contig_set_max_seg_size(mfc_dev->mem_dev[BANK2_CTX],
> @@ -1159,6 +1200,8 @@ static void s5p_mfc_unconfigure_dma_memory(struct s5p_mfc_dev *mfc_dev)
>  {
>         struct device *dev = &mfc_dev->plat_dev->dev;
>
> +       s5p_mfc_release_firmware(mfc_dev);
> +
>         if (exynos_is_iommu_available(dev)) {
>                 exynos_unconfigure_iommu(dev);
>                 vb2_dma_contig_clear_max_seg_size(dev);
> @@ -1235,10 +1278,6 @@ static int s5p_mfc_probe(struct platform_device *pdev)
>         dev->watchdog_timer.data = (unsigned long)dev;
>         dev->watchdog_timer.function = s5p_mfc_watchdog;
>
> -       ret = s5p_mfc_alloc_firmware(dev);
> -       if (ret)
> -               goto err_res;
> -
>         ret = v4l2_device_register(&pdev->dev, &dev->v4l2_dev);
>         if (ret)
>                 goto err_v4l2_dev_reg;
> @@ -1313,8 +1352,6 @@ static int s5p_mfc_probe(struct platform_device *pdev)
>  err_dec_alloc:
>         v4l2_device_unregister(&dev->v4l2_dev);
>  err_v4l2_dev_reg:
> -       s5p_mfc_release_firmware(dev);
> -err_res:
>         s5p_mfc_final_pm(dev);
>  err_dma:
>         s5p_mfc_unconfigure_dma_memory(dev);
> @@ -1356,7 +1393,6 @@ static int s5p_mfc_remove(struct platform_device *pdev)
>         video_device_release(dev->vfd_enc);
>         video_device_release(dev->vfd_dec);
>         v4l2_device_unregister(&dev->v4l2_dev);
> -       s5p_mfc_release_firmware(dev);
>         s5p_mfc_unconfigure_dma_memory(dev);
>
>         s5p_mfc_final_pm(dev);
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c b/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c
> index 50d698968049..b0cf3970117a 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c
> @@ -26,9 +26,6 @@
>  /* Allocate memory for firmware */
>  int s5p_mfc_alloc_firmware(struct s5p_mfc_dev *dev)
>  {
> -       void *bank2_virt;
> -       dma_addr_t bank2_dma_addr;
> -       unsigned int align_size = 1 << MFC_BASE_ALIGN_ORDER;
>         struct s5p_mfc_priv_buf *fw_buf = &dev->fw_buf;
>
>         fw_buf->size = dev->variant->buf_size->fw;
> @@ -44,35 +41,7 @@ int s5p_mfc_alloc_firmware(struct s5p_mfc_dev *dev)
>                 mfc_err("Allocating bitprocessor buffer failed\n");
>                 return -ENOMEM;
>         }
> -       dev->dma_base[BANK1_CTX] = fw_buf->dma;
> -
> -       if (HAS_PORTNUM(dev) && IS_TWOPORT(dev)) {
> -               bank2_virt = dma_alloc_coherent(dev->mem_dev[BANK2_CTX],
> -                                      align_size, &bank2_dma_addr, GFP_KERNEL);
> -
> -               if (!bank2_virt) {
> -                       mfc_err("Allocating bank2 base failed\n");
> -                       dma_free_coherent(dev->mem_dev[BANK1_CTX], fw_buf->size,
> -                                         fw_buf->virt, fw_buf->dma);
> -                       fw_buf->virt = NULL;
> -                       return -ENOMEM;
> -               }
> -
> -               /* Valid buffers passed to MFC encoder with LAST_FRAME command
> -                * should not have address of bank2 - MFC will treat it as a null frame.
> -                * To avoid such situation we set bank2 address below the pool address.
> -                */
> -               dev->dma_base[BANK2_CTX] = bank2_dma_addr - align_size;
>
> -               dma_free_coherent(dev->mem_dev[BANK2_CTX], align_size,
> -                                 bank2_virt, bank2_dma_addr);
> -
> -       } else {
> -               /* In this case bank2 can point to the same address as bank1.
> -                * Firmware will always occupy the beginning of this area so it is
> -                * impossible having a video frame buffer with zero address. */
> -               dev->dma_base[BANK2_CTX] = dev->dma_base[BANK1_CTX];
> -       }
>         return 0;
>  }
>
> --
> 1.9.1
>

Re: [PATCH v2 08/15] media: s5p-mfc: Move firmware allocation to DMA configure function

[Marek Szyprowski]

Hi Shuah,


On 2017-02-22 19:07, Shuah Khan wrote:
> On Mon, Feb 20, 2017 at 6:38 AM, Marek Szyprowski
> <m.szyprowski@samsung.com> wrote:
>> To complete DMA memory configuration for MFC device, allocation of the
>> firmware buffer is needed, because some parameters are dependant on its base
>> address. Till now, this has been handled in the s5p_mfc_alloc_firmware()
>> function. This patch moves that logic to s5p_mfc_configure_dma_memory() to
>> keep DMA memory related operations in a single place. This way
>> s5p_mfc_alloc_firmware() is simplified and does what it name says. The
>> other consequence of this change is moving s5p_mfc_alloc_firmware() call
>> from the s5p_mfc_probe() function to the s5p_mfc_configure_dma_memory().
> Overall looks good. This patch makes subtle change in the dma and firwmare
> initialization sequence. Might be okay, but wanted to call out just in case,
>
> Before this change:
> vb2_dma_contig_set_max_seg_size() is done for both iommu and non-iommu
> case before s5p_mfc_alloc_firmware(). With this change setting
> dma_contig max size happens after s5p_mfc_alloc_firmware(). From what
> I can tell this might not be an issue.
> vb2_dma_contig_clear_max_seg_size() still happens after
> s5p_mfc_release_firmware(), so that part hasn't changed.
>
> Do any of the dma_* calls made from s5p_mfc_alloc_firmware() and later
> during the dma congiguration sequence depend on dmap_parms being
> allocated? Doesn't looks like it from what I can tell, but safe to
> ask.

Firmware allocation doesn't depend on dma max segment size at all. The only
calls which might depend on it are dma_map_sg(), which are performed much
later as a part of video buffer allocation/preparation in videobuf2, when
dma-buf or user pointer v4l2 modes are selected.

 > [...]

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

[PATCH v2 09/15] media: s5p-mfc: Allocate firmware with internal private buffer alloc function

[Marek Szyprowski]

Once firmware buffer has been converted to use s5p_mfc_priv_buf structure,
it is possible to allocate it with existing s5p_mfc_alloc_priv_buf()
function. This change will help to reduce code variants in the next
patches.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>
---
 drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c b/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c
index b0cf3970117a..a1811ee538bd 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c
@@ -27,6 +27,7 @@
 int s5p_mfc_alloc_firmware(struct s5p_mfc_dev *dev)
 {
 	struct s5p_mfc_priv_buf *fw_buf = &dev->fw_buf;
+	int err;
 
 	fw_buf->size = dev->variant->buf_size->fw;
 
@@ -35,11 +36,10 @@ int s5p_mfc_alloc_firmware(struct s5p_mfc_dev *dev)
 		return -ENOMEM;
 	}
 
-	fw_buf->virt = dma_alloc_coherent(dev->mem_dev[BANK1_CTX], fw_buf->size,
-					 &fw_buf->dma, GFP_KERNEL);
-	if (!fw_buf->virt) {
+	err = s5p_mfc_alloc_priv_buf(dev, BANK1_CTX, &dev->fw_buf);
+	if (err) {
 		mfc_err("Allocating bitprocessor buffer failed\n");
-		return -ENOMEM;
+		return err;
 	}
 
 	return 0;
@@ -92,11 +92,7 @@ int s5p_mfc_release_firmware(struct s5p_mfc_dev *dev)
 {
 	/* Before calling this function one has to make sure
 	 * that MFC is no longer processing */
-	if (!dev->fw_buf.virt)
-		return -EINVAL;
-	dma_free_coherent(dev->mem_dev[BANK1_CTX], dev->fw_buf.size,
-			  dev->fw_buf.virt, dev->fw_buf.dma);
-	dev->fw_buf.virt = NULL;
+	s5p_mfc_release_priv_buf(dev, &dev->fw_buf);
 	return 0;
 }
 
-- 
1.9.1

[PATCH v2 10/15] media: s5p-mfc: Reduce firmware buffer size for MFC v6+ variants

[Marek Szyprowski]

Firmware for MFC v6+ variants is not larger than 400 KiB, so there is no
need to allocate a full 1 MiB buffer for it. Reduce it to 512 KiB to keep
proper alignment of allocated buffer.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>
---
 drivers/media/platform/s5p-mfc/regs-mfc-v6.h | 2 +-
 drivers/media/platform/s5p-mfc/regs-mfc-v7.h | 2 +-
 drivers/media/platform/s5p-mfc/regs-mfc-v8.h | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/s5p-mfc/regs-mfc-v6.h b/drivers/media/platform/s5p-mfc/regs-mfc-v6.h
index d2cd35916dc5..c0166ee9a455 100644
--- a/drivers/media/platform/s5p-mfc/regs-mfc-v6.h
+++ b/drivers/media/platform/s5p-mfc/regs-mfc-v6.h
@@ -403,7 +403,7 @@
 #define MFC_OTHER_ENC_CTX_BUF_SIZE_V6	(12 * SZ_1K)	/*  12KB */
 
 /* MFCv6 variant defines */
-#define MAX_FW_SIZE_V6			(SZ_1M)		/* 1MB */
+#define MAX_FW_SIZE_V6			(SZ_512K)	/* 512KB */
 #define MAX_CPB_SIZE_V6			(3 * SZ_1M)	/* 3MB */
 #define MFC_VERSION_V6			0x61
 #define MFC_NUM_PORTS_V6		1
diff --git a/drivers/media/platform/s5p-mfc/regs-mfc-v7.h b/drivers/media/platform/s5p-mfc/regs-mfc-v7.h
index 1a5c6fdf7846..9f220769d970 100644
--- a/drivers/media/platform/s5p-mfc/regs-mfc-v7.h
+++ b/drivers/media/platform/s5p-mfc/regs-mfc-v7.h
@@ -34,7 +34,7 @@
 #define S5P_FIMV_E_VP8_NUM_T_LAYER_V7			0xfdc4
 
 /* MFCv7 variant defines */
-#define MAX_FW_SIZE_V7			(SZ_1M)		/* 1MB */
+#define MAX_FW_SIZE_V7			(SZ_512K)	/* 512KB */
 #define MAX_CPB_SIZE_V7			(3 * SZ_1M)	/* 3MB */
 #define MFC_VERSION_V7			0x72
 #define MFC_NUM_PORTS_V7		1
diff --git a/drivers/media/platform/s5p-mfc/regs-mfc-v8.h b/drivers/media/platform/s5p-mfc/regs-mfc-v8.h
index 4d1c3750eb5e..75f5f7511d72 100644
--- a/drivers/media/platform/s5p-mfc/regs-mfc-v8.h
+++ b/drivers/media/platform/s5p-mfc/regs-mfc-v8.h
@@ -116,7 +116,7 @@
 #define S5P_FIMV_D_ALIGN_PLANE_SIZE_V8	64
 
 /* MFCv8 variant defines */
-#define MAX_FW_SIZE_V8			(SZ_1M)		/* 1MB */
+#define MAX_FW_SIZE_V8			(SZ_512K)	/* 512KB */
 #define MAX_CPB_SIZE_V8			(3 * SZ_1M)	/* 3MB */
 #define MFC_VERSION_V8			0x80
 #define MFC_NUM_PORTS_V8		1
-- 
1.9.1

[PATCH v2 11/15] media: s5p-mfc: Split variant DMA memory configuration into separate functions

[Marek Szyprowski]

Move code for DMA memory configuration with IOMMU into separate function
to make it easier to compare what is being done in each case.

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>
---
 drivers/media/platform/s5p-mfc/s5p_mfc.c | 102 ++++++++++++++++++-------------
 1 file changed, 61 insertions(+), 41 deletions(-)

diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c b/drivers/media/platform/s5p-mfc/s5p_mfc.c
index 4403487a494a..04067bcc3feb 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
@@ -1107,44 +1107,15 @@ static struct device *s5p_mfc_alloc_memdev(struct device *dev,
 	return NULL;
 }
 
-static int s5p_mfc_configure_dma_memory(struct s5p_mfc_dev *mfc_dev)
+static int s5p_mfc_configure_2port_memory(struct s5p_mfc_dev *mfc_dev)
 {
 	struct device *dev = &mfc_dev->plat_dev->dev;
 	void *bank2_virt;
 	dma_addr_t bank2_dma_addr;
 	unsigned long align_size = 1 << MFC_BASE_ALIGN_ORDER;
-	struct s5p_mfc_priv_buf *fw_buf = &mfc_dev->fw_buf;
 	int ret;
 
 	/*
-	 * 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)
-			return ret;
-
-		mfc_dev->mem_dev[BANK1_CTX] = mfc_dev->mem_dev[BANK2_CTX] = dev;
-		ret = s5p_mfc_alloc_firmware(mfc_dev);
-		if (ret) {
-			exynos_unconfigure_iommu(dev);
-			return ret;
-		}
-
-		mfc_dev->dma_base[BANK1_CTX] = fw_buf->dma;
-		mfc_dev->dma_base[BANK2_CTX] = fw_buf->dma;
-		vb2_dma_contig_set_max_seg_size(dev, DMA_BIT_MASK(32));
-
-		return 0;
-	}
-
-	/*
 	 * Create and initialize virtual devices for accessing
 	 * reserved memory regions.
 	 */
@@ -1167,7 +1138,7 @@ static int s5p_mfc_configure_dma_memory(struct s5p_mfc_dev *mfc_dev)
 		return ret;
 	}
 
-	mfc_dev->dma_base[BANK1_CTX] = fw_buf->dma;
+	mfc_dev->dma_base[BANK1_CTX] = mfc_dev->fw_buf.dma;
 
 	bank2_virt = dma_alloc_coherent(mfc_dev->mem_dev[BANK2_CTX], align_size,
 					&bank2_dma_addr, GFP_KERNEL);
@@ -1196,22 +1167,71 @@ static int s5p_mfc_configure_dma_memory(struct s5p_mfc_dev *mfc_dev)
 	return 0;
 }
 
-static void s5p_mfc_unconfigure_dma_memory(struct s5p_mfc_dev *mfc_dev)
+static void s5p_mfc_unconfigure_2port_memory(struct s5p_mfc_dev *mfc_dev)
 {
-	struct device *dev = &mfc_dev->plat_dev->dev;
+	device_unregister(mfc_dev->mem_dev[BANK1_CTX]);
+	device_unregister(mfc_dev->mem_dev[BANK2_CTX]);
+	vb2_dma_contig_clear_max_seg_size(mfc_dev->mem_dev[BANK1_CTX]);
+	vb2_dma_contig_clear_max_seg_size(mfc_dev->mem_dev[BANK2_CTX]);
+}
 
-	s5p_mfc_release_firmware(mfc_dev);
+static int s5p_mfc_configure_common_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.
+	 */
+	int ret = exynos_configure_iommu(dev, S5P_MFC_IOMMU_DMA_BASE,
+					 S5P_MFC_IOMMU_DMA_SIZE);
+	if (ret)
+		return ret;
 
-	if (exynos_is_iommu_available(dev)) {
+	mfc_dev->mem_dev[BANK1_CTX] = mfc_dev->mem_dev[BANK2_CTX] = dev;
+	ret = s5p_mfc_alloc_firmware(mfc_dev);
+	if (ret) {
 		exynos_unconfigure_iommu(dev);
-		vb2_dma_contig_clear_max_seg_size(dev);
-		return;
+		return ret;
 	}
 
-	device_unregister(mfc_dev->mem_dev[BANK1_CTX]);
-	device_unregister(mfc_dev->mem_dev[BANK2_CTX]);
-	vb2_dma_contig_clear_max_seg_size(mfc_dev->mem_dev[BANK1_CTX]);
-	vb2_dma_contig_clear_max_seg_size(mfc_dev->mem_dev[BANK2_CTX]);
+	mfc_dev->dma_base[BANK1_CTX] = mfc_dev->fw_buf.dma;
+	mfc_dev->dma_base[BANK2_CTX] = mfc_dev->fw_buf.dma;
+	vb2_dma_contig_set_max_seg_size(dev, DMA_BIT_MASK(32));
+
+	return 0;
+}
+
+static void s5p_mfc_unconfigure_common_memory(struct s5p_mfc_dev *mfc_dev)
+{
+	struct device *dev = &mfc_dev->plat_dev->dev;
+
+	exynos_unconfigure_iommu(dev);
+	vb2_dma_contig_clear_max_seg_size(dev);
+}
+
+static int s5p_mfc_configure_dma_memory(struct s5p_mfc_dev *mfc_dev)
+{
+	struct device *dev = &mfc_dev->plat_dev->dev;
+
+	if (exynos_is_iommu_available(dev))
+		return s5p_mfc_configure_common_memory(mfc_dev);
+	else
+		return s5p_mfc_configure_2port_memory(mfc_dev);
+}
+
+static void s5p_mfc_unconfigure_dma_memory(struct s5p_mfc_dev *mfc_dev)
+{
+	struct device *dev = &mfc_dev->plat_dev->dev;
+
+	s5p_mfc_release_firmware(mfc_dev);
+	if (exynos_is_iommu_available(dev))
+		s5p_mfc_unconfigure_common_memory(mfc_dev);
+	else
+		s5p_mfc_unconfigure_2port_memory(mfc_dev);
 }
 
 /* MFC probe function */
-- 
1.9.1

[PATCH v2 12/15] media: s5p-mfc: Add support for probe-time preallocated block based allocator

[Marek Szyprowski]

Current MFC driver depends on the fact that when IOMMU is available, the
DMA-mapping framework and its IOMMU glue will use first-fit allocator.
This was true for ARM architecture, but its not for ARM64 arch. However, in
case of MFC v6+ hardware and latest firmware, it turned out that there is
no strict requirement for ALL buffers to be allocated on higher addresses
than the firmware base. This requirement is true only for the device and
per-context buffers. All video data buffers can be allocated anywhere for
all MFC v6+ versions.

Such relaxed requirements for the memory buffers can be easily fulfilled
by allocating firmware, device and per-context buffers from the probe-time
preallocated larger buffer. This patch adds support for it. This way the
driver finally works fine on ARM64 architecture. The size of the
preallocated buffer is 8 MiB, what is enough for three instances H264
decoders or encoders (other codecs have smaller memory requirements).
If one needs more for particular use case, one can use "mem" module
parameter to force larger (or smaller) buffer (for example by adding
"s5p_mfc.mem=16M" to kernel command line).

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>
---
 drivers/media/platform/s5p-mfc/s5p_mfc.c        | 43 ++++++++++++++++---
 drivers/media/platform/s5p-mfc/s5p_mfc_common.h |  4 ++
 drivers/media/platform/s5p-mfc/s5p_mfc_opr.c    | 57 ++++++++++++++++---------
 3 files changed, 79 insertions(+), 25 deletions(-)

diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c b/drivers/media/platform/s5p-mfc/s5p_mfc.c
index 04067bcc3feb..1c5ec8257f4f 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
@@ -43,6 +43,10 @@
 module_param_named(debug, mfc_debug_level, int, S_IRUGO | S_IWUSR);
 MODULE_PARM_DESC(debug, "Debug level - higher value produces more verbose messages");
 
+static char *mfc_mem_size = NULL;
+module_param_named(mem, mfc_mem_size, charp, S_IRUGO | S_IWUSR);
+MODULE_PARM_DESC(mem, "Preallocated memory size for the firmware and context buffers");
+
 /* Helper functions for interrupt processing */
 
 /* Remove from hw execution round robin */
@@ -1178,6 +1182,8 @@ static void s5p_mfc_unconfigure_2port_memory(struct s5p_mfc_dev *mfc_dev)
 static int s5p_mfc_configure_common_memory(struct s5p_mfc_dev *mfc_dev)
 {
 	struct device *dev = &mfc_dev->plat_dev->dev;
+	unsigned long mem_size = SZ_8M;
+	unsigned int bitmap_size;
 	/*
 	 * When IOMMU is available, we cannot use the default configuration,
 	 * because of MFC firmware requirements: address space limited to
@@ -1191,17 +1197,39 @@ static int s5p_mfc_configure_common_memory(struct s5p_mfc_dev *mfc_dev)
 	if (ret)
 		return ret;
 
-	mfc_dev->mem_dev[BANK1_CTX] = mfc_dev->mem_dev[BANK2_CTX] = dev;
-	ret = s5p_mfc_alloc_firmware(mfc_dev);
-	if (ret) {
+	if (mfc_mem_size)
+		mem_size = memparse(mfc_mem_size, NULL);
+
+	bitmap_size = BITS_TO_LONGS(mem_size >> PAGE_SHIFT) * sizeof(long);
+
+	mfc_dev->mem_bitmap = kzalloc(bitmap_size, GFP_KERNEL);
+	if (!mfc_dev->mem_bitmap) {
 		exynos_unconfigure_iommu(dev);
-		return ret;
+		return -ENOMEM;
 	}
 
-	mfc_dev->dma_base[BANK1_CTX] = mfc_dev->fw_buf.dma;
-	mfc_dev->dma_base[BANK2_CTX] = mfc_dev->fw_buf.dma;
+	mfc_dev->mem_virt = dma_alloc_coherent(dev, mem_size,
+					       &mfc_dev->mem_base, GFP_KERNEL);
+	if (!mfc_dev->mem_virt) {
+		kfree(mfc_dev->mem_bitmap);
+		dev_err(dev, "failed to preallocate %ld MiB for the firmware and context buffers\n",
+			(mem_size / SZ_1M));
+		exynos_unconfigure_iommu(dev);
+		return -ENOMEM;
+	}
+	mfc_dev->mem_size = mem_size;
+	mfc_dev->dma_base[BANK1_CTX] = mfc_dev->mem_base;
+	mfc_dev->dma_base[BANK2_CTX] = mfc_dev->mem_base;
+
+	/* Firmware allocation cannot fail in this case */
+	s5p_mfc_alloc_firmware(mfc_dev);
+
+	mfc_dev->mem_dev[BANK1_CTX] = mfc_dev->mem_dev[BANK2_CTX] = dev;
 	vb2_dma_contig_set_max_seg_size(dev, DMA_BIT_MASK(32));
 
+	dev_info(dev, "preallocated %ld MiB buffer for the firmware and context buffers\n",
+		 (mem_size / SZ_1M));
+
 	return 0;
 }
 
@@ -1210,6 +1238,9 @@ static void s5p_mfc_unconfigure_common_memory(struct s5p_mfc_dev *mfc_dev)
 	struct device *dev = &mfc_dev->plat_dev->dev;
 
 	exynos_unconfigure_iommu(dev);
+	dma_free_coherent(dev, mfc_dev->mem_size, mfc_dev->mem_virt,
+			  mfc_dev->mem_base);
+	kfree(mfc_dev->mem_bitmap);
 	vb2_dma_contig_clear_max_seg_size(dev);
 }
 
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
index cea17a737ef7..e64dc6e3c75e 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
@@ -315,6 +315,10 @@ struct s5p_mfc_dev {
 	unsigned int int_err;
 	wait_queue_head_t queue;
 	struct s5p_mfc_priv_buf fw_buf;
+	size_t mem_size;
+	dma_addr_t mem_base;
+	unsigned long *mem_bitmap;
+	void *mem_virt;
 	dma_addr_t dma_base[BANK_CTX_NUM];
 	unsigned long hw_lock;
 	struct s5p_mfc_ctx *ctx[MFC_NUM_CONTEXTS];
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr.c b/drivers/media/platform/s5p-mfc/s5p_mfc_opr.c
index 9294ee124661..34a66189d980 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr.c
@@ -40,41 +40,60 @@ void s5p_mfc_init_regs(struct s5p_mfc_dev *dev)
 int s5p_mfc_alloc_priv_buf(struct s5p_mfc_dev *dev, unsigned int mem_ctx,
 			   struct s5p_mfc_priv_buf *b)
 {
-	struct device *mem_dev = dev->mem_dev[mem_ctx];
-	dma_addr_t base = dev->dma_base[mem_ctx];
+	unsigned int bits = dev->mem_size >> PAGE_SHIFT;
+	unsigned int count = b->size >> PAGE_SHIFT;
+	unsigned int align = (SZ_64K >> PAGE_SHIFT) - 1;
+	unsigned int start, offset;
 
 	mfc_debug(3, "Allocating priv: %zu\n", b->size);
 
-	b->ctx = mem_ctx;
-	b->virt = dma_alloc_coherent(mem_dev, b->size, &b->dma, GFP_KERNEL);
+	if (dev->mem_virt) {
+		start = bitmap_find_next_zero_area(dev->mem_bitmap, bits, 0, count, align);
+		if (start > bits)
+			goto no_mem;
 
-	if (!b->virt) {
-		mfc_err("Allocating private buffer of size %zu failed\n",
-			b->size);
-		return -ENOMEM;
-	}
+		bitmap_set(dev->mem_bitmap, start, count);
+		offset = start << PAGE_SHIFT;
+		b->virt = dev->mem_virt + offset;
+		b->dma = dev->mem_base + offset;
+	} else {
+		struct device *mem_dev = dev->mem_dev[mem_ctx];
+		dma_addr_t base = dev->dma_base[mem_ctx];
 
-	if (b->dma < base) {
-		mfc_err("Invalid memory configuration - buffer (%pad) is below base memory address(%pad)\n",
-			&b->dma, &base);
-		dma_free_coherent(mem_dev, b->size, b->virt, b->dma);
-		return -ENOMEM;
+		b->ctx = mem_ctx;
+		b->virt = dma_alloc_coherent(mem_dev, b->size, &b->dma, GFP_KERNEL);
+		if (!b->virt)
+			goto no_mem;
+		if (b->dma < base) {
+			mfc_err("Invalid memory configuration - buffer (%pad) is below base memory address(%pad)\n",
+				&b->dma, &base);
+			dma_free_coherent(mem_dev, b->size, b->virt, b->dma);
+			return -ENOMEM;
+		}
 	}
 
 	mfc_debug(3, "Allocated addr %p %pad\n", b->virt, &b->dma);
 	return 0;
+no_mem:
+	mfc_err("Allocating private buffer of size %zu failed\n", b->size);
+	return -ENOMEM;
 }
 
 void s5p_mfc_release_priv_buf(struct s5p_mfc_dev *dev,
 			      struct s5p_mfc_priv_buf *b)
 {
-	struct device *mem_dev = dev->mem_dev[b->ctx];
+	if (dev->mem_virt) {
+		unsigned int start = (b->dma - dev->mem_base) >> PAGE_SHIFT;
+		unsigned int count = b->size >> PAGE_SHIFT;
+
+		bitmap_clear(dev->mem_bitmap, start, count);
+	} else {
+		struct device *mem_dev = dev->mem_dev[b->ctx];
 
-	if (b->virt) {
 		dma_free_coherent(mem_dev, b->size, b->virt, b->dma);
-		b->virt = NULL;
-		b->dma = 0;
-		b->size = 0;
 	}
+	b->virt = NULL;
+	b->dma = 0;
+	b->size = 0;
 }
 
-- 
1.9.1

[PATCH v2 13/15] media: s5p-mfc: Remove special configuration of IOMMU domain

[Marek Szyprowski]

The main reason for using special configuration of IOMMU domain was the
problem with MFC firmware, which failed to operate properly when placed
at 0 DMA address. Instead of adding custom code for configuring each
variant of IOMMU domain and architecture specific glue code, simply use
what arch code provides and if the DMA base address equals zero, skip
first 128 KiB to keep required alignment. This patch also make the driver
operational on ARM64 architecture, because it no longer depends on ARM
specific DMA-mapping and IOMMU glue code functions.

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>
---
 drivers/media/platform/s5p-mfc/s5p_mfc.c       | 30 +++++++--------
 drivers/media/platform/s5p-mfc/s5p_mfc_iommu.h | 51 +-------------------------
 2 files changed, 14 insertions(+), 67 deletions(-)

diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c b/drivers/media/platform/s5p-mfc/s5p_mfc.c
index 1c5ec8257f4f..b70cbd637851 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
@@ -1184,18 +1184,6 @@ static int s5p_mfc_configure_common_memory(struct s5p_mfc_dev *mfc_dev)
 	struct device *dev = &mfc_dev->plat_dev->dev;
 	unsigned long mem_size = SZ_8M;
 	unsigned int bitmap_size;
-	/*
-	 * 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.
-	 */
-	int ret = exynos_configure_iommu(dev, S5P_MFC_IOMMU_DMA_BASE,
-					 S5P_MFC_IOMMU_DMA_SIZE);
-	if (ret)
-		return ret;
 
 	if (mfc_mem_size)
 		mem_size = memparse(mfc_mem_size, NULL);
@@ -1203,10 +1191,8 @@ static int s5p_mfc_configure_common_memory(struct s5p_mfc_dev *mfc_dev)
 	bitmap_size = BITS_TO_LONGS(mem_size >> PAGE_SHIFT) * sizeof(long);
 
 	mfc_dev->mem_bitmap = kzalloc(bitmap_size, GFP_KERNEL);
-	if (!mfc_dev->mem_bitmap) {
-		exynos_unconfigure_iommu(dev);
+	if (!mfc_dev->mem_bitmap)
 		return -ENOMEM;
-	}
 
 	mfc_dev->mem_virt = dma_alloc_coherent(dev, mem_size,
 					       &mfc_dev->mem_base, GFP_KERNEL);
@@ -1214,13 +1200,24 @@ static int s5p_mfc_configure_common_memory(struct s5p_mfc_dev *mfc_dev)
 		kfree(mfc_dev->mem_bitmap);
 		dev_err(dev, "failed to preallocate %ld MiB for the firmware and context buffers\n",
 			(mem_size / SZ_1M));
-		exynos_unconfigure_iommu(dev);
 		return -ENOMEM;
 	}
 	mfc_dev->mem_size = mem_size;
 	mfc_dev->dma_base[BANK1_CTX] = mfc_dev->mem_base;
 	mfc_dev->dma_base[BANK2_CTX] = mfc_dev->mem_base;
 
+	/*
+	 * MFC hardware cannot handle 0 as a base address, so mark first 128K
+	 * as used (to keep required base alignment) and adjust base address
+	 */
+	if (mfc_dev->mem_base == (dma_addr_t)0) {
+		unsigned int offset = 1 << MFC_BASE_ALIGN_ORDER;
+
+		bitmap_set(mfc_dev->mem_bitmap, 0, offset >> PAGE_SHIFT);
+		mfc_dev->dma_base[BANK1_CTX] += offset;
+		mfc_dev->dma_base[BANK2_CTX] += offset;
+	}
+
 	/* Firmware allocation cannot fail in this case */
 	s5p_mfc_alloc_firmware(mfc_dev);
 
@@ -1237,7 +1234,6 @@ static void s5p_mfc_unconfigure_common_memory(struct s5p_mfc_dev *mfc_dev)
 {
 	struct device *dev = &mfc_dev->plat_dev->dev;
 
-	exynos_unconfigure_iommu(dev);
 	dma_free_coherent(dev, mfc_dev->mem_size, mfc_dev->mem_virt,
 			  mfc_dev->mem_base);
 	kfree(mfc_dev->mem_bitmap);
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_iommu.h b/drivers/media/platform/s5p-mfc/s5p_mfc_iommu.h
index 6962132ae8fa..76667924ee2a 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_iommu.h
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_iommu.h
@@ -11,54 +11,13 @@
 #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
-
-#if defined(CONFIG_EXYNOS_IOMMU) && defined(CONFIG_ARM_DMA_USE_IOMMU)
-
-#include <asm/dma-iommu.h>
+#if defined(CONFIG_EXYNOS_IOMMU)
 
 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)
@@ -66,14 +25,6 @@ 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.1

[PATCH v2 14/15] media: s5p-mfc: Use preallocated block allocator always for MFC v6+

[Marek Szyprowski]

It turned out that all versions of MFC v6+ hardware doesn't have a strict
requirement for ALL buffers to be allocated on higher addresses than the
firmware base like it was documented for MFC v5. This requirement is true
only for the device and per-context buffers. All video data buffers can be
allocated anywhere for all MFC v6+ versions. Basing on this fact, the
special DMA configuration based on two reserved memory regions is not
really needed for MFC v6+ devices, because the memory requirements for the
firmware, device and per-context buffers can be fulfilled by the simple
probe-time pre-allocated block allocator instroduced in previous patch.

This patch enables support for such pre-allocated block based allocator
always for MFC v6+ devices. Due to the limitations of the memory management
subsystem the largest supported size of the pre-allocated buffer when no
CMA (Contiguous Memory Allocator) is enabled is 4MiB.

This patch also removes the requirement to provide two reserved memory
regions for MFC v6+ devices in device tree. Now the driver is fully
functional without them.

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>
---
 Documentation/devicetree/bindings/media/s5p-mfc.txt | 2 +-
 drivers/media/platform/s5p-mfc/s5p_mfc.c            | 9 ++++++---
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/s5p-mfc.txt b/Documentation/devicetree/bindings/media/s5p-mfc.txt
index 2c901286d818..d3404b5d4d17 100644
--- a/Documentation/devicetree/bindings/media/s5p-mfc.txt
+++ b/Documentation/devicetree/bindings/media/s5p-mfc.txt
@@ -28,7 +28,7 @@ Optional properties:
   - 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
+	support is available; used only by MFC v5 present in Exynos4 SoCs
 
 Obsolete properties:
   - samsung,mfc-r, samsung,mfc-l : support removed, please use memory-region
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c b/drivers/media/platform/s5p-mfc/s5p_mfc.c
index b70cbd637851..b4a13e4cc9d4 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
@@ -1182,9 +1182,12 @@ static void s5p_mfc_unconfigure_2port_memory(struct s5p_mfc_dev *mfc_dev)
 static int s5p_mfc_configure_common_memory(struct s5p_mfc_dev *mfc_dev)
 {
 	struct device *dev = &mfc_dev->plat_dev->dev;
-	unsigned long mem_size = SZ_8M;
+	unsigned long mem_size = SZ_4M;
 	unsigned int bitmap_size;
 
+	if (IS_ENABLED(CONFIG_DMA_CMA) || exynos_is_iommu_available(dev))
+		mem_size = SZ_8M;
+
 	if (mfc_mem_size)
 		mem_size = memparse(mfc_mem_size, NULL);
 
@@ -1244,7 +1247,7 @@ static int s5p_mfc_configure_dma_memory(struct s5p_mfc_dev *mfc_dev)
 {
 	struct device *dev = &mfc_dev->plat_dev->dev;
 
-	if (exynos_is_iommu_available(dev))
+	if (exynos_is_iommu_available(dev) || !IS_TWOPORT(mfc_dev))
 		return s5p_mfc_configure_common_memory(mfc_dev);
 	else
 		return s5p_mfc_configure_2port_memory(mfc_dev);
@@ -1255,7 +1258,7 @@ static void s5p_mfc_unconfigure_dma_memory(struct s5p_mfc_dev *mfc_dev)
 	struct device *dev = &mfc_dev->plat_dev->dev;
 
 	s5p_mfc_release_firmware(mfc_dev);
-	if (exynos_is_iommu_available(dev))
+	if (exynos_is_iommu_available(dev) || !IS_TWOPORT(mfc_dev))
 		s5p_mfc_unconfigure_common_memory(mfc_dev);
 	else
 		s5p_mfc_unconfigure_2port_memory(mfc_dev);
-- 
1.9.1

[PATCH v2 15/15] ARM: dts: exynos: Remove MFC reserved buffers

[Marek Szyprowski]

During my research I found that some of the requirements for the memory
buffers for MFC v6+ devices were blindly copied from the previous (v5)
version and simply turned out to be excessive. The relaxed requirements
are applied by the recent patches to the MFC driver and the driver is
now fully functional even without the reserved memory blocks for all
v6+ variants. This patch removes those reserved memory nodes from all
boards having MFC v6+ hardware block.

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>
---
 arch/arm/boot/dts/exynos5250-arndale.dts           | 1 -
 arch/arm/boot/dts/exynos5250-smdk5250.dts          | 1 -
 arch/arm/boot/dts/exynos5250-spring.dts            | 1 -
 arch/arm/boot/dts/exynos5420-arndale-octa.dts      | 1 -
 arch/arm/boot/dts/exynos5420-peach-pit.dts         | 1 -
 arch/arm/boot/dts/exynos5420-smdk5420.dts          | 1 -
 arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi | 1 -
 arch/arm/boot/dts/exynos5800-peach-pi.dts          | 1 -
 8 files changed, 8 deletions(-)

diff --git a/arch/arm/boot/dts/exynos5250-arndale.dts b/arch/arm/boot/dts/exynos5250-arndale.dts
index 6098dacd09f1..6a432460eb77 100644
--- a/arch/arm/boot/dts/exynos5250-arndale.dts
+++ b/arch/arm/boot/dts/exynos5250-arndale.dts
@@ -14,7 +14,6 @@
 #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";
diff --git a/arch/arm/boot/dts/exynos5250-smdk5250.dts b/arch/arm/boot/dts/exynos5250-smdk5250.dts
index a97a785ccc6b..6632f657394e 100644
--- a/arch/arm/boot/dts/exynos5250-smdk5250.dts
+++ b/arch/arm/boot/dts/exynos5250-smdk5250.dts
@@ -13,7 +13,6 @@
 #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";
diff --git a/arch/arm/boot/dts/exynos5250-spring.dts b/arch/arm/boot/dts/exynos5250-spring.dts
index 4d7bdb735ed3..95c3bcace9dc 100644
--- a/arch/arm/boot/dts/exynos5250-spring.dts
+++ b/arch/arm/boot/dts/exynos5250-spring.dts
@@ -14,7 +14,6 @@
 #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";
diff --git a/arch/arm/boot/dts/exynos5420-arndale-octa.dts b/arch/arm/boot/dts/exynos5420-arndale-octa.dts
index 9cc83c51c925..ee1bb9b8b366 100644
--- a/arch/arm/boot/dts/exynos5420-arndale-octa.dts
+++ b/arch/arm/boot/dts/exynos5420-arndale-octa.dts
@@ -16,7 +16,6 @@
 #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";
diff --git a/arch/arm/boot/dts/exynos5420-peach-pit.dts b/arch/arm/boot/dts/exynos5420-peach-pit.dts
index 1f964ec35c5e..2cd65699a29c 100644
--- a/arch/arm/boot/dts/exynos5420-peach-pit.dts
+++ b/arch/arm/boot/dts/exynos5420-peach-pit.dts
@@ -16,7 +16,6 @@
 #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+";
diff --git a/arch/arm/boot/dts/exynos5420-smdk5420.dts b/arch/arm/boot/dts/exynos5420-smdk5420.dts
index aaccd0da41e5..08c8ab173e87 100644
--- a/arch/arm/boot/dts/exynos5420-smdk5420.dts
+++ b/arch/arm/boot/dts/exynos5420-smdk5420.dts
@@ -13,7 +13,6 @@
 #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";
diff --git a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
index 05b9afdd6757..657535e2e3cc 100644
--- a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
+++ b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
@@ -18,7 +18,6 @@
 #include <dt-bindings/sound/samsung-i2s.h>
 #include "exynos5800.dtsi"
 #include "exynos5422-cpus.dtsi"
-#include "exynos-mfc-reserved-memory.dtsi"
 
 / {
 	memory@40000000 {
diff --git a/arch/arm/boot/dts/exynos5800-peach-pi.dts b/arch/arm/boot/dts/exynos5800-peach-pi.dts
index f9ff7f07ae0c..ecf1c916e8fc 100644
--- a/arch/arm/boot/dts/exynos5800-peach-pi.dts
+++ b/arch/arm/boot/dts/exynos5800-peach-pi.dts
@@ -16,7 +16,6 @@
 #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+";
-- 
1.9.1

[PATCH v2 00/15] Exynos MFC v6+ - remove the need for the reserved memory

[Marian Mihailescu]

Hi,

After testing these patches, encoding using MFC fails when requesting
buffers for capture (it works for output) with ENOMEM (it complains it
cannot allocate memory on bank1).
Did anyone else test encoding?

Thanks,
Marian

Either I've been missing something or nothing has been going on. (K. E. Gordon)

Re: [PATCH v2 00/15] Exynos MFC v6+ - remove the need for the reserved memory

[Javier Martinez Canillas]

Hello Marian,

On Wed, Mar 15, 2017 at 8:36 AM, Marian Mihailescu
<mihailescu2m@gmail.com> wrote:
> Hi,
>
> After testing these patches, encoding using MFC fails when requesting
> buffers for capture (it works for output) with ENOMEM (it complains it
> cannot allocate memory on bank1).
> Did anyone else test encoding?
>

I've not tested encoding, but I did test decoding and it works for me
with Shuah's patch to increase the CMA memory [0]. Did you test with
that one as well?

Also you could try using the 5p_mfc.mem kernel param as explained in
the commit message of "media: s5p-mfc: Add support for probe-time
preallocated block based allocator".

[0]: https://patchwork.kernel.org/patch/9596737/

> Thanks,
> Marian
>

Best regards,
Javier

Re: [PATCH v2 00/15] Exynos MFC v6+ - remove the need for the reserved memory

[Marian Mihailescu]

Thanks for the quick reply.

Decoding works without issues for me too.
I did not change the CMA size or used s5p_mfc.mem parameter. However, according to the Marek, the default 8M should be enough for 3 instances of H264 encoders/decoders. My test was encoding a 30 seconds 720p clip, so I thought memory should not be a big issue; also it’s working w/o these patches applied, so I think CMA size is enough.
Nevertheless, I will try setting them, but I would feel better if someone else would try encoding too.

Cheers,
Marian

> On 15 Mar. 2017, at 10:19 pm, Javier Martinez Canillas <javier@dowhile0.org> wrote:
> 
> Hello Marian,
> 
> On Wed, Mar 15, 2017 at 8:36 AM, Marian Mihailescu
> <mihailescu2m@gmail.com> wrote:
>> Hi,
>> 
>> After testing these patches, encoding using MFC fails when requesting
>> buffers for capture (it works for output) with ENOMEM (it complains it
>> cannot allocate memory on bank1).
>> Did anyone else test encoding?
>> 
> 
> I've not tested encoding, but I did test decoding and it works for me
> with Shuah's patch to increase the CMA memory [0]. Did you test with
> that one as well?
> 
> Also you could try using the 5p_mfc.mem kernel param as explained in
> the commit message of "media: s5p-mfc: Add support for probe-time
> preallocated block based allocator".
> 
> [0]: https://patchwork.kernel.org/patch/9596737/
> 
>> Thanks,
>> Marian
>> 
> 
> Best regards,
> Javier

Re: [PATCH v2 00/15] Exynos MFC v6+ - remove the need for the reserved memory

[Javier Martinez Canillas]

Hello Marian,

On Wed, Mar 15, 2017 at 9:00 AM, Marian Mihailescu
<mihailescu2m@gmail.com> wrote:
> Thanks for the quick reply.
>
> Decoding works without issues for me too.
> I did not change the CMA size or used s5p_mfc.mem parameter. However, according to the Marek, the default 8M should be enough for 3 instances of H264 encoders/decoders. My test was encoding a 30 seconds 720p clip, so I thought memory should not be a big issue; also it’s working w/o these patches applied, so I think CMA size is enough.
> Nevertheless, I will try setting them, but I would feel better if someone else would try encoding too.
>

Ok, I thought it may be that because Shuah and I needed to increase
the CMA size in order to decode a H.264 1080p video.

But if decoding is working correctly for you and only fails on
encoding, then I'm afraid that I won't be able to help you since as
mentioned I didn't test that.

> Cheers,
> Marian
>

Best regards,
Javier

Re: [PATCH v2 00/15] Exynos MFC v6+ - remove the need for the reserved memory

[Andrzej Hajda]

Hi Marian,

On 15.03.2017 12:36, Marian Mihailescu wrote:
> Hi,
> 
> After testing these patches, encoding using MFC fails when requesting
> buffers for capture (it works for output) with ENOMEM (it complains it
> cannot allocate memory on bank1).
> Did anyone else test encoding?

I have tested encoding and it works on my test target. Could you provide
more details of your setup:
- which kernel and patches,
- which hw,
- which test app.

Regards
Andrzej


> 
> Thanks,
> Marian
> 
> Either I've been missing something or nothing has been going on. (K. E. Gordon)
> 

Re: [PATCH v2 00/15] Exynos MFC v6+ - remove the need for the reserved memory

[Marian Mihailescu]

Hi,

I was testing with the linux-next kernel + the v2 patches
HW: odroid xu4
decoding (working): tested with gstreamer
encoding: tested with gstreamer && mfc-patched ffmpeg
before patches: encoding worked
after patches: encoding didn’t work.

I moved on from linux-next in the meantime and I cannot give you logs, BUT I’ve seen Hardkernel applied these patches (and all the linux-next MFC patches) on top of their 4.9 tree, and the result is very similar to mine on linux-next: https://github.com/hardkernel/linux/issues/284

Mar 21 13:04:54 odroid kernel: [   37.165153] s5p_mfc_alloc_priv_buf:78: Allocating private buffer of size 23243744 failed
Mar 21 13:04:54 odroid kernel: [   37.171865] s5p_mfc_alloc_codec_buffers_v6:244: Failed to allocate Bank1 memory
Mar 21 13:04:54 odroid kernel: [   37.179143] vidioc_reqbufs:1174: Failed to allocate encoding buffers


A user reported even adding s5p_mfc.mem=64M did not make the encoder work.
Any thoughts?

Thanks,
M.

(resent in plain text format)

> On 17 Mar. 2017, at 10:36 pm, Andrzej Hajda <a.hajda@samsung.com> wrote:
> 
> Hi Marian,
> 
> On 15.03.2017 12:36, Marian Mihailescu wrote:
>> Hi,
>> 
>> After testing these patches, encoding using MFC fails when requesting
>> buffers for capture (it works for output) with ENOMEM (it complains it
>> cannot allocate memory on bank1).
>> Did anyone else test encoding?
> 
> I have tested encoding and it works on my test target. Could you provide
> more details of your setup:
> - which kernel and patches,
> - which hw,
> - which test app.
> 
> Regards
> Andrzej
> 
> 
>> 
>> Thanks,
>> Marian
>> 
>> Either I've been missing something or nothing has been going on. (K. E. Gordon)
>> 
> 

Re: [PATCH v2 00/15] Exynos MFC v6+ - remove the need for the reserved memory

[Marek Szyprowski]

Hi Marian,

On 2017-03-22 10:33, Marian Mihailescu wrote:
> Hi,
>
> I was testing with the linux-next kernel + the v2 patches
> HW: odroid xu4
> decoding (working): tested with gstreamer
> encoding: tested with gstreamer && mfc-patched ffmpeg
> before patches: encoding worked
> after patches: encoding didn’t work.
>
> I moved on from linux-next in the meantime and I cannot give you logs, BUT I’ve seen Hardkernel applied these patches (and all the linux-next MFC patches) on top of their 4.9 tree, and the result is very similar to mine on linux-next: https://github.com/hardkernel/linux/issues/284
>
> Mar 21 13:04:54 odroid kernel: [   37.165153] s5p_mfc_alloc_priv_buf:78: Allocating private buffer of size 23243744 failed
> Mar 21 13:04:54 odroid kernel: [   37.171865] s5p_mfc_alloc_codec_buffers_v6:244: Failed to allocate Bank1 memory
> Mar 21 13:04:54 odroid kernel: [   37.179143] vidioc_reqbufs:1174: Failed to allocate encoding buffers
>
>
> A user reported even adding s5p_mfc.mem=64M did not make the encoder work.
> Any thoughts?

Thanks for the report. Could you provide a bit more information about 
the encoder configuration (selected format, frame size, etc). 23MiB for 
the temporary buffer seems to be a bit large value, but I would like to 
reproduce it here and check what can be done to avoid allocating it from 
the preallocated buffer.




> Thanks,
> M.
>
> (resent in plain text format)
>
>> On 17 Mar. 2017, at 10:36 pm, Andrzej Hajda <a.hajda@samsung.com> wrote:
>>
>> Hi Marian,
>>
>> On 15.03.2017 12:36, Marian Mihailescu wrote:
>>> Hi,
>>>
>>> After testing these patches, encoding using MFC fails when requesting
>>> buffers for capture (it works for output) with ENOMEM (it complains it
>>> cannot allocate memory on bank1).
>>> Did anyone else test encoding?
>> I have tested encoding and it works on my test target. Could you provide
>> more details of your setup:
>> - which kernel and patches,
>> - which hw,
>> - which test app.
>>
>> Regards
>> Andrzej
>>
>>
>>> Thanks,
>>> Marian
>>>
>>> Either I've been missing something or nothing has been going on. (K. E. Gordon)
>>>
>
>

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

Re: [PATCH v2 00/15] Exynos MFC v6+ - remove the need for the reserved memory

[Marian Mihailescu]

Hi Marek,

I have been using gstreamer 1.11.2 with the following patches applied to gst-plugins-good (to add the encoder element): https://gist.github.com/mihailescu2m/f52a8f4df67a3d796247337ff67211a9
Also gst-plugins-good was compiled --without-libv4l2

This is a pipeline I used to test encoding (mfc-dec on /dev/video10 and mfc-enc on /dev/video11):

gst-launch-1.0 filesrc location=~/sintel_trailer-720p.mp4 ! qtdemux ! h264parse ! v4l2video10dec !  v4l2video11h264enc extra-controls="encode,h264_level=10,h264_profile=4,frame_level_rate_control_enable=1,video_bitrate=4097152" ! h264parse ! matroskamux ! filesink location=~/sintel-encoded.mkv

I believe this uses all the default MFC encoder options, except:
H264 profile, set to High
H264 level, set to 3.2
max bitrate set to ~4M (default is unlimited, which results in ~100MB/min files).

Cheers,
Marian


> On 22 Mar. 2017, at 8:10 pm, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
> 
> Hi Marian,
> 
> On 2017-03-22 10:33, Marian Mihailescu wrote:
>> Hi,
>> 
>> I was testing with the linux-next kernel + the v2 patches
>> HW: odroid xu4
>> decoding (working): tested with gstreamer
>> encoding: tested with gstreamer && mfc-patched ffmpeg
>> before patches: encoding worked
>> after patches: encoding didn’t work.
>> 
>> I moved on from linux-next in the meantime and I cannot give you logs, BUT I’ve seen Hardkernel applied these patches (and all the linux-next MFC patches) on top of their 4.9 tree, and the result is very similar to mine on linux-next: https://github.com/hardkernel/linux/issues/284
>> 
>> Mar 21 13:04:54 odroid kernel: [   37.165153] s5p_mfc_alloc_priv_buf:78: Allocating private buffer of size 23243744 failed
>> Mar 21 13:04:54 odroid kernel: [   37.171865] s5p_mfc_alloc_codec_buffers_v6:244: Failed to allocate Bank1 memory
>> Mar 21 13:04:54 odroid kernel: [   37.179143] vidioc_reqbufs:1174: Failed to allocate encoding buffers
>> 
>> 
>> A user reported even adding s5p_mfc.mem=64M did not make the encoder work.
>> Any thoughts?
> 
> Thanks for the report. Could you provide a bit more information about the encoder configuration (selected format, frame size, etc). 23MiB for the temporary buffer seems to be a bit large value, but I would like to reproduce it here and check what can be done to avoid allocating it from the preallocated buffer.
> 
> 
> 
> 
>> Thanks,
>> M.
>> 
>> (resent in plain text format)
>> 
>>> On 17 Mar. 2017, at 10:36 pm, Andrzej Hajda <a.hajda@samsung.com> wrote:
>>> 
>>> Hi Marian,
>>> 
>>> On 15.03.2017 12:36, Marian Mihailescu wrote:
>>>> Hi,
>>>> 
>>>> After testing these patches, encoding using MFC fails when requesting
>>>> buffers for capture (it works for output) with ENOMEM (it complains it
>>>> cannot allocate memory on bank1).
>>>> Did anyone else test encoding?
>>> I have tested encoding and it works on my test target. Could you provide
>>> more details of your setup:
>>> - which kernel and patches,
>>> - which hw,
>>> - which test app.
>>> 
>>> Regards
>>> Andrzej
>>> 
>>> 
>>>> Thanks,
>>>> Marian
>>>> 
>>>> Either I've been missing something or nothing has been going on. (K. E. Gordon)
>>>> 
>> 
>> 
> 
> Best regards
> -- 
> Marek Szyprowski, PhD
> Samsung R&D Institute Poland

Re: [PATCH v2 00/15] Exynos MFC v6+ - remove the need for the reserved memory

[Marek Szyprowski]

[-- Attachment #1: Type: text/plain, Size: 1606 bytes --]

Hi Marian,


On 2017-03-22 10:33, Marian Mihailescu wrote:
> Hi,
>
> I was testing with the linux-next kernel + the v2 patches
> HW: odroid xu4
> decoding (working): tested with gstreamer
> encoding: tested with gstreamer && mfc-patched ffmpeg
> before patches: encoding worked
> after patches: encoding didn’t work.
>
> I moved on from linux-next in the meantime and I cannot give you logs, BUT I’ve seen Hardkernel applied these patches (and all the linux-next MFC patches) on top of their 4.9 tree, and the result is very similar to mine on linux-next: https://github.com/hardkernel/linux/issues/284
>
> Mar 21 13:04:54 odroid kernel: [   37.165153] s5p_mfc_alloc_priv_buf:78: Allocating private buffer of size 23243744 failed
> Mar 21 13:04:54 odroid kernel: [   37.171865] s5p_mfc_alloc_codec_buffers_v6:244: Failed to allocate Bank1 memory
> Mar 21 13:04:54 odroid kernel: [   37.179143] vidioc_reqbufs:1174: Failed to allocate encoding buffers
>
>
> A user reported even adding s5p_mfc.mem=64M did not make the encoder work.
> Any thoughts?

s5p_mfc.mem=64M should fix the issue. If not then there is some kind of different issue.

We did a bit more tests and in fact encoding h264 requires quite a lot of memory, so the default preallocated size might be not enough. However the codec temporary buffers don't need to be allocated from the preallocated region. In our tests it also worked when codec buffers were allocated in a generic way. Please check if the attached patch (applied on top of v3 patchset) fixes the issue.

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


[-- Attachment #2: 0001-media-s5p-mfc-Don-t-allocate-codec-buffers-from-pre-.patch --]
[-- Type: text/x-patch, Size: 3969 bytes --]

From: Marek Szyprowski <m.szyprowski@samsung.com>
Date: Wed, 22 Mar 2017 10:59:26 +0100
Subject: [PATCH] media: s5p-mfc: Don't allocate codec buffers from
 pre-allocated region

Further investigation revealed that codec buffers also don't need to
be allocated at higher addresses than firmware base for MFC v6+ hardware.
Those buffers can be quite large and its size depends on the selected
format and framesize. This patch changes the way the codec buffers are
allocated - driver will use generic allocator for them instead of the
pre-allocated buffer for firmware and contexts.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 drivers/media/platform/s5p-mfc/s5p_mfc_opr.c    | 28 +++++++++++++++++++++++++
 drivers/media/platform/s5p-mfc/s5p_mfc_opr.h    |  4 ++++
 drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c |  4 ++--
 3 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr.c b/drivers/media/platform/s5p-mfc/s5p_mfc_opr.c
index 34a66189d980..7f33cf23947f 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr.c
@@ -79,6 +79,25 @@ int s5p_mfc_alloc_priv_buf(struct s5p_mfc_dev *dev, unsigned int mem_ctx,
 	return -ENOMEM;
 }
 
+int s5p_mfc_alloc_generic_buf(struct s5p_mfc_dev *dev, unsigned int mem_ctx,
+			   struct s5p_mfc_priv_buf *b)
+{
+	struct device *mem_dev = dev->mem_dev[mem_ctx];
+
+	mfc_debug(3, "Allocating generic buf: %zu\n", b->size);
+
+	b->ctx = mem_ctx;
+	b->virt = dma_alloc_coherent(mem_dev, b->size, &b->dma, GFP_KERNEL);
+	if (!b->virt)
+		goto no_mem;
+
+	mfc_debug(3, "Allocated addr %p %pad\n", b->virt, &b->dma);
+	return 0;
+no_mem:
+	mfc_err("Allocating generic buffer of size %zu failed\n", b->size);
+	return -ENOMEM;
+}
+
 void s5p_mfc_release_priv_buf(struct s5p_mfc_dev *dev,
 			      struct s5p_mfc_priv_buf *b)
 {
@@ -97,3 +116,12 @@ void s5p_mfc_release_priv_buf(struct s5p_mfc_dev *dev,
 	b->size = 0;
 }
 
+void s5p_mfc_release_generic_buf(struct s5p_mfc_dev *dev,
+			      struct s5p_mfc_priv_buf *b)
+{
+	struct device *mem_dev = dev->mem_dev[b->ctx];
+	dma_free_coherent(mem_dev, b->size, b->virt, b->dma);
+	b->virt = NULL;
+	b->dma = 0;
+	b->size = 0;
+}
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr.h b/drivers/media/platform/s5p-mfc/s5p_mfc_opr.h
index 108e59382e0c..16d553fcff08 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr.h
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr.h
@@ -319,6 +319,10 @@ int s5p_mfc_alloc_priv_buf(struct s5p_mfc_dev *dev, unsigned int mem_ctx,
 			   struct s5p_mfc_priv_buf *b);
 void s5p_mfc_release_priv_buf(struct s5p_mfc_dev *dev,
 			      struct s5p_mfc_priv_buf *b);
+int s5p_mfc_alloc_generic_buf(struct s5p_mfc_dev *dev, unsigned int mem_ctx,
+			   struct s5p_mfc_priv_buf *b);
+void s5p_mfc_release_generic_buf(struct s5p_mfc_dev *dev,
+			      struct s5p_mfc_priv_buf *b);
 
 
 #endif /* S5P_MFC_OPR_H_ */
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
index 70071a12db16..85880e9106be 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
@@ -239,7 +239,7 @@ static int s5p_mfc_alloc_codec_buffers_v6(struct s5p_mfc_ctx *ctx)
 
 	/* Allocate only if memory from bank 1 is necessary */
 	if (ctx->bank1.size > 0) {
-		ret = s5p_mfc_alloc_priv_buf(dev, BANK_L_CTX, &ctx->bank1);
+		ret = s5p_mfc_alloc_generic_buf(dev, BANK_L_CTX, &ctx->bank1);
 		if (ret) {
 			mfc_err("Failed to allocate Bank1 memory\n");
 			return ret;
@@ -252,7 +252,7 @@ static int s5p_mfc_alloc_codec_buffers_v6(struct s5p_mfc_ctx *ctx)
 /* Release buffers allocated for codec */
 static void s5p_mfc_release_codec_buffers_v6(struct s5p_mfc_ctx *ctx)
 {
-	s5p_mfc_release_priv_buf(ctx->dev, &ctx->bank1);
+	s5p_mfc_release_generic_buf(ctx->dev, &ctx->bank1);
 }
 
 /* Allocate memory for instance data buffer */
-- 
1.9.1