Re: [PATCH 3/8] media: qcom: camss: Implement CSID streams API hw_ops for gen2

[Loic Poulain]

Hi Gjorgji,

On Fri, Sep 11, 2026 at 8:22 AM Gjorgji Rosikopulos
<gjorgji.rosikopulos@xxxxxxxxxxxxxxxx> wrote:
>
> From: Gjorgji Rosikopulos <gjorgji.rosikopulos@xxxxxxxxxxxxxxxx>
>
> Implement the configure_rx/enable_stream/disable_stream hw_ops added to
> struct csid_hw_ops, for the gen2 CSID hardware backend used by SM8250.
>
> __csid_configure_rdi_stream() now takes an explicit data type parameter
> instead of deriving it from the CSID source pad's format, since the
> streams API keys each stream by stream_id rather than by pad. The
> existing configure_stream() path is updated to pass its per-port format
> lookup through to this parameter, preserving current behavior.
>
> Signed-off-by: Gjorgji Rosikopulos <gjorgji.rosikopulos@xxxxxxxxxxxxxxxx>
> ---
>  .../platform/qcom/camss/camss-csid-gen2.c     | 59 ++++++++++++++++---
>  1 file changed, 51 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/camss/camss-csid-gen2.c b/drivers/media/platform/qcom/camss/camss-csid-gen2.c
> index eadcb2f7e3aa..2512cdb16d94 100644
> --- a/drivers/media/platform/qcom/camss/camss-csid-gen2.c
> +++ b/drivers/media/platform/qcom/camss/camss-csid-gen2.c
> @@ -253,13 +253,8 @@ static void __csid_configure_testgen(struct csid_device *csid, u8 enable, u8 por
>         writel_relaxed(val, csid->base + CSID_TPG_CTRL);
>  }
>
> -static void __csid_configure_rdi_stream(struct csid_device *csid, u8 enable, u8 port, u8 vc)
> +static void __csid_configure_rdi_stream(struct csid_device *csid, u8 enable, u8 port, u8 vc, u8 dt)
>  {
> -       /* Source pads matching RDI channels on hardware. Pad 1 -> RDI0, Pad 2 -> RDI1, etc. */
> -       struct v4l2_mbus_framefmt *input_format = &csid->fmt[MSM_CSID_PAD_FIRST_SRC + port];
> -       const struct csid_format_info *format = csid_get_fmt_entry(csid->res->formats->formats,
> -                                                                  csid->res->formats->nformats,
> -                                                                  input_format->code);
>         u32 val;
>
>         /*
> @@ -281,7 +276,7 @@ static void __csid_configure_rdi_stream(struct csid_device *csid, u8 enable, u8
>         val |= 1 << RDI_CFG0_TIMESTAMP_EN;
>         /* note: for non-RDI path, this should be format->decode_format */
>         val |= DECODE_FORMAT_PAYLOAD_ONLY << RDI_CFG0_DECODE_FORMAT;
> -       val |= format->data_type << RDI_CFG0_DATA_TYPE;
> +       val |= dt << RDI_CFG0_DATA_TYPE;
>         val |= vc << RDI_CFG0_VIRTUAL_CHANNEL;
>         val |= dt_id << RDI_CFG0_DT_ID;
>         writel_relaxed(val, csid->base + CSID_RDI_CFG0(port));
> @@ -330,15 +325,60 @@ static void csid_configure_stream(struct csid_device *csid, u8 enable)
>         /* Loop through all enabled ports and configure a stream for each */
>         for (i = 0; i < MSM_CSID_MAX_SRC_STREAMS; i++)
>                 if (csid->phy.en_vc & BIT(i)) {
> +                       /* Source pads match RDI channels: pad 1 -> RDI0, pad 2 -> RDI1, etc. */
> +                       struct v4l2_mbus_framefmt *input_format =
> +                               &csid->fmt[MSM_CSID_PAD_FIRST_SRC + i];
> +                       const struct csid_format_info *format =
> +                               csid_get_fmt_entry(csid->res->formats->formats,
> +                                                  csid->res->formats->nformats,
> +                                                  input_format->code);
> +                       u8 vc = 0;
> +
>                         if (tg->enabled)
>                                 __csid_configure_testgen(csid, enable, i, 0);
>
> -                       __csid_configure_rdi_stream(csid, enable, i, 0);
> +                       __csid_configure_rdi_stream(csid, enable, i, vc, format->data_type);
>                         __csid_configure_rx(csid, &csid->phy, 0);
>                         __csid_ctrl_rdi(csid, enable, i);
>                 }
>  }
>
> +/*
> + * configure_rx - Configure the CSID Rx front-end
> + */
> +static void csid_configure_rx(struct csid_device *csid)
> +{
> +       __csid_configure_rx(csid, &csid->phy, 0);
> +}
> +
> +/*
> + * stream_id is used directly as the hardware RDI port index below. This
> + * assumes a 1:1 stream-to-port mapping and should be revisited once
> + * per-platform src_streams data (stream id -> hw pipe) is added.
> + */
> +static void csid_enable_stream(struct csid_device *csid, u32 stream_id, u8 vc, u8 dt)
> +{
> +       struct csid_testgen_config *tg = &csid->testgen;
> +
> +       if (tg->enabled)
> +               __csid_configure_testgen(csid, 1, stream_id, vc);
> +
> +       __csid_configure_rdi_stream(csid, 1, stream_id, vc, dt);
> +       __csid_ctrl_rdi(csid, 1, stream_id);
> +}
> +
> +static void csid_disable_stream(struct csid_device *csid, u32 stream_id)
> +{
> +       struct csid_testgen_config *tg = &csid->testgen;
> +
> +       __csid_ctrl_rdi(csid, 0, stream_id);
> +
> +       if (tg->enabled)
> +               __csid_configure_testgen(csid, 0, stream_id, 0);
> +
> +       __csid_configure_rdi_stream(csid, 0, stream_id, 0, 0);
> +}
> +
>  static int csid_configure_testgen_pattern(struct csid_device *csid, s32 val)
>  {
>         if (val > 0 && val <= csid->testgen.nmodes)
> @@ -425,6 +465,9 @@ static void csid_subdev_init(struct csid_device *csid)
>  const struct csid_hw_ops csid_ops_gen2 = {
>         .configure_stream = csid_configure_stream,
>         .configure_testgen_pattern = csid_configure_testgen_pattern,
> +       .configure_rx = csid_configure_rx,
> +       .enable_stream = csid_enable_stream,
> +       .disable_stream = csid_disable_stream,

Would it be possible to leverage the existing CSID configure_stream()
callback to implement the stream API? This would likely require
reconfiguring all CSID ports whenever a stream is started/stopped, but
if the configuration remains unchanged, I do not think that would be
problematic. That said, we probably need to address the current
ambiguity between the concepts of "VC" and "PORT" in the existing
drivers first.

>         .hw_version = csid_hw_version,
>         .isr = csid_isr,
>         .reset = csid_reset,
> --
> 2.34.1
>