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 <hansg@kernel.org>
Reviewed-by: Andy Shevchenko <andy@kernel.org>
Link: https://lore.kernel.org/r/20250505210008.152659-6-hdegoede@redhat.com
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

diff --git a/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c b/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
index bc97fa2c374c78b84820708a8a3946dc66169b62..be5f37f4a6fd05c64d6698f0888f9fc710d067a3 100644 (file)
--- 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 57da7ddb1503343624391852a81c3f6c6a766236..c7aef066f209d41c5350dd3cc74ee2273ae6acc8 100644 (file)
--- 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 fecba2588e3839447ad1f17de387b5379e6de05e..bb8b2f2213b08f0329326e70d704e609bbffb321 100644 (file)
--- 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 bd1a198cda30e589687b5de53496f6be9b2759ea..e1d0168cb91d3c3a7e730c8877f5c59a8369e067 100644 (file)
--- 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.