[PATCH 0/5] IPU reset workaround changes

[PATCH 0/5] IPU reset workaround changes

[Sakari Ailus]

Hi folks,

Maybe I should have labelled these as RFC instead... this set reworks based on
what the ISYS is reset and how that affects UAPI.

It seems there are some issues on IPU7; at times (around once in a few hundred
cases) streaming with yavta results capturing a single frame only and an ISYS
power reset is needed to recover from that. No reboot (or driver reload) is
needed with these patches though.

Similarly with these, an ISYS reset is avoided in regular streaming cases for 
IPU7. 

Comments are welcome (as always).

Sakari Ailus (5):
  media: ipu6: Don't mark need_reset based on returned buffers
  media: ipu6: Tell about not being able to obtain frame descriptor
  media: ipu6: Prevent starting streaming if ISYS needs reset
  media: ipu6: Remove need_reset check from video device open
  media: ipu6: Return errors from stream stop and close

 drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c |  6 +-
 .../media/pci/intel/ipu6/ipu6-isys-queue.c    | 16 ++--
 .../media/pci/intel/ipu6/ipu6-isys-video.c    | 74 +++++++++++--------
 .../media/pci/intel/ipu6/ipu6-isys-video.h    |  4 +-
 4 files changed, 53 insertions(+), 47 deletions(-)


-- 
2.47.3

[PATCH 1/5] media: ipu6: Don't mark need_reset based on returned buffers

[Sakari Ailus]

The IPU7 firmware stops streaming without PIN_DATA_READY events for the
queued buffers, unlike IPU6 firmware which does queue such events to the
host. This effectively causes the need_reset field of struct ipu6_isys to
be always true after streaming, even though there's no actual problem.

Don't set need_reset field based on still-active buffers.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/pci/intel/ipu6/ipu6-isys-queue.c | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys-queue.c b/drivers/media/pci/intel/ipu6/ipu6-isys-queue.c
index 23d00e1f89e8..6596833e95d4 100644
--- a/drivers/media/pci/intel/ipu6/ipu6-isys-queue.c
+++ b/drivers/media/pci/intel/ipu6/ipu6-isys-queue.c
@@ -420,9 +420,7 @@ static int ipu6_isys_link_fmt_validate(struct ipu6_isys_queue *aq)
 static void return_buffers(struct ipu6_isys_queue *aq,
 			   enum vb2_buffer_state state)
 {
-	struct ipu6_isys_video *av = ipu6_isys_queue_to_video(aq);
 	struct ipu6_isys_buffer *ib;
-	bool need_reset = false;
 	unsigned long flags;
 
 	spin_lock_irqsave(&aq->lock, flags);
@@ -458,18 +456,9 @@ static void return_buffers(struct ipu6_isys_queue *aq,
 		vb2_buffer_done(vb, state);
 
 		spin_lock_irqsave(&aq->lock, flags);
-		need_reset = true;
 	}
 
 	spin_unlock_irqrestore(&aq->lock, flags);
-
-	if (need_reset) {
-		mutex_lock(&av->isys->mutex);
-		av->isys->need_reset = true;
-		mutex_unlock(&av->isys->mutex);
-		dev_warn(&av->isys->adev->auxdev.dev,
-			 "buffer list not empty, needs isys reset\n");
-	}
 }
 
 static void ipu6_isys_stream_cleanup(struct ipu6_isys_video *av)
-- 
2.47.3

[PATCH 2/5] media: ipu6: Tell about not being able to obtain frame descriptor

[Sakari Ailus]

Not being able to obtain a frame descriptor is a fatal error at this
point. The frame descriptor should be stored for the duration of the
streaming operation.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c b/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c
index 152545427930..3e9bfda2a8b9 100644
--- a/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c
+++ b/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c
@@ -749,8 +749,10 @@ static int ipu6_isys_csi2_disable_streams(struct v4l2_subdev *sd,
 	lockdep_assert_held(&csi2->isys->stream_mutex);
 
 	ret = ipu6_isys_get_frame_desc(remote_sd, remote_pad->index, &desc);
-	if (ret)
-		return ret;
+	if (ret) {
+		dev_err(sd->dev, "cannot obtain frame descriptor\n");
+		return 0;
+	}
 
 	sink_streams =
 		v4l2_subdev_state_xlate_streams(state, pad, CSI2_PAD_SINK,
-- 
2.47.3

Re: [PATCH 2/5] media: ipu6: Tell about not being able to obtain frame descriptor

[Antti Laakso]

On Thu, Oct 08, 2026 at 11:52:44AM +0300, Sakari Ailus wrote:
> Not being able to obtain a frame descriptor is a fatal error at this
> point. The frame descriptor should be stored for the duration of the
> streaming operation.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c b/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c
> index 152545427930..3e9bfda2a8b9 100644
> --- a/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c
> +++ b/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c
> @@ -749,8 +749,10 @@ static int ipu6_isys_csi2_disable_streams(struct v4l2_subdev *sd,
>  	lockdep_assert_held(&csi2->isys->stream_mutex);
>  
>  	ret = ipu6_isys_get_frame_desc(remote_sd, remote_pad->index, &desc);
> -	if (ret)
> -		return ret;
> +	if (ret) {
> +		dev_err(sd->dev, "cannot obtain frame descriptor\n");
> +		return 0;
> +	}

According to title this is about logging the failure, but now this also
masks the error. Should this tell about the error and take the cleanup
path?

>  
>  	sink_streams =
>  		v4l2_subdev_state_xlate_streams(state, pad, CSI2_PAD_SINK,
> -- 
> 2.47.3
> 

Re: [PATCH 2/5] media: ipu6: Tell about not being able to obtain frame descriptor

[Sakari Ailus]

Hi Antti,

On Fri, Oct 09, 2026 at 02:11:17PM +0300, Antti Laakso wrote:
> On Thu, Oct 08, 2026 at 11:52:44AM +0300, Sakari Ailus wrote:
> > Not being able to obtain a frame descriptor is a fatal error at this
> > point. The frame descriptor should be stored for the duration of the
> > streaming operation.
> > 
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > ---
> >  drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c b/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c
> > index 152545427930..3e9bfda2a8b9 100644
> > --- a/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c
> > +++ b/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c
> > @@ -749,8 +749,10 @@ static int ipu6_isys_csi2_disable_streams(struct v4l2_subdev *sd,
> >  	lockdep_assert_held(&csi2->isys->stream_mutex);
> >  
> >  	ret = ipu6_isys_get_frame_desc(remote_sd, remote_pad->index, &desc);
> > -	if (ret)
> > -		return ret;
> > +	if (ret) {
> > +		dev_err(sd->dev, "cannot obtain frame descriptor\n");
> > +		return 0;
> > +	}
> 
> According to title this is about logging the failure, but now this also
> masks the error. Should this tell about the error and take the cleanup
> path?

The problem here is that there's no way to do proper cleanup without frame
descriptors. I could return an error here to leave streaming status as-is.

The frame descriptors should be moved to sub-device state so there would be
no issue obtaining them anymore.

> 
> >  
> >  	sink_streams =
> >  		v4l2_subdev_state_xlate_streams(state, pad, CSI2_PAD_SINK,

-- 
Regards,

Sakari Ailus

[PATCH 3/5] media: ipu6: Prevent starting streaming if ISYS needs reset

[Sakari Ailus]

Fail starting streaming whenever ISYS needs to be reset to resume normal
operation.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/pci/intel/ipu6/ipu6-isys-queue.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys-queue.c b/drivers/media/pci/intel/ipu6/ipu6-isys-queue.c
index 6596833e95d4..978acbd72e63 100644
--- a/drivers/media/pci/intel/ipu6/ipu6-isys-queue.c
+++ b/drivers/media/pci/intel/ipu6/ipu6-isys-queue.c
@@ -481,6 +481,11 @@ static int start_streaming(struct vb2_queue *q, unsigned int count)
 		av->vdev.name, ipu6_isys_get_frame_width(av),
 		ipu6_isys_get_frame_height(av), pfmt->css_pixelformat);
 
+	scoped_guard(mutex, &av->isys->mutex) {
+		if (av->isys->need_reset)
+			return -EIO;
+	}
+
 	remote_pad = media_pad_remote_pad_unique(&av->pad);
 	if (IS_ERR(remote_pad)) {
 		dev_dbg(dev, "failed to get remote pad\n");
-- 
2.47.3

Re: [PATCH 3/5] media: ipu6: Prevent starting streaming if ISYS needs reset

[Antti Laakso]

On Thu, Oct 08, 2026 at 11:52:45AM +0300, Sakari Ailus wrote:
> Fail starting streaming whenever ISYS needs to be reset to resume normal
> operation.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  drivers/media/pci/intel/ipu6/ipu6-isys-queue.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys-queue.c b/drivers/media/pci/intel/ipu6/ipu6-isys-queue.c
> index 6596833e95d4..978acbd72e63 100644
> --- a/drivers/media/pci/intel/ipu6/ipu6-isys-queue.c
> +++ b/drivers/media/pci/intel/ipu6/ipu6-isys-queue.c
> @@ -481,6 +481,11 @@ static int start_streaming(struct vb2_queue *q, unsigned int count)
>  		av->vdev.name, ipu6_isys_get_frame_width(av),
>  		ipu6_isys_get_frame_height(av), pfmt->css_pixelformat);
>  
> +	scoped_guard(mutex, &av->isys->mutex) {
> +		if (av->isys->need_reset)
> +			return -EIO;

Shouldn't this goto out_return_buffers path as well?

> +	}
> +
>  	remote_pad = media_pad_remote_pad_unique(&av->pad);
>  	if (IS_ERR(remote_pad)) {
>  		dev_dbg(dev, "failed to get remote pad\n");
> -- 
> 2.47.3
> 

Re: [PATCH 3/5] media: ipu6: Prevent starting streaming if ISYS needs reset

[Sakari Ailus]

Hi Antti,

Thanks for the review.

On Fri, Oct 09, 2026 at 01:59:04PM +0300, Antti Laakso wrote:
> On Thu, Oct 08, 2026 at 11:52:45AM +0300, Sakari Ailus wrote:
> > Fail starting streaming whenever ISYS needs to be reset to resume normal
> > operation.
> > 
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > ---
> >  drivers/media/pci/intel/ipu6/ipu6-isys-queue.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys-queue.c b/drivers/media/pci/intel/ipu6/ipu6-isys-queue.c
> > index 6596833e95d4..978acbd72e63 100644
> > --- a/drivers/media/pci/intel/ipu6/ipu6-isys-queue.c
> > +++ b/drivers/media/pci/intel/ipu6/ipu6-isys-queue.c
> > @@ -481,6 +481,11 @@ static int start_streaming(struct vb2_queue *q, unsigned int count)
> >  		av->vdev.name, ipu6_isys_get_frame_width(av),
> >  		ipu6_isys_get_frame_height(av), pfmt->css_pixelformat);
> >  
> > +	scoped_guard(mutex, &av->isys->mutex) {
> > +		if (av->isys->need_reset)
> > +			return -EIO;
> 
> Shouldn't this goto out_return_buffers path as well?

Yes, I'll fix that for v2.

> 
> > +	}
> > +
> >  	remote_pad = media_pad_remote_pad_unique(&av->pad);
> >  	if (IS_ERR(remote_pad)) {
> >  		dev_dbg(dev, "failed to get remote pad\n");

-- 
Regards,

Sakari Ailus

[PATCH 4/5] media: ipu6: Remove need_reset check from video device open

[Sakari Ailus]

The ISYS requires power reset in some cases. Opening a video node is
however independent of that; allow opening it even if reset is needed.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 .../media/pci/intel/ipu6/ipu6-isys-video.c    | 19 +------------------
 1 file changed, 1 insertion(+), 18 deletions(-)

diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys-video.c b/drivers/media/pci/intel/ipu6/ipu6-isys-video.c
index 129016e57446..44541aff79b3 100644
--- a/drivers/media/pci/intel/ipu6/ipu6-isys-video.c
+++ b/drivers/media/pci/intel/ipu6/ipu6-isys-video.c
@@ -109,23 +109,6 @@ const struct ipu6_isys_pixelformat ipu6_isys_pfmts[] = {
 	  IPU6_FW_ISYS_FRAME_FORMAT_RAW16, true },
 };
 
-static int video_open(struct file *file)
-{
-	struct ipu6_isys_video *av = video_drvdata(file);
-	struct ipu6_isys *isys = av->isys;
-	struct ipu6_bus_device *adev = isys->adev;
-
-	mutex_lock(&isys->mutex);
-	if (isys->need_reset) {
-		mutex_unlock(&isys->mutex);
-		dev_warn(&adev->auxdev.dev, "isys power cycle required\n");
-		return -EIO;
-	}
-	mutex_unlock(&isys->mutex);
-
-	return v4l2_fh_open(file);
-}
-
 const struct ipu6_isys_pixelformat *
 ipu6_isys_get_isys_format(u32 pixelformat, u32 type)
 {
@@ -809,7 +792,7 @@ static const struct v4l2_file_operations isys_fops = {
 	.poll = vb2_fop_poll,
 	.unlocked_ioctl = video_ioctl2,
 	.mmap = vb2_fop_mmap,
-	.open = video_open,
+	.open = v4l2_fh_open,
 	.release = vb2_fop_release,
 };
 
-- 
2.47.3

[PATCH 5/5] media: ipu6: Return errors from stream stop and close

[Sakari Ailus]

If the firmware fails to start, stop or close a stream, either by telling
about it with an error or due to a timeout, it probably requires a reset
before being operational again. Mark isys needing a reset in such cases.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 .../media/pci/intel/ipu6/ipu6-isys-video.c    | 55 ++++++++++++++-----
 .../media/pci/intel/ipu6/ipu6-isys-video.h    |  4 +-
 2 files changed, 43 insertions(+), 16 deletions(-)

diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys-video.c b/drivers/media/pci/intel/ipu6/ipu6-isys-video.c
index 44541aff79b3..799a4be3160b 100644
--- a/drivers/media/pci/intel/ipu6/ipu6-isys-video.c
+++ b/drivers/media/pci/intel/ipu6/ipu6-isys-video.c
@@ -543,25 +543,31 @@ int ipu6_isys_start_stream_firmware(struct ipu6_isys_stream *stream,
 out_stream_close:
 	reinit_completion(&stream->stream_close_completion);
 
+	scoped_guard(mutex, &stream->isys->mutex)
+		stream->isys->need_reset = true;
+
 	retout = fw_ops->stream_close(stream->isys, stream->stream_handle);
 	if (retout < 0) {
 		dev_dbg(dev, "can't close stream (%d)\n", retout);
-		return retout;
+		return ret;
 	}
 
 	tout = wait_for_completion_timeout(&stream->stream_close_completion,
 					   IPU6_FW_CALL_TIMEOUT_JIFFIES);
-	if (!tout)
+	if (!tout) {
 		dev_err(dev, "stream close time out\n");
-	else if (stream->error)
+		ret = -ETIMEDOUT;
+	} else if (stream->error) {
 		dev_err(dev, "stream close error: %d\n", stream->error);
-	else
+		ret = -EINVAL;
+	} else {
 		dev_dbg(dev, "stream close complete\n");
+	}
 
 	return ret;
 }
 
-void ipu6_isys_stop_stream_firmware(struct ipu6_isys_stream *stream)
+int ipu6_isys_stop_stream_firmware(struct ipu6_isys_stream *stream)
 {
 	struct ipu6_bus_device *adev = stream->asd->isys->adev;
 	const struct ipu6_fw_isys_ops *fw_ops = adev->auxdrv_data->fw_ops;
@@ -573,20 +579,30 @@ void ipu6_isys_stop_stream_firmware(struct ipu6_isys_stream *stream)
 	ret = fw_ops->stream_flush(stream->isys, stream->stream_handle);
 	if (ret < 0) {
 		dev_err(dev, "can't stop stream (%d)\n", ret);
-		return;
+		return -EBUSY;
 	}
 
 	tout = wait_for_completion_timeout(&stream->stream_stop_completion,
 					   IPU6_FW_CALL_TIMEOUT_JIFFIES);
-	if (!tout)
+	if (!tout) {
 		dev_warn(dev, "stream stop time out\n");
-	else if (stream->error)
+		ret = -ETIMEDOUT;
+	} else if (stream->error) {
 		dev_warn(dev, "stream stop error: %d\n", stream->error);
-	else
+		ret = -EINVAL;
+	} else{
 		dev_dbg(dev, "stop stream: complete\n");
+	}
+
+	if (ret) {
+		scoped_guard(mutex, &stream->isys->mutex)
+			stream->isys->need_reset = true;
+	}
+
+	return ret;
 }
 
-void ipu6_isys_close_stream_firmware(struct ipu6_isys_stream *stream)
+int ipu6_isys_close_stream_firmware(struct ipu6_isys_stream *stream)
 {
 	struct ipu6_bus_device *adev = stream->asd->isys->adev;
 	const struct ipu6_fw_isys_ops *fw_ops = adev->auxdrv_data->fw_ops;
@@ -599,22 +615,33 @@ void ipu6_isys_close_stream_firmware(struct ipu6_isys_stream *stream)
 	ret = fw_ops->stream_close(stream->isys, stream->stream_handle);
 	if (ret < 0) {
 		dev_err(dev, "can't close stream (%d)\n", ret);
-		return;
+		goto out_cleanup;
 	}
 
 	tout = wait_for_completion_timeout(&stream->stream_close_completion,
 					   IPU6_FW_CALL_TIMEOUT_JIFFIES);
-	if (!tout)
+	if (!tout) {
 		dev_warn(dev, "stream close time out\n");
-	else if (stream->error)
+		ret = -ETIMEDOUT;
+	} else if (stream->error) {
 		dev_warn(dev, "stream close error: %d\n", stream->error);
-	else
+		ret = -EINVAL;
+	} else {
 		dev_dbg(dev, "close stream: complete\n");
+	}
 
+out_cleanup:
 	scoped_guard(spinlock_irqsave, &stream->isys->streams_lock) {
 		stream->isys->streams_by_handle[stream->stream_handle] = NULL;
 		csi2->streams_by_vc[stream->vc] = NULL;
 	}
+
+	if (ret) {
+		scoped_guard(mutex, &stream->isys->mutex)
+			stream->isys->need_reset = true;
+	}
+
+	return ret;
 }
 
 struct ipu6_isys_stream *
diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys-video.h b/drivers/media/pci/intel/ipu6/ipu6-isys-video.h
index 2821b9b1b943..2ab6f32716ad 100644
--- a/drivers/media/pci/intel/ipu6/ipu6-isys-video.h
+++ b/drivers/media/pci/intel/ipu6/ipu6-isys-video.h
@@ -99,8 +99,8 @@ int ipu6_isys_fw_pins_prepare(struct ipu6_isys_stream *stream,
 int ipu6_isys_start_stream_firmware(struct ipu6_isys_stream *stream,
 				    struct ipu6_isys_buffer_list *bl,
 				    struct v4l2_mbus_frame_desc *desc);
-void ipu6_isys_stop_stream_firmware(struct ipu6_isys_stream *stream);
-void ipu6_isys_close_stream_firmware(struct ipu6_isys_stream *stream);
+int ipu6_isys_stop_stream_firmware(struct ipu6_isys_stream *stream);
+int ipu6_isys_close_stream_firmware(struct ipu6_isys_stream *stream);
 struct ipu6_isys_stream *
 ipu6_isys_find_stream_firmware(struct ipu6_isys_csi2 *csi2, u8 vc);
 void ipu6_isys_free_stream_firmware(struct ipu6_isys_stream *stream);
-- 
2.47.3