diff mbox series

[04/17] media: atomisp: On streamoff wait for buffers owned by the CSS to be given back

Message ID 20221017085057.7483-5-hdegoede@redhat.com (mailing list archive)
State New, archived
Headers show
Series media: atomisp: Convert to videobuf2 | expand

Commit Message

Hans de Goede Oct. 17, 2022, 8:50 a.m. UTC
There is no guarantee that when we stop the pipeline all buffers owned
by the CSS are cleanly returned to the videobuf queue.

This is a problem with videobuf2 which will complain loudly when not
all buffers have been returned after the streamoff() queue op has
returned.

And this also allows moving a WARN() in the continuous mode path.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 .../staging/media/atomisp/pci/atomisp_fops.c  |  3 ++
 .../staging/media/atomisp/pci/atomisp_ioctl.c | 39 ++++++++-----------
 .../media/atomisp/pci/atomisp_subdev.h        |  3 +-
 3 files changed, 21 insertions(+), 24 deletions(-)
diff mbox series

Patch

diff --git a/drivers/staging/media/atomisp/pci/atomisp_fops.c b/drivers/staging/media/atomisp/pci/atomisp_fops.c
index d47b7f19125f..3b833cd5b423 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_fops.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_fops.c
@@ -222,6 +222,9 @@  static int atomisp_q_video_buffers_to_css(struct atomisp_sub_device *asd,
 	if (WARN_ON(css_pipe_id >= IA_CSS_PIPE_ID_NUM))
 		return -EINVAL;
 
+	if (pipe->stopping)
+		return -EINVAL;
+
 	while (pipe->buffers_in_css < ATOMISP_CSS_Q_DEPTH) {
 		struct videobuf_buffer *vb;
 
diff --git a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
index aefa7c07242a..bf5249b0d3bd 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
@@ -1754,6 +1754,22 @@  int atomisp_streamoff(struct file *file, void *fh, enum v4l2_buf_type type)
 		return -EINVAL;
 	}
 
+	/*
+	 * There is no guarantee that the buffers queued to / owned by the ISP
+	 * will properly be returned to the queue when stopping. Set a flag to
+	 * avoid new buffers getting queued and then wait for all the current
+	 * buffers to finish.
+	 */
+	pipe->stopping = true;
+	mutex_unlock(&isp->mutex);
+	/* wait max 1 second */
+	ret = wait_event_interruptible_timeout(pipe->capq.wait,
+					       pipe->buffers_in_css == 0, HZ);
+	mutex_lock(&isp->mutex);
+	pipe->stopping = false;
+	if (ret <= 0)
+		return ret ? ret : -ETIME;
+
 	/*
 	 * do only videobuf_streamoff for capture & vf pipes in
 	 * case of continuous capture
@@ -1768,29 +1784,6 @@  int atomisp_streamoff(struct file *file, void *fh, enum v4l2_buf_type type)
 								      0, 0, 0);
 			atomisp_freq_scaling(isp, ATOMISP_DFS_MODE_AUTO, false);
 		}
-		/*
-		 * Currently there is no way to flush buffers queued to css.
-		 * When doing videobuf_streamoff, active buffers will be
-		 * marked as VIDEOBUF_NEEDS_INIT. HAL will be able to use
-		 * these buffers again, and these buffers might be queued to
-		 * css more than once! Warn here, if HAL has not dequeued all
-		 * buffers back before calling streamoff.
-		 */
-		if (pipe->buffers_in_css != 0) {
-			WARN(1, "%s: buffers of vdev %s still in CSS!\n",
-			     __func__, pipe->vdev.name);
-
-			/*
-			 * Buffers remained in css maybe dequeued out in the
-			 * next stream on, while this will causes serious
-			 * issues as buffers already get invalid after
-			 * previous stream off.
-			 *
-			 * No way to flush buffers but to reset the whole css
-			 */
-			dev_warn(isp->dev, "Reset CSS to clean up css buffers.\n");
-			atomisp_css_flush(isp);
-		}
 
 		return videobuf_streamoff(&pipe->capq);
 	}
diff --git a/drivers/staging/media/atomisp/pci/atomisp_subdev.h b/drivers/staging/media/atomisp/pci/atomisp_subdev.h
index a1f4da35235d..65c2f8664f9d 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_subdev.h
+++ b/drivers/staging/media/atomisp/pci/atomisp_subdev.h
@@ -81,7 +81,8 @@  struct atomisp_video_pipe {
 
 	/* Store here the initial run mode */
 	unsigned int default_run_mode;
-
+	/* Set from streamoff to disallow queuing further buffers in CSS */
+	bool stopping;
 	unsigned int buffers_in_css;
 
 	/*