From 3b2a98feac6b2a955135c13c28271549a60dc067 Mon Sep 17 00:00:00 2001 From: Liu Ying Date: Thu, 30 Aug 2012 09:39:59 +0800 Subject: [PATCH] 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 (cherry picked from commit c5d3731fa0880a65efafb4826d3722aacb79edd5) --- drivers/mxc/ipu3/ipu_common.c | 29 ++++++++++------------------- 1 file 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; } -- 2.39.5