* [PATCH v3 1/4] media: qcom: camss: Use contiguous DMA buffers
2026-11-11 13:53 [PATCH v3 0/4] media: qcom: camss: contiguous buffers, RDI and CSID1 streaming fixes Allan Siriani
@ 2026-11-11 13:53 ` Allan Siriani
2026-11-11 13:53 ` [PATCH v3 2/4] media: qcom: camss: vfe-4-1: program the RDI write master buffer size Allan Siriani
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Allan Siriani @ 2026-11-11 13:53 UTC (permalink / raw)
To: Bryan O'Donoghue, Mauro Carvalho Chehab
Cc: Loic Poulain, Vladimir Zapolskiy, Konrad Dybcio,
Dmitry Baryshkov, Todor Tomov, Hans Verkuil, linux-media,
linux-arm-msm, linux-kernel
From: Loic Poulain <loic.poulain@oss.qualcomm.com>
The VFE write masters are programmed with a single base address per
plane and then write the whole plane from there. video_buf_init() took
that address from sg_dma_address() of the first scatterlist entry of a
vb2_dma_sg buffer, which is only correct when the whole plane is mapped
as one contiguous range in the device address space.
Neither the driver nor the hardware can handle a plane split across
several DMA segments, so scatter-gather buffers were never really
supported here. The issue stayed dormant because vb2_dma_sg gives no
IOVA contiguity guarantee but still tries hard to keep the number of
segments low, and with an IOMMU dma_map_sgtable() usually maps the
whole table into a single IOVA range. The mapping therefore almost
always came back contiguous in the device address space, so reading
only the first entry happened to work.
Where that does not hold, the write master runs past the first segment
and writes the rest of the frame into memory belonging to someone else.
This recently surfaced as patch proposals enforcing contiguity in
camss itself, walking the scatterlist to verify that the DMA segments
follow each other, which amounts to re-implementing what the dma-contig
framework already provides.
Use vb2_dma_contig instead. It guarantees that each plane is mapped as
a single contiguous range in the device address space, one IOVA range
when an IOMMU is present, physically contiguous memory when there is
none.
Fixes: 0ac2586c410f ("media: camss: Add files which handle the video device nodes")
Reported-by: Allan Siriani <allan@fatecpompeia.edu.br>
Link: https://lore.kernel.org/all/20261104202313.72494-2-allan@fatecpompeia.edu.br/
Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Tested-by: Allan Siriani <allan@fatecpompeia.edu.br> # MSM8226 (no IOMMU), both sensors
Signed-off-by: Allan Siriani <allan@fatecpompeia.edu.br>
---
drivers/media/platform/qcom/camss/Kconfig | 2 +-
drivers/media/platform/qcom/camss/camss-video.c | 14 ++++----------
2 files changed, 5 insertions(+), 11 deletions(-)
diff --git a/drivers/media/platform/qcom/camss/Kconfig b/drivers/media/platform/qcom/camss/Kconfig
index b51745c94..e1967bc20 100644
--- a/drivers/media/platform/qcom/camss/Kconfig
+++ b/drivers/media/platform/qcom/camss/Kconfig
@@ -5,7 +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_SG
+ select VIDEOBUF2_DMA_CONTIG
select V4L2_FWNODE
select GENERIC_PHY
imply PHY_QCOM_MIPI_CSI2
diff --git a/drivers/media/platform/qcom/camss/camss-video.c b/drivers/media/platform/qcom/camss/camss-video.c
index 0852eb6f1..5de6ff5ec 100644
--- a/drivers/media/platform/qcom/camss/camss-video.c
+++ b/drivers/media/platform/qcom/camss/camss-video.c
@@ -13,7 +13,7 @@
#include <media/v4l2-device.h>
#include <media/v4l2-ioctl.h>
#include <media/v4l2-mc.h>
-#include <media/videobuf2-dma-sg.h>
+#include <media/videobuf2-dma-contig.h>
#include "camss-video.h"
#include "camss.h"
@@ -154,16 +154,10 @@ 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;
- 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);
- }
+ for (i = 0; i < format->num_planes; i++)
+ buffer->addr[i] = vb2_dma_contig_plane_dma_addr(vb, i);
if (format->pixelformat == V4L2_PIX_FMT_NV12 ||
format->pixelformat == V4L2_PIX_FMT_NV21 ||
@@ -699,7 +693,7 @@ 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;
+ q->mem_ops = &vb2_dma_contig_memops;
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
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v3 2/4] media: qcom: camss: vfe-4-1: program the RDI write master buffer size
2026-11-11 13:53 [PATCH v3 0/4] media: qcom: camss: contiguous buffers, RDI and CSID1 streaming fixes Allan Siriani
2026-11-11 13:53 ` [PATCH v3 1/4] media: qcom: camss: Use contiguous DMA buffers Allan Siriani
@ 2026-11-11 13:53 ` Allan Siriani
2026-11-11 14:35 ` Bryan O'Donoghue
2026-11-11 13:53 ` [PATCH v3 3/4] media: qcom: camss: always stop the pipeline in stop_streaming Allan Siriani
2026-11-11 13:53 ` [PATCH v3 4/4] media: qcom: camss: do not fail CSID power up without a matching VFE Allan Siriani
3 siblings, 1 reply; 6+ messages in thread
From: Allan Siriani @ 2026-11-11 13:53 UTC (permalink / raw)
To: Bryan O'Donoghue, Mauro Carvalho Chehab
Cc: Loic Poulain, Vladimir Zapolskiy, Konrad Dybcio,
Dmitry Baryshkov, Todor Tomov, Hans Verkuil, linux-media,
linux-arm-msm, linux-kernel
The RDI outputs use the write masters in frame based mode. In that mode
vfe_wm_frame_based() only sets the FRM_BASED bit of WR_CFG and leaves
WR_BUFFER_CFG and WR_IMAGE_SIZE untouched: they keep their reset value or
whatever a previous line based (PIX) stream left there, so nothing tells
the write master how big the buffer is. If the sensor sends more data
than the configured format, the write master keeps writing past the end
of the V4L2 buffer. When the CAMSS runs without an IOMMU, as on
MSM8226 today (the SoC has one, but it is not described in the upstream
DT; its VFE 4.0 shares this register layout), that corrupts unrelated
memory.
Program WR_BUFFER_CFG in frame based mode as the downstream VFE 4.0/4.1
driver does (msm_vfe40_axi_cfg_wm_reg(): words per line, lines minus one
and burst length, IMAGE_SIZE unused), from the bytesperline and height
of the active format, and clear it again when the write master is
released.
Pass the active format to the wm_frame_based() op for that. The VFE 4.7
and 4.8 implementations, which use a different register layout, are left
unchanged.
Fixes: 4c98a5f57f90 ("media: camss: Add VFE files")
Signed-off-by: Allan Siriani <allan@fatecpompeia.edu.br>
---
.../media/platform/qcom/camss/camss-vfe-4-1.c | 23 +++++++++++++++----
.../media/platform/qcom/camss/camss-vfe-4-7.c | 3 ++-
.../media/platform/qcom/camss/camss-vfe-4-8.c | 3 ++-
.../platform/qcom/camss/camss-vfe-gen1.c | 5 ++--
.../platform/qcom/camss/camss-vfe-gen1.h | 3 ++-
5 files changed, 28 insertions(+), 9 deletions(-)
diff --git a/drivers/media/platform/qcom/camss/camss-vfe-4-1.c b/drivers/media/platform/qcom/camss/camss-vfe-4-1.c
index 9cf1ccdb2..54afd7266 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe-4-1.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe-4-1.c
@@ -107,6 +107,7 @@
#define VFE_0_BUS_IMAGE_MASTER_n_WR_UB_CFG_OFFSET_SHIFT 16
#define VFE_0_BUS_IMAGE_MASTER_n_WR_IMAGE_SIZE(n) (0x080 + 0x24 * (n))
#define VFE_0_BUS_IMAGE_MASTER_n_WR_BUFFER_CFG(n) (0x084 + 0x24 * (n))
+#define VFE_0_BUS_IMAGE_MASTER_n_WR_BUFFER_CFG_BURST_LEN 0x3
#define VFE_0_BUS_IMAGE_MASTER_n_WR_FRAMEDROP_PATTERN(n) \
(0x088 + 0x24 * (n))
#define VFE_0_BUS_IMAGE_MASTER_n_WR_IRQ_SUBSAMPLE_PATTERN(n) \
@@ -269,14 +270,28 @@ static void vfe_wm_enable(struct vfe_device *vfe, u8 wm, u8 enable)
1 << VFE_0_BUS_IMAGE_MASTER_n_WR_CFG_WR_PATH_SHIFT);
}
-static void vfe_wm_frame_based(struct vfe_device *vfe, u8 wm, u8 enable)
+static void vfe_wm_frame_based(struct vfe_device *vfe, u8 wm,
+ struct v4l2_pix_format_mplane *pix, u8 enable)
{
- if (enable)
+ u32 reg;
+
+ if (enable) {
+ reg = VFE_0_BUS_IMAGE_MASTER_n_WR_BUFFER_CFG_BURST_LEN;
+ reg |= (pix->height - 1) << 4;
+ reg |= (pix->plane_fmt[0].bytesperline / 8) << 16;
+
+ writel_relaxed(0, vfe->base +
+ VFE_0_BUS_IMAGE_MASTER_n_WR_IMAGE_SIZE(wm));
+ writel_relaxed(reg, vfe->base +
+ VFE_0_BUS_IMAGE_MASTER_n_WR_BUFFER_CFG(wm));
vfe_reg_set(vfe, VFE_0_BUS_IMAGE_MASTER_n_WR_CFG(wm),
1 << VFE_0_BUS_IMAGE_MASTER_n_WR_CFG_FRM_BASED_SHIFT);
- else
+ } else {
vfe_reg_clr(vfe, VFE_0_BUS_IMAGE_MASTER_n_WR_CFG(wm),
1 << VFE_0_BUS_IMAGE_MASTER_n_WR_CFG_FRM_BASED_SHIFT);
+ writel_relaxed(0, vfe->base +
+ VFE_0_BUS_IMAGE_MASTER_n_WR_BUFFER_CFG(wm));
+ }
}
static void vfe_get_wm_sizes(struct v4l2_pix_format_mplane *pix, u8 plane,
@@ -313,7 +328,7 @@ static void vfe_wm_line_based(struct vfe_device *vfe, u32 wm,
wpl = vfe_word_per_line(pix->pixelformat, bytesperline);
- reg = 0x3;
+ reg = VFE_0_BUS_IMAGE_MASTER_n_WR_BUFFER_CFG_BURST_LEN;
reg |= (height - 1) << 4;
reg |= wpl << 16;
diff --git a/drivers/media/platform/qcom/camss/camss-vfe-4-7.c b/drivers/media/platform/qcom/camss/camss-vfe-4-7.c
index 76729607d..308c845f2 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe-4-7.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe-4-7.c
@@ -317,7 +317,8 @@ static void vfe_wm_enable(struct vfe_device *vfe, u8 wm, u8 enable)
1 << VFE_0_BUS_IMAGE_MASTER_n_WR_CFG_WR_PATH_SHIFT);
}
-static void vfe_wm_frame_based(struct vfe_device *vfe, u8 wm, u8 enable)
+static void vfe_wm_frame_based(struct vfe_device *vfe, u8 wm,
+ struct v4l2_pix_format_mplane *pix, u8 enable)
{
if (enable)
vfe_reg_set(vfe, VFE_0_BUS_IMAGE_MASTER_n_WR_ADDR_CFG(wm),
diff --git a/drivers/media/platform/qcom/camss/camss-vfe-4-8.c b/drivers/media/platform/qcom/camss/camss-vfe-4-8.c
index b2f7d855d..65df06a42 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe-4-8.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe-4-8.c
@@ -290,7 +290,8 @@ static void vfe_halt_clear(struct vfe_device *vfe)
writel_relaxed(0x0, vfe->base + VFE_0_BUS_BDG_CMD);
}
-static void vfe_wm_frame_based(struct vfe_device *vfe, u8 wm, u8 enable)
+static void vfe_wm_frame_based(struct vfe_device *vfe, u8 wm,
+ struct v4l2_pix_format_mplane *pix, u8 enable)
{
if (enable)
vfe_reg_set(vfe, VFE_0_BUS_IMAGE_MASTER_n_WR_ADDR_CFG(wm),
diff --git a/drivers/media/platform/qcom/camss/camss-vfe-gen1.c b/drivers/media/platform/qcom/camss/camss-vfe-gen1.c
index d84a375e3..f05676c2b 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe-gen1.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe-gen1.c
@@ -66,7 +66,7 @@ static int vfe_disable_output(struct vfe_line *line)
spin_lock_irqsave(&vfe->output_lock, flags);
if (line->id != VFE_LINE_PIX) {
- vfe->ops_gen1->wm_frame_based(vfe, output->wm_idx[0], 0);
+ vfe->ops_gen1->wm_frame_based(vfe, output->wm_idx[0], NULL, 0);
vfe->ops_gen1->bus_disconnect_wm_from_rdi(vfe, output->wm_idx[0], line->id);
vfe->ops_gen1->enable_irq_wm_line(vfe, output->wm_idx[0], line->id, 0);
vfe->ops_gen1->set_cgc_override(vfe, output->wm_idx[0], 0);
@@ -244,7 +244,8 @@ static int vfe_enable_output(struct vfe_line *line)
vfe->ops_gen1->set_rdi_cid(vfe, line->id, 0);
vfe->ops_gen1->wm_set_ub_cfg(vfe, output->wm_idx[0],
(ub_size + 1) * output->wm_idx[0], ub_size);
- vfe->ops_gen1->wm_frame_based(vfe, output->wm_idx[0], 1);
+ vfe->ops_gen1->wm_frame_based(vfe, output->wm_idx[0],
+ &line->video_out.active_fmt.fmt.pix_mp, 1);
vfe->ops_gen1->wm_enable(vfe, output->wm_idx[0], 1);
vfe->ops_gen1->bus_reload_wm(vfe, output->wm_idx[0]);
} else {
diff --git a/drivers/media/platform/qcom/camss/camss-vfe-gen1.h b/drivers/media/platform/qcom/camss/camss-vfe-gen1.h
index 6d5f96565..83856c0bc 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe-gen1.h
+++ b/drivers/media/platform/qcom/camss/camss-vfe-gen1.h
@@ -44,7 +44,8 @@ struct vfe_hw_ops_gen1 {
void (*set_realign_cfg)(struct vfe_device *vfe, struct vfe_line *line, u8 enable);
void (*set_qos)(struct vfe_device *vfe);
void (*set_xbar_cfg)(struct vfe_device *vfe, struct vfe_output *output, u8 enable);
- void (*wm_frame_based)(struct vfe_device *vfe, u8 wm, u8 enable);
+ void (*wm_frame_based)(struct vfe_device *vfe, u8 wm,
+ struct v4l2_pix_format_mplane *pix, u8 enable);
void (*wm_line_based)(struct vfe_device *vfe, u32 wm, struct v4l2_pix_format_mplane *pix,
u8 plane, u32 enable);
void (*wm_set_ub_cfg)(struct vfe_device *vfe, u8 wm, u16 offset, u16 depth);
--
2.43.0
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v3 3/4] media: qcom: camss: always stop the pipeline in stop_streaming
2026-11-11 13:53 [PATCH v3 0/4] media: qcom: camss: contiguous buffers, RDI and CSID1 streaming fixes Allan Siriani
2026-11-11 13:53 ` [PATCH v3 1/4] media: qcom: camss: Use contiguous DMA buffers Allan Siriani
2026-11-11 13:53 ` [PATCH v3 2/4] media: qcom: camss: vfe-4-1: program the RDI write master buffer size Allan Siriani
@ 2026-11-11 13:53 ` Allan Siriani
2026-11-11 13:53 ` [PATCH v3 4/4] media: qcom: camss: do not fail CSID power up without a matching VFE Allan Siriani
3 siblings, 0 replies; 6+ messages in thread
From: Allan Siriani @ 2026-11-11 13:53 UTC (permalink / raw)
To: Bryan O'Donoghue, Mauro Carvalho Chehab
Cc: Loic Poulain, Vladimir Zapolskiy, Konrad Dybcio,
Dmitry Baryshkov, Todor Tomov, Hans Verkuil, linux-media,
linux-arm-msm, linux-kernel
video_stop_streaming() walks the pipeline upstream and calls
.s_stream(0) on every subdev. It gives up on the first non-zero return,
without stopping the media pipeline and without returning the queued
buffers to vb2.
A subdev without .s_stream() returns -ENOIOCTLCMD. video_start_streaming()
already ignores that, but video_stop_streaming() does not, so with such a
subdev in the pipeline, e.g. the pixel array entity of the MT9M114 sensor
driver, every stream off:
- triggers the vb2 WARN about buffers still owned by the driver,
- leaves the pipeline started, so the next VIDIOC_STREAMON and
"media-ctl -r" fail with -EBUSY.
Treat -ENOIOCTLCMD as success, as at stream on, and only log real
errors, then always stop the pipeline and flush the buffers: a
.stop_streaming() callback cannot fail and must give back all buffers.
Fixes: 89013969e232 ("media: camss: sm8250: Pipeline starting and stopping for multiple virtual channels")
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Reviewed-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
Signed-off-by: Allan Siriani <allan@fatecpompeia.edu.br>
---
drivers/media/platform/qcom/camss/camss-video.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/media/platform/qcom/camss/camss-video.c b/drivers/media/platform/qcom/camss/camss-video.c
index 5de6ff5ec..4388f2708 100644
--- a/drivers/media/platform/qcom/camss/camss-video.c
+++ b/drivers/media/platform/qcom/camss/camss-video.c
@@ -314,11 +314,9 @@ static void video_stop_streaming(struct vb2_queue *q)
subdev = media_entity_to_v4l2_subdev(entity);
ret = v4l2_subdev_call(subdev, video, s_stream, 0);
-
- if (ret) {
- dev_err(video->camss->dev, "Video pipeline stop failed: %d\n", ret);
- return;
- }
+ if (ret && ret != -ENOIOCTLCMD)
+ dev_err(video->camss->dev,
+ "Failed to stop %s: %d\n", entity->name, ret);
}
video_device_pipeline_stop(vdev);
--
2.43.0
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v3 4/4] media: qcom: camss: do not fail CSID power up without a matching VFE
2026-11-11 13:53 [PATCH v3 0/4] media: qcom: camss: contiguous buffers, RDI and CSID1 streaming fixes Allan Siriani
` (2 preceding siblings ...)
2026-11-11 13:53 ` [PATCH v3 3/4] media: qcom: camss: always stop the pipeline in stop_streaming Allan Siriani
@ 2026-11-11 13:53 ` Allan Siriani
3 siblings, 0 replies; 6+ messages in thread
From: Allan Siriani @ 2026-11-11 13:53 UTC (permalink / raw)
To: Bryan O'Donoghue, Mauro Carvalho Chehab
Cc: Loic Poulain, Vladimir Zapolskiy, Konrad Dybcio,
Dmitry Baryshkov, Todor Tomov, Hans Verkuil, linux-media,
linux-arm-msm, linux-kernel
csid_set_power() powers up the VFE with the same index as the CSID
before the CSID itself, through parent_dev_ops->get(). The VFE
implementation, vfe_parent_dev_ops_get(), returns -EINVAL when there is
no VFE with that index.
On the ISPIF based SoCs there are more CSIDs than VFEs: MSM8916 (and
MSM8226) have two CSIDs and a single VFE, and the ISPIF routes either
CSID to it. Streaming from CSID1 then fails right away with
qcom-camss ...: Failed to power up pipeline: -22
although the pipeline CSID1 -> ISPIF -> VFE0 is valid.
Return 0 when there is no VFE of the same index: there is nothing to
power up for that CSID, the VFE in the pipeline is powered by its own
subdev. vfe_parent_dev_ops_put() already ignores such indexes.
Signed-off-by: Allan Siriani <allan@fatecpompeia.edu.br>
---
drivers/media/platform/qcom/camss/camss.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
index c54185c93..532f6d36d 100644
--- a/drivers/media/platform/qcom/camss/camss.c
+++ b/drivers/media/platform/qcom/camss/camss.c
@@ -4631,15 +4631,16 @@ static int camss_read_version(struct camss *camss)
static int vfe_parent_dev_ops_get(struct camss *camss, int id)
{
- int ret = -EINVAL;
-
- if (id < camss->res->vfe_num) {
- struct vfe_device *vfe = &camss->vfe[id];
-
- ret = vfe_get(vfe);
- }
+ /*
+ * On the ISPIF based SoCs any CSID can feed any VFE, and there may be
+ * more CSIDs than VFEs (MSM8916: two CSIDs, one VFE). A CSID without a
+ * VFE of the same index has no parent to power up: the VFE that ends
+ * up in the pipeline is powered by its own subdev.
+ */
+ if (id >= camss->res->vfe_num)
+ return 0;
- return ret;
+ return vfe_get(&camss->vfe[id]);
}
static int vfe_parent_dev_ops_put(struct camss *camss, int id)
--
2.43.0
^ permalink raw reply [flat|nested] 6+ messages in thread