Re: [PATCH v4 06/13] drm/panfrost: Explicitly enable MMU interrupts at device init

[Steven Price <steven.price@arm.com>]

On 29/07/2026 03:54, Adrián Larumbe wrote:
> Because the device must be in a position to accept jobs between the time
> drm_dev_register() is called and autosuspend first kicks in, there's a very
> narrow window inbetween during which jobs targeting the tiler buffer
> object would time out, since the device's PM status is 'Active', but no MMU
> interrupts were enabled at device initialisation time.
> 
> Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com>
> Fixes: 73e467f60acd ("drm/panfrost: Consolidate reset handling")
> ---
>  drivers/gpu/drm/panfrost/panfrost_job.c |  3 ++-
>  drivers/gpu/drm/panfrost/panfrost_mmu.c | 14 ++++++++++++--
>  2 files changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c b/drivers/gpu/drm/panfrost/panfrost_job.c
> index 35ff5b0f0013..bda1494eb430 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_job.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_job.c
> @@ -871,7 +871,6 @@ int panfrost_jm_init(struct panfrost_device *pfdev)
>  	js = devm_kzalloc(pfdev->base.dev, sizeof(*js), GFP_KERNEL);
>  	if (!js)
>  		return -ENOMEM;
> -	pfdev->js = js;
>  
>  	INIT_WORK(&pfdev->reset.work, panfrost_reset_work);
>  	spin_lock_init(&js->job_lock);
> @@ -906,6 +905,8 @@ int panfrost_jm_init(struct panfrost_device *pfdev)
>  		}
>  	}
>  
> +	pfdev->js = js;
> +
>  	panfrost_jm_reset_interrupts(pfdev);
>  	panfrost_jm_enable_interrupts(pfdev);
>  
> diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c b/drivers/gpu/drm/panfrost/panfrost_mmu.c
> index 4a3162c3b659..aad0cd31516d 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_mmu.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c
> @@ -336,6 +336,12 @@ void panfrost_mmu_as_put(struct panfrost_device *pfdev, struct panfrost_mmu *mmu
>  	WARN_ON(atomic_read(&mmu->as_count) < 0);
>  }
>  
> +static void panfrost_mmu_enable_interrupts(struct panfrost_device *pfdev)
> +{
> +	mmu_write(pfdev, MMU_INT_CLEAR, ~0);
> +	mmu_write(pfdev, MMU_INT_MASK, ~0);
> +}
> +
>  void panfrost_mmu_reset(struct panfrost_device *pfdev)
>  {
>  	struct panfrost_mmu *mmu, *mmu_tmp;
> @@ -355,8 +361,7 @@ void panfrost_mmu_reset(struct panfrost_device *pfdev)
>  
>  	spin_unlock(&pfdev->as_lock);
>  
> -	mmu_write(pfdev, MMU_INT_CLEAR, ~0);
> -	mmu_write(pfdev, MMU_INT_MASK, ~0);
> +	panfrost_mmu_enable_interrupts(pfdev);
>  }
>  
>  static size_t get_pgsize(u64 addr, size_t size, size_t *count)
> @@ -880,6 +885,9 @@ static irqreturn_t panfrost_mmu_irq_handler_thread(int irq, void *data)
>  	u32 status = mmu_read(pfdev, MMU_INT_RAWSTAT);
>  	int ret;
>  
> +	if (!pfdev->js)
> +		return IRQ_NONE;
> +

I find it odd to check the status of the job scheduler here in the MMU
IRQ handler. Personally I'd prefer we just delay the enabling of the MMU
interrupts until the driver is in a state to handle them - i.e. move the
call to panfrost_mmu_enable_interrupts() to panfrost_device_init() once
the call to panfrost_jm_init() has completed.

Also note that simply returning IRQ_NONE without actually clearing the
interrupt is likely to cause problems - panfrost_mmu_irq_handler() will
have disabled the interrupt mask and it won't be re-enabled until the
timeout happens.

Thanks,
Steve

>  	while (status) {
>  		u32 as = ffs(status | (status >> 16)) - 1;
>  		u32 mask = BIT(as) | BIT(as + 16);
> @@ -970,6 +978,8 @@ int panfrost_mmu_init(struct panfrost_device *pfdev)
>  		return err;
>  	}
>  
> +	panfrost_mmu_enable_interrupts(pfdev);
> +
>  	return 0;
>  }
>  
>