[PATCH v21 04/14] media: qcom: camss: Issue stream-on REG_UPDATE from CSID
Bryan O'Donoghue
bryan.odonoghue at linaro.org
Wed Oct 21 01:53:08 PDT 2026
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 at oss.qualcomm.com>
>> ---
>> 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 at oss.qualcomm.com> # 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
More information about the linux-phy
mailing list