[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <d19d1cd1-b7fa-43ab-9ef9-ade414efc508@ti.com>
Date: Fri, 30 Jan 2026 09:37:47 +0530
From: Rishikesh Donadkar <r-donadkar@...com>
To: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>, <jai.luthra@...ux.dev>,
<laurent.pinchart@...asonboard.com>, <mripard@...nel.org>
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>
Subject: Re: [PATCH v10 12/18] media: cadence: csi2rx: Use the stream from
route to get format
On 23/01/26 13:23, Tomi Valkeinen wrote:
> Hi,
Hi Tomi,
>
> On 21/01/2026 15:54, Rishikesh Donadkar wrote:
>> In multistream configurations, different streams can have different
>> formats. Update the driver to use the stream number from the routing
>> configuration when retrieving formats instead of hardcoding stream 0
>> or ignoring streams.
>>
>> In csi2rx_configure_ext_dphy(), use the sink_stream from the first
>> route instead of always using stream 0.
>>
>> In cdns_csi2rx_negotiate_ppc(), iterate through all active routes
>> for the requested pad and retrieve the format using both pad and
>> stream information.
>>
>> Signed-off-by: Rishikesh Donadkar <r-donadkar@...com>
>> ---
>> drivers/media/platform/cadence/cdns-csi2rx.c | 34 ++++++++++++++++----
>> 1 file changed, 28 insertions(+), 6 deletions(-)
> So if I understand this right, v9 was working by luck, as it was always
> using stream 0 format (and expected stream 0 to be there)?
Yes, when I tested using IMX219 connected to the port 1 of the fpd-link
board, I faced an issue where the v4l2_subdev_state_get_format()
returned NULL when no stream was passed to it, here the default 0 was
considered but the fpd-link driver which assigns stream number wrt ports
assigned it as 1.
>
> Two thoughts here:
> - This should be merged to the previous patch, shouldn't it?
Yes, will do that
> - Are you now testing with different resolutions, fps, etc on the
> fpd-link ports?
I will test the v11 with multiple resolutions and formats. I will
mention the tested ones in the cover letter
Rishikesh
>
> Reviewed-by: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
>
> Tomi
>
>> diff --git a/drivers/media/platform/cadence/cdns-csi2rx.c b/drivers/media/platform/cadence/cdns-csi2rx.c
>> index 175366f889115..db9871fdbe3a4 100644
>> --- a/drivers/media/platform/cadence/cdns-csi2rx.c
>> +++ b/drivers/media/platform/cadence/cdns-csi2rx.c
>> @@ -279,6 +279,7 @@ static int csi2rx_configure_ext_dphy(struct csi2rx_priv *csi2rx)
>> struct v4l2_mbus_framefmt *framefmt;
>> struct v4l2_subdev_state *state;
>> const struct csi2rx_fmt *fmt;
>> + struct v4l2_subdev_route *route;
>> int source_pad = csi2rx->source_pad;
>> struct media_pad *pad = &csi2rx->source_subdev->entity.pads[source_pad];
>> s64 link_freq;
>> @@ -296,7 +297,9 @@ static int csi2rx_configure_ext_dphy(struct csi2rx_priv *csi2rx)
>> if (state->routing.num_routes > 1) {
>> bpp = 0;
>> } else {
>> - framefmt = v4l2_subdev_state_get_format(state, CSI2RX_PAD_SINK, 0);
>> + route = &state->routing.routes[0];
>> + framefmt = v4l2_subdev_state_get_format(state, CSI2RX_PAD_SINK,
>> + route->sink_stream);
>> if (!framefmt) {
>> dev_err(csi2rx->dev, "Did not find active sink format\n");
>> return -EINVAL;
>> @@ -706,25 +709,44 @@ int cdns_csi2rx_negotiate_ppc(struct v4l2_subdev *subdev, unsigned int pad,
>> {
>> struct csi2rx_priv *csi2rx = v4l2_subdev_to_csi2rx(subdev);
>> const struct csi2rx_fmt *csi_fmt;
>> + struct v4l2_subdev_route *route;
>> struct v4l2_subdev_state *state;
>> struct v4l2_mbus_framefmt *fmt;
>> + int ret = 0;
>>
>> if (!ppc || pad < CSI2RX_PAD_SOURCE_STREAM0 || pad >= CSI2RX_PAD_MAX)
>> return -EINVAL;
>>
>> state = v4l2_subdev_lock_and_get_active_state(subdev);
>> - fmt = v4l2_subdev_state_get_format(state, pad);
>> - csi_fmt = csi2rx_get_fmt_by_code(fmt->code);
>> + /* Check all streams on requested pad */
>> + for_each_active_route(&state->routing, route) {
>> + if (route->source_pad != pad)
>> + continue;
>> +
>> + fmt = v4l2_subdev_state_get_format(state, route->source_pad,
>> + route->source_stream);
>> + if (!fmt) {
>> + ret = -EPIPE;
>> + *ppc = 1;
>> + break;
>> + }
>>
>> - /* Reduce requested PPC if it is too high */
>> - *ppc = min(*ppc, csi_fmt->max_pixels);
>> + csi_fmt = csi2rx_get_fmt_by_code(fmt->code);
>> + if (!csi_fmt) {
>> + ret = -EINVAL;
>> + *ppc = 1;
>> + break;
>> + }
>>
>> + /* Reduce requested PPC if it is too high for this stream */
>> + *ppc = min(*ppc, csi_fmt->max_pixels);
>> + }
>> v4l2_subdev_unlock_state(state);
>>
>> csi2rx->num_pixels[pad - CSI2RX_PAD_SOURCE_STREAM0] =
>> CSI2RX_STREAM_CFG_NUM_PIXELS(*ppc);
>>
>> - return 0;
>> + return ret;
>> }
>> EXPORT_SYMBOL_FOR_MODULES(cdns_csi2rx_negotiate_ppc, "j721e-csi2rx");
>>
Powered by blists - more mailing lists