[PATCH] media: rockchip: rga: quiesce IRQ before releasing m2m state

[Fan Wu]

Hi Nicolas,

Thanks, that is a fair point that RGA is not a free-running IRQ source;
the interrupt should only be raised for job completion.

I looked again for the abort/drain path, though, and I could not find one
in the RGA teardown path. `v4l2_m2m_release()` only frees the m2m device.
RGA does not provide a `.job_abort` callback, and `rga_remove()` does not
reset the engine or disable the IRQ before releasing the m2m state. The
driver also leaves the devm-managed IRQ installed until devres cleanup
after remove returns.

So unless I am missing another path, teardown does not actively abort an
in-flight RGA job; it relies on there being no in-flight job by the time
remove gets there. In the normal case that is probably true, since the
hardware is not free-running and the submitted job has normally completed
already. The case I was trying to cover is the narrower one where
unbind/remove races with a still-running job.

That said, I agree the explicit irq field, `devm_free_irq()` and the long
comment may be too much for a defensive corner-case fix without a
reproducer. I can drop this patch, or respin it as a smaller ordering
cleanup if you think that is useful.

Thanks,
Fan

> On Jul 6, 2026, at 22:22, Nicolas Dufresne <nicolas at ndufresne.ca> wrote:
> 
> Hi,
> 
> Le samedi 04 juillet 2026 à 02:28 +0000, Fan Wu a écrit :
>> rga_probe() requests the interrupt with devm_request_irq(), so devres
>> does not release the IRQ until after rga_remove() returns. rga_remove()
>> currently releases rga->m2m_dev before that point.
>> 
>> rga_isr() uses rga->m2m_dev through v4l2_m2m_job_finish(),
>> leaving a window where an interrupt can run after the m2m device has been
>> released.
> 
> I have a doubt that this can really happen for this type of hardware. Its not a
> free-running HW that emits IRQ randomly, plus we have the abort sequence that
> ensure all jobs are completed before we pull it down.
> 
>> 
>> Unregister the video device first to stop new userspace submissions, then
>> free the devm-managed IRQ explicitly before releasing the m2m device. Move
>> the command buffer release after the IRQ teardown as well, so it is not
>> released while a completion interrupt can still arrive.
>> 
>> Store the IRQ number in struct rockchip_rga so rga_remove() can free the
>> IRQ without looking it up again.
>> 
>> Fixes: f7e7b48e6d79 ("[media] rockchip/rga: v4l2 m2m support")
>> Cc: stable at vger.kernel.org
>> Signed-off-by: Fan Wu <fanwu01 at zju.edu.cn>
>> 
>> ---
>> diff --git a/drivers/media/platform/rockchip/rga/rga.c b/drivers/media/platform/rockchip/rga/rga.c
>> index 43f6a8d..118887a 100644
>> --- a/drivers/media/platform/rockchip/rga/rga.c
>> +++ b/drivers/media/platform/rockchip/rga/rga.c
>> @@ -828,6 +828,8 @@ static int rga_probe(struct platform_device *pdev)
>>   goto err_put_clk;
>>   }
>>  
>> + rga->irq = irq;
>> +
>>   ret = devm_request_irq(rga->dev, irq, rga_isr, 0,
>>          dev_name(rga->dev), rga);
>>   if (ret < 0) {
>> @@ -919,13 +921,21 @@ static void rga_remove(struct platform_device *pdev)
>>  {
>>   struct rockchip_rga *rga = platform_get_drvdata(pdev);
>>  
>> - dma_free_attrs(rga->dev, RGA_CMDBUF_SIZE, rga->cmdbuf_virt,
>> -        rga->cmdbuf_phy, DMA_ATTR_WRITE_COMBINE);
>> -
>>   v4l2_info(&rga->v4l2_dev, "Removing\n");
>>  
>> - v4l2_m2m_release(rga->m2m_dev);
>>   video_unregister_device(rga->vfd);
>> +
>> + /*
>> + * The IRQ was requested with devm_request_irq() and is freed by devm
>> + * only after this function returns. Free it explicitly here, after the
>> + * video device is unregistered, but before v4l2_m2m_release() frees
>> + * rga->m2m_dev, which rga_isr() dereferences via v4l2_m2m_job_finish().
>> + */
>> + devm_free_irq(rga->dev, rga->irq, rga);
> 
> I'm not saying we cannot do that, but its quite verbose for something that
> probably can't happen in practice.
> 
> Nicolas
> 
>> +
>> + dma_free_attrs(rga->dev, RGA_CMDBUF_SIZE, rga->cmdbuf_virt,
>> +        rga->cmdbuf_phy, DMA_ATTR_WRITE_COMBINE);
>> + v4l2_m2m_release(rga->m2m_dev);
>>   v4l2_device_unregister(&rga->v4l2_dev);
>>  
>>   pm_runtime_disable(rga->dev);
>> diff --git a/drivers/media/platform/rockchip/rga/rga.h b/drivers/media/platform/rockchip/rga/rga.h
>> index 72a28b1..f76c45b 100644
>> --- a/drivers/media/platform/rockchip/rga/rga.h
>> +++ b/drivers/media/platform/rockchip/rga/rga.h
>> @@ -81,6 +81,7 @@ struct rockchip_rga {
>>   struct device *dev;
>>   struct regmap *grf;
>>   void __iomem *regs;
>> + int irq;
>>   struct clk *sclk;
>>   struct clk *aclk;
>>   struct clk *hclk;
>>