2016-12-16 11:49:17

by Pankaj Dubey

[permalink] [raw]
Subject: [PATCH 0/2] s5p-mfc fix for using reserved memory

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 <[email protected]>
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


2016-12-16 11:48:47

by Pankaj Dubey

[permalink] [raw]
Subject: [PATCH 2/2] media: s5p-mfc: fix MMAP of mfc buffer during reqbufs

From: Smitha T Murthy <[email protected]>

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 <[email protected]>
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 <[email protected]>
[pankaj.dubey: debugging issue and formatting commit message]
Signed-off-by: Pankaj Dubey <[email protected]>
---
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

2016-12-16 11:48:59

by Pankaj Dubey

[permalink] [raw]
Subject: [PATCH 1/2] media: s5p-mfc: convert drivers to use the new vb2_queue dev field

From: Smitha T Murthy <[email protected]>

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 <[email protected]>
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 <[email protected]>
[pankaj.dubey: debugging issue and formatting commit message]
Signed-off-by: Pankaj Dubey <[email protected]>
---
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

2016-12-20 11:58:19

by Marek Szyprowski

[permalink] [raw]
Subject: Re: [PATCH 0/2] s5p-mfc fix for using reserved memory

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 <[email protected]>
> 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

2016-12-21 05:42:17

by Pankaj Dubey

[permalink] [raw]
Subject: Re: [PATCH 0/2] s5p-mfc fix for using reserved memory

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 <[email protected]>
>> 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