* [RFC PATCH 0/9] Drop VB2_USERPTR + dma-contig combo
@ 2020-02-21 8:45 Hans Verkuil
2020-02-21 8:45 ` [RFC PATCH 1/9] mtk-vcodec: drop VB2_USERPTR Hans Verkuil
` (8 more replies)
0 siblings, 9 replies; 19+ messages in thread
From: Hans Verkuil @ 2020-02-21 8:45 UTC (permalink / raw)
To: linux-media
The user of VB2_USERPTR with vb2_dma_contig_memops makes no sense
since it requires that the user pointer buffer is in contiguous
memory. This was required with old hacks in the past, but these
days we have DMABUF to take that role.
This RFC series drops VB2_USERPTR from any vb2_queue that uses
vb2_dma_contig_memops.
Note that this series depends on a patch that adds VB2_DMABUF
to any driver that didn't have that:
https://patchwork.linuxtv.org/patch/61782/
And for the rkisp1 a patch to remove VB2_USERPTR has already
been posted:
https://patchwork.linuxtv.org/patch/61780/
Regards,
Hans
Hans Verkuil (9):
mtk-vcodec: drop VB2_USERPTR
solo6x10: drop VB2_USERPTR
m2m-deinterlace: drop VB2_USERPTR
mcam-core: drop VB2_USERPTR
sh_veu: drop VB2_USERPTR
mx2_emmaprp: drop VB2_USERPTR
davinci: drop VB2_USERPTR
exynos/s3c/s5p: drop VB2_USERPTR
omap3isp/rcar_fdp1/vsp1/xilinx: drop VB2_USERPTR
drivers/media/pci/solo6x10/solo6x10-v4l2.c | 2 +-
drivers/media/platform/davinci/vpbe_display.c | 2 +-
drivers/media/platform/davinci/vpif_capture.c | 2 +-
drivers/media/platform/davinci/vpif_display.c | 2 +-
drivers/media/platform/exynos-gsc/gsc-m2m.c | 4 ++--
drivers/media/platform/exynos4-is/fimc-capture.c | 2 +-
drivers/media/platform/exynos4-is/fimc-isp-video.c | 2 +-
drivers/media/platform/exynos4-is/fimc-lite.c | 2 +-
drivers/media/platform/exynos4-is/fimc-m2m.c | 4 ++--
drivers/media/platform/m2m-deinterlace.c | 4 ++--
drivers/media/platform/marvell-ccic/mcam-core.c | 2 +-
drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c | 9 ++-------
drivers/media/platform/mx2_emmaprp.c | 4 ++--
drivers/media/platform/omap3isp/ispvideo.c | 2 +-
drivers/media/platform/rcar_fdp1.c | 4 ++--
drivers/media/platform/s3c-camif/camif-capture.c | 2 +-
drivers/media/platform/s5p-g2d/g2d.c | 4 ++--
drivers/media/platform/s5p-jpeg/jpeg-core.c | 4 ++--
drivers/media/platform/s5p-mfc/s5p_mfc.c | 6 ++----
drivers/media/platform/sh_veu.c | 4 ++--
drivers/media/platform/vsp1/vsp1_video.c | 2 +-
drivers/media/platform/xilinx/xilinx-dma.c | 2 +-
22 files changed, 32 insertions(+), 39 deletions(-)
--
2.25.0
^ permalink raw reply [flat|nested] 19+ messages in thread
* [RFC PATCH 1/9] mtk-vcodec: drop VB2_USERPTR
2020-02-21 8:45 [RFC PATCH 0/9] Drop VB2_USERPTR + dma-contig combo Hans Verkuil
@ 2020-02-21 8:45 ` Hans Verkuil
2020-02-21 8:45 ` [RFC PATCH 2/9] solo6x10: " Hans Verkuil
` (7 subsequent siblings)
8 siblings, 0 replies; 19+ messages in thread
From: Hans Verkuil @ 2020-02-21 8:45 UTC (permalink / raw)
To: linux-media; +Cc: Hans Verkuil
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
index d469ff6464b2..910228922791 100644
--- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
+++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
@@ -1274,13 +1274,8 @@ int mtk_vcodec_enc_queue_init(void *priv, struct vb2_queue *src_vq,
struct mtk_vcodec_ctx *ctx = priv;
int ret;
- /* Note: VB2_USERPTR works with dma-contig because mt8173
- * support iommu
- * https://patchwork.kernel.org/patch/8335461/
- * https://patchwork.kernel.org/patch/7596181/
- */
src_vq->type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
- src_vq->io_modes = VB2_DMABUF | VB2_MMAP | VB2_USERPTR;
+ src_vq->io_modes = VB2_DMABUF | VB2_MMAP;
src_vq->drv_priv = ctx;
src_vq->buf_struct_size = sizeof(struct mtk_video_enc_buf);
src_vq->ops = &mtk_venc_vb2_ops;
@@ -1294,7 +1289,7 @@ int mtk_vcodec_enc_queue_init(void *priv, struct vb2_queue *src_vq,
return ret;
dst_vq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
- dst_vq->io_modes = VB2_DMABUF | VB2_MMAP | VB2_USERPTR;
+ dst_vq->io_modes = VB2_DMABUF | VB2_MMAP;
dst_vq->drv_priv = ctx;
dst_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
dst_vq->ops = &mtk_venc_vb2_ops;
--
2.25.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [RFC PATCH 2/9] solo6x10: drop VB2_USERPTR
2020-02-21 8:45 [RFC PATCH 0/9] Drop VB2_USERPTR + dma-contig combo Hans Verkuil
2020-02-21 8:45 ` [RFC PATCH 1/9] mtk-vcodec: drop VB2_USERPTR Hans Verkuil
@ 2020-02-21 8:45 ` Hans Verkuil
2020-02-21 8:45 ` [RFC PATCH 3/9] m2m-deinterlace: " Hans Verkuil
` (6 subsequent siblings)
8 siblings, 0 replies; 19+ messages in thread
From: Hans Verkuil @ 2020-02-21 8:45 UTC (permalink / raw)
To: linux-media; +Cc: Hans Verkuil
The combination of VB2_USERPTR and dma-contig makes no sense for
this device, drop it.
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
drivers/media/pci/solo6x10/solo6x10-v4l2.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/pci/solo6x10/solo6x10-v4l2.c b/drivers/media/pci/solo6x10/solo6x10-v4l2.c
index 9d290099b7a0..8a5ece6465a2 100644
--- a/drivers/media/pci/solo6x10/solo6x10-v4l2.c
+++ b/drivers/media/pci/solo6x10/solo6x10-v4l2.c
@@ -667,7 +667,7 @@ int solo_v4l2_init(struct solo_dev *solo_dev, unsigned nr)
video_set_drvdata(solo_dev->vfd, solo_dev);
solo_dev->vidq.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
- solo_dev->vidq.io_modes = VB2_MMAP | VB2_USERPTR | VB2_READ | VB2_DMABUF;
+ solo_dev->vidq.io_modes = VB2_MMAP | VB2_READ | VB2_DMABUF;
solo_dev->vidq.ops = &solo_video_qops;
solo_dev->vidq.mem_ops = &vb2_dma_contig_memops;
solo_dev->vidq.drv_priv = solo_dev;
--
2.25.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [RFC PATCH 3/9] m2m-deinterlace: drop VB2_USERPTR
2020-02-21 8:45 [RFC PATCH 0/9] Drop VB2_USERPTR + dma-contig combo Hans Verkuil
2020-02-21 8:45 ` [RFC PATCH 1/9] mtk-vcodec: drop VB2_USERPTR Hans Verkuil
2020-02-21 8:45 ` [RFC PATCH 2/9] solo6x10: " Hans Verkuil
@ 2020-02-21 8:45 ` Hans Verkuil
2020-02-21 8:45 ` [RFC PATCH 4/9] mcam-core: " Hans Verkuil
` (5 subsequent siblings)
8 siblings, 0 replies; 19+ messages in thread
From: Hans Verkuil @ 2020-02-21 8:45 UTC (permalink / raw)
To: linux-media; +Cc: Hans Verkuil
The combination of VB2_USERPTR and dma-contig makes no sense for
this device, drop it.
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
drivers/media/platform/m2m-deinterlace.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/m2m-deinterlace.c b/drivers/media/platform/m2m-deinterlace.c
index 9ad24c86c5ab..ed2b36bb47e4 100644
--- a/drivers/media/platform/m2m-deinterlace.c
+++ b/drivers/media/platform/m2m-deinterlace.c
@@ -799,7 +799,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
int ret;
src_vq->type = V4L2_BUF_TYPE_VIDEO_OUTPUT;
- src_vq->io_modes = VB2_MMAP | VB2_USERPTR | VB2_DMABUF;
+ src_vq->io_modes = VB2_MMAP | VB2_DMABUF;
src_vq->drv_priv = ctx;
src_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
src_vq->ops = &deinterlace_qops;
@@ -818,7 +818,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
return ret;
dst_vq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
- dst_vq->io_modes = VB2_MMAP | VB2_USERPTR | VB2_DMABUF;
+ dst_vq->io_modes = VB2_MMAP | VB2_DMABUF;
dst_vq->drv_priv = ctx;
dst_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
dst_vq->ops = &deinterlace_qops;
--
2.25.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [RFC PATCH 4/9] mcam-core: drop VB2_USERPTR
2020-02-21 8:45 [RFC PATCH 0/9] Drop VB2_USERPTR + dma-contig combo Hans Verkuil
` (2 preceding siblings ...)
2020-02-21 8:45 ` [RFC PATCH 3/9] m2m-deinterlace: " Hans Verkuil
@ 2020-02-21 8:45 ` Hans Verkuil
2020-02-21 8:45 ` [RFC PATCH 5/9] sh_veu: " Hans Verkuil
` (4 subsequent siblings)
8 siblings, 0 replies; 19+ messages in thread
From: Hans Verkuil @ 2020-02-21 8:45 UTC (permalink / raw)
To: linux-media; +Cc: Hans Verkuil
The combination of VB2_USERPTR and dma-contig makes no sense for
this device, drop it.
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
drivers/media/platform/marvell-ccic/mcam-core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c b/drivers/media/platform/marvell-ccic/mcam-core.c
index 803baf97f06e..4450307ae654 100644
--- a/drivers/media/platform/marvell-ccic/mcam-core.c
+++ b/drivers/media/platform/marvell-ccic/mcam-core.c
@@ -1300,7 +1300,7 @@ static int mcam_setup_vb2(struct mcam_camera *cam)
vq->drv_priv = cam;
vq->lock = &cam->s_mutex;
vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
- vq->io_modes = VB2_MMAP | VB2_USERPTR | VB2_DMABUF | VB2_READ;
+ vq->io_modes = VB2_MMAP | VB2_DMABUF | VB2_READ;
vq->buf_struct_size = sizeof(struct mcam_vb_buffer);
vq->dev = cam->dev;
INIT_LIST_HEAD(&cam->buffers);
--
2.25.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [RFC PATCH 5/9] sh_veu: drop VB2_USERPTR
2020-02-21 8:45 [RFC PATCH 0/9] Drop VB2_USERPTR + dma-contig combo Hans Verkuil
` (3 preceding siblings ...)
2020-02-21 8:45 ` [RFC PATCH 4/9] mcam-core: " Hans Verkuil
@ 2020-02-21 8:45 ` Hans Verkuil
2020-02-21 8:45 ` [RFC PATCH 6/9] mx2_emmaprp: " Hans Verkuil
` (3 subsequent siblings)
8 siblings, 0 replies; 19+ messages in thread
From: Hans Verkuil @ 2020-02-21 8:45 UTC (permalink / raw)
To: linux-media; +Cc: Hans Verkuil
The combination of VB2_USERPTR and dma-contig makes no sense for
this device, drop it.
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
drivers/media/platform/sh_veu.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/sh_veu.c b/drivers/media/platform/sh_veu.c
index b95a7e2ede55..fc5b31da6f83 100644
--- a/drivers/media/platform/sh_veu.c
+++ b/drivers/media/platform/sh_veu.c
@@ -924,7 +924,7 @@ static int sh_veu_queue_init(void *priv, struct vb2_queue *src_vq,
memset(src_vq, 0, sizeof(*src_vq));
src_vq->type = V4L2_BUF_TYPE_VIDEO_OUTPUT;
- src_vq->io_modes = VB2_MMAP | VB2_DMABUF | VB2_USERPTR;
+ src_vq->io_modes = VB2_MMAP | VB2_DMABUF;
src_vq->drv_priv = veu;
src_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
src_vq->ops = &sh_veu_qops;
@@ -939,7 +939,7 @@ static int sh_veu_queue_init(void *priv, struct vb2_queue *src_vq,
memset(dst_vq, 0, sizeof(*dst_vq));
dst_vq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
- dst_vq->io_modes = VB2_MMAP | VB2_DMABUF | VB2_USERPTR;
+ dst_vq->io_modes = VB2_MMAP | VB2_DMABUF;
dst_vq->drv_priv = veu;
dst_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
dst_vq->ops = &sh_veu_qops;
--
2.25.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [RFC PATCH 6/9] mx2_emmaprp: drop VB2_USERPTR
2020-02-21 8:45 [RFC PATCH 0/9] Drop VB2_USERPTR + dma-contig combo Hans Verkuil
` (4 preceding siblings ...)
2020-02-21 8:45 ` [RFC PATCH 5/9] sh_veu: " Hans Verkuil
@ 2020-02-21 8:45 ` Hans Verkuil
2020-02-21 8:45 ` [RFC PATCH 7/9] davinci: " Hans Verkuil
` (2 subsequent siblings)
8 siblings, 0 replies; 19+ messages in thread
From: Hans Verkuil @ 2020-02-21 8:45 UTC (permalink / raw)
To: linux-media; +Cc: Hans Verkuil
The combination of VB2_USERPTR and dma-contig makes no sense for
this device, drop it.
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
drivers/media/platform/mx2_emmaprp.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/mx2_emmaprp.c b/drivers/media/platform/mx2_emmaprp.c
index 27779b75df54..832871c08a21 100644
--- a/drivers/media/platform/mx2_emmaprp.c
+++ b/drivers/media/platform/mx2_emmaprp.c
@@ -688,7 +688,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
int ret;
src_vq->type = V4L2_BUF_TYPE_VIDEO_OUTPUT;
- src_vq->io_modes = VB2_MMAP | VB2_USERPTR | VB2_DMABUF;
+ src_vq->io_modes = VB2_MMAP | VB2_DMABUF;
src_vq->drv_priv = ctx;
src_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
src_vq->ops = &emmaprp_qops;
@@ -702,7 +702,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
return ret;
dst_vq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
- dst_vq->io_modes = VB2_MMAP | VB2_USERPTR | VB2_DMABUF;
+ dst_vq->io_modes = VB2_MMAP | VB2_DMABUF;
dst_vq->drv_priv = ctx;
dst_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
dst_vq->ops = &emmaprp_qops;
--
2.25.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [RFC PATCH 7/9] davinci: drop VB2_USERPTR
2020-02-21 8:45 [RFC PATCH 0/9] Drop VB2_USERPTR + dma-contig combo Hans Verkuil
` (5 preceding siblings ...)
2020-02-21 8:45 ` [RFC PATCH 6/9] mx2_emmaprp: " Hans Verkuil
@ 2020-02-21 8:45 ` Hans Verkuil
2020-02-21 8:45 ` [RFC PATCH 8/9] exynos/s3c/s5p: " Hans Verkuil
2020-02-21 8:45 ` [RFC PATCH 9/9] omap3isp/rcar_fdp1/vsp1/xilinx: " Hans Verkuil
8 siblings, 0 replies; 19+ messages in thread
From: Hans Verkuil @ 2020-02-21 8:45 UTC (permalink / raw)
To: linux-media; +Cc: Hans Verkuil, Lad Prabhakar
The combination of VB2_USERPTR and dma-contig makes no sense for
these devices, drop it.
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Cc: Lad Prabhakar <prabhakar.csengg@gmail.com>
---
drivers/media/platform/davinci/vpbe_display.c | 2 +-
drivers/media/platform/davinci/vpif_capture.c | 2 +-
drivers/media/platform/davinci/vpif_display.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/media/platform/davinci/vpbe_display.c b/drivers/media/platform/davinci/vpbe_display.c
index ae419958e420..2641973998fe 100644
--- a/drivers/media/platform/davinci/vpbe_display.c
+++ b/drivers/media/platform/davinci/vpbe_display.c
@@ -1426,7 +1426,7 @@ static int vpbe_display_probe(struct platform_device *pdev)
q = &disp_dev->dev[i]->buffer_queue;
memset(q, 0, sizeof(*q));
q->type = V4L2_BUF_TYPE_VIDEO_OUTPUT;
- q->io_modes = VB2_MMAP | VB2_USERPTR | VB2_DMABUF;
+ q->io_modes = VB2_MMAP | VB2_DMABUF;
q->drv_priv = disp_dev->dev[i];
q->ops = &video_qops;
q->mem_ops = &vb2_dma_contig_memops;
diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c
index 71f4fe882d13..9c8b05f7fc29 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -1435,7 +1435,7 @@ static int vpif_probe_complete(void)
/* Initialize vb2 queue */
q = &common->buffer_queue;
q->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
- q->io_modes = VB2_MMAP | VB2_USERPTR | VB2_DMABUF;
+ q->io_modes = VB2_MMAP | VB2_DMABUF;
q->drv_priv = ch;
q->ops = &video_qops;
q->mem_ops = &vb2_dma_contig_memops;
diff --git a/drivers/media/platform/davinci/vpif_display.c b/drivers/media/platform/davinci/vpif_display.c
index abbdbac08e6f..ee6f1fd2baac 100644
--- a/drivers/media/platform/davinci/vpif_display.c
+++ b/drivers/media/platform/davinci/vpif_display.c
@@ -1181,7 +1181,7 @@ static int vpif_probe_complete(void)
/* Initialize vb2 queue */
q = &common->buffer_queue;
q->type = V4L2_BUF_TYPE_VIDEO_OUTPUT;
- q->io_modes = VB2_MMAP | VB2_USERPTR | VB2_DMABUF;
+ q->io_modes = VB2_MMAP | VB2_DMABUF;
q->drv_priv = ch;
q->ops = &video_qops;
q->mem_ops = &vb2_dma_contig_memops;
--
2.25.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [RFC PATCH 8/9] exynos/s3c/s5p: drop VB2_USERPTR
2020-02-21 8:45 [RFC PATCH 0/9] Drop VB2_USERPTR + dma-contig combo Hans Verkuil
` (6 preceding siblings ...)
2020-02-21 8:45 ` [RFC PATCH 7/9] davinci: " Hans Verkuil
@ 2020-02-21 8:45 ` Hans Verkuil
2020-02-21 8:53 ` Tomasz Figa
2020-02-21 8:45 ` [RFC PATCH 9/9] omap3isp/rcar_fdp1/vsp1/xilinx: " Hans Verkuil
8 siblings, 1 reply; 19+ messages in thread
From: Hans Verkuil @ 2020-02-21 8:45 UTC (permalink / raw)
To: linux-media; +Cc: Hans Verkuil, Sylwester Nawrocki, Tomasz Figa
The combination of VB2_USERPTR and dma-contig makes no sense for
these devices, drop it.
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Cc: Sylwester Nawrocki <snawrocki@kernel.org>
Cc: Tomasz Figa <tfiga@chromium.org>
---
drivers/media/platform/exynos-gsc/gsc-m2m.c | 4 ++--
drivers/media/platform/exynos4-is/fimc-capture.c | 2 +-
drivers/media/platform/exynos4-is/fimc-isp-video.c | 2 +-
drivers/media/platform/exynos4-is/fimc-lite.c | 2 +-
drivers/media/platform/exynos4-is/fimc-m2m.c | 4 ++--
drivers/media/platform/s3c-camif/camif-capture.c | 2 +-
drivers/media/platform/s5p-g2d/g2d.c | 4 ++--
drivers/media/platform/s5p-jpeg/jpeg-core.c | 4 ++--
drivers/media/platform/s5p-mfc/s5p_mfc.c | 6 ++----
9 files changed, 14 insertions(+), 16 deletions(-)
diff --git a/drivers/media/platform/exynos-gsc/gsc-m2m.c b/drivers/media/platform/exynos-gsc/gsc-m2m.c
index 35a1d0d6dd66..f4b192e49c80 100644
--- a/drivers/media/platform/exynos-gsc/gsc-m2m.c
+++ b/drivers/media/platform/exynos-gsc/gsc-m2m.c
@@ -583,7 +583,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
memset(src_vq, 0, sizeof(*src_vq));
src_vq->type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
- src_vq->io_modes = VB2_MMAP | VB2_USERPTR | VB2_DMABUF;
+ src_vq->io_modes = VB2_MMAP | VB2_DMABUF;
src_vq->drv_priv = ctx;
src_vq->ops = &gsc_m2m_qops;
src_vq->mem_ops = &vb2_dma_contig_memops;
@@ -598,7 +598,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
memset(dst_vq, 0, sizeof(*dst_vq));
dst_vq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
- dst_vq->io_modes = VB2_MMAP | VB2_USERPTR | VB2_DMABUF;
+ dst_vq->io_modes = VB2_MMAP | VB2_DMABUF;
dst_vq->drv_priv = ctx;
dst_vq->ops = &gsc_m2m_qops;
dst_vq->mem_ops = &vb2_dma_contig_memops;
diff --git a/drivers/media/platform/exynos4-is/fimc-capture.c b/drivers/media/platform/exynos4-is/fimc-capture.c
index 121d609ff856..8d14207b3403 100644
--- a/drivers/media/platform/exynos4-is/fimc-capture.c
+++ b/drivers/media/platform/exynos4-is/fimc-capture.c
@@ -1771,7 +1771,7 @@ static int fimc_register_capture_device(struct fimc_dev *fimc,
memset(q, 0, sizeof(*q));
q->type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
- q->io_modes = VB2_MMAP | VB2_USERPTR | VB2_DMABUF;
+ q->io_modes = VB2_MMAP | VB2_DMABUF;
q->drv_priv = ctx;
q->ops = &fimc_capture_qops;
q->mem_ops = &vb2_dma_contig_memops;
diff --git a/drivers/media/platform/exynos4-is/fimc-isp-video.c b/drivers/media/platform/exynos4-is/fimc-isp-video.c
index 55bae20eb8db..94f3215916f6 100644
--- a/drivers/media/platform/exynos4-is/fimc-isp-video.c
+++ b/drivers/media/platform/exynos4-is/fimc-isp-video.c
@@ -587,7 +587,7 @@ int fimc_isp_video_device_register(struct fimc_isp *isp,
memset(q, 0, sizeof(*q));
q->type = type;
- q->io_modes = VB2_MMAP | VB2_DMABUF | VB2_USERPTR;
+ q->io_modes = VB2_MMAP | VB2_DMABUF;
q->ops = &isp_video_capture_qops;
q->mem_ops = &vb2_dma_contig_memops;
q->buf_struct_size = sizeof(struct isp_video_buf);
diff --git a/drivers/media/platform/exynos4-is/fimc-lite.c b/drivers/media/platform/exynos4-is/fimc-lite.c
index d06bf4865b84..3c2c70b252bb 100644
--- a/drivers/media/platform/exynos4-is/fimc-lite.c
+++ b/drivers/media/platform/exynos4-is/fimc-lite.c
@@ -1276,7 +1276,7 @@ static int fimc_lite_subdev_registered(struct v4l2_subdev *sd)
memset(q, 0, sizeof(*q));
q->type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
- q->io_modes = VB2_MMAP | VB2_DMABUF | VB2_USERPTR;
+ q->io_modes = VB2_MMAP | VB2_DMABUF;
q->ops = &fimc_lite_qops;
q->mem_ops = &vb2_dma_contig_memops;
q->buf_struct_size = sizeof(struct flite_buffer);
diff --git a/drivers/media/platform/exynos4-is/fimc-m2m.c b/drivers/media/platform/exynos4-is/fimc-m2m.c
index c70c2cbe3eb1..3323563ed913 100644
--- a/drivers/media/platform/exynos4-is/fimc-m2m.c
+++ b/drivers/media/platform/exynos4-is/fimc-m2m.c
@@ -554,7 +554,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
int ret;
src_vq->type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
- src_vq->io_modes = VB2_MMAP | VB2_USERPTR | VB2_DMABUF;
+ src_vq->io_modes = VB2_MMAP | VB2_DMABUF;
src_vq->drv_priv = ctx;
src_vq->ops = &fimc_qops;
src_vq->mem_ops = &vb2_dma_contig_memops;
@@ -568,7 +568,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
return ret;
dst_vq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
- dst_vq->io_modes = VB2_MMAP | VB2_USERPTR | VB2_DMABUF;
+ dst_vq->io_modes = VB2_MMAP | VB2_DMABUF;
dst_vq->drv_priv = ctx;
dst_vq->ops = &fimc_qops;
dst_vq->mem_ops = &vb2_dma_contig_memops;
diff --git a/drivers/media/platform/s3c-camif/camif-capture.c b/drivers/media/platform/s3c-camif/camif-capture.c
index 54989dacaf5d..eb99468a5427 100644
--- a/drivers/media/platform/s3c-camif/camif-capture.c
+++ b/drivers/media/platform/s3c-camif/camif-capture.c
@@ -1121,7 +1121,7 @@ int s3c_camif_register_video_node(struct camif_dev *camif, int idx)
memset(q, 0, sizeof(*q));
q->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
- q->io_modes = VB2_MMAP | VB2_DMABUF | VB2_USERPTR;
+ q->io_modes = VB2_MMAP | VB2_DMABUF;
q->ops = &s3c_camif_qops;
q->mem_ops = &vb2_dma_contig_memops;
q->buf_struct_size = sizeof(struct camif_buffer);
diff --git a/drivers/media/platform/s5p-g2d/g2d.c b/drivers/media/platform/s5p-g2d/g2d.c
index 98f94e1fa6b8..a8f8c9e00452 100644
--- a/drivers/media/platform/s5p-g2d/g2d.c
+++ b/drivers/media/platform/s5p-g2d/g2d.c
@@ -144,7 +144,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
int ret;
src_vq->type = V4L2_BUF_TYPE_VIDEO_OUTPUT;
- src_vq->io_modes = VB2_MMAP | VB2_DMABUF | VB2_USERPTR;
+ src_vq->io_modes = VB2_MMAP | VB2_DMABUF;
src_vq->drv_priv = ctx;
src_vq->ops = &g2d_qops;
src_vq->mem_ops = &vb2_dma_contig_memops;
@@ -158,7 +158,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
return ret;
dst_vq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
- dst_vq->io_modes = VB2_MMAP | VB2_DMABUF | VB2_USERPTR;
+ dst_vq->io_modes = VB2_MMAP | VB2_DMABUF;
dst_vq->drv_priv = ctx;
dst_vq->ops = &g2d_qops;
dst_vq->mem_ops = &vb2_dma_contig_memops;
diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c b/drivers/media/platform/s5p-jpeg/jpeg-core.c
index 4c10ec0d7da4..d03164854576 100644
--- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
+++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
@@ -2620,7 +2620,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
int ret;
src_vq->type = V4L2_BUF_TYPE_VIDEO_OUTPUT;
- src_vq->io_modes = VB2_MMAP | VB2_DMABUF | VB2_USERPTR;
+ src_vq->io_modes = VB2_MMAP | VB2_DMABUF;
src_vq->drv_priv = ctx;
src_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
src_vq->ops = &s5p_jpeg_qops;
@@ -2634,7 +2634,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
return ret;
dst_vq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
- dst_vq->io_modes = VB2_MMAP | VB2_DMABUF | VB2_USERPTR;
+ dst_vq->io_modes = VB2_MMAP | VB2_DMABUF;
dst_vq->drv_priv = ctx;
dst_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
dst_vq->ops = &s5p_jpeg_qops;
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c b/drivers/media/platform/s5p-mfc/s5p_mfc.c
index ff770328f690..32df5e26daab 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
@@ -844,11 +844,10 @@ 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->io_modes = VB2_MMAP | VB2_DMABUF;
if (vdev == dev->vfd_dec) {
- q->io_modes = VB2_MMAP | VB2_DMABUF;
q->ops = get_dec_queue_ops();
} else if (vdev == dev->vfd_enc) {
- q->io_modes = VB2_MMAP | VB2_DMABUF | VB2_USERPTR;
q->ops = get_enc_queue_ops();
} else {
ret = -ENOENT;
@@ -871,11 +870,10 @@ static int s5p_mfc_open(struct file *file)
q->type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
q->drv_priv = &ctx->fh;
q->lock = &dev->mfc_mutex;
+ q->io_modes = VB2_MMAP | VB2_DMABUF;
if (vdev == dev->vfd_dec) {
- q->io_modes = VB2_MMAP | VB2_DMABUF;
q->ops = get_dec_queue_ops();
} else if (vdev == dev->vfd_enc) {
- q->io_modes = VB2_MMAP | VB2_DMABUF | VB2_USERPTR;
q->ops = get_enc_queue_ops();
} else {
ret = -ENOENT;
--
2.25.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [RFC PATCH 9/9] omap3isp/rcar_fdp1/vsp1/xilinx: drop VB2_USERPTR
2020-02-21 8:45 [RFC PATCH 0/9] Drop VB2_USERPTR + dma-contig combo Hans Verkuil
` (7 preceding siblings ...)
2020-02-21 8:45 ` [RFC PATCH 8/9] exynos/s3c/s5p: " Hans Verkuil
@ 2020-02-21 8:45 ` Hans Verkuil
2020-02-21 9:01 ` Kieran Bingham
8 siblings, 1 reply; 19+ messages in thread
From: Hans Verkuil @ 2020-02-21 8:45 UTC (permalink / raw)
To: linux-media; +Cc: Hans Verkuil, Laurent Pinchart, Kieran Bingham
The combination of VB2_USERPTR and dma-contig makes no sense for
these devices, drop it.
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
---
drivers/media/platform/omap3isp/ispvideo.c | 2 +-
drivers/media/platform/rcar_fdp1.c | 4 ++--
drivers/media/platform/vsp1/vsp1_video.c | 2 +-
drivers/media/platform/xilinx/xilinx-dma.c | 2 +-
4 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/media/platform/omap3isp/ispvideo.c b/drivers/media/platform/omap3isp/ispvideo.c
index e8c46ff1aeb4..1104654ba438 100644
--- a/drivers/media/platform/omap3isp/ispvideo.c
+++ b/drivers/media/platform/omap3isp/ispvideo.c
@@ -1319,7 +1319,7 @@ static int isp_video_open(struct file *file)
queue = &handle->queue;
queue->type = video->type;
- queue->io_modes = VB2_MMAP | VB2_DMABUF | VB2_USERPTR;
+ queue->io_modes = VB2_MMAP | VB2_DMABUF;
queue->drv_priv = handle;
queue->ops = &isp_video_queue_ops;
queue->mem_ops = &vb2_dma_contig_memops;
diff --git a/drivers/media/platform/rcar_fdp1.c b/drivers/media/platform/rcar_fdp1.c
index 97bed45360f0..df081f66575f 100644
--- a/drivers/media/platform/rcar_fdp1.c
+++ b/drivers/media/platform/rcar_fdp1.c
@@ -2047,7 +2047,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
int ret;
src_vq->type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
- src_vq->io_modes = VB2_MMAP | VB2_USERPTR | VB2_DMABUF;
+ src_vq->io_modes = VB2_MMAP | VB2_DMABUF;
src_vq->drv_priv = ctx;
src_vq->buf_struct_size = sizeof(struct fdp1_buffer);
src_vq->ops = &fdp1_qops;
@@ -2061,7 +2061,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
return ret;
dst_vq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
- dst_vq->io_modes = VB2_MMAP | VB2_USERPTR | VB2_DMABUF;
+ dst_vq->io_modes = VB2_MMAP | VB2_DMABUF;
dst_vq->drv_priv = ctx;
dst_vq->buf_struct_size = sizeof(struct fdp1_buffer);
dst_vq->ops = &fdp1_qops;
diff --git a/drivers/media/platform/vsp1/vsp1_video.c b/drivers/media/platform/vsp1/vsp1_video.c
index 5e59ed2c3614..112e2092f6d3 100644
--- a/drivers/media/platform/vsp1/vsp1_video.c
+++ b/drivers/media/platform/vsp1/vsp1_video.c
@@ -1300,7 +1300,7 @@ struct vsp1_video *vsp1_video_create(struct vsp1_device *vsp1,
video_set_drvdata(&video->video, video);
video->queue.type = video->type;
- video->queue.io_modes = VB2_MMAP | VB2_USERPTR | VB2_DMABUF;
+ video->queue.io_modes = VB2_MMAP | VB2_DMABUF;
video->queue.lock = &video->lock;
video->queue.drv_priv = video;
video->queue.buf_struct_size = sizeof(struct vsp1_vb2_buffer);
diff --git a/drivers/media/platform/xilinx/xilinx-dma.c b/drivers/media/platform/xilinx/xilinx-dma.c
index b211380a11f2..57e52ad720dd 100644
--- a/drivers/media/platform/xilinx/xilinx-dma.c
+++ b/drivers/media/platform/xilinx/xilinx-dma.c
@@ -708,7 +708,7 @@ int xvip_dma_init(struct xvip_composite_device *xdev, struct xvip_dma *dma,
* instead of 'cat' isn't really a drawback.
*/
dma->queue.type = type;
- dma->queue.io_modes = VB2_MMAP | VB2_USERPTR | VB2_DMABUF;
+ dma->queue.io_modes = VB2_MMAP | VB2_DMABUF;
dma->queue.lock = &dma->lock;
dma->queue.drv_priv = dma;
dma->queue.buf_struct_size = sizeof(struct xvip_dma_buffer);
--
2.25.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [RFC PATCH 8/9] exynos/s3c/s5p: drop VB2_USERPTR
2020-02-21 8:45 ` [RFC PATCH 8/9] exynos/s3c/s5p: " Hans Verkuil
@ 2020-02-21 8:53 ` Tomasz Figa
2020-02-21 11:54 ` Hans Verkuil
0 siblings, 1 reply; 19+ messages in thread
From: Tomasz Figa @ 2020-02-21 8:53 UTC (permalink / raw)
To: Hans Verkuil, Marek Szyprowski
Cc: Linux Media Mailing List, Sylwester Nawrocki
Hi Hans,
On Fri, Feb 21, 2020 at 5:46 PM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
>
> The combination of VB2_USERPTR and dma-contig makes no sense for
> these devices, drop it.
Even though I personally don't like user pointers, I believe at least
some of those devices are fine with USERPTR in case they are behind an
IOMMU, like on the newer Exynos SoCs. +Marek Szyprowski too.
What makes you believe it makes no sense for them?
Best regards,
Tomasz
>
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> Cc: Sylwester Nawrocki <snawrocki@kernel.org>
> Cc: Tomasz Figa <tfiga@chromium.org>
> ---
> drivers/media/platform/exynos-gsc/gsc-m2m.c | 4 ++--
> drivers/media/platform/exynos4-is/fimc-capture.c | 2 +-
> drivers/media/platform/exynos4-is/fimc-isp-video.c | 2 +-
> drivers/media/platform/exynos4-is/fimc-lite.c | 2 +-
> drivers/media/platform/exynos4-is/fimc-m2m.c | 4 ++--
> drivers/media/platform/s3c-camif/camif-capture.c | 2 +-
> drivers/media/platform/s5p-g2d/g2d.c | 4 ++--
> drivers/media/platform/s5p-jpeg/jpeg-core.c | 4 ++--
> drivers/media/platform/s5p-mfc/s5p_mfc.c | 6 ++----
> 9 files changed, 14 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/media/platform/exynos-gsc/gsc-m2m.c b/drivers/media/platform/exynos-gsc/gsc-m2m.c
> index 35a1d0d6dd66..f4b192e49c80 100644
> --- a/drivers/media/platform/exynos-gsc/gsc-m2m.c
> +++ b/drivers/media/platform/exynos-gsc/gsc-m2m.c
> @@ -583,7 +583,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
>
> memset(src_vq, 0, sizeof(*src_vq));
> src_vq->type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
> - src_vq->io_modes = VB2_MMAP | VB2_USERPTR | VB2_DMABUF;
> + src_vq->io_modes = VB2_MMAP | VB2_DMABUF;
> src_vq->drv_priv = ctx;
> src_vq->ops = &gsc_m2m_qops;
> src_vq->mem_ops = &vb2_dma_contig_memops;
> @@ -598,7 +598,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
>
> memset(dst_vq, 0, sizeof(*dst_vq));
> dst_vq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
> - dst_vq->io_modes = VB2_MMAP | VB2_USERPTR | VB2_DMABUF;
> + dst_vq->io_modes = VB2_MMAP | VB2_DMABUF;
> dst_vq->drv_priv = ctx;
> dst_vq->ops = &gsc_m2m_qops;
> dst_vq->mem_ops = &vb2_dma_contig_memops;
> diff --git a/drivers/media/platform/exynos4-is/fimc-capture.c b/drivers/media/platform/exynos4-is/fimc-capture.c
> index 121d609ff856..8d14207b3403 100644
> --- a/drivers/media/platform/exynos4-is/fimc-capture.c
> +++ b/drivers/media/platform/exynos4-is/fimc-capture.c
> @@ -1771,7 +1771,7 @@ static int fimc_register_capture_device(struct fimc_dev *fimc,
>
> memset(q, 0, sizeof(*q));
> q->type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
> - q->io_modes = VB2_MMAP | VB2_USERPTR | VB2_DMABUF;
> + q->io_modes = VB2_MMAP | VB2_DMABUF;
> q->drv_priv = ctx;
> q->ops = &fimc_capture_qops;
> q->mem_ops = &vb2_dma_contig_memops;
> diff --git a/drivers/media/platform/exynos4-is/fimc-isp-video.c b/drivers/media/platform/exynos4-is/fimc-isp-video.c
> index 55bae20eb8db..94f3215916f6 100644
> --- a/drivers/media/platform/exynos4-is/fimc-isp-video.c
> +++ b/drivers/media/platform/exynos4-is/fimc-isp-video.c
> @@ -587,7 +587,7 @@ int fimc_isp_video_device_register(struct fimc_isp *isp,
>
> memset(q, 0, sizeof(*q));
> q->type = type;
> - q->io_modes = VB2_MMAP | VB2_DMABUF | VB2_USERPTR;
> + q->io_modes = VB2_MMAP | VB2_DMABUF;
> q->ops = &isp_video_capture_qops;
> q->mem_ops = &vb2_dma_contig_memops;
> q->buf_struct_size = sizeof(struct isp_video_buf);
> diff --git a/drivers/media/platform/exynos4-is/fimc-lite.c b/drivers/media/platform/exynos4-is/fimc-lite.c
> index d06bf4865b84..3c2c70b252bb 100644
> --- a/drivers/media/platform/exynos4-is/fimc-lite.c
> +++ b/drivers/media/platform/exynos4-is/fimc-lite.c
> @@ -1276,7 +1276,7 @@ static int fimc_lite_subdev_registered(struct v4l2_subdev *sd)
>
> memset(q, 0, sizeof(*q));
> q->type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
> - q->io_modes = VB2_MMAP | VB2_DMABUF | VB2_USERPTR;
> + q->io_modes = VB2_MMAP | VB2_DMABUF;
> q->ops = &fimc_lite_qops;
> q->mem_ops = &vb2_dma_contig_memops;
> q->buf_struct_size = sizeof(struct flite_buffer);
> diff --git a/drivers/media/platform/exynos4-is/fimc-m2m.c b/drivers/media/platform/exynos4-is/fimc-m2m.c
> index c70c2cbe3eb1..3323563ed913 100644
> --- a/drivers/media/platform/exynos4-is/fimc-m2m.c
> +++ b/drivers/media/platform/exynos4-is/fimc-m2m.c
> @@ -554,7 +554,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
> int ret;
>
> src_vq->type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
> - src_vq->io_modes = VB2_MMAP | VB2_USERPTR | VB2_DMABUF;
> + src_vq->io_modes = VB2_MMAP | VB2_DMABUF;
> src_vq->drv_priv = ctx;
> src_vq->ops = &fimc_qops;
> src_vq->mem_ops = &vb2_dma_contig_memops;
> @@ -568,7 +568,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
> return ret;
>
> dst_vq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
> - dst_vq->io_modes = VB2_MMAP | VB2_USERPTR | VB2_DMABUF;
> + dst_vq->io_modes = VB2_MMAP | VB2_DMABUF;
> dst_vq->drv_priv = ctx;
> dst_vq->ops = &fimc_qops;
> dst_vq->mem_ops = &vb2_dma_contig_memops;
> diff --git a/drivers/media/platform/s3c-camif/camif-capture.c b/drivers/media/platform/s3c-camif/camif-capture.c
> index 54989dacaf5d..eb99468a5427 100644
> --- a/drivers/media/platform/s3c-camif/camif-capture.c
> +++ b/drivers/media/platform/s3c-camif/camif-capture.c
> @@ -1121,7 +1121,7 @@ int s3c_camif_register_video_node(struct camif_dev *camif, int idx)
>
> memset(q, 0, sizeof(*q));
> q->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
> - q->io_modes = VB2_MMAP | VB2_DMABUF | VB2_USERPTR;
> + q->io_modes = VB2_MMAP | VB2_DMABUF;
> q->ops = &s3c_camif_qops;
> q->mem_ops = &vb2_dma_contig_memops;
> q->buf_struct_size = sizeof(struct camif_buffer);
> diff --git a/drivers/media/platform/s5p-g2d/g2d.c b/drivers/media/platform/s5p-g2d/g2d.c
> index 98f94e1fa6b8..a8f8c9e00452 100644
> --- a/drivers/media/platform/s5p-g2d/g2d.c
> +++ b/drivers/media/platform/s5p-g2d/g2d.c
> @@ -144,7 +144,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
> int ret;
>
> src_vq->type = V4L2_BUF_TYPE_VIDEO_OUTPUT;
> - src_vq->io_modes = VB2_MMAP | VB2_DMABUF | VB2_USERPTR;
> + src_vq->io_modes = VB2_MMAP | VB2_DMABUF;
> src_vq->drv_priv = ctx;
> src_vq->ops = &g2d_qops;
> src_vq->mem_ops = &vb2_dma_contig_memops;
> @@ -158,7 +158,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
> return ret;
>
> dst_vq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
> - dst_vq->io_modes = VB2_MMAP | VB2_DMABUF | VB2_USERPTR;
> + dst_vq->io_modes = VB2_MMAP | VB2_DMABUF;
> dst_vq->drv_priv = ctx;
> dst_vq->ops = &g2d_qops;
> dst_vq->mem_ops = &vb2_dma_contig_memops;
> diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c b/drivers/media/platform/s5p-jpeg/jpeg-core.c
> index 4c10ec0d7da4..d03164854576 100644
> --- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
> +++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
> @@ -2620,7 +2620,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
> int ret;
>
> src_vq->type = V4L2_BUF_TYPE_VIDEO_OUTPUT;
> - src_vq->io_modes = VB2_MMAP | VB2_DMABUF | VB2_USERPTR;
> + src_vq->io_modes = VB2_MMAP | VB2_DMABUF;
> src_vq->drv_priv = ctx;
> src_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
> src_vq->ops = &s5p_jpeg_qops;
> @@ -2634,7 +2634,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
> return ret;
>
> dst_vq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
> - dst_vq->io_modes = VB2_MMAP | VB2_DMABUF | VB2_USERPTR;
> + dst_vq->io_modes = VB2_MMAP | VB2_DMABUF;
> dst_vq->drv_priv = ctx;
> dst_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
> dst_vq->ops = &s5p_jpeg_qops;
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c b/drivers/media/platform/s5p-mfc/s5p_mfc.c
> index ff770328f690..32df5e26daab 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
> @@ -844,11 +844,10 @@ 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->io_modes = VB2_MMAP | VB2_DMABUF;
> if (vdev == dev->vfd_dec) {
> - q->io_modes = VB2_MMAP | VB2_DMABUF;
> q->ops = get_dec_queue_ops();
> } else if (vdev == dev->vfd_enc) {
> - q->io_modes = VB2_MMAP | VB2_DMABUF | VB2_USERPTR;
> q->ops = get_enc_queue_ops();
> } else {
> ret = -ENOENT;
> @@ -871,11 +870,10 @@ static int s5p_mfc_open(struct file *file)
> q->type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
> q->drv_priv = &ctx->fh;
> q->lock = &dev->mfc_mutex;
> + q->io_modes = VB2_MMAP | VB2_DMABUF;
> if (vdev == dev->vfd_dec) {
> - q->io_modes = VB2_MMAP | VB2_DMABUF;
> q->ops = get_dec_queue_ops();
> } else if (vdev == dev->vfd_enc) {
> - q->io_modes = VB2_MMAP | VB2_DMABUF | VB2_USERPTR;
> q->ops = get_enc_queue_ops();
> } else {
> ret = -ENOENT;
> --
> 2.25.0
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH 9/9] omap3isp/rcar_fdp1/vsp1/xilinx: drop VB2_USERPTR
2020-02-21 8:45 ` [RFC PATCH 9/9] omap3isp/rcar_fdp1/vsp1/xilinx: " Hans Verkuil
@ 2020-02-21 9:01 ` Kieran Bingham
2020-02-21 14:31 ` Laurent Pinchart
0 siblings, 1 reply; 19+ messages in thread
From: Kieran Bingham @ 2020-02-21 9:01 UTC (permalink / raw)
To: Hans Verkuil, linux-media; +Cc: Laurent Pinchart
Hi Hans,
On 21/02/2020 08:45, Hans Verkuil wrote:
> The combination of VB2_USERPTR and dma-contig makes no sense for
> these devices, drop it.
>
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Agreed, the FDP1 and VSP1 expect contiguous physical memory, so a
user-pointer doesn't make much sense to be permitted.
I haven't lokoed at the Xilinx platform in regards to this but I would
be surprised if it wasn't the same issue there of course.
For VSP1/FDP1:
Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> ---
> drivers/media/platform/omap3isp/ispvideo.c | 2 +-
> drivers/media/platform/rcar_fdp1.c | 4 ++--
> drivers/media/platform/vsp1/vsp1_video.c | 2 +-
> drivers/media/platform/xilinx/xilinx-dma.c | 2 +-
> 4 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/media/platform/omap3isp/ispvideo.c b/drivers/media/platform/omap3isp/ispvideo.c
> index e8c46ff1aeb4..1104654ba438 100644
> --- a/drivers/media/platform/omap3isp/ispvideo.c
> +++ b/drivers/media/platform/omap3isp/ispvideo.c
> @@ -1319,7 +1319,7 @@ static int isp_video_open(struct file *file)
>
> queue = &handle->queue;
> queue->type = video->type;
> - queue->io_modes = VB2_MMAP | VB2_DMABUF | VB2_USERPTR;
> + queue->io_modes = VB2_MMAP | VB2_DMABUF;
> queue->drv_priv = handle;
> queue->ops = &isp_video_queue_ops;
> queue->mem_ops = &vb2_dma_contig_memops;
> diff --git a/drivers/media/platform/rcar_fdp1.c b/drivers/media/platform/rcar_fdp1.c
> index 97bed45360f0..df081f66575f 100644
> --- a/drivers/media/platform/rcar_fdp1.c
> +++ b/drivers/media/platform/rcar_fdp1.c
> @@ -2047,7 +2047,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
> int ret;
>
> src_vq->type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
> - src_vq->io_modes = VB2_MMAP | VB2_USERPTR | VB2_DMABUF;
> + src_vq->io_modes = VB2_MMAP | VB2_DMABUF;
> src_vq->drv_priv = ctx;
> src_vq->buf_struct_size = sizeof(struct fdp1_buffer);
> src_vq->ops = &fdp1_qops;
> @@ -2061,7 +2061,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
> return ret;
>
> dst_vq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
> - dst_vq->io_modes = VB2_MMAP | VB2_USERPTR | VB2_DMABUF;
> + dst_vq->io_modes = VB2_MMAP | VB2_DMABUF;
> dst_vq->drv_priv = ctx;
> dst_vq->buf_struct_size = sizeof(struct fdp1_buffer);
> dst_vq->ops = &fdp1_qops;
> diff --git a/drivers/media/platform/vsp1/vsp1_video.c b/drivers/media/platform/vsp1/vsp1_video.c
> index 5e59ed2c3614..112e2092f6d3 100644
> --- a/drivers/media/platform/vsp1/vsp1_video.c
> +++ b/drivers/media/platform/vsp1/vsp1_video.c
> @@ -1300,7 +1300,7 @@ struct vsp1_video *vsp1_video_create(struct vsp1_device *vsp1,
> video_set_drvdata(&video->video, video);
>
> video->queue.type = video->type;
> - video->queue.io_modes = VB2_MMAP | VB2_USERPTR | VB2_DMABUF;
> + video->queue.io_modes = VB2_MMAP | VB2_DMABUF;
> video->queue.lock = &video->lock;
> video->queue.drv_priv = video;
> video->queue.buf_struct_size = sizeof(struct vsp1_vb2_buffer);
> diff --git a/drivers/media/platform/xilinx/xilinx-dma.c b/drivers/media/platform/xilinx/xilinx-dma.c
> index b211380a11f2..57e52ad720dd 100644
> --- a/drivers/media/platform/xilinx/xilinx-dma.c
> +++ b/drivers/media/platform/xilinx/xilinx-dma.c
> @@ -708,7 +708,7 @@ int xvip_dma_init(struct xvip_composite_device *xdev, struct xvip_dma *dma,
> * instead of 'cat' isn't really a drawback.
> */
> dma->queue.type = type;
> - dma->queue.io_modes = VB2_MMAP | VB2_USERPTR | VB2_DMABUF;
> + dma->queue.io_modes = VB2_MMAP | VB2_DMABUF;
> dma->queue.lock = &dma->lock;
> dma->queue.drv_priv = dma;
> dma->queue.buf_struct_size = sizeof(struct xvip_dma_buffer);
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH 8/9] exynos/s3c/s5p: drop VB2_USERPTR
2020-02-21 8:53 ` Tomasz Figa
@ 2020-02-21 11:54 ` Hans Verkuil
2020-02-21 13:11 ` Marek Szyprowski
2020-02-21 13:34 ` Tomasz Figa
0 siblings, 2 replies; 19+ messages in thread
From: Hans Verkuil @ 2020-02-21 11:54 UTC (permalink / raw)
To: Tomasz Figa, Marek Szyprowski
Cc: Linux Media Mailing List, Sylwester Nawrocki
On 2/21/20 9:53 AM, Tomasz Figa wrote:
> Hi Hans,
>
> On Fri, Feb 21, 2020 at 5:46 PM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
>>
>> The combination of VB2_USERPTR and dma-contig makes no sense for
>> these devices, drop it.
>
> Even though I personally don't like user pointers, I believe at least
> some of those devices are fine with USERPTR in case they are behind an
> IOMMU, like on the newer Exynos SoCs. +Marek Szyprowski too.
I would like this to be tested. I always wonder if that has actually
been tested, especially with regards to the partial first and last pages of
the malloc()ed memory. I.e., worst case only 8 bytes may have to be written
to a page if malloc() aligned the pointer poorly. Can the DMA handle that,
even with an IOMMU?
Note that I have the same concern for VB2_USERPTR with dma-sg.
This was a good opportunity to improve v4l2-compliance: it adds sentinels at
the start/end of the buffer, and it checks that those sentinels are never
overwritten. So if this test passes for a driver, then VB2_USERPTR can stay
in, but it should probably have a comment that it has been tested with
v4l2-compliance.
>
> What makes you believe it makes no sense for them?
Serious doubts that this has been properly tested :-)
You really need a test like I wrote today for v4l2-compliance
in order to be certain that it works.
Regards,
Hans
>
> Best regards,
> Tomasz
>
>>
>> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
>> Cc: Sylwester Nawrocki <snawrocki@kernel.org>
>> Cc: Tomasz Figa <tfiga@chromium.org>
>> ---
>> drivers/media/platform/exynos-gsc/gsc-m2m.c | 4 ++--
>> drivers/media/platform/exynos4-is/fimc-capture.c | 2 +-
>> drivers/media/platform/exynos4-is/fimc-isp-video.c | 2 +-
>> drivers/media/platform/exynos4-is/fimc-lite.c | 2 +-
>> drivers/media/platform/exynos4-is/fimc-m2m.c | 4 ++--
>> drivers/media/platform/s3c-camif/camif-capture.c | 2 +-
>> drivers/media/platform/s5p-g2d/g2d.c | 4 ++--
>> drivers/media/platform/s5p-jpeg/jpeg-core.c | 4 ++--
>> drivers/media/platform/s5p-mfc/s5p_mfc.c | 6 ++----
>> 9 files changed, 14 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/media/platform/exynos-gsc/gsc-m2m.c b/drivers/media/platform/exynos-gsc/gsc-m2m.c
>> index 35a1d0d6dd66..f4b192e49c80 100644
>> --- a/drivers/media/platform/exynos-gsc/gsc-m2m.c
>> +++ b/drivers/media/platform/exynos-gsc/gsc-m2m.c
>> @@ -583,7 +583,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
>>
>> memset(src_vq, 0, sizeof(*src_vq));
>> src_vq->type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
>> - src_vq->io_modes = VB2_MMAP | VB2_USERPTR | VB2_DMABUF;
>> + src_vq->io_modes = VB2_MMAP | VB2_DMABUF;
>> src_vq->drv_priv = ctx;
>> src_vq->ops = &gsc_m2m_qops;
>> src_vq->mem_ops = &vb2_dma_contig_memops;
>> @@ -598,7 +598,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
>>
>> memset(dst_vq, 0, sizeof(*dst_vq));
>> dst_vq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
>> - dst_vq->io_modes = VB2_MMAP | VB2_USERPTR | VB2_DMABUF;
>> + dst_vq->io_modes = VB2_MMAP | VB2_DMABUF;
>> dst_vq->drv_priv = ctx;
>> dst_vq->ops = &gsc_m2m_qops;
>> dst_vq->mem_ops = &vb2_dma_contig_memops;
>> diff --git a/drivers/media/platform/exynos4-is/fimc-capture.c b/drivers/media/platform/exynos4-is/fimc-capture.c
>> index 121d609ff856..8d14207b3403 100644
>> --- a/drivers/media/platform/exynos4-is/fimc-capture.c
>> +++ b/drivers/media/platform/exynos4-is/fimc-capture.c
>> @@ -1771,7 +1771,7 @@ static int fimc_register_capture_device(struct fimc_dev *fimc,
>>
>> memset(q, 0, sizeof(*q));
>> q->type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
>> - q->io_modes = VB2_MMAP | VB2_USERPTR | VB2_DMABUF;
>> + q->io_modes = VB2_MMAP | VB2_DMABUF;
>> q->drv_priv = ctx;
>> q->ops = &fimc_capture_qops;
>> q->mem_ops = &vb2_dma_contig_memops;
>> diff --git a/drivers/media/platform/exynos4-is/fimc-isp-video.c b/drivers/media/platform/exynos4-is/fimc-isp-video.c
>> index 55bae20eb8db..94f3215916f6 100644
>> --- a/drivers/media/platform/exynos4-is/fimc-isp-video.c
>> +++ b/drivers/media/platform/exynos4-is/fimc-isp-video.c
>> @@ -587,7 +587,7 @@ int fimc_isp_video_device_register(struct fimc_isp *isp,
>>
>> memset(q, 0, sizeof(*q));
>> q->type = type;
>> - q->io_modes = VB2_MMAP | VB2_DMABUF | VB2_USERPTR;
>> + q->io_modes = VB2_MMAP | VB2_DMABUF;
>> q->ops = &isp_video_capture_qops;
>> q->mem_ops = &vb2_dma_contig_memops;
>> q->buf_struct_size = sizeof(struct isp_video_buf);
>> diff --git a/drivers/media/platform/exynos4-is/fimc-lite.c b/drivers/media/platform/exynos4-is/fimc-lite.c
>> index d06bf4865b84..3c2c70b252bb 100644
>> --- a/drivers/media/platform/exynos4-is/fimc-lite.c
>> +++ b/drivers/media/platform/exynos4-is/fimc-lite.c
>> @@ -1276,7 +1276,7 @@ static int fimc_lite_subdev_registered(struct v4l2_subdev *sd)
>>
>> memset(q, 0, sizeof(*q));
>> q->type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
>> - q->io_modes = VB2_MMAP | VB2_DMABUF | VB2_USERPTR;
>> + q->io_modes = VB2_MMAP | VB2_DMABUF;
>> q->ops = &fimc_lite_qops;
>> q->mem_ops = &vb2_dma_contig_memops;
>> q->buf_struct_size = sizeof(struct flite_buffer);
>> diff --git a/drivers/media/platform/exynos4-is/fimc-m2m.c b/drivers/media/platform/exynos4-is/fimc-m2m.c
>> index c70c2cbe3eb1..3323563ed913 100644
>> --- a/drivers/media/platform/exynos4-is/fimc-m2m.c
>> +++ b/drivers/media/platform/exynos4-is/fimc-m2m.c
>> @@ -554,7 +554,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
>> int ret;
>>
>> src_vq->type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
>> - src_vq->io_modes = VB2_MMAP | VB2_USERPTR | VB2_DMABUF;
>> + src_vq->io_modes = VB2_MMAP | VB2_DMABUF;
>> src_vq->drv_priv = ctx;
>> src_vq->ops = &fimc_qops;
>> src_vq->mem_ops = &vb2_dma_contig_memops;
>> @@ -568,7 +568,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
>> return ret;
>>
>> dst_vq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
>> - dst_vq->io_modes = VB2_MMAP | VB2_USERPTR | VB2_DMABUF;
>> + dst_vq->io_modes = VB2_MMAP | VB2_DMABUF;
>> dst_vq->drv_priv = ctx;
>> dst_vq->ops = &fimc_qops;
>> dst_vq->mem_ops = &vb2_dma_contig_memops;
>> diff --git a/drivers/media/platform/s3c-camif/camif-capture.c b/drivers/media/platform/s3c-camif/camif-capture.c
>> index 54989dacaf5d..eb99468a5427 100644
>> --- a/drivers/media/platform/s3c-camif/camif-capture.c
>> +++ b/drivers/media/platform/s3c-camif/camif-capture.c
>> @@ -1121,7 +1121,7 @@ int s3c_camif_register_video_node(struct camif_dev *camif, int idx)
>>
>> memset(q, 0, sizeof(*q));
>> q->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
>> - q->io_modes = VB2_MMAP | VB2_DMABUF | VB2_USERPTR;
>> + q->io_modes = VB2_MMAP | VB2_DMABUF;
>> q->ops = &s3c_camif_qops;
>> q->mem_ops = &vb2_dma_contig_memops;
>> q->buf_struct_size = sizeof(struct camif_buffer);
>> diff --git a/drivers/media/platform/s5p-g2d/g2d.c b/drivers/media/platform/s5p-g2d/g2d.c
>> index 98f94e1fa6b8..a8f8c9e00452 100644
>> --- a/drivers/media/platform/s5p-g2d/g2d.c
>> +++ b/drivers/media/platform/s5p-g2d/g2d.c
>> @@ -144,7 +144,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
>> int ret;
>>
>> src_vq->type = V4L2_BUF_TYPE_VIDEO_OUTPUT;
>> - src_vq->io_modes = VB2_MMAP | VB2_DMABUF | VB2_USERPTR;
>> + src_vq->io_modes = VB2_MMAP | VB2_DMABUF;
>> src_vq->drv_priv = ctx;
>> src_vq->ops = &g2d_qops;
>> src_vq->mem_ops = &vb2_dma_contig_memops;
>> @@ -158,7 +158,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
>> return ret;
>>
>> dst_vq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
>> - dst_vq->io_modes = VB2_MMAP | VB2_DMABUF | VB2_USERPTR;
>> + dst_vq->io_modes = VB2_MMAP | VB2_DMABUF;
>> dst_vq->drv_priv = ctx;
>> dst_vq->ops = &g2d_qops;
>> dst_vq->mem_ops = &vb2_dma_contig_memops;
>> diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c b/drivers/media/platform/s5p-jpeg/jpeg-core.c
>> index 4c10ec0d7da4..d03164854576 100644
>> --- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
>> +++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
>> @@ -2620,7 +2620,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
>> int ret;
>>
>> src_vq->type = V4L2_BUF_TYPE_VIDEO_OUTPUT;
>> - src_vq->io_modes = VB2_MMAP | VB2_DMABUF | VB2_USERPTR;
>> + src_vq->io_modes = VB2_MMAP | VB2_DMABUF;
>> src_vq->drv_priv = ctx;
>> src_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
>> src_vq->ops = &s5p_jpeg_qops;
>> @@ -2634,7 +2634,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
>> return ret;
>>
>> dst_vq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
>> - dst_vq->io_modes = VB2_MMAP | VB2_DMABUF | VB2_USERPTR;
>> + dst_vq->io_modes = VB2_MMAP | VB2_DMABUF;
>> dst_vq->drv_priv = ctx;
>> dst_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
>> dst_vq->ops = &s5p_jpeg_qops;
>> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c b/drivers/media/platform/s5p-mfc/s5p_mfc.c
>> index ff770328f690..32df5e26daab 100644
>> --- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
>> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
>> @@ -844,11 +844,10 @@ 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->io_modes = VB2_MMAP | VB2_DMABUF;
>> if (vdev == dev->vfd_dec) {
>> - q->io_modes = VB2_MMAP | VB2_DMABUF;
>> q->ops = get_dec_queue_ops();
>> } else if (vdev == dev->vfd_enc) {
>> - q->io_modes = VB2_MMAP | VB2_DMABUF | VB2_USERPTR;
>> q->ops = get_enc_queue_ops();
>> } else {
>> ret = -ENOENT;
>> @@ -871,11 +870,10 @@ static int s5p_mfc_open(struct file *file)
>> q->type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
>> q->drv_priv = &ctx->fh;
>> q->lock = &dev->mfc_mutex;
>> + q->io_modes = VB2_MMAP | VB2_DMABUF;
>> if (vdev == dev->vfd_dec) {
>> - q->io_modes = VB2_MMAP | VB2_DMABUF;
>> q->ops = get_dec_queue_ops();
>> } else if (vdev == dev->vfd_enc) {
>> - q->io_modes = VB2_MMAP | VB2_DMABUF | VB2_USERPTR;
>> q->ops = get_enc_queue_ops();
>> } else {
>> ret = -ENOENT;
>> --
>> 2.25.0
>>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH 8/9] exynos/s3c/s5p: drop VB2_USERPTR
2020-02-21 11:54 ` Hans Verkuil
@ 2020-02-21 13:11 ` Marek Szyprowski
2020-02-21 13:34 ` Tomasz Figa
1 sibling, 0 replies; 19+ messages in thread
From: Marek Szyprowski @ 2020-02-21 13:11 UTC (permalink / raw)
To: Hans Verkuil, Tomasz Figa; +Cc: Linux Media Mailing List, Sylwester Nawrocki
Hi Hans,
On 21.02.2020 12:54, Hans Verkuil wrote:
> On 2/21/20 9:53 AM, Tomasz Figa wrote:
>> Hi Hans,
>>
>> On Fri, Feb 21, 2020 at 5:46 PM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
>>> The combination of VB2_USERPTR and dma-contig makes no sense for
>>> these devices, drop it.
>> Even though I personally don't like user pointers, I believe at least
>> some of those devices are fine with USERPTR in case they are behind an
>> IOMMU, like on the newer Exynos SoCs. +Marek Szyprowski too.
> I would like this to be tested. I always wonder if that has actually
> been tested, especially with regards to the partial first and last pages of
> the malloc()ed memory. I.e., worst case only 8 bytes may have to be written
> to a page if malloc() aligned the pointer poorly. Can the DMA handle that,
> even with an IOMMU?
Frankly, we never used USERPTR to access malloc()'ed memory, although it
was possible with IOMMU and I remember I tested such case. USERPTR mode
was mainly used to access buffers allocated and mmaped from different
devices. In such case the alignment was already correct. Yes, this can
be replaced with DMABUF, but that required a lots of changes in
userspace libs/apps and using USERPTR was much simpler.
Afair the video devices had very capable DMA and they were able to
transfer data to 8-byte aligned buffers. There was however a problem
with CPU cache line size - the cache can be reliably managed only
down-to cache line-size units, what means that some freshly modified by
the CPU data before and after the buffer might be trashed if it was not
aligned to CPU cache line size.
I won't cry much after that hack...
> Note that I have the same concern for VB2_USERPTR with dma-sg.
>
> This was a good opportunity to improve v4l2-compliance: it adds sentinels at
> the start/end of the buffer, and it checks that those sentinels are never
> overwritten. So if this test passes for a driver, then VB2_USERPTR can stay
> in, but it should probably have a comment that it has been tested with
> v4l2-compliance.
>
>> What makes you believe it makes no sense for them?
> Serious doubts that this has been properly tested :-)
> You really need a test like I wrote today for v4l2-compliance
> in order to be certain that it works.
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH 8/9] exynos/s3c/s5p: drop VB2_USERPTR
2020-02-21 11:54 ` Hans Verkuil
2020-02-21 13:11 ` Marek Szyprowski
@ 2020-02-21 13:34 ` Tomasz Figa
1 sibling, 0 replies; 19+ messages in thread
From: Tomasz Figa @ 2020-02-21 13:34 UTC (permalink / raw)
To: Hans Verkuil
Cc: Marek Szyprowski, Linux Media Mailing List, Sylwester Nawrocki
On Fri, Feb 21, 2020 at 8:54 PM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
>
> On 2/21/20 9:53 AM, Tomasz Figa wrote:
> > Hi Hans,
> >
> > On Fri, Feb 21, 2020 at 5:46 PM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
> >>
> >> The combination of VB2_USERPTR and dma-contig makes no sense for
> >> these devices, drop it.
> >
> > Even though I personally don't like user pointers, I believe at least
> > some of those devices are fine with USERPTR in case they are behind an
> > IOMMU, like on the newer Exynos SoCs. +Marek Szyprowski too.
>
> I would like this to be tested. I always wonder if that has actually
> been tested, especially with regards to the partial first and last pages of
> the malloc()ed memory. I.e., worst case only 8 bytes may have to be written
> to a page if malloc() aligned the pointer poorly. Can the DMA handle that,
> even with an IOMMU?
FWIW, we've been using USERPTR for encoder input and output in
Chromium. Input is now finally being transitioned to DMABUF. I think
this is basically a contract between the driver and the userspace that
it guarantees not touching the memory outside of the buffer.
That said, hardware that needs bigger DMA transfer alignment than
cache line size must not report USERPTR. s5p-mfc doesn't seem to count
as such, though.
>
> Note that I have the same concern for VB2_USERPTR with dma-sg.
>
> This was a good opportunity to improve v4l2-compliance: it adds sentinels at
> the start/end of the buffer, and it checks that those sentinels are never
> overwritten. So if this test passes for a driver, then VB2_USERPTR can stay
> in, but it should probably have a comment that it has been tested with
> v4l2-compliance.
>
> >
> > What makes you believe it makes no sense for them?
>
> Serious doubts that this has been properly tested :-)
> You really need a test like I wrote today for v4l2-compliance
> in order to be certain that it works.
I think we would have to drop some drivers completely if we were to
judge them on the same basis. ;)
>
> Regards,
>
> Hans
>
> >
> > Best regards,
> > Tomasz
> >
> >>
> >> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> >> Cc: Sylwester Nawrocki <snawrocki@kernel.org>
> >> Cc: Tomasz Figa <tfiga@chromium.org>
> >> ---
> >> drivers/media/platform/exynos-gsc/gsc-m2m.c | 4 ++--
> >> drivers/media/platform/exynos4-is/fimc-capture.c | 2 +-
> >> drivers/media/platform/exynos4-is/fimc-isp-video.c | 2 +-
> >> drivers/media/platform/exynos4-is/fimc-lite.c | 2 +-
> >> drivers/media/platform/exynos4-is/fimc-m2m.c | 4 ++--
> >> drivers/media/platform/s3c-camif/camif-capture.c | 2 +-
> >> drivers/media/platform/s5p-g2d/g2d.c | 4 ++--
> >> drivers/media/platform/s5p-jpeg/jpeg-core.c | 4 ++--
> >> drivers/media/platform/s5p-mfc/s5p_mfc.c | 6 ++----
> >> 9 files changed, 14 insertions(+), 16 deletions(-)
> >>
> >> diff --git a/drivers/media/platform/exynos-gsc/gsc-m2m.c b/drivers/media/platform/exynos-gsc/gsc-m2m.c
> >> index 35a1d0d6dd66..f4b192e49c80 100644
> >> --- a/drivers/media/platform/exynos-gsc/gsc-m2m.c
> >> +++ b/drivers/media/platform/exynos-gsc/gsc-m2m.c
> >> @@ -583,7 +583,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
> >>
> >> memset(src_vq, 0, sizeof(*src_vq));
> >> src_vq->type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
> >> - src_vq->io_modes = VB2_MMAP | VB2_USERPTR | VB2_DMABUF;
> >> + src_vq->io_modes = VB2_MMAP | VB2_DMABUF;
> >> src_vq->drv_priv = ctx;
> >> src_vq->ops = &gsc_m2m_qops;
> >> src_vq->mem_ops = &vb2_dma_contig_memops;
> >> @@ -598,7 +598,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
> >>
> >> memset(dst_vq, 0, sizeof(*dst_vq));
> >> dst_vq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
> >> - dst_vq->io_modes = VB2_MMAP | VB2_USERPTR | VB2_DMABUF;
> >> + dst_vq->io_modes = VB2_MMAP | VB2_DMABUF;
> >> dst_vq->drv_priv = ctx;
> >> dst_vq->ops = &gsc_m2m_qops;
> >> dst_vq->mem_ops = &vb2_dma_contig_memops;
> >> diff --git a/drivers/media/platform/exynos4-is/fimc-capture.c b/drivers/media/platform/exynos4-is/fimc-capture.c
> >> index 121d609ff856..8d14207b3403 100644
> >> --- a/drivers/media/platform/exynos4-is/fimc-capture.c
> >> +++ b/drivers/media/platform/exynos4-is/fimc-capture.c
> >> @@ -1771,7 +1771,7 @@ static int fimc_register_capture_device(struct fimc_dev *fimc,
> >>
> >> memset(q, 0, sizeof(*q));
> >> q->type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
> >> - q->io_modes = VB2_MMAP | VB2_USERPTR | VB2_DMABUF;
> >> + q->io_modes = VB2_MMAP | VB2_DMABUF;
> >> q->drv_priv = ctx;
> >> q->ops = &fimc_capture_qops;
> >> q->mem_ops = &vb2_dma_contig_memops;
> >> diff --git a/drivers/media/platform/exynos4-is/fimc-isp-video.c b/drivers/media/platform/exynos4-is/fimc-isp-video.c
> >> index 55bae20eb8db..94f3215916f6 100644
> >> --- a/drivers/media/platform/exynos4-is/fimc-isp-video.c
> >> +++ b/drivers/media/platform/exynos4-is/fimc-isp-video.c
> >> @@ -587,7 +587,7 @@ int fimc_isp_video_device_register(struct fimc_isp *isp,
> >>
> >> memset(q, 0, sizeof(*q));
> >> q->type = type;
> >> - q->io_modes = VB2_MMAP | VB2_DMABUF | VB2_USERPTR;
> >> + q->io_modes = VB2_MMAP | VB2_DMABUF;
> >> q->ops = &isp_video_capture_qops;
> >> q->mem_ops = &vb2_dma_contig_memops;
> >> q->buf_struct_size = sizeof(struct isp_video_buf);
> >> diff --git a/drivers/media/platform/exynos4-is/fimc-lite.c b/drivers/media/platform/exynos4-is/fimc-lite.c
> >> index d06bf4865b84..3c2c70b252bb 100644
> >> --- a/drivers/media/platform/exynos4-is/fimc-lite.c
> >> +++ b/drivers/media/platform/exynos4-is/fimc-lite.c
> >> @@ -1276,7 +1276,7 @@ static int fimc_lite_subdev_registered(struct v4l2_subdev *sd)
> >>
> >> memset(q, 0, sizeof(*q));
> >> q->type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
> >> - q->io_modes = VB2_MMAP | VB2_DMABUF | VB2_USERPTR;
> >> + q->io_modes = VB2_MMAP | VB2_DMABUF;
> >> q->ops = &fimc_lite_qops;
> >> q->mem_ops = &vb2_dma_contig_memops;
> >> q->buf_struct_size = sizeof(struct flite_buffer);
> >> diff --git a/drivers/media/platform/exynos4-is/fimc-m2m.c b/drivers/media/platform/exynos4-is/fimc-m2m.c
> >> index c70c2cbe3eb1..3323563ed913 100644
> >> --- a/drivers/media/platform/exynos4-is/fimc-m2m.c
> >> +++ b/drivers/media/platform/exynos4-is/fimc-m2m.c
> >> @@ -554,7 +554,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
> >> int ret;
> >>
> >> src_vq->type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
> >> - src_vq->io_modes = VB2_MMAP | VB2_USERPTR | VB2_DMABUF;
> >> + src_vq->io_modes = VB2_MMAP | VB2_DMABUF;
> >> src_vq->drv_priv = ctx;
> >> src_vq->ops = &fimc_qops;
> >> src_vq->mem_ops = &vb2_dma_contig_memops;
> >> @@ -568,7 +568,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
> >> return ret;
> >>
> >> dst_vq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
> >> - dst_vq->io_modes = VB2_MMAP | VB2_USERPTR | VB2_DMABUF;
> >> + dst_vq->io_modes = VB2_MMAP | VB2_DMABUF;
> >> dst_vq->drv_priv = ctx;
> >> dst_vq->ops = &fimc_qops;
> >> dst_vq->mem_ops = &vb2_dma_contig_memops;
> >> diff --git a/drivers/media/platform/s3c-camif/camif-capture.c b/drivers/media/platform/s3c-camif/camif-capture.c
> >> index 54989dacaf5d..eb99468a5427 100644
> >> --- a/drivers/media/platform/s3c-camif/camif-capture.c
> >> +++ b/drivers/media/platform/s3c-camif/camif-capture.c
> >> @@ -1121,7 +1121,7 @@ int s3c_camif_register_video_node(struct camif_dev *camif, int idx)
> >>
> >> memset(q, 0, sizeof(*q));
> >> q->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
> >> - q->io_modes = VB2_MMAP | VB2_DMABUF | VB2_USERPTR;
> >> + q->io_modes = VB2_MMAP | VB2_DMABUF;
> >> q->ops = &s3c_camif_qops;
> >> q->mem_ops = &vb2_dma_contig_memops;
> >> q->buf_struct_size = sizeof(struct camif_buffer);
> >> diff --git a/drivers/media/platform/s5p-g2d/g2d.c b/drivers/media/platform/s5p-g2d/g2d.c
> >> index 98f94e1fa6b8..a8f8c9e00452 100644
> >> --- a/drivers/media/platform/s5p-g2d/g2d.c
> >> +++ b/drivers/media/platform/s5p-g2d/g2d.c
> >> @@ -144,7 +144,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
> >> int ret;
> >>
> >> src_vq->type = V4L2_BUF_TYPE_VIDEO_OUTPUT;
> >> - src_vq->io_modes = VB2_MMAP | VB2_DMABUF | VB2_USERPTR;
> >> + src_vq->io_modes = VB2_MMAP | VB2_DMABUF;
> >> src_vq->drv_priv = ctx;
> >> src_vq->ops = &g2d_qops;
> >> src_vq->mem_ops = &vb2_dma_contig_memops;
> >> @@ -158,7 +158,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
> >> return ret;
> >>
> >> dst_vq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
> >> - dst_vq->io_modes = VB2_MMAP | VB2_DMABUF | VB2_USERPTR;
> >> + dst_vq->io_modes = VB2_MMAP | VB2_DMABUF;
> >> dst_vq->drv_priv = ctx;
> >> dst_vq->ops = &g2d_qops;
> >> dst_vq->mem_ops = &vb2_dma_contig_memops;
> >> diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c b/drivers/media/platform/s5p-jpeg/jpeg-core.c
> >> index 4c10ec0d7da4..d03164854576 100644
> >> --- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
> >> +++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
> >> @@ -2620,7 +2620,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
> >> int ret;
> >>
> >> src_vq->type = V4L2_BUF_TYPE_VIDEO_OUTPUT;
> >> - src_vq->io_modes = VB2_MMAP | VB2_DMABUF | VB2_USERPTR;
> >> + src_vq->io_modes = VB2_MMAP | VB2_DMABUF;
> >> src_vq->drv_priv = ctx;
> >> src_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
> >> src_vq->ops = &s5p_jpeg_qops;
> >> @@ -2634,7 +2634,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
> >> return ret;
> >>
> >> dst_vq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
> >> - dst_vq->io_modes = VB2_MMAP | VB2_DMABUF | VB2_USERPTR;
> >> + dst_vq->io_modes = VB2_MMAP | VB2_DMABUF;
> >> dst_vq->drv_priv = ctx;
> >> dst_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
> >> dst_vq->ops = &s5p_jpeg_qops;
> >> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c b/drivers/media/platform/s5p-mfc/s5p_mfc.c
> >> index ff770328f690..32df5e26daab 100644
> >> --- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
> >> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
> >> @@ -844,11 +844,10 @@ 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->io_modes = VB2_MMAP | VB2_DMABUF;
> >> if (vdev == dev->vfd_dec) {
> >> - q->io_modes = VB2_MMAP | VB2_DMABUF;
> >> q->ops = get_dec_queue_ops();
> >> } else if (vdev == dev->vfd_enc) {
> >> - q->io_modes = VB2_MMAP | VB2_DMABUF | VB2_USERPTR;
> >> q->ops = get_enc_queue_ops();
> >> } else {
> >> ret = -ENOENT;
> >> @@ -871,11 +870,10 @@ static int s5p_mfc_open(struct file *file)
> >> q->type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
> >> q->drv_priv = &ctx->fh;
> >> q->lock = &dev->mfc_mutex;
> >> + q->io_modes = VB2_MMAP | VB2_DMABUF;
> >> if (vdev == dev->vfd_dec) {
> >> - q->io_modes = VB2_MMAP | VB2_DMABUF;
> >> q->ops = get_dec_queue_ops();
> >> } else if (vdev == dev->vfd_enc) {
> >> - q->io_modes = VB2_MMAP | VB2_DMABUF | VB2_USERPTR;
> >> q->ops = get_enc_queue_ops();
> >> } else {
> >> ret = -ENOENT;
> >> --
> >> 2.25.0
> >>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH 9/9] omap3isp/rcar_fdp1/vsp1/xilinx: drop VB2_USERPTR
2020-02-21 9:01 ` Kieran Bingham
@ 2020-02-21 14:31 ` Laurent Pinchart
2020-02-21 19:46 ` Sakari Ailus
0 siblings, 1 reply; 19+ messages in thread
From: Laurent Pinchart @ 2020-02-21 14:31 UTC (permalink / raw)
To: Kieran Bingham; +Cc: Hans Verkuil, linux-media, Sakari Ailus
Hi Hans,
CC'ing Sakari for the omap3isp part.
On Fri, Feb 21, 2020 at 09:01:02AM +0000, Kieran Bingham wrote:
> On 21/02/2020 08:45, Hans Verkuil wrote:
> > The combination of VB2_USERPTR and dma-contig makes no sense for
> > these devices, drop it.
> >
> > Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Cc: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
>
> Agreed, the FDP1 and VSP1 expect contiguous physical memory, so a
> user-pointer doesn't make much sense to be permitted.
>
> I haven't lokoed at the Xilinx platform in regards to this but I would
> be surprised if it wasn't the same issue there of course.
>
> For VSP1/FDP1:
>
> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
for the same.
For the Xilinx driver, you may want to ask Xilinx if they use USERPTR.
> > ---
> > drivers/media/platform/omap3isp/ispvideo.c | 2 +-
> > drivers/media/platform/rcar_fdp1.c | 4 ++--
> > drivers/media/platform/vsp1/vsp1_video.c | 2 +-
> > drivers/media/platform/xilinx/xilinx-dma.c | 2 +-
> > 4 files changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/media/platform/omap3isp/ispvideo.c b/drivers/media/platform/omap3isp/ispvideo.c
> > index e8c46ff1aeb4..1104654ba438 100644
> > --- a/drivers/media/platform/omap3isp/ispvideo.c
> > +++ b/drivers/media/platform/omap3isp/ispvideo.c
> > @@ -1319,7 +1319,7 @@ static int isp_video_open(struct file *file)
> >
> > queue = &handle->queue;
> > queue->type = video->type;
> > - queue->io_modes = VB2_MMAP | VB2_DMABUF | VB2_USERPTR;
> > + queue->io_modes = VB2_MMAP | VB2_DMABUF;
> > queue->drv_priv = handle;
> > queue->ops = &isp_video_queue_ops;
> > queue->mem_ops = &vb2_dma_contig_memops;
> > diff --git a/drivers/media/platform/rcar_fdp1.c b/drivers/media/platform/rcar_fdp1.c
> > index 97bed45360f0..df081f66575f 100644
> > --- a/drivers/media/platform/rcar_fdp1.c
> > +++ b/drivers/media/platform/rcar_fdp1.c
> > @@ -2047,7 +2047,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
> > int ret;
> >
> > src_vq->type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
> > - src_vq->io_modes = VB2_MMAP | VB2_USERPTR | VB2_DMABUF;
> > + src_vq->io_modes = VB2_MMAP | VB2_DMABUF;
> > src_vq->drv_priv = ctx;
> > src_vq->buf_struct_size = sizeof(struct fdp1_buffer);
> > src_vq->ops = &fdp1_qops;
> > @@ -2061,7 +2061,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
> > return ret;
> >
> > dst_vq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
> > - dst_vq->io_modes = VB2_MMAP | VB2_USERPTR | VB2_DMABUF;
> > + dst_vq->io_modes = VB2_MMAP | VB2_DMABUF;
> > dst_vq->drv_priv = ctx;
> > dst_vq->buf_struct_size = sizeof(struct fdp1_buffer);
> > dst_vq->ops = &fdp1_qops;
> > diff --git a/drivers/media/platform/vsp1/vsp1_video.c b/drivers/media/platform/vsp1/vsp1_video.c
> > index 5e59ed2c3614..112e2092f6d3 100644
> > --- a/drivers/media/platform/vsp1/vsp1_video.c
> > +++ b/drivers/media/platform/vsp1/vsp1_video.c
> > @@ -1300,7 +1300,7 @@ struct vsp1_video *vsp1_video_create(struct vsp1_device *vsp1,
> > video_set_drvdata(&video->video, video);
> >
> > video->queue.type = video->type;
> > - video->queue.io_modes = VB2_MMAP | VB2_USERPTR | VB2_DMABUF;
> > + video->queue.io_modes = VB2_MMAP | VB2_DMABUF;
> > video->queue.lock = &video->lock;
> > video->queue.drv_priv = video;
> > video->queue.buf_struct_size = sizeof(struct vsp1_vb2_buffer);
> > diff --git a/drivers/media/platform/xilinx/xilinx-dma.c b/drivers/media/platform/xilinx/xilinx-dma.c
> > index b211380a11f2..57e52ad720dd 100644
> > --- a/drivers/media/platform/xilinx/xilinx-dma.c
> > +++ b/drivers/media/platform/xilinx/xilinx-dma.c
> > @@ -708,7 +708,7 @@ int xvip_dma_init(struct xvip_composite_device *xdev, struct xvip_dma *dma,
> > * instead of 'cat' isn't really a drawback.
> > */
> > dma->queue.type = type;
> > - dma->queue.io_modes = VB2_MMAP | VB2_USERPTR | VB2_DMABUF;
> > + dma->queue.io_modes = VB2_MMAP | VB2_DMABUF;
> > dma->queue.lock = &dma->lock;
> > dma->queue.drv_priv = dma;
> > dma->queue.buf_struct_size = sizeof(struct xvip_dma_buffer);
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH 9/9] omap3isp/rcar_fdp1/vsp1/xilinx: drop VB2_USERPTR
2020-02-21 14:31 ` Laurent Pinchart
@ 2020-02-21 19:46 ` Sakari Ailus
2020-02-22 10:43 ` Laurent Pinchart
0 siblings, 1 reply; 19+ messages in thread
From: Sakari Ailus @ 2020-02-21 19:46 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: Kieran Bingham, Hans Verkuil, linux-media
Hi Hans, Laurent,
On Fri, Feb 21, 2020 at 04:31:01PM +0200, Laurent Pinchart wrote:
> Hi Hans,
>
> CC'ing Sakari for the omap3isp part.
Thanks.
The omap3isp is behind an IOMMU, so the USERPTR memory does not need to be
physically contiguous. I don't see a reason to drop userptr support from
the driver.
--
Kind regards,
Sakari Ailus
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH 9/9] omap3isp/rcar_fdp1/vsp1/xilinx: drop VB2_USERPTR
2020-02-21 19:46 ` Sakari Ailus
@ 2020-02-22 10:43 ` Laurent Pinchart
2020-02-22 13:13 ` Sakari Ailus
0 siblings, 1 reply; 19+ messages in thread
From: Laurent Pinchart @ 2020-02-22 10:43 UTC (permalink / raw)
To: Sakari Ailus; +Cc: Kieran Bingham, Hans Verkuil, linux-media
Hi Sakari,
On Fri, Feb 21, 2020 at 09:46:41PM +0200, Sakari Ailus wrote:
> Hi Hans, Laurent,
>
> On Fri, Feb 21, 2020 at 04:31:01PM +0200, Laurent Pinchart wrote:
> > Hi Hans,
> >
> > CC'ing Sakari for the omap3isp part.
>
> Thanks.
>
> The omap3isp is behind an IOMMU, so the USERPTR memory does not need to be
> physically contiguous. I don't see a reason to drop userptr support from
> the driver.
Apart from the fact that this API should be discouraged :-) I wonder if
it's used, that's my real question. As we can't rule it out, I'd be
cautious about dropping it.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH 9/9] omap3isp/rcar_fdp1/vsp1/xilinx: drop VB2_USERPTR
2020-02-22 10:43 ` Laurent Pinchart
@ 2020-02-22 13:13 ` Sakari Ailus
0 siblings, 0 replies; 19+ messages in thread
From: Sakari Ailus @ 2020-02-22 13:13 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: Kieran Bingham, Hans Verkuil, linux-media
Hi Laurent,
On Sat, Feb 22, 2020 at 12:43:48PM +0200, Laurent Pinchart wrote:
> Hi Sakari,
>
> On Fri, Feb 21, 2020 at 09:46:41PM +0200, Sakari Ailus wrote:
> > Hi Hans, Laurent,
> >
> > On Fri, Feb 21, 2020 at 04:31:01PM +0200, Laurent Pinchart wrote:
> > > Hi Hans,
> > >
> > > CC'ing Sakari for the omap3isp part.
> >
> > Thanks.
> >
> > The omap3isp is behind an IOMMU, so the USERPTR memory does not need to be
> > physically contiguous. I don't see a reason to drop userptr support from
> > the driver.
>
> Apart from the fact that this API should be discouraged :-) I wonder if
> it's used, that's my real question. As we can't rule it out, I'd be
> cautious about dropping it.
Indeed. If we'd drop USERPTR, it should apply to the uAPI, the framework
and all the drivers, not an individual driver. But again that would be
certain to break a lot of applications...
I guess we could document USERPTR as deprecated, and not recommended for
new applications?
--
Regards,
Sakari Ailus
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2020-02-22 13:14 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-21 8:45 [RFC PATCH 0/9] Drop VB2_USERPTR + dma-contig combo Hans Verkuil
2020-02-21 8:45 ` [RFC PATCH 1/9] mtk-vcodec: drop VB2_USERPTR Hans Verkuil
2020-02-21 8:45 ` [RFC PATCH 2/9] solo6x10: " Hans Verkuil
2020-02-21 8:45 ` [RFC PATCH 3/9] m2m-deinterlace: " Hans Verkuil
2020-02-21 8:45 ` [RFC PATCH 4/9] mcam-core: " Hans Verkuil
2020-02-21 8:45 ` [RFC PATCH 5/9] sh_veu: " Hans Verkuil
2020-02-21 8:45 ` [RFC PATCH 6/9] mx2_emmaprp: " Hans Verkuil
2020-02-21 8:45 ` [RFC PATCH 7/9] davinci: " Hans Verkuil
2020-02-21 8:45 ` [RFC PATCH 8/9] exynos/s3c/s5p: " Hans Verkuil
2020-02-21 8:53 ` Tomasz Figa
2020-02-21 11:54 ` Hans Verkuil
2020-02-21 13:11 ` Marek Szyprowski
2020-02-21 13:34 ` Tomasz Figa
2020-02-21 8:45 ` [RFC PATCH 9/9] omap3isp/rcar_fdp1/vsp1/xilinx: " Hans Verkuil
2020-02-21 9:01 ` Kieran Bingham
2020-02-21 14:31 ` Laurent Pinchart
2020-02-21 19:46 ` Sakari Ailus
2020-02-22 10:43 ` Laurent Pinchart
2020-02-22 13:13 ` Sakari Ailus
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).