From: Hans de Goede Date: Fri, 4 Jul 2025 09:21:34 +0000 (+0200) Subject: media: atomisp: Fix "stop stream timeout." error X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=0193cce58b196603427e925683b54f2f64ef031c;p=thirdparty%2Flinux.git media: atomisp: Fix "stop stream timeout." error Commit c7194b21809e ("media: atomisp: On streamoff wait for buffers owned by the CSS to be given back") added draining of the CSS buffer queue to the beginning of atomisp_stop_stream(). But it turns out that when telling the CSS to stop streaming it needs at least 1 buffer queued, because the CSS firmware waits for a frame to be completed before stopping and without buffers it cannot complete a frame. At the end of atomisp_stop_stream() it is always safe to return buffer ownership to the videobuf2-core. Either atomisp_css_stop() has successfully stopped the stream; or the atomisp_reset() later on which power-cycles the ISP will definitely have stopped the stream. Drop the draining of the CSS buffer queue to fix the "stop stream timeout." error and move the atomisp_flush_video_pipe() call after atomisp_reset(), passing false for the warn_on_css_frames flag since some buffers still being marked as owned by the CSS expected on stream off. Also increase the timeout in destroy_stream(), since this waits for the last frame to be completed this can take longer then 40 ms. When e.g. using a framerate of 15 fps, this could take 66ms, make the timeout 200 ms to be on the safe side. Signed-off-by: Hans de Goede Reviewed-by: Andy Shevchenko Link: https://lore.kernel.org/r/20250505210008.152659-6-hdegoede@redhat.com Signed-off-by: Mauro Carvalho Chehab --- diff --git a/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c b/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c index bc97fa2c374c7..be5f37f4a6fd0 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c +++ b/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c @@ -387,7 +387,7 @@ static int __destroy_stream(struct atomisp_sub_device *asd, } if (stream_env->stream_state == CSS_STREAM_STARTED) { - timeout = jiffies + msecs_to_jiffies(40); + timeout = jiffies + msecs_to_jiffies(200); while (1) { if (ia_css_stream_has_stopped(stream_env->stream)) break; diff --git a/drivers/staging/media/atomisp/pci/atomisp_fops.c b/drivers/staging/media/atomisp/pci/atomisp_fops.c index 57da7ddb15033..c7aef066f209d 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_fops.c +++ b/drivers/staging/media/atomisp/pci/atomisp_fops.c @@ -234,9 +234,6 @@ 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; - space = ATOMISP_CSS_Q_DEPTH - atomisp_buffers_in_css(pipe); while (space--) { struct ia_css_frame *frame; @@ -367,7 +364,7 @@ static void atomisp_buf_queue(struct vb2_buffer *vb) mutex_lock(&asd->isp->mutex); ret = atomisp_pipe_check(pipe, false); - if (ret || pipe->stopping) { + if (ret) { spin_lock_irqsave(&pipe->irq_lock, irqflags); atomisp_buffer_done(frame, VB2_BUF_STATE_ERROR); spin_unlock_irqrestore(&pipe->irq_lock, irqflags); diff --git a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c index fecba2588e383..bb8b2f2213b08 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c +++ b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c @@ -865,22 +865,6 @@ static void atomisp_stop_stream(struct atomisp_video_pipe *pipe, unsigned long flags; int ret; - /* - * 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_timeout(pipe->vb_queue.done_wq, - atomisp_buffers_in_css(pipe) == 0, HZ); - mutex_lock(&isp->mutex); - pipe->stopping = false; - if (ret == 0) - dev_warn(isp->dev, "Warning timeout waiting for CSS to return buffers\n"); - spin_lock_irqsave(&isp->lock, flags); asd->streaming = false; spin_unlock_irqrestore(&isp->lock, flags); @@ -890,8 +874,6 @@ static void atomisp_stop_stream(struct atomisp_video_pipe *pipe, atomisp_css_stop(asd, false); - atomisp_flush_video_pipe(pipe, state, true); - atomisp_subdev_cleanup_pending_events(asd); if (stop_sensor) { @@ -920,6 +902,8 @@ static void atomisp_stop_stream(struct atomisp_video_pipe *pipe, isp->saved_regs.i_control | MRFLD_PCI_I_CONTROL_SRSE_RESET_MASK); atomisp_reset(isp); + atomisp_flush_video_pipe(pipe, state, false); + /* Streams were destroyed by atomisp_css_stop(), recreate them. */ ret = atomisp_create_pipes_stream(&isp->asd); if (ret) diff --git a/drivers/staging/media/atomisp/pci/atomisp_subdev.h b/drivers/staging/media/atomisp/pci/atomisp_subdev.h index bd1a198cda30e..e1d0168cb91d3 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_subdev.h +++ b/drivers/staging/media/atomisp/pci/atomisp_subdev.h @@ -57,9 +57,6 @@ struct atomisp_video_pipe { /* Filled through atomisp_get_css_frame_info() on queue setup */ struct ia_css_frame_info frame_info; - /* Set from streamoff to disallow queuing further buffers in CSS */ - bool stopping; - /* * irq_lock is used to protect video buffer state change operations and * also to make activeq and capq operations atomic.