[RFC,8/9] exynos/s3c/s5p: drop VB2_USERPTR

Message ID 20200221084531.576156-9-hverkuil-cisco@xs4all.nl (mailing list archive)
State TODO, archived
Delegated to: Hans Verkuil
Headers
Series Drop VB2_USERPTR + dma-contig combo |

Commit Message

Hans Verkuil Feb. 21, 2020, 8:45 a.m. UTC
  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(-)
  

Comments

Tomasz Figa Feb. 21, 2020, 8:53 a.m. UTC | #1
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
>
  
Hans Verkuil Feb. 21, 2020, 11:54 a.m. UTC | #2
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
>>
  
Marek Szyprowski Feb. 21, 2020, 1:11 p.m. UTC | #3
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
  
Tomasz Figa Feb. 21, 2020, 1:34 p.m. UTC | #4
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
> >>
>
  

Patch

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;