[PATCH v3] media: qcom: iris: guard IRQ handler with runtime PM check

[PATCH v3] media: qcom: iris: guard IRQ handler with runtime PM check

[Hungyu Lin]

Use pm_runtime_get_if_active() before accessing hardware
registers in the threaded IRQ handler. Skip interrupt processing
when the device is not active.

Signed-off-by: Hungyu Lin <dennylin0707@gmail.com>
---
v2:
- Use pm_runtime_get_if_active() instead of pm_runtime_get_if_in_use()
- Handle negative runtime PM return values correctly
- Return IRQ_NONE when interrupt processing is skipped

v3:
- Remove the early enable_irq() from the PM-inactive early-return path
- IRQ re-enablement is already handled by iris_vpu_power_on() after power-on

 drivers/media/platform/qcom/iris/iris_hfi_common.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/media/platform/qcom/iris/iris_hfi_common.c b/drivers/media/platform/qcom/iris/iris_hfi_common.c
index 621c66593d88..59040cce8cf1 100644
--- a/drivers/media/platform/qcom/iris/iris_hfi_common.c
+++ b/drivers/media/platform/qcom/iris/iris_hfi_common.c
@@ -100,10 +100,15 @@ 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 ret;
 
 	if (!core)
 		return IRQ_NONE;
 
+	ret = pm_runtime_get_if_active(core->dev);
+	if (ret <= 0)
+		return IRQ_NONE;
+
 	mutex_lock(&core->lock);
 	pm_runtime_mark_last_busy(core->dev);
 	iris_vpu_clear_interrupt(core);
@@ -111,6 +116,8 @@ irqreturn_t iris_hfi_isr_handler(int irq, void *data)
 
 	core->hfi_response_ops->hfi_response_handler(core);
 
+	pm_runtime_put_autosuspend(core->dev);
+
 	if (!iris_vpu_watchdog(core, core->intr_status))
 		enable_irq(irq);
 
-- 
2.34.1

Re: [PATCH v3] media: qcom: iris: guard IRQ handler with runtime PM check

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Critical] Early return from threaded IRQ handler skips enable_irq(), permanently masking the interrupt and completely breaking CONFIG_PM=n builds.

Pre-existing issues:
- [High] Tearing down shared queues and destroying locks in iris_remove() without synchronizing the threaded IRQ leads to Use-After-Free.
--

commit 21ac80fbe9eb1a25413cc6f4a32a9f240d73c9b7
Author: Hungyu Lin <dennylin0707@gmail.com>

media: qcom: iris: guard IRQ handler with runtime PM check

This commit adds a runtime PM check to the threaded IRQ handler using
pm_runtime_get_if_active(). It skips interrupt processing if the device
is not active and autosuspends the device after processing.

> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_common.c b/drivers/media/platform/qcom/iris/iris_hfi_common.c
> index 621c66593d88d..59040cce8cf16 100644
> --- a/drivers/media/platform/qcom/iris/iris_hfi_common.c
> +++ b/drivers/media/platform/qcom/iris/iris_hfi_common.c
> @@ -100,10 +100,15 @@ 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 ret;
>  
>  	if (!core)
>  		return IRQ_NONE;
>  
> +	ret = pm_runtime_get_if_active(core->dev);
> +	if (ret <= 0)
> +		return IRQ_NONE;
> +

[Severity: Critical]
Does this early return permanently mask the interrupt?

The top-half handler iris_hfi_isr() calls disable_irq_nosync() before
waking this threaded handler. By returning early here in
iris_hfi_isr_handler(), does the code skip the enable_irq() call at the end
of the handler and leave the interrupt permanently disabled?

Additionally, on builds without CONFIG_PM, pm_runtime_get_if_active()
always returns -EINVAL. Will this cause the handler to unconditionally
return early on the first interrupt?

>  	mutex_lock(&core->lock);
>  	pm_runtime_mark_last_busy(core->dev);
>  	iris_vpu_clear_interrupt(core);
> @@ -111,6 +116,8 @@ irqreturn_t iris_hfi_isr_handler(int irq, void *data)
>  
>  	core->hfi_response_ops->hfi_response_handler(core);
>  
> +	pm_runtime_put_autosuspend(core->dev);
> +
>  	if (!iris_vpu_watchdog(core, core->intr_status))
>  		enable_irq(irq);
>  

[Severity: High]
This isn't a bug introduced by this patch, but is it possible for this
threaded handler to execute after the queues and locks are destroyed during
device removal?

During device removal, iris_remove() calls iris_core_deinit() which disables
the IRQ via disable_irq_nosync() and frees the queues using
iris_hfi_queues_deinit(). Since disable_irq_nosync() does not wait for an
executing threaded handler to complete, could iris_hfi_isr_handler() run
concurrently and attempt to access the freed queues or the destroyed lock?

Would calling synchronize_irq() before tearing down these software structures
prevent this use-after-free?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260604082510.45733-1-dennylin0707@gmail.com?part=1

Re: [PATCH v3] media: qcom: iris: guard IRQ handler with runtime PM check

[Bryan O'Donoghue]

On 04/06/2026 09:25, Hungyu Lin wrote:
> Use pm_runtime_get_if_active() before accessing hardware
> registers in the threaded IRQ handler. Skip interrupt processing
> when the device is not active.
> 
> Signed-off-by: Hungyu Lin <dennylin0707@gmail.com>
> ---
> v2:
> - Use pm_runtime_get_if_active() instead of pm_runtime_get_if_in_use()
> - Handle negative runtime PM return values correctly
> - Return IRQ_NONE when interrupt processing is skipped
> 
> v3:
> - Remove the early enable_irq() from the PM-inactive early-return path
> - IRQ re-enablement is already handled by iris_vpu_power_on() after power-on
> 
>   drivers/media/platform/qcom/iris/iris_hfi_common.c | 7 +++++++
>   1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_common.c b/drivers/media/platform/qcom/iris/iris_hfi_common.c
> index 621c66593d88..59040cce8cf1 100644
> --- a/drivers/media/platform/qcom/iris/iris_hfi_common.c
> +++ b/drivers/media/platform/qcom/iris/iris_hfi_common.c
> @@ -100,10 +100,15 @@ 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 ret;
> 
>   	if (!core)
>   		return IRQ_NONE;
> 
> +	ret = pm_runtime_get_if_active(core->dev);
> +	if (ret <= 0)
> +		return IRQ_NONE;
> +
>   	mutex_lock(&core->lock);
>   	pm_runtime_mark_last_busy(core->dev);
>   	iris_vpu_clear_interrupt(core);
> @@ -111,6 +116,8 @@ irqreturn_t iris_hfi_isr_handler(int irq, void *data)
> 
>   	core->hfi_response_ops->hfi_response_handler(core);
> 
> +	pm_runtime_put_autosuspend(core->dev);
> +
>   	if (!iris_vpu_watchdog(core, core->intr_status))
>   		enable_irq(irq);
> 
> --
> 2.34.1
> 

Could you please put your series of individual patches but group them 
into a consistent list - so that I/we don't have to figure our how to 
apply them and in which order.

---
bod

Re: [PATCH v3] media: qcom: iris: guard IRQ handler with runtime PM check

[Denny Lin]

> Could you please put your series of individual patches but group them
> into a consistent list - so that I/we don't have to figure our how to
> apply them and in which order.

Sure. If patches are related or have an intended application order,
I'll group them into a patch series in future submissions to make that
clearer.

Thanks,
Hungyu

Re: [PATCH v3] media: qcom: iris: guard IRQ handler with runtime PM check

[Bryan O'Donoghue]

On 04/06/2026 10:07, Denny Lin wrote:
>> Could you please put your series of individual patches but group them
>> into a consistent list - so that I/we don't have to figure our how to
>> apply them and in which order.
> Sure. If patches are related or have an intended application order,
> I'll group them into a patch series in future submissions to make that
> clearer.
> 
> Thanks,
> Hungyu

Its just a bit confusing what you are doing here v1, v2, v3 in quick 
succession over a few topics in the same driver.

This feels like an aggregate fixes series. If the patches are 
individually applicable - great that's exactly what we want but, 
separating a series of fixes into individual bits and sending them out 
in quick succession is a bit of a mess in patchwork.

So could you _please_ just aggregate your fixes into a series so that 
the ordering is implicit - if ordering matters and the history of the 
reworking is contained in the series overview.

ty

---
bod

Re: [PATCH v3] media: qcom: iris: guard IRQ handler with runtime PM check

[Denny Lin]

Thanks, I'll slow down the pace a bit and make future submissions
easier to follow.

Thanks,
Hungyu

On Thu, Jun 4, 2026 at 4:12 AM Bryan O'Donoghue <bod@kernel.org> wrote:
>
> On 04/06/2026 10:07, Denny Lin wrote:
> >> Could you please put your series of individual patches but group them
> >> into a consistent list - so that I/we don't have to figure our how to
> >> apply them and in which order.
> > Sure. If patches are related or have an intended application order,
> > I'll group them into a patch series in future submissions to make that
> > clearer.
> >
> > Thanks,
> > Hungyu
>
> Its just a bit confusing what you are doing here v1, v2, v3 in quick
> succession over a few topics in the same driver.
>
> This feels like an aggregate fixes series. If the patches are
> individually applicable - great that's exactly what we want but,
> separating a series of fixes into individual bits and sending them out
> in quick succession is a bit of a mess in patchwork.
>
> So could you _please_ just aggregate your fixes into a series so that
> the ordering is implicit - if ordering matters and the history of the
> reworking is contained in the series overview.
>
> ty
>
> ---
> bod

Re: [PATCH v3] media: qcom: iris: guard IRQ handler with runtime PM check

[Bryan O'Donoghue]

On 04/06/2026 13:06, Denny Lin wrote:
> Thanks, I'll slow down the pace a bit and make future submissions
> easier to follow.

And no top posting ;)

> Thanks,
> Hungyu
I've marked as superseded everything that looked superseded - please 
ensure everything looks right for review now.

---
bod

Re: [PATCH v3] media: qcom: iris: guard IRQ handler with runtime PM check

[Dmitry Baryshkov]

On Thu, Jun 04, 2026 at 08:25:10AM +0000, Hungyu Lin wrote:
> Use pm_runtime_get_if_active() before accessing hardware
> registers in the threaded IRQ handler. Skip interrupt processing
> when the device is not active.

Please clarfiy why you are performing the changes instead of describing
the changes on their own. There should be no IRQs coming from the device
if it is not active.

> 
> Signed-off-by: Hungyu Lin <dennylin0707@gmail.com>
> ---
> v2:
> - Use pm_runtime_get_if_active() instead of pm_runtime_get_if_in_use()
> - Handle negative runtime PM return values correctly
> - Return IRQ_NONE when interrupt processing is skipped
> 
> v3:
> - Remove the early enable_irq() from the PM-inactive early-return path
> - IRQ re-enablement is already handled by iris_vpu_power_on() after power-on
> 
>  drivers/media/platform/qcom/iris/iris_hfi_common.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 

-- 
With best wishes
Dmitry

Re: [PATCH v3] media: qcom: iris: guard IRQ handler with runtime PM check

[Denny Lin]

> Please clarfiy why you are performing the changes instead of describing
> the changes on their own. There should be no IRQs coming from the device
> if it is not active.

My concern was a possible ordering where:

T1:
iris_hfi_isr()
-> return IRQ_WAKE_THREAD

T2:
iris_pm_suspend()
-> iris_hfi_pm_suspend()
-> iris_vpu_power_off()
-> power down the VPU

T3:
iris_hfi_isr_handler()
-> iris_vpu_clear_interrupt(core)

Am I missing something that prevents this ordering from occurring?

Thanks,
Hungyu

Re: [PATCH v3] media: qcom: iris: guard IRQ handler with runtime PM check

[Dmitry Baryshkov]

On Thu, Jun 04, 2026 at 08:25:10AM +0000, Hungyu Lin wrote:
> Use pm_runtime_get_if_active() before accessing hardware
> registers in the threaded IRQ handler. Skip interrupt processing
> when the device is not active.
> 
> Signed-off-by: Hungyu Lin <dennylin0707@gmail.com>

From: Denny Lin <dennylin0707@gmail.com>

So, what is the correct name?


-- 
With best wishes
Dmitry

Re: [PATCH v3] media: qcom: iris: guard IRQ handler with runtime PM check

[Dmitry Baryshkov]

On Thu, Jun 04, 2026 at 07:55:21AM -0700, Denny Lin wrote:
> > Please clarfiy why you are performing the changes instead of describing
> > the changes on their own. There should be no IRQs coming from the device
> > if it is not active.
> 
> My concern was a possible ordering where:
> 
> T1:
> iris_hfi_isr()
> -> return IRQ_WAKE_THREAD
> 
> T2:
> iris_pm_suspend()
> -> iris_hfi_pm_suspend()
> -> iris_vpu_power_off()
> -> power down the VPU
> 
> T3:
> iris_hfi_isr_handler()
> -> iris_vpu_clear_interrupt(core)
> 
> Am I missing something that prevents this ordering from occurring?

THis needs to be a part of the commit message.

> 
> Thanks,
> Hungyu

-- 
With best wishes
Dmitry