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 5/8] drm/panthor: Use a local iomem base for GPU registers
Date: Fri, 10 Apr 2026 20:11:53 +0200 [thread overview]
Message-ID: <20260410201153.1e61d880@fedora> (raw)
In-Reply-To: <20260410164637.549145-6-karunika.choo@arm.com>
On Fri, 10 Apr 2026 17:46:34 +0100
Karunika Choo <karunika.choo@arm.com> wrote:
> Add a GPU_CONTROL-local iomem pointer to struct panthor_gpu and use it
> for GPU register accesses.
>
> This limits GPU register accesses to the GPU block instead of using the
> device-wide MMIO mapping directly. Interrupt register accesses continue
> to use the IRQ-local base provided by the common IRQ helpers.
>
> This is a refactoring only and does not change behaviour.
>
> Signed-off-by: Karunika Choo <karunika.choo@arm.com>
Acked-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
> drivers/gpu/drm/panthor/panthor_gpu.c | 61 +++++++++++++---------
> drivers/gpu/drm/panthor/panthor_gpu_regs.h | 6 +--
> 2 files changed, 38 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/gpu/drm/panthor/panthor_gpu.c b/drivers/gpu/drm/panthor/panthor_gpu.c
> index 3ddce35ed8b5..abd94de5d15d 100644
> --- a/drivers/gpu/drm/panthor/panthor_gpu.c
> +++ b/drivers/gpu/drm/panthor/panthor_gpu.c
> @@ -29,6 +29,9 @@
> * struct panthor_gpu - GPU block management data.
> */
> struct panthor_gpu {
> + /** @iomem: CPU mapping of GPU_CONTROL iomem region */
> + void __iomem *iomem;
> +
> /** @irq: GPU irq. */
> struct panthor_irq irq;
>
> @@ -56,12 +59,13 @@ struct panthor_gpu {
>
> static void panthor_gpu_coherency_set(struct panthor_device *ptdev)
> {
> - gpu_write(ptdev->iomem, GPU_COHERENCY_PROTOCOL,
> + gpu_write(ptdev->gpu->iomem, GPU_COHERENCY_PROTOCOL,
> ptdev->gpu_info.selected_coherency);
> }
>
> static void panthor_gpu_l2_config_set(struct panthor_device *ptdev)
> {
> + struct panthor_gpu *gpu = ptdev->gpu;
> const struct panthor_soc_data *data = ptdev->soc_data;
> u32 l2_config;
> u32 i;
> @@ -75,26 +79,28 @@ static void panthor_gpu_l2_config_set(struct panthor_device *ptdev)
> }
>
> for (i = 0; i < ARRAY_SIZE(data->asn_hash); i++)
> - gpu_write(ptdev->iomem, GPU_ASN_HASH(i), data->asn_hash[i]);
> + gpu_write(gpu->iomem, GPU_ASN_HASH(i), data->asn_hash[i]);
>
> - l2_config = gpu_read(ptdev->iomem, GPU_L2_CONFIG);
> + l2_config = gpu_read(gpu->iomem, GPU_L2_CONFIG);
> l2_config |= GPU_L2_CONFIG_ASN_HASH_ENABLE;
> - gpu_write(ptdev->iomem, GPU_L2_CONFIG, l2_config);
> + gpu_write(gpu->iomem, GPU_L2_CONFIG, l2_config);
> }
>
> static void panthor_gpu_irq_handler(struct panthor_device *ptdev, u32 status)
> {
> - gpu_write(ptdev->iomem, GPU_INT_CLEAR, status);
> + struct panthor_gpu *gpu = ptdev->gpu;
> +
> + gpu_write(gpu->irq.iomem, INT_CLEAR, status);
>
> if (tracepoint_enabled(gpu_power_status) && (status & GPU_POWER_INTERRUPTS_MASK))
> trace_gpu_power_status(ptdev->base.dev,
> - gpu_read64(ptdev->iomem, SHADER_READY),
> - gpu_read64(ptdev->iomem, TILER_READY),
> - gpu_read64(ptdev->iomem, L2_READY));
> + gpu_read64(gpu->iomem, SHADER_READY),
> + gpu_read64(gpu->iomem, TILER_READY),
> + gpu_read64(gpu->iomem, L2_READY));
>
> if (status & GPU_IRQ_FAULT) {
> - u32 fault_status = gpu_read(ptdev->iomem, GPU_FAULT_STATUS);
> - u64 address = gpu_read64(ptdev->iomem, GPU_FAULT_ADDR);
> + u32 fault_status = gpu_read(gpu->iomem, GPU_FAULT_STATUS);
> + u64 address = gpu_read64(gpu->iomem, GPU_FAULT_ADDR);
>
> drm_warn(&ptdev->base, "GPU Fault 0x%08x (%s) at 0x%016llx\n",
> fault_status, panthor_exception_name(ptdev, fault_status & 0xFF),
> @@ -147,6 +153,7 @@ int panthor_gpu_init(struct panthor_device *ptdev)
> if (!gpu)
> return -ENOMEM;
>
> + gpu->iomem = ptdev->iomem + GPU_CONTROL_BASE;
> spin_lock_init(&gpu->reqs_lock);
> init_waitqueue_head(&gpu->reqs_acked);
> mutex_init(&gpu->cache_flush_lock);
> @@ -202,10 +209,11 @@ int panthor_gpu_block_power_off(struct panthor_device *ptdev,
> u32 pwroff_reg, u32 pwrtrans_reg,
> u64 mask, u32 timeout_us)
> {
> + struct panthor_gpu *gpu = ptdev->gpu;
> u32 val;
> int ret;
>
> - ret = gpu_read64_relaxed_poll_timeout(ptdev->iomem, pwrtrans_reg, val,
> + ret = gpu_read64_relaxed_poll_timeout(gpu->iomem, pwrtrans_reg, val,
> !(mask & val), 100, timeout_us);
> if (ret) {
> drm_err(&ptdev->base,
> @@ -214,9 +222,9 @@ int panthor_gpu_block_power_off(struct panthor_device *ptdev,
> return ret;
> }
>
> - gpu_write64(ptdev->iomem, pwroff_reg, mask);
> + gpu_write64(gpu->iomem, pwroff_reg, mask);
>
> - ret = gpu_read64_relaxed_poll_timeout(ptdev->iomem, pwrtrans_reg, val,
> + ret = gpu_read64_relaxed_poll_timeout(gpu->iomem, pwrtrans_reg, val,
> !(mask & val), 100, timeout_us);
> if (ret) {
> drm_err(&ptdev->base,
> @@ -245,10 +253,11 @@ int panthor_gpu_block_power_on(struct panthor_device *ptdev,
> u32 pwron_reg, u32 pwrtrans_reg,
> u32 rdy_reg, u64 mask, u32 timeout_us)
> {
> + struct panthor_gpu *gpu = ptdev->gpu;
> u32 val;
> int ret;
>
> - ret = gpu_read64_relaxed_poll_timeout(ptdev->iomem, pwrtrans_reg, val,
> + ret = gpu_read64_relaxed_poll_timeout(gpu->iomem, pwrtrans_reg, val,
> !(mask & val), 100, timeout_us);
> if (ret) {
> drm_err(&ptdev->base,
> @@ -257,9 +266,9 @@ int panthor_gpu_block_power_on(struct panthor_device *ptdev,
> return ret;
> }
>
> - gpu_write64(ptdev->iomem, pwron_reg, mask);
> + gpu_write64(gpu->iomem, pwron_reg, mask);
>
> - ret = gpu_read64_relaxed_poll_timeout(ptdev->iomem, rdy_reg, val,
> + ret = gpu_read64_relaxed_poll_timeout(gpu->iomem, rdy_reg, val,
> (mask & val) == val,
> 100, timeout_us);
> if (ret) {
> @@ -318,6 +327,7 @@ int panthor_gpu_l2_power_on(struct panthor_device *ptdev)
> int panthor_gpu_flush_caches(struct panthor_device *ptdev,
> u32 l2, u32 lsc, u32 other)
> {
> + struct panthor_gpu *gpu = ptdev->gpu;
> unsigned long flags;
> int ret = 0;
>
> @@ -327,7 +337,7 @@ int panthor_gpu_flush_caches(struct panthor_device *ptdev,
> spin_lock_irqsave(&ptdev->gpu->reqs_lock, flags);
> if (!(ptdev->gpu->pending_reqs & GPU_IRQ_CLEAN_CACHES_COMPLETED)) {
> ptdev->gpu->pending_reqs |= GPU_IRQ_CLEAN_CACHES_COMPLETED;
> - gpu_write(ptdev->iomem, GPU_CMD, GPU_FLUSH_CACHES(l2, lsc, other));
> + gpu_write(gpu->iomem, GPU_CMD, GPU_FLUSH_CACHES(l2, lsc, other));
> } else {
> ret = -EIO;
> }
> @@ -341,7 +351,7 @@ int panthor_gpu_flush_caches(struct panthor_device *ptdev,
> msecs_to_jiffies(100))) {
> spin_lock_irqsave(&ptdev->gpu->reqs_lock, flags);
> if ((ptdev->gpu->pending_reqs & GPU_IRQ_CLEAN_CACHES_COMPLETED) != 0 &&
> - !(gpu_read(ptdev->iomem, GPU_INT_RAWSTAT) & GPU_IRQ_CLEAN_CACHES_COMPLETED))
> + !(gpu_read(gpu->irq.iomem, INT_RAWSTAT) & GPU_IRQ_CLEAN_CACHES_COMPLETED))
> ret = -ETIMEDOUT;
> else
> ptdev->gpu->pending_reqs &= ~GPU_IRQ_CLEAN_CACHES_COMPLETED;
> @@ -364,6 +374,7 @@ int panthor_gpu_flush_caches(struct panthor_device *ptdev,
> */
> int panthor_gpu_soft_reset(struct panthor_device *ptdev)
> {
> + struct panthor_gpu *gpu = ptdev->gpu;
> bool timedout = false;
> unsigned long flags;
>
> @@ -371,8 +382,8 @@ int panthor_gpu_soft_reset(struct panthor_device *ptdev)
> if (!drm_WARN_ON(&ptdev->base,
> ptdev->gpu->pending_reqs & GPU_IRQ_RESET_COMPLETED)) {
> ptdev->gpu->pending_reqs |= GPU_IRQ_RESET_COMPLETED;
> - gpu_write(ptdev->iomem, GPU_INT_CLEAR, GPU_IRQ_RESET_COMPLETED);
> - gpu_write(ptdev->iomem, GPU_CMD, GPU_SOFT_RESET);
> + gpu_write(gpu->irq.iomem, INT_CLEAR, GPU_IRQ_RESET_COMPLETED);
> + gpu_write(gpu->iomem, GPU_CMD, GPU_SOFT_RESET);
> }
> spin_unlock_irqrestore(&ptdev->gpu->reqs_lock, flags);
>
> @@ -381,7 +392,7 @@ int panthor_gpu_soft_reset(struct panthor_device *ptdev)
> msecs_to_jiffies(100))) {
> spin_lock_irqsave(&ptdev->gpu->reqs_lock, flags);
> if ((ptdev->gpu->pending_reqs & GPU_IRQ_RESET_COMPLETED) != 0 &&
> - !(gpu_read(ptdev->iomem, GPU_INT_RAWSTAT) & GPU_IRQ_RESET_COMPLETED))
> + !(gpu_read(gpu->irq.iomem, INT_RAWSTAT) & GPU_IRQ_RESET_COMPLETED))
> timedout = true;
> else
> ptdev->gpu->pending_reqs &= ~GPU_IRQ_RESET_COMPLETED;
> @@ -430,17 +441,17 @@ void panthor_gpu_resume(struct panthor_device *ptdev)
>
> u64 panthor_gpu_get_timestap(struct panthor_device *ptdev)
> {
> - return gpu_read64_counter(ptdev->iomem, GPU_TIMESTAMP);
> + return gpu_read64_counter(ptdev->gpu->iomem, GPU_TIMESTAMP);
> }
>
> u64 panthor_gpu_get_timestap_offset(struct panthor_device *ptdev)
> {
> - return gpu_read64(ptdev->iomem, GPU_TIMESTAMP_OFFSET);
> + return gpu_read64(ptdev->gpu->iomem, GPU_TIMESTAMP_OFFSET);
> }
>
> u64 panthor_gpu_get_cycle_count(struct panthor_device *ptdev)
> {
> - return gpu_read64_counter(ptdev->iomem, GPU_CYCLE_COUNT);
> + return gpu_read64_counter(ptdev->gpu->iomem, GPU_CYCLE_COUNT);
> }
>
> int panthor_gpu_coherency_init(struct panthor_device *ptdev)
> @@ -459,7 +470,7 @@ int panthor_gpu_coherency_init(struct panthor_device *ptdev)
> /* Check if the ACE-Lite coherency protocol is actually supported by the GPU.
> * ACE protocol has never been supported for command stream frontend GPUs.
> */
> - if ((gpu_read(ptdev->iomem, GPU_COHERENCY_FEATURES) &
> + if ((gpu_read(ptdev->gpu->iomem, GPU_COHERENCY_FEATURES) &
> GPU_COHERENCY_PROT_BIT(ACE_LITE))) {
> ptdev->gpu_info.selected_coherency = GPU_COHERENCY_ACE_LITE;
> return 0;
> diff --git a/drivers/gpu/drm/panthor/panthor_gpu_regs.h b/drivers/gpu/drm/panthor/panthor_gpu_regs.h
> index d7cf5165e987..f64e7661f765 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_L2_FEATURES 0x4
> #define GPU_L2_FEATURES_LINE_SIZE(x) (1 << ((x) & GENMASK(7, 0)))
>
> @@ -20,10 +22,6 @@
> #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
> -#define GPU_INT_STAT 0x2c
> #define GPU_IRQ_FAULT BIT(0)
> #define GPU_IRQ_PROTM_FAULT BIT(1)
> #define GPU_IRQ_RESET_COMPLETED BIT(8)
next prev parent reply other threads:[~2026-04-10 18:11 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-04-10 16:46 [PATCH 0/8] drm/panthor: Localize register access by component Karunika Choo
2026-04-10 16:46 ` [PATCH 1/8] drm/panthor: Pass an iomem pointer to GPU register access helpers Karunika Choo
2026-04-10 18:11 ` Boris Brezillon
2026-04-10 16:46 ` [PATCH 2/8] drm/panthor: Split register definitions by components Karunika Choo
2026-04-10 18:08 ` Boris Brezillon
2026-04-10 16:46 ` [PATCH 3/8] drm/panthor: Replace cross-component register accesses with helpers Karunika Choo
2026-04-10 17:55 ` Boris Brezillon
2026-04-10 16:46 ` [PATCH 4/8] drm/panthor: Store IRQ register base iomem pointer in panthor_irq Karunika Choo
2026-04-10 17:53 ` Boris Brezillon
2026-04-10 16:46 ` [PATCH 5/8] drm/panthor: Use a local iomem base for GPU registers Karunika Choo
2026-04-10 18:11 ` Boris Brezillon [this message]
2026-04-10 16:46 ` [PATCH 6/8] drm/panthor: Use a local iomem base for PWR registers Karunika Choo
2026-04-10 18:12 ` Boris Brezillon
2026-04-10 16:46 ` [PATCH 7/8] drm/panthor: Use a local iomem base for firmware control registers Karunika Choo
2026-04-10 18:12 ` Boris Brezillon
2026-04-10 16:46 ` [PATCH 8/8] drm/panthor: Use a local iomem base for MMU AS registers Karunika Choo
2026-04-10 18:13 ` Boris Brezillon
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=20260410201153.1e61d880@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