Re: [PATCH v21 04/14] media: qcom: camss: Issue stream-on REG_UPDATE from CSID
From: Loic Poulain
Date: Wed Oct 21 2026 - 04:03:49 EST
On Mon, Oct 12, 2026 at 8:45 AM Hangxiang Ma
<hangxiang.ma@xxxxxxxxxxxxxxxx> 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.
So why not simply make reg_update optional and either not implement it
or nullify it for the related VFEs? That seems much cleaner to me.
If the VFE implements the callback, we invoke it. If it doesn't, we
simply skip it.
>
> 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);
>
> --
> 2.34.1
>