Re: [PATCH 2/3] media: camss: video: check that a buffer is DMA contiguous and add a prepare hook

[Loic Poulain]

On Thu, Oct 29, 2026 at 1:31 PM Qianhui Xu <xiaqinghuan@xxxxxxxxxxxxxxxx> wrote:
>
> The write master of the newer VFE generations can only be programmed with
> a buffer whose payload is physically contiguous, because the frame
> increment registers describe one linear address range.  Check that the DMA
> mapping of every plane is contiguous for at least sizeimage bytes and
> reject the buffer otherwise, instead of silently writing past the end of
> the first scatterlist entry.

Have you tested the non-contiguous case? It’s a bit surprising to me,
since the device-visible IOVA is still contiguous.

Perhaps this is related to frame subsampling, in which case the frame
buffers themselves would need to remain contiguous to take advantage
of the hardware frame-increment mechanism. But even then, the relevant
address space from the hardware's perspective should still be the
IOVA.

Regards,
Loic

>
> Also let the SoC code add its own alignment requirements to a queued
> buffer through a new prepare_buffer() video operation, which runs after
> the generic checks.
>
> Signed-off-by: Qianhui Xu <xiaqinghuan@xxxxxxxxxxxxxxxx>
> ---
>  .../media/platform/qcom/camss/camss-video.c    | 18 +++++++++++++++++-
>  .../media/platform/qcom/camss/camss-video.h    |  1 +
>  2 files changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/platform/qcom/camss/camss-video.c b/drivers/media/platform/qcom/camss/camss-video.c
> index 0852eb6f13..027cf024f9 100644
> --- a/drivers/media/platform/qcom/camss/camss-video.c
> +++ b/drivers/media/platform/qcom/camss/camss-video.c
> @@ -155,7 +155,10 @@ static int video_buf_init(struct vb2_buffer *vb)
>         const struct v4l2_pix_format_mplane *format =
>                                                 &video->active_fmt.fmt.pix_mp;
>         struct sg_table *sgt;
> -       unsigned int i;
> +       struct scatterlist *sg;
> +       dma_addr_t end;
> +       unsigned int i, j;
> +       size_t contiguous;
>
>         for (i = 0; i < format->num_planes; i++) {
>                 sgt = vb2_dma_sg_plane_desc(vb, i);
> @@ -163,6 +166,16 @@ static int video_buf_init(struct vb2_buffer *vb)
>                         return -EFAULT;
>
>                 buffer->addr[i] = sg_dma_address(sgt->sgl);
> +               end = buffer->addr[i];
> +               contiguous = 0;
> +               for_each_sgtable_dma_sg(sgt, sg, j) {
> +                       if (sg_dma_address(sg) != end)
> +                               break;
> +                       contiguous += sg_dma_len(sg);
> +                       end += sg_dma_len(sg);
> +               }
> +               if (contiguous < format->plane_fmt[i].sizeimage)
> +                       return -EINVAL;
>         }
>
>         if (format->pixelformat == V4L2_PIX_FMT_NV12 ||
> @@ -193,6 +206,9 @@ static int video_buf_prepare(struct vb2_buffer *vb)
>
>         vbuf->field = V4L2_FIELD_NONE;
>
> +       if (video->ops->prepare_buffer)
> +               return video->ops->prepare_buffer(video, vb);
> +
>         return 0;
>  }
>
> diff --git a/drivers/media/platform/qcom/camss/camss-video.h b/drivers/media/platform/qcom/camss/camss-video.h
> index d3e56e240a..af046ecb7f 100644
> --- a/drivers/media/platform/qcom/camss/camss-video.h
> +++ b/drivers/media/platform/qcom/camss/camss-video.h
> @@ -28,6 +28,7 @@ struct camss_buffer {
>  struct camss_video;
>
>  struct camss_video_ops {
> +       int (*prepare_buffer)(struct camss_video *vid, struct vb2_buffer *vb);
>         int (*queue_buffer)(struct camss_video *vid, struct camss_buffer *buf);
>         int (*flush_buffers)(struct camss_video *vid,
>                              enum vb2_buffer_state state);
>