=?y?q?=5BPATCH=200/3=5D=20media=3A=20qcom=3A=20camss=3A=20Add=20PIX=20support=20for=20CSID/VFE=E2=80=91340?=

=?y?q?=5BPATCH=200/3=5D=20media=3A=20qcom=3A=20camss=3A=20Add=20PIX=20support=20for=20CSID/VFE=E2=80=91340?=

[Loic Poulain]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 1308 bytes --]

Add PIX‑path support to the CAMSS pipeline on CSID‑340 and
VFE‑340, allowing frame capture through the VFE PIX video
device like msm_vfe0_pix.

PIX capture has been validated on Agatti / Arduino‑Uno‑Q using an
IMX219 sensor:
media-ctl -d /dev/media0 \
  -l '"msm_csiphy0":1->"msm_csid0":0[1],"msm_csid0":4->"msm_vfe0_pix":0[1]'
media-ctl -d /dev/media0 -V '"imx219 2-0010":0[fmt:SRGGB10_1X10/640x480 field:none]'
media-ctl -d /dev/media0 -V '"msm_csiphy0":0[fmt:SRGGB10_1X10/640x480 field:none]'
media-ctl -d /dev/media0 -V '"msm_csid0":0[fmt:SRGGB10_1X10/640x480 field:none]'
media-ctl -d /dev/media0 -V '"msm_csid0":4[fmt:SRGGB10_1X10/640x480 field:none]'
media-ctl -d /dev/media0 -V '"msm_vfe0_rdi0":0[fmt:SRGGB10_1X10/640x480 field:none]'
media-ctl -d /dev/media0 -V '"msm_vfe0_pix":0[fmt:SRGGB10_1X10/640x480 field:none]'

yavta -B capture-mplane --capture=30 -n 3 -f SRGGB10P -s 640x480 /dev/video3
[...]

Loic Poulain (3):
  media: qcom: camss: vfe-340: Proper client handling
  media: qcom: camss: csid-340: Enable PIX path support
  media: qcom: camss: vfe-340: Support for PIX client

 .../platform/qcom/camss/camss-csid-340.c      |  85 +++++++----
 .../media/platform/qcom/camss/camss-vfe-340.c | 142 ++++++++++++------
 2 files changed, 153 insertions(+), 74 deletions(-)

-- 
2.34.1

[PATCH 1/3] media: qcom: camss: vfe-340: Proper client handling

[Loic Poulain]

We need to properly map camss WM index to our internal WM client
instance. Today we're only support RDI interfaces with the RDI_WM
macro, introduce a __wm_to_client helper to support any interface.

Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
---
 .../media/platform/qcom/camss/camss-vfe-340.c | 94 +++++++++++--------
 1 file changed, 53 insertions(+), 41 deletions(-)

diff --git a/drivers/media/platform/qcom/camss/camss-vfe-340.c b/drivers/media/platform/qcom/camss/camss-vfe-340.c
index 30d7630b3e8b..c6ea8b6216c2 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe-340.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe-340.c
@@ -69,24 +69,19 @@
 #define TFE_BUS_FRAMEDROP_CFG_0(c)			BUS_REG(0x238 + (c) * 0x100)
 #define TFE_BUS_FRAMEDROP_CFG_1(c)			BUS_REG(0x23c + (c) * 0x100)
 
-/*
- * TODO: differentiate the port id based on requested type of RDI, BHIST etc
- *
- * TFE write master IDs (clients)
- *
- * BAYER		0
- * IDEAL_RAW		1
- * STATS_TINTLESS_BG	2
- * STATS_BHIST		3
- * STATS_AWB_BG		4
- * STATS_AEC_BG		5
- * STATS_BAF		6
- * RDI0			7
- * RDI1			8
- * RDI2			9
- */
-#define RDI_WM(n)		(7 + (n))
-#define TFE_WM_NUM		10
+enum tfe_client {
+	TFE_CLI_BAYER,
+	TFE_CLI_IDEAL_RAW,
+	TFE_CLI_STATS_TINTLESS_BG,
+	TFE_CLI_STATS_BHIST,
+	TFE_CLI_STATS_AWB_BG,
+	TFE_CLI_STATS_AEC_BG,
+	TFE_CLI_STATS_BAF,
+	TFE_CLI_RDI0,
+	TFE_CLI_RDI1,
+	TFE_CLI_RDI2,
+	TFE_CLI_NUM
+};
 
 enum tfe_iface {
 	TFE_IFACE_PIX,
@@ -108,6 +103,13 @@ enum tfe_subgroups {
 	TFE_SUBGROUP_NUM
 };
 
+static enum tfe_iface tfe_line_iface_map[VFE_LINE_NUM_MAX] = {
+	[VFE_LINE_RDI0] = TFE_CLI_RDI0,
+	[VFE_LINE_RDI1] = TFE_CLI_RDI1,
+	[VFE_LINE_RDI2] = TFE_CLI_RDI2,
+	[VFE_LINE_PIX] = TFE_CLI_BAYER,
+};
+
 static enum tfe_iface tfe_line_iface_map[VFE_LINE_NUM_MAX] = {
 	[VFE_LINE_RDI0] = TFE_IFACE_RDI0,
 	[VFE_LINE_RDI1] = TFE_IFACE_RDI1,
@@ -126,6 +128,16 @@ static enum vfe_line_id tfe_subgroup_line_map[TFE_SUBGROUP_NUM] = {
 	[TFE_SUBGROUP_RDI2] = VFE_LINE_RDI2,
 };
 
+static inline enum tfe_client  __wm_to_client(u8 wm)
+{
+	if (wm >= ARRAY_SIZE(tfe_wm_client_map)) {
+		pr_warn("VFE: Invalid WM%u\n", wm);
+		return TFE_CLI_RDI0;
+	}
+
+	return tfe_wm_client_map[wm];
+}
+
 static inline enum tfe_iface  __line_to_iface(enum vfe_line_id line_id)
 {
 	if (line_id <= VFE_LINE_NONE || line_id >= VFE_LINE_NUM_MAX) {
@@ -209,10 +221,10 @@ static irqreturn_t vfe_isr(int irq, void *dev)
 	status = readl_relaxed(vfe->base + TFE_BUS_OVERFLOW_STATUS);
 	if (status) {
 		writel_relaxed(status, vfe->base + TFE_BUS_STATUS_CLEAR);
-		for (i = 0; i < TFE_WM_NUM; i++) {
+		for (i = 0; i < TFE_CLI_NUM; i++) {
 			if (status & BIT(i))
 				dev_err_ratelimited(vfe->camss->dev,
-						    "VFE%u: bus overflow for wm %u\n",
+						    "VFE%u: bus overflow for client %u\n",
 						    vfe->id, i);
 		}
 	}
@@ -235,49 +247,49 @@ static void vfe_enable_irq(struct vfe_device *vfe)
 	       TFE_BUS_IRQ_MASK_0_IMG_VIOL, vfe->base + TFE_BUS_IRQ_MASK_0);
 }
 
-static void vfe_wm_update(struct vfe_device *vfe, u8 rdi, u32 addr,
+static void vfe_wm_update(struct vfe_device *vfe, u8 wm, u32 addr,
 			  struct vfe_line *line)
 {
-	u8 wm = RDI_WM(rdi);
+	u8 client = __wm_to_client(wm);
 
-	writel_relaxed(addr, vfe->base + TFE_BUS_IMAGE_ADDR(wm));
+	writel_relaxed(addr, vfe->base + TFE_BUS_IMAGE_ADDR(client));
 }
 
-static void vfe_wm_start(struct vfe_device *vfe, u8 rdi, struct vfe_line *line)
+static void vfe_wm_start(struct vfe_device *vfe, u8 wm, struct vfe_line *line)
 {
 	struct v4l2_pix_format_mplane *pix = &line->video_out.active_fmt.fmt.pix_mp;
 	u32 stride = pix->plane_fmt[0].bytesperline;
-	u8 wm = RDI_WM(rdi);
+	u8 client = __wm_to_client(wm);
 
 	/* Configuration for plain RDI frames */
-	writel_relaxed(TFE_BUS_IMAGE_CFG_0_DEFAULT, vfe->base + TFE_BUS_IMAGE_CFG_0(wm));
-	writel_relaxed(0u, vfe->base + TFE_BUS_IMAGE_CFG_1(wm));
-	writel_relaxed(TFE_BUS_IMAGE_CFG_2_DEFAULT, vfe->base + TFE_BUS_IMAGE_CFG_2(wm));
-	writel_relaxed(stride * pix->height, vfe->base + TFE_BUS_FRAME_INCR(wm));
-	writel_relaxed(TFE_BUS_PACKER_CFG_FMT_PLAIN64, vfe->base + TFE_BUS_PACKER_CFG(wm));
+	writel_relaxed(TFE_BUS_IMAGE_CFG_0_DEFAULT, vfe->base + TFE_BUS_IMAGE_CFG_0(client));
+	writel_relaxed(0u, vfe->base + TFE_BUS_IMAGE_CFG_1(client));
+	writel_relaxed(TFE_BUS_IMAGE_CFG_2_DEFAULT, vfe->base + TFE_BUS_IMAGE_CFG_2(client));
+	writel_relaxed(stride * pix->height, vfe->base + TFE_BUS_FRAME_INCR(client));
+	writel_relaxed(TFE_BUS_PACKER_CFG_FMT_PLAIN64, vfe->base + TFE_BUS_PACKER_CFG(client));
 
 	/* No dropped frames, one irq per frame */
-	writel_relaxed(0, vfe->base + TFE_BUS_FRAMEDROP_CFG_0(wm));
-	writel_relaxed(1, vfe->base + TFE_BUS_FRAMEDROP_CFG_1(wm));
-	writel_relaxed(0, vfe->base + TFE_BUS_IRQ_SUBSAMPLE_CFG_0(wm));
-	writel_relaxed(1, vfe->base + TFE_BUS_IRQ_SUBSAMPLE_CFG_1(wm));
+	writel_relaxed(0, vfe->base + TFE_BUS_FRAMEDROP_CFG_0(client));
+	writel_relaxed(1, vfe->base + TFE_BUS_FRAMEDROP_CFG_1(client));
+	writel_relaxed(0, vfe->base + TFE_BUS_IRQ_SUBSAMPLE_CFG_0(client));
+	writel_relaxed(1, vfe->base + TFE_BUS_IRQ_SUBSAMPLE_CFG_1(client));
 
 	vfe_enable_irq(vfe);
 
 	writel(TFE_BUS_CLIENT_CFG_EN | TFE_BUS_CLIENT_CFG_MODE_FRAME,
-	       vfe->base + TFE_BUS_CLIENT_CFG(wm));
+	       vfe->base + TFE_BUS_CLIENT_CFG(client));
 
-	dev_dbg(vfe->camss->dev, "VFE%u: Started RDI%u width %u height %u stride %u\n",
-		vfe->id, rdi, pix->width, pix->height, stride);
+	dev_dbg(vfe->camss->dev, "VFE%u: Started client %u width %u height %u stride %u\n",
+		vfe->id, client, pix->width, pix->height, client);
 }
 
-static void vfe_wm_stop(struct vfe_device *vfe, u8 rdi)
+static void vfe_wm_stop(struct vfe_device *vfe, u8 wm)
 {
-	u8 wm = RDI_WM(rdi);
+	u8 client = __wm_to_client(wm);
 
-	writel(0, vfe->base + TFE_BUS_CLIENT_CFG(wm));
+	writel(0, vfe->base + TFE_BUS_CLIENT_CFG(client));
 
-	dev_dbg(vfe->camss->dev, "VFE%u: Stopped RDI%u\n", vfe->id, rdi);
+	dev_dbg(vfe->camss->dev, "VFE%u: Stopped client %u\n", vfe->id, client);
 }
 
 static const struct camss_video_ops vfe_video_ops_520 = {
-- 
2.34.1

[PATCH 2/3] media: qcom: camss: csid-340: Enable PIX path support

[Loic Poulain]

Add support for CSID to PIX interface.

Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
---
 .../platform/qcom/camss/camss-csid-340.c      | 85 ++++++++++++-------
 1 file changed, 55 insertions(+), 30 deletions(-)

diff --git a/drivers/media/platform/qcom/camss/camss-csid-340.c b/drivers/media/platform/qcom/camss/camss-csid-340.c
index 2b50f9b96a34..f7b4cb054c55 100644
--- a/drivers/media/platform/qcom/camss/camss-csid-340.c
+++ b/drivers/media/platform/qcom/camss/camss-csid-340.c
@@ -41,19 +41,24 @@
 #define		CSI2_RX_CFG1_MISR_EN			BIT(6)
 #define		CSI2_RX_CFG1_CGC_MODE			BIT(7)
 
-#define CSID_RDI_CFG0(rdi)					(0x300 + 0x100 * (rdi))
-#define		CSID_RDI_CFG0_BYTE_CNTR_EN		BIT(0)
-#define		CSID_RDI_CFG0_TIMESTAMP_EN		BIT(1)
-#define		CSID_RDI_CFG0_DECODE_FORMAT_MASK	GENMASK(15, 12)
-#define		CSID_RDI_CFG0_DECODE_FORMAT_NOP		CSID_RDI_CFG0_DECODE_FORMAT_MASK
-#define		CSID_RDI_CFG0_DT_MASK			GENMASK(21, 16)
-#define		CSID_RDI_CFG0_VC_MASK			GENMASK(23, 22)
-#define		CSID_RDI_CFG0_DTID_MASK			GENMASK(28, 27)
-#define		CSID_RDI_CFG0_ENABLE			BIT(31)
-
-#define CSID_RDI_CTRL(rdi)					(0x308 + 0x100 * (rdi))
-#define CSID_RDI_CTRL_HALT_AT_FRAME_BOUNDARY		0
-#define CSID_RDI_CTRL_RESUME_AT_FRAME_BOUNDARY		1
+#define CSID_CFG0(iface)					(0x300 + 0x100 * (iface))
+#define		CSID_CFG0_BYTE_CNTR_EN			BIT(0)
+#define		CSID_CFG0_TIMESTAMP_EN			BIT(1)
+#define		CSID_CFG0_DECODE_FORMAT_MASK		GENMASK(15, 12)
+#define		CSID_CFG0_DECODE_FORMAT_NOP		CSID_CFG0_DECODE_FORMAT_MASK
+#define		CSID_CFG0_DT_MASK			GENMASK(21, 16)
+#define		CSID_CFG0_VC_MASK			GENMASK(23, 22)
+#define		CSID_CFG0_DTID_MASK			GENMASK(28, 27)
+#define		CSID_CFG0_ENABLE			BIT(31)
+
+#define CSID_CTRL(iface)					(0x308 + 0x100 * (iface))
+#define CSID_CTRL_HALT_AT_FRAME_BOUNDARY		0
+#define CSID_CTRL_RESUME_AT_FRAME_BOUNDARY		1
+
+#define CSID_MAX_RDI_SRC_STREAMS	(MSM_CSID_MAX_SRC_STREAMS - 1)
+#define CSID_PIX_SRC_STREAMS		CSID_MAX_RDI_SRC_STREAMS
+
+#define CSID_IFACE_PIX	-1
 
 static void __csid_configure_rx(struct csid_device *csid, struct csid_phy_config *phy)
 {
@@ -69,11 +74,6 @@ static void __csid_configure_rx(struct csid_device *csid, struct csid_phy_config
 	writel_relaxed(val, csid->base + CSID_CSI2_RX_CFG1);
 }
 
-static void __csid_ctrl_rdi(struct csid_device *csid, int enable, u8 rdi)
-{
-	writel_relaxed(!!enable, csid->base + CSID_RDI_CTRL(rdi));
-}
-
 static void __csid_configure_rdi_stream(struct csid_device *csid, u8 enable, u8 vc)
 {
 	struct v4l2_mbus_framefmt *input_format = &csid->fmt[MSM_CSID_PAD_FIRST_SRC + vc];
@@ -88,7 +88,7 @@ static void __csid_configure_rdi_stream(struct csid_device *csid, u8 enable, u8
 	 * the four least significant bits of the five bit VC
 	 * bitfield to generate an internal CID value.
 	 *
-	 * CSID_RDI_CFG0(vc)
+	 * CSID_CFG0(vc)
 	 * DT_ID : 28:27
 	 * VC    : 26:22
 	 * DT    : 21:16
@@ -97,18 +97,40 @@ static void __csid_configure_rdi_stream(struct csid_device *csid, u8 enable, u8
 	 */
 	dt_id = vc & 0x03;
 
-	val = CSID_RDI_CFG0_DECODE_FORMAT_NOP; /* only for RDI path */
-	val |= FIELD_PREP(CSID_RDI_CFG0_DT_MASK, format->data_type);
-	val |= FIELD_PREP(CSID_RDI_CFG0_VC_MASK, vc);
-	val |= FIELD_PREP(CSID_RDI_CFG0_DTID_MASK, dt_id);
+	val = CSID_CFG0_DECODE_FORMAT_NOP; /* only for RDI path */
+	val |= FIELD_PREP(CSID_CFG0_DT_MASK, format->data_type);
+	val |= FIELD_PREP(CSID_CFG0_VC_MASK, vc);
+	val |= FIELD_PREP(CSID_CFG0_DTID_MASK, dt_id);
 
 	if (enable)
-		val |= CSID_RDI_CFG0_ENABLE;
+		val |= CSID_CFG0_ENABLE;
 
-	dev_dbg(csid->camss->dev, "CSID%u: Stream %s (dt:0x%x vc=%u)\n",
+	dev_dbg(csid->camss->dev, "CSID%u: Stream %s RDI (dt:0x%x vc=%u)\n",
 		csid->id, enable ? "enable" : "disable", format->data_type, vc);
 
-	writel_relaxed(val, csid->base + CSID_RDI_CFG0(vc));
+	writel_relaxed(val, csid->base + CSID_CFG0(vc));
+	writel_relaxed(!!enable, csid->base + CSID_CTRL(vc));
+}
+
+static void __csid_configure_pix_stream(struct csid_device *csid, u8 enable)
+{
+	struct v4l2_mbus_framefmt *input_format = &csid->fmt[MSM_CSID_PADS_NUM - 1];
+	const struct csid_format_info *format = csid_get_fmt_entry(csid->res->formats->formats,
+								   csid->res->formats->nformats,
+								   input_format->code);
+	u32 val;
+
+	val = FIELD_PREP(CSID_CFG0_DECODE_FORMAT_MASK, format->decode_format);
+	val |= FIELD_PREP(CSID_CFG0_DT_MASK, format->data_type);
+
+	if (enable)
+		val |= CSID_CFG0_ENABLE;
+
+	dev_dbg(csid->camss->dev, "CSID%u: Stream %s PIX (dt=0x%x df=0x%x)\n",
+		csid->id, enable ? "enable" : "disable", format->data_type, format->decode_format);
+
+	writel_relaxed(val, csid->base + CSID_CFG0(CSID_IFACE_PIX));
+	writel_relaxed(!!enable, csid->base + CSID_CTRL(CSID_IFACE_PIX));
 }
 
 static void csid_configure_stream(struct csid_device *csid, u8 enable)
@@ -117,12 +139,15 @@ static void csid_configure_stream(struct csid_device *csid, u8 enable)
 
 	__csid_configure_rx(csid, &csid->phy);
 
-	for (i = 0; i < MSM_CSID_MAX_SRC_STREAMS; i++) {
-		if (csid->phy.en_vc & BIT(i)) {
+	/* RDIs */
+	for (i = 0; i < CSID_MAX_RDI_SRC_STREAMS; i++) {
+		if (csid->phy.en_vc & BIT(i))
 			__csid_configure_rdi_stream(csid, enable, i);
-			__csid_ctrl_rdi(csid, enable, i);
-		}
 	}
+
+	/* PIX */
+	if (csid->phy.en_vc & BIT(CSID_PIX_SRC_STREAMS))
+		__csid_configure_pix_stream(csid, enable);
 }
 
 static int csid_reset(struct csid_device *csid)
-- 
2.34.1

[PATCH 3/3] media: qcom: camss: vfe-340: Support for PIX client

[Loic Poulain]

Add support for VFE PIX write engine, allowing to capture frames
via the PIX video device (e.g. msm_vfe0_pix).

Tested on Agatti/Arduino-Uno-Q with:
media-ctl -d /dev/media0 -l '"msm_csiphy0":1->"msm_csid0":0[1],"msm_csid0":4->"msm_vfe0_pix":0[1]'
media-ctl -d /dev/media0 -V '"imx219 2-0010":0[fmt:SRGGB10_1X10/640x480 field:none]'
media-ctl -d /dev/media0 -V '"msm_csiphy0":0[fmt:SRGGB10_1X10/640x480 field:none]'
media-ctl -d /dev/media0 -V '"msm_csid0":0[fmt:SRGGB10_1X10/640x480 field:none]'
media-ctl -d /dev/media0 -V '"msm_csid0":4[fmt:SRGGB10_1X10/640x480 field:none]'
media-ctl -d /dev/media0 -V '"msm_vfe0_rdi0":0[fmt:SRGGB10_1X10/640x480 field:none]'
media-ctl -d /dev/media0 -V '"msm_vfe0_pix":0[fmt:SRGGB10_1X10/640x480 field:none]'
yavta -B capture-mplane --capture=30 -n 3 -f SRGGB10P -s 640x480 /dev/video3

Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
---
 .../media/platform/qcom/camss/camss-vfe-340.c | 64 +++++++++++++++----
 1 file changed, 53 insertions(+), 11 deletions(-)

diff --git a/drivers/media/platform/qcom/camss/camss-vfe-340.c b/drivers/media/platform/qcom/camss/camss-vfe-340.c
index c6ea8b6216c2..50ac5d84f67c 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe-340.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe-340.c
@@ -63,7 +63,10 @@
 #define TFE_BUS_IMAGE_CFG_2(c)				BUS_REG(0x214 + (c) * 0x100)
 #define		TFE_BUS_IMAGE_CFG_2_DEFAULT	0xffff
 #define TFE_BUS_PACKER_CFG(c)				BUS_REG(0x218 + (c) * 0x100)
+#define		TFE_BUS_PACKER_CFG_FMT_PLAIN8	0x1
 #define		TFE_BUS_PACKER_CFG_FMT_PLAIN64	0xa
+#define		TFE_BUS_PACKER_CFG_FMT_MIPI10	0xc
+#define		TFE_BUS_PACKER_CFG_FMT_MIPI12	0xd
 #define TFE_BUS_IRQ_SUBSAMPLE_CFG_0(c)			BUS_REG(0x230 + (c) * 0x100)
 #define TFE_BUS_IRQ_SUBSAMPLE_CFG_1(c)			BUS_REG(0x234 + (c) * 0x100)
 #define TFE_BUS_FRAMEDROP_CFG_0(c)			BUS_REG(0x238 + (c) * 0x100)
@@ -103,7 +106,7 @@ enum tfe_subgroups {
 	TFE_SUBGROUP_NUM
 };
 
-static enum tfe_iface tfe_line_iface_map[VFE_LINE_NUM_MAX] = {
+static enum tfe_client tfe_wm_client_map[VFE_LINE_NUM_MAX] = {
 	[VFE_LINE_RDI0] = TFE_CLI_RDI0,
 	[VFE_LINE_RDI1] = TFE_CLI_RDI1,
 	[VFE_LINE_RDI2] = TFE_CLI_RDI2,
@@ -255,18 +258,58 @@ static void vfe_wm_update(struct vfe_device *vfe, u8 wm, u32 addr,
 	writel_relaxed(addr, vfe->base + TFE_BUS_IMAGE_ADDR(client));
 }
 
+static u32 vfe_packer_format(struct vfe_device *vfe, u32 pixelformat)
+{
+	const struct camss_formats *fmt = vfe->res->formats_rdi;
+	int i;
+
+	for (i = 0; i < fmt->nformats; i++)
+		if (fmt->formats[i].pixelformat == pixelformat)
+			break;
+
+	if (i >= fmt->nformats)
+		return TFE_BUS_PACKER_CFG_FMT_PLAIN64;
+
+	switch (fmt->formats[i].mbus_bpp) {
+	case 8:
+		return TFE_BUS_PACKER_CFG_FMT_PLAIN8;
+	case 10:
+		return TFE_BUS_PACKER_CFG_FMT_MIPI10;
+	case 12:
+		return TFE_BUS_PACKER_CFG_FMT_MIPI12;
+	default:
+		dev_err(vfe->camss->dev, "VFE%u: Unsupported pixelformat", vfe->id);
+	}
+
+	return TFE_BUS_PACKER_CFG_FMT_PLAIN64;
+}
+
 static void vfe_wm_start(struct vfe_device *vfe, u8 wm, struct vfe_line *line)
 {
 	struct v4l2_pix_format_mplane *pix = &line->video_out.active_fmt.fmt.pix_mp;
 	u32 stride = pix->plane_fmt[0].bytesperline;
 	u8 client = __wm_to_client(wm);
-
-	/* Configuration for plain RDI frames */
-	writel_relaxed(TFE_BUS_IMAGE_CFG_0_DEFAULT, vfe->base + TFE_BUS_IMAGE_CFG_0(client));
-	writel_relaxed(0u, vfe->base + TFE_BUS_IMAGE_CFG_1(client));
-	writel_relaxed(TFE_BUS_IMAGE_CFG_2_DEFAULT, vfe->base + TFE_BUS_IMAGE_CFG_2(client));
-	writel_relaxed(stride * pix->height, vfe->base + TFE_BUS_FRAME_INCR(client));
-	writel_relaxed(TFE_BUS_PACKER_CFG_FMT_PLAIN64, vfe->base + TFE_BUS_PACKER_CFG(client));
+	u32 cfg = TFE_BUS_CLIENT_CFG_EN;
+
+	if (client == TFE_CLI_BAYER) { /* PIX */
+		writel_relaxed(pix->width + (pix->height << 16),
+			       vfe->base + TFE_BUS_IMAGE_CFG_0(client));
+		writel_relaxed(0u, vfe->base + TFE_BUS_IMAGE_CFG_1(client));
+		writel_relaxed(stride, vfe->base + TFE_BUS_IMAGE_CFG_2(client));
+		writel_relaxed(stride * pix->height, vfe->base + TFE_BUS_FRAME_INCR(client));
+		writel_relaxed(vfe_packer_format(vfe, pix->pixelformat),
+			       vfe->base + TFE_BUS_PACKER_CFG(client));
+	} else { /* RDI */
+		writel_relaxed(TFE_BUS_IMAGE_CFG_0_DEFAULT,
+			       vfe->base + TFE_BUS_IMAGE_CFG_0(client));
+		writel_relaxed(0u, vfe->base + TFE_BUS_IMAGE_CFG_1(client));
+		writel_relaxed(TFE_BUS_IMAGE_CFG_2_DEFAULT,
+			       vfe->base + TFE_BUS_IMAGE_CFG_2(client));
+		writel_relaxed(stride * pix->height, vfe->base + TFE_BUS_FRAME_INCR(client));
+		writel_relaxed(TFE_BUS_PACKER_CFG_FMT_PLAIN64,
+			       vfe->base + TFE_BUS_PACKER_CFG(client));
+		cfg |= TFE_BUS_CLIENT_CFG_MODE_FRAME;
+	}
 
 	/* No dropped frames, one irq per frame */
 	writel_relaxed(0, vfe->base + TFE_BUS_FRAMEDROP_CFG_0(client));
@@ -276,11 +319,10 @@ static void vfe_wm_start(struct vfe_device *vfe, u8 wm, struct vfe_line *line)
 
 	vfe_enable_irq(vfe);
 
-	writel(TFE_BUS_CLIENT_CFG_EN | TFE_BUS_CLIENT_CFG_MODE_FRAME,
-	       vfe->base + TFE_BUS_CLIENT_CFG(client));
+	writel(cfg, vfe->base + TFE_BUS_CLIENT_CFG(client));
 
 	dev_dbg(vfe->camss->dev, "VFE%u: Started client %u width %u height %u stride %u\n",
-		vfe->id, client, pix->width, pix->height, client);
+		vfe->id, client, pix->width, pix->height, stride);
 }
 
 static void vfe_wm_stop(struct vfe_device *vfe, u8 wm)
-- 
2.34.1

Re: [PATCH 1/3] media: qcom: camss: vfe-340: Proper client handling

[Konrad Dybcio]

On 2/19/26 4:27 PM, Loic Poulain wrote:
> We need to properly map camss WM index to our internal WM client
> instance. Today we're only support RDI interfaces with the RDI_WM
> macro, introduce a __wm_to_client helper to support any interface.

Do they actually differ between platforms, or can we simply remodel the
driver defines?

Konrad

Re: [PATCH 2/3] media: qcom: camss: csid-340: Enable PIX path support

[Konrad Dybcio]

On 2/19/26 4:27 PM, Loic Poulain wrote:
> Add support for CSID to PIX interface.
> 
> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> ---

[...]

> -#define CSID_RDI_CFG0(rdi)					(0x300 + 0x100 * (rdi))
> -#define		CSID_RDI_CFG0_BYTE_CNTR_EN		BIT(0)
> -#define		CSID_RDI_CFG0_TIMESTAMP_EN		BIT(1)
> -#define		CSID_RDI_CFG0_DECODE_FORMAT_MASK	GENMASK(15, 12)
> -#define		CSID_RDI_CFG0_DECODE_FORMAT_NOP		CSID_RDI_CFG0_DECODE_FORMAT_MASK
> -#define		CSID_RDI_CFG0_DT_MASK			GENMASK(21, 16)
> -#define		CSID_RDI_CFG0_VC_MASK			GENMASK(23, 22)
> -#define		CSID_RDI_CFG0_DTID_MASK			GENMASK(28, 27)
> -#define		CSID_RDI_CFG0_ENABLE			BIT(31)

Much of this patch is renaming, could you split this up, so it's more
obvious what it actually introduces?

[...]

> +static void __csid_configure_pix_stream(struct csid_device *csid, u8 enable)

Perhaps that's yak shaving, but I don't think enable should be non-binary

> +{
> +	struct v4l2_mbus_framefmt *input_format = &csid->fmt[MSM_CSID_PADS_NUM - 1];
> +	const struct csid_format_info *format = csid_get_fmt_entry(csid->res->formats->formats,
> +								   csid->res->formats->nformats,
> +								   input_format->code);
> +	u32 val;
> +
> +	val = FIELD_PREP(CSID_CFG0_DECODE_FORMAT_MASK, format->decode_format);
> +	val |= FIELD_PREP(CSID_CFG0_DT_MASK, format->data_type);

no vc/dt_id?


> +
> +	if (enable)
> +		val |= CSID_CFG0_ENABLE;
> +
> +	dev_dbg(csid->camss->dev, "CSID%u: Stream %s PIX (dt=0x%x df=0x%x)\n",
> +		csid->id, enable ? "enable" : "disable", format->data_type, format->decode_format);

"... %sable ..." enable ? "en" : "dis"

Konrad

Re: [PATCH 3/3] media: qcom: camss: vfe-340: Support for PIX client

[Konrad Dybcio]

On 2/19/26 4:27 PM, Loic Poulain wrote:
> Add support for VFE PIX write engine, allowing to capture frames
> via the PIX video device (e.g. msm_vfe0_pix).
> 
> Tested on Agatti/Arduino-Uno-Q with:
> media-ctl -d /dev/media0 -l '"msm_csiphy0":1->"msm_csid0":0[1],"msm_csid0":4->"msm_vfe0_pix":0[1]'
> media-ctl -d /dev/media0 -V '"imx219 2-0010":0[fmt:SRGGB10_1X10/640x480 field:none]'
> media-ctl -d /dev/media0 -V '"msm_csiphy0":0[fmt:SRGGB10_1X10/640x480 field:none]'
> media-ctl -d /dev/media0 -V '"msm_csid0":0[fmt:SRGGB10_1X10/640x480 field:none]'
> media-ctl -d /dev/media0 -V '"msm_csid0":4[fmt:SRGGB10_1X10/640x480 field:none]'
> media-ctl -d /dev/media0 -V '"msm_vfe0_rdi0":0[fmt:SRGGB10_1X10/640x480 field:none]'
> media-ctl -d /dev/media0 -V '"msm_vfe0_pix":0[fmt:SRGGB10_1X10/640x480 field:none]'
> yavta -B capture-mplane --capture=30 -n 3 -f SRGGB10P -s 640x480 /dev/video3
> 
> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> ---

[...]

> +	if (client == TFE_CLI_BAYER) { /* PIX */
> +		writel_relaxed(pix->width + (pix->height << 16),
> +			       vfe->base + TFE_BUS_IMAGE_CFG_0(client));
> +		writel_relaxed(0u, vfe->base + TFE_BUS_IMAGE_CFG_1(client));
> +		writel_relaxed(stride, vfe->base + TFE_BUS_IMAGE_CFG_2(client));
> +		writel_relaxed(stride * pix->height, vfe->base + TFE_BUS_FRAME_INCR(client));
> +		writel_relaxed(vfe_packer_format(vfe, pix->pixelformat),
> +			       vfe->base + TFE_BUS_PACKER_CFG(client));
> +	} else { /* RDI */
> +		writel_relaxed(TFE_BUS_IMAGE_CFG_0_DEFAULT,
> +			       vfe->base + TFE_BUS_IMAGE_CFG_0(client));
> +		writel_relaxed(0u, vfe->base + TFE_BUS_IMAGE_CFG_1(client));

Were these default settings (prebumably "dont care" or "always max") valid
for RDI? Would the ones you set up for PIX work/make more sense indiscrimately?

Konrad

Re: [PATCH 1/3] media: qcom: camss: vfe-340: Proper client handling

[Bryan O'Donoghue]

On 19/02/2026 15:27, Loic Poulain wrote:
> We need to properly map camss WM index to our internal WM client
> instance. Today we're only support RDI interfaces with the RDI_WM
> macro, introduce a __wm_to_client helper to support any interface.
> 
> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> ---
>   .../media/platform/qcom/camss/camss-vfe-340.c | 94 +++++++++++--------
>   1 file changed, 53 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/camss/camss-vfe-340.c b/drivers/media/platform/qcom/camss/camss-vfe-340.c
> index 30d7630b3e8b..c6ea8b6216c2 100644
> --- a/drivers/media/platform/qcom/camss/camss-vfe-340.c
> +++ b/drivers/media/platform/qcom/camss/camss-vfe-340.c
> @@ -69,24 +69,19 @@
>   #define TFE_BUS_FRAMEDROP_CFG_0(c)			BUS_REG(0x238 + (c) * 0x100)
>   #define TFE_BUS_FRAMEDROP_CFG_1(c)			BUS_REG(0x23c + (c) * 0x100)
>   
> -/*
> - * TODO: differentiate the port id based on requested type of RDI, BHIST etc
> - *
> - * TFE write master IDs (clients)
> - *
> - * BAYER		0
> - * IDEAL_RAW		1
> - * STATS_TINTLESS_BG	2
> - * STATS_BHIST		3
> - * STATS_AWB_BG		4
> - * STATS_AEC_BG		5
> - * STATS_BAF		6
> - * RDI0			7
> - * RDI1			8
> - * RDI2			9
> - */
> -#define RDI_WM(n)		(7 + (n))
> -#define TFE_WM_NUM		10
> +enum tfe_client {
> +	TFE_CLI_BAYER,
> +	TFE_CLI_IDEAL_RAW,
> +	TFE_CLI_STATS_TINTLESS_BG,
> +	TFE_CLI_STATS_BHIST,
> +	TFE_CLI_STATS_AWB_BG,
> +	TFE_CLI_STATS_AEC_BG,
> +	TFE_CLI_STATS_BAF,
> +	TFE_CLI_RDI0,
> +	TFE_CLI_RDI1,
> +	TFE_CLI_RDI2,
> +	TFE_CLI_NUM
> +};
>   
>   enum tfe_iface {
>   	TFE_IFACE_PIX,
> @@ -108,6 +103,13 @@ enum tfe_subgroups {
>   	TFE_SUBGROUP_NUM
>   };
>   
> +static enum tfe_iface tfe_line_iface_map[VFE_LINE_NUM_MAX] = {
> +	[VFE_LINE_RDI0] = TFE_CLI_RDI0,
> +	[VFE_LINE_RDI1] = TFE_CLI_RDI1,
> +	[VFE_LINE_RDI2] = TFE_CLI_RDI2,
> +	[VFE_LINE_PIX] = TFE_CLI_BAYER,
> +};
> +
>   static enum tfe_iface tfe_line_iface_map[VFE_LINE_NUM_MAX] = {
>   	[VFE_LINE_RDI0] = TFE_IFACE_RDI0,
>   	[VFE_LINE_RDI1] = TFE_IFACE_RDI1,
> @@ -126,6 +128,16 @@ static enum vfe_line_id tfe_subgroup_line_map[TFE_SUBGROUP_NUM] = {
>   	[TFE_SUBGROUP_RDI2] = VFE_LINE_RDI2,
>   };
>   
> +static inline enum tfe_client  __wm_to_client(u8 wm)
redundant space

> +{
> +	if (wm >= ARRAY_SIZE(tfe_wm_client_map)) {
> +		pr_warn("VFE: Invalid WM%u\n", wm);
> +		return TFE_CLI_RDI0;
> +	}

Why or how would this happen ?

> +
> +	return tfe_wm_client_map[wm];
> +}
> +
>   static inline enum tfe_iface  __line_to_iface(enum vfe_line_id line_id)
>   {
>   	if (line_id <= VFE_LINE_NONE || line_id >= VFE_LINE_NUM_MAX) {
> @@ -209,10 +221,10 @@ static irqreturn_t vfe_isr(int irq, void *dev)
>   	status = readl_relaxed(vfe->base + TFE_BUS_OVERFLOW_STATUS);
>   	if (status) {
>   		writel_relaxed(status, vfe->base + TFE_BUS_STATUS_CLEAR);
> -		for (i = 0; i < TFE_WM_NUM; i++) {
> +		for (i = 0; i < TFE_CLI_NUM; i++) {
>   			if (status & BIT(i))
>   				dev_err_ratelimited(vfe->camss->dev,
> -						    "VFE%u: bus overflow for wm %u\n",
> +						    "VFE%u: bus overflow for client %u\n",
>   						    vfe->id, i);
>   		}
>   	}
> @@ -235,49 +247,49 @@ static void vfe_enable_irq(struct vfe_device *vfe)
>   	       TFE_BUS_IRQ_MASK_0_IMG_VIOL, vfe->base + TFE_BUS_IRQ_MASK_0);
>   }
>   
> -static void vfe_wm_update(struct vfe_device *vfe, u8 rdi, u32 addr,
> +static void vfe_wm_update(struct vfe_device *vfe, u8 wm, u32 addr,
>   			  struct vfe_line *line)
>   {
> -	u8 wm = RDI_WM(rdi);
> +	u8 client = __wm_to_client(wm);
>   
> -	writel_relaxed(addr, vfe->base + TFE_BUS_IMAGE_ADDR(wm));
> +	writel_relaxed(addr, vfe->base + TFE_BUS_IMAGE_ADDR(client));
>   }
>   
> -static void vfe_wm_start(struct vfe_device *vfe, u8 rdi, struct vfe_line *line)
> +static void vfe_wm_start(struct vfe_device *vfe, u8 wm, struct vfe_line *line)
>   {
>   	struct v4l2_pix_format_mplane *pix = &line->video_out.active_fmt.fmt.pix_mp;
>   	u32 stride = pix->plane_fmt[0].bytesperline;
> -	u8 wm = RDI_WM(rdi);
> +	u8 client = __wm_to_client(wm);
>   
>   	/* Configuration for plain RDI frames */
> -	writel_relaxed(TFE_BUS_IMAGE_CFG_0_DEFAULT, vfe->base + TFE_BUS_IMAGE_CFG_0(wm));
> -	writel_relaxed(0u, vfe->base + TFE_BUS_IMAGE_CFG_1(wm));
> -	writel_relaxed(TFE_BUS_IMAGE_CFG_2_DEFAULT, vfe->base + TFE_BUS_IMAGE_CFG_2(wm));
> -	writel_relaxed(stride * pix->height, vfe->base + TFE_BUS_FRAME_INCR(wm));
> -	writel_relaxed(TFE_BUS_PACKER_CFG_FMT_PLAIN64, vfe->base + TFE_BUS_PACKER_CFG(wm));
> +	writel_relaxed(TFE_BUS_IMAGE_CFG_0_DEFAULT, vfe->base + TFE_BUS_IMAGE_CFG_0(client));
> +	writel_relaxed(0u, vfe->base + TFE_BUS_IMAGE_CFG_1(client));
> +	writel_relaxed(TFE_BUS_IMAGE_CFG_2_DEFAULT, vfe->base + TFE_BUS_IMAGE_CFG_2(client));
> +	writel_relaxed(stride * pix->height, vfe->base + TFE_BUS_FRAME_INCR(client));
> +	writel_relaxed(TFE_BUS_PACKER_CFG_FMT_PLAIN64, vfe->base + TFE_BUS_PACKER_CFG(client));
>   
>   	/* No dropped frames, one irq per frame */
> -	writel_relaxed(0, vfe->base + TFE_BUS_FRAMEDROP_CFG_0(wm));
> -	writel_relaxed(1, vfe->base + TFE_BUS_FRAMEDROP_CFG_1(wm));
> -	writel_relaxed(0, vfe->base + TFE_BUS_IRQ_SUBSAMPLE_CFG_0(wm));
> -	writel_relaxed(1, vfe->base + TFE_BUS_IRQ_SUBSAMPLE_CFG_1(wm));
> +	writel_relaxed(0, vfe->base + TFE_BUS_FRAMEDROP_CFG_0(client));
> +	writel_relaxed(1, vfe->base + TFE_BUS_FRAMEDROP_CFG_1(client));
> +	writel_relaxed(0, vfe->base + TFE_BUS_IRQ_SUBSAMPLE_CFG_0(client));
> +	writel_relaxed(1, vfe->base + TFE_BUS_IRQ_SUBSAMPLE_CFG_1(client));
>   
>   	vfe_enable_irq(vfe);
>   
>   	writel(TFE_BUS_CLIENT_CFG_EN | TFE_BUS_CLIENT_CFG_MODE_FRAME,
> -	       vfe->base + TFE_BUS_CLIENT_CFG(wm));
> +	       vfe->base + TFE_BUS_CLIENT_CFG(client));
>   
> -	dev_dbg(vfe->camss->dev, "VFE%u: Started RDI%u width %u height %u stride %u\n",
> -		vfe->id, rdi, pix->width, pix->height, stride);
> +	dev_dbg(vfe->camss->dev, "VFE%u: Started client %u width %u height %u stride %u\n",
> +		vfe->id, client, pix->width, pix->height, client);
>   }
>   
> -static void vfe_wm_stop(struct vfe_device *vfe, u8 rdi)
> +static void vfe_wm_stop(struct vfe_device *vfe, u8 wm)
>   {
> -	u8 wm = RDI_WM(rdi);
> +	u8 client = __wm_to_client(wm);
>   
> -	writel(0, vfe->base + TFE_BUS_CLIENT_CFG(wm));
> +	writel(0, vfe->base + TFE_BUS_CLIENT_CFG(client));
>   
> -	dev_dbg(vfe->camss->dev, "VFE%u: Stopped RDI%u\n", vfe->id, rdi);
> +	dev_dbg(vfe->camss->dev, "VFE%u: Stopped client %u\n", vfe->id, client);
>   }
>   
>   static const struct camss_video_ops vfe_video_ops_520 = {

Otherwise seems fine.

Your series title got mangled somewhere BTW.

---
bod

Re: [PATCH 2/3] media: qcom: camss: csid-340: Enable PIX path support

[Bryan O'Donoghue]

On 19/02/2026 15:27, Loic Poulain wrote:
> Add support for CSID to PIX interface.
> 
> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> ---
>   .../platform/qcom/camss/camss-csid-340.c      | 85 ++++++++++++-------
>   1 file changed, 55 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/camss/camss-csid-340.c b/drivers/media/platform/qcom/camss/camss-csid-340.c
> index 2b50f9b96a34..f7b4cb054c55 100644
> --- a/drivers/media/platform/qcom/camss/camss-csid-340.c
> +++ b/drivers/media/platform/qcom/camss/camss-csid-340.c
> @@ -41,19 +41,24 @@
>   #define		CSI2_RX_CFG1_MISR_EN			BIT(6)
>   #define		CSI2_RX_CFG1_CGC_MODE			BIT(7)
>   
> -#define CSID_RDI_CFG0(rdi)					(0x300 + 0x100 * (rdi))
> -#define		CSID_RDI_CFG0_BYTE_CNTR_EN		BIT(0)
> -#define		CSID_RDI_CFG0_TIMESTAMP_EN		BIT(1)
> -#define		CSID_RDI_CFG0_DECODE_FORMAT_MASK	GENMASK(15, 12)
> -#define		CSID_RDI_CFG0_DECODE_FORMAT_NOP		CSID_RDI_CFG0_DECODE_FORMAT_MASK
> -#define		CSID_RDI_CFG0_DT_MASK			GENMASK(21, 16)
> -#define		CSID_RDI_CFG0_VC_MASK			GENMASK(23, 22)
> -#define		CSID_RDI_CFG0_DTID_MASK			GENMASK(28, 27)
> -#define		CSID_RDI_CFG0_ENABLE			BIT(31)
> -
> -#define CSID_RDI_CTRL(rdi)					(0x308 + 0x100 * (rdi))
> -#define CSID_RDI_CTRL_HALT_AT_FRAME_BOUNDARY		0
> -#define CSID_RDI_CTRL_RESUME_AT_FRAME_BOUNDARY		1
> +#define CSID_CFG0(iface)					(0x300 + 0x100 * (iface))
> +#define		CSID_CFG0_BYTE_CNTR_EN			BIT(0)
> +#define		CSID_CFG0_TIMESTAMP_EN			BIT(1)
> +#define		CSID_CFG0_DECODE_FORMAT_MASK		GENMASK(15, 12)
> +#define		CSID_CFG0_DECODE_FORMAT_NOP		CSID_CFG0_DECODE_FORMAT_MASK
> +#define		CSID_CFG0_DT_MASK			GENMASK(21, 16)
> +#define		CSID_CFG0_VC_MASK			GENMASK(23, 22)
> +#define		CSID_CFG0_DTID_MASK			GENMASK(28, 27)
> +#define		CSID_CFG0_ENABLE			BIT(31)
> +
> +#define CSID_CTRL(iface)					(0x308 + 0x100 * (iface))
> +#define CSID_CTRL_HALT_AT_FRAME_BOUNDARY		0
> +#define CSID_CTRL_RESUME_AT_FRAME_BOUNDARY		1
> +
> +#define CSID_MAX_RDI_SRC_STREAMS	(MSM_CSID_MAX_SRC_STREAMS - 1)
> +#define CSID_PIX_SRC_STREAMS		CSID_MAX_RDI_SRC_STREAMS

This I think is fairly common - PIX is ~ always the last one of four - 
should probably be in the shared csid.h
> +
> +#define CSID_IFACE_PIX	-1

-1 ?

>   
>   static void __csid_configure_rx(struct csid_device *csid, struct csid_phy_config *phy)
>   {
> @@ -69,11 +74,6 @@ static void __csid_configure_rx(struct csid_device *csid, struct csid_phy_config
>   	writel_relaxed(val, csid->base + CSID_CSI2_RX_CFG1);
>   }
>   
> -static void __csid_ctrl_rdi(struct csid_device *csid, int enable, u8 rdi)
> -{
> -	writel_relaxed(!!enable, csid->base + CSID_RDI_CTRL(rdi));
> -}
> -
>   static void __csid_configure_rdi_stream(struct csid_device *csid, u8 enable, u8 vc)
>   {
>   	struct v4l2_mbus_framefmt *input_format = &csid->fmt[MSM_CSID_PAD_FIRST_SRC + vc];
> @@ -88,7 +88,7 @@ static void __csid_configure_rdi_stream(struct csid_device *csid, u8 enable, u8
>   	 * the four least significant bits of the five bit VC
>   	 * bitfield to generate an internal CID value.
>   	 *
> -	 * CSID_RDI_CFG0(vc)
> +	 * CSID_CFG0(vc)
>   	 * DT_ID : 28:27
>   	 * VC    : 26:22
>   	 * DT    : 21:16
> @@ -97,18 +97,40 @@ static void __csid_configure_rdi_stream(struct csid_device *csid, u8 enable, u8
>   	 */
>   	dt_id = vc & 0x03;
>   
> -	val = CSID_RDI_CFG0_DECODE_FORMAT_NOP; /* only for RDI path */
> -	val |= FIELD_PREP(CSID_RDI_CFG0_DT_MASK, format->data_type);
> -	val |= FIELD_PREP(CSID_RDI_CFG0_VC_MASK, vc);
> -	val |= FIELD_PREP(CSID_RDI_CFG0_DTID_MASK, dt_id);
> +	val = CSID_CFG0_DECODE_FORMAT_NOP; /* only for RDI path */
> +	val |= FIELD_PREP(CSID_CFG0_DT_MASK, format->data_type);
> +	val |= FIELD_PREP(CSID_CFG0_VC_MASK, vc);
> +	val |= FIELD_PREP(CSID_CFG0_DTID_MASK, dt_id);
>   
>   	if (enable)
> -		val |= CSID_RDI_CFG0_ENABLE;
> +		val |= CSID_CFG0_ENABLE;
>   
> -	dev_dbg(csid->camss->dev, "CSID%u: Stream %s (dt:0x%x vc=%u)\n",
> +	dev_dbg(csid->camss->dev, "CSID%u: Stream %s RDI (dt:0x%x vc=%u)\n",
>   		csid->id, enable ? "enable" : "disable", format->data_type, vc);
>   
> -	writel_relaxed(val, csid->base + CSID_RDI_CFG0(vc));
> +	writel_relaxed(val, csid->base + CSID_CFG0(vc));
> +	writel_relaxed(!!enable, csid->base + CSID_CTRL(vc));
> +}
> +
> +static void __csid_configure_pix_stream(struct csid_device *csid, u8 enable)
> +{
> +	struct v4l2_mbus_framefmt *input_format = &csid->fmt[MSM_CSID_PADS_NUM - 1];
> +	const struct csid_format_info *format = csid_get_fmt_entry(csid->res->formats->formats,
> +								   csid->res->formats->nformats,
> +								   input_format->code);
> +	u32 val;
> +
> +	val = FIELD_PREP(CSID_CFG0_DECODE_FORMAT_MASK, format->decode_format);
> +	val |= FIELD_PREP(CSID_CFG0_DT_MASK, format->data_type);
> +
> +	if (enable)
> +		val |= CSID_CFG0_ENABLE;
> +
> +	dev_dbg(csid->camss->dev, "CSID%u: Stream %s PIX (dt=0x%x df=0x%x)\n",
> +		csid->id, enable ? "enable" : "disable", format->data_type, format->decode_format);
> +
> +	writel_relaxed(val, csid->base + CSID_CFG0(CSID_IFACE_PIX));
> +	writel_relaxed(!!enable, csid->base + CSID_CTRL(CSID_IFACE_PIX));
>   }
>   
>   static void csid_configure_stream(struct csid_device *csid, u8 enable)
> @@ -117,12 +139,15 @@ static void csid_configure_stream(struct csid_device *csid, u8 enable)
>   
>   	__csid_configure_rx(csid, &csid->phy);
>   
> -	for (i = 0; i < MSM_CSID_MAX_SRC_STREAMS; i++) {
> -		if (csid->phy.en_vc & BIT(i)) {
> +	/* RDIs */
> +	for (i = 0; i < CSID_MAX_RDI_SRC_STREAMS; i++) {
> +		if (csid->phy.en_vc & BIT(i))
>   			__csid_configure_rdi_stream(csid, enable, i);
> -			__csid_ctrl_rdi(csid, enable, i);
> -		}
>   	}
> +
> +	/* PIX */
> +	if (csid->phy.en_vc & BIT(CSID_PIX_SRC_STREAMS))
> +		__csid_configure_pix_stream(csid, enable);
>   }
>   
>   static int csid_reset(struct csid_device *csid)

Re: [PATCH 2/3] media: qcom: camss: csid-340: Enable PIX path support

[Loic Poulain]

On Thu, Feb 19, 2026 at 4:43 PM Konrad Dybcio
<konrad.dybcio@oss.qualcomm.com> wrote:
>
> On 2/19/26 4:27 PM, Loic Poulain wrote:
> > Add support for CSID to PIX interface.
> >
> > Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> > ---
>
> [...]
>
> > -#define CSID_RDI_CFG0(rdi)                                   (0x300 + 0x100 * (rdi))
> > -#define              CSID_RDI_CFG0_BYTE_CNTR_EN              BIT(0)
> > -#define              CSID_RDI_CFG0_TIMESTAMP_EN              BIT(1)
> > -#define              CSID_RDI_CFG0_DECODE_FORMAT_MASK        GENMASK(15, 12)
> > -#define              CSID_RDI_CFG0_DECODE_FORMAT_NOP         CSID_RDI_CFG0_DECODE_FORMAT_MASK
> > -#define              CSID_RDI_CFG0_DT_MASK                   GENMASK(21, 16)
> > -#define              CSID_RDI_CFG0_VC_MASK                   GENMASK(23, 22)
> > -#define              CSID_RDI_CFG0_DTID_MASK                 GENMASK(28, 27)
> > -#define              CSID_RDI_CFG0_ENABLE                    BIT(31)
>
> Much of this patch is renaming, could you split this up, so it's more
> obvious what it actually introduces?

Sure.

>
> [...]
>
> > +static void __csid_configure_pix_stream(struct csid_device *csid, u8 enable)
>
> Perhaps that's yak shaving, but I don't think enable should be non-binary

Indeed.

>
> > +{
> > +     struct v4l2_mbus_framefmt *input_format = &csid->fmt[MSM_CSID_PADS_NUM - 1];
> > +     const struct csid_format_info *format = csid_get_fmt_entry(csid->res->formats->formats,
> > +                                                                csid->res->formats->nformats,
> > +                                                                input_format->code);
> > +     u32 val;
> > +
> > +     val = FIELD_PREP(CSID_CFG0_DECODE_FORMAT_MASK, format->decode_format);
> > +     val |= FIELD_PREP(CSID_CFG0_DT_MASK, format->data_type);
>
> no vc/dt_id?

In CAMSS, each CSI‑2 Virtual Channel (VC) is statically tied to a
specific RDI instance: VC0 → RDI0, VC1 → RDI1, VC2 → RDI2. VC0
typically carries the main image stream, while the higher-numbered
channels are *usually* used for embedded or metadata streams that
don't require pixel processing. This is why we select VC0 for the PIX
path, as it reliably covers the vast majority of use cases.

Achieving more flexible or dynamic VC selection would require
decoupling the current VC-to‑RDI association and restructuring how the
CAMSS pipeline represents and routes CSI‑2 streams. This would be a
non‑trivial architectural change, touching both the CSID routing logic
and how VFE input paths are modeled.

The DT_ID is a per‑VC, 2‑bit identifier that we currently assign to
uniquely represent or map a given data type. At the moment, all CSID
drivers simply use DT_ID = VC_ID, meaning each RDI/VC instance
implicitly handles a different data type. For the PIX path, we keep
DT_ID = 0, matching the RDI0/VC0 stream. Ideally, DT_ID allocation
should be based on the actual data type rather than tied to the VC
number, but that belongs to a separate series.


> > +
> > +     if (enable)
> > +             val |= CSID_CFG0_ENABLE;
> > +
> > +     dev_dbg(csid->camss->dev, "CSID%u: Stream %s PIX (dt=0x%x df=0x%x)\n",
> > +             csid->id, enable ? "enable" : "disable", format->data_type, format->decode_format);
>
> "... %sable ..." enable ? "en" : "dis"
>
> Konrad

Re: [PATCH 2/3] media: qcom: camss: csid-340: Enable PIX path support

[Loic Poulain]

Hi Bryan,

On Thu, Feb 19, 2026 at 4:51 PM Bryan O'Donoghue
<bryan.odonoghue@linaro.org> wrote:
>
> On 19/02/2026 15:27, Loic Poulain wrote:
> > Add support for CSID to PIX interface.
> >
> > Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> > ---
> >   .../platform/qcom/camss/camss-csid-340.c      | 85 ++++++++++++-------
> >   1 file changed, 55 insertions(+), 30 deletions(-)
> >
> > diff --git a/drivers/media/platform/qcom/camss/camss-csid-340.c b/drivers/media/platform/qcom/camss/camss-csid-340.c
> > index 2b50f9b96a34..f7b4cb054c55 100644
> > --- a/drivers/media/platform/qcom/camss/camss-csid-340.c
> > +++ b/drivers/media/platform/qcom/camss/camss-csid-340.c
> > @@ -41,19 +41,24 @@
> >   #define             CSI2_RX_CFG1_MISR_EN                    BIT(6)
> >   #define             CSI2_RX_CFG1_CGC_MODE                   BIT(7)
> >
> > -#define CSID_RDI_CFG0(rdi)                                   (0x300 + 0x100 * (rdi))
> > -#define              CSID_RDI_CFG0_BYTE_CNTR_EN              BIT(0)
> > -#define              CSID_RDI_CFG0_TIMESTAMP_EN              BIT(1)
> > -#define              CSID_RDI_CFG0_DECODE_FORMAT_MASK        GENMASK(15, 12)
> > -#define              CSID_RDI_CFG0_DECODE_FORMAT_NOP         CSID_RDI_CFG0_DECODE_FORMAT_MASK
> > -#define              CSID_RDI_CFG0_DT_MASK                   GENMASK(21, 16)
> > -#define              CSID_RDI_CFG0_VC_MASK                   GENMASK(23, 22)
> > -#define              CSID_RDI_CFG0_DTID_MASK                 GENMASK(28, 27)
> > -#define              CSID_RDI_CFG0_ENABLE                    BIT(31)
> > -
> > -#define CSID_RDI_CTRL(rdi)                                   (0x308 + 0x100 * (rdi))
> > -#define CSID_RDI_CTRL_HALT_AT_FRAME_BOUNDARY         0
> > -#define CSID_RDI_CTRL_RESUME_AT_FRAME_BOUNDARY               1
> > +#define CSID_CFG0(iface)                                     (0x300 + 0x100 * (iface))
> > +#define              CSID_CFG0_BYTE_CNTR_EN                  BIT(0)
> > +#define              CSID_CFG0_TIMESTAMP_EN                  BIT(1)
> > +#define              CSID_CFG0_DECODE_FORMAT_MASK            GENMASK(15, 12)
> > +#define              CSID_CFG0_DECODE_FORMAT_NOP             CSID_CFG0_DECODE_FORMAT_MASK
> > +#define              CSID_CFG0_DT_MASK                       GENMASK(21, 16)
> > +#define              CSID_CFG0_VC_MASK                       GENMASK(23, 22)
> > +#define              CSID_CFG0_DTID_MASK                     GENMASK(28, 27)
> > +#define              CSID_CFG0_ENABLE                        BIT(31)
> > +
> > +#define CSID_CTRL(iface)                                     (0x308 + 0x100 * (iface))
> > +#define CSID_CTRL_HALT_AT_FRAME_BOUNDARY             0
> > +#define CSID_CTRL_RESUME_AT_FRAME_BOUNDARY           1
> > +
> > +#define CSID_MAX_RDI_SRC_STREAMS     (MSM_CSID_MAX_SRC_STREAMS - 1)
> > +#define CSID_PIX_SRC_STREAMS         CSID_MAX_RDI_SRC_STREAMS
>
> This I think is fairly common - PIX is ~ always the last one of four -
> should probably be in the shared csid.h

Yes, that makes sense.

> > +
> > +#define CSID_IFACE_PIX       -1
>
> -1 ?

In the CSID register map, the block offsets are as follows:
IPP: 0x200
RDI0: 0x300
RDI1: 0x400
RDI2: 0x500

However, CAMSS uses a different internal interface numbering:
0: RDI0
1: RDI1
2: RDI2
3: PIX

To keep helper macros such as CSID_CFG0(iface) simple and avoid
'complex' conditionals or mapping, we maintain a direct mapping from
CAMSS iface to CSID register offsets. Only the PIX path requires
special treatment (-1 => 0x300 + -1 * 0x100).

Regards,
Loic

Re: [PATCH 2/3] media: qcom: camss: csid-340: Enable PIX path support

[Bryan O'Donoghue]

On 19/02/2026 20:19, Loic Poulain wrote:
>>> +#define CSID_IFACE_PIX       -1
>> -1 ?
> In the CSID register map, the block offsets are as follows:
> IPP: 0x200
> RDI0: 0x300
> RDI1: 0x400
> RDI2: 0x500

You use that define in two places ?

I think the -1 indexing is a bit non-intuitive. Maybe make a special 
accessor for those two special cases you have instead of a negative index.

---
bod

Re: [PATCH 2/3] media: qcom: camss: csid-340: Enable PIX path support

[Konrad Dybcio]

On 2/19/26 8:49 PM, Loic Poulain wrote:
> On Thu, Feb 19, 2026 at 4:43 PM Konrad Dybcio
> <konrad.dybcio@oss.qualcomm.com> wrote:
>>
>> On 2/19/26 4:27 PM, Loic Poulain wrote:
>>> Add support for CSID to PIX interface.
>>>
>>> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
>>> ---

[...]

>>> +{
>>> +     struct v4l2_mbus_framefmt *input_format = &csid->fmt[MSM_CSID_PADS_NUM - 1];
>>> +     const struct csid_format_info *format = csid_get_fmt_entry(csid->res->formats->formats,
>>> +                                                                csid->res->formats->nformats,
>>> +                                                                input_format->code);
>>> +     u32 val;
>>> +
>>> +     val = FIELD_PREP(CSID_CFG0_DECODE_FORMAT_MASK, format->decode_format);
>>> +     val |= FIELD_PREP(CSID_CFG0_DT_MASK, format->data_type);
>>
>> no vc/dt_id?
> 
> In CAMSS, each CSI‑2 Virtual Channel (VC) is statically tied to a
> specific RDI instance: VC0 → RDI0, VC1 → RDI1, VC2 → RDI2. VC0
> typically carries the main image stream, while the higher-numbered
> channels are *usually* used for embedded or metadata streams that
> don't require pixel processing. This is why we select VC0 for the PIX
> path, as it reliably covers the vast majority of use cases.
> 
> Achieving more flexible or dynamic VC selection would require
> decoupling the current VC-to‑RDI association and restructuring how the
> CAMSS pipeline represents and routes CSI‑2 streams. This would be a
> non‑trivial architectural change, touching both the CSID routing logic
> and how VFE input paths are modeled.
> 
> The DT_ID is a per‑VC, 2‑bit identifier that we currently assign to
> uniquely represent or map a given data type. At the moment, all CSID
> drivers simply use DT_ID = VC_ID, meaning each RDI/VC instance
> implicitly handles a different data type. For the PIX path, we keep
> DT_ID = 0, matching the RDI0/VC0 stream. Ideally, DT_ID allocation
> should be based on the actual data type rather than tied to the VC
> number, but that belongs to a separate series.

OK, let's maybe write the vc and dt_id == 0 explicitly then

Konrad

Re: [PATCH 3/3] media: qcom: camss: vfe-340: Support for PIX client

[Dmitry Baryshkov]

On Thu, Feb 19, 2026 at 04:27:37PM +0100, Loic Poulain wrote:
> Add support for VFE PIX write engine, allowing to capture frames
> via the PIX video device (e.g. msm_vfe0_pix).

And nothing specifies, what is PIX.

> 
> Tested on Agatti/Arduino-Uno-Q with:
> media-ctl -d /dev/media0 -l '"msm_csiphy0":1->"msm_csid0":0[1],"msm_csid0":4->"msm_vfe0_pix":0[1]'
> media-ctl -d /dev/media0 -V '"imx219 2-0010":0[fmt:SRGGB10_1X10/640x480 field:none]'
> media-ctl -d /dev/media0 -V '"msm_csiphy0":0[fmt:SRGGB10_1X10/640x480 field:none]'
> media-ctl -d /dev/media0 -V '"msm_csid0":0[fmt:SRGGB10_1X10/640x480 field:none]'
> media-ctl -d /dev/media0 -V '"msm_csid0":4[fmt:SRGGB10_1X10/640x480 field:none]'
> media-ctl -d /dev/media0 -V '"msm_vfe0_rdi0":0[fmt:SRGGB10_1X10/640x480 field:none]'

If we are using PIX, why do we need to setup RDI0 too? I thought that
they are two alternatives.

> media-ctl -d /dev/media0 -V '"msm_vfe0_pix":0[fmt:SRGGB10_1X10/640x480 field:none]'
> yavta -B capture-mplane --capture=30 -n 3 -f SRGGB10P -s 640x480 /dev/video3
> 
> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> ---
>  .../media/platform/qcom/camss/camss-vfe-340.c | 64 +++++++++++++++----
>  1 file changed, 53 insertions(+), 11 deletions(-)
> 

-- 
With best wishes
Dmitry

Re: [PATCH 3/3] media: qcom: camss: vfe-340: Support for PIX client

[Loic Poulain]

Hi Konrad,

On Thu, Feb 19, 2026 at 4:46 PM Konrad Dybcio
<konrad.dybcio@oss.qualcomm.com> wrote:
>
> On 2/19/26 4:27 PM, Loic Poulain wrote:
> > Add support for VFE PIX write engine, allowing to capture frames
> > via the PIX video device (e.g. msm_vfe0_pix).
> >
> > Tested on Agatti/Arduino-Uno-Q with:
> > media-ctl -d /dev/media0 -l '"msm_csiphy0":1->"msm_csid0":0[1],"msm_csid0":4->"msm_vfe0_pix":0[1]'
> > media-ctl -d /dev/media0 -V '"imx219 2-0010":0[fmt:SRGGB10_1X10/640x480 field:none]'
> > media-ctl -d /dev/media0 -V '"msm_csiphy0":0[fmt:SRGGB10_1X10/640x480 field:none]'
> > media-ctl -d /dev/media0 -V '"msm_csid0":0[fmt:SRGGB10_1X10/640x480 field:none]'
> > media-ctl -d /dev/media0 -V '"msm_csid0":4[fmt:SRGGB10_1X10/640x480 field:none]'
> > media-ctl -d /dev/media0 -V '"msm_vfe0_rdi0":0[fmt:SRGGB10_1X10/640x480 field:none]'
> > media-ctl -d /dev/media0 -V '"msm_vfe0_pix":0[fmt:SRGGB10_1X10/640x480 field:none]'
> > yavta -B capture-mplane --capture=30 -n 3 -f SRGGB10P -s 640x480 /dev/video3
> >
> > Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> > ---
>
> [...]
>
> > +     if (client == TFE_CLI_BAYER) { /* PIX */
> > +             writel_relaxed(pix->width + (pix->height << 16),
> > +                            vfe->base + TFE_BUS_IMAGE_CFG_0(client));
> > +             writel_relaxed(0u, vfe->base + TFE_BUS_IMAGE_CFG_1(client));
> > +             writel_relaxed(stride, vfe->base + TFE_BUS_IMAGE_CFG_2(client));
> > +             writel_relaxed(stride * pix->height, vfe->base + TFE_BUS_FRAME_INCR(client));
> > +             writel_relaxed(vfe_packer_format(vfe, pix->pixelformat),
> > +                            vfe->base + TFE_BUS_PACKER_CFG(client));
> > +     } else { /* RDI */
> > +             writel_relaxed(TFE_BUS_IMAGE_CFG_0_DEFAULT,
> > +                            vfe->base + TFE_BUS_IMAGE_CFG_0(client));
> > +             writel_relaxed(0u, vfe->base + TFE_BUS_IMAGE_CFG_1(client));
>
> Were these default settings (prebumably "dont care" or "always max") valid
> for RDI? Would the ones you set up for PIX work/make more sense indiscrimately?

That's a good question, the configuration above is the typical setup
used for RDI paths, and it matches what other drivers generally do...
The RDI path traditionally uses very loose or “don’t care” format
settings, and simply 'raw' dumps everything between the CSI Frame
Start and Frame End packets as an opaque byte sequence.
On the PIX path, however, we expect at least some level of processing
(even minimal), such as cropping, plus later statistics... For that,
we need a 'description' of the frame format.

So theoretically we could describe the frame more precisely for RDI as
well, but that would diverge from what existing drivers typically do,
and the benefit is limited since RDI does not really consume most of
that information.
Also, I think we may end up dumping content that has no line
delimiters at all via RDI, such as sensor‑generated metadata blocks
that don’t follow the usual LS/LE framing.

Regards,
Loic

Re: [PATCH 3/3] media: qcom: camss: vfe-340: Support for PIX client

[Loic Poulain]

On Sat, Feb 21, 2026 at 9:47 AM Dmitry Baryshkov
<dmitry.baryshkov@oss.qualcomm.com> wrote:
>
> On Thu, Feb 19, 2026 at 04:27:37PM +0100, Loic Poulain wrote:
> > Add support for VFE PIX write engine, allowing to capture frames
> > via the PIX video device (e.g. msm_vfe0_pix).
>
> And nothing specifies, what is PIX.

I will describe and explain why we want PIX in V2.

>
> >
> > Tested on Agatti/Arduino-Uno-Q with:
> > media-ctl -d /dev/media0 -l '"msm_csiphy0":1->"msm_csid0":0[1],"msm_csid0":4->"msm_vfe0_pix":0[1]'
> > media-ctl -d /dev/media0 -V '"imx219 2-0010":0[fmt:SRGGB10_1X10/640x480 field:none]'
> > media-ctl -d /dev/media0 -V '"msm_csiphy0":0[fmt:SRGGB10_1X10/640x480 field:none]'
> > media-ctl -d /dev/media0 -V '"msm_csid0":0[fmt:SRGGB10_1X10/640x480 field:none]'
> > media-ctl -d /dev/media0 -V '"msm_csid0":4[fmt:SRGGB10_1X10/640x480 field:none]'
> > media-ctl -d /dev/media0 -V '"msm_vfe0_rdi0":0[fmt:SRGGB10_1X10/640x480 field:none]'
>
> If we are using PIX, why do we need to setup RDI0 too? I thought that
> they are two alternatives.

Good catch, that was just a leftover from my test script. We don’t
need to configure the RDI subdevice(s) when using the PIX path.

Regards,
Loic

Re: [PATCH 3/3] media: qcom: camss: vfe-340: Support for PIX client

[Konrad Dybcio]

On 2/23/26 3:46 PM, Loic Poulain wrote:
> Hi Konrad,
> 
> On Thu, Feb 19, 2026 at 4:46 PM Konrad Dybcio
> <konrad.dybcio@oss.qualcomm.com> wrote:
>>
>> On 2/19/26 4:27 PM, Loic Poulain wrote:
>>> Add support for VFE PIX write engine, allowing to capture frames
>>> via the PIX video device (e.g. msm_vfe0_pix).
>>>
>>> Tested on Agatti/Arduino-Uno-Q with:
>>> media-ctl -d /dev/media0 -l '"msm_csiphy0":1->"msm_csid0":0[1],"msm_csid0":4->"msm_vfe0_pix":0[1]'
>>> media-ctl -d /dev/media0 -V '"imx219 2-0010":0[fmt:SRGGB10_1X10/640x480 field:none]'
>>> media-ctl -d /dev/media0 -V '"msm_csiphy0":0[fmt:SRGGB10_1X10/640x480 field:none]'
>>> media-ctl -d /dev/media0 -V '"msm_csid0":0[fmt:SRGGB10_1X10/640x480 field:none]'
>>> media-ctl -d /dev/media0 -V '"msm_csid0":4[fmt:SRGGB10_1X10/640x480 field:none]'
>>> media-ctl -d /dev/media0 -V '"msm_vfe0_rdi0":0[fmt:SRGGB10_1X10/640x480 field:none]'
>>> media-ctl -d /dev/media0 -V '"msm_vfe0_pix":0[fmt:SRGGB10_1X10/640x480 field:none]'
>>> yavta -B capture-mplane --capture=30 -n 3 -f SRGGB10P -s 640x480 /dev/video3
>>>
>>> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
>>> ---
>>
>> [...]
>>
>>> +     if (client == TFE_CLI_BAYER) { /* PIX */
>>> +             writel_relaxed(pix->width + (pix->height << 16),
>>> +                            vfe->base + TFE_BUS_IMAGE_CFG_0(client));
>>> +             writel_relaxed(0u, vfe->base + TFE_BUS_IMAGE_CFG_1(client));
>>> +             writel_relaxed(stride, vfe->base + TFE_BUS_IMAGE_CFG_2(client));
>>> +             writel_relaxed(stride * pix->height, vfe->base + TFE_BUS_FRAME_INCR(client));
>>> +             writel_relaxed(vfe_packer_format(vfe, pix->pixelformat),
>>> +                            vfe->base + TFE_BUS_PACKER_CFG(client));
>>> +     } else { /* RDI */
>>> +             writel_relaxed(TFE_BUS_IMAGE_CFG_0_DEFAULT,
>>> +                            vfe->base + TFE_BUS_IMAGE_CFG_0(client));
>>> +             writel_relaxed(0u, vfe->base + TFE_BUS_IMAGE_CFG_1(client));
>>
>> Were these default settings (prebumably "dont care" or "always max") valid
>> for RDI? Would the ones you set up for PIX work/make more sense indiscrimately?
> 
> That's a good question, the configuration above is the typical setup
> used for RDI paths, and it matches what other drivers generally do...
> The RDI path traditionally uses very loose or “don’t care” format
> settings, and simply 'raw' dumps everything between the CSI Frame
> Start and Frame End packets as an opaque byte sequence.
> On the PIX path, however, we expect at least some level of processing
> (even minimal), such as cropping, plus later statistics... For that,
> we need a 'description' of the frame format.
> 
> So theoretically we could describe the frame more precisely for RDI as
> well, but that would diverge from what existing drivers typically do,
> and the benefit is limited since RDI does not really consume most of
> that information.
> Also, I think we may end up dumping content that has no line
> delimiters at all via RDI, such as sensor‑generated metadata blocks
> that don’t follow the usual LS/LE framing.

That sounds convincing. Would such data be the hw-design-intended usecase
for RDI (with the pixel data going through the full pipeline), or is that
just a possibility you're mentioning?

Konrad

Re: [PATCH 1/3] media: qcom: camss: vfe-340: Proper client handling

[Loic Poulain]

On Thu, Feb 19, 2026 at 4:47 PM Bryan O'Donoghue
<bryan.odonoghue@linaro.org> wrote:
>
> On 19/02/2026 15:27, Loic Poulain wrote:
> > We need to properly map camss WM index to our internal WM client
> > instance. Today we're only support RDI interfaces with the RDI_WM
> > macro, introduce a __wm_to_client helper to support any interface.
> >
> > Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> > ---
> >   .../media/platform/qcom/camss/camss-vfe-340.c | 94 +++++++++++--------
> >   1 file changed, 53 insertions(+), 41 deletions(-)
> >
> > diff --git a/drivers/media/platform/qcom/camss/camss-vfe-340.c b/drivers/media/platform/qcom/camss/camss-vfe-340.c
> > index 30d7630b3e8b..c6ea8b6216c2 100644
> > --- a/drivers/media/platform/qcom/camss/camss-vfe-340.c
> > +++ b/drivers/media/platform/qcom/camss/camss-vfe-340.c
> > @@ -69,24 +69,19 @@
> >   #define TFE_BUS_FRAMEDROP_CFG_0(c)                  BUS_REG(0x238 + (c) * 0x100)
> >   #define TFE_BUS_FRAMEDROP_CFG_1(c)                  BUS_REG(0x23c + (c) * 0x100)
> >
> > -/*
> > - * TODO: differentiate the port id based on requested type of RDI, BHIST etc
> > - *
> > - * TFE write master IDs (clients)
> > - *
> > - * BAYER             0
> > - * IDEAL_RAW         1
> > - * STATS_TINTLESS_BG 2
> > - * STATS_BHIST               3
> > - * STATS_AWB_BG              4
> > - * STATS_AEC_BG              5
> > - * STATS_BAF         6
> > - * RDI0                      7
> > - * RDI1                      8
> > - * RDI2                      9
> > - */
> > -#define RDI_WM(n)            (7 + (n))
> > -#define TFE_WM_NUM           10
> > +enum tfe_client {
> > +     TFE_CLI_BAYER,
> > +     TFE_CLI_IDEAL_RAW,
> > +     TFE_CLI_STATS_TINTLESS_BG,
> > +     TFE_CLI_STATS_BHIST,
> > +     TFE_CLI_STATS_AWB_BG,
> > +     TFE_CLI_STATS_AEC_BG,
> > +     TFE_CLI_STATS_BAF,
> > +     TFE_CLI_RDI0,
> > +     TFE_CLI_RDI1,
> > +     TFE_CLI_RDI2,
> > +     TFE_CLI_NUM
> > +};
> >
> >   enum tfe_iface {
> >       TFE_IFACE_PIX,
> > @@ -108,6 +103,13 @@ enum tfe_subgroups {
> >       TFE_SUBGROUP_NUM
> >   };
> >
> > +static enum tfe_iface tfe_line_iface_map[VFE_LINE_NUM_MAX] = {
> > +     [VFE_LINE_RDI0] = TFE_CLI_RDI0,
> > +     [VFE_LINE_RDI1] = TFE_CLI_RDI1,
> > +     [VFE_LINE_RDI2] = TFE_CLI_RDI2,
> > +     [VFE_LINE_PIX] = TFE_CLI_BAYER,
> > +};
> > +
> >   static enum tfe_iface tfe_line_iface_map[VFE_LINE_NUM_MAX] = {
> >       [VFE_LINE_RDI0] = TFE_IFACE_RDI0,
> >       [VFE_LINE_RDI1] = TFE_IFACE_RDI1,
> > @@ -126,6 +128,16 @@ static enum vfe_line_id tfe_subgroup_line_map[TFE_SUBGROUP_NUM] = {
> >       [TFE_SUBGROUP_RDI2] = VFE_LINE_RDI2,
> >   };
> >
> > +static inline enum tfe_client  __wm_to_client(u8 wm)
> redundant space
>
> > +{
> > +     if (wm >= ARRAY_SIZE(tfe_wm_client_map)) {
> > +             pr_warn("VFE: Invalid WM%u\n", wm);
> > +             return TFE_CLI_RDI0;
> > +     }
>
> Why or how would this happen ?

This is just a paranoid/defensive boundary check as this value comes
from outside this driver (camss-core), but it should never be
triggered.

>
> > +
> > +     return tfe_wm_client_map[wm];
> > +}
> > +
> >   static inline enum tfe_iface  __line_to_iface(enum vfe_line_id line_id)
> >   {
> >       if (line_id <= VFE_LINE_NONE || line_id >= VFE_LINE_NUM_MAX) {
> > @@ -209,10 +221,10 @@ static irqreturn_t vfe_isr(int irq, void *dev)
> >       status = readl_relaxed(vfe->base + TFE_BUS_OVERFLOW_STATUS);
> >       if (status) {
> >               writel_relaxed(status, vfe->base + TFE_BUS_STATUS_CLEAR);
> > -             for (i = 0; i < TFE_WM_NUM; i++) {
> > +             for (i = 0; i < TFE_CLI_NUM; i++) {
> >                       if (status & BIT(i))
> >                               dev_err_ratelimited(vfe->camss->dev,
> > -                                                 "VFE%u: bus overflow for wm %u\n",
> > +                                                 "VFE%u: bus overflow for client %u\n",
> >                                                   vfe->id, i);
> >               }
> >       }
> > @@ -235,49 +247,49 @@ static void vfe_enable_irq(struct vfe_device *vfe)
> >              TFE_BUS_IRQ_MASK_0_IMG_VIOL, vfe->base + TFE_BUS_IRQ_MASK_0);
> >   }
> >
> > -static void vfe_wm_update(struct vfe_device *vfe, u8 rdi, u32 addr,
> > +static void vfe_wm_update(struct vfe_device *vfe, u8 wm, u32 addr,
> >                         struct vfe_line *line)
> >   {
> > -     u8 wm = RDI_WM(rdi);
> > +     u8 client = __wm_to_client(wm);
> >
> > -     writel_relaxed(addr, vfe->base + TFE_BUS_IMAGE_ADDR(wm));
> > +     writel_relaxed(addr, vfe->base + TFE_BUS_IMAGE_ADDR(client));
> >   }
> >
> > -static void vfe_wm_start(struct vfe_device *vfe, u8 rdi, struct vfe_line *line)
> > +static void vfe_wm_start(struct vfe_device *vfe, u8 wm, struct vfe_line *line)
> >   {
> >       struct v4l2_pix_format_mplane *pix = &line->video_out.active_fmt.fmt.pix_mp;
> >       u32 stride = pix->plane_fmt[0].bytesperline;
> > -     u8 wm = RDI_WM(rdi);
> > +     u8 client = __wm_to_client(wm);
> >
> >       /* Configuration for plain RDI frames */
> > -     writel_relaxed(TFE_BUS_IMAGE_CFG_0_DEFAULT, vfe->base + TFE_BUS_IMAGE_CFG_0(wm));
> > -     writel_relaxed(0u, vfe->base + TFE_BUS_IMAGE_CFG_1(wm));
> > -     writel_relaxed(TFE_BUS_IMAGE_CFG_2_DEFAULT, vfe->base + TFE_BUS_IMAGE_CFG_2(wm));
> > -     writel_relaxed(stride * pix->height, vfe->base + TFE_BUS_FRAME_INCR(wm));
> > -     writel_relaxed(TFE_BUS_PACKER_CFG_FMT_PLAIN64, vfe->base + TFE_BUS_PACKER_CFG(wm));
> > +     writel_relaxed(TFE_BUS_IMAGE_CFG_0_DEFAULT, vfe->base + TFE_BUS_IMAGE_CFG_0(client));
> > +     writel_relaxed(0u, vfe->base + TFE_BUS_IMAGE_CFG_1(client));
> > +     writel_relaxed(TFE_BUS_IMAGE_CFG_2_DEFAULT, vfe->base + TFE_BUS_IMAGE_CFG_2(client));
> > +     writel_relaxed(stride * pix->height, vfe->base + TFE_BUS_FRAME_INCR(client));
> > +     writel_relaxed(TFE_BUS_PACKER_CFG_FMT_PLAIN64, vfe->base + TFE_BUS_PACKER_CFG(client));
> >
> >       /* No dropped frames, one irq per frame */
> > -     writel_relaxed(0, vfe->base + TFE_BUS_FRAMEDROP_CFG_0(wm));
> > -     writel_relaxed(1, vfe->base + TFE_BUS_FRAMEDROP_CFG_1(wm));
> > -     writel_relaxed(0, vfe->base + TFE_BUS_IRQ_SUBSAMPLE_CFG_0(wm));
> > -     writel_relaxed(1, vfe->base + TFE_BUS_IRQ_SUBSAMPLE_CFG_1(wm));
> > +     writel_relaxed(0, vfe->base + TFE_BUS_FRAMEDROP_CFG_0(client));
> > +     writel_relaxed(1, vfe->base + TFE_BUS_FRAMEDROP_CFG_1(client));
> > +     writel_relaxed(0, vfe->base + TFE_BUS_IRQ_SUBSAMPLE_CFG_0(client));
> > +     writel_relaxed(1, vfe->base + TFE_BUS_IRQ_SUBSAMPLE_CFG_1(client));
> >
> >       vfe_enable_irq(vfe);
> >
> >       writel(TFE_BUS_CLIENT_CFG_EN | TFE_BUS_CLIENT_CFG_MODE_FRAME,
> > -            vfe->base + TFE_BUS_CLIENT_CFG(wm));
> > +            vfe->base + TFE_BUS_CLIENT_CFG(client));
> >
> > -     dev_dbg(vfe->camss->dev, "VFE%u: Started RDI%u width %u height %u stride %u\n",
> > -             vfe->id, rdi, pix->width, pix->height, stride);
> > +     dev_dbg(vfe->camss->dev, "VFE%u: Started client %u width %u height %u stride %u\n",
> > +             vfe->id, client, pix->width, pix->height, client);
> >   }
> >
> > -static void vfe_wm_stop(struct vfe_device *vfe, u8 rdi)
> > +static void vfe_wm_stop(struct vfe_device *vfe, u8 wm)
> >   {
> > -     u8 wm = RDI_WM(rdi);
> > +     u8 client = __wm_to_client(wm);
> >
> > -     writel(0, vfe->base + TFE_BUS_CLIENT_CFG(wm));
> > +     writel(0, vfe->base + TFE_BUS_CLIENT_CFG(client));
> >
> > -     dev_dbg(vfe->camss->dev, "VFE%u: Stopped RDI%u\n", vfe->id, rdi);
> > +     dev_dbg(vfe->camss->dev, "VFE%u: Stopped client %u\n", vfe->id, client);
> >   }
> >
> >   static const struct camss_video_ops vfe_video_ops_520 = {
>
> Otherwise seems fine.
>
> Your series title got mangled somewhere BTW.
>
> ---
> bod
>

Re: [PATCH 1/3] media: qcom: camss: vfe-340: Proper client handling

[Loic Poulain]

On Thu, Feb 19, 2026 at 4:29 PM Konrad Dybcio
<konrad.dybcio@oss.qualcomm.com> wrote:
>
> On 2/19/26 4:27 PM, Loic Poulain wrote:
> > We need to properly map camss WM index to our internal WM client
> > instance. Today we're only support RDI interfaces with the RDI_WM
> > macro, introduce a __wm_to_client helper to support any interface.
>
> Do they actually differ between platforms, or can we simply remodel the
> driver defines?

They can differ, yes, not all platforms have the same order and number
of write engines/clients.

Regards,
Loic

Re: [PATCH 3/3] media: qcom: camss: vfe-340: Support for PIX client

[Loic Poulain]

On Tue, Feb 24, 2026 at 2:23 PM Konrad Dybcio
<konrad.dybcio@oss.qualcomm.com> wrote:
>
> On 2/23/26 3:46 PM, Loic Poulain wrote:
> > Hi Konrad,
> >
> > On Thu, Feb 19, 2026 at 4:46 PM Konrad Dybcio
> > <konrad.dybcio@oss.qualcomm.com> wrote:
> >>
> >> On 2/19/26 4:27 PM, Loic Poulain wrote:
> >>> Add support for VFE PIX write engine, allowing to capture frames
> >>> via the PIX video device (e.g. msm_vfe0_pix).
> >>>
> >>> Tested on Agatti/Arduino-Uno-Q with:
> >>> media-ctl -d /dev/media0 -l '"msm_csiphy0":1->"msm_csid0":0[1],"msm_csid0":4->"msm_vfe0_pix":0[1]'
> >>> media-ctl -d /dev/media0 -V '"imx219 2-0010":0[fmt:SRGGB10_1X10/640x480 field:none]'
> >>> media-ctl -d /dev/media0 -V '"msm_csiphy0":0[fmt:SRGGB10_1X10/640x480 field:none]'
> >>> media-ctl -d /dev/media0 -V '"msm_csid0":0[fmt:SRGGB10_1X10/640x480 field:none]'
> >>> media-ctl -d /dev/media0 -V '"msm_csid0":4[fmt:SRGGB10_1X10/640x480 field:none]'
> >>> media-ctl -d /dev/media0 -V '"msm_vfe0_rdi0":0[fmt:SRGGB10_1X10/640x480 field:none]'
> >>> media-ctl -d /dev/media0 -V '"msm_vfe0_pix":0[fmt:SRGGB10_1X10/640x480 field:none]'
> >>> yavta -B capture-mplane --capture=30 -n 3 -f SRGGB10P -s 640x480 /dev/video3
> >>>
> >>> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> >>> ---
> >>
> >> [...]
> >>
> >>> +     if (client == TFE_CLI_BAYER) { /* PIX */
> >>> +             writel_relaxed(pix->width + (pix->height << 16),
> >>> +                            vfe->base + TFE_BUS_IMAGE_CFG_0(client));
> >>> +             writel_relaxed(0u, vfe->base + TFE_BUS_IMAGE_CFG_1(client));
> >>> +             writel_relaxed(stride, vfe->base + TFE_BUS_IMAGE_CFG_2(client));
> >>> +             writel_relaxed(stride * pix->height, vfe->base + TFE_BUS_FRAME_INCR(client));
> >>> +             writel_relaxed(vfe_packer_format(vfe, pix->pixelformat),
> >>> +                            vfe->base + TFE_BUS_PACKER_CFG(client));
> >>> +     } else { /* RDI */
> >>> +             writel_relaxed(TFE_BUS_IMAGE_CFG_0_DEFAULT,
> >>> +                            vfe->base + TFE_BUS_IMAGE_CFG_0(client));
> >>> +             writel_relaxed(0u, vfe->base + TFE_BUS_IMAGE_CFG_1(client));
> >>
> >> Were these default settings (prebumably "dont care" or "always max") valid
> >> for RDI? Would the ones you set up for PIX work/make more sense indiscrimately?
> >
> > That's a good question, the configuration above is the typical setup
> > used for RDI paths, and it matches what other drivers generally do...
> > The RDI path traditionally uses very loose or “don’t care” format
> > settings, and simply 'raw' dumps everything between the CSI Frame
> > Start and Frame End packets as an opaque byte sequence.
> > On the PIX path, however, we expect at least some level of processing
> > (even minimal), such as cropping, plus later statistics... For that,
> > we need a 'description' of the frame format.
> >
> > So theoretically we could describe the frame more precisely for RDI as
> > well, but that would diverge from what existing drivers typically do,
> > and the benefit is limited since RDI does not really consume most of
> > that information.
> > Also, I think we may end up dumping content that has no line
> > delimiters at all via RDI, such as sensor‑generated metadata blocks
> > that don’t follow the usual LS/LE framing.
>
> That sounds convincing. Would such data be the hw-design-intended usecase
> for RDI (with the pixel data going through the full pipeline), or is that
> just a possibility you're mentioning?

I’m not sure this is a dedicated design goal rather than a natural
consequence of how RDI operates. RDI is intended for raw pixel capture
with no processing, and because it doesn’t enforce line‑based
semantics or interpret the incoming payload, it inherently tolerates
arbitrary CSI streams.
It’s also the interface to use when the sensor includes an on‑sensor
ISP and outputs non‑Bayer formats such as YUV.

Regards,
Loic

Re: [PATCH 3/3] media: qcom: camss: vfe-340: Support for PIX client

[Konrad Dybcio]

On 2/25/26 2:14 PM, Loic Poulain wrote:
> On Tue, Feb 24, 2026 at 2:23 PM Konrad Dybcio
> <konrad.dybcio@oss.qualcomm.com> wrote:
>>
>> On 2/23/26 3:46 PM, Loic Poulain wrote:
>>> Hi Konrad,
>>>
>>> On Thu, Feb 19, 2026 at 4:46 PM Konrad Dybcio
>>> <konrad.dybcio@oss.qualcomm.com> wrote:
>>>>
>>>> On 2/19/26 4:27 PM, Loic Poulain wrote:
>>>>> Add support for VFE PIX write engine, allowing to capture frames
>>>>> via the PIX video device (e.g. msm_vfe0_pix).
>>>>>
>>>>> Tested on Agatti/Arduino-Uno-Q with:
>>>>> media-ctl -d /dev/media0 -l '"msm_csiphy0":1->"msm_csid0":0[1],"msm_csid0":4->"msm_vfe0_pix":0[1]'
>>>>> media-ctl -d /dev/media0 -V '"imx219 2-0010":0[fmt:SRGGB10_1X10/640x480 field:none]'
>>>>> media-ctl -d /dev/media0 -V '"msm_csiphy0":0[fmt:SRGGB10_1X10/640x480 field:none]'
>>>>> media-ctl -d /dev/media0 -V '"msm_csid0":0[fmt:SRGGB10_1X10/640x480 field:none]'
>>>>> media-ctl -d /dev/media0 -V '"msm_csid0":4[fmt:SRGGB10_1X10/640x480 field:none]'
>>>>> media-ctl -d /dev/media0 -V '"msm_vfe0_rdi0":0[fmt:SRGGB10_1X10/640x480 field:none]'
>>>>> media-ctl -d /dev/media0 -V '"msm_vfe0_pix":0[fmt:SRGGB10_1X10/640x480 field:none]'
>>>>> yavta -B capture-mplane --capture=30 -n 3 -f SRGGB10P -s 640x480 /dev/video3
>>>>>
>>>>> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
>>>>> ---
>>>>
>>>> [...]
>>>>
>>>>> +     if (client == TFE_CLI_BAYER) { /* PIX */
>>>>> +             writel_relaxed(pix->width + (pix->height << 16),
>>>>> +                            vfe->base + TFE_BUS_IMAGE_CFG_0(client));
>>>>> +             writel_relaxed(0u, vfe->base + TFE_BUS_IMAGE_CFG_1(client));
>>>>> +             writel_relaxed(stride, vfe->base + TFE_BUS_IMAGE_CFG_2(client));
>>>>> +             writel_relaxed(stride * pix->height, vfe->base + TFE_BUS_FRAME_INCR(client));
>>>>> +             writel_relaxed(vfe_packer_format(vfe, pix->pixelformat),
>>>>> +                            vfe->base + TFE_BUS_PACKER_CFG(client));
>>>>> +     } else { /* RDI */
>>>>> +             writel_relaxed(TFE_BUS_IMAGE_CFG_0_DEFAULT,
>>>>> +                            vfe->base + TFE_BUS_IMAGE_CFG_0(client));
>>>>> +             writel_relaxed(0u, vfe->base + TFE_BUS_IMAGE_CFG_1(client));
>>>>
>>>> Were these default settings (prebumably "dont care" or "always max") valid
>>>> for RDI? Would the ones you set up for PIX work/make more sense indiscrimately?
>>>
>>> That's a good question, the configuration above is the typical setup
>>> used for RDI paths, and it matches what other drivers generally do...
>>> The RDI path traditionally uses very loose or “don’t care” format
>>> settings, and simply 'raw' dumps everything between the CSI Frame
>>> Start and Frame End packets as an opaque byte sequence.
>>> On the PIX path, however, we expect at least some level of processing
>>> (even minimal), such as cropping, plus later statistics... For that,
>>> we need a 'description' of the frame format.
>>>
>>> So theoretically we could describe the frame more precisely for RDI as
>>> well, but that would diverge from what existing drivers typically do,
>>> and the benefit is limited since RDI does not really consume most of
>>> that information.
>>> Also, I think we may end up dumping content that has no line
>>> delimiters at all via RDI, such as sensor‑generated metadata blocks
>>> that don’t follow the usual LS/LE framing.
>>
>> That sounds convincing. Would such data be the hw-design-intended usecase
>> for RDI (with the pixel data going through the full pipeline), or is that
>> just a possibility you're mentioning?
> 
> I’m not sure this is a dedicated design goal rather than a natural
> consequence of how RDI operates. RDI is intended for raw pixel capture
> with no processing, and because it doesn’t enforce line‑based
> semantics or interpret the incoming payload, it inherently tolerates
> arbitrary CSI streams.
> It’s also the interface to use when the sensor includes an on‑sensor
> ISP and outputs non‑Bayer formats such as YUV.

I see thanks, for the explanation!

Konrad