All of lore.kernel.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: 29+ 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-22  9:34   ` Steven Price
2026-04-22 16:08     ` Karunika Choo
2026-04-24 10:38       ` Steven Price
2026-04-24 11:03         ` Boris Brezillon
2026-04-24 11:20           ` Steven Price
2026-04-24 12:09             ` Boris Brezillon
2026-04-27 16:08               ` Karunika Choo
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
2026-04-22  9:34 ` [PATCH v2 0/8] drm/panthor: Localize register access by component Steven Price

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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.