linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] s5p-mfc fix for using reserved memory
@ 2016-12-16 11:48 ` Pankaj Dubey
  2016-12-16 11:48   ` [PATCH 1/2] media: s5p-mfc: convert drivers to use the new vb2_queue dev field Pankaj Dubey
                     ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Pankaj Dubey @ 2016-12-16 11:48 UTC (permalink / raw)
  To: linux-media, linux-kernel, linux-samsung-soc, linux-arm-kernel
  Cc: kyungmin.park, jtp.park, mchehab, mchehab, hans.verkuil, krzk,
	kgene, javier, Pankaj Dubey

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/2] media: s5p-mfc: convert drivers to use the new vb2_queue dev field
  2016-12-16 11:48 ` [PATCH 0/2] s5p-mfc fix for using reserved memory Pankaj Dubey
@ 2016-12-16 11:48   ` Pankaj Dubey
  2017-02-24 19:22     ` Javier Martinez Canillas
  2016-12-16 11:48   ` [PATCH 2/2] media: s5p-mfc: fix MMAP of mfc buffer during reqbufs Pankaj Dubey
  2016-12-20 11:58   ` [PATCH 0/2] s5p-mfc fix for using reserved memory Marek Szyprowski
  2 siblings, 1 reply; 9+ messages in thread
From: Pankaj Dubey @ 2016-12-16 11:48 UTC (permalink / raw)
  To: linux-media, linux-kernel, linux-samsung-soc, linux-arm-kernel
  Cc: kyungmin.park, jtp.park, mchehab, mchehab, hans.verkuil, krzk,
	kgene, javier, Smitha T Murthy, Pankaj Dubey

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

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/2] media: s5p-mfc: fix MMAP of mfc buffer during reqbufs
  2016-12-16 11:48 ` [PATCH 0/2] s5p-mfc fix for using reserved memory Pankaj Dubey
  2016-12-16 11:48   ` [PATCH 1/2] media: s5p-mfc: convert drivers to use the new vb2_queue dev field Pankaj Dubey
@ 2016-12-16 11:48   ` Pankaj Dubey
  2017-02-24 19:42     ` Javier Martinez Canillas
  2016-12-20 11:58   ` [PATCH 0/2] s5p-mfc fix for using reserved memory Marek Szyprowski
  2 siblings, 1 reply; 9+ messages in thread
From: Pankaj Dubey @ 2016-12-16 11:48 UTC (permalink / raw)
  To: linux-media, linux-kernel, linux-samsung-soc, linux-arm-kernel
  Cc: kyungmin.park, jtp.park, mchehab, mchehab, hans.verkuil, krzk,
	kgene, javier, Smitha T Murthy, Pankaj Dubey

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

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/2] s5p-mfc fix for using reserved memory
  2016-12-16 11:48 ` [PATCH 0/2] s5p-mfc fix for using reserved memory Pankaj Dubey
  2016-12-16 11:48   ` [PATCH 1/2] media: s5p-mfc: convert drivers to use the new vb2_queue dev field Pankaj Dubey
  2016-12-16 11:48   ` [PATCH 2/2] media: s5p-mfc: fix MMAP of mfc buffer during reqbufs Pankaj Dubey
@ 2016-12-20 11:58   ` Marek Szyprowski
  2016-12-21  5:44     ` pankaj.dubey
  2 siblings, 1 reply; 9+ messages in thread
From: Marek Szyprowski @ 2016-12-20 11:58 UTC (permalink / raw)
  To: Pankaj Dubey, linux-media, linux-kernel, linux-samsung-soc,
	linux-arm-kernel
  Cc: kyungmin.park, jtp.park, mchehab, mchehab, hans.verkuil, krzk,
	kgene, javier

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/2] s5p-mfc fix for using reserved memory
  2016-12-20 11:58   ` [PATCH 0/2] s5p-mfc fix for using reserved memory Marek Szyprowski
@ 2016-12-21  5:44     ` pankaj.dubey
  0 siblings, 0 replies; 9+ messages in thread
From: pankaj.dubey @ 2016-12-21  5:44 UTC (permalink / raw)
  To: Marek Szyprowski, linux-media, linux-kernel, linux-samsung-soc,
	linux-arm-kernel
  Cc: kyungmin.park, jtp.park, mchehab, mchehab, hans.verkuil, krzk,
	kgene, javier

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] media: s5p-mfc: convert drivers to use the new vb2_queue dev field
  2016-12-16 11:48   ` [PATCH 1/2] media: s5p-mfc: convert drivers to use the new vb2_queue dev field Pankaj Dubey
@ 2017-02-24 19:22     ` Javier Martinez Canillas
  2017-02-27  3:09       ` pankaj.dubey
  0 siblings, 1 reply; 9+ messages in thread
From: Javier Martinez Canillas @ 2017-02-24 19:22 UTC (permalink / raw)
  To: Pankaj Dubey, linux-media, linux-kernel, linux-samsung-soc,
	linux-arm-kernel
  Cc: kyungmin.park, jtp.park, mchehab, mchehab, hans.verkuil, krzk,
	kgene, Smitha T Murthy

Hello Pankaj,

On 12/16/2016 08:48 AM, Pankaj Dubey wrote:
> 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
> -------------------------------------------------------
> 

On which kernel version did you face this issue?

> 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();
>

Please correct me if I'm wrong, but AFAIU this shouldn't be needed in
the s5p-mfc driver since the videobuf2 core either uses the vb2_queue
.dev field or the vb2_queue .alloc_devs. And the latter is set in the
s5p_mfc_queue_setup() function, so the .dev field shouldn't be used.

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] media: s5p-mfc: fix MMAP of mfc buffer during reqbufs
  2016-12-16 11:48   ` [PATCH 2/2] media: s5p-mfc: fix MMAP of mfc buffer during reqbufs Pankaj Dubey
@ 2017-02-24 19:42     ` Javier Martinez Canillas
  2017-02-27  3:17       ` pankaj.dubey
  0 siblings, 1 reply; 9+ messages in thread
From: Javier Martinez Canillas @ 2017-02-24 19:42 UTC (permalink / raw)
  To: Pankaj Dubey, linux-media, linux-kernel, linux-samsung-soc,
	linux-arm-kernel
  Cc: kyungmin.park, jtp.park, mchehab, mchehab, hans.verkuil, krzk,
	kgene, Smitha T Murthy

Hello Pankaj,

On 12/16/2016 08:48 AM, Pankaj Dubey wrote:
> 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.
> 

I see, so likely you were facing the issue described in patch 1/2 after this
patch since the driver doesn't set alloc_devs when IOMMU is disabled, right?

In any case, I guess these patches have been superseded by Marek's series[0]
so they are no longer needed?

[0]: https://www.spinics.net/lists/linux-media/msg111156.html

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] media: s5p-mfc: convert drivers to use the new vb2_queue dev field
  2017-02-24 19:22     ` Javier Martinez Canillas
@ 2017-02-27  3:09       ` pankaj.dubey
  0 siblings, 0 replies; 9+ messages in thread
From: pankaj.dubey @ 2017-02-27  3:09 UTC (permalink / raw)
  To: Javier Martinez Canillas, linux-media, linux-kernel,
	linux-samsung-soc, linux-arm-kernel
  Cc: kyungmin.park, jtp.park, mchehab, mchehab, hans.verkuil, krzk,
	kgene, Smitha T Murthy

Hello Javier,

On Saturday 25 February 2017 12:52 AM, Javier Martinez Canillas wrote:
> Hello Pankaj,
> 
> On 12/16/2016 08:48 AM, Pankaj Dubey wrote:
>> 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
>> -------------------------------------------------------
>>
> 
> On which kernel version did you face this issue?
> 

We observed this issue, on Linux 4.9 kernel while doing some experiment
for using reserved-memory for MFC on Exynos7880 based development board.

Anyways FYI, This patch is series is superseded by patch [1] after
review comments and suggestion from Marek. Patch [1] has been accepted
and merged and working well for us.

[1]: https://patchwork.kernel.org/patch/9482499/

Thanks,
Pankaj Dubey

>> 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();
>>
> 
> Please correct me if I'm wrong, but AFAIU this shouldn't be needed in
> the s5p-mfc driver since the videobuf2 core either uses the vb2_queue
> .dev field or the vb2_queue .alloc_devs. And the latter is set in the
> s5p_mfc_queue_setup() function, so the .dev field shouldn't be used.
> 
> Best regards,
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] media: s5p-mfc: fix MMAP of mfc buffer during reqbufs
  2017-02-24 19:42     ` Javier Martinez Canillas
@ 2017-02-27  3:17       ` pankaj.dubey
  0 siblings, 0 replies; 9+ messages in thread
From: pankaj.dubey @ 2017-02-27  3:17 UTC (permalink / raw)
  To: Javier Martinez Canillas, linux-media, linux-kernel,
	linux-samsung-soc, linux-arm-kernel
  Cc: kyungmin.park, jtp.park, mchehab, mchehab, hans.verkuil, krzk,
	kgene, Smitha T Murthy

Hello Javier,

On Saturday 25 February 2017 01:12 AM, Javier Martinez Canillas wrote:
> Hello Pankaj,
> 
> On 12/16/2016 08:48 AM, Pankaj Dubey wrote:
>> 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.
>>
> 
> I see, so likely you were facing the issue described in patch 1/2 after this
> patch since the driver doesn't set alloc_devs when IOMMU is disabled, right?
> 

Yes.

> In any case, I guess these patches have been superseded by Marek's series[0]
> so they are no longer needed?
> 

Yes, these patches have been superseded but now by Marek's series.
I missed to check Marek's series [0] due to some official assignment,
but we followed up our patch series with Marek, and fix was provided in
of_reserved_mem.c via patch [1] which has been accepted and merged as
well. I will try to find out some time and test Marek's patch series [0].

[1]: https://patchwork.kernel.org/patch/9482499/

Thanks,
Pankaj Dubey

> [0]: https://www.spinics.net/lists/linux-media/msg111156.html
> 
> Best regards,
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2017-02-27  3:14 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CGME20161216115004epcas1p276eddca803dcafe3470e223386b86da0@epcas1p2.samsung.com>
2016-12-16 11:48 ` [PATCH 0/2] s5p-mfc fix for using reserved memory Pankaj Dubey
2016-12-16 11:48   ` [PATCH 1/2] media: s5p-mfc: convert drivers to use the new vb2_queue dev field Pankaj Dubey
2017-02-24 19:22     ` Javier Martinez Canillas
2017-02-27  3:09       ` pankaj.dubey
2016-12-16 11:48   ` [PATCH 2/2] media: s5p-mfc: fix MMAP of mfc buffer during reqbufs Pankaj Dubey
2017-02-24 19:42     ` Javier Martinez Canillas
2017-02-27  3:17       ` pankaj.dubey
2016-12-20 11:58   ` [PATCH 0/2] s5p-mfc fix for using reserved memory Marek Szyprowski
2016-12-21  5:44     ` pankaj.dubey

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).