[PATCH v2 0/2] media: nxp: imx8-isi: fix streaming cleanup on release

[PATCH v2 0/2] media: nxp: imx8-isi: fix streaming cleanup on release

[Laurent Pinchart]

Hello,

This patch series fixes an issue initially reported by Richard in [1],
with a proposed fix. I've recommended an alternative approach, and gave
it a try.

Patch 1/2 fixes streaming cleanup on release. Patch 2/2 then cleans up
the driver a bit by dropping an unneeded structure field.

The changes have been tested by opening and closing the video device
while streaming in another process. I have also tested suspend/resume
during streaming.

[1] https://lore.kernel.org/linux-media/20250709-imx8-isi-release-fix-v1-1-c47c659ce1a6@linux.dev/

Laurent Pinchart (1):
  media: nxp: imx8-isi: Drop mxc_isi_video.is_streaming field

Richard Leitner (1):
  media: nxp: imx8-isi: Fix streaming cleanup on release

 .../platform/nxp/imx8-isi/imx8-isi-core.h     |   3 +-
 .../platform/nxp/imx8-isi/imx8-isi-video.c    | 156 +++++++-----------
 2 files changed, 57 insertions(+), 102 deletions(-)


base-commit: 078f1a7eb48eef9b3cb78bcd2254356f3a332358
-- 
Regards,

Laurent Pinchart

[PATCH v2 1/2] media: nxp: imx8-isi: Fix streaming cleanup on release

[Laurent Pinchart]

From: Richard Leitner <richard.leitner@linux.dev>

The current implementation unconditionally calls
mxc_isi_video_cleanup_streaming() in mxc_isi_video_release(). This can
lead to situations where any release call (like from a simple
"v4l2-ctl -l") may release a currently streaming queue when called on
such a device.

This is reproducible on an i.MX8MP board by streaming from an ISI
capture device using gstreamer:

	gst-launch-1.0 -v v4l2src device=/dev/videoX ! \
	    video/x-raw,format=GRAY8,width=1280,height=800,framerate=1/120 ! \
	    fakesink

While this stream is running, querying the caps of the same device
provokes the error state:

	v4l2-ctl -l -d /dev/videoX

This results in the following trace:

[  155.452152] ------------[ cut here ]------------
[  155.452163] WARNING: CPU: 0 PID: 1708 at drivers/media/platform/nxp/imx8-isi/imx8-isi-pipe.c:713 mxc_isi_pipe_irq_handler+0x19c/0x1b0 [imx8_isi]
[  157.004248] Modules linked in: cfg80211 rpmsg_ctrl rpmsg_char rpmsg_tty virtio_rpmsg_bus rpmsg_ns rpmsg_core rfkill nft_ct nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 nf_tables mcp251x6
[  157.053499] CPU: 0 UID: 0 PID: 1708 Comm: python3 Not tainted 6.15.4-00114-g1f61ca5cad76 #1 PREEMPT
[  157.064369] Hardware name: imx8mp_board_01 (DT)
[  157.068205] pstate: 400000c5 (nZcv daIF -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[  157.075169] pc : mxc_isi_pipe_irq_handler+0x19c/0x1b0 [imx8_isi]
[  157.081195] lr : mxc_isi_pipe_irq_handler+0x38/0x1b0 [imx8_isi]
[  157.087126] sp : ffff800080003ee0
[  157.090438] x29: ffff800080003ee0 x28: ffff0000c3688000 x27: 0000000000000000
[  157.097580] x26: 0000000000000000 x25: ffff0000c1e7ac00 x24: ffff800081b5ad50
[  157.104723] x23: 00000000000000d1 x22: 0000000000000000 x21: ffff0000c25e4000
[  157.111866] x20: 0000000060000200 x19: ffff80007a0608d0 x18: 0000000000000000
[  157.119008] x17: ffff80006a4e3000 x16: ffff800080000000 x15: 0000000000000000
[  157.126146] x14: 0000000000000000 x13: 0000000000000000 x12: 0000000000000000
[  157.133287] x11: 0000000000000040 x10: ffff0000c01445f0 x9 : ffff80007a053a38
[  157.140425] x8 : ffff0000c04004b8 x7 : 0000000000000000 x6 : 0000000000000000
[  157.147567] x5 : ffff0000c0400490 x4 : ffff80006a4e3000 x3 : ffff0000c25e4000
[  157.154706] x2 : 0000000000000000 x1 : ffff8000825c0014 x0 : 0000000060000200
[  157.161850] Call trace:
[  157.164296]  mxc_isi_pipe_irq_handler+0x19c/0x1b0 [imx8_isi] (P)
[  157.170319]  __handle_irq_event_percpu+0x58/0x218
[  157.175029]  handle_irq_event+0x54/0xb8
[  157.178867]  handle_fasteoi_irq+0xac/0x248
[  157.182968]  handle_irq_desc+0x48/0x68
[  157.186723]  generic_handle_domain_irq+0x24/0x38
[  157.191346]  gic_handle_irq+0x54/0x120
[  157.195098]  call_on_irq_stack+0x24/0x30
[  157.199027]  do_interrupt_handler+0x88/0x98
[  157.203212]  el0_interrupt+0x44/0xc0
[  157.206792]  __el0_irq_handler_common+0x18/0x28
[  157.211328]  el0t_64_irq_handler+0x10/0x20
[  157.215429]  el0t_64_irq+0x198/0x1a0
[  157.219009] ---[ end trace 0000000000000000 ]---

Address this issue by moving the streaming preparation and cleanup to
the vb2 .prepare_streaming() and .unprepare_streaming() operations. This
also simplifies the driver by allowing direct usage of the
vb2_ioctl_streamon() and vb2_ioctl_streamoff() helpers, and removal of
the manual cleanup from mxc_isi_video_release().

Signed-off-by: Richard Leitner <richard.leitner@linux.dev>
Co-developed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 .../platform/nxp/imx8-isi/imx8-isi-video.c    | 156 +++++++-----------
 1 file changed, 58 insertions(+), 98 deletions(-)

diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-video.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-video.c
index 8654150728a8..042b554d2775 100644
--- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-video.c
+++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-video.c
@@ -937,6 +937,49 @@ static void mxc_isi_video_init_channel(struct mxc_isi_video *video)
 	mxc_isi_channel_set_output_format(pipe, video->fmtinfo, &video->pix);
 }
 
+static int mxc_isi_vb2_prepare_streaming(struct vb2_queue *q)
+{
+	struct mxc_isi_video *video = vb2_get_drv_priv(q);
+	struct media_device *mdev = &video->pipe->isi->media_dev;
+	struct media_pipeline *pipe;
+	int ret;
+
+	/* Get a pipeline for the video node and start it. */
+	scoped_guard(mutex, &mdev->graph_mutex) {
+		ret = mxc_isi_pipe_acquire(video->pipe,
+					   &mxc_isi_video_frame_write_done);
+		if (ret)
+			return ret;
+
+		pipe = media_entity_pipeline(&video->vdev.entity)
+		     ? : &video->pipe->pipe;
+
+		ret = __video_device_pipeline_start(&video->vdev, pipe);
+		if (ret)
+			goto err_release;
+	}
+
+	/* Verify that the video format matches the output of the subdev. */
+	ret = mxc_isi_video_validate_format(video);
+	if (ret)
+		goto err_stop;
+
+	/* Allocate buffers for discard operation. */
+	ret = mxc_isi_video_alloc_discard_buffers(video);
+	if (ret)
+		goto err_stop;
+
+	video->is_streaming = true;
+
+	return 0;
+
+err_stop:
+	video_device_pipeline_stop(&video->vdev);
+err_release:
+	mxc_isi_pipe_release(video->pipe);
+	return ret;
+}
+
 static int mxc_isi_vb2_start_streaming(struct vb2_queue *q, unsigned int count)
 {
 	struct mxc_isi_video *video = vb2_get_drv_priv(q);
@@ -985,13 +1028,26 @@ static void mxc_isi_vb2_stop_streaming(struct vb2_queue *q)
 	mxc_isi_video_return_buffers(video, VB2_BUF_STATE_ERROR);
 }
 
+static void mxc_isi_vb2_unprepare_streaming(struct vb2_queue *q)
+{
+	struct mxc_isi_video *video = vb2_get_drv_priv(q);
+
+	mxc_isi_video_free_discard_buffers(video);
+	video_device_pipeline_stop(&video->vdev);
+	mxc_isi_pipe_release(video->pipe);
+
+	video->is_streaming = false;
+}
+
 static const struct vb2_ops mxc_isi_vb2_qops = {
 	.queue_setup		= mxc_isi_vb2_queue_setup,
 	.buf_init		= mxc_isi_vb2_buffer_init,
 	.buf_prepare		= mxc_isi_vb2_buffer_prepare,
 	.buf_queue		= mxc_isi_vb2_buffer_queue,
+	.prepare_streaming	= mxc_isi_vb2_prepare_streaming,
 	.start_streaming	= mxc_isi_vb2_start_streaming,
 	.stop_streaming		= mxc_isi_vb2_stop_streaming,
+	.unprepare_streaming	= mxc_isi_vb2_unprepare_streaming,
 };
 
 /* -----------------------------------------------------------------------------
@@ -1145,97 +1201,6 @@ static int mxc_isi_video_s_fmt(struct file *file, void *priv,
 	return 0;
 }
 
-static int mxc_isi_video_streamon(struct file *file, void *priv,
-				  enum v4l2_buf_type type)
-{
-	struct mxc_isi_video *video = video_drvdata(file);
-	struct media_device *mdev = &video->pipe->isi->media_dev;
-	struct media_pipeline *pipe;
-	int ret;
-
-	if (vb2_queue_is_busy(&video->vb2_q, file))
-		return -EBUSY;
-
-	/*
-	 * Get a pipeline for the video node and start it. This must be done
-	 * here and not in the queue .start_streaming() handler, so that
-	 * pipeline start errors can be reported from VIDIOC_STREAMON and not
-	 * delayed until subsequent VIDIOC_QBUF calls.
-	 */
-	mutex_lock(&mdev->graph_mutex);
-
-	ret = mxc_isi_pipe_acquire(video->pipe, &mxc_isi_video_frame_write_done);
-	if (ret) {
-		mutex_unlock(&mdev->graph_mutex);
-		return ret;
-	}
-
-	pipe = media_entity_pipeline(&video->vdev.entity) ? : &video->pipe->pipe;
-
-	ret = __video_device_pipeline_start(&video->vdev, pipe);
-	if (ret) {
-		mutex_unlock(&mdev->graph_mutex);
-		goto err_release;
-	}
-
-	mutex_unlock(&mdev->graph_mutex);
-
-	/* Verify that the video format matches the output of the subdev. */
-	ret = mxc_isi_video_validate_format(video);
-	if (ret)
-		goto err_stop;
-
-	/* Allocate buffers for discard operation. */
-	ret = mxc_isi_video_alloc_discard_buffers(video);
-	if (ret)
-		goto err_stop;
-
-	ret = vb2_streamon(&video->vb2_q, type);
-	if (ret)
-		goto err_free;
-
-	video->is_streaming = true;
-
-	return 0;
-
-err_free:
-	mxc_isi_video_free_discard_buffers(video);
-err_stop:
-	video_device_pipeline_stop(&video->vdev);
-err_release:
-	mxc_isi_pipe_release(video->pipe);
-	return ret;
-}
-
-static void mxc_isi_video_cleanup_streaming(struct mxc_isi_video *video)
-{
-	lockdep_assert_held(&video->lock);
-
-	if (!video->is_streaming)
-		return;
-
-	mxc_isi_video_free_discard_buffers(video);
-	video_device_pipeline_stop(&video->vdev);
-	mxc_isi_pipe_release(video->pipe);
-
-	video->is_streaming = false;
-}
-
-static int mxc_isi_video_streamoff(struct file *file, void *priv,
-				   enum v4l2_buf_type type)
-{
-	struct mxc_isi_video *video = video_drvdata(file);
-	int ret;
-
-	ret = vb2_ioctl_streamoff(file, priv, type);
-	if (ret)
-		return ret;
-
-	mxc_isi_video_cleanup_streaming(video);
-
-	return 0;
-}
-
 static int mxc_isi_video_enum_framesizes(struct file *file, void *priv,
 					 struct v4l2_frmsizeenum *fsize)
 {
@@ -1291,9 +1256,8 @@ static const struct v4l2_ioctl_ops mxc_isi_video_ioctl_ops = {
 	.vidioc_expbuf			= vb2_ioctl_expbuf,
 	.vidioc_prepare_buf		= vb2_ioctl_prepare_buf,
 	.vidioc_create_bufs		= vb2_ioctl_create_bufs,
-
-	.vidioc_streamon		= mxc_isi_video_streamon,
-	.vidioc_streamoff		= mxc_isi_video_streamoff,
+	.vidioc_streamon		= vb2_ioctl_streamon,
+	.vidioc_streamoff		= vb2_ioctl_streamoff,
 
 	.vidioc_enum_framesizes		= mxc_isi_video_enum_framesizes,
 
@@ -1332,10 +1296,6 @@ static int mxc_isi_video_release(struct file *file)
 	if (ret)
 		dev_err(video->pipe->isi->dev, "%s fail\n", __func__);
 
-	mutex_lock(&video->lock);
-	mxc_isi_video_cleanup_streaming(video);
-	mutex_unlock(&video->lock);
-
 	pm_runtime_put(video->pipe->isi->dev);
 	return ret;
 }
-- 
Regards,

Laurent Pinchart

[PATCH v2 2/2] media: nxp: imx8-isi: Drop mxc_isi_video.is_streaming field

[Laurent Pinchart]

The mxc_isi_video.is_streaming field is used to track the streaming
status of the video device. The same information is also tracked by the
videobuf2 queue. Drop the is_streaming field, and check the queue
streaming status instead.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h  | 3 +--
 drivers/media/platform/nxp/imx8-isi/imx8-isi-video.c | 8 ++------
 2 files changed, 3 insertions(+), 8 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 206995bedca4..d86f5ede0c0e 100644
--- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
+++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
@@ -202,9 +202,8 @@ struct mxc_isi_video {
 	struct video_device		vdev;
 	struct media_pad		pad;
 
-	/* Protects is_streaming, and the vdev and vb2_q operations */
+	/* Protects the vdev and vb2_q operations */
 	struct mutex			lock;
-	bool				is_streaming;
 
 	struct v4l2_pix_format_mplane	pix;
 	const struct mxc_isi_format_info *fmtinfo;
diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-video.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-video.c
index 042b554d2775..13682bf6e9f8 100644
--- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-video.c
+++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-video.c
@@ -969,8 +969,6 @@ static int mxc_isi_vb2_prepare_streaming(struct vb2_queue *q)
 	if (ret)
 		goto err_stop;
 
-	video->is_streaming = true;
-
 	return 0;
 
 err_stop:
@@ -1035,8 +1033,6 @@ static void mxc_isi_vb2_unprepare_streaming(struct vb2_queue *q)
 	mxc_isi_video_free_discard_buffers(video);
 	video_device_pipeline_stop(&video->vdev);
 	mxc_isi_pipe_release(video->pipe);
-
-	video->is_streaming = false;
 }
 
 static const struct vb2_ops mxc_isi_vb2_qops = {
@@ -1317,7 +1313,7 @@ void mxc_isi_video_suspend(struct mxc_isi_pipe *pipe)
 {
 	struct mxc_isi_video *video = &pipe->video;
 
-	if (!video->is_streaming)
+	if (!vb2_is_streaming(&video->vb2_q))
 		return;
 
 	mxc_isi_pipe_disable(pipe);
@@ -1348,7 +1344,7 @@ int mxc_isi_video_resume(struct mxc_isi_pipe *pipe)
 {
 	struct mxc_isi_video *video = &pipe->video;
 
-	if (!video->is_streaming)
+	if (!vb2_is_streaming(&video->vb2_q))
 		return 0;
 
 	mxc_isi_video_init_channel(video);
-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 0/2] media: nxp: imx8-isi: fix streaming cleanup on release

[Richard Leitner]

Hi Laurent,

On Thu, Aug 14, 2025 at 12:24:49AM +0300, Laurent Pinchart wrote:
> Hello,
> 
> This patch series fixes an issue initially reported by Richard in [1],
> with a proposed fix. I've recommended an alternative approach, and gave
> it a try.

Thanks for reviewing that patch of mine [1].
And thank you for providing this alternative approach. It looks way
cleaner now :-) I previously tought of a similar approach as yours,
but then opted for the "less invasive" one. So again what learned for
me. Thanks!

As I've never had such a situation and wasn't able to find something in
the docs [2] about it: Do you need/want a Reviewed or Tested-By tag on
patch 1/2 of this series? (Altough there's also a SoB from me before
your Co-Developed tag)

[2] https://www.kernel.org/doc/html/latest/process/submitting-patches.html

regards;rl

> 
> Patch 1/2 fixes streaming cleanup on release. Patch 2/2 then cleans up
> the driver a bit by dropping an unneeded structure field.
> 
> The changes have been tested by opening and closing the video device
> while streaming in another process. I have also tested suspend/resume
> during streaming.
> 
> [1] https://lore.kernel.org/linux-media/20250709-imx8-isi-release-fix-v1-1-c47c659ce1a6@linux.dev/
> 
> Laurent Pinchart (1):
>   media: nxp: imx8-isi: Drop mxc_isi_video.is_streaming field
> 
> Richard Leitner (1):
>   media: nxp: imx8-isi: Fix streaming cleanup on release
> 
>  .../platform/nxp/imx8-isi/imx8-isi-core.h     |   3 +-
>  .../platform/nxp/imx8-isi/imx8-isi-video.c    | 156 +++++++-----------
>  2 files changed, 57 insertions(+), 102 deletions(-)
> 
> 
> base-commit: 078f1a7eb48eef9b3cb78bcd2254356f3a332358
> -- 
> Regards,
> 
> Laurent Pinchart
> 

Re: [PATCH v2 0/2] media: nxp: imx8-isi: fix streaming cleanup on release

[Laurent Pinchart]

On Wed, Aug 13, 2025 at 11:46:03PM +0200, Richard Leitner wrote:
> Hi Laurent,
> 
> On Thu, Aug 14, 2025 at 12:24:49AM +0300, Laurent Pinchart wrote:
> > Hello,
> > 
> > This patch series fixes an issue initially reported by Richard in [1],
> > with a proposed fix. I've recommended an alternative approach, and gave
> > it a try.
> 
> Thanks for reviewing that patch of mine [1].
> And thank you for providing this alternative approach. It looks way
> cleaner now :-) I previously tought of a similar approach as yours,
> but then opted for the "less invasive" one. So again what learned for
> me. Thanks!
> 
> As I've never had such a situation and wasn't able to find something in
> the docs [2] about it: Do you need/want a Reviewed or Tested-By tag on
> patch 1/2 of this series? (Altough there's also a SoB from me before
> your Co-Developed tag)

Both tags are useful. Even if you're listes as the patch author, I've
made modifications, so your review and tests are valuable.

> [2] https://www.kernel.org/doc/html/latest/process/submitting-patches.html
> 
> regards;rl
> 
> > 
> > Patch 1/2 fixes streaming cleanup on release. Patch 2/2 then cleans up
> > the driver a bit by dropping an unneeded structure field.
> > 
> > The changes have been tested by opening and closing the video device
> > while streaming in another process. I have also tested suspend/resume
> > during streaming.
> > 
> > [1] https://lore.kernel.org/linux-media/20250709-imx8-isi-release-fix-v1-1-c47c659ce1a6@linux.dev/
> > 
> > Laurent Pinchart (1):
> >   media: nxp: imx8-isi: Drop mxc_isi_video.is_streaming field
> > 
> > Richard Leitner (1):
> >   media: nxp: imx8-isi: Fix streaming cleanup on release
> > 
> >  .../platform/nxp/imx8-isi/imx8-isi-core.h     |   3 +-
> >  .../platform/nxp/imx8-isi/imx8-isi-video.c    | 156 +++++++-----------
> >  2 files changed, 57 insertions(+), 102 deletions(-)
> > 
> > 
> > base-commit: 078f1a7eb48eef9b3cb78bcd2254356f3a332358

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 1/2] media: nxp: imx8-isi: Fix streaming cleanup on release

[Richard Leitner]

Hi Laurent,

On Thu, Aug 14, 2025 at 12:24:50AM +0300, Laurent Pinchart wrote:
> From: Richard Leitner <richard.leitner@linux.dev>
> 
> The current implementation unconditionally calls
> mxc_isi_video_cleanup_streaming() in mxc_isi_video_release(). This can
> lead to situations where any release call (like from a simple
> "v4l2-ctl -l") may release a currently streaming queue when called on
> such a device.
> 
> This is reproducible on an i.MX8MP board by streaming from an ISI
> capture device using gstreamer:
> 
> 	gst-launch-1.0 -v v4l2src device=/dev/videoX ! \
> 	    video/x-raw,format=GRAY8,width=1280,height=800,framerate=1/120 ! \
> 	    fakesink
> 
> While this stream is running, querying the caps of the same device
> provokes the error state:
> 
> 	v4l2-ctl -l -d /dev/videoX
> 
> This results in the following trace:
> 
> [  155.452152] ------------[ cut here ]------------
> [  155.452163] WARNING: CPU: 0 PID: 1708 at drivers/media/platform/nxp/imx8-isi/imx8-isi-pipe.c:713 mxc_isi_pipe_irq_handler+0x19c/0x1b0 [imx8_isi]
> [  157.004248] Modules linked in: cfg80211 rpmsg_ctrl rpmsg_char rpmsg_tty virtio_rpmsg_bus rpmsg_ns rpmsg_core rfkill nft_ct nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 nf_tables mcp251x6
> [  157.053499] CPU: 0 UID: 0 PID: 1708 Comm: python3 Not tainted 6.15.4-00114-g1f61ca5cad76 #1 PREEMPT
> [  157.064369] Hardware name: imx8mp_board_01 (DT)
> [  157.068205] pstate: 400000c5 (nZcv daIF -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> [  157.075169] pc : mxc_isi_pipe_irq_handler+0x19c/0x1b0 [imx8_isi]
> [  157.081195] lr : mxc_isi_pipe_irq_handler+0x38/0x1b0 [imx8_isi]
> [  157.087126] sp : ffff800080003ee0
> [  157.090438] x29: ffff800080003ee0 x28: ffff0000c3688000 x27: 0000000000000000
> [  157.097580] x26: 0000000000000000 x25: ffff0000c1e7ac00 x24: ffff800081b5ad50
> [  157.104723] x23: 00000000000000d1 x22: 0000000000000000 x21: ffff0000c25e4000
> [  157.111866] x20: 0000000060000200 x19: ffff80007a0608d0 x18: 0000000000000000
> [  157.119008] x17: ffff80006a4e3000 x16: ffff800080000000 x15: 0000000000000000
> [  157.126146] x14: 0000000000000000 x13: 0000000000000000 x12: 0000000000000000
> [  157.133287] x11: 0000000000000040 x10: ffff0000c01445f0 x9 : ffff80007a053a38
> [  157.140425] x8 : ffff0000c04004b8 x7 : 0000000000000000 x6 : 0000000000000000
> [  157.147567] x5 : ffff0000c0400490 x4 : ffff80006a4e3000 x3 : ffff0000c25e4000
> [  157.154706] x2 : 0000000000000000 x1 : ffff8000825c0014 x0 : 0000000060000200
> [  157.161850] Call trace:
> [  157.164296]  mxc_isi_pipe_irq_handler+0x19c/0x1b0 [imx8_isi] (P)
> [  157.170319]  __handle_irq_event_percpu+0x58/0x218
> [  157.175029]  handle_irq_event+0x54/0xb8
> [  157.178867]  handle_fasteoi_irq+0xac/0x248
> [  157.182968]  handle_irq_desc+0x48/0x68
> [  157.186723]  generic_handle_domain_irq+0x24/0x38
> [  157.191346]  gic_handle_irq+0x54/0x120
> [  157.195098]  call_on_irq_stack+0x24/0x30
> [  157.199027]  do_interrupt_handler+0x88/0x98
> [  157.203212]  el0_interrupt+0x44/0xc0
> [  157.206792]  __el0_irq_handler_common+0x18/0x28
> [  157.211328]  el0t_64_irq_handler+0x10/0x20
> [  157.215429]  el0t_64_irq+0x198/0x1a0
> [  157.219009] ---[ end trace 0000000000000000 ]---
> 
> Address this issue by moving the streaming preparation and cleanup to
> the vb2 .prepare_streaming() and .unprepare_streaming() operations. This
> also simplifies the driver by allowing direct usage of the
> vb2_ioctl_streamon() and vb2_ioctl_streamoff() helpers, and removal of
> the manual cleanup from mxc_isi_video_release().
> 
> Signed-off-by: Richard Leitner <richard.leitner@linux.dev>
> Co-developed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  .../platform/nxp/imx8-isi/imx8-isi-video.c    | 156 +++++++-----------
>  1 file changed, 58 insertions(+), 98 deletions(-)

Thanks for picking this up and improving it! The patch looks fine to me.
Furthermore I just verified it still works as expected for my use-case.
Therefore please feel free to add

Tested-By: Richard Leitner <richard.leitner@linux.dev> # i.MX8MP

> -- 
> Regards,
> 
> Laurent Pinchart
> 

regards;rl

Re: [PATCH v2 2/2] media: nxp: imx8-isi: Drop mxc_isi_video.is_streaming field

[Richard Leitner]

Hi Laurent,

On Thu, Aug 14, 2025 at 12:24:51AM +0300, Laurent Pinchart wrote:
> The mxc_isi_video.is_streaming field is used to track the streaming
> status of the video device. The same information is also tracked by the
> videobuf2 queue. Drop the is_streaming field, and check the queue
> streaming status instead.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h  | 3 +--
>  drivers/media/platform/nxp/imx8-isi/imx8-isi-video.c | 8 ++------
>  2 files changed, 3 insertions(+), 8 deletions(-)

Of course I also tested patch 2/2 from this series ;-).
Therefore please feel free to add

Tested-By: Richard Leitner <richard.leitner@linux.dev> # i.MX8MP

> 
> 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 206995bedca4..d86f5ede0c0e 100644
> --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
> +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
> @@ -202,9 +202,8 @@ struct mxc_isi_video {
>  	struct video_device		vdev;
>  	struct media_pad		pad;
>  
> -	/* Protects is_streaming, and the vdev and vb2_q operations */
> +	/* Protects the vdev and vb2_q operations */
>  	struct mutex			lock;
> -	bool				is_streaming;
>  
>  	struct v4l2_pix_format_mplane	pix;
>  	const struct mxc_isi_format_info *fmtinfo;
> diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-video.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-video.c
> index 042b554d2775..13682bf6e9f8 100644
> --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-video.c
> +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-video.c
> @@ -969,8 +969,6 @@ static int mxc_isi_vb2_prepare_streaming(struct vb2_queue *q)
>  	if (ret)
>  		goto err_stop;
>  
> -	video->is_streaming = true;
> -
>  	return 0;
>  
>  err_stop:
> @@ -1035,8 +1033,6 @@ static void mxc_isi_vb2_unprepare_streaming(struct vb2_queue *q)
>  	mxc_isi_video_free_discard_buffers(video);
>  	video_device_pipeline_stop(&video->vdev);
>  	mxc_isi_pipe_release(video->pipe);
> -
> -	video->is_streaming = false;
>  }
>  
>  static const struct vb2_ops mxc_isi_vb2_qops = {
> @@ -1317,7 +1313,7 @@ void mxc_isi_video_suspend(struct mxc_isi_pipe *pipe)
>  {
>  	struct mxc_isi_video *video = &pipe->video;
>  
> -	if (!video->is_streaming)
> +	if (!vb2_is_streaming(&video->vb2_q))
>  		return;
>  
>  	mxc_isi_pipe_disable(pipe);
> @@ -1348,7 +1344,7 @@ int mxc_isi_video_resume(struct mxc_isi_pipe *pipe)
>  {
>  	struct mxc_isi_video *video = &pipe->video;
>  
> -	if (!video->is_streaming)
> +	if (!vb2_is_streaming(&video->vb2_q))
>  		return 0;
>  
>  	mxc_isi_video_init_channel(video);
> -- 
> Regards,
> 
> Laurent Pinchart
>