[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <f6242795-d8d4-44a9-8ab8-cdaa3dc0eeff@ti.com>
Date: Fri, 16 Jan 2026 16:34:12 +0530
From: Rishikesh Donadkar <r-donadkar@...com>
To: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
CC: <y-abhilashchandra@...com>, <devarsht@...com>, <s-jain1@...com>,
<vigneshr@...com>, <mchehab@...nel.org>, <robh@...nel.org>,
<krzk+dt@...nel.org>, <p.zabel@...gutronix.de>, <conor+dt@...nel.org>,
<sakari.ailus@...ux.intel.com>, <hverkuil-cisco@...all.nl>,
<jai.luthra@...asonboard.com>, <changhuang.liang@...rfivetech.com>,
<jack.zhu@...rfivetech.com>, <sjoerd@...labora.com>,
<dan.carpenter@...aro.org>, <hverkuil+cisco@...nel.org>,
<linux-kernel@...r.kernel.org>, <linux-media@...r.kernel.org>,
<devicetree@...r.kernel.org>, <jai.luthra@...ux.dev>,
<laurent.pinchart@...asonboard.com>, <mripard@...nel.org>
Subject: Re: [PATCH v9 12/19] media: cadence: csi2rx: add multistream support
On 15/01/26 17:31, Tomi Valkeinen wrote:
> Hi,
Hi Tomi,
Thank you for the review !
>
> On 30/12/2025 10:32, Rishikesh Donadkar wrote:
>> From: Jai Luthra <j-luthra@...com>
>>
>> Cadence CSI-2 bridge IP supports capturing multiple virtual "streams"
>> of data over the same physical interface using MIPI Virtual Channels.
>>
>> While the hardware IP supports usecases where streams coming in the sink
>> pad can be broadcasted to multiple source pads, the driver will need
>> significant re-architecture to make that possible. The two users of this
>> IP in mainline linux are TI Shim and StarFive JH7110 CAMSS, and both
>> have only integrated the first source pad i.e stream0 of this IP. So for
>> now keep it simple and only allow 1-to-1 mapping of streams from sink to
>> source, without any broadcasting.
>>
>> Signed-off-by: Jai Luthra <j-luthra@...com>
>> Reviewed-by: Changhuang Liang <changhuang.liang@...rfivetech.com>
>> Reviewed-by: Yemike Abhilash Chandra <y-abhilashchandra@...com>
>> Co-developed-by: Rishikesh Donadkar <r-donadkar@...com>
>> Signed-off-by: Rishikesh Donadkar <r-donadkar@...com>
>> ---
>> drivers/media/platform/cadence/cdns-csi2rx.c | 248 +++++++++++++++----
>> 1 file changed, 201 insertions(+), 47 deletions(-)
>>
>> diff --git a/drivers/media/platform/cadence/cdns-csi2rx.c b/drivers/media/platform/cadence/cdns-csi2rx.c
>> index 65c6acb02f85b..5c16a2e509136 100644
>> --- a/drivers/media/platform/cadence/cdns-csi2rx.c
>> +++ b/drivers/media/platform/cadence/cdns-csi2rx.c
>> @@ -135,6 +135,7 @@ struct csi2rx_priv {
>> struct phy *dphy;
>>
>> u8 num_pixels[CSI2RX_STREAMS_MAX];
>> + u32 vc_select[CSI2RX_STREAMS_MAX];
>> u8 lanes[CSI2RX_LANES_MAX];
>> u8 num_lanes;
>> u8 max_lanes;
>> @@ -273,30 +274,43 @@ static void csi2rx_reset(struct csi2rx_priv *csi2rx)
>>
>> static int csi2rx_configure_ext_dphy(struct csi2rx_priv *csi2rx)
>> {
>> - struct media_pad *src_pad =
>> - &csi2rx->source_subdev->entity.pads[csi2rx->source_pad];
>> union phy_configure_opts opts = { };
>> struct phy_configure_opts_mipi_dphy *cfg = &opts.mipi_dphy;
>> - struct v4l2_subdev_state *state;
>> struct v4l2_mbus_framefmt *framefmt;
>> + struct v4l2_subdev_state *state;
>> const struct csi2rx_fmt *fmt;
>> + int source_pad = csi2rx->source_pad;
>> + struct media_pad *pad = &csi2rx->source_subdev->entity.pads[source_pad];
>> s64 link_freq;
>> int ret;
>> + u32 bpp;
>>
>> state = v4l2_subdev_get_locked_active_state(&csi2rx->subdev);
>>
>> - framefmt = v4l2_subdev_state_get_format(state, CSI2RX_PAD_SINK, 0);
>> - if (!framefmt) {
>> - dev_err(csi2rx->dev, "Did not find active sink format\n");
>> - return -EINVAL;
>> - }
>> + /*
>> + * For multi-stream transmitters there is no single pixel rate.
>> + *
>> + * In multistream usecase pass bpp as 0 so that v4l2_get_link_freq()
>> + * returns an error if it falls back to V4L2_CID_PIXEL_RATE.
>> + */
>> + if (state->routing.num_routes > 1) {
>> + bpp = 0;
>> + } else {
>> + framefmt = v4l2_subdev_state_get_format(state, CSI2RX_PAD_SINK, 0);
>> + if (!framefmt) {
>> + dev_err(csi2rx->dev, "Did not find active sink format\n");
>> + return -EINVAL;
>> + }
>>
>> - fmt = csi2rx_get_fmt_by_code(framefmt->code);
>> + fmt = csi2rx_get_fmt_by_code(framefmt->code);
>> + bpp = fmt->bpp;
>> + }
>>
>> - link_freq = v4l2_get_link_freq(src_pad,
>> - fmt->bpp, 2 * csi2rx->num_lanes);
>> - if (link_freq < 0)
>> + link_freq = v4l2_get_link_freq(pad, bpp, 2 * csi2rx->num_lanes);
>> + if (link_freq < 0) {
>> + dev_err(csi2rx->dev, "Unable to calculate link frequency\n");
>> return link_freq;
>> + }
>>
>> ret = phy_mipi_dphy_get_default_config_for_hsclk(link_freq,
>> csi2rx->num_lanes, cfg);
>> @@ -394,11 +408,7 @@ static int csi2rx_start(struct csi2rx_priv *csi2rx)
>> csi2rx->num_pixels[i]),
>> csi2rx->base + CSI2RX_STREAM_CFG_REG(i));
>>
>> - /*
>> - * Enable one virtual channel. When multiple virtual channels
>> - * are supported this will have to be changed.
>> - */
>> - writel(CSI2RX_STREAM_DATA_CFG_VC_SELECT(0),
>> + writel(csi2rx->vc_select[i],
>> csi2rx->base + CSI2RX_STREAM_DATA_CFG_REG(i));
>>
>> writel(CSI2RX_STREAM_CTRL_START,
>> @@ -486,18 +496,59 @@ static int csi2rx_log_status(struct v4l2_subdev *sd)
>> return 0;
>> }
>>
>> +static void csi2rx_update_vc_select(struct csi2rx_priv *csi2rx,
>> + struct v4l2_subdev_state *state)
>> +{
>> + struct v4l2_mbus_frame_desc fd = {0};
>> + struct v4l2_subdev_route *route;
>> + unsigned int i;
>> + int ret;
>> +
>> + /* Capture VC=0 by default */
>> + for (i = 0; i < CSI2RX_STREAMS_MAX; i++)
>> + csi2rx->vc_select[i] = CSI2RX_STREAM_DATA_CFG_VC_SELECT(0);
> This should be inside the if-block below, as in the other code path you
> just memset the whole vc_select.
Will do
Rishikesh
>
> With that fixed:
>
> Reviewed-by: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
>
> Tomi
>
Powered by blists - more mailing lists