Re: [PATCH 11/11] drm/panthor: Keep interrupts masked until they are needed

[Liviu Dudau <liviu.dudau@arm.com>]

On Thu, Jun 25, 2026 at 02:40:37PM +0200, Boris Brezillon wrote:
> The autogenerated panthor_request_xx_irq() helpers unmask Mali
> interrupts before we're sure we'll have a handler registered. For
> non-shared IRQ lines, that's fine, but for shared ones, it might cause
> an interrupt flood if the HW block raises an interrupt for any reason.
> 
> We could reworking the calls in panthor_request_xx_irq(), but it's just
           ^ rework

> simpler to let the caller decide when they are ready to handle interrupts
> and call panthor_pwr_irq_resume() themselves. While at it, rework the
> prototype to let users call panthor_pwr_irq_enable_events() explicitly
> instead of passing an initial mask to panthor_request_pwr_irq().
> 
> Fixes: 5fe909cae118 ("drm/panthor: Add the device logical block")
> Reported-by: Shashiko <sashiko-bot@kernel.org>
> Closes: https://sashiko.dev/#/patchset/20260623-panthor-signal-from-irq-v3-0-2ece396f8ee0@collabora.com?part=3
> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>

Looks good to me!

Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>

Best regards,
Liviu

> ---
>  drivers/gpu/drm/panthor/panthor_device.h | 7 ++++---
>  drivers/gpu/drm/panthor/panthor_fw.c     | 2 +-
>  drivers/gpu/drm/panthor/panthor_gpu.c    | 3 ++-
>  drivers/gpu/drm/panthor/panthor_mmu.c    | 9 +++++++--
>  drivers/gpu/drm/panthor/panthor_pwr.c    | 7 ++++---
>  5 files changed, 18 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panthor/panthor_device.h b/drivers/gpu/drm/panthor/panthor_device.h
> index a39386bd6382..0fda64fbe5f2 100644
> --- a/drivers/gpu/drm/panthor/panthor_device.h
> +++ b/drivers/gpu/drm/panthor/panthor_device.h
> @@ -588,14 +588,15 @@ static inline void panthor_ ## __name ## _irq_resume(struct panthor_irq *pirq)
>  												\
>  static int panthor_request_ ## __name ## _irq(struct panthor_device *ptdev,			\
>  					      struct panthor_irq *pirq,				\
> -					      int irq, u32 mask, void __iomem *iomem)		\
> +					      int irq, void __iomem *iomem)			\
>  {												\
>  	pirq->ptdev = ptdev;									\
>  	pirq->irq = irq;									\
> -	pirq->mask = mask;									\
> +	pirq->mask = 0;										\
>  	pirq->iomem = iomem;									\
>  	spin_lock_init(&pirq->mask_lock);							\
> -	panthor_ ## __name ## _irq_resume(pirq);						\
> +	atomic_set(&pirq->state, PANTHOR_IRQ_STATE_SUSPENDED);					\
> +	gpu_write(pirq->iomem, INT_MASK, 0);							\
>  												\
>  	return devm_request_threaded_irq(ptdev->base.dev, irq,					\
>  					 panthor_ ## __name ## _irq_raw_handler,		\
> diff --git a/drivers/gpu/drm/panthor/panthor_fw.c b/drivers/gpu/drm/panthor/panthor_fw.c
> index 4fbddb9e18c8..de8e6689a869 100644
> --- a/drivers/gpu/drm/panthor/panthor_fw.c
> +++ b/drivers/gpu/drm/panthor/panthor_fw.c
> @@ -1474,7 +1474,7 @@ int panthor_fw_init(struct panthor_device *ptdev)
>  	if (irq <= 0)
>  		return -ENODEV;
>  
> -	ret = panthor_request_job_irq(ptdev, &fw->irq, irq, 0,
> +	ret = panthor_request_job_irq(ptdev, &fw->irq, irq,
>  				      ptdev->iomem + JOB_INT_BASE);
>  	if (ret) {
>  		drm_err(&ptdev->base, "failed to request job irq");
> diff --git a/drivers/gpu/drm/panthor/panthor_gpu.c b/drivers/gpu/drm/panthor/panthor_gpu.c
> index e52c5675981f..c013d6bf9a59 100644
> --- a/drivers/gpu/drm/panthor/panthor_gpu.c
> +++ b/drivers/gpu/drm/panthor/panthor_gpu.c
> @@ -170,11 +170,12 @@ int panthor_gpu_init(struct panthor_device *ptdev)
>  		return irq;
>  
>  	ret = panthor_request_gpu_irq(ptdev, &ptdev->gpu->irq, irq,
> -				      GPU_INTERRUPTS_MASK,
>  				      ptdev->iomem + GPU_INT_BASE);
>  	if (ret)
>  		return ret;
>  
> +	panthor_gpu_irq_enable_events(&ptdev->gpu->irq, GPU_INTERRUPTS_MASK);
> +	panthor_gpu_irq_resume(&ptdev->gpu->irq);
>  	return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/panthor/panthor_mmu.c
> index 31cc57029c12..1fef3c5c1b50 100644
> --- a/drivers/gpu/drm/panthor/panthor_mmu.c
> +++ b/drivers/gpu/drm/panthor/panthor_mmu.c
> @@ -3406,7 +3406,6 @@ int panthor_mmu_init(struct panthor_device *ptdev)
>  		return -ENODEV;
>  
>  	ret = panthor_request_mmu_irq(ptdev, &mmu->irq, irq,
> -				      panthor_mmu_fault_mask(ptdev, ~0),
>  				      ptdev->iomem + MMU_INT_BASE);
>  	if (ret)
>  		return ret;
> @@ -3424,7 +3423,13 @@ int panthor_mmu_init(struct panthor_device *ptdev)
>  		ptdev->gpu_info.mmu_features |= BITS_PER_LONG;
>  	}
>  
> -	return drmm_add_action_or_reset(&ptdev->base, panthor_mmu_release_wq, mmu->vm.wq);
> +	ret = drmm_add_action_or_reset(&ptdev->base, panthor_mmu_release_wq, mmu->vm.wq);
> +	if (ret)
> +		return ret;
> +
> +	panthor_mmu_irq_enable_events(&mmu->irq, panthor_mmu_fault_mask(ptdev, ~0));
> +	panthor_mmu_irq_resume(&mmu->irq);
> +	return 0;
>  }
>  
>  #ifdef CONFIG_DEBUG_FS
> diff --git a/drivers/gpu/drm/panthor/panthor_pwr.c b/drivers/gpu/drm/panthor/panthor_pwr.c
> index 090362bd700b..f2c2c3000590 100644
> --- a/drivers/gpu/drm/panthor/panthor_pwr.c
> +++ b/drivers/gpu/drm/panthor/panthor_pwr.c
> @@ -484,12 +484,13 @@ int panthor_pwr_init(struct panthor_device *ptdev)
>  	if (irq < 0)
>  		return irq;
>  
> -	err = panthor_request_pwr_irq(
> -		ptdev, &pwr->irq, irq, PWR_INTERRUPTS_MASK,
> -		pwr->iomem + PWR_INT_BASE);
> +	err = panthor_request_pwr_irq(ptdev, &pwr->irq, irq,
> +				      pwr->iomem + PWR_INT_BASE);
>  	if (err)
>  		return err;
>  
> +	panthor_pwr_irq_enable_events(&pwr->irq, PWR_INTERRUPTS_MASK);
> +	panthor_pwr_irq_resume(&pwr->irq);
>  	return 0;
>  }
>  
> 
> -- 
> 2.54.0
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯