mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
To: Rishikesh Donadkar <r-donadkar@ti.com>,
	jai.luthra@linux.dev, laurent.pinchart@ideasonboard.com,
	mripard@kernel.org
Cc: y-abhilashchandra@ti.com, devarsht@ti.com, s-jain1@ti.com,
	vigneshr@ti.com, mchehab@kernel.org, robh@kernel.org,
	krzk+dt@kernel.org, p.zabel@pengutronix.de, conor+dt@kernel.org,
	sakari.ailus@linux.intel.com, hverkuil-cisco@xs4all.nl,
	jai.luthra@ideasonboard.com, changhuang.liang@starfivetech.com,
	jack.zhu@starfivetech.com, sjoerd@collabora.com,
	hverkuil+cisco@kernel.org, linux-kernel@vger.kernel.org,
	linux-media@vger.kernel.org, devicetree@vger.kernel.org
Subject: Re: [PATCH v7 11/16] media: ti: j721e-csi2rx: add multistream support
Date: Thu, 25 Sep 2025 15:17:38 +0300	[thread overview]
Message-ID: <4534a09b-7eef-4e61-835a-c70d07df3416@ideasonboard.com> (raw)
In-Reply-To: <20250911102832.1583440-12-r-donadkar@ti.com>

Hi,

On 11/09/2025 13:28, Rishikesh Donadkar wrote:
> From: Jai Luthra <j-luthra@ti.com>
> 
> Each CSI2 stream can be multiplexed into 4 independent streams, each
> identified by its virtual channel number. To capture this multiplexed

The split can also be done with the datatype. I don't see it supported
in the driver, but afaics the HW supports it. Was there a reason not to
support DT filtering? I would think it would be very simple addition.

 Tomi

> stream, the application needs to tell the driver how it wants to route
> the data. It needs to specify which context should process which stream.
> This is done via the new routing APIs.
> 
> Add ioctls to accept routing information from the application and save
> that in the driver. This can be used when starting streaming on a
> context to determine which route and consequently which virtual channel
> it should process.
> 
> Support the new enable_stream()/disable_stream() APIs in the subdev
> instead of s_stream() hook.
> 
> De-assert the pixel interface reset on first start_streaming() and assert
> it on the last stop_streaming().
> 
> Reviewed-by: Yemike Abhilash Chandra <y-abhilashchandra@ti.com>
> Co-developed-by: Pratyush Yadav <p.yadav@ti.com>
> Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
> Signed-off-by: Jai Luthra <j-luthra@ti.com>
> Co-developed-by: Rishikesh Donadkar <r-donadkar@ti.com>
> Signed-off-by: Rishikesh Donadkar <r-donadkar@ti.com>
> ---
>  .../platform/ti/j721e-csi2rx/j721e-csi2rx.c   | 194 ++++++++++++------
>  1 file changed, 136 insertions(+), 58 deletions(-)
> 
> diff --git a/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c b/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c
> index 903722b5aea7..f0c6f931bfc7 100644
> --- a/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c
> +++ b/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c
> @@ -146,17 +146,6 @@ static inline struct ti_csi2rx_dev *to_csi2rx_dev(struct v4l2_subdev *sd)
>  	return container_of(sd, struct ti_csi2rx_dev, subdev);
>  }
>  
> -static const struct v4l2_mbus_framefmt ti_csi2rx_default_fmt = {
> -	.width = 640,
> -	.height = 480,
> -	.code = MEDIA_BUS_FMT_UYVY8_1X16,
> -	.field = V4L2_FIELD_NONE,
> -	.colorspace = V4L2_COLORSPACE_SRGB,
> -	.ycbcr_enc = V4L2_YCBCR_ENC_601,
> -	.quantization = V4L2_QUANTIZATION_LIM_RANGE,
> -	.xfer_func = V4L2_XFER_FUNC_SRGB,
> -};
> -
>  static const struct ti_csi2rx_fmt ti_csi2rx_formats[] = {
>  	{
>  		.fourcc			= V4L2_PIX_FMT_YUYV,
> @@ -568,8 +557,10 @@ static void ti_csi2rx_setup_shim(struct ti_csi2rx_ctx *ctx)
>  	fmt = find_format_by_fourcc(ctx->v_fmt.fmt.pix.pixelformat);
>  
>  	/* De-assert the pixel interface reset. */
> -	reg = SHIM_CNTL_PIX_RST;
> -	writel(reg, csi->shim + SHIM_CNTL);
> +	if (!csi->enable_count) {
> +		reg = SHIM_CNTL_PIX_RST;
> +		writel(reg, csi->shim + SHIM_CNTL);
> +	}
>  
>  	/* Negotiate pixel count from the source */
>  	ti_csi2rx_request_max_ppc(csi);
> @@ -1023,7 +1014,10 @@ static int ti_csi2rx_start_streaming(struct vb2_queue *vq, unsigned int count)
>  	dma->state = TI_CSI2RX_DMA_ACTIVE;
>  	spin_unlock_irqrestore(&dma->lock, flags);
>  
> -	ret = v4l2_subdev_call(&csi->subdev, video, s_stream, 1);
> +	/* Start stream 0, we don't allow multiple streams on the source pad */
> +	ret = v4l2_subdev_enable_streams(&csi->subdev,
> +					 TI_CSI2RX_PAD_FIRST_SOURCE + ctx->idx,
> +					 BIT(0));
>  	if (ret)
>  		goto err_dma;
>  
> @@ -1046,19 +1040,26 @@ static void ti_csi2rx_stop_streaming(struct vb2_queue *vq)
>  	struct ti_csi2rx_dev *csi = ctx->csi;
>  	int ret;
>  
> -	video_device_pipeline_stop(&ctx->vdev);
> +	mutex_lock(&csi->mutex);
>  
> -	writel(0, csi->shim + SHIM_CNTL);
>  	writel(0, csi->shim + SHIM_DMACNTX(ctx->idx));
>  
> -	mutex_lock(&csi->mutex);
> -	csi->vc_cached = false;
> -	mutex_unlock(&csi->mutex);
> +	/* assert pixel reset to prevent stale data */
> +	if (csi->enable_count == 1) {
> +		writel(0, csi->shim + SHIM_CNTL);
> +		csi->vc_cached = false;
> +	}
>  
> -	ret = v4l2_subdev_call(&csi->subdev, video, s_stream, 0);
> +	video_device_pipeline_stop(&ctx->vdev);
> +
> +	ret = v4l2_subdev_disable_streams(&csi->subdev,
> +					  TI_CSI2RX_PAD_FIRST_SOURCE + ctx->idx,
> +					  BIT(0));
>  	if (ret)
>  		dev_err(csi->dev, "Failed to stop subdev stream\n");
>  
> +	mutex_unlock(&csi->mutex);
> +
>  	ti_csi2rx_stop_dma(ctx);
>  	ti_csi2rx_cleanup_buffers(ctx, VB2_BUF_STATE_ERROR);
>  }
> @@ -1101,74 +1102,151 @@ static int ti_csi2rx_sd_set_fmt(struct v4l2_subdev *sd,
>  	fmt = v4l2_subdev_state_get_format(state, format->pad, format->stream);
>  	*fmt = format->format;
>  
> -	fmt = v4l2_subdev_state_get_format(state, TI_CSI2RX_PAD_FIRST_SOURCE,
> -					   format->stream);
> +	fmt = v4l2_subdev_state_get_opposite_stream_format(state, format->pad,
> +							   format->stream);
> +	if (!fmt)
> +		return -EINVAL;
> +
>  	*fmt = format->format;
>  
>  	return 0;
>  }
>  
> -static int ti_csi2rx_sd_init_state(struct v4l2_subdev *sd,
> -				   struct v4l2_subdev_state *state)
> +static int _ti_csi2rx_sd_set_routing(struct v4l2_subdev *sd,
> +				     struct v4l2_subdev_state *state,
> +				     struct v4l2_subdev_krouting *routing)
>  {
> -	struct v4l2_mbus_framefmt *fmt;
> +	int ret;
>  
> -	fmt = v4l2_subdev_state_get_format(state, TI_CSI2RX_PAD_SINK);
> -	*fmt = ti_csi2rx_default_fmt;
> +	const struct v4l2_mbus_framefmt format = {
> +		.width = 640,
> +		.height = 480,
> +		.code = MEDIA_BUS_FMT_UYVY8_1X16,
> +		.field = V4L2_FIELD_NONE,
> +		.colorspace = V4L2_COLORSPACE_SRGB,
> +		.ycbcr_enc = V4L2_YCBCR_ENC_601,
> +		.quantization = V4L2_QUANTIZATION_LIM_RANGE,
> +		.xfer_func = V4L2_XFER_FUNC_SRGB,
> +	};
>  
> -	fmt = v4l2_subdev_state_get_format(state, TI_CSI2RX_PAD_FIRST_SOURCE);
> -	*fmt = ti_csi2rx_default_fmt;
> +	ret = v4l2_subdev_routing_validate(sd, routing,
> +					   V4L2_SUBDEV_ROUTING_ONLY_1_TO_1 |
> +					   V4L2_SUBDEV_ROUTING_NO_SOURCE_MULTIPLEXING);
>  
> -	return 0;
> +	if (ret)
> +		return ret;
> +
> +	/* Only stream ID 0 allowed on source pads */
> +	for (unsigned int i = 0; i < routing->num_routes; ++i) {
> +		const struct v4l2_subdev_route *route = &routing->routes[i];
> +
> +		if (route->source_stream != 0)
> +			return -EINVAL;
> +	}
> +
> +	ret = v4l2_subdev_set_routing_with_fmt(sd, state, routing, &format);
> +
> +	return ret;
> +}
> +
> +static int ti_csi2rx_sd_set_routing(struct v4l2_subdev *sd,
> +				    struct v4l2_subdev_state *state,
> +				    enum v4l2_subdev_format_whence which,
> +				    struct v4l2_subdev_krouting *routing)
> +{
> +	struct ti_csi2rx_dev *csi = to_csi2rx_dev(sd);
> +
> +	if (csi->enable_count > 0)
> +		return -EBUSY;
> +
> +	return _ti_csi2rx_sd_set_routing(sd, state, routing);
> +}
> +
> +static int ti_csi2rx_sd_init_state(struct v4l2_subdev *sd,
> +				   struct v4l2_subdev_state *state)
> +{
> +	struct v4l2_subdev_route routes[] = { {
> +		.sink_pad = 0,
> +		.sink_stream = 0,
> +		.source_pad = TI_CSI2RX_PAD_FIRST_SOURCE,
> +		.source_stream = 0,
> +		.flags = V4L2_SUBDEV_ROUTE_FL_ACTIVE,
> +	} };
> +
> +	struct v4l2_subdev_krouting routing = {
> +		.num_routes = 1,
> +		.routes = routes,
> +	};
> +
> +	/* Initialize routing to single route to the fist source pad */
> +	return _ti_csi2rx_sd_set_routing(sd, state, &routing);
>  }
>  
> -static int ti_csi2rx_sd_s_stream(struct v4l2_subdev *sd, int enable)
> +static int ti_csi2rx_sd_enable_streams(struct v4l2_subdev *sd,
> +				       struct v4l2_subdev_state *state,
> +				       u32 pad, u64 streams_mask)
>  {
>  	struct ti_csi2rx_dev *csi = to_csi2rx_dev(sd);
> +	struct media_pad *remote_pad;
> +	u64 sink_streams;
>  	int ret = 0;
>  
> +	remote_pad = media_entity_remote_source_pad_unique(&csi->subdev.entity);
> +	if (!remote_pad)
> +		return -ENODEV;
> +	sink_streams = v4l2_subdev_state_xlate_streams(state, pad,
> +						       TI_CSI2RX_PAD_SINK,
> +						       &streams_mask);
> +
> +	ret = v4l2_subdev_enable_streams(csi->source, remote_pad->index,
> +					 sink_streams);
> +	if (ret)
> +		return ret;
> +
>  	mutex_lock(&csi->mutex);
> +	csi->enable_count++;
> +	mutex_unlock(&csi->mutex);
>  
> -	if (enable) {
> -		if (csi->enable_count > 0) {
> -			csi->enable_count++;
> -			goto out;
> -		}
> +	return 0;
> +}
>  
> -		ret = v4l2_subdev_call(csi->source, video, s_stream, 1);
> -		if (ret)
> -			goto out;
> +static int ti_csi2rx_sd_disable_streams(struct v4l2_subdev *sd,
> +					struct v4l2_subdev_state *state,
> +					u32 pad, u64 streams_mask)
> +{
> +	struct ti_csi2rx_dev *csi = to_csi2rx_dev(sd);
> +	struct media_pad *remote_pad;
> +	u64 sink_streams;
> +	int ret = 0;
>  
> -		csi->enable_count++;
> -	} else {
> -		if (csi->enable_count == 0) {
> -			ret = -EINVAL;
> -			goto out;
> -		}
> +	remote_pad = media_entity_remote_source_pad_unique(&csi->subdev.entity);
> +	if (!remote_pad)
> +		return -ENODEV;
> +	sink_streams = v4l2_subdev_state_xlate_streams(state, pad,
> +						       TI_CSI2RX_PAD_SINK,
> +						       &streams_mask);
>  
> -		if (--csi->enable_count > 0)
> -			goto out;
> +	if (csi->enable_count == 0)
> +		return -EINVAL;
>  
> -		ret = v4l2_subdev_call(csi->source, video, s_stream, 0);
> -	}
> +	ret = v4l2_subdev_disable_streams(csi->source, remote_pad->index,
> +					  sink_streams);
> +	if (!ret)
> +		--csi->enable_count;
>  
> -out:
> -	mutex_unlock(&csi->mutex);
> -	return ret;
> +	return 0;
>  }
>  
>  static const struct v4l2_subdev_pad_ops ti_csi2rx_subdev_pad_ops = {
>  	.enum_mbus_code	= ti_csi2rx_enum_mbus_code,
> +	.set_routing = ti_csi2rx_sd_set_routing,
>  	.get_fmt = v4l2_subdev_get_fmt,
>  	.set_fmt = ti_csi2rx_sd_set_fmt,
> -};
> -
> -static const struct v4l2_subdev_video_ops ti_csi2rx_subdev_video_ops = {
> -	.s_stream = ti_csi2rx_sd_s_stream,
> +	.enable_streams = ti_csi2rx_sd_enable_streams,
> +	.disable_streams = ti_csi2rx_sd_disable_streams,
>  };
>  
>  static const struct v4l2_subdev_ops ti_csi2rx_subdev_ops = {
> -	.video = &ti_csi2rx_subdev_video_ops,
>  	.pad = &ti_csi2rx_subdev_pad_ops,
>  };
>  
> @@ -1345,7 +1423,7 @@ static int ti_csi2rx_v4l2_init(struct ti_csi2rx_dev *csi)
>  	v4l2_subdev_init(sd, &ti_csi2rx_subdev_ops);
>  	sd->internal_ops = &ti_csi2rx_internal_ops;
>  	sd->entity.function = MEDIA_ENT_F_VID_IF_BRIDGE;
> -	sd->flags = V4L2_SUBDEV_FL_HAS_DEVNODE;
> +	sd->flags = V4L2_SUBDEV_FL_HAS_DEVNODE | V4L2_SUBDEV_FL_STREAMS;
>  	strscpy(sd->name, dev_name(csi->dev), sizeof(sd->name));
>  	sd->dev = csi->dev;
>  	sd->entity.ops = &ti_csi2rx_subdev_entity_ops;


  reply	other threads:[~2025-09-25 12:17 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-11 10:28 [PATCH v7 00/16] media: cadence,ti: CSI2RX Multistream Support Rishikesh Donadkar
2025-09-11 10:28 ` [PATCH v7 01/16] media: ti: j721e-csi2rx: Remove word size alignment on frame width Rishikesh Donadkar
2025-09-22 11:56   ` Tomi Valkeinen
2025-10-17  4:53     ` Donadkar, Rishikesh
2025-09-11 10:28 ` [PATCH v7 02/16] dt-bindings: media: ti,j721e-csi2rx-shim: Support 32 dma chans Rishikesh Donadkar
2025-09-11 10:28 ` [PATCH v7 03/16] media: ti: j721e-csi2rx: separate out device and context Rishikesh Donadkar
2025-09-22 12:03   ` Tomi Valkeinen
2025-09-11 10:28 ` [PATCH v7 04/16] media: ti: j721e-csi2rx: prepare SHIM code for multiple contexts Rishikesh Donadkar
2025-09-22 12:03   ` Tomi Valkeinen
2025-09-11 10:28 ` [PATCH v7 05/16] media: ti: j721e-csi2rx: allocate DMA channel based on context index Rishikesh Donadkar
2025-09-22 12:04   ` Tomi Valkeinen
2025-09-11 10:28 ` [PATCH v7 06/16] media: ti: j721e-csi2rx: add a subdev for the core device Rishikesh Donadkar
2025-09-22 12:38   ` Tomi Valkeinen
2025-11-06  9:53     ` Rishikesh Donadkar
2025-09-11 10:28 ` [PATCH v7 07/16] media: ti: j721e-csi2rx: get number of contexts from device tree Rishikesh Donadkar
2025-09-22 12:39   ` Tomi Valkeinen
2025-09-11 10:28 ` [PATCH v7 08/16] media: cadence: csi2rx: add get_frame_desc wrapper Rishikesh Donadkar
2025-09-22 12:49   ` Tomi Valkeinen
2025-09-11 10:28 ` [PATCH v7 09/16] media: ti: j721e-csi2rx: add support for processing virtual channels Rishikesh Donadkar
2025-09-22 13:30   ` Tomi Valkeinen
2025-11-06 10:35     ` Rishikesh Donadkar
2025-09-11 10:28 ` [PATCH v7 10/16] media: cadence: csi2rx: add multistream support Rishikesh Donadkar
2025-09-22 14:39   ` Tomi Valkeinen
2025-09-25 12:44   ` Tomi Valkeinen
2025-11-10 12:23     ` Rishikesh Donadkar
2025-11-11  9:46     ` Rishikesh Donadkar
2025-09-11 10:28 ` [PATCH v7 11/16] media: ti: j721e-csi2rx: " Rishikesh Donadkar
2025-09-25 12:17   ` Tomi Valkeinen [this message]
2025-10-07 11:52     ` Jai Luthra
2025-11-11 12:19       ` Rishikesh Donadkar
2025-09-11 10:28 ` [PATCH v7 12/16] media: ti: j721e-csi2rx: Submit all available buffers Rishikesh Donadkar
2025-09-11 10:28 ` [PATCH v7 13/16] media: ti: j721e-csi2rx: Change the drain architecture for multistream Rishikesh Donadkar
2025-09-11 10:28 ` [PATCH v7 14/16] media: cadence: csi2rx: Support runtime PM Rishikesh Donadkar
2025-09-11 10:28 ` [PATCH v7 15/16] media: ti: j721e-csi2rx: Support runtime suspend Rishikesh Donadkar
2025-09-11 10:28 ` [PATCH v7 16/16] media: ti: j721e-csi2rx: Support system suspend using pm_notifier Rishikesh Donadkar
2025-09-12  9:45 ` [PATCH v7 00/16] media: cadence,ti: CSI2RX Multistream Support Jai Luthra
2025-09-24  9:33 ` Tomi Valkeinen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4534a09b-7eef-4e61-835a-c70d07df3416@ideasonboard.com \
    --to=tomi.valkeinen@ideasonboard.com \
    --cc=changhuang.liang@starfivetech.com \
    --cc=conor+dt@kernel.org \
    --cc=devarsht@ti.com \
    --cc=devicetree@vger.kernel.org \
    --cc=hverkuil+cisco@kernel.org \
    --cc=hverkuil-cisco@xs4all.nl \
    --cc=jack.zhu@starfivetech.com \
    --cc=jai.luthra@ideasonboard.com \
    --cc=jai.luthra@linux.dev \
    --cc=krzk+dt@kernel.org \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=mripard@kernel.org \
    --cc=p.zabel@pengutronix.de \
    --cc=r-donadkar@ti.com \
    --cc=robh@kernel.org \
    --cc=s-jain1@ti.com \
    --cc=sakari.ailus@linux.intel.com \
    --cc=sjoerd@collabora.com \
    --cc=vigneshr@ti.com \
    --cc=y-abhilashchandra@ti.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®