From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 0C2C935C1A9 for ; Mon, 13 Apr 2026 10:39:17 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=217.140.110.172 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1776076759; cv=none; b=KXrfVT1knyNAAm9hQm006QmgH/Y1+Jo54d6sa/VPljEJHKZEPQ68bj56Hvi8ojvXfRyiS3ubUMtTQn3Up77WLDUJNx/aTpFMsEaYFtFzCpANDqmjJ8TUs5TtjEH7cjNdZuQYPYqjI8JIK83A7whrVjITQ6kgNs/JKVyAzvq0vWY= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1776076759; c=relaxed/simple; bh=/BAHsMeDpxaSk3A6Tn1Qq0vSm9nyLodQ3ltCXaIKXf4=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=E/EfSzl2JJfd4NfawObuBXaEG51qDIDxHlkArFJk1UeWnynfiq/I2K8JezmqAifKx89P9/T/eklnjGVs4CbPfxDQm8X/CB5cpXY7EYIGruhCQBJqdFWAG0grvhGZVgXVp1eLuFA7zkWvqcqc1X4Dsqu1QI2EiJxxGZ6inwljcoY= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com; spf=pass smtp.mailfrom=arm.com; dkim=pass (1024-bit key) header.d=arm.com header.i=@arm.com header.b=IUryZnM7; arc=none smtp.client-ip=217.140.110.172 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=arm.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=arm.com header.i=@arm.com header.b="IUryZnM7" Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id B60363548 for ; Mon, 13 Apr 2026 03:39:11 -0700 (PDT) Received: from [192.168.0.1] (usa-sjc-imap-foss1.foss.arm.com [10.121.207.14]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPA id 3016D3F641 for ; Mon, 13 Apr 2026 03:39:17 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=arm.com; s=foss; t=1776076757; bh=/BAHsMeDpxaSk3A6Tn1Qq0vSm9nyLodQ3ltCXaIKXf4=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=IUryZnM7o72Byi/cOAowX9QrItnHYu+1pCJvgGwVxfMfIxWvFvZSrlcXgDrCO055P PoXWWNECv+z8e10Az0FtE9hKQBybwPG8eiVsL7eq6uR1kkT4hCShayUAQUkjoLptDh UgT+RNXKpdESvHympe1QRbBRcZyTb9wUtKGihUjA= Date: Mon, 13 Apr 2026 11:39:06 +0100 From: Liviu Dudau To: Karunika Choo Cc: dri-devel@lists.freedesktop.org, nd@arm.com, Boris Brezillon , Steven Price , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , David Airlie , Simona Vetter , linux-kernel@vger.kernel.org Subject: Re: [PATCH 5/8] drm/panthor: Use a local iomem base for GPU registers Message-ID: References: <20260410164637.549145-1-karunika.choo@arm.com> <20260410164637.549145-6-karunika.choo@arm.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20260410164637.549145-6-karunika.choo@arm.com> On Fri, Apr 10, 2026 at 05:46:34PM +0100, Karunika Choo 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 Reviewed-by: Liviu Dudau Best regards, Liviu > --- > 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) > -- > 2.43.0 > -- ==================== | I would like to | | fix the world, | | but they're not | | giving me the | \ source code! / --------------- ¯\_(ツ)_/¯