Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1543396 > unrolled thread

[PATCH 0/2] s5p-mfc fix for using reserved memory

Started byPankaj Dubey <pankaj.dubey@samsung.com>
First post2016-12-16 12:50 +0100
Last post2016-12-21 06:50 +0100
Articles 5 — 3 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/2] s5p-mfc fix for using reserved memory Pankaj Dubey <pankaj.dubey@samsung.com> - 2016-12-16 12:50 +0100
    [PATCH 2/2] media: s5p-mfc: fix MMAP of mfc buffer during reqbufs Pankaj Dubey <pankaj.dubey@samsung.com> - 2016-12-16 12:50 +0100
    [PATCH 1/2] media: s5p-mfc: convert drivers to use the new vb2_queue  dev field Pankaj Dubey <pankaj.dubey@samsung.com> - 2016-12-16 12:50 +0100
    Re: [PATCH 0/2] s5p-mfc fix for using reserved memory Marek Szyprowski <m.szyprowski@samsung.com> - 2016-12-20 13:00 +0100
      Re: [PATCH 0/2] s5p-mfc fix for using reserved memory "pankaj.dubey" <pankaj.dubey@samsung.com> - 2016-12-21 06:50 +0100

#1543396 — [PATCH 0/2] s5p-mfc fix for using reserved memory

FromPankaj Dubey <pankaj.dubey@samsung.com>
Date2016-12-16 12:50 +0100
Subject[PATCH 0/2] s5p-mfc fix for using reserved memory
Message-ID<sOZbr-32P-23@gated-at.bofh.it>
It has been observed on ARM64 based Exynos SoC, if IOMMU is not enabled
and we try to use reserved memory for MFC, reqbufs fails with below
mentioned error
---------------------------------------------------------------------------
V4L2 Codec decoding example application
Kamil Debski <k.debski@samsung.com>
Copyright 2012 Samsung Electronics Co., Ltd.

Opening MFC.
(mfc.c:mfc_open:58): MFC Info (/dev/video0): driver="s5p-mfc" \
bus_info="platform:12c30000.mfc0" card="s5p-mfc-dec" fd=0x4[
42.339165] Remapping memory failed, error: -6

MFC Open Success.
(main.c:main:711): Successfully opened all necessary files and devices
(mfc.c:mfc_dec_setup_output:103): Setup MFC decoding OUTPUT buffer \
size=4194304 (requested=4194304)
(mfc.c:mfc_dec_setup_output:120): Number of MFC OUTPUT buffers is 2 \
(requested 2)

[App] Out buf phy : 0x00000000, virt : 0xffffffff
Output Length is = 0x300000
Error (mfc.c:mfc_dec_setup_output:145): Failed to MMAP MFC OUTPUT buffer
-------------------------------------------------------------------------
This is because the device requesting for memory is mfc0.left not the parent mfc0.
Hence setting of alloc_devs need to be done only if IOMMU is enabled
and in that case both the left and right device is treated as mfc0 only.
Also we need to populate vb2_queue's dev pointer with mfc dev pointer.

Smitha T Murthy (2):
  media: s5p-mfc: convert drivers to use the new vb2_queue dev field
  media: s5p-mfc: fix MMAP of mfc buffer during reqbufs

 drivers/media/platform/s5p-mfc/s5p_mfc.c     |  2 ++
 drivers/media/platform/s5p-mfc/s5p_mfc_dec.c | 17 ++++++++++-------
 drivers/media/platform/s5p-mfc/s5p_mfc_enc.c | 18 +++++++++++-------
 3 files changed, 23 insertions(+), 14 deletions(-)

-- 
2.7.4

[toc] | [next] | [standalone]

#1543402 — [PATCH 2/2] media: s5p-mfc: fix MMAP of mfc buffer during reqbufs

FromPankaj Dubey <pankaj.dubey@samsung.com>
Date2016-12-16 12:50 +0100
Subject[PATCH 2/2] media: s5p-mfc: fix MMAP of mfc buffer during reqbufs
Message-ID<sOZbs-32P-37@gated-at.bofh.it>
In reply to#1543396
From: Smitha T Murthy <smitha.t@samsung.com>

It has been observed on ARM64 based Exynos SoC, if IOMMU is not enabled
and we try to use reserved memory for MFC, reqbufs fails with below
mentioned error
---------------------------------------------------------------------------
V4L2 Codec decoding example application
Kamil Debski <k.debski@samsung.com>
Copyright 2012 Samsung Electronics Co., Ltd.

Opening MFC.
(mfc.c:mfc_open:58): MFC Info (/dev/video0): driver="s5p-mfc" \
bus_info="platform:12c30000.mfc0" card="s5p-mfc-dec" fd=0x4[
42.339165] Remapping memory failed, error: -6

MFC Open Success.
(main.c:main:711): Successfully opened all necessary files and devices
(mfc.c:mfc_dec_setup_output:103): Setup MFC decoding OUTPUT buffer \
size=4194304 (requested=4194304)
(mfc.c:mfc_dec_setup_output:120): Number of MFC OUTPUT buffers is 2 \
(requested 2)

[App] Out buf phy : 0x00000000, virt : 0xffffffff
Output Length is = 0x300000
Error (mfc.c:mfc_dec_setup_output:145): Failed to MMAP MFC OUTPUT buffer
-------------------------------------------------------------------------
This is because the device requesting for memory is mfc0.left not the parent mfc0.
Hence setting of alloc_devs need to be done only if IOMMU is enabled
and in that case both the left and right device is treated as mfc0 only.

Signed-off-by: Smitha T Murthy <smitha.t@samsung.com>
[pankaj.dubey: debugging issue and formatting commit message]
Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
---
 drivers/media/platform/s5p-mfc/s5p_mfc_dec.c | 17 ++++++++++-------
 drivers/media/platform/s5p-mfc/s5p_mfc_enc.c | 18 +++++++++++-------
 2 files changed, 21 insertions(+), 14 deletions(-)

diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c b/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
index 52081dd..9cfca5d 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
@@ -30,6 +30,7 @@
 #include "s5p_mfc_intr.h"
 #include "s5p_mfc_opr.h"
 #include "s5p_mfc_pm.h"
+#include "s5p_mfc_iommu.h"
 
 static struct s5p_mfc_fmt formats[] = {
 	{
@@ -930,16 +931,18 @@ static int s5p_mfc_queue_setup(struct vb2_queue *vq,
 	    vq->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) {
 		psize[0] = ctx->luma_size;
 		psize[1] = ctx->chroma_size;
-
-		if (IS_MFCV6_PLUS(dev))
-			alloc_devs[0] = ctx->dev->mem_dev_l;
-		else
-			alloc_devs[0] = ctx->dev->mem_dev_r;
-		alloc_devs[1] = ctx->dev->mem_dev_l;
+		if (exynos_is_iommu_available(&dev->plat_dev->dev)) {
+			if (IS_MFCV6_PLUS(dev))
+				alloc_devs[0] = ctx->dev->mem_dev_l;
+			else
+				alloc_devs[0] = ctx->dev->mem_dev_r;
+			alloc_devs[1] = ctx->dev->mem_dev_l;
+		}
 	} 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;
+		if (exynos_is_iommu_available(&dev->plat_dev->dev))
+			alloc_devs[0] = ctx->dev->mem_dev_l;
 	} 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 fcc2e05..eb8f06d 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
@@ -30,6 +30,7 @@
 #include "s5p_mfc_enc.h"
 #include "s5p_mfc_intr.h"
 #include "s5p_mfc_opr.h"
+#include "s5p_mfc_iommu.h"
 
 #define DEF_SRC_FMT_ENC	V4L2_PIX_FMT_NV12M
 #define DEF_DST_FMT_ENC	V4L2_PIX_FMT_H264
@@ -1832,7 +1833,8 @@ 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;
+		if (exynos_is_iommu_available(&dev->plat_dev->dev))
+			alloc_devs[0] = ctx->dev->mem_dev_l;
 	} else if (vq->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) {
 		if (ctx->src_fmt)
 			*plane_count = ctx->src_fmt->num_planes;
@@ -1847,12 +1849,14 @@ static int s5p_mfc_queue_setup(struct vb2_queue *vq,
 		psize[0] = ctx->luma_size;
 		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;
-		} else {
-			alloc_devs[0] = ctx->dev->mem_dev_r;
-			alloc_devs[1] = ctx->dev->mem_dev_r;
+		if (exynos_is_iommu_available(&dev->plat_dev->dev)) {
+			if (IS_MFCV6_PLUS(dev)) {
+				alloc_devs[0] = ctx->dev->mem_dev_l;
+				alloc_devs[1] = ctx->dev->mem_dev_l;
+			} else {
+				alloc_devs[0] = ctx->dev->mem_dev_r;
+				alloc_devs[1] = ctx->dev->mem_dev_r;
+			}
 		}
 	} else {
 		mfc_err("invalid queue type: %d\n", vq->type);
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1543404 — [PATCH 1/2] media: s5p-mfc: convert drivers to use the new vb2_queue dev field

FromPankaj Dubey <pankaj.dubey@samsung.com>
Date2016-12-16 12:50 +0100
Subject[PATCH 1/2] media: s5p-mfc: convert drivers to use the new vb2_queue dev field
Message-ID<sOZbs-32P-41@gated-at.bofh.it>
In reply to#1543396
From: Smitha T Murthy <smitha.t@samsung.com>

commit 2548fee63d9e ("[media] media/platform: convert drivers to use the new
vb2_queue dev field") has missed to set dev pointer of vb2_queue which will be
used in reqbufs of mfc driver. Without this change following error is observed:

---------------------------------------------------------------
V4L2 Codec decoding example application
Kamil Debski <k.debski@samsung.com>
Copyright 2012 Samsung Electronics Co., Ltd.

Opening MFC.
(mfc.c:mfc_open:58): MFC Info (/dev/video0): driver="s5p-mfc" \
bus_info="platform:12c30000.mfc0" card="s5p-mfc-dec" fd=0x4[
42.339165] Remapping memory failed, error: -6

MFC Open Success.
(main.c:main:711): Successfully opened all necessary files and devices
(mfc.c:mfc_dec_setup_output:103): Setup MFC decoding OUTPUT buffer \
size=4194304 (requested=4194304)
(mfc.c:mfc_dec_setup_output:120): Number of MFC OUTPUT buffers is 2 \
(requested 2)

[App] Out buf phy : 0x00000000, virt : 0xffffffff
Output Length is = 0x300000
Error (mfc.c:mfc_dec_setup_output:145): Failed to MMAP MFC OUTPUT buffer
-------------------------------------------------------

Signed-off-by: Smitha T Murthy <smitha.t@samsung.com>
[pankaj.dubey: debugging issue and formatting commit message]
Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
---
 drivers/media/platform/s5p-mfc/s5p_mfc.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c b/drivers/media/platform/s5p-mfc/s5p_mfc.c
index 0a5b8f5..6ea8246 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
@@ -838,6 +838,7 @@ static int s5p_mfc_open(struct file *file)
 	q->type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
 	q->drv_priv = &ctx->fh;
 	q->lock = &dev->mfc_mutex;
+	q->dev = &dev->plat_dev->dev;
 	if (vdev == dev->vfd_dec) {
 		q->io_modes = VB2_MMAP;
 		q->ops = get_dec_queue_ops();
@@ -861,6 +862,7 @@ static int s5p_mfc_open(struct file *file)
 	q->io_modes = VB2_MMAP;
 	q->drv_priv = &ctx->fh;
 	q->lock = &dev->mfc_mutex;
+	q->dev = &dev->plat_dev->dev;
 	if (vdev == dev->vfd_dec) {
 		q->io_modes = VB2_MMAP;
 		q->ops = get_dec_queue_ops();
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1545070

FromMarek Szyprowski <m.szyprowski@samsung.com>
Date2016-12-20 13:00 +0100
Message-ID<sQrfj-56i-13@gated-at.bofh.it>
In reply to#1543396
Hi Pankaj


On 2016-12-16 12:48, Pankaj Dubey wrote:
> It has been observed on ARM64 based Exynos SoC, if IOMMU is not enabled
> and we try to use reserved memory for MFC, reqbufs fails with below
> mentioned error
> ---------------------------------------------------------------------------
> V4L2 Codec decoding example application
> Kamil Debski <k.debski@samsung.com>
> Copyright 2012 Samsung Electronics Co., Ltd.
>
> Opening MFC.
> (mfc.c:mfc_open:58): MFC Info (/dev/video0): driver="s5p-mfc" \
> bus_info="platform:12c30000.mfc0" card="s5p-mfc-dec" fd=0x4[
> 42.339165] Remapping memory failed, error: -6
>
> MFC Open Success.
> (main.c:main:711): Successfully opened all necessary files and devices
> (mfc.c:mfc_dec_setup_output:103): Setup MFC decoding OUTPUT buffer \
> size=4194304 (requested=4194304)
> (mfc.c:mfc_dec_setup_output:120): Number of MFC OUTPUT buffers is 2 \
> (requested 2)
>
> [App] Out buf phy : 0x00000000, virt : 0xffffffff
> Output Length is = 0x300000
> Error (mfc.c:mfc_dec_setup_output:145): Failed to MMAP MFC OUTPUT buffer
> -------------------------------------------------------------------------
> This is because the device requesting for memory is mfc0.left not the parent mfc0.
> Hence setting of alloc_devs need to be done only if IOMMU is enabled
> and in that case both the left and right device is treated as mfc0 only.
> Also we need to populate vb2_queue's dev pointer with mfc dev pointer.

I also got this issue but Your solution is imho not the proper approach.
Too much hacking in the driver, while the issue is in the core. ARM64 
requires
to call arch_setup_dma_ops() for each device that will be used for 
dma-mapping.
So the issue is in drivers/of/of_reserved_mem.c - in
of_reserved_mem_device_init_by_idx() function, which should ensure that
arch_setup_dma_ops() is called also for the virtual devices for reserved 
memory.

s5p-mfc driver however still requires some patching for 
dma-mapping/iommu glue
used on ARM64 architecture.

> Smitha T Murthy (2):
>    media: s5p-mfc: convert drivers to use the new vb2_queue dev field
>    media: s5p-mfc: fix MMAP of mfc buffer during reqbufs
>
>   drivers/media/platform/s5p-mfc/s5p_mfc.c     |  2 ++
>   drivers/media/platform/s5p-mfc/s5p_mfc_dec.c | 17 ++++++++++-------
>   drivers/media/platform/s5p-mfc/s5p_mfc_enc.c | 18 +++++++++++-------
>   3 files changed, 23 insertions(+), 14 deletions(-)
>

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

[toc] | [prev] | [next] | [standalone]

#1545590

From"pankaj.dubey" <pankaj.dubey@samsung.com>
Date2016-12-21 06:50 +0100
Message-ID<sQHWN-7KW-9@gated-at.bofh.it>
In reply to#1545070
Hi Marek,

On Tuesday 20 December 2016 05:28 PM, Marek Szyprowski wrote:
> Hi Pankaj
> 
> 
> On 2016-12-16 12:48, Pankaj Dubey wrote:
>> It has been observed on ARM64 based Exynos SoC, if IOMMU is not enabled
>> and we try to use reserved memory for MFC, reqbufs fails with below
>> mentioned error
>> ---------------------------------------------------------------------------
>>
>> V4L2 Codec decoding example application
>> Kamil Debski <k.debski@samsung.com>
>> Copyright 2012 Samsung Electronics Co., Ltd.
>>
>> Opening MFC.
>> (mfc.c:mfc_open:58): MFC Info (/dev/video0): driver="s5p-mfc" \
>> bus_info="platform:12c30000.mfc0" card="s5p-mfc-dec" fd=0x4[
>> 42.339165] Remapping memory failed, error: -6
>>
>> MFC Open Success.
>> (main.c:main:711): Successfully opened all necessary files and devices
>> (mfc.c:mfc_dec_setup_output:103): Setup MFC decoding OUTPUT buffer \
>> size=4194304 (requested=4194304)
>> (mfc.c:mfc_dec_setup_output:120): Number of MFC OUTPUT buffers is 2 \
>> (requested 2)
>>
>> [App] Out buf phy : 0x00000000, virt : 0xffffffff
>> Output Length is = 0x300000
>> Error (mfc.c:mfc_dec_setup_output:145): Failed to MMAP MFC OUTPUT buffer
>> -------------------------------------------------------------------------
>> This is because the device requesting for memory is mfc0.left not the
>> parent mfc0.
>> Hence setting of alloc_devs need to be done only if IOMMU is enabled
>> and in that case both the left and right device is treated as mfc0 only.
>> Also we need to populate vb2_queue's dev pointer with mfc dev pointer.
> 
> I also got this issue but Your solution is imho not the proper approach.
> Too much hacking in the driver, while the issue is in the core. ARM64
> requires
> to call arch_setup_dma_ops() for each device that will be used for
> dma-mapping.
> So the issue is in drivers/of/of_reserved_mem.c - in
> of_reserved_mem_device_init_by_idx() function, which should ensure that
> arch_setup_dma_ops() is called also for the virtual devices for reserved
> memory.
> 
> s5p-mfc driver however still requires some patching for
> dma-mapping/iommu glue
> used on ARM64 architecture.
> 

Thanks for review and suggestion.
Our initial investigation pointed out issue due to change in vb2_queue
dev field by commit 2548fee63d9e ("[media] media/platform: convert
drivers to use the new vb2_queue dev field"), where only for mfc q->dev
pointer was not set.

But I think you are right, and of_reserved_mem_device_init_by_idx should
take care of setting dma_ops for devices which calls this function. So I
will be submitting new change soon to fix this issue after testing it.

Thanks,
Pankaj Dubey

>> Smitha T Murthy (2):
>>    media: s5p-mfc: convert drivers to use the new vb2_queue dev field
>>    media: s5p-mfc: fix MMAP of mfc buffer during reqbufs
>>
>>   drivers/media/platform/s5p-mfc/s5p_mfc.c     |  2 ++
>>   drivers/media/platform/s5p-mfc/s5p_mfc_dec.c | 17 ++++++++++-------
>>   drivers/media/platform/s5p-mfc/s5p_mfc_enc.c | 18 +++++++++++-------
>>   3 files changed, 23 insertions(+), 14 deletions(-)
>>
> 
> Best regards

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web