Re: [PATCH 2/3] media: camss: video: check that a buffer is DMA contiguous and add a prepare hook
From: Bryan O'Donoghue
Date: Thu Oct 29 2026 - 10:12:07 EST
On 29/10/2026 12:31, Qianhui Xu wrote:
The write master of the newer VFE generations can only be programmed with
a buffer whose payload is physically contiguous
The first problem is that is _not_ what your code checks - you should use sg_phys() for that - which gets the physical address of a CPU scatterlist entry.
What your code currently does is check that the IOVA addresses are contiguous.
But what happens is an IOVA allocation is made which is linearly contiguous in the IOVA address space and then SMMU entries map the IOVA back to physical entries.
So all we need to do is give the IOVA base to CAMSS and it can write IOVA buffers - its the SMMU's job to ensure the resulting upstream writes land at the right physical address.
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.
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>
And I can prove it too.
buildroot# uname -a
Linux buildroot 7.3.0-rc1+ #19 SMP PREEMPT Thu Oct 29 13:26:34 GMT 2026 aarch64 GNU/Linux
buildroot# dmesg | grep -i crd
[ 0.000000] Machine model: Qualcomm Technologies, Inc. X1E80100 CRD
buildroot# dmesg | grep "plane 0"
qcom-camss acb7000.isp: plane 0 chunk 0: phys 0x000000089a800000 len 8388608
qcom-camss acb7000.isp: plane 0 chunk 1: phys 0x000000089a600000 len 2097152
qcom-camss acb7000.isp: plane 0 chunk 2: phys 0x0000000899900000 len 1048576
qcom-camss acb7000.isp: plane 0 chunk 3: phys 0x0000000899b00000 len 131072
qcom-camss acb7000.isp: plane 0: 11677696 bytes, 6 cpu entries, 5 physical breaks, largest physical run 8388608, 1 dma segments, 0 iova breaks, iova 0x00000000ff000000
Plane 0 == 11677696 bytes
Largest physical run = 8388608
Interestingly we see some physically adjacent allocations here and then also non-contiguous allocations, which is the negation of the thesis, constraining IOVA to physically contiguous maps is provably incorrect.
NAK
diff --git a/drivers/media/platform/qcom/camss/camss-video.c b/drivers/media/platform/qcom/camss/camss-video.c
index 0852eb6f13..d2df368d5f 100644
--- a/drivers/media/platform/qcom/camss/camss-video.c
+++ b/drivers/media/platform/qcom/camss/camss-video.c
@@ -146,6 +146,44 @@ static int video_queue_setup(struct vb2_queue *q,
return 0;
}
+/* DEBUG: how a plane is laid out in physical and IOVA space */
+static void video_buf_dump_sg(struct camss_video *video, unsigned int plane,
+ struct sg_table *sgt)
+{
+ dma_addr_t iova = sg_dma_address(sgt->sgl);
+ unsigned int j, pbreaks = 0, dbreaks = 0;
+ size_t total = 0, run = 0, maxrun = 0;
+ struct scatterlist *sg;
+ phys_addr_t pend = 0;
+ dma_addr_t dend = 0;
+
+ for_each_sgtable_sg(sgt, sg, j) {
+ if (j && sg_phys(sg) != pend) {
+ pbreaks++;
+ run = 0;
+ }
+ if (j < 4)
+ dev_info(video->camss->dev, "plane %u chunk %u: phys %pa len %u\n",
+ plane, j, &(phys_addr_t){ sg_phys(sg) }, sg->length);
+ run += sg->length;
+ if (run > maxrun)
+ maxrun = run;
+ total += sg->length;
+ pend = sg_phys(sg) + sg->length;
+ }
+
+ for_each_sgtable_dma_sg(sgt, sg, j) {
+ if (j && sg_dma_address(sg) != dend)
+ dbreaks++;
+ dend = sg_dma_address(sg) + sg_dma_len(sg);
+ }
+
+ dev_info(video->camss->dev,
+ "plane %u: %zu bytes, %u cpu entries, %u physical breaks, largest physical run %zu, %u dma segments, %u iova breaks, iova %pad\n",
+ plane, total, sgt->orig_nents, pbreaks, maxrun, sgt->nents,
+ dbreaks, &iova);
+}
+
static int video_buf_init(struct vb2_buffer *vb)
{
struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
@@ -163,6 +201,7 @@ static int video_buf_init(struct vb2_buffer *vb)
return -EFAULT;
buffer->addr[i] = sg_dma_address(sgt->sgl);
+ video_buf_dump_sg(video, i, sgt);
}
if (format->pixelformat == V4L2_PIX_FMT_NV12 ||