drm/msm/dpu: Avoid ABBA deadlock between IRQ modules

Handling of the interrupt callback lists is done in dpu_core_irq.c,
under the "cb_lock" spinlock. When these operations results in the need
for enableing or disabling the IRQ in the hardware the code jumps to
dpu_hw_interrupts.c, which protects its operations with "irq_lock"
spinlock.

When an interrupt fires, dpu_hw_intr_dispatch_irq() inspects the
hardware state while holding the "irq_lock" spinlock and jumps to
dpu_core_irq_callback_handler() to invoke the registered handlers, which
traverses the callback list under the "cb_lock" spinlock.

As such, in the event that these happens concurrently we'll end up with
a deadlock.

Prior to '1c1e7763a6d4 ("drm/msm/dpu: simplify IRQ enabling/disabling")'
the enable/disable of the hardware interrupt was done outside the
"cb_lock" region, optimitically by using an atomic enable-counter for
each interrupt and an warning print if someone changed the list between
the atomic_read and the time the operation concluded.

Rather than re-introducing the large array of atomics, this change
embraces the fact that dpu_core_irq and dpu_hw_interrupts are deeply
entangled and make them share the single "irq_lock".

Following this step it's suggested that we squash the two parts into a
single irq handling thing.

Fixes: 1c1e7763a6d4 ("drm/msm/dpu: simplify IRQ enabling/disabling")
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Link: https://lore.kernel.org/r/20210611170003.3539059-1-bjorn.andersson@linaro.org
Signed-off-by: Rob Clark <robdclark@chromium.org>

1 files changed, 18 insertions, 2 deletions

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h
index 3c8b788d78e6..ac83c1159815 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h
@@ -45,7 +45,7 @@ struct dpu_hw_intr_ops {
 	 * @irq_idx:	Lookup irq index return from irq_idx_lookup
 	 * @return:	0 for success, otherwise failure
 	 */
-	int (*enable_irq)(
+	int (*enable_irq_locked)(
 			struct dpu_hw_intr *intr,
 			int irq_idx);
 
@@ -55,7 +55,7 @@ struct dpu_hw_intr_ops {
 	 * @irq_idx:	Lookup irq index return from irq_idx_lookup
 	 * @return:	0 for success, otherwise failure
 	 */
-	int (*disable_irq)(
+	int (*disable_irq_locked)(
 			struct dpu_hw_intr *intr,
 			int irq_idx);
 
@@ -100,6 +100,22 @@ struct dpu_hw_intr_ops {
 			struct dpu_hw_intr *intr,
 			int irq_idx,
 			bool clear);
+
+	/**
+	 * lock - take the IRQ lock
+	 * @intr:	HW interrupt handle
+	 * @return:	irq_flags for the taken spinlock
+	 */
+	unsigned long (*lock)(
+			struct dpu_hw_intr *intr);
+
+	/**
+	 * unlock - take the IRQ lock
+	 * @intr:	HW interrupt handle
+	 * @irq_flags:  the irq_flags returned from lock
+	 */
+	void (*unlock)(
+			struct dpu_hw_intr *intr, unsigned long irq_flags);
 };
 
 /**