[PATCH 0/8] media: qcom: camss: add V4L2 subdev streams API support

[PATCH 0/8] media: qcom: camss: add V4L2 subdev streams API support

[Gjorgji.Rosikopulos.gjorgji.rosikopulos]

From: Gjorgji Rosikopulos <gjorgji.rosikopulos@oss.qualcomm.com>

This series adds V4L2 subdev streams API support to the CAMSS driver.

Each subdevice gains streams-aware enable_streams/disable_streams pad
ops alongside the existing legacy (non-streams) subdev ops, guarded by
a new per-instance streams_enable resource flag.

Patches 1-4 add the CSIPHY/CSID mechanism:
 - CSIPHY: passthrough routing, NO_STREAM_MIX/NO_N_TO_1 validation, and
   shared D-PHY lane enable/disable gated on stream-count transitions.
 - CSID: per-source-pad routing (a single sink stream propagated to
   every source pad by default, remappable for multi-VC sensors),
   VC/DT discovery via get_frame_desc, and new hw_ops
   (configure_rx/enable_stream/disable_stream) with a gen2 backend
   implementation.

Patch 5 is a standalone bug fix, independent of the streams API:
camss_link_entities() used to create an all-to-all CSID-to-VFE
crossbar, but SM8250's hardware wiring is a fixed 1:1 pairing
(csid[i] <-> vfe[i]). Enabling a mismatched link (e.g. csid0 -> vfe1)
exposed a media link with no real hardware datapath. Fixed via an
opt-in csid_vfe_fixed_pairing flag, set only for sm8250_resources.

Patches 6-8 complete the mechanism and turn it on for real hardware:
 - VFE: streams-aware pad ops. VFE lines are inherently single-consumer
   (vfe_link_setup() enforces one link per pad), so no refcounting is
   needed there.
 - camss-video: the video device pipeline walk now checks, via
   v4l2_subdev_has_op(), whether the directly-connected subdev supports
   enable_streams/disable_streams; if so it issues a single top-level
   call instead of manually walking the pipeline one subdev at a time
   with .s_stream(). Falls back to the existing legacy path unchanged
   when the remote subdev doesn't support the streams API, so no other
   platform is affected.
 - SM8250: streams_enable is set true on every CSIPHY, CSID, and VFE
   line resource entry, turning the mechanism on for real hardware.
   Every other platform keeps using the legacy non-streams subdev ops,
   so this is a no-op everywhere else.

A practical benefit of the CSID routing change (patch 4) is routing
flexibility for multi-VC sensors: the CSID's routing table maps sink
streams to source pads/streams via userspace-configurable
v4l2_subdev_route entries instead of a fixed pad<->VC assignment, so a
sensor emitting multiple virtual channels can have each VC directed to
a different RDI output (and thus a different VFE line/video node)
with a set_routing call, rather than being constrained to whatever
fixed mapping the driver hardcodes.

When a sink stream is shared by multiple source pads/streams, CSID
only enables the corresponding upstream CSIPHY stream on the first
source stream that needs it, and only disables it once the last
remaining source stream using it is disabled. Enabling or disabling
additional consumers of an already-active shared stream is a no-op
upstream, so no consumer can double-enable or prematurely disable a
stream still in use by another. This also avoids ever hitting v4l2
core's own -EALREADY re-enable gate.

Verified clean with checkpatch --strict. Built, flashed, and tested on
RB5/SM8250 hardware; ran the no-routing capture verification test
across all four CSID/VFE RDI pairs (csid0->vfe0, csid1->vfe1,
csid2->vfe2, csid3->vfe3) at 4056x3040 - all four passed with
correctly-sized frame captures.

Gjorgji Rosikopulos (8):
  media: qcom: camss: Add streams API support for CSIPHY
  media: qcom: camss: Add streams API hw_ops to CSID interface
  media: qcom: camss: Implement CSID streams API hw_ops for gen2
  media: qcom: camss: Add streams API support in CSID subdevice
  media: qcom: camss: Fix CSID-to-VFE all-to-all link crossbar on sm8250
  media: qcom: camss: add streams API support for VFE
  media: qcom: camss: add streams API support in camss-video
  media: qcom: camss: enable streams API on SM8250

 .../platform/qcom/camss/camss-csid-gen2.c     |  59 ++-
 .../media/platform/qcom/camss/camss-csid.c    | 494 +++++++++++++++++-
 .../media/platform/qcom/camss/camss-csid.h    |  45 ++
 .../media/platform/qcom/camss/camss-csiphy.c  | 223 +++++++-
 .../media/platform/qcom/camss/camss-csiphy.h  |   2 +
 drivers/media/platform/qcom/camss/camss-vfe.c | 119 ++++-
 drivers/media/platform/qcom/camss/camss-vfe.h |   1 +
 .../media/platform/qcom/camss/camss-video.c   | 119 ++++-
 drivers/media/platform/qcom/camss/camss.c     |  21 +-
 drivers/media/platform/qcom/camss/camss.h     |   7 +
 10 files changed, 1046 insertions(+), 44 deletions(-)

-- 
2.34.1

[PATCH 1/8] media: qcom: camss: Add streams API support for CSIPHY

[Gjorgji.Rosikopulos.gjorgji.rosikopulos]

From: Gjorgji Rosikopulos <gjorgji.rosikopulos@oss.qualcomm.com>

Add the V4L2 subdev streams API to the CSIPHY driver: a passthrough
routing table (one VC per stream) and NO_STREAM_MIX routing validation.

enable/disable_streams pad ops track active streams per pad via a
per-pad enabled_streams[] bitmask, so that multiple source-pad
consumers can share a single propagated sink stream without redundant
or colliding propagation to the sensor. The shared D-PHY lanes are
enabled once, on the transition from no active streams to at least
one, and disabled once all streams have gone idle.

This is opt-in per CSIPHY instance via the new streams_enable resource
flag; no platform sets it yet, so CSIPHYs continue to use the legacy
non-streams subdev ops unchanged.

Signed-off-by: Gjorgji Rosikopulos <gjorgji.rosikopulos@oss.qualcomm.com>
---
 .../media/platform/qcom/camss/camss-csiphy.c  | 223 +++++++++++++++++-
 .../media/platform/qcom/camss/camss-csiphy.h  |   2 +
 2 files changed, 222 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/qcom/camss/camss-csiphy.c b/drivers/media/platform/qcom/camss/camss-csiphy.c
index 539ac4888b60..7e9748e92ab8 100644
--- a/drivers/media/platform/qcom/camss/camss-csiphy.c
+++ b/drivers/media/platform/qcom/camss/camss-csiphy.c
@@ -332,6 +332,112 @@ static int csiphy_set_stream(struct v4l2_subdev *sd, int enable)
 	return ret;
 }
 
+/*
+ * csiphy_pad_enable_streams - Enable one or more streams on the source pad
+ * @sd: CSIPHY V4L2 subdevice
+ * @state: V4L2 subdevice state
+ * @pad: Pad number
+ * @streams_mask: Bitmask of streams to enable
+ *
+ * The shared D-PHY lanes are enabled once, on the transition from no active
+ * sink streams to at least one. The sink stream(s) are propagated upstream to
+ * the sensor only for the subset that isn't already active, so a stream
+ * that's already running is never redundantly re-propagated.
+ *
+ * CSIPHY is only ever linked to a single entity on its source pad, and that
+ * entity is responsible for only enabling a stream on this pad while it
+ * itself still needs it, so no cross-consumer refcounting is needed here.
+ *
+ * Return 0 on success or a negative error code otherwise
+ */
+static int csiphy_pad_enable_streams(struct v4l2_subdev *sd,
+				     struct v4l2_subdev_state *state,
+				     u32 pad, u64 streams_mask)
+{
+	struct csiphy_device *csiphy = v4l2_get_subdevdata(sd);
+	struct media_pad *remote_pad =
+		media_pad_remote_pad_first(&csiphy->pads[MSM_CSIPHY_PAD_SINK]);
+	bool first_arrival = !csiphy->enabled_streams[MSM_CSIPHY_PAD_SINK];
+	u64 sink_streams, propagate_mask;
+	int ret;
+
+	sink_streams = v4l2_subdev_state_xlate_streams(state, pad, MSM_CSIPHY_PAD_SINK,
+						       &streams_mask);
+
+	propagate_mask = sink_streams & ~csiphy->enabled_streams[MSM_CSIPHY_PAD_SINK];
+
+	if (first_arrival) {
+		ret = csiphy_set_stream(sd, 1);
+		if (ret)
+			return ret;
+	}
+
+	csiphy->enabled_streams[MSM_CSIPHY_PAD_SINK] |= sink_streams;
+	csiphy->enabled_streams[pad] |= streams_mask;
+
+	if (propagate_mask && remote_pad) {
+		ret = v4l2_subdev_enable_streams(media_entity_to_v4l2_subdev(remote_pad->entity),
+						 remote_pad->index, propagate_mask);
+		if (ret) {
+			csiphy->enabled_streams[MSM_CSIPHY_PAD_SINK] &= ~propagate_mask;
+			csiphy->enabled_streams[pad] &= ~streams_mask;
+
+			if (first_arrival)
+				csiphy_set_stream(sd, 0);
+
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+/*
+ * csiphy_pad_disable_streams - Disable one or more streams on the source pad
+ * @sd: CSIPHY V4L2 subdevice
+ * @state: V4L2 subdevice state
+ * @pad: Pad number
+ * @streams_mask: Bitmask of streams to disable
+ *
+ * The shared D-PHY lanes, and the propagation to the sensor, are only torn
+ * down once no sink stream is referenced by any source pad any more.
+ *
+ * CSIPHY is only ever linked to a single entity on its source pad, and that
+ * entity is responsible for only disabling a stream on this pad once it no
+ * longer needs it, so no cross-consumer refcounting is needed here.
+ *
+ * Return 0 on success or a negative error code otherwise
+ */
+static int csiphy_pad_disable_streams(struct v4l2_subdev *sd,
+				      struct v4l2_subdev_state *state,
+				      u32 pad, u64 streams_mask)
+{
+	struct csiphy_device *csiphy = v4l2_get_subdevdata(sd);
+	struct media_pad *remote_pad =
+		media_pad_remote_pad_first(&csiphy->pads[MSM_CSIPHY_PAD_SINK]);
+	u64 sink_streams;
+	int ret = 0;
+
+	sink_streams = v4l2_subdev_state_xlate_streams(state, pad, MSM_CSIPHY_PAD_SINK,
+						       &streams_mask);
+
+	csiphy->enabled_streams[pad] &= ~streams_mask;
+	csiphy->enabled_streams[MSM_CSIPHY_PAD_SINK] &= ~sink_streams;
+
+	if (sink_streams && remote_pad)
+		ret = v4l2_subdev_disable_streams(media_entity_to_v4l2_subdev(remote_pad->entity),
+						  remote_pad->index, sink_streams);
+
+	if (!csiphy->enabled_streams[MSM_CSIPHY_PAD_SINK]) {
+		int stream_ret = csiphy_set_stream(sd, 0);
+
+		if (!ret)
+			ret = stream_ret;
+	}
+
+	return ret;
+}
+
 /*
  * __csiphy_get_format - Get pointer to format structure
  * @csiphy: CSIPHY device
@@ -743,6 +849,71 @@ static int csiphy_link_setup(struct media_entity *entity,
 	return 0;
 }
 
+static int csiphy_init_state(struct v4l2_subdev *sd,
+			     struct v4l2_subdev_state *state)
+{
+	struct v4l2_subdev_route routes[] = {
+		{
+			.sink_pad = MSM_CSIPHY_PAD_SINK,
+			.sink_stream = 0,
+			.source_pad = MSM_CSIPHY_PAD_SRC,
+			.source_stream = 0,
+			.flags = V4L2_SUBDEV_ROUTE_FL_ACTIVE,
+		},
+	};
+	struct v4l2_subdev_krouting routing = {
+		.num_routes = ARRAY_SIZE(routes),
+		.routes = routes,
+	};
+
+	/*
+	 * CSIPHY is a transparent D-PHY with no per-VC demux, so every sink
+	 * stream (VC) a multi-stream sensor may drive must pass straight
+	 * through as the same source stream, or downstream link validation
+	 * (e.g. against CSID's multi-pad sink) will flag it as dangling. A
+	 * multi-VC sensor is supported by userspace adding further routes via
+	 * .set_routing; this default covers the common single-VC case.
+	 */
+	return v4l2_subdev_set_routing(sd, state, &routing);
+}
+
+/*
+ * csiphy_set_routing - Set routing for the CSIPHY subdev
+ * @sd: CSIPHY V4L2 subdevice
+ * @state: V4L2 subdevice state
+ * @which: Type of format state (V4L2_SUBDEV_FORMAT_ACTIVE or TRY)
+ * @routing: Routing table to set
+ *
+ * CSIPHY is a transparent D-PHY with no per-VC demux, so every route must
+ * pass a sink stream straight through as the same source stream.
+ *
+ * Return 0 on success or a negative error code otherwise
+ */
+static int csiphy_set_routing(struct v4l2_subdev *sd,
+			      struct v4l2_subdev_state *state,
+			      enum v4l2_subdev_format_whence which,
+			      struct v4l2_subdev_krouting *routing)
+{
+	struct csiphy_device *csiphy = v4l2_get_subdevdata(sd);
+	unsigned int i;
+	int ret;
+
+	if (which == V4L2_SUBDEV_FORMAT_ACTIVE && csiphy->enabled_streams[MSM_CSIPHY_PAD_SINK])
+		return -EBUSY;
+
+	for (i = 0; i < routing->num_routes; i++)
+		if (routing->routes[i].sink_stream != routing->routes[i].source_stream)
+			return -EINVAL;
+
+	ret = v4l2_subdev_routing_validate(sd, routing,
+					   V4L2_SUBDEV_ROUTING_NO_STREAM_MIX |
+					   V4L2_SUBDEV_ROUTING_NO_N_TO_1);
+	if (ret)
+		return ret;
+
+	return v4l2_subdev_set_routing(sd, state, routing);
+}
+
 static const struct v4l2_subdev_core_ops csiphy_core_ops = {
 	.s_power = csiphy_set_power,
 };
@@ -764,10 +935,35 @@ static const struct v4l2_subdev_ops csiphy_v4l2_ops = {
 	.pad = &csiphy_pad_ops,
 };
 
+static const struct v4l2_subdev_pad_ops csiphy_streams_pad_ops = {
+	.enum_mbus_code = csiphy_enum_mbus_code,
+	.enum_frame_size = csiphy_enum_frame_size,
+	.get_fmt = csiphy_get_format,
+	.set_fmt = csiphy_set_format,
+	.get_frame_desc = v4l2_subdev_get_frame_desc_passthrough,
+	.set_routing = csiphy_set_routing,
+	.enable_streams = csiphy_pad_enable_streams,
+	.disable_streams = csiphy_pad_disable_streams,
+};
+
+static const struct v4l2_subdev_video_ops csiphy_streams_video_ops = {
+	.s_stream = v4l2_subdev_s_stream_helper,
+};
+
+static const struct v4l2_subdev_ops csiphy_streams_v4l2_ops = {
+	.core = &csiphy_core_ops,
+	.pad = &csiphy_streams_pad_ops,
+	.video = &csiphy_streams_video_ops,
+};
+
 static const struct v4l2_subdev_internal_ops csiphy_v4l2_internal_ops = {
 	.open = csiphy_init_formats,
 };
 
+static const struct v4l2_subdev_internal_ops csiphy_streams_internal_ops = {
+	.init_state = csiphy_init_state,
+};
+
 static const struct media_entity_operations csiphy_media_ops = {
 	.link_setup = csiphy_link_setup,
 	.link_validate = v4l2_subdev_link_validate,
@@ -786,11 +982,16 @@ int msm_csiphy_register_entity(struct csiphy_device *csiphy,
 	struct v4l2_subdev *sd = &csiphy->subdev;
 	struct media_pad *pads = csiphy->pads;
 	struct device *dev = csiphy->camss->dev;
+	bool streams_api = csiphy->res->streams_enable;
 	int ret;
 
-	v4l2_subdev_init(sd, &csiphy_v4l2_ops);
-	sd->internal_ops = &csiphy_v4l2_internal_ops;
+	v4l2_subdev_init(sd, streams_api ? &csiphy_streams_v4l2_ops
+					      : &csiphy_v4l2_ops);
+	sd->internal_ops = streams_api ? &csiphy_streams_internal_ops
+					: &csiphy_v4l2_internal_ops;
 	sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
+	if (streams_api)
+		sd->flags |= V4L2_SUBDEV_FL_STREAMS;
 	snprintf(sd->name, ARRAY_SIZE(sd->name), "%s%d",
 		 MSM_CSIPHY_NAME, csiphy->id);
 	sd->grp_id = CSIPHY_GRP_ID;
@@ -813,12 +1014,27 @@ int msm_csiphy_register_entity(struct csiphy_device *csiphy,
 		return ret;
 	}
 
+	if (streams_api) {
+		ret = v4l2_subdev_init_finalize(sd);
+		if (ret) {
+			dev_err(dev, "Failed to finalize subdev: %d\n", ret);
+			goto err_media_entity_cleanup;
+		}
+	}
+
 	ret = v4l2_device_register_subdev(v4l2_dev, sd);
 	if (ret < 0) {
 		dev_err(dev, "Failed to register subdev: %d\n", ret);
-		media_entity_cleanup(&sd->entity);
+		goto err_v4l2_subdev_cleanup;
 	}
 
+	return 0;
+
+err_v4l2_subdev_cleanup:
+	v4l2_subdev_cleanup(sd);
+err_media_entity_cleanup:
+	media_entity_cleanup(&sd->entity);
+
 	return ret;
 }
 
@@ -829,5 +1045,6 @@ int msm_csiphy_register_entity(struct csiphy_device *csiphy,
 void msm_csiphy_unregister_entity(struct csiphy_device *csiphy)
 {
 	v4l2_device_unregister_subdev(&csiphy->subdev);
+	v4l2_subdev_cleanup(&csiphy->subdev);
 	media_entity_cleanup(&csiphy->subdev.entity);
 }
diff --git a/drivers/media/platform/qcom/camss/camss-csiphy.h b/drivers/media/platform/qcom/camss/camss-csiphy.h
index 9d9657b82f74..e55c098dbe67 100644
--- a/drivers/media/platform/qcom/camss/camss-csiphy.h
+++ b/drivers/media/platform/qcom/camss/camss-csiphy.h
@@ -84,6 +84,7 @@ struct csiphy_hw_ops {
 
 struct csiphy_subdev_resources {
 	u8 id;
+	bool streams_enable;
 	const struct csiphy_hw_ops *hw_ops;
 	const struct csiphy_formats *formats;
 };
@@ -114,6 +115,7 @@ struct csiphy_device {
 	struct v4l2_mbus_framefmt fmt[MSM_CSIPHY_PADS_NUM];
 	const struct csiphy_subdev_resources *res;
 	struct csiphy_device_regs *regs;
+	u64 enabled_streams[MSM_CSIPHY_PADS_NUM];
 };
 
 struct camss_subdev_resources;
-- 
2.34.1

[PATCH 2/8] media: qcom: camss: Add streams API hw_ops to CSID interface

[Gjorgji.Rosikopulos.gjorgji.rosikopulos]

From: Gjorgji Rosikopulos <gjorgji.rosikopulos@oss.qualcomm.com>

Add configure_rx/enable_stream/disable_stream to struct csid_hw_ops:
per-stream hardware operations needed to support the V4L2 subdev
streams API, where the CSID Rx front-end is configured once
independently of the per-stream RDI programming.

No functional change: these are new optional hw_ops entries, not yet
implemented by any backend or called by any code path.

Signed-off-by: Gjorgji Rosikopulos <gjorgji.rosikopulos@oss.qualcomm.com>
---
 .../media/platform/qcom/camss/camss-csid.h    | 43 +++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/drivers/media/platform/qcom/camss/camss-csid.h b/drivers/media/platform/qcom/camss/camss-csid.h
index 5296b10f6bac..90ee611b9092 100644
--- a/drivers/media/platform/qcom/camss/camss-csid.h
+++ b/drivers/media/platform/qcom/camss/camss-csid.h
@@ -74,6 +74,25 @@ struct csid_phy_config {
 
 struct csid_device;
 
+/*
+ * struct csid_hw_ops - CSID hardware version specific operations
+ *
+ * configure_stream() is the legacy (non-streams-API) path and is mutually
+ * exclusive with enable_stream()/disable_stream() - a given CSID instance
+ * uses one family or the other, chosen by streams_enable.
+ *
+ * stream_id, as passed to enable_stream()/disable_stream(), is a hardware
+ * RDI/PIX port index in [0, MSM_CSID_MAX_SRC_STREAMS). It is currently
+ * always equal to pad - MSM_CSID_PAD_FIRST_SRC (multi-pad, 1:1
+ * pad-to-port mapping).
+ *
+ * configure_rx() is called once, on the transition from zero to non-zero
+ * active sink streams, strictly before the first enable_stream() call.
+ * enable_stream()/disable_stream() calls are always made under the
+ * subdev's active-state lock (core-enforced via
+ * v4l2_subdev_enable_streams()/disable_streams()), so hw_ops
+ * implementations don't need their own serialization.
+ */
 struct csid_hw_ops {
 	/*
 	 * configure_stream - Configures and starts CSID input stream
@@ -81,6 +100,30 @@ struct csid_hw_ops {
 	 */
 	void (*configure_stream)(struct csid_device *csid, u8 enable);
 
+	/*
+	 * configure_rx - Configure the CSID Rx front-end
+	 * @csid: CSID device
+	 *
+	 * Called once, when the first stream is enabled.
+	 */
+	void (*configure_rx)(struct csid_device *csid);
+
+	/*
+	 * enable_stream - Enable a single CSID output stream
+	 * @csid: CSID device
+	 * @stream_id: stream id to enable
+	 * @vc: virtual channel to program
+	 * @dt: data type to program
+	 */
+	void (*enable_stream)(struct csid_device *csid, u32 stream_id, u8 vc, u8 dt);
+
+	/*
+	 * disable_stream - Disable a single CSID output stream
+	 * @csid: CSID device
+	 * @stream_id: stream id to disable
+	 */
+	void (*disable_stream)(struct csid_device *csid, u32 stream_id);
+
 	/*
 	 * configure_testgen_pattern - Validates and configures output pattern mode
 	 * of test pattern generator
-- 
2.34.1

[PATCH 3/8] media: qcom: camss: Implement CSID streams API hw_ops for gen2

[Gjorgji.Rosikopulos.gjorgji.rosikopulos]

From: Gjorgji Rosikopulos <gjorgji.rosikopulos@oss.qualcomm.com>

Implement the configure_rx/enable_stream/disable_stream hw_ops added to
struct csid_hw_ops, for the gen2 CSID hardware backend used by SM8250.

__csid_configure_rdi_stream() now takes an explicit data type parameter
instead of deriving it from the CSID source pad's format, since the
streams API keys each stream by stream_id rather than by pad. The
existing configure_stream() path is updated to pass its per-port format
lookup through to this parameter, preserving current behavior.

Signed-off-by: Gjorgji Rosikopulos <gjorgji.rosikopulos@oss.qualcomm.com>
---
 .../platform/qcom/camss/camss-csid-gen2.c     | 59 ++++++++++++++++---
 1 file changed, 51 insertions(+), 8 deletions(-)

diff --git a/drivers/media/platform/qcom/camss/camss-csid-gen2.c b/drivers/media/platform/qcom/camss/camss-csid-gen2.c
index eadcb2f7e3aa..2512cdb16d94 100644
--- a/drivers/media/platform/qcom/camss/camss-csid-gen2.c
+++ b/drivers/media/platform/qcom/camss/camss-csid-gen2.c
@@ -253,13 +253,8 @@ static void __csid_configure_testgen(struct csid_device *csid, u8 enable, u8 por
 	writel_relaxed(val, csid->base + CSID_TPG_CTRL);
 }
 
-static void __csid_configure_rdi_stream(struct csid_device *csid, u8 enable, u8 port, u8 vc)
+static void __csid_configure_rdi_stream(struct csid_device *csid, u8 enable, u8 port, u8 vc, u8 dt)
 {
-	/* Source pads matching RDI channels on hardware. Pad 1 -> RDI0, Pad 2 -> RDI1, etc. */
-	struct v4l2_mbus_framefmt *input_format = &csid->fmt[MSM_CSID_PAD_FIRST_SRC + port];
-	const struct csid_format_info *format = csid_get_fmt_entry(csid->res->formats->formats,
-								   csid->res->formats->nformats,
-								   input_format->code);
 	u32 val;
 
 	/*
@@ -281,7 +276,7 @@ static void __csid_configure_rdi_stream(struct csid_device *csid, u8 enable, u8
 	val |= 1 << RDI_CFG0_TIMESTAMP_EN;
 	/* note: for non-RDI path, this should be format->decode_format */
 	val |= DECODE_FORMAT_PAYLOAD_ONLY << RDI_CFG0_DECODE_FORMAT;
-	val |= format->data_type << RDI_CFG0_DATA_TYPE;
+	val |= dt << RDI_CFG0_DATA_TYPE;
 	val |= vc << RDI_CFG0_VIRTUAL_CHANNEL;
 	val |= dt_id << RDI_CFG0_DT_ID;
 	writel_relaxed(val, csid->base + CSID_RDI_CFG0(port));
@@ -330,15 +325,60 @@ static void csid_configure_stream(struct csid_device *csid, u8 enable)
 	/* Loop through all enabled ports and configure a stream for each */
 	for (i = 0; i < MSM_CSID_MAX_SRC_STREAMS; i++)
 		if (csid->phy.en_vc & BIT(i)) {
+			/* Source pads match RDI channels: pad 1 -> RDI0, pad 2 -> RDI1, etc. */
+			struct v4l2_mbus_framefmt *input_format =
+				&csid->fmt[MSM_CSID_PAD_FIRST_SRC + i];
+			const struct csid_format_info *format =
+				csid_get_fmt_entry(csid->res->formats->formats,
+						   csid->res->formats->nformats,
+						   input_format->code);
+			u8 vc = 0;
+
 			if (tg->enabled)
 				__csid_configure_testgen(csid, enable, i, 0);
 
-			__csid_configure_rdi_stream(csid, enable, i, 0);
+			__csid_configure_rdi_stream(csid, enable, i, vc, format->data_type);
 			__csid_configure_rx(csid, &csid->phy, 0);
 			__csid_ctrl_rdi(csid, enable, i);
 		}
 }
 
+/*
+ * configure_rx - Configure the CSID Rx front-end
+ */
+static void csid_configure_rx(struct csid_device *csid)
+{
+	__csid_configure_rx(csid, &csid->phy, 0);
+}
+
+/*
+ * stream_id is used directly as the hardware RDI port index below. This
+ * assumes a 1:1 stream-to-port mapping and should be revisited once
+ * per-platform src_streams data (stream id -> hw pipe) is added.
+ */
+static void csid_enable_stream(struct csid_device *csid, u32 stream_id, u8 vc, u8 dt)
+{
+	struct csid_testgen_config *tg = &csid->testgen;
+
+	if (tg->enabled)
+		__csid_configure_testgen(csid, 1, stream_id, vc);
+
+	__csid_configure_rdi_stream(csid, 1, stream_id, vc, dt);
+	__csid_ctrl_rdi(csid, 1, stream_id);
+}
+
+static void csid_disable_stream(struct csid_device *csid, u32 stream_id)
+{
+	struct csid_testgen_config *tg = &csid->testgen;
+
+	__csid_ctrl_rdi(csid, 0, stream_id);
+
+	if (tg->enabled)
+		__csid_configure_testgen(csid, 0, stream_id, 0);
+
+	__csid_configure_rdi_stream(csid, 0, stream_id, 0, 0);
+}
+
 static int csid_configure_testgen_pattern(struct csid_device *csid, s32 val)
 {
 	if (val > 0 && val <= csid->testgen.nmodes)
@@ -425,6 +465,9 @@ static void csid_subdev_init(struct csid_device *csid)
 const struct csid_hw_ops csid_ops_gen2 = {
 	.configure_stream = csid_configure_stream,
 	.configure_testgen_pattern = csid_configure_testgen_pattern,
+	.configure_rx = csid_configure_rx,
+	.enable_stream = csid_enable_stream,
+	.disable_stream = csid_disable_stream,
 	.hw_version = csid_hw_version,
 	.isr = csid_isr,
 	.reset = csid_reset,
-- 
2.34.1

[PATCH 4/8] media: qcom: camss: Add streams API support in CSID subdevice

[Gjorgji.Rosikopulos.gjorgji.rosikopulos]

From: Gjorgji Rosikopulos <gjorgji.rosikopulos@oss.qualcomm.com>

Add the V4L2 subdev streams API to the CSID driver: per-source-pad
enable/disable_streams pad ops, VC/DT discovery via get_frame_desc on
the remote sink pad, and a routing table that routes a single incoming
sink stream to every source pad by default (remappable via
set_routing for multi-VC sensors).

Active streams are tracked per pad via a per-pad enabled_streams[]
bitmask, so that enable/disable_streams correctly propagates to the
CSIPHY only on first arrival/last departure of a sink stream, and
multiple source-pad consumers can share a single propagated sink
stream without redundant or colliding propagation.

csid_init_state() caps the number of default routes to
MSM_CSID_MAX_SRC_STREAMS - 1 for non-lite CSIDs, matching the 3 usable
RDI pads on full-IFE CSIDs (the 4th/pix pad is non-functional).
msm_csid_register_entity() mirrors the same is_lite check for the pad
count itself, so non-lite CSIDs no longer register a pix source pad
that no route ever targets.

csid_pad_enable_streams() rejects enabling with -ENOLINK when there is
no remote sink link and the test generator is disabled, matching the
equivalent check the legacy csid_set_stream() already performs.

msm_csid_register_entity() assigns the ctrl handler's lock as the
subdev's state_lock before v4l2_subdev_init_finalize(), when the test
pattern control is present. Without this, the test-pattern S_CTRL
handler and the streams API's active-state accessors serialize on two
independent locks despite both touching csid->testgen.enabled,
allowing a concurrent S_CTRL(TEST_PATTERN) and stream enable/disable to
race.

This is opt-in per CSID instance via the new streams_enable resource
flag; no platform sets it yet, so CSIDs continue to use the legacy
non-streams subdev ops unchanged.

Signed-off-by: Gjorgji Rosikopulos <gjorgji.rosikopulos@oss.qualcomm.com>
---
 .../media/platform/qcom/camss/camss-csid.c    | 494 +++++++++++++++++-
 .../media/platform/qcom/camss/camss-csid.h    |   2 +
 2 files changed, 491 insertions(+), 5 deletions(-)

diff --git a/drivers/media/platform/qcom/camss/camss-csid.c b/drivers/media/platform/qcom/camss/camss-csid.c
index 48459b46a981..ce4b07c0c1c2 100644
--- a/drivers/media/platform/qcom/camss/camss-csid.c
+++ b/drivers/media/platform/qcom/camss/camss-csid.c
@@ -842,7 +842,7 @@ static void csid_try_format(struct csid_device *csid,
 
 		break;
 
-	case MSM_CSID_PAD_SRC:
+	default:
 		if (csid->testgen.nmodes == CSID_PAYLOAD_MODE_DISABLED ||
 		    csid->testgen_mode->cur.val == 0) {
 			/* Test generator is disabled, */
@@ -1338,10 +1338,476 @@ static const struct v4l2_subdev_ops csid_v4l2_ops = {
 	.pad = &csid_pad_ops,
 };
 
+/*
+ * csid_get_stream_csi2_desc - Discover the virtual channel and data type
+ *			       used by a given sink stream, from an
+ *			       already-fetched frame descriptor
+ * @frame_desc: Frame descriptor fetched via .get_frame_desc from the remote
+ *		subdev linked on the sink pad
+ * @sink_stream: Sink-side stream number to look up
+ * @desc_csi2: Returns the discovered virtual channel/data type on success
+ *
+ * A frame descriptor with a single entry means the remote only exposes one
+ * stream (e.g. a single-VC sensor), which feeds every CSID source pad, so
+ * that entry is used regardless of @sink_stream.
+ *
+ * Return true if a matching entry was found, false otherwise
+ */
+static bool csid_get_stream_csi2_desc(struct v4l2_mbus_frame_desc *frame_desc,
+				      u32 sink_stream,
+				      struct v4l2_mbus_frame_desc_entry_csi2 *desc_csi2)
+{
+	unsigned int i;
+
+	if (frame_desc->type != V4L2_MBUS_FRAME_DESC_TYPE_CSI2 || !frame_desc->num_entries)
+		return false;
+
+	if (frame_desc->num_entries == 1) {
+		*desc_csi2 = frame_desc->entry[0].bus.csi2;
+		return true;
+	}
+
+	for (i = 0; i < frame_desc->num_entries; i++) {
+		if (frame_desc->entry[i].stream == sink_stream) {
+			*desc_csi2 = frame_desc->entry[i].bus.csi2;
+			return true;
+		}
+	}
+
+	return false;
+}
+
+/*
+ * csid_get_stream_vc_dt - Discover the virtual channel and data type to
+ *			   program for a given sink pad/stream, falling back
+ *			   to @format_dt when no frame descriptor is available
+ * @csid: CSID device
+ * @state: V4L2 subdevice state
+ * @remote_pad: Remote pad linked on the CSID sink pad, or NULL if unlinked
+ * @pad: Source pad number the caller is enabling a stream on
+ * @format_dt: Data type derived from the sink format, used as a fallback
+ *	       and sanity-checked against the discovered data type
+ *
+ * Return the discovered virtual channel/data type, or {0, @format_dt} if
+ * not discovered
+ */
+static struct v4l2_mbus_frame_desc_entry_csi2
+csid_get_stream_vc_dt(struct csid_device *csid, struct v4l2_subdev_state *state,
+		      struct media_pad *remote_pad, u32 pad, u8 format_dt)
+{
+	struct v4l2_mbus_frame_desc_entry_csi2 desc_csi2 = { .dt = format_dt };
+	struct v4l2_mbus_frame_desc fd = { };
+	u32 sink_stream;
+
+	if (!remote_pad ||
+	    v4l2_subdev_call(media_entity_to_v4l2_subdev(remote_pad->entity),
+			     pad, get_frame_desc, remote_pad->index, &fd))
+		return desc_csi2;
+
+	if (v4l2_subdev_routing_find_opposite_end(&state->routing, pad, 0, NULL, &sink_stream))
+		return desc_csi2;
+
+	if (!csid_get_stream_csi2_desc(&fd, sink_stream, &desc_csi2)) {
+		dev_warn(csid->camss->dev,
+			 "Failed to find CSI2 descriptor for sink stream %u, using vc=%u dt=%u\n",
+			 sink_stream, desc_csi2.vc, desc_csi2.dt);
+		return desc_csi2;
+	}
+
+	if (desc_csi2.dt != format_dt)
+		dev_warn(csid->camss->dev,
+			 "Sink stream %u frame desc dt=%u differs from format dt=%u, using dt=%u\n",
+			 sink_stream, desc_csi2.dt, format_dt, desc_csi2.dt);
+
+	return desc_csi2;
+}
+
+/*
+ * csid_pad_enable_streams - Enable one or more streams on a source pad
+ * @sd: CSID V4L2 subdevice
+ * @state: V4L2 subdevice state
+ * @pad: Pad number
+ * @streams_mask: Bitmask of v4l2 streams to enable
+ *
+ * The v4l2 core only calls this on a source pad (v4l2_subdev_enable_streams()
+ * rejects sink pads with -EOPNOTSUPP before reaching the driver), so @pad is
+ * not checked here. Each source pad only ever carries stream 0.
+ *
+ * The shared sink stream(s) are propagated upstream only once, on the
+ * transition from no active sink streams to at least one, so that a second
+ * consumer of the same shared sink stream never triggers a second, redundant
+ * propagation to the sensor. The Rx front-end is likewise only configured
+ * once, on that same transition.
+ *
+ * Return 0 on success, -ENOLINK if there is no remote sink link and the test
+ * generator is disabled, or another negative error code otherwise
+ */
+static int csid_pad_enable_streams(struct v4l2_subdev *sd,
+				   struct v4l2_subdev_state *state,
+				   u32 pad, u64 streams_mask)
+{
+	struct csid_device *csid = v4l2_get_subdevdata(sd);
+	const struct csid_hw_ops *hw_ops = csid->res->hw_ops;
+	struct media_pad *remote_pad = media_pad_remote_pad_first(&csid->pads[MSM_CSID_PAD_SINK]);
+	unsigned int hw_port = pad - MSM_CSID_PAD_FIRST_SRC;
+	const struct csid_format_info *format;
+	struct v4l2_mbus_frame_desc_entry_csi2 desc_csi2;
+	u64 sink_streams, propagate_mask;
+	int ret;
+
+	if (!csid->testgen.enabled && !remote_pad)
+		return -ENOLINK;
+
+	sink_streams = v4l2_subdev_state_xlate_streams(state, pad, MSM_CSID_PAD_SINK,
+						       &streams_mask);
+
+	if (!csid->enabled_streams[MSM_CSID_PAD_SINK]) {
+		if (csid->testgen.nmodes != CSID_PAYLOAD_MODE_DISABLED) {
+			/*
+			 * sd->state_lock is aliased to csid->ctrls.lock, and is
+			 * already held here by the v4l2_subdev_enable_streams()
+			 * caller, so use the lock-free variant to avoid
+			 * self-deadlocking on the same mutex.
+			 */
+			ret = __v4l2_ctrl_handler_setup(&csid->ctrls);
+			if (ret < 0) {
+				dev_err(csid->camss->dev,
+					"could not sync v4l2 controls: %d\n", ret);
+				return ret;
+			}
+		}
+
+		hw_ops->configure_rx(csid);
+	}
+
+	/* Sink streams already active elsewhere don't need re-propagating. */
+	propagate_mask = sink_streams & ~csid->enabled_streams[MSM_CSID_PAD_SINK];
+	csid->enabled_streams[MSM_CSID_PAD_SINK] |= sink_streams;
+	csid->enabled_streams[pad] |= streams_mask;
+
+	format = csid_get_fmt_entry(csid->res->formats->formats,
+				    csid->res->formats->nformats,
+				    csid->fmt[pad].code);
+	desc_csi2 = csid_get_stream_vc_dt(csid, state, remote_pad, pad, format->data_type);
+
+	hw_ops->enable_stream(csid, hw_port, desc_csi2.vc, desc_csi2.dt);
+
+	if (propagate_mask && remote_pad) {
+		ret = v4l2_subdev_enable_streams(media_entity_to_v4l2_subdev(remote_pad->entity),
+						 remote_pad->index, propagate_mask);
+		if (ret) {
+			csid->enabled_streams[MSM_CSID_PAD_SINK] &= ~propagate_mask;
+			csid->enabled_streams[pad] &= ~streams_mask;
+
+			hw_ops->disable_stream(csid, hw_port);
+
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+/*
+ * csid_sink_streams_in_use - Compute the subset of sink streams still
+ *			      referenced by a source pad other than @pad
+ * @csid: CSID device
+ * @state: V4L2 subdevice state
+ * @pad: Source pad to exclude from the check
+ * @sink_streams: Candidate sink streams to check
+ *
+ * Return the subset of @sink_streams still referenced by some other source
+ * pad
+ */
+static u64 csid_sink_streams_in_use(struct csid_device *csid, struct v4l2_subdev_state *state,
+				    u32 pad, u64 sink_streams)
+{
+	u64 in_use = 0;
+	unsigned int i;
+
+	for (i = MSM_CSID_PAD_FIRST_SRC; i < MSM_CSID_PADS_NUM; i++) {
+		u64 other_streams = csid->enabled_streams[i];
+		u64 other_sink_streams;
+
+		if (i == pad)
+			continue;
+
+		other_sink_streams = v4l2_subdev_state_xlate_streams(state, i, MSM_CSID_PAD_SINK,
+								     &other_streams);
+		in_use |= sink_streams & other_sink_streams;
+	}
+
+	return in_use;
+}
+
+/*
+ * csid_pad_disable_streams - Disable one or more streams on a source pad
+ * @sd: CSID V4L2 subdevice
+ * @state: V4L2 subdevice state
+ * @pad: Pad number
+ * @streams_mask: Bitmask of v4l2 streams to disable
+ *
+ * The v4l2 core only calls this on a source pad (v4l2_subdev_disable_streams()
+ * rejects sink pads with -EOPNOTSUPP before reaching the driver), so @pad is
+ * not checked here. Each source pad only ever carries stream 0.
+ *
+ * A sink stream is only disabled, and propagated upstream to disable it there
+ * too, once no source pad references it any more.
+ *
+ * Return 0 on success or a negative error code otherwise
+ */
+static int csid_pad_disable_streams(struct v4l2_subdev *sd,
+				    struct v4l2_subdev_state *state,
+				    u32 pad, u64 streams_mask)
+{
+	struct csid_device *csid = v4l2_get_subdevdata(sd);
+	const struct csid_hw_ops *hw_ops = csid->res->hw_ops;
+	struct media_pad *remote_pad = media_pad_remote_pad_first(&csid->pads[MSM_CSID_PAD_SINK]);
+	unsigned int hw_port = pad - MSM_CSID_PAD_FIRST_SRC;
+	u64 sink_streams, disable_sink_streams;
+	int ret = 0;
+
+	sink_streams = v4l2_subdev_state_xlate_streams(state, pad, MSM_CSID_PAD_SINK,
+						       &streams_mask);
+
+	/* Keep a sink stream active as long as any other source pad still uses it. */
+	disable_sink_streams = sink_streams &
+				~csid_sink_streams_in_use(csid, state, pad, sink_streams);
+
+	if (disable_sink_streams && remote_pad) {
+		ret = v4l2_subdev_disable_streams(media_entity_to_v4l2_subdev(remote_pad->entity),
+						  remote_pad->index, disable_sink_streams);
+		if (ret)
+			dev_err(csid->camss->dev,
+				"Failed to disable stream on remote pad: %d\n", ret);
+	}
+
+	hw_ops->disable_stream(csid, hw_port);
+
+	csid->enabled_streams[pad] &= ~streams_mask;
+	csid->enabled_streams[MSM_CSID_PAD_SINK] &= ~disable_sink_streams;
+
+	return ret;
+}
+
+static const struct v4l2_mbus_framefmt csid_default_format = {
+	.code = MEDIA_BUS_FMT_UYVY8_1X16,
+	.width = 1920,
+	.height = 1080,
+	.field = V4L2_FIELD_NONE,
+	.colorspace = V4L2_COLORSPACE_SRGB,
+};
+
+/*
+ * csid_set_routing - Handle setting of routing table
+ * @sd: CSID V4L2 subdevice
+ * @state: V4L2 subdevice state
+ * @which: TRY or ACTIVE routing
+ * @routing: Routing table to set
+ *
+ * Return 0 on success or a negative error code otherwise
+ */
+static int csid_set_routing(struct v4l2_subdev *sd,
+			    struct v4l2_subdev_state *state,
+			    enum v4l2_subdev_format_whence which,
+			    struct v4l2_subdev_krouting *routing)
+{
+	struct csid_device *csid = v4l2_get_subdevdata(sd);
+	unsigned int i;
+	int ret;
+
+	if (which == V4L2_SUBDEV_FORMAT_ACTIVE && csid->enabled_streams[MSM_CSID_PAD_SINK])
+		return -EBUSY;
+
+	for (i = 0; i < routing->num_routes; i++)
+		if (routing->routes[i].source_stream != 0)
+			return -EINVAL;
+
+	ret = v4l2_subdev_routing_validate(sd, routing,
+					   V4L2_SUBDEV_ROUTING_NO_SOURCE_STREAM_MIX |
+					   V4L2_SUBDEV_ROUTING_NO_SOURCE_MULTIPLEXING |
+					   V4L2_SUBDEV_ROUTING_NO_N_TO_1);
+	if (ret)
+		return ret;
+
+	return v4l2_subdev_set_routing_with_fmt(sd, state, routing, &csid_default_format);
+}
+
+/*
+ * __csid_get_stream_format - Get pointer to per-stream format structure
+ * @csid: CSID device
+ * @sd_state: V4L2 subdev state
+ * @pad: pad from which format is requested
+ * @stream: stream from which format is requested
+ * @which: TRY or ACTIVE format
+ *
+ * Same as __csid_get_format(), but honors @stream for TRY-state lookups.
+ * For ACTIVE state, csid->fmt[] is indexed by pad + stream. @stream is
+ * always 0 and @pad selects the RDI channel (0-3).
+ *
+ * Return pointer to TRY or ACTIVE format structure
+ */
+static struct v4l2_mbus_framefmt *
+__csid_get_stream_format(struct csid_device *csid,
+			 struct v4l2_subdev_state *sd_state,
+			 unsigned int pad, u32 stream,
+			 enum v4l2_subdev_format_whence which)
+{
+	if (which == V4L2_SUBDEV_FORMAT_TRY)
+		return v4l2_subdev_state_get_format(sd_state, pad, stream);
+
+	if (pad == MSM_CSID_PAD_SINK)
+		return &csid->fmt[MSM_CSID_PAD_SINK];
+
+	return &csid->fmt[pad + stream];
+}
+
+/*
+ * csid_streams_get_format - Handle get format by pads subdev method
+ * @sd: CSID V4L2 subdevice
+ * @sd_state: V4L2 subdev state
+ * @fmt: pointer to v4l2 subdev format structure
+ *
+ * Return -EINVAL or zero on success
+ */
+static int csid_streams_get_format(struct v4l2_subdev *sd,
+				   struct v4l2_subdev_state *sd_state,
+				   struct v4l2_subdev_format *fmt)
+{
+	struct csid_device *csid = v4l2_get_subdevdata(sd);
+	struct v4l2_mbus_framefmt *format;
+
+	format = __csid_get_stream_format(csid, sd_state, fmt->pad, fmt->stream, fmt->which);
+	if (!format)
+		return -EINVAL;
+
+	fmt->format = *format;
+
+	return 0;
+}
+
+/*
+ * csid_streams_set_format - Handle set format by pads subdev method
+ * @sd: CSID V4L2 subdevice
+ * @sd_state: V4L2 subdev state
+ * @fmt: pointer to v4l2 subdev format structure
+ *
+ * Return -EINVAL or zero on success
+ */
+static int csid_streams_set_format(struct v4l2_subdev *sd,
+				   struct v4l2_subdev_state *sd_state,
+				   struct v4l2_subdev_format *fmt)
+{
+	struct csid_device *csid = v4l2_get_subdevdata(sd);
+	struct v4l2_mbus_framefmt *format;
+	struct v4l2_subdev_route *route;
+
+	if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE && csid->enabled_streams[MSM_CSID_PAD_SINK])
+		return -EBUSY;
+
+	format = __csid_get_stream_format(csid, sd_state, fmt->pad, fmt->stream, fmt->which);
+	if (!format)
+		return -EINVAL;
+
+	csid_try_format(csid, sd_state, fmt->pad, &fmt->format, fmt->which);
+	*format = fmt->format;
+
+	/* Propagate the format from the sink stream to every source stream it feeds */
+	for_each_active_route(&sd_state->routing, route) {
+		struct v4l2_mbus_framefmt *src_format;
+
+		if (route->sink_pad != fmt->pad || route->sink_stream != fmt->stream)
+			continue;
+
+		src_format = __csid_get_stream_format(csid, sd_state, route->source_pad,
+						      route->source_stream, fmt->which);
+		if (!src_format)
+			continue;
+
+		*src_format = fmt->format;
+		csid_try_format(csid, sd_state, route->source_pad, src_format, fmt->which);
+	}
+
+	return 0;
+}
+
+static const struct v4l2_subdev_pad_ops csid_streams_pad_ops = {
+	.enum_mbus_code = csid_enum_mbus_code,
+	.enum_frame_size = csid_enum_frame_size,
+	.get_fmt = csid_streams_get_format,
+	.set_fmt = csid_streams_set_format,
+	.set_routing = csid_set_routing,
+	.enable_streams = csid_pad_enable_streams,
+	.disable_streams = csid_pad_disable_streams,
+};
+
+static const struct v4l2_subdev_video_ops csid_streams_video_ops = {
+	.s_stream = v4l2_subdev_s_stream_helper,
+};
+
+static const struct v4l2_subdev_ops csid_streams_v4l2_ops = {
+	.core = &csid_core_ops,
+	.pad = &csid_streams_pad_ops,
+	.video = &csid_streams_video_ops,
+};
+
+/*
+ * csid_init_state - Initialize the routing table for the streams API subdev
+ * @sd: CSID V4L2 subdevice
+ * @state: V4L2 subdev state
+ *
+ * source_stream is always 0: each source pad MSM_CSID_PAD_FIRST_SRC + i
+ * links to its own independent downstream subdev, and a link's sink side is
+ * validated against the implicit stream 0 exposed by any subdev without
+ * V4L2_SUBDEV_FL_STREAMS (see v4l2_link_validate_get_streams()) — every
+ * downstream VFE line is such a subdev.
+ *
+ * All source pads route from sink_stream 0 by default, fanning the single
+ * incoming stream out to every port; a multi-VC source is supported by
+ * remapping each route's sink_stream via .set_routing, leaving
+ * source_pad/source_stream untouched.
+ *
+ * Return 0 on success or a negative error code otherwise
+ */
+static int csid_init_state(struct v4l2_subdev *sd, struct v4l2_subdev_state *state)
+{
+	struct csid_device *csid = v4l2_get_subdevdata(sd);
+	struct v4l2_subdev_route routes[MSM_CSID_MAX_SRC_STREAMS];
+	struct v4l2_subdev_krouting routing = { };
+	unsigned int num_routes;
+	int i, ret;
+
+	/* The full IFE has only 3 rdi's and pix output is not functional */
+	if (csid_is_lite(csid))
+		num_routes = MSM_CSID_MAX_SRC_STREAMS;
+	else
+		num_routes = MSM_CSID_MAX_SRC_STREAMS - 1;
+
+	for (i = 0; i < num_routes; i++) {
+		routes[i].sink_pad = MSM_CSID_PAD_SINK;
+		routes[i].sink_stream = 0;
+		routes[i].source_pad = MSM_CSID_PAD_FIRST_SRC + i;
+		routes[i].source_stream = 0;
+		routes[i].flags = V4L2_SUBDEV_ROUTE_FL_ACTIVE;
+	}
+
+	routing.num_routes = num_routes;
+	routing.routes = routes;
+	ret = v4l2_subdev_set_routing_with_fmt(sd, state, &routing, &csid_default_format);
+	if (ret)
+		dev_err(csid->camss->dev, "Failed to set routing: %d\n", ret);
+
+	return ret;
+}
+
 static const struct v4l2_subdev_internal_ops csid_v4l2_internal_ops = {
 	.open = csid_init_formats,
 };
 
+static const struct v4l2_subdev_internal_ops csid_streams_internal_ops = {
+	.init_state = csid_init_state,
+};
+
 static const struct media_entity_operations csid_media_ops = {
 	.link_setup = csid_link_setup,
 	.link_validate = v4l2_subdev_link_validate,
@@ -1360,13 +1826,18 @@ int msm_csid_register_entity(struct csid_device *csid,
 	struct v4l2_subdev *sd = &csid->subdev;
 	struct media_pad *pads = csid->pads;
 	struct device *dev = csid->camss->dev;
+	bool streams_api = csid->res->streams_enable;
+	unsigned int num_pads = csid_is_lite(csid) ? MSM_CSID_PADS_NUM : MSM_CSID_PADS_NUM - 1;
 	int i;
 	int ret;
 
-	v4l2_subdev_init(sd, &csid_v4l2_ops);
-	sd->internal_ops = &csid_v4l2_internal_ops;
+	v4l2_subdev_init(sd, streams_api ? &csid_streams_v4l2_ops : &csid_v4l2_ops);
+	sd->internal_ops = streams_api ? &csid_streams_internal_ops
+					: &csid_v4l2_internal_ops;
 	sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE |
 		     V4L2_SUBDEV_FL_HAS_EVENTS;
+	if (streams_api)
+		sd->flags |= V4L2_SUBDEV_FL_STREAMS;
 	snprintf(sd->name, ARRAY_SIZE(sd->name), "%s%d",
 		 MSM_CSID_NAME, csid->id);
 	v4l2_set_subdevdata(sd, csid);
@@ -1400,17 +1871,28 @@ int msm_csid_register_entity(struct csid_device *csid,
 	}
 
 	pads[MSM_CSID_PAD_SINK].flags = MEDIA_PAD_FL_SINK;
-	for (i = MSM_CSID_PAD_FIRST_SRC; i < MSM_CSID_PADS_NUM; ++i)
+	for (i = MSM_CSID_PAD_FIRST_SRC; i < num_pads; ++i)
 		pads[i].flags = MEDIA_PAD_FL_SOURCE;
 
 	sd->entity.function = MEDIA_ENT_F_PROC_VIDEO_PIXEL_FORMATTER;
 	sd->entity.ops = &csid_media_ops;
-	ret = media_entity_pads_init(&sd->entity, MSM_CSID_PADS_NUM, pads);
+	ret = media_entity_pads_init(&sd->entity, num_pads, pads);
 	if (ret < 0) {
 		dev_err(dev, "Failed to init media entity: %d\n", ret);
 		goto free_ctrl;
 	}
 
+	if (streams_api) {
+		if (csid->testgen.nmodes != CSID_PAYLOAD_MODE_DISABLED)
+			sd->state_lock = csid->ctrls.lock;
+
+		ret = v4l2_subdev_init_finalize(sd);
+		if (ret) {
+			dev_err(dev, "Failed to finalize subdev: %d\n", ret);
+			goto media_cleanup;
+		}
+	}
+
 	ret = v4l2_device_register_subdev(v4l2_dev, sd);
 	if (ret < 0) {
 		dev_err(dev, "Failed to register subdev: %d\n", ret);
@@ -1420,6 +1902,7 @@ int msm_csid_register_entity(struct csid_device *csid,
 	return 0;
 
 media_cleanup:
+	v4l2_subdev_cleanup(sd);
 	media_entity_cleanup(&sd->entity);
 free_ctrl:
 	if (csid->testgen.nmodes != CSID_PAYLOAD_MODE_DISABLED)
@@ -1435,6 +1918,7 @@ int msm_csid_register_entity(struct csid_device *csid,
 void msm_csid_unregister_entity(struct csid_device *csid)
 {
 	v4l2_device_unregister_subdev(&csid->subdev);
+	v4l2_subdev_cleanup(&csid->subdev);
 	media_entity_cleanup(&csid->subdev.entity);
 	if (csid->testgen.nmodes != CSID_PAYLOAD_MODE_DISABLED)
 		v4l2_ctrl_handler_free(&csid->ctrls);
diff --git a/drivers/media/platform/qcom/camss/camss-csid.h b/drivers/media/platform/qcom/camss/camss-csid.h
index 90ee611b9092..a312103cf86d 100644
--- a/drivers/media/platform/qcom/camss/camss-csid.h
+++ b/drivers/media/platform/qcom/camss/camss-csid.h
@@ -184,6 +184,7 @@ struct csid_hw_ops {
 
 struct csid_subdev_resources {
 	bool is_lite;
+	bool streams_enable;
 	const struct csid_hw_ops *hw_ops;
 	const struct parent_dev_ops *parent_dev_ops;
 	const struct csid_formats *formats;
@@ -209,6 +210,7 @@ struct csid_device {
 	struct v4l2_mbus_framefmt fmt[MSM_CSID_PADS_NUM];
 	struct v4l2_ctrl_handler ctrls;
 	struct v4l2_ctrl *testgen_mode;
+	u64 enabled_streams[MSM_CSID_PADS_NUM];
 	const struct csid_subdev_resources *res;
 };
 
-- 
2.34.1

[PATCH 5/8] media: qcom: camss: Fix CSID-to-VFE all-to-all link crossbar on sm8250

[Gjorgji.Rosikopulos.gjorgji.rosikopulos]

From: Gjorgji Rosikopulos <gjorgji.rosikopulos@oss.qualcomm.com>

camss_link_entities() unconditionally links every CSID to every VFE,
creating an all-to-all crossbar. On SM8250 the hardware wiring is
fixed: each CSID is hardwired to exactly one VFE (csid[i] <-> vfe[i]),
with no crossbar between instances. Enabling a mismatched link (e.g.
csid0 -> vfe1) creates a media link that does not correspond to any
real hardware datapath.

Add a csid_vfe_fixed_pairing flag to struct camss_resources and set it
for sm8250_resources. When set, camss_link_entities() skips creating
links between CSID and VFE instances whose indices do not match.
Other platforms keep the historical all-to-all link creation.

Signed-off-by: Gjorgji Rosikopulos <gjorgji.rosikopulos@oss.qualcomm.com>
---
 drivers/media/platform/qcom/camss/camss.c | 7 ++++++-
 drivers/media/platform/qcom/camss/camss.h | 7 +++++++
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
index 2123f6388e3d..1bb22cd23c5e 100644
--- a/drivers/media/platform/qcom/camss/camss.c
+++ b/drivers/media/platform/qcom/camss/camss.c
@@ -5000,7 +5000,10 @@ static int camss_link_entities(struct camss *camss)
 				}
 	} else {
 		for (i = 0; i < camss->res->csid_num; i++)
-			for (k = 0; k < camss->res->vfe_num; k++)
+			for (k = 0; k < camss->res->vfe_num; k++) {
+				if (camss->res->csid_vfe_fixed_pairing && i != k)
+					continue;
+
 				for (j = 0; j < camss->vfe[k].res->line_num; j++) {
 					struct v4l2_subdev *csid = &camss->csid[i].subdev;
 					struct v4l2_subdev *vfe = &camss->vfe[k].line[j].subdev;
@@ -5017,6 +5020,7 @@ static int camss_link_entities(struct camss *camss)
 						return ret;
 					}
 				}
+			}
 	}
 
 	return 0;
@@ -5666,6 +5670,7 @@ static const struct camss_resources sm8250_resources = {
 	.csiphy_num = ARRAY_SIZE(csiphy_res_8250),
 	.csid_num = ARRAY_SIZE(csid_res_8250),
 	.vfe_num = ARRAY_SIZE(vfe_res_8250),
+	.csid_vfe_fixed_pairing = true,
 };
 
 static const struct camss_resources sc8280xp_resources = {
diff --git a/drivers/media/platform/qcom/camss/camss.h b/drivers/media/platform/qcom/camss/camss.h
index 93d691c8ac63..ad0b42719788 100644
--- a/drivers/media/platform/qcom/camss/camss.h
+++ b/drivers/media/platform/qcom/camss/camss.h
@@ -119,6 +119,13 @@ struct camss_resources {
 	const unsigned int tpg_num;
 	const unsigned int csid_num;
 	const unsigned int vfe_num;
+	/*
+	 * True on platforms where each CSID is wired to exactly one VFE at
+	 * the same index (csid[i] <-> vfe[i]). When set, camss_link_entities()
+	 * only creates CSID->VFE links for matching indices instead of an
+	 * all-to-all crossbar.
+	 */
+	const bool csid_vfe_fixed_pairing;
 };
 
 struct camss {
-- 
2.34.1

[PATCH 6/8] media: qcom: camss: add streams API support for VFE

[Gjorgji.Rosikopulos.gjorgji.rosikopulos]

From: Gjorgji Rosikopulos <gjorgji.rosikopulos@oss.qualcomm.com>

Add the V4L2 subdev streams API to the VFE driver. VFE lines are
inherently single-consumer, so no per-stream refcounting is needed:
enable_streams reserves the output and enables the line, then
propagates a single-stream enable to the CSID; disable_streams mirrors
this in reverse.

On disable_streams, both the local VFE hardware disable and the
propagated disable to the remote CSID pad are attempted
unconditionally, and a failure on either side is logged rather than
skipping the other side's teardown or attempting a rollback. This
matches the best-effort error-handling convention used by other
streams-API drivers (e.g. dw-mipi-csi2rx, ds90ub960, cdns-csi2rx) for
this same situation, since there is no way to un-disable a peer that
already tore itself down. If the local disable also failed, its error
takes precedence in the value returned to the core.

This is opt-in per VFE instance via the new streams_enable resource
flag; no platform sets it yet, so VFE lines continue to use the
legacy non-streams subdev ops unchanged.

Signed-off-by: Gjorgji Rosikopulos <gjorgji.rosikopulos@oss.qualcomm.com>
---
 drivers/media/platform/qcom/camss/camss-vfe.c | 119 +++++++++++++++++-
 drivers/media/platform/qcom/camss/camss-vfe.h |   1 +
 2 files changed, 118 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c b/drivers/media/platform/qcom/camss/camss-vfe.c
index 319d19158988..84da6600cf99 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe.c
@@ -1993,6 +1993,104 @@ static const struct v4l2_subdev_internal_ops vfe_v4l2_internal_ops = {
 	.open = vfe_init_formats,
 };
 
+/*
+ * vfe_pad_enable_streams - Enable one or more streams on the source pad
+ * @sd: VFE V4L2 subdevice
+ * @state: V4L2 subdevice state
+ * @pad: Pad number
+ * @streams_mask: Bitmask of streams to enable
+ *
+ * VFE lines are inherently single-consumer (vfe_link_setup() enforces one
+ * link per pad, and each line exposes only the implicit stream 0 of a
+ * non-streams subdev), so no local refcount is needed here.
+ *
+ * Return 0 on success or a negative error code otherwise
+ */
+static int vfe_pad_enable_streams(struct v4l2_subdev *sd, struct v4l2_subdev_state *state,
+				  u32 pad, u64 streams_mask)
+{
+	struct vfe_line *line = v4l2_get_subdevdata(sd);
+	struct vfe_device *vfe = to_vfe(line);
+	struct media_pad *sink_pad = &line->pads[MSM_VFE_PAD_SINK];
+	struct media_pad *remote_pad = media_pad_remote_pad_first(sink_pad);
+	int ret;
+
+	line->output.state = VFE_OUTPUT_RESERVED;
+	ret = vfe->res->hw_ops->vfe_enable(line);
+	if (ret)
+		return ret;
+
+	if (remote_pad) {
+		ret = v4l2_subdev_enable_streams(media_entity_to_v4l2_subdev(remote_pad->entity),
+						 remote_pad->index, BIT_ULL(0));
+		if (ret) {
+			vfe->res->hw_ops->vfe_disable(line);
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+/*
+ * vfe_pad_disable_streams - Disable one or more streams on the source pad
+ * @sd: VFE V4L2 subdevice
+ * @state: V4L2 subdevice state
+ * @pad: Pad number
+ * @streams_mask: Bitmask of streams to disable
+ *
+ * Local and remote teardown are both attempted unconditionally on a
+ * best-effort basis, matching the convention used by other streams-API
+ * drivers (e.g. dw-mipi-csi2rx, ds90ub960, cdns-csi2rx): a failure on
+ * either side is logged and does not skip the other side's teardown, since
+ * there is no way to roll back a partially disabled pipeline. If the local
+ * disable also failed, its error takes precedence in the return value.
+ *
+ * Return 0 on success or a negative error code otherwise
+ */
+static int vfe_pad_disable_streams(struct v4l2_subdev *sd, struct v4l2_subdev_state *state,
+				   u32 pad, u64 streams_mask)
+{
+	struct vfe_line *line = v4l2_get_subdevdata(sd);
+	struct vfe_device *vfe = to_vfe(line);
+	struct media_pad *sink_pad = &line->pads[MSM_VFE_PAD_SINK];
+	struct media_pad *remote_pad = media_pad_remote_pad_first(sink_pad);
+	int ret;
+
+	ret = vfe->res->hw_ops->vfe_disable(line);
+
+	if (remote_pad) {
+		struct v4l2_subdev *remote_sd = media_entity_to_v4l2_subdev(remote_pad->entity);
+		int remote_ret;
+
+		remote_ret = v4l2_subdev_disable_streams(remote_sd, remote_pad->index, BIT_ULL(0));
+		if (remote_ret) {
+			dev_err(vfe->camss->dev,
+				"Failed to disable stream on remote pad: %d\n", remote_ret);
+			if (!ret)
+				ret = remote_ret;
+		}
+	}
+
+	return ret;
+}
+
+static const struct v4l2_subdev_pad_ops vfe_streams_pad_ops = {
+	.enum_mbus_code = vfe_enum_mbus_code,
+	.enum_frame_size = vfe_enum_frame_size,
+	.get_fmt = vfe_get_format,
+	.set_fmt = vfe_set_format,
+	.get_selection = vfe_get_selection,
+	.set_selection = vfe_set_selection,
+	.enable_streams = vfe_pad_enable_streams,
+	.disable_streams = vfe_pad_disable_streams,
+};
+
+static const struct v4l2_subdev_ops vfe_streams_v4l2_ops = {
+	.core = &vfe_core_ops,
+	.pad = &vfe_streams_pad_ops,
+};
+
 static const struct media_entity_operations vfe_media_ops = {
 	.link_setup = vfe_link_setup,
 	.link_validate = v4l2_subdev_link_validate,
@@ -2070,7 +2168,8 @@ int msm_vfe_register_entities(struct vfe_device *vfe,
 		pads = vfe->line[i].pads;
 		video_out = &vfe->line[i].video_out;
 
-		v4l2_subdev_init(sd, &vfe_v4l2_ops);
+		v4l2_subdev_init(sd, vfe->res->streams_enable ? &vfe_streams_v4l2_ops
+								   : &vfe_v4l2_ops);
 		sd->internal_ops = &vfe_v4l2_internal_ops;
 		sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
 		if (i == VFE_LINE_PIX && vfe->res->is_lite == false)
@@ -2100,10 +2199,18 @@ int msm_vfe_register_entities(struct vfe_device *vfe,
 			goto error_init;
 		}
 
+		if (vfe->res->streams_enable) {
+			ret = v4l2_subdev_init_finalize(sd);
+			if (ret) {
+				dev_err(dev, "Failed to finalize subdev: %d\n", ret);
+				goto error_reg_subdev;
+			}
+		}
+
 		ret = v4l2_device_register_subdev(v4l2_dev, sd);
 		if (ret < 0) {
 			dev_err(dev, "Failed to register subdev: %d\n", ret);
-			goto error_reg_subdev;
+			goto error_subdev_cleanup;
 		}
 
 		video_out->ops = &vfe->video_ops;
@@ -2147,6 +2254,10 @@ int msm_vfe_register_entities(struct vfe_device *vfe,
 error_reg_video:
 	v4l2_device_unregister_subdev(sd);
 
+error_subdev_cleanup:
+	if (vfe->res->streams_enable)
+		v4l2_subdev_cleanup(sd);
+
 error_reg_subdev:
 	media_entity_cleanup(&sd->entity);
 
@@ -2157,6 +2268,8 @@ int msm_vfe_register_entities(struct vfe_device *vfe,
 
 		msm_video_unregister(video_out);
 		v4l2_device_unregister_subdev(sd);
+		if (vfe->res->streams_enable)
+			v4l2_subdev_cleanup(sd);
 		media_entity_cleanup(&sd->entity);
 	}
 
@@ -2180,6 +2293,8 @@ void msm_vfe_unregister_entities(struct vfe_device *vfe)
 
 		msm_video_unregister(video_out);
 		v4l2_device_unregister_subdev(sd);
+		if (vfe->res->streams_enable)
+			v4l2_subdev_cleanup(sd);
 		media_entity_cleanup(&sd->entity);
 	}
 }
diff --git a/drivers/media/platform/qcom/camss/camss-vfe.h b/drivers/media/platform/qcom/camss/camss-vfe.h
index ae9dad353a37..d1c54a516ba1 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe.h
+++ b/drivers/media/platform/qcom/camss/camss-vfe.h
@@ -138,6 +138,7 @@ struct vfe_subdev_resources {
 	char *pd_name;
 	bool has_vbif;
 	char *vbif_name;
+	bool streams_enable;
 	const struct vfe_hw_ops *hw_ops;
 	const struct camss_formats *formats_rdi;
 	const struct camss_formats *formats_pix;
-- 
2.34.1

[PATCH 7/8] media: qcom: camss: add streams API support in camss-video

[Gjorgji.Rosikopulos.gjorgji.rosikopulos]

From: Gjorgji Rosikopulos <gjorgji.rosikopulos@oss.qualcomm.com>

Generalize the video device pipeline walk to be streams-API-aware:
when a directly-connected subdev supports enable_streams/
disable_streams, use it; otherwise fall back to the existing legacy
start/stop path, which walks the pipeline one subdev at a time via
.s_stream(), unchanged.

video_stop_streaming() must always call video_device_pipeline_stop()
and flush_buffers(), regardless of whether the disable path succeeded:
vb2 requires stop_streaming() to return all buffers, and a successful
pipeline_alloc_start() must always be matched by a pipeline_stop().
Make the legacy fallback branch match the streams-API branch by
logging a failure instead of returning early on it.

Signed-off-by: Gjorgji Rosikopulos <gjorgji.rosikopulos@oss.qualcomm.com>
---
 .../media/platform/qcom/camss/camss-video.c   | 119 ++++++++++++++----
 1 file changed, 94 insertions(+), 25 deletions(-)

diff --git a/drivers/media/platform/qcom/camss/camss-video.c b/drivers/media/platform/qcom/camss/camss-video.c
index 0852eb6f1315..7f7212996c4a 100644
--- a/drivers/media/platform/qcom/camss/camss-video.c
+++ b/drivers/media/platform/qcom/camss/camss-video.c
@@ -249,26 +249,27 @@ static int video_prepare_streaming(struct vb2_queue *q)
 	return ret;
 }
 
-static int video_start_streaming(struct vb2_queue *q, unsigned int count)
+/*
+ * video_start_streaming_legacy - Walk the whole subdev chain and enable
+ *                                streaming via .s_stream()
+ * @video: Video device
+ * @sink_pad: Video device's own (single) pad
+ *
+ * Fallback used when the subdev directly connected to @sink_pad has no
+ * .enable_streams op. Mirrors the pre-streams-API pipeline walk: each
+ * subdev's pad 0 is assumed to be its sink pad, and streaming is started
+ * one subdev at a time via .s_stream() rather than the streams API.
+ *
+ * Return 0 on success or a negative error code otherwise
+ */
+static int video_start_streaming_legacy(struct camss_video *video,
+					struct media_pad *sink_pad)
 {
-	struct camss_video *video = vb2_get_drv_priv(q);
-	struct video_device *vdev = &video->vdev;
-	struct media_entity *entity;
+	struct media_entity *entity = sink_pad->entity;
 	struct media_pad *pad;
 	struct v4l2_subdev *subdev;
 	int ret;
 
-	ret = video_device_pipeline_alloc_start(vdev);
-	if (ret < 0) {
-		dev_err(video->camss->dev, "Failed to start media pipeline: %d\n", ret);
-		goto flush_buffers;
-	}
-
-	ret = video_check_format(video);
-	if (ret < 0)
-		goto error;
-
-	entity = &vdev->entity;
 	while (1) {
 		pad = &entity->pads[0];
 		if (!(pad->flags & MEDIA_PAD_FL_SINK))
@@ -283,6 +284,42 @@ static int video_start_streaming(struct vb2_queue *q, unsigned int count)
 
 		ret = v4l2_subdev_call(subdev, video, s_stream, 1);
 		if (ret < 0 && ret != -ENOIOCTLCMD)
+			return ret;
+	}
+
+	return 0;
+}
+
+static int video_start_streaming(struct vb2_queue *q, unsigned int count)
+{
+	struct camss_video *video = vb2_get_drv_priv(q);
+	struct video_device *vdev = &video->vdev;
+	struct media_pad *sink_pad = &vdev->entity.pads[0];
+	struct media_pad *src_pad;
+	struct v4l2_subdev *subdev = NULL;
+	int ret;
+
+	ret = video_device_pipeline_alloc_start(vdev);
+	if (ret < 0) {
+		dev_err(video->camss->dev, "Failed to start media pipeline: %d\n", ret);
+		goto flush_buffers;
+	}
+
+	ret = video_check_format(video);
+	if (ret < 0)
+		goto error;
+
+	src_pad = media_pad_remote_pad_first(sink_pad);
+	if (src_pad && is_media_entity_v4l2_subdev(src_pad->entity))
+		subdev = media_entity_to_v4l2_subdev(src_pad->entity);
+
+	if (subdev && v4l2_subdev_has_op(subdev, pad, enable_streams)) {
+		ret = v4l2_subdev_enable_streams(subdev, src_pad->index, BIT_ULL(0));
+		if (ret < 0)
+			goto error;
+	} else {
+		ret = video_start_streaming_legacy(video, sink_pad);
+		if (ret < 0)
 			goto error;
 	}
 
@@ -297,16 +334,27 @@ static int video_start_streaming(struct vb2_queue *q, unsigned int count)
 	return ret;
 }
 
-static void video_stop_streaming(struct vb2_queue *q)
+/*
+ * video_stop_streaming_legacy - Walk the whole subdev chain and disable
+ *                               streaming via .s_stream()
+ * @video: Video device
+ * @sink_pad: Video device's own (single) pad
+ *
+ * Fallback used when the subdev directly connected to @sink_pad has no
+ * .disable_streams op. Mirrors the pre-streams-API pipeline walk: each
+ * subdev's pad 0 is assumed to be its sink pad, and streaming is stopped
+ * one subdev at a time via .s_stream() rather than the streams API.
+ *
+ * Return 0 on success or a negative error code otherwise
+ */
+static int video_stop_streaming_legacy(struct camss_video *video,
+				       struct media_pad *sink_pad)
 {
-	struct camss_video *video = vb2_get_drv_priv(q);
-	struct video_device *vdev = &video->vdev;
-	struct media_entity *entity;
+	struct media_entity *entity = sink_pad->entity;
 	struct media_pad *pad;
 	struct v4l2_subdev *subdev;
 	int ret;
 
-	entity = &vdev->entity;
 	while (1) {
 		pad = &entity->pads[0];
 		if (!(pad->flags & MEDIA_PAD_FL_SINK))
@@ -320,13 +368,34 @@ 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)
+			return ret;
 	}
 
+	return 0;
+}
+
+static void video_stop_streaming(struct vb2_queue *q)
+{
+	struct camss_video *video = vb2_get_drv_priv(q);
+	struct video_device *vdev = &video->vdev;
+	struct media_pad *sink_pad = &vdev->entity.pads[0];
+	struct media_pad *src_pad;
+	struct v4l2_subdev *subdev = NULL;
+	int ret;
+
+	src_pad = media_pad_remote_pad_first(sink_pad);
+	if (src_pad && is_media_entity_v4l2_subdev(src_pad->entity))
+		subdev = media_entity_to_v4l2_subdev(src_pad->entity);
+
+	if (subdev && v4l2_subdev_has_op(subdev, pad, disable_streams))
+		ret = v4l2_subdev_disable_streams(subdev, src_pad->index, BIT_ULL(0));
+	else
+		ret = video_stop_streaming_legacy(video, sink_pad);
+
+	if (ret)
+		dev_err(video->camss->dev, "Video pipeline stop failed: %d\n", ret);
+
 	video_device_pipeline_stop(vdev);
 
 	video->ops->flush_buffers(video, VB2_BUF_STATE_ERROR);
-- 
2.34.1

[PATCH 8/8] media: qcom: camss: enable streams API on SM8250

[Gjorgji.Rosikopulos.gjorgji.rosikopulos]

From: Gjorgji Rosikopulos <gjorgji.rosikopulos@oss.qualcomm.com>

Now that CSIPHY, CSID, and VFE support the V4L2 subdev streams API,
and CSID-to-VFE linking is fixed to the correct 1:1 pairing, switch
SM8250 over: set streams_enable on every CSIPHY, CSID, and VFE line
resource entry in csiphy_res_8250[], csid_res_8250[], and
vfe_res_8250[].

No other platform is affected; they keep using the legacy non-streams
subdev ops.

Signed-off-by: Gjorgji Rosikopulos <gjorgji.rosikopulos@oss.qualcomm.com>
---
 drivers/media/platform/qcom/camss/camss.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
index 1bb22cd23c5e..9bfb0290395b 100644
--- a/drivers/media/platform/qcom/camss/camss.c
+++ b/drivers/media/platform/qcom/camss/camss.c
@@ -1964,6 +1964,7 @@ static const struct camss_subdev_resources csiphy_res_8250[] = {
 		.interrupt = { "csiphy0" },
 		.csiphy = {
 			.id = 0,
+			.streams_enable = true,
 			.hw_ops = &csiphy_ops_3ph_1_0,
 			.formats = &csiphy_formats_sdm845
 		}
@@ -1981,6 +1982,7 @@ static const struct camss_subdev_resources csiphy_res_8250[] = {
 		.interrupt = { "csiphy1" },
 		.csiphy = {
 			.id = 1,
+			.streams_enable = true,
 			.hw_ops = &csiphy_ops_3ph_1_0,
 			.formats = &csiphy_formats_sdm845
 		}
@@ -1998,6 +2000,7 @@ static const struct camss_subdev_resources csiphy_res_8250[] = {
 		.interrupt = { "csiphy2" },
 		.csiphy = {
 			.id = 2,
+			.streams_enable = true,
 			.hw_ops = &csiphy_ops_3ph_1_0,
 			.formats = &csiphy_formats_sdm845
 		}
@@ -2015,6 +2018,7 @@ static const struct camss_subdev_resources csiphy_res_8250[] = {
 		.interrupt = { "csiphy3" },
 		.csiphy = {
 			.id = 3,
+			.streams_enable = true,
 			.hw_ops = &csiphy_ops_3ph_1_0,
 			.formats = &csiphy_formats_sdm845
 		}
@@ -2032,6 +2036,7 @@ static const struct camss_subdev_resources csiphy_res_8250[] = {
 		.interrupt = { "csiphy4" },
 		.csiphy = {
 			.id = 4,
+			.streams_enable = true,
 			.hw_ops = &csiphy_ops_3ph_1_0,
 			.formats = &csiphy_formats_sdm845
 		}
@@ -2049,6 +2054,7 @@ static const struct camss_subdev_resources csiphy_res_8250[] = {
 		.interrupt = { "csiphy5" },
 		.csiphy = {
 			.id = 5,
+			.streams_enable = true,
 			.hw_ops = &csiphy_ops_3ph_1_0,
 			.formats = &csiphy_formats_sdm845
 		}
@@ -2068,6 +2074,7 @@ static const struct camss_subdev_resources csid_res_8250[] = {
 		.reg = { "csid0" },
 		.interrupt = { "csid0" },
 		.csid = {
+			.streams_enable = true,
 			.hw_ops = &csid_ops_gen2,
 			.parent_dev_ops = &vfe_parent_dev_ops,
 			.formats = &csid_formats_gen2
@@ -2085,6 +2092,7 @@ static const struct camss_subdev_resources csid_res_8250[] = {
 		.reg = { "csid1" },
 		.interrupt = { "csid1" },
 		.csid = {
+			.streams_enable = true,
 			.hw_ops = &csid_ops_gen2,
 			.parent_dev_ops = &vfe_parent_dev_ops,
 			.formats = &csid_formats_gen2
@@ -2102,6 +2110,7 @@ static const struct camss_subdev_resources csid_res_8250[] = {
 		.interrupt = { "csid2" },
 		.csid = {
 			.is_lite = true,
+			.streams_enable = true,
 			.hw_ops = &csid_ops_gen2,
 			.parent_dev_ops = &vfe_parent_dev_ops,
 			.formats = &csid_formats_gen2
@@ -2119,6 +2128,7 @@ static const struct camss_subdev_resources csid_res_8250[] = {
 		.interrupt = { "csid3" },
 		.csid = {
 			.is_lite = true,
+			.streams_enable = true,
 			.hw_ops = &csid_ops_gen2,
 			.parent_dev_ops = &vfe_parent_dev_ops,
 			.formats = &csid_formats_gen2
@@ -2148,6 +2158,7 @@ static const struct camss_subdev_resources vfe_res_8250[] = {
 			.line_num = 3,
 			.has_pd = true,
 			.pd_name = "ife0",
+			.streams_enable = true,
 			.hw_ops = &vfe_ops_480,
 			.formats_rdi = &vfe_formats_rdi_845,
 			.formats_pix = &vfe_formats_pix_845
@@ -2174,6 +2185,7 @@ static const struct camss_subdev_resources vfe_res_8250[] = {
 			.line_num = 3,
 			.has_pd = true,
 			.pd_name = "ife1",
+			.streams_enable = true,
 			.hw_ops = &vfe_ops_480,
 			.formats_rdi = &vfe_formats_rdi_845,
 			.formats_pix = &vfe_formats_pix_845
@@ -2198,6 +2210,7 @@ static const struct camss_subdev_resources vfe_res_8250[] = {
 		.vfe = {
 			.is_lite = true,
 			.line_num = 4,
+			.streams_enable = true,
 			.hw_ops = &vfe_ops_480,
 			.formats_rdi = &vfe_formats_rdi_845,
 			.formats_pix = &vfe_formats_pix_845
@@ -2222,6 +2235,7 @@ static const struct camss_subdev_resources vfe_res_8250[] = {
 		.vfe = {
 			.is_lite = true,
 			.line_num = 4,
+			.streams_enable = true,
 			.hw_ops = &vfe_ops_480,
 			.formats_rdi = &vfe_formats_rdi_845,
 			.formats_pix = &vfe_formats_pix_845
-- 
2.34.1

Re: [PATCH 0/8] media: qcom: camss: add V4L2 subdev streams API support

[Bryan O'Donoghue]

On 11/09/2026 07:22, 
Gjorgji.Rosikopulos.gjorgji.rosikopulos@oss.qualcomm.com wrote:
> From: Gjorgji Rosikopulos <gjorgji.rosikopulos@oss.qualcomm.com>
> 
> This series adds V4L2 subdev streams API support to the CAMSS driver.

Can you please provide a use-case and test in your overview.

i.e. show what it does and show it doesn't break anything in a way a 
reviewer can test ?

> Each subdevice gains streams-aware enable_streams/disable_streams pad
> ops alongside the existing legacy (non-streams) subdev ops, guarded by
> a new per-instance streams_enable resource flag.
> 
> Patches 1-4 add the CSIPHY/CSID mechanism:
>   - CSIPHY: passthrough routing, NO_STREAM_MIX/NO_N_TO_1 validation, and
>     shared D-PHY lane enable/disable gated on stream-count transitions.
>   - CSID: per-source-pad routing (a single sink stream propagated to
>     every source pad by default, remappable for multi-VC sensors),
>     VC/DT discovery via get_frame_desc, and new hw_ops
>     (configure_rx/enable_stream/disable_stream) with a gen2 backend
>     implementation.
> 
> Patch 5 is a standalone bug fix, independent of the streams API:
> camss_link_entities() used to create an all-to-all CSID-to-VFE
> crossbar, but SM8250's hardware wiring is a fixed 1:1 pairing
> (csid[i] <-> vfe[i]). Enabling a mismatched link (e.g. csid0 -> vfe1)
> exposed a media link with no real hardware datapath. Fixed via an
> opt-in csid_vfe_fixed_pairing flag, set only for sm8250_resources.
> 
> Patches 6-8 complete the mechanism and turn it on for real hardware:
>   - VFE: streams-aware pad ops. VFE lines are inherently single-consumer
>     (vfe_link_setup() enforces one link per pad), so no refcounting is
>     needed there.
>   - camss-video: the video device pipeline walk now checks, via
>     v4l2_subdev_has_op(), whether the directly-connected subdev supports
>     enable_streams/disable_streams; if so it issues a single top-level
>     call instead of manually walking the pipeline one subdev at a time
>     with .s_stream(). Falls back to the existing legacy path unchanged
>     when the remote subdev doesn't support the streams API, so no other
>     platform is affected.
>   - SM8250: streams_enable is set true on every CSIPHY, CSID, and VFE
>     line resource entry, turning the mechanism on for real hardware.
>     Every other platform keeps using the legacy non-streams subdev ops,
>     so this is a no-op everywhere else.
> 
> A practical benefit of the CSID routing change (patch 4) is routing
> flexibility for multi-VC sensors: the CSID's routing table maps sink
> streams to source pads/streams via userspace-configurable
> v4l2_subdev_route entries instead of a fixed pad<->VC assignment, so a
> sensor emitting multiple virtual channels can have each VC directed to
> a different RDI output (and thus a different VFE line/video node)
> with a set_routing call, rather than being constrained to whatever
> fixed mapping the driver hardcodes.
> 
> When a sink stream is shared by multiple source pads/streams, CSID
> only enables the corresponding upstream CSIPHY stream on the first
> source stream that needs it, and only disables it once the last
> remaining source stream using it is disabled. Enabling or disabling
> additional consumers of an already-active shared stream is a no-op
> upstream, so no consumer can double-enable or prematurely disable a
> stream still in use by another. This also avoids ever hitting v4l2
> core's own -EALREADY re-enable gate.
> 
> Verified clean with checkpatch --strict. Built, flashed, and tested on
> RB5/SM8250 hardware; ran the no-routing capture verification test
> across all four CSID/VFE RDI pairs (csid0->vfe0, csid1->vfe1,
> csid2->vfe2, csid3->vfe3) at 4056x3040 - all four passed with
> correctly-sized frame captures.

What's that - please detail your exact steps in the cover letter. What I 
need to see in the first instance is that nothing breaks.

Maybe try running libcamera cam with or without gpuisp. Show some yavta 
commands to prove nothing breaks and then something to show how to use 
your code.

> 
> Gjorgji Rosikopulos (8):
>    media: qcom: camss: Add streams API support for CSIPHY
>    media: qcom: camss: Add streams API hw_ops to CSID interface
>    media: qcom: camss: Implement CSID streams API hw_ops for gen2
>    media: qcom: camss: Add streams API support in CSID subdevice
>    media: qcom: camss: Fix CSID-to-VFE all-to-all link crossbar on sm8250
>    media: qcom: camss: add streams API support for VFE
>    media: qcom: camss: add streams API support in camss-video
>    media: qcom: camss: enable streams API on SM8250
> 
>   .../platform/qcom/camss/camss-csid-gen2.c     |  59 ++-
>   .../media/platform/qcom/camss/camss-csid.c    | 494 +++++++++++++++++-
>   .../media/platform/qcom/camss/camss-csid.h    |  45 ++
>   .../media/platform/qcom/camss/camss-csiphy.c  | 223 +++++++-
>   .../media/platform/qcom/camss/camss-csiphy.h  |   2 +
>   drivers/media/platform/qcom/camss/camss-vfe.c | 119 ++++-
>   drivers/media/platform/qcom/camss/camss-vfe.h |   1 +
>   .../media/platform/qcom/camss/camss-video.c   | 119 ++++-
>   drivers/media/platform/qcom/camss/camss.c     |  21 +-
>   drivers/media/platform/qcom/camss/camss.h     |   7 +
>   10 files changed, 1046 insertions(+), 44 deletions(-)
> 

Re: [PATCH 1/8] media: qcom: camss: Add streams API support for CSIPHY

[Bryan O'Donoghue]

On 11/09/2026 07:22, 
Gjorgji.Rosikopulos.gjorgji.rosikopulos@oss.qualcomm.com wrote:
> From: Gjorgji Rosikopulos <gjorgji.rosikopulos@oss.qualcomm.com>
> 
> Add the V4L2 subdev streams API to the CSIPHY driver: a passthrough
> routing table (one VC per stream) and NO_STREAM_MIX routing validation.
> 
> enable/disable_streams pad ops track active streams per pad via a
> per-pad enabled_streams[] bitmask, so that multiple source-pad
> consumers can share a single propagated sink stream without redundant
> or colliding propagation to the sensor. The shared D-PHY lanes are
> enabled once, on the transition from no active streams to at least
> one, and disabled once all streams have gone idle.
> 
> This is opt-in per CSIPHY instance via the new streams_enable resource
> flag; no platform sets it yet, so CSIPHYs continue to use the legacy
> non-streams subdev ops unchanged.
> 
> Signed-off-by: Gjorgji Rosikopulos <gjorgji.rosikopulos@oss.qualcomm.com>
> ---
>   .../media/platform/qcom/camss/camss-csiphy.c  | 223 +++++++++++++++++-
>   .../media/platform/qcom/camss/camss-csiphy.h  |   2 +
>   2 files changed, 222 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/camss/camss-csiphy.c b/drivers/media/platform/qcom/camss/camss-csiphy.c
> index 539ac4888b60..7e9748e92ab8 100644
> --- a/drivers/media/platform/qcom/camss/camss-csiphy.c
> +++ b/drivers/media/platform/qcom/camss/camss-csiphy.c
> @@ -332,6 +332,112 @@ static int csiphy_set_stream(struct v4l2_subdev *sd, int enable)
>   	return ret;
>   }
>   
> +/*
> + * csiphy_pad_enable_streams - Enable one or more streams on the source pad
> + * @sd: CSIPHY V4L2 subdevice
> + * @state: V4L2 subdevice state
> + * @pad: Pad number
> + * @streams_mask: Bitmask of streams to enable
> + *
> + * The shared D-PHY lanes are enabled once, on the transition from no active
> + * sink streams to at least one. 
on the transition to an active state.

The sink stream(s) are propagated upstream to
> + * the sensor only for the subset that isn't already active,
only for streams which are inactive

  so a stream
> + * that's already running is never redundantly re-propagated.
> + *
> + * CSIPHY is only ever linked to a single entity on its source pad, and that
> + * entity is responsible for only enabling a stream on this pad while it
> + * itself still needs it, so no cross-consumer refcounting is needed here.
> + *
> + * Return 0 on success or a negative error code otherwise
> + */
> +static int csiphy_pad_enable_streams(struct v4l2_subdev *sd,
> +				     struct v4l2_subdev_state *state,
> +				     u32 pad, u64 streams_mask)
> +{
> +	struct csiphy_device *csiphy = v4l2_get_subdevdata(sd);
> +	struct media_pad *remote_pad =
> +		media_pad_remote_pad_first(&csiphy->pads[MSM_CSIPHY_PAD_SINK]);
> +	bool first_arrival = !csiphy->enabled_streams[MSM_CSIPHY_PAD_SINK];
> +	u64 sink_streams, propagate_mask;
> +	int ret;
> +
> +	sink_streams = v4l2_subdev_state_xlate_streams(state, pad, MSM_CSIPHY_PAD_SINK,
> +						       &streams_mask);
> +
> +	propagate_mask = sink_streams & ~csiphy->enabled_streams[MSM_CSIPHY_PAD_SINK];
> +
> +	if (first_arrival) {
> +		ret = csiphy_set_stream(sd, 1);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	csiphy->enabled_streams[MSM_CSIPHY_PAD_SINK] |= sink_streams;
> +	csiphy->enabled_streams[pad] |= streams_mask;
> +
> +	if (propagate_mask && remote_pad) {

When is remote pad false ?

> +		ret = v4l2_subdev_enable_streams(media_entity_to_v4l2_subdev(remote_pad->entity),
> +						 remote_pad->index, propagate_mask);
> +		if (ret) {
> +			csiphy->enabled_streams[MSM_CSIPHY_PAD_SINK] &= ~propagate_mask;
> +			csiphy->enabled_streams[pad] &= ~streams_mask;
> +
> +			if (first_arrival)
> +				csiphy_set_stream(sd, 0);
> +
> +			return ret;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +/*
> + * csiphy_pad_disable_streams - Disable one or more streams on the source pad
> + * @sd: CSIPHY V4L2 subdevice
> + * @state: V4L2 subdevice state
> + * @pad: Pad number
> + * @streams_mask: Bitmask of streams to disable
> + *
> + * The shared D-PHY lanes, and the propagation to the sensor, are only torn
> + * down once no sink stream is referenced by any source pad any more.
> + *
> + * CSIPHY is only ever linked to a single entity on its source pad, and that
> + * entity is responsible for only disabling a stream on this pad once it no
> + * longer needs it, so no cross-consumer refcounting is needed here.
> + *
> + * Return 0 on success or a negative error code otherwise
> + */
> +static int csiphy_pad_disable_streams(struct v4l2_subdev *sd,
> +				      struct v4l2_subdev_state *state,
> +				      u32 pad, u64 streams_mask)
> +{
> +	struct csiphy_device *csiphy = v4l2_get_subdevdata(sd);
> +	struct media_pad *remote_pad =
> +		media_pad_remote_pad_first(&csiphy->pads[MSM_CSIPHY_PAD_SINK]);
> +	u64 sink_streams;
> +	int ret = 0;
> +
> +	sink_streams = v4l2_subdev_state_xlate_streams(state, pad, MSM_CSIPHY_PAD_SINK,
> +						       &streams_mask);
> +
> +	csiphy->enabled_streams[pad] &= ~streams_mask;
> +	csiphy->enabled_streams[MSM_CSIPHY_PAD_SINK] &= ~sink_streams;
> +
> +	if (sink_streams && remote_pad)
> +		ret = v4l2_subdev_disable_streams(media_entity_to_v4l2_subdev(remote_pad->entity),
> +						  remote_pad->index, sink_streams);

I'm again questioning when the remote_pad is false i.e. can a PHY have a 
sink_stream without a remote ?
> +
> +	if (!csiphy->enabled_streams[MSM_CSIPHY_PAD_SINK]) {
> +		int stream_ret = csiphy_set_stream(sd, 0);
> +
> +		if (!ret)
> +			ret = stream_ret;
> +	}
> +
> +	return ret;
> +}
> +
>   /*
>    * __csiphy_get_format - Get pointer to format structure
>    * @csiphy: CSIPHY device
> @@ -743,6 +849,71 @@ static int csiphy_link_setup(struct media_entity *entity,
>   	return 0;
>   }
>   
> +static int csiphy_init_state(struct v4l2_subdev *sd,
> +			     struct v4l2_subdev_state *state)
> +{
> +	struct v4l2_subdev_route routes[] = {
> +		{
> +			.sink_pad = MSM_CSIPHY_PAD_SINK,
> +			.sink_stream = 0,
> +			.source_pad = MSM_CSIPHY_PAD_SRC,
> +			.source_stream = 0,
> +			.flags = V4L2_SUBDEV_ROUTE_FL_ACTIVE,
> +		},
> +	};
> +	struct v4l2_subdev_krouting routing = {
> +		.num_routes = ARRAY_SIZE(routes),
> +		.routes = routes,
> +	};
> +
> +	/*
> +	 * CSIPHY is a transparent D-PHY with no per-VC demux, so every sink
> +	 * stream (VC) a multi-stream sensor may drive must pass straight
> +	 * through as the same source stream, or downstream link validation
> +	 * (e.g. against CSID's multi-pad sink) will flag it as dangling. A
> +	 * multi-VC sensor is supported by userspace adding further routes via
> +	 * .set_routing; this default covers the common single-VC case.
> +	 */

Drop the references to DPHY - people are actively working on CPHY support.

> +	return v4l2_subdev_set_routing(sd, state, &routing);
> +}
> +
> +/*
> + * csiphy_set_routing - Set routing for the CSIPHY subdev
> + * @sd: CSIPHY V4L2 subdevice
> + * @state: V4L2 subdevice state
> + * @which: Type of format state (V4L2_SUBDEV_FORMAT_ACTIVE or TRY)
> + * @routing: Routing table to set
> + *
> + * CSIPHY is a transparent D-PHY with no per-VC demux, so every route must
> + * pass a sink stream straight through as the same source stream.
> + *
> + * Return 0 on success or a negative error code otherwise
> + */
> +static int csiphy_set_routing(struct v4l2_subdev *sd,
> +			      struct v4l2_subdev_state *state,
> +			      enum v4l2_subdev_format_whence which,
> +			      struct v4l2_subdev_krouting *routing)
> +{
> +	struct csiphy_device *csiphy = v4l2_get_subdevdata(sd);
> +	unsigned int i;
> +	int ret;
> +
> +	if (which == V4L2_SUBDEV_FORMAT_ACTIVE && csiphy->enabled_streams[MSM_CSIPHY_PAD_SINK])
> +		return -EBUSY;

Can this happen ?

dev_err() ?

> +
> +	for (i = 0; i < routing->num_routes; i++)
> +		if (routing->routes[i].sink_stream != routing->routes[i].source_stream)
> +			return -EINVAL;
> +
> +	ret = v4l2_subdev_routing_validate(sd, routing,
> +					   V4L2_SUBDEV_ROUTING_NO_STREAM_MIX |
> +					   V4L2_SUBDEV_ROUTING_NO_N_TO_1);
> +	if (ret)
> +		return ret;
> +
> +	return v4l2_subdev_set_routing(sd, state, routing);
> +}
> +
>   static const struct v4l2_subdev_core_ops csiphy_core_ops = {
>   	.s_power = csiphy_set_power,
>   };
> @@ -764,10 +935,35 @@ static const struct v4l2_subdev_ops csiphy_v4l2_ops = {
>   	.pad = &csiphy_pad_ops,
>   };
>   
> +static const struct v4l2_subdev_pad_ops csiphy_streams_pad_ops = {
> +	.enum_mbus_code = csiphy_enum_mbus_code,
> +	.enum_frame_size = csiphy_enum_frame_size,
> +	.get_fmt = csiphy_get_format,
> +	.set_fmt = csiphy_set_format,
> +	.get_frame_desc = v4l2_subdev_get_frame_desc_passthrough,
> +	.set_routing = csiphy_set_routing,
> +	.enable_streams = csiphy_pad_enable_streams,
> +	.disable_streams = csiphy_pad_disable_streams,
> +};
> +
> +static const struct v4l2_subdev_video_ops csiphy_streams_video_ops = {
> +	.s_stream = v4l2_subdev_s_stream_helper,
> +};
> +
> +static const struct v4l2_subdev_ops csiphy_streams_v4l2_ops = {
> +	.core = &csiphy_core_ops,
> +	.pad = &csiphy_streams_pad_ops,
> +	.video = &csiphy_streams_video_ops,
> +};
> +
>   static const struct v4l2_subdev_internal_ops csiphy_v4l2_internal_ops = {
>   	.open = csiphy_init_formats,
>   };
>   
> +static const struct v4l2_subdev_internal_ops csiphy_streams_internal_ops = {
> +	.init_state = csiphy_init_state,
> +};
> +
>   static const struct media_entity_operations csiphy_media_ops = {
>   	.link_setup = csiphy_link_setup,
>   	.link_validate = v4l2_subdev_link_validate,
> @@ -786,11 +982,16 @@ int msm_csiphy_register_entity(struct csiphy_device *csiphy,
>   	struct v4l2_subdev *sd = &csiphy->subdev;
>   	struct media_pad *pads = csiphy->pads;
>   	struct device *dev = csiphy->camss->dev;
> +	bool streams_api = csiphy->res->streams_enable;

Is this really a feature of the CSIPHY though ?

Its the CSI decoder that has to program registers in itself to trap the 
appropriate VC and route it.

Its a supported feature for a particular SoC.

This flag should live in struct camss_resources and you have a pointer o 
that structure in this routine anyway so..

>   	int ret;
>   
> -	v4l2_subdev_init(sd, &csiphy_v4l2_ops);
> -	sd->internal_ops = &csiphy_v4l2_internal_ops;
> +	v4l2_subdev_init(sd, streams_api ? &csiphy_streams_v4l2_ops
> +					      : &csiphy_v4l2_ops);
> +	sd->internal_ops = streams_api ? &csiphy_streams_internal_ops
> +					: &csiphy_v4l2_internal_ops;
>   	sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> +	if (streams_api)
> +		sd->flags |= V4L2_SUBDEV_FL_STREAMS;
>   	snprintf(sd->name, ARRAY_SIZE(sd->name), "%s%d",
>   		 MSM_CSIPHY_NAME, csiphy->id);
>   	sd->grp_id = CSIPHY_GRP_ID;
> @@ -813,12 +1014,27 @@ int msm_csiphy_register_entity(struct csiphy_device *csiphy,
>   		return ret;
>   	}
>   
> +	if (streams_api) {
> +		ret = v4l2_subdev_init_finalize(sd);
> +		if (ret) {
> +			dev_err(dev, "Failed to finalize subdev: %d\n", ret);
> +			goto err_media_entity_cleanup;
> +		}
> +	}
> +
>   	ret = v4l2_device_register_subdev(v4l2_dev, sd);
>   	if (ret < 0) {
>   		dev_err(dev, "Failed to register subdev: %d\n", ret);
> -		media_entity_cleanup(&sd->entity);
> +		goto err_v4l2_subdev_cleanup;
>   	}
>   
> +	return 0;
> +
> +err_v4l2_subdev_cleanup:
> +	v4l2_subdev_cleanup(sd);
> +err_media_entity_cleanup:
> +	media_entity_cleanup(&sd->entity);
> +
>   	return ret;
>   }
>   
> @@ -829,5 +1045,6 @@ int msm_csiphy_register_entity(struct csiphy_device *csiphy,
>   void msm_csiphy_unregister_entity(struct csiphy_device *csiphy)
>   {
>   	v4l2_device_unregister_subdev(&csiphy->subdev);
> +	v4l2_subdev_cleanup(&csiphy->subdev);
>   	media_entity_cleanup(&csiphy->subdev.entity);
>   }
> diff --git a/drivers/media/platform/qcom/camss/camss-csiphy.h b/drivers/media/platform/qcom/camss/camss-csiphy.h
> index 9d9657b82f74..e55c098dbe67 100644
> --- a/drivers/media/platform/qcom/camss/camss-csiphy.h
> +++ b/drivers/media/platform/qcom/camss/camss-csiphy.h
> @@ -84,6 +84,7 @@ struct csiphy_hw_ops {
>   
>   struct csiphy_subdev_resources {
>   	u8 id;
> +	bool streams_enable;
>   	const struct csiphy_hw_ops *hw_ops;
>   	const struct csiphy_formats *formats;
>   };
> @@ -114,6 +115,7 @@ struct csiphy_device {
>   	struct v4l2_mbus_framefmt fmt[MSM_CSIPHY_PADS_NUM];
>   	const struct csiphy_subdev_resources *res;
>   	struct csiphy_device_regs *regs;
> +	u64 enabled_streams[MSM_CSIPHY_PADS_NUM];
>   };
>   
>   struct camss_subdev_resources;

Re: [PATCH 2/8] media: qcom: camss: Add streams API hw_ops to CSID interface

[Bryan O'Donoghue]

On 11/09/2026 07:22, 
Gjorgji.Rosikopulos.gjorgji.rosikopulos@oss.qualcomm.com wrote:
> From: Gjorgji Rosikopulos <gjorgji.rosikopulos@oss.qualcomm.com>
> 
> Add configure_rx/enable_stream/disable_stream to struct csid_hw_ops:
> per-stream hardware operations needed to support the V4L2 subdev
> streams API, where the CSID Rx front-end is configured once
> independently of the per-stream RDI programming.
> 
> No functional change: these are new optional hw_ops entries, not yet
> implemented by any backend or called by any code path.
> 
> Signed-off-by: Gjorgji Rosikopulos <gjorgji.rosikopulos@oss.qualcomm.com>
> ---
>   .../media/platform/qcom/camss/camss-csid.h    | 43 +++++++++++++++++++
>   1 file changed, 43 insertions(+)
> 
> diff --git a/drivers/media/platform/qcom/camss/camss-csid.h b/drivers/media/platform/qcom/camss/camss-csid.h
> index 5296b10f6bac..90ee611b9092 100644
> --- a/drivers/media/platform/qcom/camss/camss-csid.h
> +++ b/drivers/media/platform/qcom/camss/camss-csid.h
> @@ -74,6 +74,25 @@ struct csid_phy_config {
>   
>   struct csid_device;
>   
> +/*
> + * struct csid_hw_ops - CSID hardware version specific operations
> + *
> + * configure_stream() is the legacy (non-streams-API) path and is mutually
> + * exclusive with enable_stream()/disable_stream() - a given CSID instance
> + * uses one family or the other, chosen by streams_enable.
> + *
> + * stream_id, as passed to enable_stream()/disable_stream(), is a hardware
> + * RDI/PIX port index in [0, MSM_CSID_MAX_SRC_STREAMS). It is currently
> + * always equal to pad - MSM_CSID_PAD_FIRST_SRC (multi-pad, 1:1
> + * pad-to-port mapping).

Lets not mix and match names. There has historically been a huge amount 
of sin and liberty taken, not the good kind unfortunately - with port_id 
and write-master.

port I believe is the misnomer we are currently stuck with - so lets 
continue to use it until a more crisp naming convention happens.

> + *
> + * configure_rx() is called once, on the transition from zero to non-zero
> + * active sink streams, strictly before the first enable_stream() call.
> + * enable_stream()/disable_stream() calls are always made under the
> + * subdev's active-state lock (core-enforced via
> + * v4l2_subdev_enable_streams()/disable_streams()), so hw_ops
> + * implementations don't need their own serialization.
> + */
>   struct csid_hw_ops {
>   	/*
>   	 * configure_stream - Configures and starts CSID input stream
> @@ -81,6 +100,30 @@ struct csid_hw_ops {
>   	 */
>   	void (*configure_stream)(struct csid_device *csid, u8 enable);
>   
> +	/*
> +	 * configure_rx - Configure the CSID Rx front-end
> +	 * @csid: CSID device
> +	 *
> +	 * Called once, when the first stream is enabled.
> +	 */
> +	void (*configure_rx)(struct csid_device *csid);
> +
> +	/*
> +	 * enable_stream - Enable a single CSID output stream
> +	 * @csid: CSID device
> +	 * @stream_id: stream id to enable
> +	 * @vc: virtual channel to program
> +	 * @dt: data type to program
> +	 */
> +	void (*enable_stream)(struct csid_device *csid, u32 stream_id, u8 vc, u8 dt);
> +
> +	/*
> +	 * disable_stream - Disable a single CSID output stream
> +	 * @csid: CSID device
> +	 * @stream_id: stream id to disable
> +	 */
> +	void (*disable_stream)(struct csid_device *csid, u32 stream_id);
> +

Seems odd that none of these have integer return types to propagate errors.

>   	/*
>   	 * configure_testgen_pattern - Validates and configures output pattern mode
>   	 * of test pattern generator

Re: [PATCH 3/8] media: qcom: camss: Implement CSID streams API hw_ops for gen2

[Bryan O'Donoghue]

On 11/09/2026 07:22, 
Gjorgji.Rosikopulos.gjorgji.rosikopulos@oss.qualcomm.com wrote:
> From: Gjorgji Rosikopulos <gjorgji.rosikopulos@oss.qualcomm.com>
> 
> Implement the configure_rx/enable_stream/disable_stream hw_ops added to
> struct csid_hw_ops, for the gen2 CSID hardware backend used by SM8250.
> 
> __csid_configure_rdi_stream() now takes an explicit data type parameter
> instead of deriving it from the CSID source pad's format, since the
> streams API keys each stream by stream_id rather than by pad. The
> existing configure_stream() path is updated to pass its per-port format
> lookup through to this parameter, preserving current behavior.
> 
> Signed-off-by: Gjorgji Rosikopulos <gjorgji.rosikopulos@oss.qualcomm.com>
> ---
>   .../platform/qcom/camss/camss-csid-gen2.c     | 59 ++++++++++++++++---
>   1 file changed, 51 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/camss/camss-csid-gen2.c b/drivers/media/platform/qcom/camss/camss-csid-gen2.c
> index eadcb2f7e3aa..2512cdb16d94 100644
> --- a/drivers/media/platform/qcom/camss/camss-csid-gen2.c
> +++ b/drivers/media/platform/qcom/camss/camss-csid-gen2.c
> @@ -253,13 +253,8 @@ static void __csid_configure_testgen(struct csid_device *csid, u8 enable, u8 por
>   	writel_relaxed(val, csid->base + CSID_TPG_CTRL);
>   }
>   
> -static void __csid_configure_rdi_stream(struct csid_device *csid, u8 enable, u8 port, u8 vc)
> +static void __csid_configure_rdi_stream(struct csid_device *csid, u8 enable, u8 port, u8 vc, u8 dt)
>   {
> -	/* Source pads matching RDI channels on hardware. Pad 1 -> RDI0, Pad 2 -> RDI1, etc. */
> -	struct v4l2_mbus_framefmt *input_format = &csid->fmt[MSM_CSID_PAD_FIRST_SRC + port];
> -	const struct csid_format_info *format = csid_get_fmt_entry(csid->res->formats->formats,
> -								   csid->res->formats->nformats,
> -								   input_format->code);
>   	u32 val;
>   
>   	/*
> @@ -281,7 +276,7 @@ static void __csid_configure_rdi_stream(struct csid_device *csid, u8 enable, u8
>   	val |= 1 << RDI_CFG0_TIMESTAMP_EN;
>   	/* note: for non-RDI path, this should be format->decode_format */
>   	val |= DECODE_FORMAT_PAYLOAD_ONLY << RDI_CFG0_DECODE_FORMAT;
> -	val |= format->data_type << RDI_CFG0_DATA_TYPE;
> +	val |= dt << RDI_CFG0_DATA_TYPE;
>   	val |= vc << RDI_CFG0_VIRTUAL_CHANNEL;
>   	val |= dt_id << RDI_CFG0_DT_ID;
>   	writel_relaxed(val, csid->base + CSID_RDI_CFG0(port));
> @@ -330,15 +325,60 @@ static void csid_configure_stream(struct csid_device *csid, u8 enable)
>   	/* Loop through all enabled ports and configure a stream for each */
>   	for (i = 0; i < MSM_CSID_MAX_SRC_STREAMS; i++)
>   		if (csid->phy.en_vc & BIT(i)) {
> +			/* Source pads match RDI channels: pad 1 -> RDI0, pad 2 -> RDI1, etc. */
> +			struct v4l2_mbus_framefmt *input_format =
> +				&csid->fmt[MSM_CSID_PAD_FIRST_SRC + i];
> +			const struct csid_format_info *format =
> +				csid_get_fmt_entry(csid->res->formats->formats,
> +						   csid->res->formats->nformats,
> +						   input_format->code);
> +			u8 vc = 0;
> +
>   			if (tg->enabled)
>   				__csid_configure_testgen(csid, enable, i, 0);
>   
> -			__csid_configure_rdi_stream(csid, enable, i, 0);
> +			__csid_configure_rdi_stream(csid, enable, i, vc, format->data_type);
>   			__csid_configure_rx(csid, &csid->phy, 0);
>   			__csid_ctrl_rdi(csid, enable, i);
>   		}
>   }
>   
> +/*
> + * configure_rx - Configure the CSID Rx front-end
> + */
> +static void csid_configure_rx(struct csid_device *csid)
> +{
> +	__csid_configure_rx(csid, &csid->phy, 0);
> +}
> +
> +/*
> + * stream_id is used directly as the hardware RDI port index below. This
> + * assumes a 1:1 stream-to-port mapping and should be revisited once
> + * per-platform src_streams data (stream id -> hw pipe) is added.
> + */
> +static void csid_enable_stream(struct csid_device *csid, u32 stream_id, u8 vc, u8 dt)
> +{
> +	struct csid_testgen_config *tg = &csid->testgen;
> +
> +	if (tg->enabled)
> +		__csid_configure_testgen(csid, 1, stream_id, vc);
> +
> +	__csid_configure_rdi_stream(csid, 1, stream_id, vc, dt);
> +	__csid_ctrl_rdi(csid, 1, stream_id);
> +}
> +
> +static void csid_disable_stream(struct csid_device *csid, u32 stream_id)
> +{
> +	struct csid_testgen_config *tg = &csid->testgen;
> +
> +	__csid_ctrl_rdi(csid, 0, stream_id);
> +
> +	if (tg->enabled)
> +		__csid_configure_testgen(csid, 0, stream_id, 0);
> +
> +	__csid_configure_rdi_stream(csid, 0, stream_id, 0, 0);
> +}
> +
>   static int csid_configure_testgen_pattern(struct csid_device *csid, s32 val)
>   {
>   	if (val > 0 && val <= csid->testgen.nmodes)
> @@ -425,6 +465,9 @@ static void csid_subdev_init(struct csid_device *csid)
>   const struct csid_hw_ops csid_ops_gen2 = {
>   	.configure_stream = csid_configure_stream,
>   	.configure_testgen_pattern = csid_configure_testgen_pattern,
> +	.configure_rx = csid_configure_rx,

Keep the namespace consistent and include _stream somewhere in this name

> +	.enable_stream = csid_enable_stream,
> +	.disable_stream = csid_disable_stream,
>   	.hw_version = csid_hw_version,
>   	.isr = csid_isr,
>   	.reset = csid_reset,

Re: [PATCH 4/8] media: qcom: camss: Add streams API support in CSID subdevice

[Bryan O'Donoghue]

On 11/09/2026 07:22, 
Gjorgji.Rosikopulos.gjorgji.rosikopulos@oss.qualcomm.com wrote:
> From: Gjorgji Rosikopulos <gjorgji.rosikopulos@oss.qualcomm.com>
> 
> Add the V4L2 subdev streams API to the CSID driver: per-source-pad
> enable/disable_streams pad ops, VC/DT discovery via get_frame_desc on
> the remote sink pad, and a routing table that routes a single incoming
> sink stream to every source pad by default (remappable via
> set_routing for multi-VC sensors).
> 
> Active streams are tracked per pad via a per-pad enabled_streams[]
> bitmask, so that enable/disable_streams correctly propagates to the
> CSIPHY only on first arrival/last departure of a sink stream, and
> multiple source-pad consumers can share a single propagated sink
> stream without redundant or colliding propagation.
> 
> csid_init_state() caps the number of default routes to
> MSM_CSID_MAX_SRC_STREAMS - 1 for non-lite CSIDs, matching the 3 usable
> RDI pads on full-IFE CSIDs (the 4th/pix pad is non-functional).

That makes sense.

> msm_csid_register_entity() mirrors the same is_lite check for the pad
> count itself, so non-lite CSIDs no longer register a pix source pad
> that no route ever targets.

Nope. I'll send a different solution. Fixing the non-functional and 
incorrectly mapped pix is a Fixes: level thing not a workaround it 
inline thing.

Just drop the workaround and assume any CSID you are working with 
actually works and is valid.

> csid_pad_enable_streams() rejects enabling with -ENOLINK when there is
> no remote sink link and the test generator is disabled, matching the
> equivalent check the legacy csid_set_stream() already performs.
> 
> msm_csid_register_entity() assigns the ctrl handler's lock as the
> subdev's state_lock before v4l2_subdev_init_finalize(), when the test
> pattern control is present. Without this, the test-pattern S_CTRL
> handler and the streams API's active-state accessors serialize on two
> independent locks despite both touching csid->testgen.enabled,
> allowing a concurrent S_CTRL(TEST_PATTERN) and stream enable/disable to
> race.

Actually this reminds me of the first go at VCs in CAMSS which ended up 
getting rolled back.

How will this be tested ? Is the TPG capable of generating different VCs ?

_That_ would be very useful.

> 
> This is opt-in per CSID instance via the new streams_enable resource
> flag; no platform sets it yet, so CSIDs continue to use the legacy
> non-streams subdev ops unchanged.
> 
> Signed-off-by: Gjorgji Rosikopulos <gjorgji.rosikopulos@oss.qualcomm.com>
> ---
>   .../media/platform/qcom/camss/camss-csid.c    | 494 +++++++++++++++++-
>   .../media/platform/qcom/camss/camss-csid.h    |   2 +
>   2 files changed, 491 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/camss/camss-csid.c b/drivers/media/platform/qcom/camss/camss-csid.c
> index 48459b46a981..ce4b07c0c1c2 100644
> --- a/drivers/media/platform/qcom/camss/camss-csid.c
> +++ b/drivers/media/platform/qcom/camss/camss-csid.c
> @@ -842,7 +842,7 @@ static void csid_try_format(struct csid_device *csid,
>   
>   		break;
>   
> -	case MSM_CSID_PAD_SRC:
> +	default:

Why subtract the case ?
Just add the default:

Also I don't think that code change is anything todo with adding streams 
support to CSID.

Separate patches for separate things.

>   		if (csid->testgen.nmodes == CSID_PAYLOAD_MODE_DISABLED ||
>   		    csid->testgen_mode->cur.val == 0) {
>   			/* Test generator is disabled, */
> @@ -1338,10 +1338,476 @@ static const struct v4l2_subdev_ops csid_v4l2_ops = {
>   	.pad = &csid_pad_ops,
>   };
>   
> +/*
> + * csid_get_stream_csi2_desc - Discover the virtual channel and data type
> + *			       used by a given sink stream, from an
> + *			       already-fetched frame descriptor
> + * @frame_desc: Frame descriptor fetched via .get_frame_desc from the remote
> + *		subdev linked on the sink pad
> + * @sink_stream: Sink-side stream number to look up
> + * @desc_csi2: Returns the discovered virtual channel/data type on success
> + *
> + * A frame descriptor with a single entry means the remote only exposes one
> + * stream (e.g. a single-VC sensor), which feeds every CSID source pad, so
> + * that entry is used regardless of @sink_stream.
> + *
> + * Return true if a matching entry was found, false otherwise
> + */
> +static bool csid_get_stream_csi2_desc(struct v4l2_mbus_frame_desc *frame_desc,
> +				      u32 sink_stream,
> +				      struct v4l2_mbus_frame_desc_entry_csi2 *desc_csi2)
> +{
> +	unsigned int i;
> +
> +	if (frame_desc->type != V4L2_MBUS_FRAME_DESC_TYPE_CSI2 || !frame_desc->num_entries)
> +		return false;
> +
> +	if (frame_desc->num_entries == 1) {
> +		*desc_csi2 = frame_desc->entry[0].bus.csi2;
> +		return true;
> +	}

You can drop that check entirely, the below loop will do exactly the 
same thing for num_entires == 1..

> +
> +	for (i = 0; i < frame_desc->num_entries; i++) {
> +		if (frame_desc->entry[i].stream == sink_stream) {
> +			*desc_csi2 = frame_desc->entry[i].bus.csi2;
> +			return true;
> +		}
> +	}
> +
> +	return false;
> +}
> +
> +/*
> + * csid_get_stream_vc_dt - Discover the virtual channel and data type to
> + *			   program for a given sink pad/stream, falling back
> + *			   to @format_dt when no frame descriptor is available
> + * @csid: CSID device
> + * @state: V4L2 subdevice state
> + * @remote_pad: Remote pad linked on the CSID sink pad, or NULL if unlinked
> + * @pad: Source pad number the caller is enabling a stream on
> + * @format_dt: Data type derived from the sink format, used as a fallback
> + *	       and sanity-checked against the discovered data type
> + *
> + * Return the discovered virtual channel/data type, or {0, @format_dt} if
> + * not discovered
> + */
> +static struct v4l2_mbus_frame_desc_entry_csi2
> +csid_get_stream_vc_dt(struct csid_device *csid, struct v4l2_subdev_state *state,
> +		      struct media_pad *remote_pad, u32 pad, u8 format_dt)
> +{
> +	struct v4l2_mbus_frame_desc_entry_csi2 desc_csi2 = { .dt = format_dt };
> +	struct v4l2_mbus_frame_desc fd = { };
> +	u32 sink_stream;
> +
> +	if (!remote_pad ||
> +	    v4l2_subdev_call(media_entity_to_v4l2_subdev(remote_pad->entity),
> +			     pad, get_frame_desc, remote_pad->index, &fd))
> +		return desc_csi2;
> +

if (thing ||
     some_other_thing) {
	return desc_csi2;
}

> +	if (v4l2_subdev_routing_find_opposite_end(&state->routing, pad, 0, NULL, &sink_stream))
> +		return desc_csi2;
> +
> +	if (!csid_get_stream_csi2_desc(&fd, sink_stream, &desc_csi2)) {
> +		dev_warn(csid->camss->dev,
> +			 "Failed to find CSI2 descriptor for sink stream %u, using vc=%u dt=%u\n",
> +			 sink_stream, desc_csi2.vc, desc_csi2.dt);
> +		return desc_csi2;

Is this an error it seems like it should be ?

> +	}
> +
> +	if (desc_csi2.dt != format_dt)
> +		dev_warn(csid->camss->dev,
> +			 "Sink stream %u frame desc dt=%u differs from format dt=%u, using dt=%u\n",
> +			 sink_stream, desc_csi2.dt, format_dt, desc_csi2.dt);
> +
> +	return desc_csi2;

I'd return a pointer here.

> +}
> +
> +/*
> + * csid_pad_enable_streams - Enable one or more streams on a source pad
> + * @sd: CSID V4L2 subdevice
> + * @state: V4L2 subdevice state
> + * @pad: Pad number
> + * @streams_mask: Bitmask of v4l2 streams to enable
> + *
> + * The v4l2 core only calls this on a source pad (v4l2_subdev_enable_streams()
> + * rejects sink pads with -EOPNOTSUPP before reaching the driver), so @pad is
> + * not checked here. Each source pad only ever carries stream 0.
> + *
> + * The shared sink stream(s) are propagated upstream only once, on the
> + * transition from no active sink streams to at least one, so that a second
> + * consumer of the same shared sink stream never triggers a second, redundant
> + * propagation to the sensor. The Rx front-end is likewise only configured
> + * once, on that same transition.
> + *
> + * Return 0 on success, -ENOLINK if there is no remote sink link and the test
> + * generator is disabled, or another negative error code otherwise
> + */
> +static int csid_pad_enable_streams(struct v4l2_subdev *sd,
> +				   struct v4l2_subdev_state *state,
> +				   u32 pad, u64 streams_mask)
> +{
> +	struct csid_device *csid = v4l2_get_subdevdata(sd);
> +	const struct csid_hw_ops *hw_ops = csid->res->hw_ops;
> +	struct media_pad *remote_pad = media_pad_remote_pad_first(&csid->pads[MSM_CSID_PAD_SINK]);
> +	unsigned int hw_port = pad - MSM_CSID_PAD_FIRST_SRC;
> +	const struct csid_format_info *format;
> +	struct v4l2_mbus_frame_desc_entry_csi2 desc_csi2;
> +	u64 sink_streams, propagate_mask;
> +	int ret;
> +
> +	if (!csid->testgen.enabled && !remote_pad)
> +		return -ENOLINK;
> +
> +	sink_streams = v4l2_subdev_state_xlate_streams(state, pad, MSM_CSID_PAD_SINK,
> +						       &streams_mask);
> +
> +	if (!csid->enabled_streams[MSM_CSID_PAD_SINK]) {
> +		if (csid->testgen.nmodes != CSID_PAYLOAD_MODE_DISABLED) {
> +			/*
> +			 * sd->state_lock is aliased to csid->ctrls.lock, and is
> +			 * already held here by the v4l2_subdev_enable_streams()
> +			 * caller, so use the lock-free variant to avoid
> +			 * self-deadlocking on the same mutex.
> +			 */
> +			ret = __v4l2_ctrl_handler_setup(&csid->ctrls);
> +			if (ret < 0) {
> +				dev_err(csid->camss->dev,
> +					"could not sync v4l2 controls: %d\n", ret);
> +				return ret;
> +			}
> +		}
> +
> +		hw_ops->configure_rx(csid);
> +	}
> +
> +	/* Sink streams already active elsewhere don't need re-propagating. */
> +	propagate_mask = sink_streams & ~csid->enabled_streams[MSM_CSID_PAD_SINK];
> +	csid->enabled_streams[MSM_CSID_PAD_SINK] |= sink_streams;
> +	csid->enabled_streams[pad] |= streams_mask;
> +
> +	format = csid_get_fmt_entry(csid->res->formats->formats,
> +				    csid->res->formats->nformats,
> +				    csid->fmt[pad].code);
> +	desc_csi2 = csid_get_stream_vc_dt(csid, state, remote_pad, pad, format->data_type);

You're doing an implict memcpy() here - just return a pointer.

And I really wonder based on the activity within the method why 
returning an error isn't part of this ?

> +
> +	hw_ops->enable_stream(csid, hw_port, desc_csi2.vc, desc_csi2.dt);

I commented elsewhere should this be void or int ?

I'm not suggesting either more asking rhetorically.

> +
> +	if (propagate_mask && remote_pad) {
> +		ret = v4l2_subdev_enable_streams(media_entity_to_v4l2_subdev(remote_pad->entity),
> +						 remote_pad->index, propagate_mask);
> +		if (ret) {
> +			csid->enabled_streams[MSM_CSID_PAD_SINK] &= ~propagate_mask;
> +			csid->enabled_streams[pad] &= ~streams_mask;
> +
> +			hw_ops->disable_stream(csid, hw_port);
> +
> +			return ret;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +/*
> + * csid_sink_streams_in_use - Compute the subset of sink streams still
> + *			      referenced by a source pad other than @pad
> + * @csid: CSID device
> + * @state: V4L2 subdevice state
> + * @pad: Source pad to exclude from the check
> + * @sink_streams: Candidate sink streams to check
> + *
> + * Return the subset of @sink_streams still referenced by some other source
> + * pad
> + */
> +static u64 csid_sink_streams_in_use(struct csid_device *csid, struct v4l2_subdev_state *state,
> +				    u32 pad, u64 sink_streams)
> +{
> +	u64 in_use = 0;
> +	unsigned int i;
> +
> +	for (i = MSM_CSID_PAD_FIRST_SRC; i < MSM_CSID_PADS_NUM; i++) {
> +		u64 other_streams = csid->enabled_streams[i];
> +		u64 other_sink_streams;
> +
> +		if (i == pad)
> +			continue;
> +
> +		other_sink_streams = v4l2_subdev_state_xlate_streams(state, i, MSM_CSID_PAD_SINK,
> +								     &other_streams);
> +		in_use |= sink_streams & other_sink_streams;
> +	}
> +
> +	return in_use;
> +}
> +
> +/*
> + * csid_pad_disable_streams - Disable one or more streams on a source pad
> + * @sd: CSID V4L2 subdevice
> + * @state: V4L2 subdevice state
> + * @pad: Pad number
> + * @streams_mask: Bitmask of v4l2 streams to disable
> + *
> + * The v4l2 core only calls this on a source pad (v4l2_subdev_disable_streams()
> + * rejects sink pads with -EOPNOTSUPP before reaching the driver), so @pad is
> + * not checked here. Each source pad only ever carries stream 0.
> + *
> + * A sink stream is only disabled, and propagated upstream to disable it there
> + * too, once no source pad references it any more.
> + *
> + * Return 0 on success or a negative error code otherwise
> + */
> +static int csid_pad_disable_streams(struct v4l2_subdev *sd,
> +				    struct v4l2_subdev_state *state,
> +				    u32 pad, u64 streams_mask)
> +{
> +	struct csid_device *csid = v4l2_get_subdevdata(sd);
> +	const struct csid_hw_ops *hw_ops = csid->res->hw_ops;
> +	struct media_pad *remote_pad = media_pad_remote_pad_first(&csid->pads[MSM_CSID_PAD_SINK]);
> +	unsigned int hw_port = pad - MSM_CSID_PAD_FIRST_SRC;
> +	u64 sink_streams, disable_sink_streams;
> +	int ret = 0;
> +
> +	sink_streams = v4l2_subdev_state_xlate_streams(state, pad, MSM_CSID_PAD_SINK,
> +						       &streams_mask);
> +
> +	/* Keep a sink stream active as long as any other source pad still uses it. */
> +	disable_sink_streams = sink_streams &
> +				~csid_sink_streams_in_use(csid, state, pad, sink_streams);
> +
> +	if (disable_sink_streams && remote_pad) {
> +		ret = v4l2_subdev_disable_streams(media_entity_to_v4l2_subdev(remote_pad->entity),
> +						  remote_pad->index, disable_sink_streams);
> +		if (ret)
> +			dev_err(csid->camss->dev,
> +				"Failed to disable stream on remote pad: %d\n", ret);
> +	}
> +
> +	hw_ops->disable_stream(csid, hw_port);
> +
> +	csid->enabled_streams[pad] &= ~streams_mask;
> +	csid->enabled_streams[MSM_CSID_PAD_SINK] &= ~disable_sink_streams;
> +
> +	return ret;
> +}
> +
> +static const struct v4l2_mbus_framefmt csid_default_format = {
> +	.code = MEDIA_BUS_FMT_UYVY8_1X16,
> +	.width = 1920,
> +	.height = 1080,
> +	.field = V4L2_FIELD_NONE,
> +	.colorspace = V4L2_COLORSPACE_SRGB,
> +};
> +
> +/*
> + * csid_set_routing - Handle setting of routing table
> + * @sd: CSID V4L2 subdevice
> + * @state: V4L2 subdevice state
> + * @which: TRY or ACTIVE routing
> + * @routing: Routing table to set
> + *
> + * Return 0 on success or a negative error code otherwise
> + */
> +static int csid_set_routing(struct v4l2_subdev *sd,
> +			    struct v4l2_subdev_state *state,
> +			    enum v4l2_subdev_format_whence which,
> +			    struct v4l2_subdev_krouting *routing)
> +{
> +	struct csid_device *csid = v4l2_get_subdevdata(sd);
> +	unsigned int i;
> +	int ret;
> +
> +	if (which == V4L2_SUBDEV_FORMAT_ACTIVE && csid->enabled_streams[MSM_CSID_PAD_SINK])
> +		return -EBUSY;
> +
> +	for (i = 0; i < routing->num_routes; i++)
> +		if (routing->routes[i].source_stream != 0)
> +			return -EINVAL;
> +
> +	ret = v4l2_subdev_routing_validate(sd, routing,
> +					   V4L2_SUBDEV_ROUTING_NO_SOURCE_STREAM_MIX |
> +					   V4L2_SUBDEV_ROUTING_NO_SOURCE_MULTIPLEXING |
> +					   V4L2_SUBDEV_ROUTING_NO_N_TO_1);
> +	if (ret)
> +		return ret;
> +
> +	return v4l2_subdev_set_routing_with_fmt(sd, state, routing, &csid_default_format);
> +}
> +
> +/*
> + * __csid_get_stream_format - Get pointer to per-stream format structure
> + * @csid: CSID device
> + * @sd_state: V4L2 subdev state
> + * @pad: pad from which format is requested
> + * @stream: stream from which format is requested
> + * @which: TRY or ACTIVE format
> + *
> + * Same as __csid_get_format(), but honors @stream for TRY-state lookups.
> + * For ACTIVE state, csid->fmt[] is indexed by pad + stream. @stream is
> + * always 0 and @pad selects the RDI channel (0-3).
> + *
> + * Return pointer to TRY or ACTIVE format structure
> + */
> +static struct v4l2_mbus_framefmt *
> +__csid_get_stream_format(struct csid_device *csid,
> +			 struct v4l2_subdev_state *sd_state,
> +			 unsigned int pad, u32 stream,
> +			 enum v4l2_subdev_format_whence which)
> +{
> +	if (which == V4L2_SUBDEV_FORMAT_TRY)
> +		return v4l2_subdev_state_get_format(sd_state, pad, stream);
> +
> +	if (pad == MSM_CSID_PAD_SINK)
> +		return &csid->fmt[MSM_CSID_PAD_SINK];
> +
> +	return &csid->fmt[pad + stream];
> +}
> +
> +/*
> + * csid_streams_get_format - Handle get format by pads subdev method
> + * @sd: CSID V4L2 subdevice
> + * @sd_state: V4L2 subdev state
> + * @fmt: pointer to v4l2 subdev format structure
> + *
> + * Return -EINVAL or zero on success
> + */
> +static int csid_streams_get_format(struct v4l2_subdev *sd,
> +				   struct v4l2_subdev_state *sd_state,
> +				   struct v4l2_subdev_format *fmt)
> +{
> +	struct csid_device *csid = v4l2_get_subdevdata(sd);
> +	struct v4l2_mbus_framefmt *format;
> +
> +	format = __csid_get_stream_format(csid, sd_state, fmt->pad, fmt->stream, fmt->which);
> +	if (!format)
> +		return -EINVAL;
> +
> +	fmt->format = *format;
> +
> +	return 0;
> +}
> +
> +/*
> + * csid_streams_set_format - Handle set format by pads subdev method
> + * @sd: CSID V4L2 subdevice
> + * @sd_state: V4L2 subdev state
> + * @fmt: pointer to v4l2 subdev format structure
> + *
> + * Return -EINVAL or zero on success
> + */
> +static int csid_streams_set_format(struct v4l2_subdev *sd,
> +				   struct v4l2_subdev_state *sd_state,
> +				   struct v4l2_subdev_format *fmt)
> +{
> +	struct csid_device *csid = v4l2_get_subdevdata(sd);
> +	struct v4l2_mbus_framefmt *format;
> +	struct v4l2_subdev_route *route;
> +
> +	if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE && csid->enabled_streams[MSM_CSID_PAD_SINK])
> +		return -EBUSY;
> +
> +	format = __csid_get_stream_format(csid, sd_state, fmt->pad, fmt->stream, fmt->which);
> +	if (!format)
> +		return -EINVAL;
> +
> +	csid_try_format(csid, sd_state, fmt->pad, &fmt->format, fmt->which);
> +	*format = fmt->format;
> +
> +	/* Propagate the format from the sink stream to every source stream it feeds */
> +	for_each_active_route(&sd_state->routing, route) {
> +		struct v4l2_mbus_framefmt *src_format;
> +
> +		if (route->sink_pad != fmt->pad || route->sink_stream != fmt->stream)
> +			continue;
> +
> +		src_format = __csid_get_stream_format(csid, sd_state, route->source_pad,
> +						      route->source_stream, fmt->which);
> +		if (!src_format)
> +			continue;
> +
> +		*src_format = fmt->format;
> +		csid_try_format(csid, sd_state, route->source_pad, src_format, fmt->which);
> +	}
> +
> +	return 0;
> +}

Why do we need a full new set of get-format and set-format ?

"Feels" like this could wrapper the existing code.

> +
> +static const struct v4l2_subdev_pad_ops csid_streams_pad_ops = {
> +	.enum_mbus_code = csid_enum_mbus_code,
> +	.enum_frame_size = csid_enum_frame_size,
> +	.get_fmt = csid_streams_get_format,
> +	.set_fmt = csid_streams_set_format,
> +	.set_routing = csid_set_routing,
> +	.enable_streams = csid_pad_enable_streams,
> +	.disable_streams = csid_pad_disable_streams,
> +};
> +
> +static const struct v4l2_subdev_video_ops csid_streams_video_ops = {
> +	.s_stream = v4l2_subdev_s_stream_helper,
> +};
> +
> +static const struct v4l2_subdev_ops csid_streams_v4l2_ops = {
> +	.core = &csid_core_ops,
> +	.pad = &csid_streams_pad_ops,
> +	.video = &csid_streams_video_ops,
> +};
> +
> +/*
> + * csid_init_state - Initialize the routing table for the streams API subdev
> + * @sd: CSID V4L2 subdevice
> + * @state: V4L2 subdev state
> + *
> + * source_stream is always 0: each source pad MSM_CSID_PAD_FIRST_SRC + i
> + * links to its own independent downstream subdev, and a link's sink side is
> + * validated against the implicit stream 0 exposed by any subdev without
> + * V4L2_SUBDEV_FL_STREAMS (see v4l2_link_validate_get_streams()) — every
> + * downstream VFE line is such a subdev.
> + *
> + * All source pads route from sink_stream 0 by default, fanning the single
> + * incoming stream out to every port; a multi-VC source is supported by
> + * remapping each route's sink_stream via .set_routing, leaving
> + * source_pad/source_stream untouched.
> + *
> + * Return 0 on success or a negative error code otherwise
> + */
> +static int csid_init_state(struct v4l2_subdev *sd, struct v4l2_subdev_state *state)
> +{
> +	struct csid_device *csid = v4l2_get_subdevdata(sd);
> +	struct v4l2_subdev_route routes[MSM_CSID_MAX_SRC_STREAMS];
> +	struct v4l2_subdev_krouting routing = { };
> +	unsigned int num_routes;
> +	int i, ret;
> +
> +	/* The full IFE has only 3 rdi's and pix output is not functional */
> +	if (csid_is_lite(csid))
> +		num_routes = MSM_CSID_MAX_SRC_STREAMS;
> +	else
> +		num_routes = MSM_CSID_MAX_SRC_STREAMS - 1;

No. Don't code around this here.

I'll make a Fixes: patch for the pix stuff - I have it in tree. I don't 
want to add work-arounds in code.

> +
> +	for (i = 0; i < num_routes; i++) {
> +		routes[i].sink_pad = MSM_CSID_PAD_SINK;
> +		routes[i].sink_stream = 0;
> +		routes[i].source_pad = MSM_CSID_PAD_FIRST_SRC + i;
> +		routes[i].source_stream = 0;
> +		routes[i].flags = V4L2_SUBDEV_ROUTE_FL_ACTIVE;
> +	}
> +
> +	routing.num_routes = num_routes;
> +	routing.routes = routes;
> +	ret = v4l2_subdev_set_routing_with_fmt(sd, state, &routing, &csid_default_format);
> +	if (ret)
> +		dev_err(csid->camss->dev, "Failed to set routing: %d\n", ret);
> +
> +	return ret;
> +}
> +
>   static const struct v4l2_subdev_internal_ops csid_v4l2_internal_ops = {
>   	.open = csid_init_formats,
>   };
>   
> +static const struct v4l2_subdev_internal_ops csid_streams_internal_ops = {
> +	.init_state = csid_init_state,
> +};
> +
>   static const struct media_entity_operations csid_media_ops = {
>   	.link_setup = csid_link_setup,
>   	.link_validate = v4l2_subdev_link_validate,
> @@ -1360,13 +1826,18 @@ int msm_csid_register_entity(struct csid_device *csid,
>   	struct v4l2_subdev *sd = &csid->subdev;
>   	struct media_pad *pads = csid->pads;
>   	struct device *dev = csid->camss->dev;
> +	bool streams_api = csid->res->streams_enable;

As I stated elsewhere there's no need to have this flag copy/pasted.

Just move it one level up does this SoC support streams, in 
camss->supports_streams;


> +	unsigned int num_pads = csid_is_lite(csid) ? MSM_CSID_PADS_NUM : MSM_CSID_PADS_NUM - 1;
>   	int i;
>   	int ret;
>   
> -	v4l2_subdev_init(sd, &csid_v4l2_ops);
> -	sd->internal_ops = &csid_v4l2_internal_ops;
> +	v4l2_subdev_init(sd, streams_api ? &csid_streams_v4l2_ops : &csid_v4l2_ops);
> +	sd->internal_ops = streams_api ? &csid_streams_internal_ops
> +					: &csid_v4l2_internal_ops;
>   	sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE |
>   		     V4L2_SUBDEV_FL_HAS_EVENTS;
> +	if (streams_api)
> +		sd->flags |= V4L2_SUBDEV_FL_STREAMS;
>   	snprintf(sd->name, ARRAY_SIZE(sd->name), "%s%d",
>   		 MSM_CSID_NAME, csid->id);
>   	v4l2_set_subdevdata(sd, csid);
> @@ -1400,17 +1871,28 @@ int msm_csid_register_entity(struct csid_device *csid,
>   	}
>   
>   	pads[MSM_CSID_PAD_SINK].flags = MEDIA_PAD_FL_SINK;
> -	for (i = MSM_CSID_PAD_FIRST_SRC; i < MSM_CSID_PADS_NUM; ++i)
> +	for (i = MSM_CSID_PAD_FIRST_SRC; i < num_pads; ++i)
>   		pads[i].flags = MEDIA_PAD_FL_SOURCE;
>   
>   	sd->entity.function = MEDIA_ENT_F_PROC_VIDEO_PIXEL_FORMATTER;
>   	sd->entity.ops = &csid_media_ops;
> -	ret = media_entity_pads_init(&sd->entity, MSM_CSID_PADS_NUM, pads);
> +	ret = media_entity_pads_init(&sd->entity, num_pads, pads);
>   	if (ret < 0) {
>   		dev_err(dev, "Failed to init media entity: %d\n", ret);
>   		goto free_ctrl;
>   	}
>   
> +	if (streams_api) {
> +		if (csid->testgen.nmodes != CSID_PAYLOAD_MODE_DISABLED)
> +			sd->state_lock = csid->ctrls.lock;
> +
> +		ret = v4l2_subdev_init_finalize(sd);
> +		if (ret) {
> +			dev_err(dev, "Failed to finalize subdev: %d\n", ret);
> +			goto media_cleanup;
> +		}
> +	}
> +
>   	ret = v4l2_device_register_subdev(v4l2_dev, sd);
>   	if (ret < 0) {
>   		dev_err(dev, "Failed to register subdev: %d\n", ret);
> @@ -1420,6 +1902,7 @@ int msm_csid_register_entity(struct csid_device *csid,
>   	return 0;
>   
>   media_cleanup:
> +	v4l2_subdev_cleanup(sd);
>   	media_entity_cleanup(&sd->entity);
>   free_ctrl:
>   	if (csid->testgen.nmodes != CSID_PAYLOAD_MODE_DISABLED)
> @@ -1435,6 +1918,7 @@ int msm_csid_register_entity(struct csid_device *csid,
>   void msm_csid_unregister_entity(struct csid_device *csid)
>   {
>   	v4l2_device_unregister_subdev(&csid->subdev);
> +	v4l2_subdev_cleanup(&csid->subdev);
>   	media_entity_cleanup(&csid->subdev.entity);
>   	if (csid->testgen.nmodes != CSID_PAYLOAD_MODE_DISABLED)
>   		v4l2_ctrl_handler_free(&csid->ctrls);
> diff --git a/drivers/media/platform/qcom/camss/camss-csid.h b/drivers/media/platform/qcom/camss/camss-csid.h
> index 90ee611b9092..a312103cf86d 100644
> --- a/drivers/media/platform/qcom/camss/camss-csid.h
> +++ b/drivers/media/platform/qcom/camss/camss-csid.h
> @@ -184,6 +184,7 @@ struct csid_hw_ops {
>   
>   struct csid_subdev_resources {
>   	bool is_lite;
> +	bool streams_enable;
>   	const struct csid_hw_ops *hw_ops;
>   	const struct parent_dev_ops *parent_dev_ops;
>   	const struct csid_formats *formats;
> @@ -209,6 +210,7 @@ struct csid_device {
>   	struct v4l2_mbus_framefmt fmt[MSM_CSID_PADS_NUM];
>   	struct v4l2_ctrl_handler ctrls;
>   	struct v4l2_ctrl *testgen_mode;
> +	u64 enabled_streams[MSM_CSID_PADS_NUM];
>   	const struct csid_subdev_resources *res;
>   };
>   

Re: [PATCH 5/8] media: qcom: camss: Fix CSID-to-VFE all-to-all link crossbar on sm8250

[Bryan O'Donoghue]

On 11/09/2026 07:22, 
Gjorgji.Rosikopulos.gjorgji.rosikopulos@oss.qualcomm.com wrote:
> From: Gjorgji Rosikopulos <gjorgji.rosikopulos@oss.qualcomm.com>
> 
> camss_link_entities() unconditionally links every CSID to every VFE,
> creating an all-to-all crossbar. On SM8250 the hardware wiring is
> fixed: each CSID is hardwired to exactly one VFE (csid[i] <-> vfe[i]),
> with no crossbar between instances. Enabling a mismatched link (e.g.
> csid0 -> vfe1) creates a media link that does not correspond to any
> real hardware datapath.
> 
> Add a csid_vfe_fixed_pairing flag to struct camss_resources and set it
> for sm8250_resources. When set, camss_link_entities() skips creating
> links between CSID and VFE instances whose indices do not match.
> Other platforms keep the historical all-to-all link creation.
> 
> Signed-off-by: Gjorgji Rosikopulos <gjorgji.rosikopulos@oss.qualcomm.com>
> ---
>   drivers/media/platform/qcom/camss/camss.c | 7 ++++++-
>   drivers/media/platform/qcom/camss/camss.h | 7 +++++++
>   2 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
> index 2123f6388e3d..1bb22cd23c5e 100644
> --- a/drivers/media/platform/qcom/camss/camss.c
> +++ b/drivers/media/platform/qcom/camss/camss.c
> @@ -5000,7 +5000,10 @@ static int camss_link_entities(struct camss *camss)
>   				}
>   	} else {
>   		for (i = 0; i < camss->res->csid_num; i++)
> -			for (k = 0; k < camss->res->vfe_num; k++)
> +			for (k = 0; k < camss->res->vfe_num; k++) {
> +				if (camss->res->csid_vfe_fixed_pairing && i != k)
> +					continue;
> +

A) This should be a Fixes
B) This should not be limited to sm8250

>   				for (j = 0; j < camss->vfe[k].res->line_num; j++) {
>   					struct v4l2_subdev *csid = &camss->csid[i].subdev;
>   					struct v4l2_subdev *vfe = &camss->vfe[k].line[j].subdev;
> @@ -5017,6 +5020,7 @@ static int camss_link_entities(struct camss *camss)
>   						return ret;
>   					}
>   				}
> +			}
>   	}
>   
>   	return 0;
> @@ -5666,6 +5670,7 @@ static const struct camss_resources sm8250_resources = {
>   	.csiphy_num = ARRAY_SIZE(csiphy_res_8250),
>   	.csid_num = ARRAY_SIZE(csid_res_8250),
>   	.vfe_num = ARRAY_SIZE(vfe_res_8250),
> +	.csid_vfe_fixed_pairing = true,
>   };
>   
>   static const struct camss_resources sc8280xp_resources = {
> diff --git a/drivers/media/platform/qcom/camss/camss.h b/drivers/media/platform/qcom/camss/camss.h
> index 93d691c8ac63..ad0b42719788 100644
> --- a/drivers/media/platform/qcom/camss/camss.h
> +++ b/drivers/media/platform/qcom/camss/camss.h
> @@ -119,6 +119,13 @@ struct camss_resources {
>   	const unsigned int tpg_num;
>   	const unsigned int csid_num;
>   	const unsigned int vfe_num;
> +	/*
> +	 * True on platforms where each CSID is wired to exactly one VFE at
> +	 * the same index (csid[i] <-> vfe[i]). When set, camss_link_entities()
> +	 * only creates CSID->VFE links for matching indices instead of an
> +	 * all-to-all crossbar.
> +	 */
> +	const bool csid_vfe_fixed_pairing;

I think the number of platforms where all-to-all is possible is quite 
low - if possible on any.


>   };
>   
>   struct camss {

Re: [PATCH 0/8] media: qcom: camss: add V4L2 subdev streams API support

[Gjorgji Rosikopulos (Consultant)]

Hi Bryan thanks for the review,

On 9/11/2026 1:19 PM, Bryan O'Donoghue wrote:
> On 11/09/2026 07:22, 
> Gjorgji.Rosikopulos.gjorgji.rosikopulos@oss.qualcomm.com wrote:
>> From: Gjorgji Rosikopulos <gjorgji.rosikopulos@oss.qualcomm.com>
>>
>> This series adds V4L2 subdev streams API support to the CAMSS driver.
> 
> Can you please provide a use-case and test in your overview.

The use case is having multi VC sensor operating in DOL mode,
i have locally some register settings for enabling that and verify
the multi VC but unfortunately i dont have permission to post that upstream
for now... 

> 
> i.e. show what it does and show it doesn't break anything in a way a 
> reviewer can test ?

I have shell scripts setting up the links using media-ctl and streaming
with v4l2-ctl, i will post the content in the cover letter in next patchset.

> 
>> Each subdevice gains streams-aware enable_streams/disable_streams pad
>> ops alongside the existing legacy (non-streams) subdev ops, guarded by
>> a new per-instance streams_enable resource flag.
>>
>> Patches 1-4 add the CSIPHY/CSID mechanism:
>>   - CSIPHY: passthrough routing, NO_STREAM_MIX/NO_N_TO_1 validation, and
>>     shared D-PHY lane enable/disable gated on stream-count transitions.
>>   - CSID: per-source-pad routing (a single sink stream propagated to
>>     every source pad by default, remappable for multi-VC sensors),
>>     VC/DT discovery via get_frame_desc, and new hw_ops
>>     (configure_rx/enable_stream/disable_stream) with a gen2 backend
>>     implementation.
>>
>> Patch 5 is a standalone bug fix, independent of the streams API:
>> camss_link_entities() used to create an all-to-all CSID-to-VFE
>> crossbar, but SM8250's hardware wiring is a fixed 1:1 pairing
>> (csid[i] <-> vfe[i]). Enabling a mismatched link (e.g. csid0 -> vfe1)
>> exposed a media link with no real hardware datapath. Fixed via an
>> opt-in csid_vfe_fixed_pairing flag, set only for sm8250_resources.
>>
>> Patches 6-8 complete the mechanism and turn it on for real hardware:
>>   - VFE: streams-aware pad ops. VFE lines are inherently single-consumer
>>     (vfe_link_setup() enforces one link per pad), so no refcounting is
>>     needed there.
>>   - camss-video: the video device pipeline walk now checks, via
>>     v4l2_subdev_has_op(), whether the directly-connected subdev supports
>>     enable_streams/disable_streams; if so it issues a single top-level
>>     call instead of manually walking the pipeline one subdev at a time
>>     with .s_stream(). Falls back to the existing legacy path unchanged
>>     when the remote subdev doesn't support the streams API, so no other
>>     platform is affected.
>>   - SM8250: streams_enable is set true on every CSIPHY, CSID, and VFE
>>     line resource entry, turning the mechanism on for real hardware.
>>     Every other platform keeps using the legacy non-streams subdev ops,
>>     so this is a no-op everywhere else.
>>
>> A practical benefit of the CSID routing change (patch 4) is routing
>> flexibility for multi-VC sensors: the CSID's routing table maps sink
>> streams to source pads/streams via userspace-configurable
>> v4l2_subdev_route entries instead of a fixed pad<->VC assignment, so a
>> sensor emitting multiple virtual channels can have each VC directed to
>> a different RDI output (and thus a different VFE line/video node)
>> with a set_routing call, rather than being constrained to whatever
>> fixed mapping the driver hardcodes.
>>
>> When a sink stream is shared by multiple source pads/streams, CSID
>> only enables the corresponding upstream CSIPHY stream on the first
>> source stream that needs it, and only disables it once the last
>> remaining source stream using it is disabled. Enabling or disabling
>> additional consumers of an already-active shared stream is a no-op
>> upstream, so no consumer can double-enable or prematurely disable a
>> stream still in use by another. This also avoids ever hitting v4l2
>> core's own -EALREADY re-enable gate.
>>
>> Verified clean with checkpatch --strict. Built, flashed, and tested on
>> RB5/SM8250 hardware; ran the no-routing capture verification test
>> across all four CSID/VFE RDI pairs (csid0->vfe0, csid1->vfe1,
>> csid2->vfe2, csid3->vfe3) at 4056x3040 - all four passed with
>> correctly-sized frame captures.
> 
> What's that - please detail your exact steps in the cover letter. What I 
> need to see in the first instance is that nothing breaks.
>

Yes i will post them in the next patchset. Along with the routing commands when
stream_api is enabled in the kernel.

 
> Maybe try running libcamera cam with or without gpuisp. Show some yavta 
> commands to prove nothing breaks and then something to show how to use 
> your code.

Also i will verify with libcamera and gpuisp and update the results in
second patchset.

~Gjorgji

Re: [PATCH 1/8] media: qcom: camss: Add streams API support for CSIPHY

[Gjorgji Rosikopulos (Consultant)]

Hi Bryan,

Thanks for the review.

On 9/11/2026 1:37 PM, Bryan O'Donoghue wrote:
> On 11/09/2026 07:22, 
> Gjorgji.Rosikopulos.gjorgji.rosikopulos@oss.qualcomm.com wrote:
>> From: Gjorgji Rosikopulos <gjorgji.rosikopulos@oss.qualcomm.com>
>>
>> Add the V4L2 subdev streams API to the CSIPHY driver: a passthrough
>> routing table (one VC per stream) and NO_STREAM_MIX routing validation.
>>
>> enable/disable_streams pad ops track active streams per pad via a
>> per-pad enabled_streams[] bitmask, so that multiple source-pad
>> consumers can share a single propagated sink stream without redundant
>> or colliding propagation to the sensor. The shared D-PHY lanes are
>> enabled once, on the transition from no active streams to at least
>> one, and disabled once all streams have gone idle.
>>
>> This is opt-in per CSIPHY instance via the new streams_enable resource
>> flag; no platform sets it yet, so CSIPHYs continue to use the legacy
>> non-streams subdev ops unchanged.
>>
>> Signed-off-by: Gjorgji Rosikopulos <gjorgji.rosikopulos@oss.qualcomm.com>
>> ---
>>   .../media/platform/qcom/camss/camss-csiphy.c  | 223 +++++++++++++++++-
>>   .../media/platform/qcom/camss/camss-csiphy.h  |   2 +
>>   2 files changed, 222 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/media/platform/qcom/camss/camss-csiphy.c b/drivers/media/platform/qcom/camss/camss-csiphy.c
>> index 539ac4888b60..7e9748e92ab8 100644
>> --- a/drivers/media/platform/qcom/camss/camss-csiphy.c
>> +++ b/drivers/media/platform/qcom/camss/camss-csiphy.c
>> @@ -332,6 +332,112 @@ static int csiphy_set_stream(struct v4l2_subdev *sd, int enable)
>>   	return ret;
>>   }
>>   
>> +/*
>> + * csiphy_pad_enable_streams - Enable one or more streams on the source pad
>> + * @sd: CSIPHY V4L2 subdevice
>> + * @state: V4L2 subdevice state
>> + * @pad: Pad number
>> + * @streams_mask: Bitmask of streams to enable
>> + *
>> + * The shared D-PHY lanes are enabled once, on the transition from no active
>> + * sink streams to at least one. 
> on the transition to an active state.
> 
> The sink stream(s) are propagated upstream to
>> + * the sensor only for the subset that isn't already active,
> only for streams which are inactive
> 
>   so a stream

Thanks that will be incorporated in next patchset.

>> + * that's already running is never redundantly re-propagated.
>> + *
>> + * CSIPHY is only ever linked to a single entity on its source pad, and that
>> + * entity is responsible for only enabling a stream on this pad while it
>> + * itself still needs it, so no cross-consumer refcounting is needed here.
>> + *
>> + * Return 0 on success or a negative error code otherwise
>> + */
>> +static int csiphy_pad_enable_streams(struct v4l2_subdev *sd,
>> +				     struct v4l2_subdev_state *state,
>> +				     u32 pad, u64 streams_mask)
>> +{
>> +	struct csiphy_device *csiphy = v4l2_get_subdevdata(sd);
>> +	struct media_pad *remote_pad =
>> +		media_pad_remote_pad_first(&csiphy->pads[MSM_CSIPHY_PAD_SINK]);
>> +	bool first_arrival = !csiphy->enabled_streams[MSM_CSIPHY_PAD_SINK];
>> +	u64 sink_streams, propagate_mask;
>> +	int ret;
>> +
>> +	sink_streams = v4l2_subdev_state_xlate_streams(state, pad, MSM_CSIPHY_PAD_SINK,
>> +						       &streams_mask);
>> +
>> +	propagate_mask = sink_streams & ~csiphy->enabled_streams[MSM_CSIPHY_PAD_SINK];
>> +
>> +	if (first_arrival) {
>> +		ret = csiphy_set_stream(sd, 1);
>> +		if (ret)
>> +			return ret;
>> +	}
>> +
>> +	csiphy->enabled_streams[MSM_CSIPHY_PAD_SINK] |= sink_streams;
>> +	csiphy->enabled_streams[pad] |= streams_mask;
>> +
>> +	if (propagate_mask && remote_pad) {
> 
> When is remote pad false ?

Never, but i get some internal AI bot review comments so i have added that check, i agree it can be removed.

> 
>> +		ret = v4l2_subdev_enable_streams(media_entity_to_v4l2_subdev(remote_pad->entity),
>> +						 remote_pad->index, propagate_mask);
>> +		if (ret) {
>> +			csiphy->enabled_streams[MSM_CSIPHY_PAD_SINK] &= ~propagate_mask;
>> +			csiphy->enabled_streams[pad] &= ~streams_mask;
>> +
>> +			if (first_arrival)
>> +				csiphy_set_stream(sd, 0);
>> +
>> +			return ret;
>> +		}
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +/*
>> + * csiphy_pad_disable_streams - Disable one or more streams on the source pad
>> + * @sd: CSIPHY V4L2 subdevice
>> + * @state: V4L2 subdevice state
>> + * @pad: Pad number
>> + * @streams_mask: Bitmask of streams to disable
>> + *
>> + * The shared D-PHY lanes, and the propagation to the sensor, are only torn
>> + * down once no sink stream is referenced by any source pad any more.
>> + *
>> + * CSIPHY is only ever linked to a single entity on its source pad, and that
>> + * entity is responsible for only disabling a stream on this pad once it no
>> + * longer needs it, so no cross-consumer refcounting is needed here.
>> + *
>> + * Return 0 on success or a negative error code otherwise
>> + */
>> +static int csiphy_pad_disable_streams(struct v4l2_subdev *sd,
>> +				      struct v4l2_subdev_state *state,
>> +				      u32 pad, u64 streams_mask)
>> +{
>> +	struct csiphy_device *csiphy = v4l2_get_subdevdata(sd);
>> +	struct media_pad *remote_pad =
>> +		media_pad_remote_pad_first(&csiphy->pads[MSM_CSIPHY_PAD_SINK]);
>> +	u64 sink_streams;
>> +	int ret = 0;
>> +
>> +	sink_streams = v4l2_subdev_state_xlate_streams(state, pad, MSM_CSIPHY_PAD_SINK,
>> +						       &streams_mask);
>> +
>> +	csiphy->enabled_streams[pad] &= ~streams_mask;
>> +	csiphy->enabled_streams[MSM_CSIPHY_PAD_SINK] &= ~sink_streams;
>> +
>> +	if (sink_streams && remote_pad)
>> +		ret = v4l2_subdev_disable_streams(media_entity_to_v4l2_subdev(remote_pad->entity),
>> +						  remote_pad->index, sink_streams);
> 
> I'm again questioning when the remote_pad is false i.e. can a PHY have a 
> sink_stream without a remote ?

Never, it will be removed...

>> +
>> +	if (!csiphy->enabled_streams[MSM_CSIPHY_PAD_SINK]) {
>> +		int stream_ret = csiphy_set_stream(sd, 0);
>> +
>> +		if (!ret)
>> +			ret = stream_ret;
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>>   /*
>>    * __csiphy_get_format - Get pointer to format structure
>>    * @csiphy: CSIPHY device
>> @@ -743,6 +849,71 @@ static int csiphy_link_setup(struct media_entity *entity,
>>   	return 0;
>>   }
>>   
>> +static int csiphy_init_state(struct v4l2_subdev *sd,
>> +			     struct v4l2_subdev_state *state)
>> +{
>> +	struct v4l2_subdev_route routes[] = {
>> +		{
>> +			.sink_pad = MSM_CSIPHY_PAD_SINK,
>> +			.sink_stream = 0,
>> +			.source_pad = MSM_CSIPHY_PAD_SRC,
>> +			.source_stream = 0,
>> +			.flags = V4L2_SUBDEV_ROUTE_FL_ACTIVE,
>> +		},
>> +	};
>> +	struct v4l2_subdev_krouting routing = {
>> +		.num_routes = ARRAY_SIZE(routes),
>> +		.routes = routes,
>> +	};
>> +
>> +	/*
>> +	 * CSIPHY is a transparent D-PHY with no per-VC demux, so every sink
>> +	 * stream (VC) a multi-stream sensor may drive must pass straight
>> +	 * through as the same source stream, or downstream link validation
>> +	 * (e.g. against CSID's multi-pad sink) will flag it as dangling. A
>> +	 * multi-VC sensor is supported by userspace adding further routes via
>> +	 * .set_routing; this default covers the common single-VC case.
>> +	 */
> 
> Drop the references to DPHY - people are actively working on CPHY support.

Noted it will be removed.

> 
>> +	return v4l2_subdev_set_routing(sd, state, &routing);
>> +}
>> +
>> +/*
>> + * csiphy_set_routing - Set routing for the CSIPHY subdev
>> + * @sd: CSIPHY V4L2 subdevice
>> + * @state: V4L2 subdevice state
>> + * @which: Type of format state (V4L2_SUBDEV_FORMAT_ACTIVE or TRY)
>> + * @routing: Routing table to set
>> + *
>> + * CSIPHY is a transparent D-PHY with no per-VC demux, so every route must
>> + * pass a sink stream straight through as the same source stream.
>> + *
>> + * Return 0 on success or a negative error code otherwise
>> + */
>> +static int csiphy_set_routing(struct v4l2_subdev *sd,
>> +			      struct v4l2_subdev_state *state,
>> +			      enum v4l2_subdev_format_whence which,
>> +			      struct v4l2_subdev_krouting *routing)
>> +{
>> +	struct csiphy_device *csiphy = v4l2_get_subdevdata(sd);
>> +	unsigned int i;
>> +	int ret;
>> +
>> +	if (which == V4L2_SUBDEV_FORMAT_ACTIVE && csiphy->enabled_streams[MSM_CSIPHY_PAD_SINK])
>> +		return -EBUSY;
> 
> Can this happen ?
> 
> dev_err() ?

I need to check the core but there should be protection to not be able to set the routing while stream is active.
If that is the case i will remove this check.

> 
>> +
>> +	for (i = 0; i < routing->num_routes; i++)
>> +		if (routing->routes[i].sink_stream != routing->routes[i].source_stream)
>> +			return -EINVAL;
>> +
>> +	ret = v4l2_subdev_routing_validate(sd, routing,
>> +					   V4L2_SUBDEV_ROUTING_NO_STREAM_MIX |
>> +					   V4L2_SUBDEV_ROUTING_NO_N_TO_1);
>> +	if (ret)
>> +		return ret;
>> +
>> +	return v4l2_subdev_set_routing(sd, state, routing);
>> +}
>> +
>>   static const struct v4l2_subdev_core_ops csiphy_core_ops = {
>>   	.s_power = csiphy_set_power,
>>   };
>> @@ -764,10 +935,35 @@ static const struct v4l2_subdev_ops csiphy_v4l2_ops = {
>>   	.pad = &csiphy_pad_ops,
>>   };
>>   
>> +static const struct v4l2_subdev_pad_ops csiphy_streams_pad_ops = {
>> +	.enum_mbus_code = csiphy_enum_mbus_code,
>> +	.enum_frame_size = csiphy_enum_frame_size,
>> +	.get_fmt = csiphy_get_format,
>> +	.set_fmt = csiphy_set_format,
>> +	.get_frame_desc = v4l2_subdev_get_frame_desc_passthrough,
>> +	.set_routing = csiphy_set_routing,
>> +	.enable_streams = csiphy_pad_enable_streams,
>> +	.disable_streams = csiphy_pad_disable_streams,
>> +};
>> +
>> +static const struct v4l2_subdev_video_ops csiphy_streams_video_ops = {
>> +	.s_stream = v4l2_subdev_s_stream_helper,
>> +};
>> +
>> +static const struct v4l2_subdev_ops csiphy_streams_v4l2_ops = {
>> +	.core = &csiphy_core_ops,
>> +	.pad = &csiphy_streams_pad_ops,
>> +	.video = &csiphy_streams_video_ops,
>> +};
>> +
>>   static const struct v4l2_subdev_internal_ops csiphy_v4l2_internal_ops = {
>>   	.open = csiphy_init_formats,
>>   };
>>   
>> +static const struct v4l2_subdev_internal_ops csiphy_streams_internal_ops = {
>> +	.init_state = csiphy_init_state,
>> +};
>> +
>>   static const struct media_entity_operations csiphy_media_ops = {
>>   	.link_setup = csiphy_link_setup,
>>   	.link_validate = v4l2_subdev_link_validate,
>> @@ -786,11 +982,16 @@ int msm_csiphy_register_entity(struct csiphy_device *csiphy,
>>   	struct v4l2_subdev *sd = &csiphy->subdev;
>>   	struct media_pad *pads = csiphy->pads;
>>   	struct device *dev = csiphy->camss->dev;
>> +	bool streams_api = csiphy->res->streams_enable;
> 
> Is this really a feature of the CSIPHY though ?
> 
> Its the CSI decoder that has to program registers in itself to trap the 
> appropriate VC and route it.
> 
> Its a supported feature for a particular SoC.
> 
> This flag should live in struct camss_resources and you have a pointer o 
> that structure in this routine anyway so..

Hmm yes there should not be separate flag for each sub-device becouse either the whole topolgy
supports multistream or not. I will move the flag in camss_resourcess. Thanks 

> 
>>   	int ret;
>>   
>> -	v4l2_subdev_init(sd, &csiphy_v4l2_ops);
>> -	sd->internal_ops = &csiphy_v4l2_internal_ops;
>> +	v4l2_subdev_init(sd, streams_api ? &csiphy_streams_v4l2_ops
>> +					      : &csiphy_v4l2_ops);
>> +	sd->internal_ops = streams_api ? &csiphy_streams_internal_ops
>> +					: &csiphy_v4l2_internal_ops;
>>   	sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
>> +	if (streams_api)
>> +		sd->flags |= V4L2_SUBDEV_FL_STREAMS;
>>   	snprintf(sd->name, ARRAY_SIZE(sd->name), "%s%d",
>>   		 MSM_CSIPHY_NAME, csiphy->id);
>>   	sd->grp_id = CSIPHY_GRP_ID;
>> @@ -813,12 +1014,27 @@ int msm_csiphy_register_entity(struct csiphy_device *csiphy,
>>   		return ret;
>>   	}
>>   
>> +	if (streams_api) {
>> +		ret = v4l2_subdev_init_finalize(sd);
>> +		if (ret) {
>> +			dev_err(dev, "Failed to finalize subdev: %d\n", ret);
>> +			goto err_media_entity_cleanup;
>> +		}
>> +	}
>> +
>>   	ret = v4l2_device_register_subdev(v4l2_dev, sd);
>>   	if (ret < 0) {
>>   		dev_err(dev, "Failed to register subdev: %d\n", ret);
>> -		media_entity_cleanup(&sd->entity);
>> +		goto err_v4l2_subdev_cleanup;
>>   	}
>>   
>> +	return 0;
>> +
>> +err_v4l2_subdev_cleanup:
>> +	v4l2_subdev_cleanup(sd);
>> +err_media_entity_cleanup:
>> +	media_entity_cleanup(&sd->entity);
>> +
>>   	return ret;
>>   }
>>   
>> @@ -829,5 +1045,6 @@ int msm_csiphy_register_entity(struct csiphy_device *csiphy,
>>   void msm_csiphy_unregister_entity(struct csiphy_device *csiphy)
>>   {
>>   	v4l2_device_unregister_subdev(&csiphy->subdev);
>> +	v4l2_subdev_cleanup(&csiphy->subdev);
>>   	media_entity_cleanup(&csiphy->subdev.entity);
>>   }
>> diff --git a/drivers/media/platform/qcom/camss/camss-csiphy.h b/drivers/media/platform/qcom/camss/camss-csiphy.h
>> index 9d9657b82f74..e55c098dbe67 100644
>> --- a/drivers/media/platform/qcom/camss/camss-csiphy.h
>> +++ b/drivers/media/platform/qcom/camss/camss-csiphy.h
>> @@ -84,6 +84,7 @@ struct csiphy_hw_ops {
>>   
>>   struct csiphy_subdev_resources {
>>   	u8 id;
>> +	bool streams_enable;
>>   	const struct csiphy_hw_ops *hw_ops;
>>   	const struct csiphy_formats *formats;
>>   };
>> @@ -114,6 +115,7 @@ struct csiphy_device {
>>   	struct v4l2_mbus_framefmt fmt[MSM_CSIPHY_PADS_NUM];
>>   	const struct csiphy_subdev_resources *res;
>>   	struct csiphy_device_regs *regs;
>> +	u64 enabled_streams[MSM_CSIPHY_PADS_NUM];
>>   };
>>   
>>   struct camss_subdev_resources;
> 

Re: [PATCH 3/8] media: qcom: camss: Implement CSID streams API hw_ops for gen2

[Loic Poulain]

Hi Gjorgji,

On Fri, Sep 11, 2026 at 8:22 AM Gjorgji Rosikopulos
<gjorgji.rosikopulos@oss.qualcomm.com> wrote:
>
> From: Gjorgji Rosikopulos <gjorgji.rosikopulos@oss.qualcomm.com>
>
> Implement the configure_rx/enable_stream/disable_stream hw_ops added to
> struct csid_hw_ops, for the gen2 CSID hardware backend used by SM8250.
>
> __csid_configure_rdi_stream() now takes an explicit data type parameter
> instead of deriving it from the CSID source pad's format, since the
> streams API keys each stream by stream_id rather than by pad. The
> existing configure_stream() path is updated to pass its per-port format
> lookup through to this parameter, preserving current behavior.
>
> Signed-off-by: Gjorgji Rosikopulos <gjorgji.rosikopulos@oss.qualcomm.com>
> ---
>  .../platform/qcom/camss/camss-csid-gen2.c     | 59 ++++++++++++++++---
>  1 file changed, 51 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/camss/camss-csid-gen2.c b/drivers/media/platform/qcom/camss/camss-csid-gen2.c
> index eadcb2f7e3aa..2512cdb16d94 100644
> --- a/drivers/media/platform/qcom/camss/camss-csid-gen2.c
> +++ b/drivers/media/platform/qcom/camss/camss-csid-gen2.c
> @@ -253,13 +253,8 @@ static void __csid_configure_testgen(struct csid_device *csid, u8 enable, u8 por
>         writel_relaxed(val, csid->base + CSID_TPG_CTRL);
>  }
>
> -static void __csid_configure_rdi_stream(struct csid_device *csid, u8 enable, u8 port, u8 vc)
> +static void __csid_configure_rdi_stream(struct csid_device *csid, u8 enable, u8 port, u8 vc, u8 dt)
>  {
> -       /* Source pads matching RDI channels on hardware. Pad 1 -> RDI0, Pad 2 -> RDI1, etc. */
> -       struct v4l2_mbus_framefmt *input_format = &csid->fmt[MSM_CSID_PAD_FIRST_SRC + port];
> -       const struct csid_format_info *format = csid_get_fmt_entry(csid->res->formats->formats,
> -                                                                  csid->res->formats->nformats,
> -                                                                  input_format->code);
>         u32 val;
>
>         /*
> @@ -281,7 +276,7 @@ static void __csid_configure_rdi_stream(struct csid_device *csid, u8 enable, u8
>         val |= 1 << RDI_CFG0_TIMESTAMP_EN;
>         /* note: for non-RDI path, this should be format->decode_format */
>         val |= DECODE_FORMAT_PAYLOAD_ONLY << RDI_CFG0_DECODE_FORMAT;
> -       val |= format->data_type << RDI_CFG0_DATA_TYPE;
> +       val |= dt << RDI_CFG0_DATA_TYPE;
>         val |= vc << RDI_CFG0_VIRTUAL_CHANNEL;
>         val |= dt_id << RDI_CFG0_DT_ID;
>         writel_relaxed(val, csid->base + CSID_RDI_CFG0(port));
> @@ -330,15 +325,60 @@ static void csid_configure_stream(struct csid_device *csid, u8 enable)
>         /* Loop through all enabled ports and configure a stream for each */
>         for (i = 0; i < MSM_CSID_MAX_SRC_STREAMS; i++)
>                 if (csid->phy.en_vc & BIT(i)) {
> +                       /* Source pads match RDI channels: pad 1 -> RDI0, pad 2 -> RDI1, etc. */
> +                       struct v4l2_mbus_framefmt *input_format =
> +                               &csid->fmt[MSM_CSID_PAD_FIRST_SRC + i];
> +                       const struct csid_format_info *format =
> +                               csid_get_fmt_entry(csid->res->formats->formats,
> +                                                  csid->res->formats->nformats,
> +                                                  input_format->code);
> +                       u8 vc = 0;
> +
>                         if (tg->enabled)
>                                 __csid_configure_testgen(csid, enable, i, 0);
>
> -                       __csid_configure_rdi_stream(csid, enable, i, 0);
> +                       __csid_configure_rdi_stream(csid, enable, i, vc, format->data_type);
>                         __csid_configure_rx(csid, &csid->phy, 0);
>                         __csid_ctrl_rdi(csid, enable, i);
>                 }
>  }
>
> +/*
> + * configure_rx - Configure the CSID Rx front-end
> + */
> +static void csid_configure_rx(struct csid_device *csid)
> +{
> +       __csid_configure_rx(csid, &csid->phy, 0);
> +}
> +
> +/*
> + * stream_id is used directly as the hardware RDI port index below. This
> + * assumes a 1:1 stream-to-port mapping and should be revisited once
> + * per-platform src_streams data (stream id -> hw pipe) is added.
> + */
> +static void csid_enable_stream(struct csid_device *csid, u32 stream_id, u8 vc, u8 dt)
> +{
> +       struct csid_testgen_config *tg = &csid->testgen;
> +
> +       if (tg->enabled)
> +               __csid_configure_testgen(csid, 1, stream_id, vc);
> +
> +       __csid_configure_rdi_stream(csid, 1, stream_id, vc, dt);
> +       __csid_ctrl_rdi(csid, 1, stream_id);
> +}
> +
> +static void csid_disable_stream(struct csid_device *csid, u32 stream_id)
> +{
> +       struct csid_testgen_config *tg = &csid->testgen;
> +
> +       __csid_ctrl_rdi(csid, 0, stream_id);
> +
> +       if (tg->enabled)
> +               __csid_configure_testgen(csid, 0, stream_id, 0);
> +
> +       __csid_configure_rdi_stream(csid, 0, stream_id, 0, 0);
> +}
> +
>  static int csid_configure_testgen_pattern(struct csid_device *csid, s32 val)
>  {
>         if (val > 0 && val <= csid->testgen.nmodes)
> @@ -425,6 +465,9 @@ static void csid_subdev_init(struct csid_device *csid)
>  const struct csid_hw_ops csid_ops_gen2 = {
>         .configure_stream = csid_configure_stream,
>         .configure_testgen_pattern = csid_configure_testgen_pattern,
> +       .configure_rx = csid_configure_rx,
> +       .enable_stream = csid_enable_stream,
> +       .disable_stream = csid_disable_stream,

Would it be possible to leverage the existing CSID configure_stream()
callback to implement the stream API? This would likely require
reconfiguring all CSID ports whenever a stream is started/stopped, but
if the configuration remains unchanged, I do not think that would be
problematic. That said, we probably need to address the current
ambiguity between the concepts of "VC" and "PORT" in the existing
drivers first.

>         .hw_version = csid_hw_version,
>         .isr = csid_isr,
>         .reset = csid_reset,
> --
> 2.34.1
>

Re: [PATCH 2/8] media: qcom: camss: Add streams API hw_ops to CSID interface

[Gjorgji Rosikopulos (Consultant)]

Hi Bryan,

Thanks for the review,

On 9/11/2026 1:43 PM, Bryan O'Donoghue wrote:
> On 11/09/2026 07:22, 
> Gjorgji.Rosikopulos.gjorgji.rosikopulos@oss.qualcomm.com wrote:
>> From: Gjorgji Rosikopulos <gjorgji.rosikopulos@oss.qualcomm.com>
>>
>> Add configure_rx/enable_stream/disable_stream to struct csid_hw_ops:
>> per-stream hardware operations needed to support the V4L2 subdev
>> streams API, where the CSID Rx front-end is configured once
>> independently of the per-stream RDI programming.
>>
>> No functional change: these are new optional hw_ops entries, not yet
>> implemented by any backend or called by any code path.
>>
>> Signed-off-by: Gjorgji Rosikopulos <gjorgji.rosikopulos@oss.qualcomm.com>
>> ---
>>   .../media/platform/qcom/camss/camss-csid.h    | 43 +++++++++++++++++++
>>   1 file changed, 43 insertions(+)
>>
>> diff --git a/drivers/media/platform/qcom/camss/camss-csid.h b/drivers/media/platform/qcom/camss/camss-csid.h
>> index 5296b10f6bac..90ee611b9092 100644
>> --- a/drivers/media/platform/qcom/camss/camss-csid.h
>> +++ b/drivers/media/platform/qcom/camss/camss-csid.h
>> @@ -74,6 +74,25 @@ struct csid_phy_config {
>>   
>>   struct csid_device;
>>   
>> +/*
>> + * struct csid_hw_ops - CSID hardware version specific operations
>> + *
>> + * configure_stream() is the legacy (non-streams-API) path and is mutually
>> + * exclusive with enable_stream()/disable_stream() - a given CSID instance
>> + * uses one family or the other, chosen by streams_enable.
>> + *
>> + * stream_id, as passed to enable_stream()/disable_stream(), is a hardware
>> + * RDI/PIX port index in [0, MSM_CSID_MAX_SRC_STREAMS). It is currently
>> + * always equal to pad - MSM_CSID_PAD_FIRST_SRC (multi-pad, 1:1
>> + * pad-to-port mapping).
> 
> Lets not mix and match names. There has historically been a huge amount 
> of sin and liberty taken, not the good kind unfortunately - with port_id 
> and write-master.
> 
> port I believe is the misnomer we are currently stuck with - so lets 
> continue to use it until a more crisp naming convention happens.

I agree will switch to port until we have better name and change it everywhere.

> 
>> + *
>> + * configure_rx() is called once, on the transition from zero to non-zero
>> + * active sink streams, strictly before the first enable_stream() call.
>> + * enable_stream()/disable_stream() calls are always made under the
>> + * subdev's active-state lock (core-enforced via
>> + * v4l2_subdev_enable_streams()/disable_streams()), so hw_ops
>> + * implementations don't need their own serialization.
>> + */
>>   struct csid_hw_ops {
>>   	/*
>>   	 * configure_stream - Configures and starts CSID input stream
>> @@ -81,6 +100,30 @@ struct csid_hw_ops {
>>   	 */
>>   	void (*configure_stream)(struct csid_device *csid, u8 enable);
>>   
>> +	/*
>> +	 * configure_rx - Configure the CSID Rx front-end
>> +	 * @csid: CSID device
>> +	 *
>> +	 * Called once, when the first stream is enabled.
>> +	 */
>> +	void (*configure_rx)(struct csid_device *csid);
>> +
>> +	/*
>> +	 * enable_stream - Enable a single CSID output stream
>> +	 * @csid: CSID device
>> +	 * @stream_id: stream id to enable
>> +	 * @vc: virtual channel to program
>> +	 * @dt: data type to program
>> +	 */
>> +	void (*enable_stream)(struct csid_device *csid, u32 stream_id, u8 vc, u8 dt);
>> +
>> +	/*
>> +	 * disable_stream - Disable a single CSID output stream
>> +	 * @csid: CSID device
>> +	 * @stream_id: stream id to disable
>> +	 */
>> +	void (*disable_stream)(struct csid_device *csid, u32 stream_id);
>> +
> 
> Seems odd that none of these have integer return types to propagate errors.

Well initially they were int indeed, but i get some internal review comments to switch 
them to void, currently the implementation is writing only to the registers. 
So i am ok to switch to int return type, other option is to leave them as they are
and if needed in future to switch to int return type.

> 
>>   	/*
>>   	 * configure_testgen_pattern - Validates and configures output pattern mode
>>   	 * of test pattern generator
> 

Re: [PATCH 3/8] media: qcom: camss: Implement CSID streams API hw_ops for gen2

[Gjorgji Rosikopulos (Consultant)]

Hi Bryan,

Thanks for the review,

On 9/11/2026 1:46 PM, Bryan O'Donoghue wrote:
> On 11/09/2026 07:22, 
> Gjorgji.Rosikopulos.gjorgji.rosikopulos@oss.qualcomm.com wrote:
>> From: Gjorgji Rosikopulos <gjorgji.rosikopulos@oss.qualcomm.com>
>>
>> Implement the configure_rx/enable_stream/disable_stream hw_ops added to
>> struct csid_hw_ops, for the gen2 CSID hardware backend used by SM8250.
>>
>> __csid_configure_rdi_stream() now takes an explicit data type parameter
>> instead of deriving it from the CSID source pad's format, since the
>> streams API keys each stream by stream_id rather than by pad. The
>> existing configure_stream() path is updated to pass its per-port format
>> lookup through to this parameter, preserving current behavior.
>>
>> Signed-off-by: Gjorgji Rosikopulos <gjorgji.rosikopulos@oss.qualcomm.com>
>> ---
>>   .../platform/qcom/camss/camss-csid-gen2.c     | 59 ++++++++++++++++---
>>   1 file changed, 51 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/media/platform/qcom/camss/camss-csid-gen2.c b/drivers/media/platform/qcom/camss/camss-csid-gen2.c
>> index eadcb2f7e3aa..2512cdb16d94 100644
>> --- a/drivers/media/platform/qcom/camss/camss-csid-gen2.c
>> +++ b/drivers/media/platform/qcom/camss/camss-csid-gen2.c
>> @@ -253,13 +253,8 @@ static void __csid_configure_testgen(struct csid_device *csid, u8 enable, u8 por
>>   	writel_relaxed(val, csid->base + CSID_TPG_CTRL);
>>   }
>>   
>> -static void __csid_configure_rdi_stream(struct csid_device *csid, u8 enable, u8 port, u8 vc)
>> +static void __csid_configure_rdi_stream(struct csid_device *csid, u8 enable, u8 port, u8 vc, u8 dt)
>>   {
>> -	/* Source pads matching RDI channels on hardware. Pad 1 -> RDI0, Pad 2 -> RDI1, etc. */
>> -	struct v4l2_mbus_framefmt *input_format = &csid->fmt[MSM_CSID_PAD_FIRST_SRC + port];
>> -	const struct csid_format_info *format = csid_get_fmt_entry(csid->res->formats->formats,
>> -								   csid->res->formats->nformats,
>> -								   input_format->code);
>>   	u32 val;
>>   
>>   	/*
>> @@ -281,7 +276,7 @@ static void __csid_configure_rdi_stream(struct csid_device *csid, u8 enable, u8
>>   	val |= 1 << RDI_CFG0_TIMESTAMP_EN;
>>   	/* note: for non-RDI path, this should be format->decode_format */
>>   	val |= DECODE_FORMAT_PAYLOAD_ONLY << RDI_CFG0_DECODE_FORMAT;
>> -	val |= format->data_type << RDI_CFG0_DATA_TYPE;
>> +	val |= dt << RDI_CFG0_DATA_TYPE;
>>   	val |= vc << RDI_CFG0_VIRTUAL_CHANNEL;
>>   	val |= dt_id << RDI_CFG0_DT_ID;
>>   	writel_relaxed(val, csid->base + CSID_RDI_CFG0(port));
>> @@ -330,15 +325,60 @@ static void csid_configure_stream(struct csid_device *csid, u8 enable)
>>   	/* Loop through all enabled ports and configure a stream for each */
>>   	for (i = 0; i < MSM_CSID_MAX_SRC_STREAMS; i++)
>>   		if (csid->phy.en_vc & BIT(i)) {
>> +			/* Source pads match RDI channels: pad 1 -> RDI0, pad 2 -> RDI1, etc. */
>> +			struct v4l2_mbus_framefmt *input_format =
>> +				&csid->fmt[MSM_CSID_PAD_FIRST_SRC + i];
>> +			const struct csid_format_info *format =
>> +				csid_get_fmt_entry(csid->res->formats->formats,
>> +						   csid->res->formats->nformats,
>> +						   input_format->code);
>> +			u8 vc = 0;
>> +
>>   			if (tg->enabled)
>>   				__csid_configure_testgen(csid, enable, i, 0);
>>   
>> -			__csid_configure_rdi_stream(csid, enable, i, 0);
>> +			__csid_configure_rdi_stream(csid, enable, i, vc, format->data_type);
>>   			__csid_configure_rx(csid, &csid->phy, 0);
>>   			__csid_ctrl_rdi(csid, enable, i);
>>   		}
>>   }
>>   
>> +/*
>> + * configure_rx - Configure the CSID Rx front-end
>> + */
>> +static void csid_configure_rx(struct csid_device *csid)
>> +{
>> +	__csid_configure_rx(csid, &csid->phy, 0);
>> +}
>> +
>> +/*
>> + * stream_id is used directly as the hardware RDI port index below. This
>> + * assumes a 1:1 stream-to-port mapping and should be revisited once
>> + * per-platform src_streams data (stream id -> hw pipe) is added.
>> + */
>> +static void csid_enable_stream(struct csid_device *csid, u32 stream_id, u8 vc, u8 dt)
>> +{
>> +	struct csid_testgen_config *tg = &csid->testgen;
>> +
>> +	if (tg->enabled)
>> +		__csid_configure_testgen(csid, 1, stream_id, vc);
>> +
>> +	__csid_configure_rdi_stream(csid, 1, stream_id, vc, dt);
>> +	__csid_ctrl_rdi(csid, 1, stream_id);
>> +}
>> +
>> +static void csid_disable_stream(struct csid_device *csid, u32 stream_id)
>> +{
>> +	struct csid_testgen_config *tg = &csid->testgen;
>> +
>> +	__csid_ctrl_rdi(csid, 0, stream_id);
>> +
>> +	if (tg->enabled)
>> +		__csid_configure_testgen(csid, 0, stream_id, 0);
>> +
>> +	__csid_configure_rdi_stream(csid, 0, stream_id, 0, 0);
>> +}
>> +
>>   static int csid_configure_testgen_pattern(struct csid_device *csid, s32 val)
>>   {
>>   	if (val > 0 && val <= csid->testgen.nmodes)
>> @@ -425,6 +465,9 @@ static void csid_subdev_init(struct csid_device *csid)
>>   const struct csid_hw_ops csid_ops_gen2 = {
>>   	.configure_stream = csid_configure_stream,
>>   	.configure_testgen_pattern = csid_configure_testgen_pattern,
>> +	.configure_rx = csid_configure_rx,
> 
> Keep the namespace consistent and include _stream somewhere in this name

I agree it will be added in next patchset.

~Gjorgji

Re: [PATCH 3/8] media: qcom: camss: Implement CSID streams API hw_ops for gen2

[Gjorgji Rosikopulos (Consultant)]

Hi Loic, 

Thanks for the review,

On 9/11/2026 4:30 PM, Loic Poulain wrote:
> Hi Gjorgji,
> 
> On Fri, Sep 11, 2026 at 8:22 AM Gjorgji Rosikopulos
> <gjorgji.rosikopulos@oss.qualcomm.com> wrote:
>>
>> From: Gjorgji Rosikopulos <gjorgji.rosikopulos@oss.qualcomm.com>
>>
>> Implement the configure_rx/enable_stream/disable_stream hw_ops added to
>> struct csid_hw_ops, for the gen2 CSID hardware backend used by SM8250.
>>
>> __csid_configure_rdi_stream() now takes an explicit data type parameter
>> instead of deriving it from the CSID source pad's format, since the
>> streams API keys each stream by stream_id rather than by pad. The
>> existing configure_stream() path is updated to pass its per-port format
>> lookup through to this parameter, preserving current behavior.
>>
>> Signed-off-by: Gjorgji Rosikopulos <gjorgji.rosikopulos@oss.qualcomm.com>
>> ---
>>  .../platform/qcom/camss/camss-csid-gen2.c     | 59 ++++++++++++++++---
>>  1 file changed, 51 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/media/platform/qcom/camss/camss-csid-gen2.c b/drivers/media/platform/qcom/camss/camss-csid-gen2.c
>> index eadcb2f7e3aa..2512cdb16d94 100644
>> --- a/drivers/media/platform/qcom/camss/camss-csid-gen2.c
>> +++ b/drivers/media/platform/qcom/camss/camss-csid-gen2.c
>> @@ -253,13 +253,8 @@ static void __csid_configure_testgen(struct csid_device *csid, u8 enable, u8 por
>>         writel_relaxed(val, csid->base + CSID_TPG_CTRL);
>>  }
>>
>> -static void __csid_configure_rdi_stream(struct csid_device *csid, u8 enable, u8 port, u8 vc)
>> +static void __csid_configure_rdi_stream(struct csid_device *csid, u8 enable, u8 port, u8 vc, u8 dt)
>>  {
>> -       /* Source pads matching RDI channels on hardware. Pad 1 -> RDI0, Pad 2 -> RDI1, etc. */
>> -       struct v4l2_mbus_framefmt *input_format = &csid->fmt[MSM_CSID_PAD_FIRST_SRC + port];
>> -       const struct csid_format_info *format = csid_get_fmt_entry(csid->res->formats->formats,
>> -                                                                  csid->res->formats->nformats,
>> -                                                                  input_format->code);
>>         u32 val;
>>
>>         /*
>> @@ -281,7 +276,7 @@ static void __csid_configure_rdi_stream(struct csid_device *csid, u8 enable, u8
>>         val |= 1 << RDI_CFG0_TIMESTAMP_EN;
>>         /* note: for non-RDI path, this should be format->decode_format */
>>         val |= DECODE_FORMAT_PAYLOAD_ONLY << RDI_CFG0_DECODE_FORMAT;
>> -       val |= format->data_type << RDI_CFG0_DATA_TYPE;
>> +       val |= dt << RDI_CFG0_DATA_TYPE;
>>         val |= vc << RDI_CFG0_VIRTUAL_CHANNEL;
>>         val |= dt_id << RDI_CFG0_DT_ID;
>>         writel_relaxed(val, csid->base + CSID_RDI_CFG0(port));
>> @@ -330,15 +325,60 @@ static void csid_configure_stream(struct csid_device *csid, u8 enable)
>>         /* Loop through all enabled ports and configure a stream for each */
>>         for (i = 0; i < MSM_CSID_MAX_SRC_STREAMS; i++)
>>                 if (csid->phy.en_vc & BIT(i)) {
>> +                       /* Source pads match RDI channels: pad 1 -> RDI0, pad 2 -> RDI1, etc. */
>> +                       struct v4l2_mbus_framefmt *input_format =
>> +                               &csid->fmt[MSM_CSID_PAD_FIRST_SRC + i];
>> +                       const struct csid_format_info *format =
>> +                               csid_get_fmt_entry(csid->res->formats->formats,
>> +                                                  csid->res->formats->nformats,
>> +                                                  input_format->code);
>> +                       u8 vc = 0;
>> +
>>                         if (tg->enabled)
>>                                 __csid_configure_testgen(csid, enable, i, 0);
>>
>> -                       __csid_configure_rdi_stream(csid, enable, i, 0);
>> +                       __csid_configure_rdi_stream(csid, enable, i, vc, format->data_type);
>>                         __csid_configure_rx(csid, &csid->phy, 0);
>>                         __csid_ctrl_rdi(csid, enable, i);
>>                 }
>>  }
>>
>> +/*
>> + * configure_rx - Configure the CSID Rx front-end
>> + */
>> +static void csid_configure_rx(struct csid_device *csid)
>> +{
>> +       __csid_configure_rx(csid, &csid->phy, 0);
>> +}
>> +
>> +/*
>> + * stream_id is used directly as the hardware RDI port index below. This
>> + * assumes a 1:1 stream-to-port mapping and should be revisited once
>> + * per-platform src_streams data (stream id -> hw pipe) is added.
>> + */
>> +static void csid_enable_stream(struct csid_device *csid, u32 stream_id, u8 vc, u8 dt)
>> +{
>> +       struct csid_testgen_config *tg = &csid->testgen;
>> +
>> +       if (tg->enabled)
>> +               __csid_configure_testgen(csid, 1, stream_id, vc);
>> +
>> +       __csid_configure_rdi_stream(csid, 1, stream_id, vc, dt);
>> +       __csid_ctrl_rdi(csid, 1, stream_id);
>> +}
>> +
>> +static void csid_disable_stream(struct csid_device *csid, u32 stream_id)
>> +{
>> +       struct csid_testgen_config *tg = &csid->testgen;
>> +
>> +       __csid_ctrl_rdi(csid, 0, stream_id);
>> +
>> +       if (tg->enabled)
>> +               __csid_configure_testgen(csid, 0, stream_id, 0);
>> +
>> +       __csid_configure_rdi_stream(csid, 0, stream_id, 0, 0);
>> +}
>> +
>>  static int csid_configure_testgen_pattern(struct csid_device *csid, s32 val)
>>  {
>>         if (val > 0 && val <= csid->testgen.nmodes)
>> @@ -425,6 +465,9 @@ static void csid_subdev_init(struct csid_device *csid)
>>  const struct csid_hw_ops csid_ops_gen2 = {
>>         .configure_stream = csid_configure_stream,
>>         .configure_testgen_pattern = csid_configure_testgen_pattern,
>> +       .configure_rx = csid_configure_rx,
>> +       .enable_stream = csid_enable_stream,
>> +       .disable_stream = csid_disable_stream,
> 
> Would it be possible to leverage the existing CSID configure_stream()
> callback to implement the stream API? This would likely require
> reconfiguring all CSID ports whenever a stream is started/stopped, but
> if the configuration remains unchanged, I do not think that would be
> problematic. That said, we probably need to address the current
> ambiguity between the concepts of "VC" and "PORT" in the existing
> drivers first.

I have experimented with something similar, we know in advance which
streams will be routed and active using stream routes, we can enable
them in advance. The drawback of that was that all the streams (even inactive)
will be enabled when first stream is enabled, but that is the case even today.

What you propose is something different, but that will require changing of the existing
api and verification of all csid implementations, i am ok with the proposal 
if someone can help with the verification of other devices.

~Gjorgji

Re: [PATCH 4/8] media: qcom: camss: Add streams API support in CSID subdevice

[Gjorgji Rosikopulos (Consultant)]

Hi Bryan,

Thanks for the review,

On 9/11/2026 2:35 PM, Bryan O'Donoghue wrote:
> On 11/09/2026 07:22, 
> Gjorgji.Rosikopulos.gjorgji.rosikopulos@oss.qualcomm.com wrote:
>> From: Gjorgji Rosikopulos <gjorgji.rosikopulos@oss.qualcomm.com>
>>
>> Add the V4L2 subdev streams API to the CSID driver: per-source-pad
>> enable/disable_streams pad ops, VC/DT discovery via get_frame_desc on
>> the remote sink pad, and a routing table that routes a single incoming
>> sink stream to every source pad by default (remappable via
>> set_routing for multi-VC sensors).
>>
>> Active streams are tracked per pad via a per-pad enabled_streams[]
>> bitmask, so that enable/disable_streams correctly propagates to the
>> CSIPHY only on first arrival/last departure of a sink stream, and
>> multiple source-pad consumers can share a single propagated sink
>> stream without redundant or colliding propagation.
>>
>> csid_init_state() caps the number of default routes to
>> MSM_CSID_MAX_SRC_STREAMS - 1 for non-lite CSIDs, matching the 3 usable
>> RDI pads on full-IFE CSIDs (the 4th/pix pad is non-functional).
> 
> That makes sense.
> 
>> msm_csid_register_entity() mirrors the same is_lite check for the pad
>> count itself, so non-lite CSIDs no longer register a pix source pad
>> that no route ever targets.
> 
> Nope. I'll send a different solution. Fixing the non-functional and 
> incorrectly mapped pix is a Fixes: level thing not a workaround it 
> inline thing.
> 
> Just drop the workaround and assume any CSID you are working with 
> actually works and is valid.

Ok will do that in the next patchset.

> 
>> csid_pad_enable_streams() rejects enabling with -ENOLINK when there is
>> no remote sink link and the test generator is disabled, matching the
>> equivalent check the legacy csid_set_stream() already performs.
>>
>> msm_csid_register_entity() assigns the ctrl handler's lock as the
>> subdev's state_lock before v4l2_subdev_init_finalize(), when the test
>> pattern control is present. Without this, the test-pattern S_CTRL
>> handler and the streams API's active-state accessors serialize on two
>> independent locks despite both touching csid->testgen.enabled,
>> allowing a concurrent S_CTRL(TEST_PATTERN) and stream enable/disable to
>> race.
> 
> Actually this reminds me of the first go at VCs in CAMSS which ended up 
> getting rolled back.
> 
> How will this be tested ? Is the TPG capable of generating different VCs ?

TPG can not produce simultaneous streams with different VC's so can not be
used for verification. I have setup for verification but checking if can
be shared publicly. I agree having implementation without a way to test and
verify by the maintaner is same as having nothing...

> 
> _That_ would be very useful.
> 
>>
>> This is opt-in per CSID instance via the new streams_enable resource
>> flag; no platform sets it yet, so CSIDs continue to use the legacy
>> non-streams subdev ops unchanged.
>>
>> Signed-off-by: Gjorgji Rosikopulos <gjorgji.rosikopulos@oss.qualcomm.com>
>> ---
>>   .../media/platform/qcom/camss/camss-csid.c    | 494 +++++++++++++++++-
>>   .../media/platform/qcom/camss/camss-csid.h    |   2 +
>>   2 files changed, 491 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/media/platform/qcom/camss/camss-csid.c b/drivers/media/platform/qcom/camss/camss-csid.c
>> index 48459b46a981..ce4b07c0c1c2 100644
>> --- a/drivers/media/platform/qcom/camss/camss-csid.c
>> +++ b/drivers/media/platform/qcom/camss/camss-csid.c
>> @@ -842,7 +842,7 @@ static void csid_try_format(struct csid_device *csid,
>>   
>>   		break;
>>   
>> -	case MSM_CSID_PAD_SRC:
>> +	default:
> 
> Why subtract the case ?
> Just add the default:
> 
> Also I don't think that code change is anything todo with adding streams 
> support to CSID.
> 
> Separate patches for separate things.

I agree somehow this was included, it will be moved to separate patch.

> 
>>   		if (csid->testgen.nmodes == CSID_PAYLOAD_MODE_DISABLED ||
>>   		    csid->testgen_mode->cur.val == 0) {
>>   			/* Test generator is disabled, */
>> @@ -1338,10 +1338,476 @@ static const struct v4l2_subdev_ops csid_v4l2_ops = {
>>   	.pad = &csid_pad_ops,
>>   };
>>   
>> +/*
>> + * csid_get_stream_csi2_desc - Discover the virtual channel and data type
>> + *			       used by a given sink stream, from an
>> + *			       already-fetched frame descriptor
>> + * @frame_desc: Frame descriptor fetched via .get_frame_desc from the remote
>> + *		subdev linked on the sink pad
>> + * @sink_stream: Sink-side stream number to look up
>> + * @desc_csi2: Returns the discovered virtual channel/data type on success
>> + *
>> + * A frame descriptor with a single entry means the remote only exposes one
>> + * stream (e.g. a single-VC sensor), which feeds every CSID source pad, so
>> + * that entry is used regardless of @sink_stream.
>> + *
>> + * Return true if a matching entry was found, false otherwise
>> + */
>> +static bool csid_get_stream_csi2_desc(struct v4l2_mbus_frame_desc *frame_desc,
>> +				      u32 sink_stream,
>> +				      struct v4l2_mbus_frame_desc_entry_csi2 *desc_csi2)
>> +{
>> +	unsigned int i;
>> +
>> +	if (frame_desc->type != V4L2_MBUS_FRAME_DESC_TYPE_CSI2 || !frame_desc->num_entries)
>> +		return false;
>> +
>> +	if (frame_desc->num_entries == 1) {
>> +		*desc_csi2 = frame_desc->entry[0].bus.csi2;
>> +		return true;
>> +	}
> 
> You can drop that check entirely, the below loop will do exactly the 
> same thing for num_entires == 1..

I Agree, will be done in next patchset.

> 
>> +
>> +	for (i = 0; i < frame_desc->num_entries; i++) {
>> +		if (frame_desc->entry[i].stream == sink_stream) {
>> +			*desc_csi2 = frame_desc->entry[i].bus.csi2;
>> +			return true;
>> +		}
>> +	}
>> +
>> +	return false;
>> +}
>> +
>> +/*
>> + * csid_get_stream_vc_dt - Discover the virtual channel and data type to
>> + *			   program for a given sink pad/stream, falling back
>> + *			   to @format_dt when no frame descriptor is available
>> + * @csid: CSID device
>> + * @state: V4L2 subdevice state
>> + * @remote_pad: Remote pad linked on the CSID sink pad, or NULL if unlinked
>> + * @pad: Source pad number the caller is enabling a stream on
>> + * @format_dt: Data type derived from the sink format, used as a fallback
>> + *	       and sanity-checked against the discovered data type
>> + *
>> + * Return the discovered virtual channel/data type, or {0, @format_dt} if
>> + * not discovered
>> + */
>> +static struct v4l2_mbus_frame_desc_entry_csi2
>> +csid_get_stream_vc_dt(struct csid_device *csid, struct v4l2_subdev_state *state,
>> +		      struct media_pad *remote_pad, u32 pad, u8 format_dt)
>> +{
>> +	struct v4l2_mbus_frame_desc_entry_csi2 desc_csi2 = { .dt = format_dt };
>> +	struct v4l2_mbus_frame_desc fd = { };
>> +	u32 sink_stream;
>> +
>> +	if (!remote_pad ||
>> +	    v4l2_subdev_call(media_entity_to_v4l2_subdev(remote_pad->entity),
>> +			     pad, get_frame_desc, remote_pad->index, &fd))
>> +		return desc_csi2;
>> +
> 
> if (thing ||
>      some_other_thing) {
> 	return desc_csi2;
> }
> 

I agree that will be fixed.

>> +	if (v4l2_subdev_routing_find_opposite_end(&state->routing, pad, 0, NULL, &sink_stream))
>> +		return desc_csi2;
>> +
>> +	if (!csid_get_stream_csi2_desc(&fd, sink_stream, &desc_csi2)) {
>> +		dev_warn(csid->camss->dev,
>> +			 "Failed to find CSI2 descriptor for sink stream %u, using vc=%u dt=%u\n",
>> +			 sink_stream, desc_csi2.vc, desc_csi2.dt);
>> +		return desc_csi2;
> 
> Is this an error it seems like it should be ?

I was also not sure. Maybe is better to mark as an error anyways the streaming will likely to not have
calid vc/dt.

> 
>> +	}
>> +
>> +	if (desc_csi2.dt != format_dt)
>> +		dev_warn(csid->camss->dev,
>> +			 "Sink stream %u frame desc dt=%u differs from format dt=%u, using dt=%u\n",
>> +			 sink_stream, desc_csi2.dt, format_dt, desc_csi2.dt);
>> +
>> +	return desc_csi2;
> 
> I'd return a pointer here.

Well the structure has only two u8 fields i think that passing pointer as argument or allocating and returing
pointer will likely be to match. But if you think is better i can switch to pointer.

> 
>> +}
>> +
>> +/*
>> + * csid_pad_enable_streams - Enable one or more streams on a source pad
>> + * @sd: CSID V4L2 subdevice
>> + * @state: V4L2 subdevice state
>> + * @pad: Pad number
>> + * @streams_mask: Bitmask of v4l2 streams to enable
>> + *
>> + * The v4l2 core only calls this on a source pad (v4l2_subdev_enable_streams()
>> + * rejects sink pads with -EOPNOTSUPP before reaching the driver), so @pad is
>> + * not checked here. Each source pad only ever carries stream 0.
>> + *
>> + * The shared sink stream(s) are propagated upstream only once, on the
>> + * transition from no active sink streams to at least one, so that a second
>> + * consumer of the same shared sink stream never triggers a second, redundant
>> + * propagation to the sensor. The Rx front-end is likewise only configured
>> + * once, on that same transition.
>> + *
>> + * Return 0 on success, -ENOLINK if there is no remote sink link and the test
>> + * generator is disabled, or another negative error code otherwise
>> + */
>> +static int csid_pad_enable_streams(struct v4l2_subdev *sd,
>> +				   struct v4l2_subdev_state *state,
>> +				   u32 pad, u64 streams_mask)
>> +{
>> +	struct csid_device *csid = v4l2_get_subdevdata(sd);
>> +	const struct csid_hw_ops *hw_ops = csid->res->hw_ops;
>> +	struct media_pad *remote_pad = media_pad_remote_pad_first(&csid->pads[MSM_CSID_PAD_SINK]);
>> +	unsigned int hw_port = pad - MSM_CSID_PAD_FIRST_SRC;
>> +	const struct csid_format_info *format;
>> +	struct v4l2_mbus_frame_desc_entry_csi2 desc_csi2;
>> +	u64 sink_streams, propagate_mask;
>> +	int ret;
>> +
>> +	if (!csid->testgen.enabled && !remote_pad)
>> +		return -ENOLINK;
>> +
>> +	sink_streams = v4l2_subdev_state_xlate_streams(state, pad, MSM_CSID_PAD_SINK,
>> +						       &streams_mask);
>> +
>> +	if (!csid->enabled_streams[MSM_CSID_PAD_SINK]) {
>> +		if (csid->testgen.nmodes != CSID_PAYLOAD_MODE_DISABLED) {
>> +			/*
>> +			 * sd->state_lock is aliased to csid->ctrls.lock, and is
>> +			 * already held here by the v4l2_subdev_enable_streams()
>> +			 * caller, so use the lock-free variant to avoid
>> +			 * self-deadlocking on the same mutex.
>> +			 */
>> +			ret = __v4l2_ctrl_handler_setup(&csid->ctrls);
>> +			if (ret < 0) {
>> +				dev_err(csid->camss->dev,
>> +					"could not sync v4l2 controls: %d\n", ret);
>> +				return ret;
>> +			}
>> +		}
>> +
>> +		hw_ops->configure_rx(csid);
>> +	}
>> +
>> +	/* Sink streams already active elsewhere don't need re-propagating. */
>> +	propagate_mask = sink_streams & ~csid->enabled_streams[MSM_CSID_PAD_SINK];
>> +	csid->enabled_streams[MSM_CSID_PAD_SINK] |= sink_streams;
>> +	csid->enabled_streams[pad] |= streams_mask;
>> +
>> +	format = csid_get_fmt_entry(csid->res->formats->formats,
>> +				    csid->res->formats->nformats,
>> +				    csid->fmt[pad].code);
>> +	desc_csi2 = csid_get_stream_vc_dt(csid, state, remote_pad, pad, format->data_type);
> 
> You're doing an implict memcpy() here - just return a pointer.

Yes it is just two u8 fields vc and dt.

> 
> And I really wonder based on the activity within the method why 
> returning an error isn't part of this ?

Well yet to be discussed the get_frame_desc is not standartazied across the sensors,
What we have today.
If sensor supports get_frame_desc it will return the vc/dt per stream. You also need
to validate if the dt reported is matching with the dt converted from mbus format.

If sensor does not support get_frame_desc you need to asume that vc is 0 
and get the dt based on the mbus format. The idea of that function is to hide that
and return vc/dt regardless if sensor supports that op or not.

So if you think that having return and passing pointer for desc_csi2 is 
more portable for future implementations and extensions i will switch to that.

> 
>> +
>> +	hw_ops->enable_stream(csid, hw_port, desc_csi2.vc, desc_csi2.dt);
> 
> I commented elsewhere should this be void or int ?
> 
> I'm not suggesting either more asking rhetorically.

Not sure either. We are just writing the registers, if there is a way to validate that
streaming is actually started and having return code yes make sense to have return code,
but with the current implementation i dont see that is the case.

> 
>> +
>> +	if (propagate_mask && remote_pad) {
>> +		ret = v4l2_subdev_enable_streams(media_entity_to_v4l2_subdev(remote_pad->entity),
>> +						 remote_pad->index, propagate_mask);
>> +		if (ret) {
>> +			csid->enabled_streams[MSM_CSID_PAD_SINK] &= ~propagate_mask;
>> +			csid->enabled_streams[pad] &= ~streams_mask;
>> +
>> +			hw_ops->disable_stream(csid, hw_port);
>> +
>> +			return ret;
>> +		}
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +/*
>> + * csid_sink_streams_in_use - Compute the subset of sink streams still
>> + *			      referenced by a source pad other than @pad
>> + * @csid: CSID device
>> + * @state: V4L2 subdevice state
>> + * @pad: Source pad to exclude from the check
>> + * @sink_streams: Candidate sink streams to check
>> + *
>> + * Return the subset of @sink_streams still referenced by some other source
>> + * pad
>> + */
>> +static u64 csid_sink_streams_in_use(struct csid_device *csid, struct v4l2_subdev_state *state,
>> +				    u32 pad, u64 sink_streams)
>> +{
>> +	u64 in_use = 0;
>> +	unsigned int i;
>> +
>> +	for (i = MSM_CSID_PAD_FIRST_SRC; i < MSM_CSID_PADS_NUM; i++) {
>> +		u64 other_streams = csid->enabled_streams[i];
>> +		u64 other_sink_streams;
>> +
>> +		if (i == pad)
>> +			continue;
>> +
>> +		other_sink_streams = v4l2_subdev_state_xlate_streams(state, i, MSM_CSID_PAD_SINK,
>> +								     &other_streams);
>> +		in_use |= sink_streams & other_sink_streams;
>> +	}
>> +
>> +	return in_use;
>> +}
>> +
>> +/*
>> + * csid_pad_disable_streams - Disable one or more streams on a source pad
>> + * @sd: CSID V4L2 subdevice
>> + * @state: V4L2 subdevice state
>> + * @pad: Pad number
>> + * @streams_mask: Bitmask of v4l2 streams to disable
>> + *
>> + * The v4l2 core only calls this on a source pad (v4l2_subdev_disable_streams()
>> + * rejects sink pads with -EOPNOTSUPP before reaching the driver), so @pad is
>> + * not checked here. Each source pad only ever carries stream 0.
>> + *
>> + * A sink stream is only disabled, and propagated upstream to disable it there
>> + * too, once no source pad references it any more.
>> + *
>> + * Return 0 on success or a negative error code otherwise
>> + */
>> +static int csid_pad_disable_streams(struct v4l2_subdev *sd,
>> +				    struct v4l2_subdev_state *state,
>> +				    u32 pad, u64 streams_mask)
>> +{
>> +	struct csid_device *csid = v4l2_get_subdevdata(sd);
>> +	const struct csid_hw_ops *hw_ops = csid->res->hw_ops;
>> +	struct media_pad *remote_pad = media_pad_remote_pad_first(&csid->pads[MSM_CSID_PAD_SINK]);
>> +	unsigned int hw_port = pad - MSM_CSID_PAD_FIRST_SRC;
>> +	u64 sink_streams, disable_sink_streams;
>> +	int ret = 0;
>> +
>> +	sink_streams = v4l2_subdev_state_xlate_streams(state, pad, MSM_CSID_PAD_SINK,
>> +						       &streams_mask);
>> +
>> +	/* Keep a sink stream active as long as any other source pad still uses it. */
>> +	disable_sink_streams = sink_streams &
>> +				~csid_sink_streams_in_use(csid, state, pad, sink_streams);
>> +
>> +	if (disable_sink_streams && remote_pad) {
>> +		ret = v4l2_subdev_disable_streams(media_entity_to_v4l2_subdev(remote_pad->entity),
>> +						  remote_pad->index, disable_sink_streams);
>> +		if (ret)
>> +			dev_err(csid->camss->dev,
>> +				"Failed to disable stream on remote pad: %d\n", ret);
>> +	}
>> +
>> +	hw_ops->disable_stream(csid, hw_port);
>> +
>> +	csid->enabled_streams[pad] &= ~streams_mask;
>> +	csid->enabled_streams[MSM_CSID_PAD_SINK] &= ~disable_sink_streams;
>> +
>> +	return ret;
>> +}
>> +
>> +static const struct v4l2_mbus_framefmt csid_default_format = {
>> +	.code = MEDIA_BUS_FMT_UYVY8_1X16,
>> +	.width = 1920,
>> +	.height = 1080,
>> +	.field = V4L2_FIELD_NONE,
>> +	.colorspace = V4L2_COLORSPACE_SRGB,
>> +};
>> +
>> +/*
>> + * csid_set_routing - Handle setting of routing table
>> + * @sd: CSID V4L2 subdevice
>> + * @state: V4L2 subdevice state
>> + * @which: TRY or ACTIVE routing
>> + * @routing: Routing table to set
>> + *
>> + * Return 0 on success or a negative error code otherwise
>> + */
>> +static int csid_set_routing(struct v4l2_subdev *sd,
>> +			    struct v4l2_subdev_state *state,
>> +			    enum v4l2_subdev_format_whence which,
>> +			    struct v4l2_subdev_krouting *routing)
>> +{
>> +	struct csid_device *csid = v4l2_get_subdevdata(sd);
>> +	unsigned int i;
>> +	int ret;
>> +
>> +	if (which == V4L2_SUBDEV_FORMAT_ACTIVE && csid->enabled_streams[MSM_CSID_PAD_SINK])
>> +		return -EBUSY;
>> +
>> +	for (i = 0; i < routing->num_routes; i++)
>> +		if (routing->routes[i].source_stream != 0)
>> +			return -EINVAL;
>> +
>> +	ret = v4l2_subdev_routing_validate(sd, routing,
>> +					   V4L2_SUBDEV_ROUTING_NO_SOURCE_STREAM_MIX |
>> +					   V4L2_SUBDEV_ROUTING_NO_SOURCE_MULTIPLEXING |
>> +					   V4L2_SUBDEV_ROUTING_NO_N_TO_1);
>> +	if (ret)
>> +		return ret;
>> +
>> +	return v4l2_subdev_set_routing_with_fmt(sd, state, routing, &csid_default_format);
>> +}
>> +
>> +/*
>> + * __csid_get_stream_format - Get pointer to per-stream format structure
>> + * @csid: CSID device
>> + * @sd_state: V4L2 subdev state
>> + * @pad: pad from which format is requested
>> + * @stream: stream from which format is requested
>> + * @which: TRY or ACTIVE format
>> + *
>> + * Same as __csid_get_format(), but honors @stream for TRY-state lookups.
>> + * For ACTIVE state, csid->fmt[] is indexed by pad + stream. @stream is
>> + * always 0 and @pad selects the RDI channel (0-3).
>> + *
>> + * Return pointer to TRY or ACTIVE format structure
>> + */
>> +static struct v4l2_mbus_framefmt *
>> +__csid_get_stream_format(struct csid_device *csid,
>> +			 struct v4l2_subdev_state *sd_state,
>> +			 unsigned int pad, u32 stream,
>> +			 enum v4l2_subdev_format_whence which)
>> +{
>> +	if (which == V4L2_SUBDEV_FORMAT_TRY)
>> +		return v4l2_subdev_state_get_format(sd_state, pad, stream);
>> +
>> +	if (pad == MSM_CSID_PAD_SINK)
>> +		return &csid->fmt[MSM_CSID_PAD_SINK];
>> +
>> +	return &csid->fmt[pad + stream];
>> +}
>> +
>> +/*
>> + * csid_streams_get_format - Handle get format by pads subdev method
>> + * @sd: CSID V4L2 subdevice
>> + * @sd_state: V4L2 subdev state
>> + * @fmt: pointer to v4l2 subdev format structure
>> + *
>> + * Return -EINVAL or zero on success
>> + */
>> +static int csid_streams_get_format(struct v4l2_subdev *sd,
>> +				   struct v4l2_subdev_state *sd_state,
>> +				   struct v4l2_subdev_format *fmt)
>> +{
>> +	struct csid_device *csid = v4l2_get_subdevdata(sd);
>> +	struct v4l2_mbus_framefmt *format;
>> +
>> +	format = __csid_get_stream_format(csid, sd_state, fmt->pad, fmt->stream, fmt->which);
>> +	if (!format)
>> +		return -EINVAL;
>> +
>> +	fmt->format = *format;
>> +
>> +	return 0;
>> +}
>> +
>> +/*
>> + * csid_streams_set_format - Handle set format by pads subdev method
>> + * @sd: CSID V4L2 subdevice
>> + * @sd_state: V4L2 subdev state
>> + * @fmt: pointer to v4l2 subdev format structure
>> + *
>> + * Return -EINVAL or zero on success
>> + */
>> +static int csid_streams_set_format(struct v4l2_subdev *sd,
>> +				   struct v4l2_subdev_state *sd_state,
>> +				   struct v4l2_subdev_format *fmt)
>> +{
>> +	struct csid_device *csid = v4l2_get_subdevdata(sd);
>> +	struct v4l2_mbus_framefmt *format;
>> +	struct v4l2_subdev_route *route;
>> +
>> +	if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE && csid->enabled_streams[MSM_CSID_PAD_SINK])
>> +		return -EBUSY;
>> +
>> +	format = __csid_get_stream_format(csid, sd_state, fmt->pad, fmt->stream, fmt->which);
>> +	if (!format)
>> +		return -EINVAL;
>> +
>> +	csid_try_format(csid, sd_state, fmt->pad, &fmt->format, fmt->which);
>> +	*format = fmt->format;
>> +
>> +	/* Propagate the format from the sink stream to every source stream it feeds */
>> +	for_each_active_route(&sd_state->routing, route) {
>> +		struct v4l2_mbus_framefmt *src_format;
>> +
>> +		if (route->sink_pad != fmt->pad || route->sink_stream != fmt->stream)
>> +			continue;
>> +
>> +		src_format = __csid_get_stream_format(csid, sd_state, route->source_pad,
>> +						      route->source_stream, fmt->which);
>> +		if (!src_format)
>> +			continue;
>> +
>> +		*src_format = fmt->format;
>> +		csid_try_format(csid, sd_state, route->source_pad, src_format, fmt->which);
>> +	}
>> +
>> +	return 0;
>> +}
> 
> Why do we need a full new set of get-format and set-format ?
> 
> "Feels" like this could wrapper the existing code.

It can, for stream api you can have in one pad multiple streams and each stream
can have different format. In csid case becouse we have one stream per pad existing
API can be reused. If we decide to have muiltiple streams per source pad then we need
to intrudoce this function. I am ok to reuse existing function with some small change.

> 
>> +
>> +static const struct v4l2_subdev_pad_ops csid_streams_pad_ops = {
>> +	.enum_mbus_code = csid_enum_mbus_code,
>> +	.enum_frame_size = csid_enum_frame_size,
>> +	.get_fmt = csid_streams_get_format,
>> +	.set_fmt = csid_streams_set_format,
>> +	.set_routing = csid_set_routing,
>> +	.enable_streams = csid_pad_enable_streams,
>> +	.disable_streams = csid_pad_disable_streams,
>> +};
>> +
>> +static const struct v4l2_subdev_video_ops csid_streams_video_ops = {
>> +	.s_stream = v4l2_subdev_s_stream_helper,
>> +};
>> +
>> +static const struct v4l2_subdev_ops csid_streams_v4l2_ops = {
>> +	.core = &csid_core_ops,
>> +	.pad = &csid_streams_pad_ops,
>> +	.video = &csid_streams_video_ops,
>> +};
>> +
>> +/*
>> + * csid_init_state - Initialize the routing table for the streams API subdev
>> + * @sd: CSID V4L2 subdevice
>> + * @state: V4L2 subdev state
>> + *
>> + * source_stream is always 0: each source pad MSM_CSID_PAD_FIRST_SRC + i
>> + * links to its own independent downstream subdev, and a link's sink side is
>> + * validated against the implicit stream 0 exposed by any subdev without
>> + * V4L2_SUBDEV_FL_STREAMS (see v4l2_link_validate_get_streams()) — every
>> + * downstream VFE line is such a subdev.
>> + *
>> + * All source pads route from sink_stream 0 by default, fanning the single
>> + * incoming stream out to every port; a multi-VC source is supported by
>> + * remapping each route's sink_stream via .set_routing, leaving
>> + * source_pad/source_stream untouched.
>> + *
>> + * Return 0 on success or a negative error code otherwise
>> + */
>> +static int csid_init_state(struct v4l2_subdev *sd, struct v4l2_subdev_state *state)
>> +{
>> +	struct csid_device *csid = v4l2_get_subdevdata(sd);
>> +	struct v4l2_subdev_route routes[MSM_CSID_MAX_SRC_STREAMS];
>> +	struct v4l2_subdev_krouting routing = { };
>> +	unsigned int num_routes;
>> +	int i, ret;
>> +
>> +	/* The full IFE has only 3 rdi's and pix output is not functional */
>> +	if (csid_is_lite(csid))
>> +		num_routes = MSM_CSID_MAX_SRC_STREAMS;
>> +	else
>> +		num_routes = MSM_CSID_MAX_SRC_STREAMS - 1;
> 
> No. Don't code around this here.
> 
> I'll make a Fixes: patch for the pix stuff - I have it in tree. I don't 
> want to add work-arounds in code.

Ok, Sorry i did know that. Just that my scripts are iterating and verifying all
the paths, and this was failing i have introduced this change.

> 
>> +
>> +	for (i = 0; i < num_routes; i++) {
>> +		routes[i].sink_pad = MSM_CSID_PAD_SINK;
>> +		routes[i].sink_stream = 0;
>> +		routes[i].source_pad = MSM_CSID_PAD_FIRST_SRC + i;
>> +		routes[i].source_stream = 0;
>> +		routes[i].flags = V4L2_SUBDEV_ROUTE_FL_ACTIVE;
>> +	}
>> +
>> +	routing.num_routes = num_routes;
>> +	routing.routes = routes;
>> +	ret = v4l2_subdev_set_routing_with_fmt(sd, state, &routing, &csid_default_format);
>> +	if (ret)
>> +		dev_err(csid->camss->dev, "Failed to set routing: %d\n", ret);
>> +
>> +	return ret;
>> +}
>> +
>>   static const struct v4l2_subdev_internal_ops csid_v4l2_internal_ops = {
>>   	.open = csid_init_formats,
>>   };
>>   
>> +static const struct v4l2_subdev_internal_ops csid_streams_internal_ops = {
>> +	.init_state = csid_init_state,
>> +};
>> +
>>   static const struct media_entity_operations csid_media_ops = {
>>   	.link_setup = csid_link_setup,
>>   	.link_validate = v4l2_subdev_link_validate,
>> @@ -1360,13 +1826,18 @@ int msm_csid_register_entity(struct csid_device *csid,
>>   	struct v4l2_subdev *sd = &csid->subdev;
>>   	struct media_pad *pads = csid->pads;
>>   	struct device *dev = csid->camss->dev;
>> +	bool streams_api = csid->res->streams_enable;
> 
> As I stated elsewhere there's no need to have this flag copy/pasted.
> 
> Just move it one level up does this SoC support streams, in 
> camss->supports_streams;

Yes that is noted. Will be fixed in the next patchset.

~Gjorgji

Re: [PATCH 5/8] media: qcom: camss: Fix CSID-to-VFE all-to-all link crossbar on sm8250

[Gjorgji Rosikopulos (Consultant)]

Hi Bryan,

Thanks for the review,

On 9/11/2026 2:37 PM, Bryan O'Donoghue wrote:
> On 11/09/2026 07:22, 
> Gjorgji.Rosikopulos.gjorgji.rosikopulos@oss.qualcomm.com wrote:
>> From: Gjorgji Rosikopulos <gjorgji.rosikopulos@oss.qualcomm.com>
>>
>> camss_link_entities() unconditionally links every CSID to every VFE,
>> creating an all-to-all crossbar. On SM8250 the hardware wiring is
>> fixed: each CSID is hardwired to exactly one VFE (csid[i] <-> vfe[i]),
>> with no crossbar between instances. Enabling a mismatched link (e.g.
>> csid0 -> vfe1) creates a media link that does not correspond to any
>> real hardware datapath.
>>
>> Add a csid_vfe_fixed_pairing flag to struct camss_resources and set it
>> for sm8250_resources. When set, camss_link_entities() skips creating
>> links between CSID and VFE instances whose indices do not match.
>> Other platforms keep the historical all-to-all link creation.
>>
>> Signed-off-by: Gjorgji Rosikopulos <gjorgji.rosikopulos@oss.qualcomm.com>
>> ---
>>   drivers/media/platform/qcom/camss/camss.c | 7 ++++++-
>>   drivers/media/platform/qcom/camss/camss.h | 7 +++++++
>>   2 files changed, 13 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
>> index 2123f6388e3d..1bb22cd23c5e 100644
>> --- a/drivers/media/platform/qcom/camss/camss.c
>> +++ b/drivers/media/platform/qcom/camss/camss.c
>> @@ -5000,7 +5000,10 @@ static int camss_link_entities(struct camss *camss)
>>   				}
>>   	} else {
>>   		for (i = 0; i < camss->res->csid_num; i++)
>> -			for (k = 0; k < camss->res->vfe_num; k++)
>> +			for (k = 0; k < camss->res->vfe_num; k++) {
>> +				if (camss->res->csid_vfe_fixed_pairing && i != k)
>> +					continue;
>> +
> 
> A) This should be a Fixes
> B) This should not be limited to sm8250

I agree. I think either we can have this based on platform, or based on ispif availability.
The csid routing to all ife's is possible only in the platforms with ispif available.
Please tell me what is your preference i am fine with both.

> 
>>   				for (j = 0; j < camss->vfe[k].res->line_num; j++) {
>>   					struct v4l2_subdev *csid = &camss->csid[i].subdev;
>>   					struct v4l2_subdev *vfe = &camss->vfe[k].line[j].subdev;
>> @@ -5017,6 +5020,7 @@ static int camss_link_entities(struct camss *camss)
>>   						return ret;
>>   					}
>>   				}
>> +			}
>>   	}
>>   
>>   	return 0;
>> @@ -5666,6 +5670,7 @@ static const struct camss_resources sm8250_resources = {
>>   	.csiphy_num = ARRAY_SIZE(csiphy_res_8250),
>>   	.csid_num = ARRAY_SIZE(csid_res_8250),
>>   	.vfe_num = ARRAY_SIZE(vfe_res_8250),
>> +	.csid_vfe_fixed_pairing = true,
>>   };
>>   
>>   static const struct camss_resources sc8280xp_resources = {
>> diff --git a/drivers/media/platform/qcom/camss/camss.h b/drivers/media/platform/qcom/camss/camss.h
>> index 93d691c8ac63..ad0b42719788 100644
>> --- a/drivers/media/platform/qcom/camss/camss.h
>> +++ b/drivers/media/platform/qcom/camss/camss.h
>> @@ -119,6 +119,13 @@ struct camss_resources {
>>   	const unsigned int tpg_num;
>>   	const unsigned int csid_num;
>>   	const unsigned int vfe_num;
>> +	/*
>> +	 * True on platforms where each CSID is wired to exactly one VFE at
>> +	 * the same index (csid[i] <-> vfe[i]). When set, camss_link_entities()
>> +	 * only creates CSID->VFE links for matching indices instead of an
>> +	 * all-to-all crossbar.
>> +	 */
>> +	const bool csid_vfe_fixed_pairing;
> 
> I think the number of platforms where all-to-all is possible is quite 
> low - if possible on any.

Yes that is correct.

~Gjorgji

Re: [PATCH 3/8] media: qcom: camss: Implement CSID streams API hw_ops for gen2

[Gjorgji Rosikopulos (Consultant)]

On 9/11/2026 5:17 PM, Gjorgji Rosikopulos (Consultant) wrote:
> Hi Loic, 
> 
> Thanks for the review,
> 
> On 9/11/2026 4:30 PM, Loic Poulain wrote:
>> Hi Gjorgji,
>>
>> On Fri, Sep 11, 2026 at 8:22 AM Gjorgji Rosikopulos
>> <gjorgji.rosikopulos@oss.qualcomm.com> wrote:
>>>
>>> From: Gjorgji Rosikopulos <gjorgji.rosikopulos@oss.qualcomm.com>
>>>
>>> Implement the configure_rx/enable_stream/disable_stream hw_ops added to
>>> struct csid_hw_ops, for the gen2 CSID hardware backend used by SM8250.
>>>
>>> __csid_configure_rdi_stream() now takes an explicit data type parameter
>>> instead of deriving it from the CSID source pad's format, since the
>>> streams API keys each stream by stream_id rather than by pad. The
>>> existing configure_stream() path is updated to pass its per-port format
>>> lookup through to this parameter, preserving current behavior.
>>>
>>> Signed-off-by: Gjorgji Rosikopulos <gjorgji.rosikopulos@oss.qualcomm.com>
>>> ---
>>>  .../platform/qcom/camss/camss-csid-gen2.c     | 59 ++++++++++++++++---
>>>  1 file changed, 51 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/media/platform/qcom/camss/camss-csid-gen2.c b/drivers/media/platform/qcom/camss/camss-csid-gen2.c
>>> index eadcb2f7e3aa..2512cdb16d94 100644
>>> --- a/drivers/media/platform/qcom/camss/camss-csid-gen2.c
>>> +++ b/drivers/media/platform/qcom/camss/camss-csid-gen2.c
>>> @@ -253,13 +253,8 @@ static void __csid_configure_testgen(struct csid_device *csid, u8 enable, u8 por
>>>         writel_relaxed(val, csid->base + CSID_TPG_CTRL);
>>>  }
>>>
>>> -static void __csid_configure_rdi_stream(struct csid_device *csid, u8 enable, u8 port, u8 vc)
>>> +static void __csid_configure_rdi_stream(struct csid_device *csid, u8 enable, u8 port, u8 vc, u8 dt)
>>>  {
>>> -       /* Source pads matching RDI channels on hardware. Pad 1 -> RDI0, Pad 2 -> RDI1, etc. */
>>> -       struct v4l2_mbus_framefmt *input_format = &csid->fmt[MSM_CSID_PAD_FIRST_SRC + port];
>>> -       const struct csid_format_info *format = csid_get_fmt_entry(csid->res->formats->formats,
>>> -                                                                  csid->res->formats->nformats,
>>> -                                                                  input_format->code);
>>>         u32 val;
>>>
>>>         /*
>>> @@ -281,7 +276,7 @@ static void __csid_configure_rdi_stream(struct csid_device *csid, u8 enable, u8
>>>         val |= 1 << RDI_CFG0_TIMESTAMP_EN;
>>>         /* note: for non-RDI path, this should be format->decode_format */
>>>         val |= DECODE_FORMAT_PAYLOAD_ONLY << RDI_CFG0_DECODE_FORMAT;
>>> -       val |= format->data_type << RDI_CFG0_DATA_TYPE;
>>> +       val |= dt << RDI_CFG0_DATA_TYPE;
>>>         val |= vc << RDI_CFG0_VIRTUAL_CHANNEL;
>>>         val |= dt_id << RDI_CFG0_DT_ID;
>>>         writel_relaxed(val, csid->base + CSID_RDI_CFG0(port));
>>> @@ -330,15 +325,60 @@ static void csid_configure_stream(struct csid_device *csid, u8 enable)
>>>         /* Loop through all enabled ports and configure a stream for each */
>>>         for (i = 0; i < MSM_CSID_MAX_SRC_STREAMS; i++)
>>>                 if (csid->phy.en_vc & BIT(i)) {
>>> +                       /* Source pads match RDI channels: pad 1 -> RDI0, pad 2 -> RDI1, etc. */
>>> +                       struct v4l2_mbus_framefmt *input_format =
>>> +                               &csid->fmt[MSM_CSID_PAD_FIRST_SRC + i];
>>> +                       const struct csid_format_info *format =
>>> +                               csid_get_fmt_entry(csid->res->formats->formats,
>>> +                                                  csid->res->formats->nformats,
>>> +                                                  input_format->code);
>>> +                       u8 vc = 0;
>>> +
>>>                         if (tg->enabled)
>>>                                 __csid_configure_testgen(csid, enable, i, 0);
>>>
>>> -                       __csid_configure_rdi_stream(csid, enable, i, 0);
>>> +                       __csid_configure_rdi_stream(csid, enable, i, vc, format->data_type);
>>>                         __csid_configure_rx(csid, &csid->phy, 0);
>>>                         __csid_ctrl_rdi(csid, enable, i);
>>>                 }
>>>  }
>>>
>>> +/*
>>> + * configure_rx - Configure the CSID Rx front-end
>>> + */
>>> +static void csid_configure_rx(struct csid_device *csid)
>>> +{
>>> +       __csid_configure_rx(csid, &csid->phy, 0);
>>> +}
>>> +
>>> +/*
>>> + * stream_id is used directly as the hardware RDI port index below. This
>>> + * assumes a 1:1 stream-to-port mapping and should be revisited once
>>> + * per-platform src_streams data (stream id -> hw pipe) is added.
>>> + */
>>> +static void csid_enable_stream(struct csid_device *csid, u32 stream_id, u8 vc, u8 dt)
>>> +{
>>> +       struct csid_testgen_config *tg = &csid->testgen;
>>> +
>>> +       if (tg->enabled)
>>> +               __csid_configure_testgen(csid, 1, stream_id, vc);
>>> +
>>> +       __csid_configure_rdi_stream(csid, 1, stream_id, vc, dt);
>>> +       __csid_ctrl_rdi(csid, 1, stream_id);
>>> +}
>>> +
>>> +static void csid_disable_stream(struct csid_device *csid, u32 stream_id)
>>> +{
>>> +       struct csid_testgen_config *tg = &csid->testgen;
>>> +
>>> +       __csid_ctrl_rdi(csid, 0, stream_id);
>>> +
>>> +       if (tg->enabled)
>>> +               __csid_configure_testgen(csid, 0, stream_id, 0);
>>> +
>>> +       __csid_configure_rdi_stream(csid, 0, stream_id, 0, 0);
>>> +}
>>> +
>>>  static int csid_configure_testgen_pattern(struct csid_device *csid, s32 val)
>>>  {
>>>         if (val > 0 && val <= csid->testgen.nmodes)
>>> @@ -425,6 +465,9 @@ static void csid_subdev_init(struct csid_device *csid)
>>>  const struct csid_hw_ops csid_ops_gen2 = {
>>>         .configure_stream = csid_configure_stream,
>>>         .configure_testgen_pattern = csid_configure_testgen_pattern,
>>> +       .configure_rx = csid_configure_rx,
>>> +       .enable_stream = csid_enable_stream,
>>> +       .disable_stream = csid_disable_stream,
>>
>> Would it be possible to leverage the existing CSID configure_stream()
>> callback to implement the stream API? This would likely require
>> reconfiguring all CSID ports whenever a stream is started/stopped, but
>> if the configuration remains unchanged, I do not think that would be
>> problematic. That said, we probably need to address the current
>> ambiguity between the concepts of "VC" and "PORT" in the existing
>> drivers first.
> 
> I have experimented with something similar, we know in advance which
> streams will be routed and active using stream routes, we can enable
> them in advance. The drawback of that was that all the streams (even inactive)
> will be enabled when first stream is enabled, but that is the case even today.
> 
> What you propose is something different, but that will require changing of the existing
> api and verification of all csid implementations, i am ok with the proposal 
> if someone can help with the verification of other devices.

In addition using configure_stream and one api for multiple streams will make the interface
more complex. We need to extend the interface with stream id's and pass vc/dt for each stream.
I think that current enable/disable stream is more simple for this implementation.

~Gjorgji