[PATCH v3 0/5] media: qcom: camss: fixes for several cameras behind a CSI-2 bridge

[PATCH v3 0/5] media: qcom: camss: fixes for several cameras behind a CSI-2 bridge

[Hitesh Patel]

This series contains the CAMSS changes needed to run two GMSL cameras
on the RB3 Gen2 (QCS6490 / SC7280) vision mezzanine, where a MAX9296A
deserializer sits between the sensors and the SoC. The deserializer and
serializer drivers (the out-of-tree maxim-serdes work for MAX9296A and
MAX96717) and the AR0234/IMX900 sensor drivers are out of tree and not
part of this submission; only the SoC side is here.

Patches 1-3 are fixes for two RDI lines streaming on the same VFE, hit
by any configuration in which the CSID demultiplexes several virtual
channels, not only by a bridge: the VFE 17x interrupt handler drops
write master buffer done events, VFE 17x hands the second line the
wrong write master, and stopping one line resets the VFE underneath
the other.

Patch 4 addresses the assumption that the sensor is the CSI-2
transmitter: the rate the receiver has to be programmed for belongs to
whatever drives the bus, and v4l2_get_link_freq() already knows how to
ask it. Patch 5 lets one transmitter with several CSI-2 outputs link
each output to its own CSIPHY.

v2 was sent in reply to the v1 thread, which made it unclear whether it
was a series or a comment on v1 [1]. This v3 is the same series posted
as its own thread; there are no code changes since v2.

The v1 patches 6-8 (shared CSIPHY/CSID refcount and the streams API
handling in camss-video) remain dropped. Gjorgji's "add V4L2 subdev
streams API support" series [2] covers that ground properly: tested on
RB3 Gen2 with streams enabled for SC7280, both cameras stream and start
and stop independently without them.

A CCI fix found during the same bring-up, enabling SCL clock stretching,
has been sent separately to linux-i2c [3]. It is independent of this
series.

Tested on RB3 Gen2 with AR0234 and IMX900 cameras on MAX96717
serializers, on the vendor 6.18 tree and on the qualcomm-linux qcom-next
branch (v7.2 based): both cameras streaming concurrently on the two
CSI-2 ports, and each camera started and stopped repeatedly while the
other keeps streaming, without interference. The CSID test pattern
generator was exercised after patch 4 as well.

Re-checked against next-20260925: the series applies cleanly, every
patch builds on its own with W=1 for arm64 (defconfig plus
CONFIG_VIDEO_QCOM_CAMSS=m) and checkpatch --strict is clean.

Changes in v3:
- Resend as a standalone series rather than a reply to the v1 thread
  (Bryan); no code changes
- Rebased on next-20260925

Changes in v2:
- Fixes first, with Fixes: tags and "Fix" titles, commit logs rewritten
  (Bryan)
- 1/5: no PIX special case, plain removal of the IRQ_STATUS_0 gate
  (Bryan)
- 2/5: use vfe_get_output_v2() on 17x like the other gen2 VFEs instead
  of a 17x-only mapping; no PIX special case (Bryan)
- 3/5: commit log rewritten without part names and with the affected
  line spelled out (Bryan)
- 4/5: camss taken from the entity's media device instead of an extra
  argument, so the callers are unchanged; multi-line clause split
  (Bryan)
- 5/5: Reviewed-by added (Bryan)
- v1 6-8 dropped in favour of [2] (Loic, Bryan)

[1] https://lore.kernel.org/linux-media/20260915121557.20910-1-hitesh@ebytelogic.com/
[2] https://lore.kernel.org/all/20260911062213.195007-1-gjorgji.rosikopulos@oss.qualcomm.com/
[3] https://lore.kernel.org/linux-i2c/20260921131954.1690578-1-hitesh@ebytelogic.com/

Hitesh Patel (5):
  media: qcom: camss: vfe-17x: Fix write master buffer done being
    dropped
  media: qcom: camss: vfe-17x: Fix write master selection for RDI lines
  media: qcom: camss: vfe: Fix VFE reset while another line is streaming
  media: qcom: camss: Take the link frequency from the CSI-2 transmitter
  media: qcom: camss: Create the source to CSIPHY link per endpoint

 .../media/platform/qcom/camss/camss-vfe-17x.c |  46 +------
 drivers/media/platform/qcom/camss/camss-vfe.c |   8 ++
 drivers/media/platform/qcom/camss/camss.c     | 121 ++++++++++++------
 3 files changed, 93 insertions(+), 82 deletions(-)


base-commit: f5f84daefcd92d7a630066635ecea1433ed5eac7
-- 
2.43.0

[PATCH v3 1/5] media: qcom: camss: vfe-17x: Fix write master buffer done being dropped

[Hitesh Patel]

The VFE 17x interrupt handler reads and clears the bus status
registers on every interrupt, but only acts on the per write master
BUF_DONE bits when bit 9 of IRQ_STATUS_0 is set as well. Bit 9 is the
ping-pong flag of image master 1. It has nothing to do with the other
write masters, and it is not guaranteed to be set in the same
interrupt in which a write master reports a completed buffer.

The bus status is read-to-clear. When a buffer done arrives while bit
9 is not set, the handler clears the status and never calls
wm_done() for it. The buffer stays queued in the driver and the video
node never receives that frame.

With one RDI streaming this is rare. With two RDIs streaming on the
same VFE, for instance two virtual channels demultiplexed by the CSID,
the interrupt rate doubles and one of the two lines loses buffer done
events continuously.

Act on the bus status alone, as the other gen2 VFE handlers do.

Fixes: 7319cdf189bb ("media: camss: Add support for VFE hardware version Titan 170")
Signed-off-by: Hitesh Patel <hitesh@ebytelogic.com>
---
 drivers/media/platform/qcom/camss/camss-vfe-17x.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/qcom/camss/camss-vfe-17x.c b/drivers/media/platform/qcom/camss/camss-vfe-17x.c
index e5ee7e717b..c011f64f63 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe-17x.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe-17x.c
@@ -364,9 +364,8 @@ static irqreturn_t vfe_isr(int irq, void *dev)
 			vfe->isr_ops.comp_done(vfe, i);
 
 	for (wm = 0; wm < MSM_VFE_IMAGE_MASTERS_NUM; wm++)
-		if (status0 & BIT(9))
-			if (vfe_bus_status[1] & STATUS1_WM_CLIENT_BUF_DONE(wm))
-				vfe->isr_ops.wm_done(vfe, wm);
+		if (vfe_bus_status[1] & STATUS1_WM_CLIENT_BUF_DONE(wm))
+			vfe->isr_ops.wm_done(vfe, wm);
 
 	return IRQ_HANDLED;
 }
-- 
2.43.0

[PATCH v3 2/5] media: qcom: camss: vfe-17x: Fix write master selection for RDI lines

[Hitesh Patel]

On the gen2 VFE bus there is no crossbar between the RDI paths and
the bus write masters: RDI n is served by write master n. The common
vfe_get_output_v2() reflects this by mapping line->id to write master
line->id, and every gen2 VFE except 17x uses it.

The 17x code kept its own vfe_get_output(), which reserves whichever
write master is free first. That happens to be the right one as long
as a single line streams and it is RDI0. As soon as two lines of the
same VFE stream, for instance two virtual channels demultiplexed by
the CSID to RDI0 and RDI1, the second line to start is given the
write master of the other RDI. Both write masters are then programmed
with the buffer address and frame size of the wrong line. The frames
are truncated to the smaller of the two buffers and the SMMU faults
on the larger one.

Use vfe_get_output_v2() on 17x like the other gen2 VFEs and drop the
duplicate. This also removes an error path that released
output->wm_idx[0] before it had been assigned.

The gen1 VFEs are not affected: they have a bus crossbar and any write
master can serve any line, so vfe_reserve_wm() is correct there.

Fixes: 7319cdf189bb ("media: camss: Add support for VFE hardware version Titan 170")
Signed-off-by: Hitesh Patel <hitesh@ebytelogic.com>
---
 .../media/platform/qcom/camss/camss-vfe-17x.c | 41 +------------------
 1 file changed, 1 insertion(+), 40 deletions(-)

diff --git a/drivers/media/platform/qcom/camss/camss-vfe-17x.c b/drivers/media/platform/qcom/camss/camss-vfe-17x.c
index c011f64f63..62c2405359 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe-17x.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe-17x.c
@@ -382,45 +382,6 @@ static int vfe_halt(struct vfe_device *vfe)
 	return 0;
 }
 
-static int vfe_get_output(struct vfe_line *line)
-{
-	struct vfe_device *vfe = to_vfe(line);
-	struct vfe_output *output;
-	unsigned long flags;
-	int wm_idx;
-
-	spin_lock_irqsave(&vfe->output_lock, flags);
-
-	output = &line->output;
-	if (output->state > VFE_OUTPUT_RESERVED) {
-		dev_err(vfe->camss->dev, "Output is running\n");
-		goto error;
-	}
-
-	output->wm_num = 1;
-
-	wm_idx = vfe_reserve_wm(vfe, line->id);
-	if (wm_idx < 0) {
-		dev_err(vfe->camss->dev, "Can not reserve wm\n");
-		goto error_get_wm;
-	}
-	output->wm_idx[0] = wm_idx;
-
-	output->drop_update_idx = 0;
-
-	spin_unlock_irqrestore(&vfe->output_lock, flags);
-
-	return 0;
-
-error_get_wm:
-	vfe_release_wm(vfe, output->wm_idx[0]);
-	output->state = VFE_OUTPUT_OFF;
-error:
-	spin_unlock_irqrestore(&vfe->output_lock, flags);
-
-	return -EINVAL;
-}
-
 /*
  * vfe_enable - Enable streaming on VFE line
  * @line: VFE line
@@ -441,7 +402,7 @@ static int vfe_enable(struct vfe_line *line)
 
 	mutex_unlock(&vfe->stream_lock);
 
-	ret = vfe_get_output(line);
+	ret = vfe_get_output_v2(line);
 	if (ret < 0)
 		goto error_get_output;
 
-- 
2.43.0

[PATCH v3 3/5] media: qcom: camss: vfe: Fix VFE reset while another line is streaming

[Hitesh Patel]

vfe_disable_output() stops the write masters of the line being
disabled and then resets the whole VFE. The reset is not limited to
that line.

When two lines of one VFE stream at the same time, which is the case
whenever a CSID demultiplexes several virtual channels to different
RDIs, stopping the first line resets the VFE underneath the second
line. The second line's write master configuration is wiped, the
buffers it had in flight are never completed, and the VFE is left in
a state in which the next global reset is not acknowledged. Stopping
or restarting the second line then fails with:

  VFE reset timeout

Only reset the VFE when the line being disabled is the last one
streaming, which vfe->stream_count already tracks. Stopping the
line's write masters is enough to quiesce that line on its own. With
a single line streaming, the reset happens exactly as before.

Fixes: 7319cdf189bb ("media: camss: Add support for VFE hardware version Titan 170")
Signed-off-by: Hitesh Patel <hitesh@ebytelogic.com>
---
 drivers/media/platform/qcom/camss/camss-vfe.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c b/drivers/media/platform/qcom/camss/camss-vfe.c
index c14d97a131..3442dacc80 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe.c
@@ -814,6 +814,7 @@ static int vfe_disable_output(struct vfe_line *line)
 	struct vfe_output *output = &line->output;
 	unsigned long flags;
 	unsigned int i;
+	bool last;
 
 	spin_lock_irqsave(&vfe->output_lock, flags);
 	for (i = 0; i < output->wm_num; i++)
@@ -821,6 +822,13 @@ static int vfe_disable_output(struct vfe_line *line)
 	output->gen2.active_num = 0;
 	spin_unlock_irqrestore(&vfe->output_lock, flags);
 
+	mutex_lock(&vfe->stream_lock);
+	last = vfe->stream_count == 1;
+	mutex_unlock(&vfe->stream_lock);
+
+	if (!last)
+		return 0;
+
 	return vfe_reset(vfe);
 }
 
-- 
2.43.0

[PATCH v3 4/5] media: qcom: camss: Take the link frequency from the CSI-2 transmitter

[Hitesh Patel]

camss_get_link_freq() walks the pipeline up to an entity with the
MEDIA_ENT_F_CAM_SENSOR function and reads the link frequency there.
The CSIPHY settle count and the CSID clock are then derived from it.

The frequency the receiver has to be programmed for is the one on the
CSI-2 bus, which belongs to whatever drives that bus. When the sensor
is wired straight to the CSIPHY that is the sensor, and the walk gives
the right answer. When a CSI-2 to CSI-2 bridge sits in between, such
as a GMSL or FPD-Link deserializer, the bridge re-times the data onto
its own output at its own rate: it may aggregate several sensors onto
one link, forward one sensor at a different rate, or generate a test
pattern with no sensor at all. The sensor's rate is then not what
arrives at the SoC, and the CSIPHY does not lock.

The walk can also fail before reaching a sensor. A deserializer has
one sink pad per serial link and the walk always follows pad 0; a
sensor attached to any other link is never found and streaming is
refused with "Cannot get CSI2 transmitter's link frequency".

Stop the walk at the first entity that is not a CAMSS receiver, i.e.
at the external subdev feeding the CSIPHY, and ask that pad with
v4l2_get_link_freq(). The helper queries the transmitter through
.get_mbus_config first and falls back to its V4L2_CID_LINK_FREQ and
V4L2_CID_PIXEL_RATE controls, so a bridge and a bare sensor are both
handled by the standard mechanism.

For a sensor connected directly to a CSIPHY the transmitter is the
sensor, so the pad found and the value returned do not change.
camss_find_sensor_pad() is kept for camss_get_pixel_clock() and the
frame skip query, which do want the sensor.

Signed-off-by: Hitesh Patel <hitesh@ebytelogic.com>
---
 drivers/media/platform/qcom/camss/camss.c | 70 +++++++++++++++++++++--
 1 file changed, 64 insertions(+), 6 deletions(-)

diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
index 23f3cc30a1..327ac7b280 100644
--- a/drivers/media/platform/qcom/camss/camss.c
+++ b/drivers/media/platform/qcom/camss/camss.c
@@ -4619,24 +4619,82 @@ struct media_pad *camss_find_sensor_pad(struct media_entity *entity)
 	}
 }
 
+/*
+ * camss_is_receiver_subdev - Test whether a subdev is a CAMSS CSI-2 receiver
+ * @camss: CAMSS device
+ * @sd: Subdevice to test
+ *
+ * Return true for a CSIPHY or CSID belonging to @camss, false for anything
+ * else, in particular for the external subdev transmitting to them.
+ */
+static bool camss_is_receiver_subdev(struct camss *camss,
+				     struct v4l2_subdev *sd)
+{
+	unsigned int i;
+
+	for (i = 0; i < camss->res->csiphy_num; i++)
+		if (sd == &camss->csiphy[i].subdev)
+			return true;
+
+	for (i = 0; i < camss->res->csid_num; i++)
+		if (sd == &camss->csid[i].subdev)
+			return true;
+
+	return false;
+}
+
+/*
+ * camss_find_transmitter_pad - Find the pad of the CSI-2 transmitter
+ * @entity: Media entity in the current pipeline
+ *
+ * Walk the pipeline upstream through the CAMSS receiver subdevs and return the
+ * source pad of the first entity that is not one of them: the CSI-2
+ * transmitter driving the SoC.
+ *
+ * Return a pointer to the transmitter media pad or NULL if not found
+ */
+static struct media_pad *camss_find_transmitter_pad(struct media_entity *entity)
+{
+	struct camss *camss = container_of(entity->graph_obj.mdev,
+					   struct camss, media_dev);
+	struct v4l2_subdev *sd;
+	struct media_pad *pad;
+
+	while (1) {
+		pad = &entity->pads[0];
+		if (!(pad->flags & MEDIA_PAD_FL_SINK))
+			return NULL;
+
+		pad = media_pad_remote_pad_first(pad);
+		if (!pad || !is_media_entity_v4l2_subdev(pad->entity))
+			return NULL;
+
+		entity = pad->entity;
+		sd = media_entity_to_v4l2_subdev(entity);
+
+		if (!camss_is_receiver_subdev(camss, sd))
+			return pad;
+	}
+}
+
 /**
- * camss_get_link_freq - Get link frequency from sensor
+ * camss_get_link_freq - Get link frequency from the CSI-2 transmitter
  * @entity: Media entity in the current pipeline
  * @bpp: Number of bits per pixel for the current format
- * @lanes: Number of lanes in the link to the sensor
+ * @lanes: Number of lanes in the link to the transmitter
  *
  * Return link frequency on success or a negative error code otherwise
  */
 s64 camss_get_link_freq(struct media_entity *entity, unsigned int bpp,
 			unsigned int lanes)
 {
-	struct media_pad *sensor_pad;
+	struct media_pad *tx_pad;
 
-	sensor_pad = camss_find_sensor_pad(entity);
-	if (!sensor_pad)
+	tx_pad = camss_find_transmitter_pad(entity);
+	if (!tx_pad)
 		return -ENODEV;
 
-	return v4l2_get_link_freq(sensor_pad, bpp, 2 * lanes);
+	return v4l2_get_link_freq(tx_pad, bpp, 2 * lanes);
 }
 
 /*
-- 
2.43.0

[PATCH v3 5/5] media: qcom: camss: Create the source to CSIPHY link per endpoint

[Hitesh Patel]

The link from the external CSI-2 transmitter to the CSIPHY is created
in the notifier .complete() callback by walking every registered
subdev, reading the CSIPHY it was bound to from sd->host_priv and
linking the subdev's first source pad to that CSIPHY.

This assumes one transmitter feeds exactly one CSIPHY. A GMSL
deserializer such as the MAX9296A has two independent CSI-2 output
ports which, on the RB3 Gen2 vision mezzanine, are wired to two
different SoC CSIPHYs. The same subdev is then bound once per CAMSS
port endpoint, the second .bound() overwrites host_priv, and
.complete() creates a single link from source pad 0 to the last
CSIPHY. The second output port is left with no link at all, so a
second camera can never be routed to the SoC.

Move the link creation into .bound(), where both the endpoint and
the CSIPHY are known, and resolve the transmitter's source pad from
the endpoint fwnode with media_entity_get_fwnode_pad(). Each
endpoint then gets its own link between the right source pad and
the right CSIPHY.

For a subdev that does not implement .get_fwnode_pad,
media_entity_get_fwnode_pad() falls back to the first pad matching
the requested direction, which is exactly what the .complete() loop
did, so ordinary single-output sensors keep the same link as before.

Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Hitesh Patel <hitesh@ebytelogic.com>
---
 drivers/media/platform/qcom/camss/camss.c | 51 ++++++++---------------
 1 file changed, 18 insertions(+), 33 deletions(-)

diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
index 327ac7b280..cd9b66f265 100644
--- a/drivers/media/platform/qcom/camss/camss.c
+++ b/drivers/media/platform/qcom/camss/camss.c
@@ -5224,49 +5224,34 @@ static int camss_subdev_notifier_bound(struct v4l2_async_notifier *async,
 		container_of(asd, struct camss_async_subdev, asd);
 	u8 id = csd->interface.csiphy_id;
 	struct csiphy_device *csiphy = &camss->csiphy[id];
+	struct media_entity *input = &csiphy->subdev.entity;
+	struct media_entity *sensor = &subdev->entity;
+	int pad, ret;
 
 	csiphy->cfg.csi2 = &csd->interface.csi2;
 	subdev->host_priv = csiphy;
 
+	pad = media_entity_get_fwnode_pad(sensor, asd->match.fwnode,
+					  MEDIA_PAD_FL_SOURCE);
+	if (pad < 0) {
+		dev_err(camss->dev, "No source pad in external entity %s: %d\n",
+			sensor->name, pad);
+		return pad;
+	}
+
+	ret = media_create_pad_link(sensor, pad, input, MSM_CSIPHY_PAD_SINK,
+				    MEDIA_LNK_FL_IMMUTABLE | MEDIA_LNK_FL_ENABLED);
+	if (ret < 0) {
+		camss_link_err(camss, sensor->name, input->name, ret);
+		return ret;
+	}
+
 	return 0;
 }
 
 static int camss_subdev_notifier_complete(struct v4l2_async_notifier *async)
 {
 	struct camss *camss = container_of(async, struct camss, notifier);
-	struct v4l2_device *v4l2_dev = &camss->v4l2_dev;
-	struct v4l2_subdev *sd;
-
-	list_for_each_entry(sd, &v4l2_dev->subdevs, list) {
-		struct csiphy_device *csiphy = sd->host_priv;
-		struct media_entity *input, *sensor;
-		unsigned int i;
-		int ret;
-
-		if (!csiphy)
-			continue;
-
-		input = &csiphy->subdev.entity;
-		sensor = &sd->entity;
-
-		for (i = 0; i < sensor->num_pads; i++) {
-			if (sensor->pads[i].flags & MEDIA_PAD_FL_SOURCE)
-				break;
-		}
-		if (i == sensor->num_pads) {
-			dev_err(camss->dev,
-				"No source pad in external entity\n");
-			return -EINVAL;
-		}
-
-		ret = media_create_pad_link(sensor, i, input,
-					    MSM_CSIPHY_PAD_SINK,
-					    MEDIA_LNK_FL_IMMUTABLE | MEDIA_LNK_FL_ENABLED);
-		if (ret < 0) {
-			camss_link_err(camss, sensor->name, input->name, ret);
-			return ret;
-		}
-	}
 
 	return v4l2_device_register_subdev_nodes(&camss->v4l2_dev);
 }
-- 
2.43.0

Re: [PATCH v3 1/5] media: qcom: camss: vfe-17x: Fix write master buffer done being dropped

[Loic Poulain]

On Mon, Sep 28, 2026 at 8:43 AM Hitesh Patel <hitesh@ebytelogic.com> wrote:
>
> The VFE 17x interrupt handler reads and clears the bus status
> registers on every interrupt, but only acts on the per write master
> BUF_DONE bits when bit 9 of IRQ_STATUS_0 is set as well. Bit 9 is the
> ping-pong flag of image master 1. It has nothing to do with the other
> write masters, and it is not guaranteed to be set in the same
> interrupt in which a write master reports a completed buffer.
>
> The bus status is read-to-clear. When a buffer done arrives while bit
> 9 is not set, the handler clears the status and never calls
> wm_done() for it. The buffer stays queued in the driver and the video
> node never receives that frame.
>
> With one RDI streaming this is rare. With two RDIs streaming on the
> same VFE, for instance two virtual channels demultiplexed by the CSID,
> the interrupt rate doubles and one of the two lines loses buffer done
> events continuously.
>
> Act on the bus status alone, as the other gen2 VFE handlers do.
>
> Fixes: 7319cdf189bb ("media: camss: Add support for VFE hardware version Titan 170")
> Signed-off-by: Hitesh Patel <hitesh@ebytelogic.com>

Reviewed-by: Loic Poulain <loic.poulain@oss.qualcomm.com>

> ---
>  drivers/media/platform/qcom/camss/camss-vfe-17x.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/camss/camss-vfe-17x.c b/drivers/media/platform/qcom/camss/camss-vfe-17x.c
> index e5ee7e717b..c011f64f63 100644
> --- a/drivers/media/platform/qcom/camss/camss-vfe-17x.c
> +++ b/drivers/media/platform/qcom/camss/camss-vfe-17x.c
> @@ -364,9 +364,8 @@ static irqreturn_t vfe_isr(int irq, void *dev)
>                         vfe->isr_ops.comp_done(vfe, i);
>
>         for (wm = 0; wm < MSM_VFE_IMAGE_MASTERS_NUM; wm++)
> -               if (status0 & BIT(9))
> -                       if (vfe_bus_status[1] & STATUS1_WM_CLIENT_BUF_DONE(wm))
> -                               vfe->isr_ops.wm_done(vfe, wm);
> +               if (vfe_bus_status[1] & STATUS1_WM_CLIENT_BUF_DONE(wm))
> +                       vfe->isr_ops.wm_done(vfe, wm);
>
>         return IRQ_HANDLED;
>  }
> --
> 2.43.0
>

Re: [PATCH v3 2/5] media: qcom: camss: vfe-17x: Fix write master selection for RDI lines

[Loic Poulain]

On Mon, Sep 28, 2026 at 8:43 AM Hitesh Patel <hitesh@ebytelogic.com> wrote:
>
> On the gen2 VFE bus there is no crossbar between the RDI paths and
> the bus write masters: RDI n is served by write master n. The common
> vfe_get_output_v2() reflects this by mapping line->id to write master
> line->id, and every gen2 VFE except 17x uses it.
>
> The 17x code kept its own vfe_get_output(), which reserves whichever
> write master is free first. That happens to be the right one as long
> as a single line streams and it is RDI0. As soon as two lines of the
> same VFE stream, for instance two virtual channels demultiplexed by
> the CSID to RDI0 and RDI1, the second line to start is given the
> write master of the other RDI. Both write masters are then programmed
> with the buffer address and frame size of the wrong line. The frames
> are truncated to the smaller of the two buffers and the SMMU faults
> on the larger one.
>
> Use vfe_get_output_v2() on 17x like the other gen2 VFEs and drop the
> duplicate. This also removes an error path that released
> output->wm_idx[0] before it had been assigned.
>
> The gen1 VFEs are not affected: they have a bus crossbar and any write
> master can serve any line, so vfe_reserve_wm() is correct there.
>
> Fixes: 7319cdf189bb ("media: camss: Add support for VFE hardware version Titan 170")
> Signed-off-by: Hitesh Patel <hitesh@ebytelogic.com>

Reviewed-by: Loic Poulain <loic.poulain@oss.qualcomm.com>

> ---
>  .../media/platform/qcom/camss/camss-vfe-17x.c | 41 +------------------
>  1 file changed, 1 insertion(+), 40 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/camss/camss-vfe-17x.c b/drivers/media/platform/qcom/camss/camss-vfe-17x.c
> index c011f64f63..62c2405359 100644
> --- a/drivers/media/platform/qcom/camss/camss-vfe-17x.c
> +++ b/drivers/media/platform/qcom/camss/camss-vfe-17x.c
> @@ -382,45 +382,6 @@ static int vfe_halt(struct vfe_device *vfe)
>         return 0;
>  }
>
> -static int vfe_get_output(struct vfe_line *line)
> -{
> -       struct vfe_device *vfe = to_vfe(line);
> -       struct vfe_output *output;
> -       unsigned long flags;
> -       int wm_idx;
> -
> -       spin_lock_irqsave(&vfe->output_lock, flags);
> -
> -       output = &line->output;
> -       if (output->state > VFE_OUTPUT_RESERVED) {
> -               dev_err(vfe->camss->dev, "Output is running\n");
> -               goto error;
> -       }
> -
> -       output->wm_num = 1;
> -
> -       wm_idx = vfe_reserve_wm(vfe, line->id);
> -       if (wm_idx < 0) {
> -               dev_err(vfe->camss->dev, "Can not reserve wm\n");
> -               goto error_get_wm;
> -       }
> -       output->wm_idx[0] = wm_idx;
> -
> -       output->drop_update_idx = 0;
> -
> -       spin_unlock_irqrestore(&vfe->output_lock, flags);
> -
> -       return 0;
> -
> -error_get_wm:
> -       vfe_release_wm(vfe, output->wm_idx[0]);
> -       output->state = VFE_OUTPUT_OFF;
> -error:
> -       spin_unlock_irqrestore(&vfe->output_lock, flags);
> -
> -       return -EINVAL;
> -}
> -
>  /*
>   * vfe_enable - Enable streaming on VFE line
>   * @line: VFE line
> @@ -441,7 +402,7 @@ static int vfe_enable(struct vfe_line *line)
>
>         mutex_unlock(&vfe->stream_lock);
>
> -       ret = vfe_get_output(line);
> +       ret = vfe_get_output_v2(line);
>         if (ret < 0)
>                 goto error_get_output;
>
> --
> 2.43.0
>

Re: [PATCH v3 3/5] media: qcom: camss: vfe: Fix VFE reset while another line is streaming

[Loic Poulain]

On Mon, Sep 28, 2026 at 8:43 AM Hitesh Patel <hitesh@ebytelogic.com> wrote:
>
> vfe_disable_output() stops the write masters of the line being
> disabled and then resets the whole VFE. The reset is not limited to
> that line.
>
> When two lines of one VFE stream at the same time, which is the case
> whenever a CSID demultiplexes several virtual channels to different
> RDIs, stopping the first line resets the VFE underneath the second
> line. The second line's write master configuration is wiped, the
> buffers it had in flight are never completed, and the VFE is left in
> a state in which the next global reset is not acknowledged. Stopping
> or restarting the second line then fails with:
>
>   VFE reset timeout
>
> Only reset the VFE when the line being disabled is the last one
> streaming, which vfe->stream_count already tracks. Stopping the
> line's write masters is enough to quiesce that line on its own. With
> a single line streaming, the reset happens exactly as before.
>
> Fixes: 7319cdf189bb ("media: camss: Add support for VFE hardware version Titan 170")
> Signed-off-by: Hitesh Patel <hitesh@ebytelogic.com>
> ---
>  drivers/media/platform/qcom/camss/camss-vfe.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c b/drivers/media/platform/qcom/camss/camss-vfe.c
> index c14d97a131..3442dacc80 100644
> --- a/drivers/media/platform/qcom/camss/camss-vfe.c
> +++ b/drivers/media/platform/qcom/camss/camss-vfe.c
> @@ -814,6 +814,7 @@ static int vfe_disable_output(struct vfe_line *line)
>         struct vfe_output *output = &line->output;
>         unsigned long flags;
>         unsigned int i;
> +       bool last;
>
>         spin_lock_irqsave(&vfe->output_lock, flags);
>         for (i = 0; i < output->wm_num; i++)
> @@ -821,6 +822,13 @@ static int vfe_disable_output(struct vfe_line *line)
>         output->gen2.active_num = 0;
>         spin_unlock_irqrestore(&vfe->output_lock, flags);
>
> +       mutex_lock(&vfe->stream_lock);
> +       last = vfe->stream_count == 1;
> +       mutex_unlock(&vfe->stream_lock);
> +
> +       if (!last)
> +               return 0;

Instead I would move the vfe_reset() into vfe_disable().

> +
>         return vfe_reset(vfe);
>  }
>
> --
> 2.43.0
>

Re: [PATCH v3 4/5] media: qcom: camss: Take the link frequency from the CSI-2 transmitter

[Loic Poulain]

On Mon, Sep 28, 2026 at 8:44 AM Hitesh Patel <hitesh@ebytelogic.com> wrote:
>
> camss_get_link_freq() walks the pipeline up to an entity with the
> MEDIA_ENT_F_CAM_SENSOR function and reads the link frequency there.
> The CSIPHY settle count and the CSID clock are then derived from it.
>
> The frequency the receiver has to be programmed for is the one on the
> CSI-2 bus, which belongs to whatever drives that bus. When the sensor
> is wired straight to the CSIPHY that is the sensor, and the walk gives
> the right answer. When a CSI-2 to CSI-2 bridge sits in between, such
> as a GMSL or FPD-Link deserializer, the bridge re-times the data onto
> its own output at its own rate: it may aggregate several sensors onto
> one link, forward one sensor at a different rate, or generate a test
> pattern with no sensor at all. The sensor's rate is then not what
> arrives at the SoC, and the CSIPHY does not lock.
>
> The walk can also fail before reaching a sensor. A deserializer has
> one sink pad per serial link and the walk always follows pad 0; a
> sensor attached to any other link is never found and streaming is
> refused with "Cannot get CSI2 transmitter's link frequency".
>
> Stop the walk at the first entity that is not a CAMSS receiver, i.e.
> at the external subdev feeding the CSIPHY, and ask that pad with
> v4l2_get_link_freq(). The helper queries the transmitter through
> .get_mbus_config first and falls back to its V4L2_CID_LINK_FREQ and
> V4L2_CID_PIXEL_RATE controls, so a bridge and a bare sensor are both
> handled by the standard mechanism.
>
> For a sensor connected directly to a CSIPHY the transmitter is the
> sensor, so the pad found and the value returned do not change.
> camss_find_sensor_pad() is kept for camss_get_pixel_clock() and the
> frame skip query, which do want the sensor.
>
> Signed-off-by: Hitesh Patel <hitesh@ebytelogic.com>
> ---
>  drivers/media/platform/qcom/camss/camss.c | 70 +++++++++++++++++++++--
>  1 file changed, 64 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
> index 23f3cc30a1..327ac7b280 100644
> --- a/drivers/media/platform/qcom/camss/camss.c
> +++ b/drivers/media/platform/qcom/camss/camss.c
> @@ -4619,24 +4619,82 @@ struct media_pad *camss_find_sensor_pad(struct media_entity *entity)
>         }
>  }
>
> +/*
> + * camss_is_receiver_subdev - Test whether a subdev is a CAMSS CSI-2 receiver
> + * @camss: CAMSS device
> + * @sd: Subdevice to test
> + *
> + * Return true for a CSIPHY or CSID belonging to @camss, false for anything
> + * else, in particular for the external subdev transmitting to them.
> + */
> +static bool camss_is_receiver_subdev(struct camss *camss,
> +                                    struct v4l2_subdev *sd)
> +{
> +       unsigned int i;
> +
> +       for (i = 0; i < camss->res->csiphy_num; i++)
> +               if (sd == &camss->csiphy[i].subdev)
> +                       return true;
> +
> +       for (i = 0; i < camss->res->csid_num; i++)
> +               if (sd == &camss->csid[i].subdev)
> +                       return true;
> +
> +       return false;
> +}
> +
> +/*
> + * camss_find_transmitter_pad - Find the pad of the CSI-2 transmitter
> + * @entity: Media entity in the current pipeline
> + *
> + * Walk the pipeline upstream through the CAMSS receiver subdevs and return the
> + * source pad of the first entity that is not one of them: the CSI-2
> + * transmitter driving the SoC.
> + *
> + * Return a pointer to the transmitter media pad or NULL if not found
> + */
> +static struct media_pad *camss_find_transmitter_pad(struct media_entity *entity)
> +{
> +       struct camss *camss = container_of(entity->graph_obj.mdev,
> +                                          struct camss, media_dev);
> +       struct v4l2_subdev *sd;
> +       struct media_pad *pad;
> +
> +       while (1) {
> +               pad = &entity->pads[0];
> +               if (!(pad->flags & MEDIA_PAD_FL_SINK))
> +                       return NULL;
> +
> +               pad = media_pad_remote_pad_first(pad);
> +               if (!pad || !is_media_entity_v4l2_subdev(pad->entity))
> +                       return NULL;
> +
> +               entity = pad->entity;
> +               sd = media_entity_to_v4l2_subdev(entity);
> +
> +               if (!camss_is_receiver_subdev(camss, sd))
> +                       return pad;
> +       }
> +}
> +
>  /**
> - * camss_get_link_freq - Get link frequency from sensor
> + * camss_get_link_freq - Get link frequency from the CSI-2 transmitter
>   * @entity: Media entity in the current pipeline
>   * @bpp: Number of bits per pixel for the current format
> - * @lanes: Number of lanes in the link to the sensor
> + * @lanes: Number of lanes in the link to the transmitter
>   *
>   * Return link frequency on success or a negative error code otherwise
>   */
>  s64 camss_get_link_freq(struct media_entity *entity, unsigned int bpp,
>                         unsigned int lanes)
>  {
> -       struct media_pad *sensor_pad;
> +       struct media_pad *tx_pad;
>
> -       sensor_pad = camss_find_sensor_pad(entity);
> -       if (!sensor_pad)
> +       tx_pad = camss_find_transmitter_pad(entity);
> +       if (!tx_pad)
>                 return -ENODEV;
>
> -       return v4l2_get_link_freq(sensor_pad, bpp, 2 * lanes);
> +       return v4l2_get_link_freq(tx_pad, bpp, 2 * lanes);
>  }
>
>  /*
> --

I think we could even simply replacing/refactoring
camss_find_sensor_pad() with camss_find_transmitter_pad()?

Re: [PATCH v3 1/5] media: qcom: camss: vfe-17x: Fix write master buffer done being dropped

[Bryan O'Donoghue]

On 28/09/2026 07:43, Hitesh Patel wrote:
> The VFE 17x interrupt handler reads and clears the bus status
> registers on every interrupt, but only acts on the per write master
> BUF_DONE bits when bit 9 of IRQ_STATUS_0 is set as well. Bit 9 is the
> ping-pong flag of image master 1. It has nothing to do with the other
> write masters, and it is not guaranteed to be set in the same
> interrupt in which a write master reports a completed buffer.
> 
> The bus status is read-to-clear. When a buffer done arrives while bit
> 9 is not set, the handler clears the status and never calls
> wm_done() for it. The buffer stays queued in the driver and the video
> node never receives that frame.
> 
> With one RDI streaming this is rare. With two RDIs streaming on the
> same VFE, for instance two virtual channels demultiplexed by the CSID,
> the interrupt rate doubles and one of the two lines loses buffer done
> events continuously.
> 
> Act on the bus status alone, as the other gen2 VFE handlers do.
> 
> Fixes: 7319cdf189bb ("media: camss: Add support for VFE hardware version Titan 170")
> Signed-off-by: Hitesh Patel <hitesh@ebytelogic.com>
> ---
>   drivers/media/platform/qcom/camss/camss-vfe-17x.c | 5 ++---
>   1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/camss/camss-vfe-17x.c b/drivers/media/platform/qcom/camss/camss-vfe-17x.c
> index e5ee7e717b..c011f64f63 100644
> --- a/drivers/media/platform/qcom/camss/camss-vfe-17x.c
> +++ b/drivers/media/platform/qcom/camss/camss-vfe-17x.c
> @@ -364,9 +364,8 @@ static irqreturn_t vfe_isr(int irq, void *dev)
>   			vfe->isr_ops.comp_done(vfe, i);
> 
>   	for (wm = 0; wm < MSM_VFE_IMAGE_MASTERS_NUM; wm++)
> -		if (status0 & BIT(9))
> -			if (vfe_bus_status[1] & STATUS1_WM_CLIENT_BUF_DONE(wm))
> -				vfe->isr_ops.wm_done(vfe, wm);
> +		if (vfe_bus_status[1] & STATUS1_WM_CLIENT_BUF_DONE(wm))
> +			vfe->isr_ops.wm_done(vfe, wm);
> 
>   	return IRQ_HANDLED;
>   }
> --
> 2.43.0
> 
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

Re: [PATCH v3 2/5] media: qcom: camss: vfe-17x: Fix write master selection for RDI lines

[Bryan O'Donoghue]

On 28/09/2026 07:43, Hitesh Patel wrote:
> On the gen2 VFE bus there is no crossbar between the RDI paths and
> the bus write masters: RDI n is served by write master n. The common
> vfe_get_output_v2() reflects this by mapping line->id to write master
> line->id, and every gen2 VFE except 17x uses it.
> 
> The 17x code kept its own vfe_get_output(), which reserves whichever
> write master is free first. That happens to be the right one as long
> as a single line streams and it is RDI0. As soon as two lines of the
> same VFE stream, for instance two virtual channels demultiplexed by
> the CSID to RDI0 and RDI1, the second line to start is given the
> write master of the other RDI. Both write masters are then programmed
> with the buffer address and frame size of the wrong line. The frames
> are truncated to the smaller of the two buffers and the SMMU faults
> on the larger one.
> 
> Use vfe_get_output_v2() on 17x like the other gen2 VFEs and drop the
> duplicate. This also removes an error path that released
> output->wm_idx[0] before it had been assigned.
> 
> The gen1 VFEs are not affected: they have a bus crossbar and any write
> master can serve any line, so vfe_reserve_wm() is correct there.
> 
> Fixes: 7319cdf189bb ("media: camss: Add support for VFE hardware version Titan 170")
> Signed-off-by: Hitesh Patel <hitesh@ebytelogic.com>
> ---
>   .../media/platform/qcom/camss/camss-vfe-17x.c | 41 +------------------
>   1 file changed, 1 insertion(+), 40 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/camss/camss-vfe-17x.c b/drivers/media/platform/qcom/camss/camss-vfe-17x.c
> index c011f64f63..62c2405359 100644
> --- a/drivers/media/platform/qcom/camss/camss-vfe-17x.c
> +++ b/drivers/media/platform/qcom/camss/camss-vfe-17x.c
> @@ -382,45 +382,6 @@ static int vfe_halt(struct vfe_device *vfe)
>   	return 0;
>   }
> 
> -static int vfe_get_output(struct vfe_line *line)
> -{
> -	struct vfe_device *vfe = to_vfe(line);
> -	struct vfe_output *output;
> -	unsigned long flags;
> -	int wm_idx;
> -
> -	spin_lock_irqsave(&vfe->output_lock, flags);
> -
> -	output = &line->output;
> -	if (output->state > VFE_OUTPUT_RESERVED) {
> -		dev_err(vfe->camss->dev, "Output is running\n");
> -		goto error;
> -	}
> -
> -	output->wm_num = 1;
> -
> -	wm_idx = vfe_reserve_wm(vfe, line->id);
> -	if (wm_idx < 0) {
> -		dev_err(vfe->camss->dev, "Can not reserve wm\n");
> -		goto error_get_wm;
> -	}
> -	output->wm_idx[0] = wm_idx;
> -
> -	output->drop_update_idx = 0;
> -
> -	spin_unlock_irqrestore(&vfe->output_lock, flags);
> -
> -	return 0;
> -
> -error_get_wm:
> -	vfe_release_wm(vfe, output->wm_idx[0]);
> -	output->state = VFE_OUTPUT_OFF;
> -error:
> -	spin_unlock_irqrestore(&vfe->output_lock, flags);
> -
> -	return -EINVAL;
> -}
> -
>   /*
>    * vfe_enable - Enable streaming on VFE line
>    * @line: VFE line
> @@ -441,7 +402,7 @@ static int vfe_enable(struct vfe_line *line)
> 
>   	mutex_unlock(&vfe->stream_lock);
> 
> -	ret = vfe_get_output(line);
> +	ret = vfe_get_output_v2(line);
>   	if (ret < 0)
>   		goto error_get_output;
> 
> --
> 2.43.0
> 
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

Re: [PATCH v3 3/5] media: qcom: camss: vfe: Fix VFE reset while another line is streaming

[Bryan O'Donoghue]

On 28/09/2026 07:43, Hitesh Patel wrote:
> @@ -814,6 +814,7 @@ static int vfe_disable_output(struct vfe_line *line)
>   	struct vfe_output *output = &line->output;
>   	unsigned long flags;
>   	unsigned int i;
> +	bool last;
> 
>   	spin_lock_irqsave(&vfe->output_lock, flags);
>   	for (i = 0; i < output->wm_num; i++)
> @@ -821,6 +822,13 @@ static int vfe_disable_output(struct vfe_line *line)
>   	output->gen2.active_num = 0;
>   	spin_unlock_irqrestore(&vfe->output_lock, flags);
> 
> +	mutex_lock(&vfe->stream_lock);
> +	last = vfe->stream_count == 1;
> +	mutex_unlock(&vfe->stream_lock);
> +
> +	if (!last)
> +		return 0;
> +

NAK.

This can't be correct, in fact its very racy and will break. Here is the 
existing code.

static int vfe_disable_output(struct vfe_line *line)
{
         struct vfe_device *vfe = to_vfe(line);
         struct vfe_output *output = &line->output;
         unsigned long flags;
         unsigned int i;

         spin_lock_irqsave(&vfe->output_lock, flags);
         for (i = 0; i < output->wm_num; i++)
                 vfe->res->hw_ops->vfe_wm_stop(vfe, output->wm_idx[i]);
         output->gen2.active_num = 0;
         spin_unlock_irqrestore(&vfe->output_lock, flags);

         return vfe_reset(vfe);
}

/*
  * vfe_disable - Disable streaming on VFE line
  * @line: VFE line
  *
  * Return 0 on success or a negative error code otherwise
  */
int vfe_disable(struct vfe_line *line)
{
         struct vfe_device *vfe = to_vfe(line);
         int ret;

         ret = vfe_disable_output(line);
         if (ret)
                 goto error;

         vfe_put_output(line);

         mutex_lock(&vfe->stream_lock);

         vfe->stream_count--;

         mutex_unlock(&vfe->stream_lock);

error:
         return ret;
}

Here's how

stream_count = 2;

thread0:
   vfe_disable()
	vfe_disable_output()
	mutex_lock();
	last = stream_count == 1; //false
	mutex_unlock();
	return;

thread1:
   vfe_disable()
	vfe_disable_output()
	mutex_lock();
	last = stream_count == 1; //false
	mutex_unlock();
	return;

thread0:
   vfe_disable()
	mutex_lock();
	stream_count--; // => stream_count = 1;
	mutex_unlock();

thread1:
   vfe_disable()
	mutex_lock();
	stream_count--; // => stream_count = 0;
	mutex_unlock();

vfe_reset() is never called.

this->

diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c 
b/drivers/media/platform/qcom/camss/camss-vfe.c
index 34e4319d8f80c..3dafd5d778630 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe.c
@@ -828,7 +828,7 @@ static int vfe_disable_output(struct vfe_line *line)
         output->gen2.active_num = 0;
         spin_unlock_irqrestore(&vfe->output_lock, flags);

-       return vfe_reset(vfe);
+       return 0;
  }

  /*
@@ -850,7 +850,8 @@ int vfe_disable(struct vfe_line *line)

         mutex_lock(&vfe->stream_lock);

-       vfe->stream_count--;
+       if (--vfe->stream_count == 0)
+               ret = vfe_reset(vfe);

         mutex_unlock(&vfe->stream_lock);

---
bod

Re: [PATCH v3 3/5] media: qcom: camss: vfe: Fix VFE reset while another line is streaming

[Hitesh Patel]

On 28/09/2026 10:49, Bryan O'Donoghue wrote:
> NAK.
>
> This can't be correct, in fact its very racy and will break.

You are right, thanks for spelling out the interleaving. Sampling
stream_count before the decrement, and outside the lock that protects
it, lets both callers conclude they are not the last one and the reset
is then never done.

I have taken your diff as is for v4: vfe_disable_output() only stops the
write masters, and vfe_disable() does

	if (--vfe->stream_count == 0)
		ret = vfe_reset(vfe);

inside stream_lock. Keeping the reset under the lock also closes a
second window I had left open, since vfe_enable_v2() increments
stream_count under the same lock and then configures the output after
releasing it, so a line could have started coming up while the reset
ran.

Retested on the RB3 Gen2 with both cameras aggregated as VC0 and VC1 on
one CSIPHY, so that CSID0 feeds RDI0 and RDI1 of the same VFE: the two
stream concurrently (60/60 frames each) and VC1 restarts five times out
of five while VC0 keeps streaming, 200 frames without a gap and no
"VFE reset timeout".

v4 sent as a new thread.

Thanks,
Hitesh