Re: [PATCH 1/4] media: qcom: camss: use contiguous buffers when there is no IOMMU

[Loic Poulain]

On Wed, Nov 4, 2026 at 9:23 PM Allan Siriani <allan@xxxxxxxxxxxxxxxxxxx> wrote:
>
> The VFE write masters are programmed with a single base address per plane
> and then write the whole plane from there. video_buf_init() takes that
> address from sg_dma_address() of the first scatterlist entry of a
> vb2_dma_sg buffer, which is only correct when the whole buffer is mapped
> as one contiguous range in the device address space.
>
> That holds when the CAMSS is behind an IOMMU, but not on SoCs where it is
> not, such as MSM8226: there vb2_dma_sg allocates scattered pages, the
> write master runs past the first segment and the frame lands in memory
> that belongs to someone else, which quickly brings the system down.
>
> When the CAMSS device is not IOMMU mapped, use vb2_dma_contig instead,
> which allocates physically contiguous (CMA) buffers and checks that
> imported USERPTR/DMABUF buffers are contiguous. Keep vb2_dma_sg when there
> is an IOMMU, but refuse a buffer whose DMA segments are not contiguous
> instead of handing a partial mapping to the hardware.
>
> Signed-off-by: Allan Siriani <allan@xxxxxxxxxxxxxxxxxxx>
> ---
>  drivers/media/platform/qcom/camss/Kconfig     |  1 +
>  .../media/platform/qcom/camss/camss-video.c   | 64 +++++++++++++++++--
>  2 files changed, 58 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/camss/Kconfig b/drivers/media/platform/qcom/camss/Kconfig
> index b51745c94..3d3555b55 100644
> --- a/drivers/media/platform/qcom/camss/Kconfig
> +++ b/drivers/media/platform/qcom/camss/Kconfig
> @@ -5,6 +5,7 @@ config VIDEO_QCOM_CAMSS
>         depends on (ARCH_QCOM && IOMMU_DMA) || COMPILE_TEST
>         select MEDIA_CONTROLLER
>         select VIDEO_V4L2_SUBDEV_API
> +       select VIDEOBUF2_DMA_CONTIG
>         select VIDEOBUF2_DMA_SG
>         select V4L2_FWNODE
>         select GENERIC_PHY
> diff --git a/drivers/media/platform/qcom/camss/camss-video.c b/drivers/media/platform/qcom/camss/camss-video.c
> index 0852eb6f1..2c15546c8 100644
> --- a/drivers/media/platform/qcom/camss/camss-video.c
> +++ b/drivers/media/platform/qcom/camss/camss-video.c
> @@ -7,12 +7,14 @@
>   * Copyright (c) 2013-2015, The Linux Foundation. All rights reserved.
>   * Copyright (C) 2015-2018 Linaro Ltd.
>   */
> +#include <linux/device.h>
>  #include <linux/slab.h>
>  #include <media/media-entity.h>
>  #include <media/v4l2-dev.h>
>  #include <media/v4l2-device.h>
>  #include <media/v4l2-ioctl.h>
>  #include <media/v4l2-mc.h>
> +#include <media/videobuf2-dma-contig.h>
>  #include <media/videobuf2-dma-sg.h>
>
>  #include "camss-video.h"
> @@ -146,6 +148,48 @@ static int video_queue_setup(struct vb2_queue *q,
>         return 0;
>  }
>
> +/*
> + * The VFE write masters take a single base address per plane and write the
> + * whole plane from there, so every plane must be contiguous in the device
> + * address space. Without an IOMMU the buffers come from vb2_dma_contig, which
> + * guarantees that. With an IOMMU the scatterlist mapping may still be split
> + * in several DMA segments (max_seg_size): accept it only when the segments
> + * follow each other in the IOVA space.
> + */
> +static int video_buf_plane_addr(struct camss_video *video,
> +                               struct vb2_buffer *vb, unsigned int plane,
> +                               dma_addr_t *addr)
> +{
> +       struct scatterlist *sg;
> +       struct sg_table *sgt;
> +       dma_addr_t next;
> +       unsigned int i;
> +
> +       if (video->vb2_q.mem_ops == &vb2_dma_contig_memops) {
> +               *addr = vb2_dma_contig_plane_dma_addr(vb, plane);
> +               return 0;
> +       }
> +
> +       sgt = vb2_dma_sg_plane_desc(vb, plane);
> +       if (!sgt || !sgt->nents)
> +               return -EFAULT;
> +
> +       next = sg_dma_address(sgt->sgl);
> +       for_each_sgtable_dma_sg(sgt, sg, i) {
> +               if (sg_dma_address(sg) != next) {
> +                       dev_err(video->camss->dev,
> +                               "Plane %u is not contiguous in DMA space\n",
> +                               plane);
> +                       return -EINVAL;
> +               }
> +               next += sg_dma_len(sg);
> +       }
> +
> +       *addr = sg_dma_address(sgt->sgl);
> +
> +       return 0;
> +}
> +
>  static int video_buf_init(struct vb2_buffer *vb)
>  {
>         struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
> @@ -154,15 +198,13 @@ static int video_buf_init(struct vb2_buffer *vb)
>                                                    vb);
>         const struct v4l2_pix_format_mplane *format =
>                                                 &video->active_fmt.fmt.pix_mp;
> -       struct sg_table *sgt;
>         unsigned int i;
> +       int ret;
>
>         for (i = 0; i < format->num_planes; i++) {
> -               sgt = vb2_dma_sg_plane_desc(vb, i);
> -               if (!sgt)
> -                       return -EFAULT;
> -
> -               buffer->addr[i] = sg_dma_address(sgt->sgl);
> +               ret = video_buf_plane_addr(video, vb, i, &buffer->addr[i]);
> +               if (ret < 0)
> +                       return ret;
>         }
>
>         if (format->pixelformat == V4L2_PIX_FMT_NV12 ||
> @@ -699,7 +741,15 @@ int msm_video_register(struct camss_video *video, struct v4l2_device *v4l2_dev,
>
>         q = &video->vb2_q;
>         q->drv_priv = video;
> -       q->mem_ops = &vb2_dma_sg_memops;
> +       /*
> +        * Without an IOMMU a scatter-gather buffer is physically scattered and
> +        * the write masters, which only take a base address, would run past
> +        * its first segment. Use physically contiguous (CMA) buffers instead.
> +        */
> +       if (device_iommu_mapped(video->camss->dev))
> +               q->mem_ops = &vb2_dma_sg_memops;
> +       else
> +               q->mem_ops = &vb2_dma_contig_memops;

So I think we should simply use vb2_dma_contig_memops in all cases. We
don't appear to support scatter-gather buffers properly anyway as we
only program a single address per plane and neither the driver nor the
hardware handles multiple DMA segments. We've likely been lucky so far
because the DMA mapping ended up as a single segment, or as contiguous
DMA segments...

>         q->ops = &msm_video_vb2_q_ops;
>         q->type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
>         q->io_modes = VB2_DMABUF | VB2_MMAP | VB2_READ;
> --
> 2.43.0
>