Re: [PATCH v7 08/17] drm/panfrost: Split subsystem init/reset from interrupt enablement

[Boris Brezillon]

On Fri, 28 Aug 2026 21:56:48 +0100
Adrián Larumbe <adrian.larumbe@xxxxxxxxxxxxx> wrote:

> Because MMU interrupts are only enabled when the device is reset, it
> happened that after DRM device registration, the very first job targeting
> the tiler heap BO would always time out. The reason is the reset sequence
> is only part of PM runtime resume, which is not called explicitly at driver
> probe time, and an actual reset work item manually triggered after a HW
> error.
> 
> I have attempted a somewhat drastic solution, which is completely
> decoupling GPU/MMU/JM subsystem initialisation and reset from interrupt
> enablement, so that we can handle IRQ toggling a bit more flexibly.
> 
> To this end:
> - Ensure every subsystem with its own IRQ has an 'enable interrupts'
> method, and that it doesn't enable them anywhere else.
> - Force IRQ masking at MMU reset time. Up until, now, panfrost_mmu_reset()
> was clearing the MMU IRQ suspension bit, but at no point that is set during
> the reset sequence.
> 
> Then manually enable all interrupts when the device is fully initialised at
> probe time, right before DRM device registration, or after the reset
> sequence is complete. Also disable all interrupts at device remove time,
> so that their IRQs can be sync'ed right before tearing the device down.
> 
> Fixes: 635430797d3f ("drm/panfrost: Rework runtime PM initialization")
> Fixes: 876b15d2c88d ("drm/panfrost: Fix module unload")
> Signed-off-by: Adrián Larumbe <adrian.larumbe@xxxxxxxxxxxxx>
> ---
>  drivers/gpu/drm/panfrost/panfrost_device.c | 40 ++++++++++++++++++++++--------
>  drivers/gpu/drm/panfrost/panfrost_device.h |  3 ++-
>  drivers/gpu/drm/panfrost/panfrost_gpu.c    | 19 ++++++++------
>  drivers/gpu/drm/panfrost/panfrost_gpu.h    |  2 ++
>  drivers/gpu/drm/panfrost/panfrost_job.c    |  7 +++---
>  drivers/gpu/drm/panfrost/panfrost_mmu.c    |  9 +++++--
>  drivers/gpu/drm/panfrost/panfrost_mmu.h    |  2 ++
>  7 files changed, 56 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c b/drivers/gpu/drm/panfrost/panfrost_device.c
> index 9e02fb5f73c8..99f7da2180f9 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_device.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_device.c
> @@ -226,6 +226,27 @@ static int panfrost_pm_domain_init(struct panfrost_device *pfdev)
>  	return err;
>  }
>  
> +void panfrost_device_enable_int(struct panfrost_device *pfdev)
> +{
> +	panfrost_gpu_enable_interrupts(pfdev);
> +	panfrost_mmu_enable_interrupts(pfdev);
> +	panfrost_jm_enable_interrupts(pfdev);
> +}
> +
> +static void panfrost_device_enable_hw(struct panfrost_device *pfdev)
> +{
> +	panfrost_device_enable_int(pfdev);
> +	panfrost_devfreq_resume(pfdev);
> +}
> +
> +static void panfrost_device_disable_hw(struct panfrost_device *pfdev)
> +{
> +	panfrost_devfreq_suspend(pfdev);
> +	panfrost_jm_suspend_irq(pfdev);
> +	panfrost_mmu_suspend_irq(pfdev);
> +	panfrost_gpu_suspend_irq(pfdev);

Hm, I think I'd prefer if those suspend/resume_irq() were hidden in
some subcomponent panfrost_<subcomp>_suspend,resume() helpers. And
then we just have to resume/suspend component in the right order
instead of treating IRQs as a standalone object (enabling/disabling
only makes sense if the subcomponent handling those interrupts is
resumed/suspended).