* [PATCH v2 0/3] media: qcom: iris: Fix IRQ synchronization during power-off
@ 2026-11-11 9:05 Vishnu Reddy
2026-11-11 9:05 ` [PATCH v2 1/3] Revert "media: qcom: iris: use disable_irq() during power-off" Vishnu Reddy
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Vishnu Reddy @ 2026-11-11 9:05 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, Joonhoe Kim, Birk Skyum
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>
---
Changes in v2:
- Added new patch to handle the irq depth
- Link to v1: https://patch.msgid.link/20261007-fix-disable-irq-safely-v1-0-45746f9a92de@oss.qualcomm.com
---
Vishnu Reddy (3):
Revert "media: qcom: iris: use disable_irq() during power-off"
media: qcom: iris: Wait for threaded IRQ completion during power-off
media: qcom: iris: Fix IRQ disable depth imbalance on watchdog interrupt
drivers/media/platform/qcom/iris/iris_core.c | 10 +++++-----
drivers/media/platform/qcom/iris/iris_core.h | 7 +++++++
drivers/media/platform/qcom/iris/iris_hfi_common.c | 6 +++++-
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 | 19 ++++++++++++++++---
7 files changed, 56 insertions(+), 13 deletions(-)
---
base-commit: 793807ca13bf3072fc6d16ce6d90e9ab9beff2a5
change-id: 20261004-fix-disable-irq-safely-15b2f712800b
Best regards,
--
Vishnu Reddy <busanna.reddy@oss.qualcomm.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 1/3] Revert "media: qcom: iris: use disable_irq() during power-off"
2026-11-11 9:05 [PATCH v2 0/3] media: qcom: iris: Fix IRQ synchronization during power-off Vishnu Reddy
@ 2026-11-11 9:05 ` Vishnu Reddy
2026-11-13 0:55 ` Bryan O'Donoghue
2026-11-11 9:05 ` [PATCH v2 2/3] media: qcom: iris: Wait for threaded IRQ completion during power-off Vishnu Reddy
2026-11-11 9:05 ` [PATCH v2 3/3] media: qcom: iris: Fix IRQ disable depth imbalance on watchdog interrupt Vishnu Reddy
2 siblings, 1 reply; 6+ messages in thread
From: Vishnu Reddy @ 2026-11-11 9:05 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] 6+ messages in thread
* [PATCH v2 2/3] media: qcom: iris: Wait for threaded IRQ completion during power-off
2026-11-11 9:05 [PATCH v2 0/3] media: qcom: iris: Fix IRQ synchronization during power-off Vishnu Reddy
2026-11-11 9:05 ` [PATCH v2 1/3] Revert "media: qcom: iris: use disable_irq() during power-off" Vishnu Reddy
@ 2026-11-11 9:05 ` Vishnu Reddy
2026-11-13 0:55 ` Bryan O'Donoghue
2026-11-11 9:05 ` [PATCH v2 3/3] media: qcom: iris: Fix IRQ disable depth imbalance on watchdog interrupt Vishnu Reddy
2 siblings, 1 reply; 6+ messages in thread
From: Vishnu Reddy @ 2026-11-11 9:05 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, Joonhoe Kim, Birk Skyum
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
Tested-by: Joonhoe Kim <26rote@gmail.com> # Kaanapali
Tested-by: Birk Skyum <birk.skyum@pm.me> # Lenovo Yoga Slim 7x (X1E80100)
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 343e44c6b9c46..f6684ce09334f 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] 6+ messages in thread
* [PATCH v2 3/3] media: qcom: iris: Fix IRQ disable depth imbalance on watchdog interrupt
2026-11-11 9:05 [PATCH v2 0/3] media: qcom: iris: Fix IRQ synchronization during power-off Vishnu Reddy
2026-11-11 9:05 ` [PATCH v2 1/3] Revert "media: qcom: iris: use disable_irq() during power-off" Vishnu Reddy
2026-11-11 9:05 ` [PATCH v2 2/3] media: qcom: iris: Wait for threaded IRQ completion during power-off Vishnu Reddy
@ 2026-11-11 9:05 ` Vishnu Reddy
2 siblings, 0 replies; 6+ messages in thread
From: Vishnu Reddy @ 2026-11-11 9:05 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_hfi_isr() disables the IRQ for every interrupt and the threaded
handler enables it again, except for a watchdog interrupt, which is left
masked until the firmware is reset. iris_vpu_power_off() decides whether
to add its own disable from core->intr_status, but that is only updated
by the threaded handler. If a watchdog interrupt is still pending when
power-off checks it, power-off adds a disable and the handler then skips
its enable, leaving the depth one higher than the single enable_irq() in
iris_vpu_power_on() can undo. The IRQ then stays disabled after the
reset and the firmware can no longer be reached.
Make the depth independent of when intr_status is read. Power-off now
always takes one disable, paired with the enable in power-on. The
threaded handler evaluates the watchdog status once under core->lock and
sets core->wdt_irq_enable_pending when it skips its enable. Power-on
drops that extra disable together with its own.
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.h | 2 ++
drivers/media/platform/qcom/iris/iris_hfi_common.c | 6 +++++-
drivers/media/platform/qcom/iris/iris_vpu_common.c | 7 +++++--
3 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/media/platform/qcom/iris/iris_core.h b/drivers/media/platform/qcom/iris/iris_core.h
index 0b1b9721e6f9..1909b91deab7 100644
--- a/drivers/media/platform/qcom/iris/iris_core.h
+++ b/drivers/media/platform/qcom/iris/iris_core.h
@@ -73,6 +73,7 @@ struct qcom_ubwc_cfg_data;
* @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
+ * @wdt_irq_enable_pending: enable_irq skipped on wdt in isr, done on power-on
* @response_packet: a pointer to response packet from fw to driver
* @header_id: id of packet header
* @packet_id: id of packet
@@ -121,6 +122,7 @@ struct iris_core {
struct mutex lock; /* lock for core related operations */
bool irq_disabling;
wait_queue_head_t irq_disable_waitqueue;
+ bool wdt_irq_enable_pending;
u8 *response_packet;
u32 header_id;
u32 packet_id;
diff --git a/drivers/media/platform/qcom/iris/iris_hfi_common.c b/drivers/media/platform/qcom/iris/iris_hfi_common.c
index dad906d03fdc..7a2969579697 100644
--- a/drivers/media/platform/qcom/iris/iris_hfi_common.c
+++ b/drivers/media/platform/qcom/iris/iris_hfi_common.c
@@ -100,6 +100,7 @@ irqreturn_t iris_hfi_isr(int irq, void *data)
irqreturn_t iris_hfi_isr_handler(int irq, void *data)
{
struct iris_core *core = data;
+ int wdt_irq;
if (!core)
return IRQ_NONE;
@@ -107,12 +108,15 @@ irqreturn_t iris_hfi_isr_handler(int irq, void *data)
mutex_lock(&core->lock);
pm_runtime_mark_last_busy(core->dev);
iris_vpu_clear_interrupt(core);
+ wdt_irq = iris_vpu_watchdog(core, core->intr_status);
+ if (wdt_irq)
+ core->wdt_irq_enable_pending = true;
mutex_unlock(&core->lock);
if (core->hfi_sys_ops)
core->hfi_sys_ops->sys_hfi_response_handler(core);
- if (!iris_vpu_watchdog(core, core->intr_status))
+ if (!wdt_irq)
enable_irq(irq);
return IRQ_HANDLED;
diff --git a/drivers/media/platform/qcom/iris/iris_vpu_common.c b/drivers/media/platform/qcom/iris/iris_vpu_common.c
index fcf060a72c4c..0ce5094c0bb9 100644
--- a/drivers/media/platform/qcom/iris/iris_vpu_common.c
+++ b/drivers/media/platform/qcom/iris/iris_vpu_common.c
@@ -244,8 +244,7 @@ static void iris_vpu_disable_irq(struct iris_core *core)
void iris_vpu_power_off(struct iris_core *core)
{
- if (!iris_vpu_watchdog(core, core->intr_status))
- iris_vpu_disable_irq(core);
+ iris_vpu_disable_irq(core);
iris_opp_set_rate(core->dev, 0);
core->iris_platform_data->vpu_ops->power_off_hw(core);
@@ -531,6 +530,10 @@ int iris_vpu_power_on(struct iris_core *core)
core->iris_platform_data->vpu_ops->interrupt_init(core);
core->intr_status = 0;
+ if (core->wdt_irq_enable_pending) {
+ core->wdt_irq_enable_pending = false;
+ enable_irq(core->irq);
+ }
enable_irq(core->irq);
return 0;
--
2.34.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/3] media: qcom: iris: Wait for threaded IRQ completion during power-off
2026-11-11 9:05 ` [PATCH v2 2/3] media: qcom: iris: Wait for threaded IRQ completion during power-off Vishnu Reddy
@ 2026-11-13 0:55 ` Bryan O'Donoghue
0 siblings, 0 replies; 6+ messages in thread
From: Bryan O'Donoghue @ 2026-11-13 0:55 UTC (permalink / raw)
To: Vishnu Reddy, Vikash Garodia, Dikshita Agarwal, Abhinav Kumar,
Bryan O'Donoghue, Mauro Carvalho Chehab, Konrad Dybcio,
Dmitry Baryshkov, Hungyu Lin, Stefan Schmidt, Hans Verkuil
Cc: linux-media, linux-arm-msm, linux-kernel, stable, Joonhoe Kim,
Birk Skyum
On 11/11/2026 09:05, Vishnu Reddy wrote:
> +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);
> + }
> +}
> +
> +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);
> +}
> +
Ouch, this is some mad amount of jumping through hoops.
As General Pierre François Joseph Bosquet said of the Charge of the
Light Brigade in the Crimean war "C’est magnifique, mais ce n’est pas la
guerre" - It is magnificent, but it is not war.
Why do we have to hold core->lock over iris_vpu_power_off() at all ?
- What is the data-structure or
- Logical state (the state variable I suspect)
- Or guarantee of atomicity
We are providing holding that lock ?
These are pretty much rhetorical questions - the above locking scheme
can't be right. It implies we are doing something wrong and not really
fixing that wrong thing.
Holding the lock across power_off() seems to be that wrong thing.
disable_irq_nosync() seems to be a correct fix - so if our locking is so
muddled we have to go to the above - there better be a fully reasoned
and unavoidable reason why.
Perhaps I'm being overly reductive here but - why not "just" refactor
the code so that the power-off comes after the piece held in the lock ?
i.e. push that into a worker and then do a synchronous wait on the worker ?
The chatbot told me I was wrong about the worker because "it would
deadlock" and I can't be bothered burning the tokens to tell it "I
didn't mean hold the lock in the worker"
It did make the point that it thinks the only thing being protected by
the lock is
intr_status = readl(core->reg_base + WRAPPER_INTR_STATUS);
...
writel(CLEAR_XTENSA2HOST_INTR, core->reg_base + CPU_CS_A2HSOFTINTCLR);
Which venus does like this:
static irqreturn_t venus_isr(struct venus_core *core) /*
hfi_venus.c */
{
status = readl(wrapper_base + WRAPPER_INTR_STATUS);
...
writel(1, cpu_cs_base + CPU_CS_A2HSOFTINTCLR);
return IRQ_WAKE_THREAD;
}
i.e. power off in the IRQ.
Perhaps even simpler - just do the power-off directly outside of the
locked section... once the IRQ is quiescent ?
Either way I don't think we have a good reason to hold this lock and can
certainly find a way not to hold it - assuming the only thing we care
about is kicking that register - do it in the IRQ why not else do a
worker with a rendezvous or even more straight forward.
void iris_core_deinit(struct iris_core *core)
{
bool power_off = false;
int ret;
ret = pm_runtime_resume_and_get(core->dev);
mutex_lock(&core->lock);
if (core->state != IRIS_CORE_DEINIT) {
iris_fw_unload(core);
power_off = !ret;
iris_hfi_queues_deinit(core);
core->state = IRIS_CORE_DEINIT;
}
mutex_unlock(&core->lock);
if (power_off)
iris_vpu_power_off(core); <- this ?
if (!ret)
pm_runtime_put_sync(core->dev);
}
---
bod
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/3] Revert "media: qcom: iris: use disable_irq() during power-off"
2026-11-11 9:05 ` [PATCH v2 1/3] Revert "media: qcom: iris: use disable_irq() during power-off" Vishnu Reddy
@ 2026-11-13 0:55 ` Bryan O'Donoghue
0 siblings, 0 replies; 6+ messages in thread
From: Bryan O'Donoghue @ 2026-11-13 0:55 UTC (permalink / raw)
To: Vishnu Reddy, Vikash Garodia, Dikshita Agarwal, Abhinav Kumar,
Bryan O'Donoghue, Mauro Carvalho Chehab, Konrad Dybcio,
Dmitry Baryshkov, Hungyu Lin, Stefan Schmidt, Hans Verkuil
Cc: linux-media, linux-arm-msm, linux-kernel, stable
On 11/11/2026 09:05, Vishnu Reddy wrote:
> iris_vpu_power_off() is always called with core->lock held.
This statement is not true : int iris_hfi_pm_suspend(struct iris_core *core)
And TBH as I look at the code I wonder if it should have a lockdep
assert if that lock is a requirement...
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)
>
Looking at the next patch I'm fully recoiling.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-11-13 0:55 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-11 9:05 [PATCH v2 0/3] media: qcom: iris: Fix IRQ synchronization during power-off Vishnu Reddy
2026-11-11 9:05 ` [PATCH v2 1/3] Revert "media: qcom: iris: use disable_irq() during power-off" Vishnu Reddy
2026-11-13 0:55 ` Bryan O'Donoghue
2026-11-11 9:05 ` [PATCH v2 2/3] media: qcom: iris: Wait for threaded IRQ completion during power-off Vishnu Reddy
2026-11-13 0:55 ` Bryan O'Donoghue
2026-11-11 9:05 ` [PATCH v2 3/3] media: qcom: iris: Fix IRQ disable depth imbalance on watchdog interrupt 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®