Re: [PATCH v21 04/14] media: qcom: camss: Issue stream-on REG_UPDATE from CSID
From: Bryan O'Donoghue
Date: Wed Oct 21 2026 - 04:57:14 EST
On 21/10/2026 08:37, Wenmeng Liu wrote:
Hi Bryan, Hangxiang,
On 10/12/2026 2:45 PM, Hangxiang Ma wrote:
On stream-on, CAMSS starts the subdevices upstream from the video node
through their sink pads, so the VFE is started before the CSID.
Starting with CSID 680, the REG_UPDATE is issued through the CSID
rather than the VFE. However, vfe_enable_output_v2() still issues a
REG_UPDATE after queueing each initial buffer, before the CSID has
configured the stream. The VFE then never signals BUF_DONE. Therefore,
the CAMSS now issues the REG_UPDATE from configure_stream() on CSID 680
and CSID Gen3 instead. The operation happens after the RX configuration
and before the RDI path is enabled.
Skip the VFE-side stream-on REG_UPDATE only when the CSID connected to
the VFE sink pad implements the reg_update callback. Platforms whose
CSID does not provide this callback, as well as lines fed by ISPIF,
keep the existing VFE-side REG_UPDATE.
Signed-off-by: Hangxiang Ma <hangxiang.ma@xxxxxxxxxxxxxxxx>
---
drivers/media/platform/qcom/camss/camss-csid-680.c | 3 ++
.../media/platform/qcom/camss/camss-csid-gen3.c | 3 ++
drivers/media/platform/qcom/camss/camss-vfe.c | 37 ++++++++++++ +++++++++-
3 files changed, 42 insertions(+), 1 deletion(-)
diff --git a/drivers/media/platform/qcom/camss/camss-csid-680.c b/ drivers/media/platform/qcom/camss/camss-csid-680.c
index 345a67c8fb94..556cdfef6207 100644
--- a/drivers/media/platform/qcom/camss/camss-csid-680.c
+++ b/drivers/media/platform/qcom/camss/camss-csid-680.c
@@ -296,6 +296,8 @@ static void __csid_configure_rdi_stream(struct csid_device *csid, u8 enable, u8
writel(val, csid->base + CSID_RDI_CFG0(port));
}
+static void csid_subdev_reg_update(struct csid_device *csid, int port_id, bool is_clear);
+
static void csid_configure_stream(struct csid_device *csid, u8 enable)
{
int i;
@@ -307,6 +309,7 @@ static void csid_configure_stream(struct csid_device *csid, u8 enable)
if (csid->phy.en_vc & BIT(i)) {
__csid_configure_rdi_stream(csid, enable, i, 0);
__csid_configure_rx(csid, &csid->phy, 0);
+ csid_subdev_reg_update(csid, i, false);
__csid_ctrl_rdi(csid, enable, i);
}
}
diff --git a/drivers/media/platform/qcom/camss/camss-csid-gen3.c b/ drivers/media/platform/qcom/camss/camss-csid-gen3.c
index 0fdbf75fb27d..830c9c818d3d 100644
--- a/drivers/media/platform/qcom/camss/camss-csid-gen3.c
+++ b/drivers/media/platform/qcom/camss/camss-csid-gen3.c
@@ -219,6 +219,8 @@ static void __csid_configure_rdi_stream(struct csid_device *csid, u8 enable, u8
writel(val, csid->base + CSID_RDI_CFG0(port));
}
+static void csid_subdev_reg_update(struct csid_device *csid, int port_id, bool is_clear);
+
static void csid_configure_stream(struct csid_device *csid, u8 enable)
{
u8 i;
@@ -230,6 +232,7 @@ static void csid_configure_stream(struct csid_device *csid, u8 enable)
if (csid->phy.en_vc & BIT(i)) {
__csid_configure_rdi_stream(csid, enable, i, 0);
__csid_configure_rx(csid, &csid->phy, 0);
+ csid_subdev_reg_update(csid, i, false);
__csid_ctrl_rdi(csid, enable, i);
}
}
diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c b/drivers/ media/platform/qcom/camss/camss-vfe.c
index 2680a9914765..16a0b5663fe2 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe.c
@@ -502,6 +502,37 @@ void vfe_buf_done(struct vfe_device *vfe, int wm)
spin_unlock_irqrestore(&vfe->output_lock, flags);
}
+/*
+ * vfe_upstream_csid_has_reg_update - Check whether the CSID feeding this line
+ * issues its own streamon RUP (i.e. its configure_stream does the RUP).
+ *
+ * The CSID->VFE link is created in camss.c for all CSIDs and VFEs, so the
+ * upstream entity of the sink pad is either a CSID (possibly with RUP) or an
+ * ISPIF (no RUP). Only enabled links are considered.
+ */
+static bool vfe_upstream_csid_has_reg_update(struct vfe_line *line)
+{
+ struct vfe_device *vfe = to_vfe(line);
+ struct camss *camss = vfe->camss;
+ struct media_pad *remote;
+ unsigned int i;
+
+ remote = media_pad_remote_pad_first(&line->pads[MSM_VFE_PAD_SINK]);
+ if (!remote || !is_media_entity_v4l2_subdev(remote->entity))
+ return false;
+
+ for (i = 0; i < camss->res->csid_num; i++) {
+ struct csid_device *csid = &camss->csid[i];
+
+ if (&csid->subdev.entity != remote->entity)
+ continue;
+
+ return !!csid->res->hw_ops->reg_update;
+ }
+
+ return false;
+}
+
int vfe_enable_output_v2(struct vfe_line *line)
{
struct vfe_device *vfe = to_vfe(line);
@@ -511,6 +542,7 @@ int vfe_enable_output_v2(struct vfe_line *line)
unsigned long flags;
unsigned int frame_skip = 0;
unsigned int i;
+ bool rup_in_csid;
sensor_pad = camss_find_sensor_pad(&line->subdev.entity);
if (sensor_pad) {
@@ -523,6 +555,8 @@ int vfe_enable_output_v2(struct vfe_line *line)
frame_skip = VFE_FRAME_DROP_VAL - 1;
}
+ rup_in_csid = vfe_upstream_csid_has_reg_update(line);
+
spin_lock_irqsave(&vfe->output_lock, flags);
ops->reg_update_clear(vfe, line->id);
@@ -552,7 +586,8 @@ int vfe_enable_output_v2(struct vfe_line *line)
output->gen2.active_num++;
ops->vfe_wm_update(vfe, output->wm_idx[0],
output->buf[i]->addr[0], line);
- ops->reg_update(vfe, line->id);
+ if (!rup_in_csid)
+ ops->reg_update(vfe, line->id);
}
spin_unlock_irqrestore(&vfe->output_lock, flags);
According to the hardware documentation:
Starting with CSID 680, CAMIF has moved into the CSID. As a result, RUP is now issued by the CSID and propagated to the IFE.
Starting with CSID 980, software must also set the SET bit in RUP_AUP_CMD after issuing the RUP/AUP command. Otherwise, the hardware ignores the command and the corresponding register update does not take effect.
Bryan, according to the HW documentation, the change itself is consistent with the required programming sequence. The main question is whether this is the right implementation from the driver perspective, or if there is a cleaner way to handle it.
I have tested the change on the Hamoa EVK board.
Tested-by: Wenmeng Liu <wenmeng.liu@xxxxxxxxxxxxxxxx> # Hamoa EVK
Thanks,
Wenmeng
OK thank you for the explanation.
OTOH I agree with Loic's statement - it is cleaner to subtract the callback and disjunct on its presence than to add a flag to the same effect in reverse.
---
bod