media: rzv2h-ivc: Avoid double job scheduling

The scheduling of a new buffer transfer in the IVC driver is triggered
by two occurrences of the "frame completed" interrupt.

The first interrupt occurrence identifies when all image data have been
transferred to the ISP, the second occurrence identifies when the
post-transfer VBLANK has completed and a new buffer can be transferred.

Under heavy system load conditions the actual execution of the workqueue
item might be delayed and two items might happen to run concurrently,
leading to a new frame transfer being triggered while the previous one
has not yet finished.

This error condition is only visible because the driver maintains a
status variable that counts the number of interrupts since the last
transfer, and warns in case an IRQ happens before the counter has been
reset.

To ensure sequential execution of the worqueue items and avoid a double
buffer transfer to run concurrently, protect the whole function body
with the spinlock that so far was solely used to reset the counter and
inspect the interrupt counter variable at the beginning of the buffer
transfer function.

As soon as the ongoing transfer completes, the workqueue item will be
re-scheduled and will consume the pending buffer.

Cc: stable@vger.kernel.org
Fixes: f0b3984d821b ("media: platform: Add Renesas Input Video Control block driver")
Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
Signed-off-by: Hans Verkuil <hverkuil+cisco@kernel.org>

diff --git a/drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc-video.c b/drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc-video.c
index a22aee0fe1cf7617c27eb150a85e99b7226f902e..3580a57738a67f4b091274a21a8f1c42397795d3 100644 (file)
--- a/drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc-video.c
+++ b/drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc-video.c
@@ -149,6 +149,11 @@ static void rzv2h_ivc_transfer_buffer(struct work_struct *work)
                                             buffers.work);
        struct rzv2h_ivc_buf *buf;
 
+       guard(spinlock_irqsave)(&ivc->spinlock);
+
+       if (ivc->vvalid_ifp)
+               return;
+
        /* Setup buffers */
        scoped_guard(spinlock_irqsave, &ivc->buffers.lock) {
                buf = list_first_entry_or_null(&ivc->buffers.queue,
@@ -163,9 +168,7 @@ static void rzv2h_ivc_transfer_buffer(struct work_struct *work)
        buf->addr = vb2_dma_contig_plane_dma_addr(&buf->vb.vb2_buf, 0);
        rzv2h_ivc_write(ivc, RZV2H_IVC_REG_AXIRX_SADDL_P0, buf->addr);
 
-       scoped_guard(spinlock_irqsave, &ivc->spinlock) {
-               ivc->vvalid_ifp = 2;
-       }
+       ivc->vvalid_ifp = 2;
        rzv2h_ivc_write(ivc, RZV2H_IVC_REG_FM_FRCON, 0x1);
 }
 
@@ -200,7 +203,7 @@ static void rzv2h_ivc_buf_queue(struct vb2_buffer *vb)
        }
 
        scoped_guard(spinlock_irq, &ivc->spinlock) {
-               if (vb2_is_streaming(vb->vb2_queue) && !ivc->vvalid_ifp)
+               if (vb2_is_streaming(vb->vb2_queue))
                        queue_work(ivc->buffers.async_wq, &ivc->buffers.work);
        }
 }