ENGR00221370 IPUv3:Clean up IPUv3 interrupt handler

1) In the interrupt handler, we access sync interrupt
   control registers 2 times, and each time with spin
   lock being held and then released, which may cause
   potential racing on the registers. We see that
   as long as the racing happens with two displays
   enabled on the same IPU, one IPU display channel
   will lose EOF interrupt and it makes its fb's pan
   display ioctrl fail with timeout. This patch changes
   to hold the spin lock one time for the whole irq
   handler, as the handler should return quickly.
   Holding and releasing the spin lock unnecessarily
   may bring performance penalty as well.
2) We do not need to use spin_lock_irqsave() and
   spin_unlock_irqrestore() in the interrupt handler,
   as we are already in the hard irq context. Using
   spin_lock() and spin_unlock() is enough to protect
   the registers.
3) Clear an interrupt control bit as soon as its related
   handler finishes.

Signed-off-by: Liu Ying <Ying.Liu@freescale.com>

1 files changed, 10 insertions, 19 deletions

diff --git a/drivers/mxc/ipu3/ipu_common.c b/drivers/mxc/ipu3/ipu_common.c
index ae8b4ed93699..5954eb651ad5 100644
--- a/drivers/mxc/ipu3/ipu_common.c
+++ b/drivers/mxc/ipu3/ipu_common.c
@@ -2381,15 +2381,13 @@ static irqreturn_t ipu_irq_handler(int irq, void *desc)
 	uint32_t int_ctrl;
 	const int err_reg[] = { 5, 6, 9, 10, 0 };
 	const int int_reg[] = { 1, 2, 3, 4, 11, 12, 13, 14, 15, 0 };
-	unsigned long lock_flags;
-	uint32_t oneshot;
+
+	spin_lock(&ipu->spin_lock);
 
 	for (i = 0;; i++) {
 		if (err_reg[i] == 0)
 			break;
 
-		spin_lock_irqsave(&ipu->spin_lock, lock_flags);
-
 		int_stat = ipu_cm_read(ipu, IPU_INT_STAT(err_reg[i]));
 		int_stat &= ipu_cm_read(ipu, IPU_INT_CTRL(err_reg[i]));
 		if (int_stat) {
@@ -2402,41 +2400,34 @@ static irqreturn_t ipu_irq_handler(int irq, void *desc)
 			    ipu_cm_read(ipu, IPU_INT_CTRL(err_reg[i])) & ~int_stat;
 			ipu_cm_write(ipu, int_stat, IPU_INT_CTRL(err_reg[i]));
 		}
-
-		spin_unlock_irqrestore(&ipu->spin_lock, lock_flags);
 	}
 
 	for (i = 0;; i++) {
 		if (int_reg[i] == 0)
 			break;
-		spin_lock_irqsave(&ipu->spin_lock, lock_flags);
+
 		int_stat = ipu_cm_read(ipu, IPU_INT_STAT(int_reg[i]));
 		int_ctrl = ipu_cm_read(ipu, IPU_INT_CTRL(int_reg[i]));
 		int_stat &= int_ctrl;
 		ipu_cm_write(ipu, int_stat, IPU_INT_STAT(int_reg[i]));
-		spin_unlock_irqrestore(&ipu->spin_lock, lock_flags);
-		oneshot = 0;
 		while ((line = ffs(int_stat)) != 0) {
 			bit = --line;
 			int_stat &= ~(1UL << line);
 			line += (int_reg[i] - 1) * 32;
-			if (ipu->irq_list[line].flags & IPU_IRQF_ONESHOT)
-				oneshot |= 1UL << bit;
 			result |=
 			    ipu->irq_list[line].handler(line,
 						       ipu->irq_list[line].
 						       dev_id);
-		}
-		if (oneshot) {
-			spin_lock_irqsave(&ipu->spin_lock, lock_flags);
-			if ((~int_ctrl) & oneshot)
-				BUG();
-			int_ctrl &= ~oneshot;
-			ipu_cm_write(ipu, int_ctrl, IPU_INT_CTRL(int_reg[i]));
-			spin_unlock_irqrestore(&ipu->spin_lock, lock_flags);
+			if (ipu->irq_list[line].flags & IPU_IRQF_ONESHOT) {
+				int_ctrl &= ~(1UL << bit);
+				ipu_cm_write(ipu, int_ctrl,
+						IPU_INT_CTRL(int_reg[i]));
+			}
 		}
 	}
 
+	spin_unlock(&ipu->spin_lock);
+
 	return result;
 }