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

[Loic Poulain]

Hi Hitesh,

On Mon, Sep 28, 2026 at 12:23 PM Hitesh Patel <hitesh@xxxxxxxxxxxxxx> wrote:
>
> camss_find_sensor_pad() walks the pipeline up to an entity with the
> MEDIA_ENT_F_CAM_SENSOR function, and camss_get_link_freq() 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".
>
> Replace the helper with camss_find_transmitter_pad(), which stops 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 values returned do not change
> anywhere. Only a bridge setup sees a difference, and there the two
> other users of the old helper now describe the hardware better as
> well:
>
>  - camss_get_pixel_clock() reads V4L2_CID_PIXEL_RATE, which for the
>    same reason as the link frequency belongs to the device driving the
>    bus.
>  - the g_skip_frames query in vfe_enable_output_v2() and its gen1
>    counterpart is a sensor operation. A bridge does not implement it,
>    so the call fails and frame_skip stays 0, which is what already
>    happens today whenever the walk cannot reach a sensor.
>
> Signed-off-by: Hitesh Patel <hitesh@xxxxxxxxxxxxxx>
> ---
>  .../platform/qcom/camss/camss-vfe-gen1.c      |  8 +--
>  drivers/media/platform/qcom/camss/camss-vfe.c |  8 +--
>  drivers/media/platform/qcom/camss/camss.c     | 64 ++++++++++++++-----
>  drivers/media/platform/qcom/camss/camss.h     |  2 +-
>  4 files changed, 57 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/camss/camss-vfe-gen1.c b/drivers/media/platform/qcom/camss/camss-vfe-gen1.c
> index d84a375e33..8827b51694 100644
> --- a/drivers/media/platform/qcom/camss/camss-vfe-gen1.c
> +++ b/drivers/media/platform/qcom/camss/camss-vfe-gen1.c
> @@ -170,7 +170,7 @@ static int vfe_enable_output(struct vfe_line *line)
>         struct vfe_device *vfe = to_vfe(line);
>         struct vfe_output *output = &line->output;
>         const struct vfe_hw_ops *ops = vfe->res->hw_ops;
> -       struct media_pad *sensor_pad;
> +       struct media_pad *tx_pad;
>         unsigned long flags;
>         unsigned int frame_skip = 0;
>         unsigned int i;
> @@ -180,10 +180,10 @@ static int vfe_enable_output(struct vfe_line *line)
>         if (!ub_size)
>                 return -EINVAL;
>
> -       sensor_pad = camss_find_sensor_pad(&line->subdev.entity);
> -       if (sensor_pad) {
> +       tx_pad = camss_find_transmitter_pad(&line->subdev.entity);
> +       if (tx_pad) {
>                 struct v4l2_subdev *subdev =
> -                       media_entity_to_v4l2_subdev(sensor_pad->entity);
> +                       media_entity_to_v4l2_subdev(tx_pad->entity);
>
>                 v4l2_subdev_call(subdev, sensor, g_skip_frames, &frame_skip);
>                 /* Max frame skip is 29 frames */
> diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c b/drivers/media/platform/qcom/camss/camss-vfe.c
> index d20cd4dfb9..a2ff6ae503 100644
> --- a/drivers/media/platform/qcom/camss/camss-vfe.c
> +++ b/drivers/media/platform/qcom/camss/camss-vfe.c
> @@ -507,15 +507,15 @@ int vfe_enable_output_v2(struct vfe_line *line)
>         struct vfe_device *vfe = to_vfe(line);
>         struct vfe_output *output = &line->output;
>         const struct vfe_hw_ops *ops = vfe->res->hw_ops;
> -       struct media_pad *sensor_pad;
> +       struct media_pad *tx_pad;
>         unsigned long flags;
>         unsigned int frame_skip = 0;
>         unsigned int i;
>
> -       sensor_pad = camss_find_sensor_pad(&line->subdev.entity);
> -       if (sensor_pad) {
> +       tx_pad = camss_find_transmitter_pad(&line->subdev.entity);
> +       if (tx_pad) {
>                 struct v4l2_subdev *subdev =
> -                       media_entity_to_v4l2_subdev(sensor_pad->entity);
> +                       media_entity_to_v4l2_subdev(tx_pad->entity);
>
>                 v4l2_subdev_call(subdev, sensor, g_skip_frames, &frame_skip);

I sent a patch to remove the frame_skip retrieval from this code path,
as it is currently unused:
https://lore.kernel.org/all/20260928-camss-misc-fixes-v1-1-3e0155af6df5@xxxxxxxxxxxxxxxx/

Feel free to include this change in your series ahead of that patch,
so you don't need to worry about handling it separately.

>                 /* Max frame skip is 29 frames */
> diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
> index 23f3cc30a1..42d8654fd0 100644
> --- a/drivers/media/platform/qcom/camss/camss.c
> +++ b/drivers/media/platform/qcom/camss/camss.c
> @@ -4594,13 +4594,44 @@ void camss_disable_clocks(int nclocks, struct camss_clock *clock)
>  }
>
>  /*
> - * camss_find_sensor_pad - Find the media pad via which the sensor is linked
> - * @entity: Media entity to start searching from
> + * 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
>   *
> - * Return a pointer to sensor media pad or NULL if not found
> + * 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
>   */
> -struct media_pad *camss_find_sensor_pad(struct media_entity *entity)
> +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) {
> @@ -4613,34 +4644,35 @@ struct media_pad *camss_find_sensor_pad(struct media_entity *entity)
>                         return NULL;
>
>                 entity = pad->entity;
> +               sd = media_entity_to_v4l2_subdev(entity);
>
> -               if (entity->function == MEDIA_ENT_F_CAM_SENSOR)
> +               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);
>  }
>
>  /*
> - * camss_get_pixel_clock - Get pixel clock rate from sensor
> + * camss_get_pixel_clock - Get pixel clock rate from the CSI-2 transmitter
>   * @entity: Media entity in the current pipeline
>   * @pixel_clock: Received pixel clock value
>   *
> @@ -4648,15 +4680,15 @@ s64 camss_get_link_freq(struct media_entity *entity, unsigned int bpp,
>   */
>  int camss_get_pixel_clock(struct media_entity *entity, u64 *pixel_clock)
>  {
> -       struct media_pad *sensor_pad;
> +       struct media_pad *tx_pad;
>         struct v4l2_subdev *subdev;
>         struct v4l2_ctrl *ctrl;
>
> -       sensor_pad = camss_find_sensor_pad(entity);
> -       if (!sensor_pad)
> +       tx_pad = camss_find_transmitter_pad(entity);
> +       if (!tx_pad)
>                 return -ENODEV;
>
> -       subdev = media_entity_to_v4l2_subdev(sensor_pad->entity);
> +       subdev = media_entity_to_v4l2_subdev(tx_pad->entity);
>
>         ctrl = v4l2_ctrl_find(subdev->ctrl_handler, V4L2_CID_PIXEL_RATE);
>
> diff --git a/drivers/media/platform/qcom/camss/camss.h b/drivers/media/platform/qcom/camss/camss.h
> index 93d691c8ac..1560ad0dc5 100644
> --- a/drivers/media/platform/qcom/camss/camss.h
> +++ b/drivers/media/platform/qcom/camss/camss.h
> @@ -167,7 +167,7 @@ void camss_add_clock_margin(u64 *rate);
>  int camss_enable_clocks(int nclocks, struct camss_clock *clock,
>                         struct device *dev);
>  void camss_disable_clocks(int nclocks, struct camss_clock *clock);
> -struct media_pad *camss_find_sensor_pad(struct media_entity *entity);
> +struct media_pad *camss_find_transmitter_pad(struct media_entity *entity);
>  s64 camss_get_link_freq(struct media_entity *entity, unsigned int bpp,
>                         unsigned int lanes);
>  int camss_get_pixel_clock(struct media_entity *entity, u64 *pixel_clock);
> --
> 2.43.0
>