[PATCH v1 4/9] media: chips-media: wave5: ack the interrupt after dispatching it

[Jackson.lee]

From: Jackson Lee <jackson.lee@xxxxxxxxxxxxxxx>

W5_RET_QUEUE_CMD_DONE_INST is written by both the driver and the VPU:
the VPU sets the bit of every instance whose command completed, and the
driver clears the bits it has consumed. The driver does that as a
read-modify-write over a snapshot taken at the top of the handler, so
any bit the VPU adds in between is erased.

The VPU will not raise the same reason again while W5_VPU_VINT_REASON
still holds it, so keeping the reason set for the whole dispatch is what
keeps that register still. Acknowledging first opened the race instead:
the VPU could rewrite the register, and the write-back then dropped the
instance it had just added. That instance is never reported again -- its
queue count stays non-zero and no further completion arrives, so it
stops decoding for good while the others keep running.

Move both acknowledge writes past the dispatch loop. The re-read of
W5_VPU_VINT_REASON that tried to narrow the window is no longer needed.

Fixes: 9707a6254a8a ("media: chips-media: wave5: Add the v4l2 layer")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Jackson Lee <jackson.lee@xxxxxxxxxxxxxxx>
Signed-off-by: Nas Chung <nas.chung@xxxxxxxxxxxxxxx>
---
 .../platform/chips-media/wave5/wave5-vpu.c    | 28 +++++++++++--------
 1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu.c b/drivers/media/platform/chips-media/wave5/wave5-vpu.c
index 76d57c6b636a..3ab75f3d23d3 100644
--- a/drivers/media/platform/chips-media/wave5/wave5-vpu.c
+++ b/drivers/media/platform/chips-media/wave5/wave5-vpu.c
@@ -51,7 +51,6 @@ static void wave5_vpu_handle_irq(void *dev_id)
 	u32 seq_done;
 	u32 cmd_done;
 	u32 irq_reason;
-	u32 irq_subreason;
 	struct vpu_instance *inst, *tmp;
 	struct vpu_device *dev = dev_id;
 	int val;
@@ -60,8 +59,6 @@ static void wave5_vpu_handle_irq(void *dev_id)
 	irq_reason = wave5_vdi_read_register(dev, W5_VPU_VINT_REASON);
 	seq_done = wave5_vdi_read_register(dev, W5_RET_SEQ_DONE_INSTANCE_INFO);
 	cmd_done = wave5_vdi_read_register(dev, W5_RET_QUEUE_CMD_DONE_INST);
-	wave5_vdi_write_register(dev, W5_VPU_VINT_REASON_CLR, irq_reason);
-	wave5_vdi_write_register(dev, W5_VPU_VINT_CLEAR, 0x1);
 
 	spin_lock_irqsave(&dev->irq_spinlock, flags);
 	list_for_each_entry_safe(inst, tmp, &dev->instances, list) {
@@ -86,14 +83,10 @@ static void wave5_vpu_handle_irq(void *dev_id)
 		    irq_reason & BIT(INT_WAVE5_ENC_PIC)) {
 			if (cmd_done & BIT(inst->id)) {
 				cmd_done &= ~BIT(inst->id);
-				if (dev->irq >= 0) {
-					irq_subreason =
-						wave5_vdi_read_register(dev, W5_VPU_VINT_REASON);
-					if (!(irq_subreason & BIT(INT_WAVE5_DEC_PIC)))
-						wave5_vdi_write_register(dev,
-									 W5_RET_QUEUE_CMD_DONE_INST,
-									 cmd_done);
-				}
+				if (dev->irq >= 0)
+					wave5_vdi_write_register(dev,
+								 W5_RET_QUEUE_CMD_DONE_INST,
+								 cmd_done);
 				val = BIT(INT_WAVE5_DEC_PIC);
 				kfifo_in(&inst->irq_status, &val, sizeof(int));
 			}
@@ -101,6 +94,19 @@ static void wave5_vpu_handle_irq(void *dev_id)
 	}
 	spin_unlock_irqrestore(&dev->irq_spinlock, flags);
 
+	/*
+	 * Acknowledge only now that every instance bit this interrupt carried
+	 * has been consumed. W5_RET_QUEUE_CMD_DONE_INST is written by both
+	 * sides, and the VPU refuses to raise the same reason again while
+	 * W5_VPU_VINT_REASON still holds it. Clearing the reason first would
+	 * therefore let the VPU rewrite that register underneath the snapshot
+	 * taken above, and the read-modify-write in the loop would then drop
+	 * the instance bit the VPU had just added -- an instance whose
+	 * completion is never reported again.
+	 */
+	wave5_vdi_write_register(dev, W5_VPU_VINT_REASON_CLR, irq_reason);
+	wave5_vdi_write_register(dev, W5_VPU_VINT_CLEAR, 0x1);
+
 	if (dev->irq < 0)
 		up(&dev->irq_sem);
 }
-- 
2.43.0