mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v3 0/4] media: qcom: camss: contiguous buffers, RDI and CSID1 streaming fixes
@ 2026-11-11 13:53 Allan Siriani
  2026-11-11 13:53 ` [PATCH v3 1/4] media: qcom: camss: Use contiguous DMA buffers Allan Siriani
                   ` (3 more replies)
  0 siblings, 4 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

Fixes found while bringing up the CAMSS on MSM8226 (Moto G 2013). The
SoC has an IOMMU in front of the CAMSS, but it is not described in the
upstream DT, so the CAMSS runs without one there today.

1. Loic's patch: use vb2_dma_contig, the write masters take a single
   address per plane.
2. Program the VFE 4.1 write master buffer size in frame based (RDI)
   mode.
3. Do not abort stop_streaming on -ENOIOCTLCMD from a subdev without
   .s_stream().
4. Do not fail CSID power up when there is no VFE with the same index
   (CSID1 on MSM8916/MSM8226).

Build tested (W=1, LLVM, ARM) on next-20261105, each patch on its own.
Runtime tested on MSM8226 with the same patches on a 6.16 based kernel.
Known issue: on the MT9M114 the first frame after STREAMON can still run
128 bytes past sizeimage; later frames are exact.

Changes in v3:
- Patch 2: added Fixes: tag, removed the comments, no clamp on the
  height (Bryan).
- Patch 3: removed the comment (Bryan), added Loic's Reviewed-by.

Changes in v2:
- Patch 1 replaced by Loic's "media: qcom: camss: Use contiguous DMA
  buffers", with his Signed-off-by fixed and Bryan's Reviewed-by.
- Reworded the IOMMU statements (Konrad).
- Patch 3: added Fixes: tag and Bryan's Reviewed-by.

v2: https://lore.kernel.org/all/20261109112611.94010-1-allan@fatecpompeia.edu.br/
v1: https://lore.kernel.org/all/20261104202313.72494-1-allan@fatecpompeia.edu.br/

Allan Siriani (3):
  media: qcom: camss: vfe-4-1: program the RDI write master buffer size
  media: qcom: camss: always stop the pipeline in stop_streaming
  media: qcom: camss: do not fail CSID power up without a matching VFE

Loic Poulain (1):
  media: qcom: camss: Use contiguous DMA buffers

 drivers/media/platform/qcom/camss/Kconfig     |  2 +-
 .../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 ++-
 .../media/platform/qcom/camss/camss-video.c   | 22 ++++++------------
 drivers/media/platform/qcom/camss/camss.c     | 17 +++++++-------
 8 files changed, 45 insertions(+), 33 deletions(-)


base-commit: ddfe51b0a0f5dc9e415cc2c8ad8db17532dd426b
-- 
2.43.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [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

* Re: [PATCH v3 2/4] media: qcom: camss: vfe-4-1: program the RDI write master buffer size
  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 14:35   ` Bryan O'Donoghue
  0 siblings, 0 replies; 6+ messages in thread
From: Bryan O'Donoghue @ 2026-11-11 14:35 UTC (permalink / raw)
  To: Allan Siriani, Mauro Carvalho Chehab
  Cc: Loic Poulain, Vladimir Zapolskiy, Konrad Dybcio,
	Dmitry Baryshkov, Todor Tomov, Hans Verkuil, linux-media,
	linux-arm-msm, linux-kernel

On 11/11/2026 13:53, Allan Siriani wrote:
> 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.

This is a long and rambling problem description that looks like LLMese 
to me.

I would appreciate a more direct and succinct problem description.

"MSM8226 doesn't have an IOMMU defined. If a sensor writes more data 
than WR_BUFER_CFG and/or WR_IMW_SIZE expect we will get an overrun."

"Program WM_IMAGE_SIZE and WR_BUFFER_CFG to restrict the maximum size 
the write masters can write"
> 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>
> ---

Something like that. Please don't let the LLM write it for you - the 
word-salad is difficult to take in.

Commit logs should be elucidative not occluding in nature.

---
bod

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-11-11 14:35 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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 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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®