Re: [PATCH v10 12/18] media: cadence: csi2rx: Use the stream from route to get format

[Rishikesh Donadkar <r-donadkar@ti.com>]

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@ti.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@ideasonboard.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");
>>