Re: [PATCH 04/10] drm/panthor: Extend the IRQ logic to allow fast/raw IRQ handlers

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

On Wed, Apr 29, 2026 at 11:38:31AM +0200, Boris Brezillon wrote:
> All drivers except panthor signal their fences from their interrupt
> handler to minimize latency. We could do the same from the interrupt
> handler, but the latency is still quite high in that case, so let's
> allow components to choose the context they want their IRQ handler
> to run in.

Starting here

> 
> This takes the form of an extra fast_handler() returning an irqreturn_t
> reflecting the need to wake-up a thread or not.
> A new PANTHOR_IRQ_ADV_HANDLER() macro taking this extra fast_handler
> argument is added, PANTHOR_IRQ_HANDLER() is implemented as a wrapper
> around PANTHOR_IRQ_ADV_HANDLER() with a default fast_handler
> returning IRQ_WAKE_THREAD.

up to here: there is no code matching the description. Left over from
earlier iteration?

> The fast and slow handler are still assumed
> to be mutually exclusive. In case a fast handler is provided, the
> slow_handler is expected to be run when the event can't be processed
> directly in the fast handler, or when the driver thinks it would be
> beneficial to coalesce interrupts by polling in the thread rather than
> re-enabling interrupts immediately.

This part is not really describing any code, just the intent. Maybe worth
moving it inside the code as a comment?

Otherwise, the change looks fine to me.

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

Best regards,
Liviu

> 
> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
>  drivers/gpu/drm/panthor/panthor_device.h | 5 ++---
>  drivers/gpu/drm/panthor/panthor_fw.c     | 1 +
>  drivers/gpu/drm/panthor/panthor_gpu.c    | 1 +
>  drivers/gpu/drm/panthor/panthor_mmu.c    | 1 +
>  drivers/gpu/drm/panthor/panthor_pwr.c    | 1 +
>  5 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panthor/panthor_device.h b/drivers/gpu/drm/panthor/panthor_device.h
> index afa202546316..1c130b8394ab 100644
> --- a/drivers/gpu/drm/panthor/panthor_device.h
> +++ b/drivers/gpu/drm/panthor/panthor_device.h
> @@ -672,6 +672,7 @@ static inline void panthor_irq_disable_events(struct panthor_irq *pirq, u32 mask
>  static inline int
>  panthor_irq_request(struct panthor_device *ptdev, struct panthor_irq *pirq,
>  		    int irq, u32 mask, void __iomem *iomem, const char *name,
> +		    irqreturn_t (*raw_handler)(int, void *data),
>  		    irqreturn_t (*threaded_handler)(int, void *data))
>  {
>  	const char *full_name;
> @@ -687,9 +688,7 @@ panthor_irq_request(struct panthor_device *ptdev, struct panthor_irq *pirq,
>  	if (!full_name)
>  		return -ENOMEM;
>  
> -	return devm_request_threaded_irq(ptdev->base.dev, irq,
> -					 panthor_irq_default_raw_handler,
> -					 threaded_handler,
> +	return devm_request_threaded_irq(ptdev->base.dev, irq, raw_handler, threaded_handler,
>  					 IRQF_SHARED, full_name, pirq);
>  }
>  
> diff --git a/drivers/gpu/drm/panthor/panthor_fw.c b/drivers/gpu/drm/panthor/panthor_fw.c
> index eaf599b0a887..8239a6951569 100644
> --- a/drivers/gpu/drm/panthor/panthor_fw.c
> +++ b/drivers/gpu/drm/panthor/panthor_fw.c
> @@ -1483,6 +1483,7 @@ int panthor_fw_init(struct panthor_device *ptdev)
>  
>  	ret = panthor_irq_request(ptdev, &fw->irq, irq, 0,
>  				  ptdev->iomem + JOB_INT_BASE, "job",
> +				  panthor_irq_default_raw_handler,
>  				  panthor_job_irq_threaded_handler);
>  	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 ce208e384762..d0be758ea3e1 100644
> --- a/drivers/gpu/drm/panthor/panthor_gpu.c
> +++ b/drivers/gpu/drm/panthor/panthor_gpu.c
> @@ -177,6 +177,7 @@ int panthor_gpu_init(struct panthor_device *ptdev)
>  	ret = panthor_irq_request(ptdev, &ptdev->gpu->irq, irq,
>  				  GPU_INTERRUPTS_MASK,
>  				  ptdev->iomem + GPU_INT_BASE, "gpu",
> +				  panthor_irq_default_raw_handler,
>  				  panthor_gpu_irq_threaded_handler);
>  	if (ret)
>  		return ret;
> diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/panthor/panthor_mmu.c
> index a0d0a9b2926f..2cb07933b629 100644
> --- a/drivers/gpu/drm/panthor/panthor_mmu.c
> +++ b/drivers/gpu/drm/panthor/panthor_mmu.c
> @@ -3260,6 +3260,7 @@ int panthor_mmu_init(struct panthor_device *ptdev)
>  	ret = panthor_irq_request(ptdev, &mmu->irq, irq,
>  				  panthor_mmu_fault_mask(ptdev, ~0),
>  				  ptdev->iomem + MMU_INT_BASE, "mmu",
> +				  panthor_irq_default_raw_handler,
>  				  panthor_mmu_irq_threaded_handler);
>  	if (ret)
>  		return ret;
> diff --git a/drivers/gpu/drm/panthor/panthor_pwr.c b/drivers/gpu/drm/panthor/panthor_pwr.c
> index 80cf78007896..1efb7f3482ba 100644
> --- a/drivers/gpu/drm/panthor/panthor_pwr.c
> +++ b/drivers/gpu/drm/panthor/panthor_pwr.c
> @@ -491,6 +491,7 @@ int panthor_pwr_init(struct panthor_device *ptdev)
>  	err = panthor_irq_request(
>  		ptdev, &pwr->irq, irq, PWR_INTERRUPTS_MASK,
>  		pwr->iomem + PWR_INT_BASE, "pwr",
> +		panthor_irq_default_raw_handler,
>  		panthor_pwr_irq_threaded_handler);
>  	if (err)
>  		return err;
> 
> -- 
> 2.53.0
> 

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