public inbox for dri-devel@lists.freedesktop.org
 help / color / mirror / Atom feed
From: Boris Brezillon <boris.brezillon@collabora.com>
To: Karunika Choo <karunika.choo@arm.com>
Cc: dri-devel@lists.freedesktop.org, nd@arm.com,
	Steven Price <steven.price@arm.com>,
	Liviu Dudau <liviu.dudau@arm.com>,
	Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	Maxime Ripard <mripard@kernel.org>,
	Thomas Zimmermann <tzimmermann@suse.de>,
	David Airlie <airlied@gmail.com>, Simona Vetter <simona@ffwll.ch>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 4/8] drm/panthor: Store IRQ register base iomem pointer in panthor_irq
Date: Mon, 13 Apr 2026 09:46:11 +0200	[thread overview]
Message-ID: <20260413094611.12f4c21a@fedora> (raw)
In-Reply-To: <20260412142951.2309135-5-karunika.choo@arm.com>

On Sun, 12 Apr 2026 15:29:47 +0100
Karunika Choo <karunika.choo@arm.com> wrote:

> Update common IRQ handling code to work from an IRQ-local iomem base
> instead of referencing block-specific interrupt register offsets.
> 
> Store the interrupt base address iomem pointer in struct panthor_irq and
> switch the shared IRQ helpers to use generic INT_* offsets from that
> local base. This removes the need for each caller to expose absolute IRQ
> register addresses while keeping the common IRQ flow unchanged.
> 
> No functional change intended.
> 
> v2:
> - Change IRQ request function to accept an iomem pointer instead of
>   computing it from an offset argument.
> 
> Signed-off-by: Karunika Choo <karunika.choo@arm.com>

Acked-by: Boris Brezillon <boris.brezillon@collabora.com>

> ---
>  drivers/gpu/drm/panthor/panthor_device.h   | 32 ++++++++++++++--------
>  drivers/gpu/drm/panthor/panthor_fw.c       |  5 ++--
>  drivers/gpu/drm/panthor/panthor_fw_regs.h  |  2 ++
>  drivers/gpu/drm/panthor/panthor_gpu.c      |  6 ++--
>  drivers/gpu/drm/panthor/panthor_gpu_regs.h |  3 ++
>  drivers/gpu/drm/panthor/panthor_mmu.c      |  5 ++--
>  drivers/gpu/drm/panthor/panthor_mmu_regs.h |  3 ++
>  drivers/gpu/drm/panthor/panthor_pwr.c      |  6 ++--
>  8 files changed, 42 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panthor/panthor_device.h b/drivers/gpu/drm/panthor/panthor_device.h
> index 285bf7e4439e..4e4607bca7cc 100644
> --- a/drivers/gpu/drm/panthor/panthor_device.h
> +++ b/drivers/gpu/drm/panthor/panthor_device.h
> @@ -82,6 +82,9 @@ struct panthor_irq {
>  	/** @ptdev: Panthor device */
>  	struct panthor_device *ptdev;
>  
> +	/** @iomem: CPU mapping of IRQ base address */
> +	void __iomem *iomem;
> +
>  	/** @irq: IRQ number. */
>  	int irq;
>  
> @@ -488,6 +491,11 @@ panthor_exception_is_fault(u32 exception_code)
>  const char *panthor_exception_name(struct panthor_device *ptdev,
>  				   u32 exception_code);
>  
> +#define INT_RAWSTAT 0x0
> +#define INT_CLEAR   0x4
> +#define INT_MASK    0x8
> +#define INT_STAT    0xc
> +
>  /**
>   * PANTHOR_IRQ_HANDLER() - Define interrupt handlers and the interrupt
>   * registration function.
> @@ -498,14 +506,13 @@ const char *panthor_exception_name(struct panthor_device *ptdev,
>   *
>   * void (*handler)(struct panthor_device *, u32 status);
>   */
> -#define PANTHOR_IRQ_HANDLER(__name, __reg_prefix, __handler)					\
> +#define PANTHOR_IRQ_HANDLER(__name, __handler)							\
>  static irqreturn_t panthor_ ## __name ## _irq_raw_handler(int irq, void *data)			\
>  {												\
>  	struct panthor_irq *pirq = data;							\
> -	struct panthor_device *ptdev = pirq->ptdev;						\
>  	enum panthor_irq_state old_state;							\
>  												\
> -	if (!gpu_read(ptdev->iomem, __reg_prefix ## _INT_STAT))					\
> +	if (!gpu_read(pirq->iomem, INT_STAT))							\
>  		return IRQ_NONE;								\
>  												\
>  	guard(spinlock_irqsave)(&pirq->mask_lock);						\
> @@ -515,7 +522,7 @@ static irqreturn_t panthor_ ## __name ## _irq_raw_handler(int irq, void *data)
>  	if (old_state != PANTHOR_IRQ_STATE_ACTIVE)						\
>  		return IRQ_NONE;								\
>  												\
> -	gpu_write(ptdev->iomem, __reg_prefix ## _INT_MASK, 0);					\
> +	gpu_write(pirq->iomem, INT_MASK, 0);							\
>  	return IRQ_WAKE_THREAD;									\
>  }												\
>  												\
> @@ -534,7 +541,7 @@ static irqreturn_t panthor_ ## __name ## _irq_threaded_handler(int irq, void *da
>  		 * right before the HW event kicks in. TLDR; it's all expected races we're	\
>  		 * covered for.									\
>  		 */										\
> -		u32 status = gpu_read(ptdev->iomem, __reg_prefix ## _INT_RAWSTAT) & pirq->mask;	\
> +		u32 status = gpu_read(pirq->iomem, INT_RAWSTAT) & pirq->mask;			\
>  												\
>  		if (!status)									\
>  			break;									\
> @@ -550,7 +557,7 @@ static irqreturn_t panthor_ ## __name ## _irq_threaded_handler(int irq, void *da
>  					   PANTHOR_IRQ_STATE_PROCESSING,			\
>  					   PANTHOR_IRQ_STATE_ACTIVE);				\
>  		if (old_state == PANTHOR_IRQ_STATE_PROCESSING)					\
> -			gpu_write(ptdev->iomem, __reg_prefix ## _INT_MASK, pirq->mask);		\
> +			gpu_write(pirq->iomem, INT_MASK, pirq->mask);				\
>  	}											\
>  												\
>  	return ret;										\
> @@ -560,7 +567,7 @@ static inline void panthor_ ## __name ## _irq_suspend(struct panthor_irq *pirq)
>  {												\
>  	scoped_guard(spinlock_irqsave, &pirq->mask_lock) {					\
>  		atomic_set(&pirq->state, PANTHOR_IRQ_STATE_SUSPENDING);				\
> -		gpu_write(pirq->ptdev->iomem, __reg_prefix ## _INT_MASK, 0);			\
> +		gpu_write(pirq->iomem, INT_MASK, 0);						\
>  	}											\
>  	synchronize_irq(pirq->irq);								\
>  	atomic_set(&pirq->state, PANTHOR_IRQ_STATE_SUSPENDED);					\
> @@ -571,17 +578,18 @@ static inline void panthor_ ## __name ## _irq_resume(struct panthor_irq *pirq)
>  	guard(spinlock_irqsave)(&pirq->mask_lock);						\
>  												\
>  	atomic_set(&pirq->state, PANTHOR_IRQ_STATE_ACTIVE);					\
> -	gpu_write(pirq->ptdev->iomem, __reg_prefix ## _INT_CLEAR, pirq->mask);			\
> -	gpu_write(pirq->ptdev->iomem, __reg_prefix ## _INT_MASK, pirq->mask);			\
> +	gpu_write(pirq->iomem, INT_CLEAR, pirq->mask);						\
> +	gpu_write(pirq->iomem, INT_MASK, pirq->mask);						\
>  }												\
>  												\
>  static int panthor_request_ ## __name ## _irq(struct panthor_device *ptdev,			\
>  					      struct panthor_irq *pirq,				\
> -					      int irq, u32 mask)				\
> +					      int irq, u32 mask, void __iomem *iomem)		\
>  {												\
>  	pirq->ptdev = ptdev;									\
>  	pirq->irq = irq;									\
>  	pirq->mask = mask;									\
> +	pirq->iomem = iomem;									\
>  	spin_lock_init(&pirq->mask_lock);							\
>  	panthor_ ## __name ## _irq_resume(pirq);						\
>  												\
> @@ -603,7 +611,7 @@ static inline void panthor_ ## __name ## _irq_enable_events(struct panthor_irq *
>  	 * If the IRQ is suspended/suspending, the mask is restored at resume time.		\
>  	 */											\
>  	if (atomic_read(&pirq->state) == PANTHOR_IRQ_STATE_ACTIVE)				\
> -		gpu_write(pirq->ptdev->iomem, __reg_prefix ## _INT_MASK, pirq->mask);		\
> +		gpu_write(pirq->iomem, INT_MASK, pirq->mask);					\
>  }												\
>  												\
>  static inline void panthor_ ## __name ## _irq_disable_events(struct panthor_irq *pirq, u32 mask)\
> @@ -617,7 +625,7 @@ static inline void panthor_ ## __name ## _irq_disable_events(struct panthor_irq
>  	 * If the IRQ is suspended/suspending, the mask is restored at resume time.		\
>  	 */											\
>  	if (atomic_read(&pirq->state) == PANTHOR_IRQ_STATE_ACTIVE)				\
> -		gpu_write(pirq->ptdev->iomem, __reg_prefix ## _INT_MASK, pirq->mask);		\
> +		gpu_write(pirq->iomem, INT_MASK, pirq->mask);					\
>  }
>  
>  extern struct workqueue_struct *panthor_cleanup_wq;
> diff --git a/drivers/gpu/drm/panthor/panthor_fw.c b/drivers/gpu/drm/panthor/panthor_fw.c
> index 4a0c23e987b6..73ef07a37e22 100644
> --- a/drivers/gpu/drm/panthor/panthor_fw.c
> +++ b/drivers/gpu/drm/panthor/panthor_fw.c
> @@ -1088,7 +1088,7 @@ static void panthor_job_irq_handler(struct panthor_device *ptdev, u32 status)
>  		trace_gpu_job_irq(ptdev->base.dev, status, duration);
>  	}
>  }
> -PANTHOR_IRQ_HANDLER(job, JOB, panthor_job_irq_handler);
> +PANTHOR_IRQ_HANDLER(job, panthor_job_irq_handler);
>  
>  static int panthor_fw_start(struct panthor_device *ptdev)
>  {
> @@ -1470,7 +1470,8 @@ 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, 0,
> +				      ptdev->iomem + JOB_INT_BASE);
>  	if (ret) {
>  		drm_err(&ptdev->base, "failed to request job irq");
>  		return ret;
> diff --git a/drivers/gpu/drm/panthor/panthor_fw_regs.h b/drivers/gpu/drm/panthor/panthor_fw_regs.h
> index d523d41e18dd..eeb41aff249b 100644
> --- a/drivers/gpu/drm/panthor/panthor_fw_regs.h
> +++ b/drivers/gpu/drm/panthor/panthor_fw_regs.h
> @@ -15,6 +15,8 @@
>  #define   MCU_STATUS_HALT				2
>  #define   MCU_STATUS_FATAL				3
>  
> +#define JOB_INT_BASE					0x1000
> +
>  #define JOB_INT_RAWSTAT					0x1000
>  #define JOB_INT_CLEAR					0x1004
>  #define JOB_INT_MASK					0x1008
> diff --git a/drivers/gpu/drm/panthor/panthor_gpu.c b/drivers/gpu/drm/panthor/panthor_gpu.c
> index 747ac332be64..f00f3d9be240 100644
> --- a/drivers/gpu/drm/panthor/panthor_gpu.c
> +++ b/drivers/gpu/drm/panthor/panthor_gpu.c
> @@ -110,7 +110,7 @@ static void panthor_gpu_irq_handler(struct panthor_device *ptdev, u32 status)
>  	}
>  	spin_unlock(&ptdev->gpu->reqs_lock);
>  }
> -PANTHOR_IRQ_HANDLER(gpu, GPU, panthor_gpu_irq_handler);
> +PANTHOR_IRQ_HANDLER(gpu, panthor_gpu_irq_handler);
>  
>  /**
>   * panthor_gpu_unplug() - Called when the GPU is unplugged.
> @@ -162,7 +162,9 @@ int panthor_gpu_init(struct panthor_device *ptdev)
>  	if (irq < 0)
>  		return irq;
>  
> -	ret = panthor_request_gpu_irq(ptdev, &ptdev->gpu->irq, irq, GPU_INTERRUPTS_MASK);
> +	ret = panthor_request_gpu_irq(ptdev, &ptdev->gpu->irq, irq,
> +				      GPU_INTERRUPTS_MASK,
> +				      ptdev->iomem + GPU_INT_BASE);
>  	if (ret)
>  		return ret;
>  
> diff --git a/drivers/gpu/drm/panthor/panthor_gpu_regs.h b/drivers/gpu/drm/panthor/panthor_gpu_regs.h
> index 75474b7d7341..3f60c45985a7 100644
> --- a/drivers/gpu/drm/panthor/panthor_gpu_regs.h
> +++ b/drivers/gpu/drm/panthor/panthor_gpu_regs.h
> @@ -4,6 +4,8 @@
>  #ifndef __PANTHOR_GPU_REGS_H__
>  #define __PANTHOR_GPU_REGS_H__
>  
> +#define GPU_CONTROL_BASE				0x0
> +
>  #define GPU_ID						0x0
>  #define   GPU_ARCH_MAJOR(x)				((x) >> 28)
>  #define   GPU_ARCH_MINOR(x)				(((x) & GENMASK(27, 24)) >> 24)
> @@ -28,6 +30,7 @@
>  #define GPU_AS_PRESENT					0x18
>  #define GPU_CSF_ID					0x1C
>  
> +#define GPU_INT_BASE					0x20
>  #define GPU_INT_RAWSTAT					0x20
>  #define GPU_INT_CLEAR					0x24
>  #define GPU_INT_MASK					0x28
> diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/panthor/panthor_mmu.c
> index b9f6031e24a4..501dc0ea3ae7 100644
> --- a/drivers/gpu/drm/panthor/panthor_mmu.c
> +++ b/drivers/gpu/drm/panthor/panthor_mmu.c
> @@ -584,7 +584,7 @@ static u32 panthor_mmu_as_fault_mask(struct panthor_device *ptdev, u32 as)
>  
>  /* Forward declaration to call helpers within as_enable/disable */
>  static void panthor_mmu_irq_handler(struct panthor_device *ptdev, u32 status);
> -PANTHOR_IRQ_HANDLER(mmu, MMU, panthor_mmu_irq_handler);
> +PANTHOR_IRQ_HANDLER(mmu, panthor_mmu_irq_handler);
>  
>  static int panthor_mmu_as_enable(struct panthor_device *ptdev, u32 as_nr,
>  				 u64 transtab, u64 transcfg, u64 memattr)
> @@ -3229,7 +3229,8 @@ 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));
> +				      panthor_mmu_fault_mask(ptdev, ~0),
> +				      ptdev->iomem + MMU_INT_BASE);
>  	if (ret)
>  		return ret;
>  
> diff --git a/drivers/gpu/drm/panthor/panthor_mmu_regs.h b/drivers/gpu/drm/panthor/panthor_mmu_regs.h
> index cc9cf603cec6..de460042651d 100644
> --- a/drivers/gpu/drm/panthor/panthor_mmu_regs.h
> +++ b/drivers/gpu/drm/panthor/panthor_mmu_regs.h
> @@ -5,6 +5,9 @@
>  #define __PANTHOR_MMU_REGS_H__
>  
>  /* MMU regs */
> +
> +#define MMU_INT_BASE					0x2000
> +
>  #define MMU_INT_RAWSTAT					0x2000
>  #define MMU_INT_CLEAR					0x2004
>  #define MMU_INT_MASK					0x2008
> diff --git a/drivers/gpu/drm/panthor/panthor_pwr.c b/drivers/gpu/drm/panthor/panthor_pwr.c
> index aafb0c5c7d23..11c43de1ddd5 100644
> --- a/drivers/gpu/drm/panthor/panthor_pwr.c
> +++ b/drivers/gpu/drm/panthor/panthor_pwr.c
> @@ -70,7 +70,7 @@ static void panthor_pwr_irq_handler(struct panthor_device *ptdev, u32 status)
>  	}
>  	spin_unlock(&ptdev->pwr->reqs_lock);
>  }
> -PANTHOR_IRQ_HANDLER(pwr, PWR, panthor_pwr_irq_handler);
> +PANTHOR_IRQ_HANDLER(pwr, panthor_pwr_irq_handler);
>  
>  static void panthor_pwr_write_command(struct panthor_device *ptdev, u32 command, u64 args)
>  {
> @@ -464,7 +464,9 @@ 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);
> +	err = panthor_request_pwr_irq(
> +		ptdev, &pwr->irq, irq, PWR_INTERRUPTS_MASK,
> +		ptdev->iomem + GPU_CONTROL_BASE + PWR_CONTROL_BASE);
>  	if (err)
>  		return err;
>  


  reply	other threads:[~2026-04-13  7:46 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-12 14:29 [PATCH v2 0/8] drm/panthor: Localize register access by component Karunika Choo
2026-04-12 14:29 ` [PATCH v2 1/8] drm/panthor: Pass an iomem pointer to GPU register access helpers Karunika Choo
2026-04-15 11:46   ` Liviu Dudau
2026-04-12 14:29 ` [PATCH v2 2/8] drm/panthor: Split register definitions by components Karunika Choo
2026-04-13  7:43   ` Boris Brezillon
2026-04-15 11:47   ` Liviu Dudau
2026-04-12 14:29 ` [PATCH v2 3/8] drm/panthor: Replace cross-component register accesses with helpers Karunika Choo
2026-04-13  7:44   ` Boris Brezillon
2026-04-15 11:48   ` Liviu Dudau
2026-04-12 14:29 ` [PATCH v2 4/8] drm/panthor: Store IRQ register base iomem pointer in panthor_irq Karunika Choo
2026-04-13  7:46   ` Boris Brezillon [this message]
2026-04-15 12:16   ` Liviu Dudau
2026-04-12 14:29 ` [PATCH v2 5/8] drm/panthor: Use a local iomem base for GPU registers Karunika Choo
2026-04-15 12:19   ` Liviu Dudau
2026-04-12 14:29 ` [PATCH v2 6/8] drm/panthor: Use a local iomem base for PWR registers Karunika Choo
2026-04-13  7:51   ` Boris Brezillon
2026-04-15 12:21   ` Liviu Dudau
2026-04-12 14:29 ` [PATCH v2 7/8] drm/panthor: Use a local iomem base for firmware control registers Karunika Choo
2026-04-15 12:22   ` Liviu Dudau
2026-04-12 14:29 ` [PATCH v2 8/8] drm/panthor: Use a local iomem base for MMU AS registers Karunika Choo
2026-04-15 12:23   ` Liviu Dudau

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260413094611.12f4c21a@fedora \
    --to=boris.brezillon@collabora.com \
    --cc=airlied@gmail.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=karunika.choo@arm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=liviu.dudau@arm.com \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=mripard@kernel.org \
    --cc=nd@arm.com \
    --cc=simona@ffwll.ch \
    --cc=steven.price@arm.com \
    --cc=tzimmermann@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox