Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Frank Li <Frank.li@oss.nxp.com>
To: Guoniu Zhou <guoniu.zhou@oss.nxp.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Frank Li <Frank.Li@nxp.com>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	Fabio Estevam <festevam@gmail.com>,
	Christian Hemp <c.hemp@phytec.de>,
	Stefan Riedmueller <s.riedmueller@phytec.de>,
	Jacopo Mondi <jacopo@jmondi.org>,
	Dong Aisheng <aisheng.dong@nxp.com>,
	Guoniu Zhou <guoniu.zhou@nxp.com>,
	linux-media@vger.kernel.org, imx@lists.linux.dev,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, stable@vger.kernel.org
Subject: Re: [PATCH 2/5] media: nxp: imx8-isi: Fix per-stream reference counting for multiplexed streams
Date: Mon, 29 Jun 2026 09:55:16 -0500	[thread overview]
Message-ID: <akKHVAcjsotmIRad@SMW015318> (raw)
In-Reply-To: <20260629-isi-v1-2-deebfdb1b07b@oss.nxp.com>

On Mon, Jun 29, 2026 at 03:44:56PM +0800, Guoniu Zhou wrote:

subject: use per-stream state to fix ...

> The ISI crossbar fails to properly enable multiple streams from different
> virtual channels on the same input pad. Only the first stream gets enabled
> in hardware, subsequent streams are silently ignored.
>
> The driver uses a single enable_count per input to track the input state.
> When enable_count is non-zero, the code assumes the input is already active
> and skips calling v4l2_subdev_enable_streams() for additional streams:
>
>   Call 1: enable_streams(stream 0)
>     -> enable_count == 0, enable gasket and stream 0 in hardware
>     -> enable_count = 1
>
>   Call 2: enable_streams(stream 1)
>     -> enable_count == 1, skip hardware enable (BUG!)
>     -> enable_count = 2
>     -> stream 1 never gets enabled
>
> Similarly on disable, when enable_count reaches zero, ALL streams are
> disabled regardless of which streams are actually still active.
>
> Fix this by tracking per-stream state using:
> - enabled_streams (u64 bitmask): tracks which streams are currently enabled
> - enabled_count[] (array): per-stream reference counter to support the same
>   stream being enabled/disabled multiple times
>
> Now each stream is independently enabled/disabled in hardware based on the
> enabled_streams bitmask, while enabled_count[] provides reference counting
> for scenarios where the same stream is enabled multiple times,  such as
> duplicate cases in the ISI stream.
>
> Fixes: cf21f328fcaf ("media: nxp: Add i.MX8 ISI driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Guoniu Zhou <guoniu.zhou@oss.nxp.com>
> ---
>  .../media/platform/nxp/imx8-isi/imx8-isi-core.h    |   4 +-
>  .../platform/nxp/imx8-isi/imx8-isi-crossbar.c      | 121 +++++++++++++++++----
>  2 files changed, 104 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
> index 7547a6559d4c..bb2cfba27e20 100644
> --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
> +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
> @@ -185,7 +185,9 @@ struct mxc_isi_dma_buffer {
>  };
>
>  struct mxc_isi_input {
> -	unsigned int			enable_count;
> +	u64				enabled_streams;
> +	/* Counter per stream */
> +	unsigned int			*enabled_count;
>  };
>
>  struct mxc_isi_crossbar {
> diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
> index 29f14d30dbbb..a4a063c60c76 100644
> --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
> +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
> @@ -345,6 +345,8 @@ static int mxc_isi_crossbar_enable_streams(struct v4l2_subdev *sd,
>  	struct mxc_isi_crossbar *xbar = to_isi_crossbar(sd);
>  	struct v4l2_subdev *remote_sd;
>  	struct mxc_isi_input *input;
> +	u64 streams_to_enable;
> +	unsigned long stream;
>  	u64 sink_streams;
>  	u32 sink_pad;
>  	u32 remote_pad;
> @@ -358,30 +360,47 @@ static int mxc_isi_crossbar_enable_streams(struct v4l2_subdev *sd,
>
>  	input = &xbar->inputs[sink_pad];
>
> -	/*
> -	 * TODO: Track per-stream enable counts to support multiplexed
> -	 * streams.
> -	 */
> -	if (!input->enable_count) {
> +	if (!input->enabled_streams) {
>  		ret = mxc_isi_crossbar_gasket_enable(xbar, state, remote_sd,
>  						     remote_pad, sink_pad);
>  		if (ret)
>  			return ret;
> +	}
> +
> +	/*
> +	 * Track per-stream enable counts to support multiplexed streams.
> +	 * Only enable streams that are not already enabled.
> +	 */
> +	streams_to_enable = sink_streams & ~input->enabled_streams;
>
> +	if (streams_to_enable) {
>  		ret = v4l2_subdev_enable_streams(remote_sd, remote_pad,
> -						 sink_streams);
> +						 streams_to_enable);
>  		if (ret) {
>  			dev_err(xbar->isi->dev,
>  				"failed to enable streams 0x%llx on '%s':%u: %d\n",
> -				sink_streams, remote_sd->name, remote_pad, ret);
> -			mxc_isi_crossbar_gasket_disable(xbar, sink_pad);
> -			return ret;
> +				streams_to_enable, remote_sd->name, remote_pad, ret);
> +			goto err_gasket_disable;
>  		}
> +
> +		input->enabled_streams |= streams_to_enable;
>  	}
>
> -	input->enable_count++;
> +	/* Increment reference count for all requested streams */
> +	for (stream = 0; stream < xbar->num_sources; stream++) {
> +		if (!(sink_streams & BIT(stream)))
> +			continue;
> +
> +		input->enabled_count[stream]++;
> +	}
>
>  	return 0;
> +
> +err_gasket_disable:
> +	if (!input->enabled_streams)
> +		mxc_isi_crossbar_gasket_disable(xbar, sink_pad);
> +
> +	return ret;
>  }
>
>  static int mxc_isi_crossbar_disable_streams(struct v4l2_subdev *sd,
> @@ -391,6 +410,8 @@ static int mxc_isi_crossbar_disable_streams(struct v4l2_subdev *sd,
>  	struct mxc_isi_crossbar *xbar = to_isi_crossbar(sd);
>  	struct v4l2_subdev *remote_sd;
>  	struct mxc_isi_input *input;
> +	u64 streams_to_disable = 0;
> +	unsigned long stream;
>  	u64 sink_streams;
>  	u32 sink_pad;
>  	u32 remote_pad;
> @@ -404,19 +425,36 @@ static int mxc_isi_crossbar_disable_streams(struct v4l2_subdev *sd,
>
>  	input = &xbar->inputs[sink_pad];
>
> -	input->enable_count--;
> +	/*
> +	 * Decrease the enable count for each stream. Only disable streams
> +	 * whose count reaches zero.
> +	 */
> +	for (stream = 0; stream < xbar->num_sources; stream++) {
> +		if (!(sink_streams & BIT(stream)))
> +			continue;
>
> -	if (!input->enable_count) {
> -		ret = v4l2_subdev_disable_streams(remote_sd, remote_pad,
> -						  sink_streams);
> -		if (ret)
> -			dev_err(xbar->isi->dev,
> -				"failed to disable streams 0x%llx on '%s':%u: %d\n",
> -				sink_streams, remote_sd->name, remote_pad, ret);
> +		if (!(input->enabled_streams & BIT(stream)))
> +			continue;
>
> -		mxc_isi_crossbar_gasket_disable(xbar, sink_pad);
> +		if (--input->enabled_count[stream] == 0)
> +			streams_to_disable |= BIT(stream);
>  	}
>
> +	if (!streams_to_disable)
> +		return 0;
> +
> +	ret = v4l2_subdev_disable_streams(remote_sd, remote_pad,
> +					  streams_to_disable);
> +	if (ret)
> +		dev_err(xbar->isi->dev,
> +			"failed to disable streams 0x%llx on '%s':%u: %d\n",
> +			streams_to_disable, remote_sd->name, remote_pad, ret);
> +
> +	input->enabled_streams &= ~streams_to_disable;
> +
> +	if (!input->enabled_streams)
> +		mxc_isi_crossbar_gasket_disable(xbar, sink_pad);
> +
>  	return ret;
>  }
>
> @@ -447,6 +485,42 @@ static const struct media_entity_operations mxc_isi_cross_entity_ops = {
>   * Init & cleanup
>   */
>
> +static int mxc_isi_stream_counters_alloc(struct mxc_isi_crossbar *xbar)
> +{
> +	unsigned int i;
> +	int ret;
> +
> +	for (i = 0; i < xbar->num_sinks; ++i) {
> +		struct mxc_isi_input *input = &xbar->inputs[i];
> +
> +		input->enabled_count = kcalloc(xbar->num_sources,
> +					       sizeof(*input->enabled_count),
> +					       GFP_KERNEL);

kzalloc_objs();

Frank
> +		if (!input->enabled_count) {
> +			dev_err(xbar->isi->dev,
> +				"failed to alloc memory for ISI input(%d)\n", i);
> +			ret = -ENOMEM;
> +			goto err_free;
> +		}
> +	}
> +
> +	return 0;
> +
> +err_free:
> +	while (i--)
> +		kfree(xbar->inputs[i].enabled_count);
> +
> +	return ret;
> +}
> +
> +static void mxc_isi_stream_counters_free(struct mxc_isi_crossbar *xbar)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < xbar->num_sinks; ++i)
> +		kfree(xbar->inputs[i].enabled_count);
> +}
> +
>  int mxc_isi_crossbar_init(struct mxc_isi_dev *isi)
>  {
>  	struct mxc_isi_crossbar *xbar = &isi->crossbar;
> @@ -484,6 +558,10 @@ int mxc_isi_crossbar_init(struct mxc_isi_dev *isi)
>  		goto err_free;
>  	}
>
> +	ret = mxc_isi_stream_counters_alloc(xbar);
> +	if (ret)
> +		goto err_free;
> +
>  	for (i = 0; i < xbar->num_sinks; ++i)
>  		xbar->pads[i].flags = MEDIA_PAD_FL_SINK
>  				    | MEDIA_PAD_FL_MUST_CONNECT;
> @@ -492,7 +570,7 @@ int mxc_isi_crossbar_init(struct mxc_isi_dev *isi)
>
>  	ret = media_entity_pads_init(&sd->entity, num_pads, xbar->pads);
>  	if (ret)
> -		goto err_free;
> +		goto err_free_cnt;
>
>  	ret = v4l2_subdev_init_finalize(sd);
>  	if (ret < 0)
> @@ -502,6 +580,8 @@ int mxc_isi_crossbar_init(struct mxc_isi_dev *isi)
>
>  err_entity:
>  	media_entity_cleanup(&sd->entity);
> +err_free_cnt:
> +	mxc_isi_stream_counters_free(xbar);
>  err_free:
>  	kfree(xbar->pads);
>  	kfree(xbar->inputs);
> @@ -512,6 +592,7 @@ int mxc_isi_crossbar_init(struct mxc_isi_dev *isi)
>  void mxc_isi_crossbar_cleanup(struct mxc_isi_crossbar *xbar)
>  {
>  	v4l2_subdev_cleanup(&xbar->sd);
> +	mxc_isi_stream_counters_free(xbar);
>  	media_entity_cleanup(&xbar->sd.entity);
>  	kfree(xbar->pads);
>  	kfree(xbar->inputs);
>
> --
> 2.34.1
>
>


  reply	other threads:[~2026-06-29 14:55 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-29  7:44 [PATCH 0/5] imx8-isi: Bug fixes and format support enhancements Guoniu Zhou
2026-06-29  7:44 ` [PATCH 1/5] media: nxp: imx8-isi: Fix stream ID validation bypass in crossbar routing Guoniu Zhou
2026-06-29 14:33   ` Frank Li
2026-06-29  7:44 ` [PATCH 2/5] media: nxp: imx8-isi: Fix per-stream reference counting for multiplexed streams Guoniu Zhou
2026-06-29 14:55   ` Frank Li [this message]
2026-06-29  7:44 ` [PATCH 3/5] media: nxp: imx8-isi: Add 16-bit raw Bayer format support guoniu.zhou
2026-06-29 14:57   ` Frank Li
2026-06-29  7:44 ` [PATCH 4/5] media: nxp: imx8-isi: Correct color map between V4L2 and ISI Guoniu Zhou
2026-06-29 15:08   ` Frank Li
2026-06-29  7:44 ` [PATCH 5/5] media: nxp: imx8-isi: Add additional 32-bit RGB format support Guoniu Zhou
2026-06-29 15:10   ` Frank Li

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=akKHVAcjsotmIRad@SMW015318 \
    --to=frank.li@oss.nxp.com \
    --cc=Frank.Li@nxp.com \
    --cc=aisheng.dong@nxp.com \
    --cc=c.hemp@phytec.de \
    --cc=festevam@gmail.com \
    --cc=guoniu.zhou@nxp.com \
    --cc=guoniu.zhou@oss.nxp.com \
    --cc=imx@lists.linux.dev \
    --cc=jacopo@jmondi.org \
    --cc=kernel@pengutronix.de \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=s.hauer@pengutronix.de \
    --cc=s.riedmueller@phytec.de \
    --cc=stable@vger.kernel.org \
    /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