From: Hans de Goede Date: Fri, 30 Sep 2022 19:05:44 +0000 (+0100) Subject: media: atomisp: On streamoff wait for buffers owned by the CSS to be given back X-Git-Url: https://www.infradead.org/git/?a=commitdiff_plain;h=c7194b21809ec53db2f8ae5a5e2389ed774d2af6;p=users%2Fjedix%2Flinux-maple.git media: atomisp: On streamoff wait for buffers owned by the CSS to be given back 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 removing a WARN() in the continuous mode path. Reviewed-by: Andy Shevchenko Signed-off-by: Hans de Goede Signed-off-by: Mauro Carvalho Chehab --- 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 0ddb0ed42dd9..0fd162f61534 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c +++ b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c @@ -1752,6 +1752,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 ?: -ETIMEDOUT; + /* * do only videobuf_streamoff for capture & vf pipes in * case of continuous capture @@ -1766,29 +1782,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; /*