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

Hungyu Lin posted 1 patch 4 days ago
There is a newer version of this series
drivers/media/platform/qcom/iris/iris_hfi_common.c | 7 +++++++
1 file changed, 7 insertions(+)
[PATCH v3] media: qcom: iris: guard IRQ handler with runtime PM check
Posted by Hungyu Lin 4 days ago
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
Posted by Dmitry Baryshkov 12 hours ago
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
Posted by Dmitry Baryshkov 3 days, 19 hours ago
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
Posted by Denny Lin 3 days, 17 hours ago
> 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
Posted by Dmitry Baryshkov 11 hours ago
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
Re: [PATCH v3] media: qcom: iris: guard IRQ handler with runtime PM check
Posted by Bryan O'Donoghue 3 days, 23 hours ago
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
Posted by Denny Lin 3 days, 23 hours ago
> 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
Posted by Bryan O'Donoghue 3 days, 21 hours ago
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
Posted by Denny Lin 3 days, 20 hours ago
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
Posted by Bryan O'Donoghue 3 days, 20 hours ago
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