mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 0/2] media: qcom: iris: Fix IRQ synchronization during power-off
@ 2026-10-07 11:31 Vishnu Reddy
  2026-10-07 11:31 ` [PATCH 1/2] Revert "media: qcom: iris: use disable_irq() during power-off" Vishnu Reddy
  2026-10-07 11:31 ` [PATCH 2/2] media: qcom: iris: Wait for threaded IRQ completion during power-off Vishnu Reddy
  0 siblings, 2 replies; 5+ messages in thread
From: Vishnu Reddy @ 2026-10-07 11:31 UTC (permalink / raw)
  To: Vikash Garodia, Dikshita Agarwal, Abhinav Kumar,
	Bryan O'Donoghue, Mauro Carvalho Chehab, Konrad Dybcio,
	Dmitry Baryshkov, Hungyu Lin, Stefan Schmidt, Hans Verkuil
  Cc: Bryan O'Donoghue, linux-media, linux-arm-msm, linux-kernel,
	Vishnu Reddy, stable

Commit b9c2215bdedc9c ("media: qcom: iris: use disable_irq() during
power-off") changed the VPU power-off sequence to use disable_irq()
instead of disable_irq_nosync(). This was intended to wait for any
queued threaded IRQ handler to finish before gating VPU power.

However, iris_vpu_power_off() is called with core->lock held, and the
threaded IRQ handler (iris_hfi_isr_handler()) takes this same lock as
its first action. If the handler is queued when power-off begins,
disable_irq() blocks waiting for the handler, while the handler blocks
waiting for core->lock. This results in a deadlock.

This series reverts the commit that introduced the deadlock and
reintroduces the hardware protection safely.

The fix drops core->lock before calling disable_irq(), allowing the
pending ISR to acquire the lock and run to completion. To prevent race
conditions from concurrent power-off callers during this unlocked
window, an irq_disabling flag and a waitqueue are introduced to
serialize operations. Finally, disable_irq() is called before the
hardware is powered down, ensuring a safe hardware state.

Signed-off-by: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>
---
Vishnu Reddy (2):
      Revert "media: qcom: iris: use disable_irq() during power-off"
      media: qcom: iris: Wait for threaded IRQ completion during power-off

 drivers/media/platform/qcom/iris/iris_core.c       | 10 +++++-----
 drivers/media/platform/qcom/iris/iris_core.h       |  5 +++++
 drivers/media/platform/qcom/iris/iris_probe.c      |  9 +++++----
 drivers/media/platform/qcom/iris/iris_resources.c  | 16 ++++++++++++++++
 drivers/media/platform/qcom/iris/iris_resources.h  |  2 ++
 drivers/media/platform/qcom/iris/iris_vpu_common.c | 16 +++++++++++++---
 6 files changed, 46 insertions(+), 12 deletions(-)
---
base-commit: eea3fef32a9cf36abcb5975a5a594e4135a6b026
change-id: 20261004-fix-disable-irq-safely-15b2f712800b

Best regards,
--  
Vishnu Reddy <busanna.reddy@oss.qualcomm.com>


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/2] Revert "media: qcom: iris: use disable_irq() during power-off"
  2026-10-07 11:31 [PATCH 0/2] media: qcom: iris: Fix IRQ synchronization during power-off Vishnu Reddy
@ 2026-10-07 11:31 ` Vishnu Reddy
  2026-10-07 11:31 ` [PATCH 2/2] media: qcom: iris: Wait for threaded IRQ completion during power-off Vishnu Reddy
  1 sibling, 0 replies; 5+ messages in thread
From: Vishnu Reddy @ 2026-10-07 11:31 UTC (permalink / raw)
  To: Vikash Garodia, Dikshita Agarwal, Abhinav Kumar,
	Bryan O'Donoghue, Mauro Carvalho Chehab, Konrad Dybcio,
	Dmitry Baryshkov, Hungyu Lin, Stefan Schmidt, Hans Verkuil
  Cc: Bryan O'Donoghue, linux-media, linux-arm-msm, linux-kernel,
	Vishnu Reddy, stable

iris_vpu_power_off() is always called with core->lock held. The threaded
IRQ handler also requires core->lock to proceed. If the handler is woken
and queued around the same time power-off is entered, it cannot acquire
core->lock because it is held by the power-off caller. disable_irq() waits
for the handler to complete, resulting in a deadlock.

Fixes: b9c2215bdedc9c ("media: qcom: iris: use disable_irq() during power-off")
Cc: stable@vger.kernel.org
Signed-off-by: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>
---
 drivers/media/platform/qcom/iris/iris_vpu_common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/qcom/iris/iris_vpu_common.c b/drivers/media/platform/qcom/iris/iris_vpu_common.c
index 243bd845349f..1e183fc4e58c 100644
--- a/drivers/media/platform/qcom/iris/iris_vpu_common.c
+++ b/drivers/media/platform/qcom/iris/iris_vpu_common.c
@@ -240,7 +240,7 @@ void iris_vpu_power_off(struct iris_core *core)
 	iris_unset_icc_bw(core);
 
 	if (!iris_vpu_watchdog(core, core->intr_status))
-		disable_irq(core->irq);
+		disable_irq_nosync(core->irq);
 }
 
 int iris_vpu_power_on_controller(struct iris_core *core)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 2/2] media: qcom: iris: Wait for threaded IRQ completion during power-off
  2026-10-07 11:31 [PATCH 0/2] media: qcom: iris: Fix IRQ synchronization during power-off Vishnu Reddy
  2026-10-07 11:31 ` [PATCH 1/2] Revert "media: qcom: iris: use disable_irq() during power-off" Vishnu Reddy
@ 2026-10-07 11:31 ` Vishnu Reddy
  2026-10-07 17:29   ` Hungyu Lin
  1 sibling, 1 reply; 5+ messages in thread
From: Vishnu Reddy @ 2026-10-07 11:31 UTC (permalink / raw)
  To: Vikash Garodia, Dikshita Agarwal, Abhinav Kumar,
	Bryan O'Donoghue, Mauro Carvalho Chehab, Konrad Dybcio,
	Dmitry Baryshkov, Hungyu Lin, Stefan Schmidt, Hans Verkuil
  Cc: Bryan O'Donoghue, linux-media, linux-arm-msm, linux-kernel,
	Vishnu Reddy, stable

The VPU power-off path calls disable_irq_nosync() after powering down the
hardware, allowing a pending threaded IRQ handler to access powered-off
hardware. Switch to disable_irq() so that any in-flight threaded IRQ
handler completes before power is removed.

However, disable_irq() waits for the threaded IRQ handler to complete,
while the handler requires core->lock to proceed. Since the caller of
iris_vpu_power_off() holds core->lock, calling disable_irq() directly can
deadlock if the IRQ handler is queued and waiting for the same lock.
Release core->lock before calling disable_irq() and reacquire it after the
handler completes. Introduce an irq_disabling flag and waitqueue to
serialize concurrent power management operations while the core->lock is
released.

Fixes: bb8a95aa038e ("media: iris: implement power management")
Cc: stable@vger.kernel.org
Signed-off-by: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>
---
 drivers/media/platform/qcom/iris/iris_core.c       | 10 +++++-----
 drivers/media/platform/qcom/iris/iris_core.h       |  5 +++++
 drivers/media/platform/qcom/iris/iris_probe.c      |  9 +++++----
 drivers/media/platform/qcom/iris/iris_resources.c  | 16 ++++++++++++++++
 drivers/media/platform/qcom/iris/iris_resources.h  |  2 ++
 drivers/media/platform/qcom/iris/iris_vpu_common.c | 16 +++++++++++++---
 6 files changed, 46 insertions(+), 12 deletions(-)

diff --git a/drivers/media/platform/qcom/iris/iris_core.c b/drivers/media/platform/qcom/iris/iris_core.c
index d558475986ff0..f81fb6d42557d 100644
--- a/drivers/media/platform/qcom/iris/iris_core.c
+++ b/drivers/media/platform/qcom/iris/iris_core.c
@@ -17,7 +17,7 @@ void iris_core_deinit(struct iris_core *core)
 
 	ret = pm_runtime_resume_and_get(core->dev);
 
-	mutex_lock(&core->lock);
+	iris_acquire_core_lock(core);
 	if (core->state != IRIS_CORE_DEINIT) {
 		iris_fw_unload(core);
 
@@ -27,7 +27,7 @@ void iris_core_deinit(struct iris_core *core)
 		iris_hfi_queues_deinit(core);
 		core->state = IRIS_CORE_DEINIT;
 	}
-	mutex_unlock(&core->lock);
+	iris_release_core_lock(core);
 
 	if (!ret)
 		pm_runtime_put_sync(core->dev);
@@ -55,7 +55,7 @@ int iris_core_init(struct iris_core *core)
 	const struct vpu_ops *vpu_ops = core->iris_platform_data->vpu_ops;
 	int ret;
 
-	mutex_lock(&core->lock);
+	iris_acquire_core_lock(core);
 	if (core->state == IRIS_CORE_INIT) {
 		ret = 0;
 		goto exit;
@@ -96,7 +96,7 @@ int iris_core_init(struct iris_core *core)
 	if (ret)
 		goto error_unload_fw;
 
-	mutex_unlock(&core->lock);
+	iris_release_core_lock(core);
 
 	return iris_wait_for_system_response(core);
 
@@ -109,7 +109,7 @@ int iris_core_init(struct iris_core *core)
 error:
 	core->state = IRIS_CORE_DEINIT;
 exit:
-	mutex_unlock(&core->lock);
+	iris_release_core_lock(core);
 
 	return ret;
 }
diff --git a/drivers/media/platform/qcom/iris/iris_core.h b/drivers/media/platform/qcom/iris/iris_core.h
index 3e4cf652bf39a..0b1b9721e6f9a 100644
--- a/drivers/media/platform/qcom/iris/iris_core.h
+++ b/drivers/media/platform/qcom/iris/iris_core.h
@@ -10,6 +10,7 @@
 #include <linux/dma-mapping.h>
 #include <linux/pm_domain.h>
 #include <media/v4l2-device.h>
+#include <linux/wait.h>
 
 #include "iris_hfi_common.h"
 #include "iris_hfi_queue.h"
@@ -70,6 +71,8 @@ struct qcom_ubwc_cfg_data;
  * @message_queue: shared interface queue to receive responses from firmware
  * @debug_queue: shared interface queue to receive debug info from firmware
  * @lock: a lock for this strucure
+ * @irq_disabling: flag to indicate an IRQ-disable is in-progress
+ * @irq_disable_waitqueue: waitqueue to wait for the IRQ-disable to complete
  * @response_packet: a pointer to response packet from fw to driver
  * @header_id: id of packet header
  * @packet_id: id of packet
@@ -116,6 +119,8 @@ struct iris_core {
 	struct iris_iface_q_info		message_queue;
 	struct iris_iface_q_info		debug_queue;
 	struct mutex				lock; /* lock for core related operations */
+	bool					irq_disabling;
+	wait_queue_head_t			irq_disable_waitqueue;
 	u8					*response_packet;
 	u32					header_id;
 	u32					packet_id;
diff --git a/drivers/media/platform/qcom/iris/iris_probe.c b/drivers/media/platform/qcom/iris/iris_probe.c
index 41dec90f3e2be..d207d5757648c 100644
--- a/drivers/media/platform/qcom/iris/iris_probe.c
+++ b/drivers/media/platform/qcom/iris/iris_probe.c
@@ -288,6 +288,7 @@ static int iris_probe(struct platform_device *pdev)
 
 	core->state = IRIS_CORE_DEINIT;
 	mutex_init(&core->lock);
+	init_waitqueue_head(&core->irq_disable_waitqueue);
 	init_completion(&core->core_init_done);
 
 	core->response_packet = devm_kzalloc(core->dev, IFACEQ_CORE_PKT_SIZE, GFP_KERNEL);
@@ -379,14 +380,14 @@ static int __maybe_unused iris_pm_suspend(struct device *dev)
 
 	core = dev_get_drvdata(dev);
 
-	mutex_lock(&core->lock);
+	iris_acquire_core_lock(core);
 	if (core->state != IRIS_CORE_INIT)
 		goto exit;
 
 	ret = iris_hfi_pm_suspend(core);
 
 exit:
-	mutex_unlock(&core->lock);
+	iris_release_core_lock(core);
 
 	return ret;
 }
@@ -398,7 +399,7 @@ static int __maybe_unused iris_pm_resume(struct device *dev)
 
 	core = dev_get_drvdata(dev);
 
-	mutex_lock(&core->lock);
+	iris_acquire_core_lock(core);
 	if (core->state != IRIS_CORE_INIT)
 		goto exit;
 
@@ -406,7 +407,7 @@ static int __maybe_unused iris_pm_resume(struct device *dev)
 	pm_runtime_mark_last_busy(core->dev);
 
 exit:
-	mutex_unlock(&core->lock);
+	iris_release_core_lock(core);
 
 	return ret;
 }
diff --git a/drivers/media/platform/qcom/iris/iris_resources.c b/drivers/media/platform/qcom/iris/iris_resources.c
index 883ef7e752915..6c92857a6b8eb 100644
--- a/drivers/media/platform/qcom/iris/iris_resources.c
+++ b/drivers/media/platform/qcom/iris/iris_resources.c
@@ -16,6 +16,22 @@
 
 #define BW_THRESHOLD 50000
 
+void iris_acquire_core_lock(struct iris_core *core)
+{
+	mutex_lock(&core->lock);
+
+	while (READ_ONCE(core->irq_disabling)) {
+		mutex_unlock(&core->lock);
+		wait_event(core->irq_disable_waitqueue, !READ_ONCE(core->irq_disabling));
+		mutex_lock(&core->lock);
+	}
+}
+
+void iris_release_core_lock(struct iris_core *core)
+{
+	mutex_unlock(&core->lock);
+}
+
 int iris_set_icc_bw(struct iris_core *core, unsigned long icc_bw)
 {
 	u32 icc_ib_multiplier = core->iris_platform_data->icc_ib_multiplier;
diff --git a/drivers/media/platform/qcom/iris/iris_resources.h b/drivers/media/platform/qcom/iris/iris_resources.h
index 6bfbd2dc6db09..6e1a5e734925f 100644
--- a/drivers/media/platform/qcom/iris/iris_resources.h
+++ b/drivers/media/platform/qcom/iris/iris_resources.h
@@ -8,6 +8,8 @@
 
 struct iris_core;
 
+void iris_acquire_core_lock(struct iris_core *core);
+void iris_release_core_lock(struct iris_core *core);
 int iris_opp_set_rate(struct device *dev, unsigned long freq);
 int iris_enable_power_domains(struct iris_core *core, struct device *pd_dev);
 int iris_disable_power_domains(struct iris_core *core, struct device *pd_dev);
diff --git a/drivers/media/platform/qcom/iris/iris_vpu_common.c b/drivers/media/platform/qcom/iris/iris_vpu_common.c
index 1e183fc4e58ce..fcf060a72c4cf 100644
--- a/drivers/media/platform/qcom/iris/iris_vpu_common.c
+++ b/drivers/media/platform/qcom/iris/iris_vpu_common.c
@@ -232,15 +232,25 @@ void iris_vpu_power_off_hw(struct iris_core *core)
 	iris_disable_power_domains(core, core->pmdomain_tbl->pd_devs[IRIS_HW_POWER_DOMAIN]);
 }
 
+static void iris_vpu_disable_irq(struct iris_core *core)
+{
+	WRITE_ONCE(core->irq_disabling, true);
+	mutex_unlock(&core->lock);
+	disable_irq(core->irq);
+	mutex_lock(&core->lock);
+	WRITE_ONCE(core->irq_disabling, false);
+	wake_up_all(&core->irq_disable_waitqueue);
+}
+
 void iris_vpu_power_off(struct iris_core *core)
 {
+	if (!iris_vpu_watchdog(core, core->intr_status))
+		iris_vpu_disable_irq(core);
+
 	iris_opp_set_rate(core->dev, 0);
 	core->iris_platform_data->vpu_ops->power_off_hw(core);
 	core->iris_platform_data->vpu_ops->power_off_controller(core);
 	iris_unset_icc_bw(core);
-
-	if (!iris_vpu_watchdog(core, core->intr_status))
-		disable_irq_nosync(core->irq);
 }
 
 int iris_vpu_power_on_controller(struct iris_core *core)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 2/2] media: qcom: iris: Wait for threaded IRQ completion during power-off
  2026-10-07 11:31 ` [PATCH 2/2] media: qcom: iris: Wait for threaded IRQ completion during power-off Vishnu Reddy
@ 2026-10-07 17:29   ` Hungyu Lin
  2026-10-11  6:39     ` Vishnu Reddy
  0 siblings, 1 reply; 5+ messages in thread
From: Hungyu Lin @ 2026-10-07 17:29 UTC (permalink / raw)
  To: Vishnu Reddy
  Cc: Vikash Garodia, Dikshita Agarwal, Abhinav Kumar,
	Bryan O'Donoghue, Mauro Carvalho Chehab, Konrad Dybcio,
	Dmitry Baryshkov, Stefan Schmidt, Hans Verkuil,
	Bryan O'Donoghue, linux-media, linux-arm-msm, linux-kernel,
	stable

> +       if (!iris_vpu_watchdog(core, core->intr_status))
> +               iris_vpu_disable_irq(core);

If the IRQ thread records a watchdog after this check, while
disable_irq() is waiting, it skips enable_irq(). The hard handler and
helper would then have disabled the IRQ twice, but the next power-on
only enables it once.

Could the helper recheck the watchdog after disable_irq() returns and
core->lock is reacquired, then call enable_irq() once to balance its
extra disable in that case?

If you use this approach for the watchdog fix, could you also add:

Suggested-by: Hungyu Lin <dennylin0707@gmail.com>

Thanks,
Hungyu

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 2/2] media: qcom: iris: Wait for threaded IRQ completion during power-off
  2026-10-07 17:29   ` Hungyu Lin
@ 2026-10-11  6:39     ` Vishnu Reddy
  0 siblings, 0 replies; 5+ messages in thread
From: Vishnu Reddy @ 2026-10-11  6:39 UTC (permalink / raw)
  To: Hungyu Lin
  Cc: Vikash Garodia, Dikshita Agarwal, Abhinav Kumar,
	Bryan O'Donoghue, Mauro Carvalho Chehab, Konrad Dybcio,
	Dmitry Baryshkov, Stefan Schmidt, Hans Verkuil,
	Bryan O'Donoghue, linux-media, linux-arm-msm, linux-kernel,
	stable


On 10/7/2026 10:59 PM, Hungyu Lin wrote:
>> +       if (!iris_vpu_watchdog(core, core->intr_status))
>> +               iris_vpu_disable_irq(core);
> If the IRQ thread records a watchdog after this check, while
> disable_irq() is waiting, it skips enable_irq(). The hard handler and
> helper would then have disabled the IRQ twice, but the next power-on
> only enables it once.
>
> Could the helper recheck the watchdog after disable_irq() returns and
> core->lock is reacquired, then call enable_irq() once to balance its
> extra disable in that case?

This patch is not about the IRQ disable depth. Its purpose is to make sure
the threaded handler has finished before the VPU is powered down.

Re-enabling the IRQ here, during driver shutdown or removal, could let a
watchdog interrupt fire and reschedule the sys_error_handler work, which
can lead to a use-after-free once the core is torn down.

For that reason I would prefer not to add the enable_irq() recheck in this
patch.

Regards,
Vishnu Reddy.

> If you use this approach for the watchdog fix, could you also add:
>
> Suggested-by: Hungyu Lin <dennylin0707@gmail.com>
>
> Thanks,
> Hungyu

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-10-11  6:40 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-07 11:31 [PATCH 0/2] media: qcom: iris: Fix IRQ synchronization during power-off Vishnu Reddy
2026-10-07 11:31 ` [PATCH 1/2] Revert "media: qcom: iris: use disable_irq() during power-off" Vishnu Reddy
2026-10-07 11:31 ` [PATCH 2/2] media: qcom: iris: Wait for threaded IRQ completion during power-off Vishnu Reddy
2026-10-07 17:29   ` Hungyu Lin
2026-10-11  6:39     ` Vishnu Reddy

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®