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 A86FC31A567 for ; Mon, 13 Apr 2026 10:30:06 +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=1776076208; cv=none; b=Uv3BZR3ZBS9+NHKnlfJwTc9brw7zLVnkAZ5JjDmrRgOivPNOaDcWJuy+vlFNpBO7lBXXbuvSfJ+tGmrqHVWl2pSXIEpoYu7OqtDV3onwZ7LnFk3HSlqBEs7pHdSt5W2KaTG2LCfFTTTtt8wBy3gFYXAm647Yq6jHSJVRD+ioW24= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1776076208; c=relaxed/simple; bh=00LIVjWWByiSGB/8MkpuDBq/0IcUH17vn5Bq9kGtYKg=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=IqDEE+SIAI3UKig3wsRSnegg7mtH1QSMlQFllO+R+CzmyB7Xhn7QU1uhIsIhhXw8PfxZJOdnF4S0nB9lGIn8VgW+RmGfO/qfTvpHQ/cz44KEvlASqexoPG9V4I0/vPW+pPRyL9Po/uS/3T6A0vFrzqtC7lYzhN3kNHFLZmxJbI4= 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=f/iLSznC; 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="f/iLSznC" 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 456E816F8 for ; Mon, 13 Apr 2026 03:30:00 -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 B73D33F641 for ; Mon, 13 Apr 2026 03:30:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=arm.com; s=foss; t=1776076205; bh=00LIVjWWByiSGB/8MkpuDBq/0IcUH17vn5Bq9kGtYKg=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=f/iLSznC8M/swAUu5xQoCuQbomOx/unUAfF0Ib6aRU96m8/iklmGFA+VuuisJzigV ZNZpk1tlOAvch3akqBIgIXx5+5xXE4QtQiQTb3V8o/1A0WPvueBqmMAyN/aHd84njj KeE4BZcTJYgD6v8H7AZJ2fI5hrBi2JtjQjRT4QJw= Date: Mon, 13 Apr 2026 11:29:55 +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 3/8] drm/panthor: Replace cross-component register accesses with helpers Message-ID: References: <20260410164637.549145-1-karunika.choo@arm.com> <20260410164637.549145-4-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-4-karunika.choo@arm.com> On Fri, Apr 10, 2026 at 05:46:32PM +0100, Karunika Choo wrote: > Stop reaching into other components' registers directly and route those > operations through the component that owns them. > > Move the timestamp/coherency helpers into panthor_gpu, add a doorbell > helper, and update call sites accordingly. This keeps register knowledge > local to each block and avoids spreading cross-component register > accesses across the driver. > > This is a preparatory cleanup for using per-component iomem bases. > > Signed-off-by: Karunika Choo With the spelling of timestamp fixed in the function names, Reviewed-by: Liviu Dudau Best regards, Liviu > --- > drivers/gpu/drm/panthor/panthor_device.c | 27 ---------------- > drivers/gpu/drm/panthor/panthor_drv.c | 7 ++--- > drivers/gpu/drm/panthor/panthor_fw.c | 15 ++++++--- > drivers/gpu/drm/panthor/panthor_fw.h | 1 + > drivers/gpu/drm/panthor/panthor_gpu.c | 40 ++++++++++++++++++++++++ > drivers/gpu/drm/panthor/panthor_gpu.h | 6 ++++ > drivers/gpu/drm/panthor/panthor_pwr.c | 2 +- > drivers/gpu/drm/panthor/panthor_sched.c | 2 +- > 8 files changed, 62 insertions(+), 38 deletions(-) > > diff --git a/drivers/gpu/drm/panthor/panthor_device.c b/drivers/gpu/drm/panthor/panthor_device.c > index f876b13492ae..bd417d6ae8c0 100644 > --- a/drivers/gpu/drm/panthor/panthor_device.c > +++ b/drivers/gpu/drm/panthor/panthor_device.c > @@ -22,38 +22,11 @@ > #include "panthor_fw_regs.h" > #include "panthor_gem.h" > #include "panthor_gpu.h" > -#include "panthor_gpu_regs.h" > #include "panthor_hw.h" > #include "panthor_mmu.h" > #include "panthor_pwr.h" > #include "panthor_sched.h" > > -static int panthor_gpu_coherency_init(struct panthor_device *ptdev) > -{ > - BUILD_BUG_ON(GPU_COHERENCY_NONE != DRM_PANTHOR_GPU_COHERENCY_NONE); > - BUILD_BUG_ON(GPU_COHERENCY_ACE_LITE != DRM_PANTHOR_GPU_COHERENCY_ACE_LITE); > - BUILD_BUG_ON(GPU_COHERENCY_ACE != DRM_PANTHOR_GPU_COHERENCY_ACE); > - > - /* Start with no coherency, and update it if the device is flagged coherent. */ > - ptdev->gpu_info.selected_coherency = GPU_COHERENCY_NONE; > - ptdev->coherent = device_get_dma_attr(ptdev->base.dev) == DEV_DMA_COHERENT; > - > - if (!ptdev->coherent) > - return 0; > - > - /* 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) & > - GPU_COHERENCY_PROT_BIT(ACE_LITE))) { > - ptdev->gpu_info.selected_coherency = GPU_COHERENCY_ACE_LITE; > - return 0; > - } > - > - drm_err(&ptdev->base, "Coherency not supported by the device"); > - return -ENOTSUPP; > -} > - > static int panthor_clk_init(struct panthor_device *ptdev) > { > ptdev->clks.core = devm_clk_get(ptdev->base.dev, NULL); > diff --git a/drivers/gpu/drm/panthor/panthor_drv.c b/drivers/gpu/drm/panthor/panthor_drv.c > index e63210b01e6e..8cd39e6c3f5c 100644 > --- a/drivers/gpu/drm/panthor/panthor_drv.c > +++ b/drivers/gpu/drm/panthor/panthor_drv.c > @@ -34,7 +34,6 @@ > #include "panthor_fw.h" > #include "panthor_gem.h" > #include "panthor_gpu.h" > -#include "panthor_gpu_regs.h" > #include "panthor_heap.h" > #include "panthor_mmu.h" > #include "panthor_sched.h" > @@ -839,7 +838,7 @@ static int panthor_query_timestamp_info(struct panthor_device *ptdev, > } > > if (flags & DRM_PANTHOR_TIMESTAMP_GPU_OFFSET) > - arg->timestamp_offset = gpu_read64(ptdev->iomem, GPU_TIMESTAMP_OFFSET); > + arg->timestamp_offset = panthor_gpu_get_timestap_offset(ptdev); > else > arg->timestamp_offset = 0; > > @@ -854,7 +853,7 @@ static int panthor_query_timestamp_info(struct panthor_device *ptdev, > query_start_time = 0; > > if (flags & DRM_PANTHOR_TIMESTAMP_GPU) > - arg->current_timestamp = gpu_read64_counter(ptdev->iomem, GPU_TIMESTAMP); > + arg->current_timestamp = panthor_gpu_get_timestap(ptdev); > else > arg->current_timestamp = 0; > > @@ -870,7 +869,7 @@ static int panthor_query_timestamp_info(struct panthor_device *ptdev, > } > > if (flags & DRM_PANTHOR_TIMESTAMP_GPU_CYCLE_COUNT) > - arg->cycle_count = gpu_read64_counter(ptdev->iomem, GPU_CYCLE_COUNT); > + arg->cycle_count = panthor_gpu_get_cycle_count(ptdev); > else > arg->cycle_count = 0; > > diff --git a/drivers/gpu/drm/panthor/panthor_fw.c b/drivers/gpu/drm/panthor/panthor_fw.c > index 4704275b9c8f..1c13a4884201 100644 > --- a/drivers/gpu/drm/panthor/panthor_fw.c > +++ b/drivers/gpu/drm/panthor/panthor_fw.c > @@ -1054,7 +1054,7 @@ static void panthor_fw_init_global_iface(struct panthor_device *ptdev) > GLB_CFG_POWEROFF_TIMER | > GLB_CFG_PROGRESS_TIMER); > > - gpu_write(ptdev->iomem, CSF_DOORBELL(CSF_GLB_DOORBELL_ID), 1); > + panthor_fw_ring_doorbell(ptdev, CSF_GLB_DOORBELL_ID); > > /* Kick the watchdog. */ > mod_delayed_work(ptdev->reset.wq, &ptdev->fw->watchdog.ping_work, > @@ -1156,7 +1156,7 @@ static void panthor_fw_halt_mcu(struct panthor_device *ptdev) > else > panthor_fw_update_reqs(glb_iface, req, GLB_HALT, GLB_HALT); > > - gpu_write(ptdev->iomem, CSF_DOORBELL(CSF_GLB_DOORBELL_ID), 1); > + panthor_fw_ring_doorbell(ptdev, CSF_GLB_DOORBELL_ID); > } > > static bool panthor_fw_wait_mcu_halted(struct panthor_device *ptdev) > @@ -1400,6 +1400,11 @@ int panthor_fw_csg_wait_acks(struct panthor_device *ptdev, u32 csg_slot, > return ret; > } > > +void panthor_fw_ring_doorbell(struct panthor_device *ptdev, u32 doorbell_id) > +{ > + gpu_write(ptdev->iomem, CSF_DOORBELL(doorbell_id), 1); > +} > + > /** > * panthor_fw_ring_csg_doorbells() - Ring command stream group doorbells. > * @ptdev: Device. > @@ -1413,8 +1418,8 @@ void panthor_fw_ring_csg_doorbells(struct panthor_device *ptdev, u32 csg_mask) > { > struct panthor_fw_global_iface *glb_iface = panthor_fw_get_glb_iface(ptdev); > > - panthor_fw_toggle_reqs(glb_iface, doorbell_req, doorbell_ack, csg_mask); > - gpu_write(ptdev->iomem, CSF_DOORBELL(CSF_GLB_DOORBELL_ID), 1); > + panthor_fw_toggle_reqs(glb_iface, doorbell_req, doorbell_ack, csg_mask);\ > + panthor_fw_ring_doorbell(ptdev, CSF_GLB_DOORBELL_ID); > } > > static void panthor_fw_ping_work(struct work_struct *work) > @@ -1429,7 +1434,7 @@ static void panthor_fw_ping_work(struct work_struct *work) > return; > > panthor_fw_toggle_reqs(glb_iface, req, ack, GLB_PING); > - gpu_write(ptdev->iomem, CSF_DOORBELL(CSF_GLB_DOORBELL_ID), 1); > + panthor_fw_ring_doorbell(ptdev, CSF_GLB_DOORBELL_ID); > > ret = panthor_fw_glb_wait_acks(ptdev, GLB_PING, &acked, 100); > if (ret) { > diff --git a/drivers/gpu/drm/panthor/panthor_fw.h b/drivers/gpu/drm/panthor/panthor_fw.h > index fbdc21469ba3..a99a9b6f4825 100644 > --- a/drivers/gpu/drm/panthor/panthor_fw.h > +++ b/drivers/gpu/drm/panthor/panthor_fw.h > @@ -500,6 +500,7 @@ int panthor_fw_csg_wait_acks(struct panthor_device *ptdev, u32 csg_id, u32 req_m > int panthor_fw_glb_wait_acks(struct panthor_device *ptdev, u32 req_mask, u32 *acked, > u32 timeout_ms); > > +void panthor_fw_ring_doorbell(struct panthor_device *ptdev, u32 doorbell_id); > void panthor_fw_ring_csg_doorbells(struct panthor_device *ptdev, u32 csg_slot); > > struct panthor_kernel_bo * > diff --git a/drivers/gpu/drm/panthor/panthor_gpu.c b/drivers/gpu/drm/panthor/panthor_gpu.c > index fecc30747acf..ef0aca2b7532 100644 > --- a/drivers/gpu/drm/panthor/panthor_gpu.c > +++ b/drivers/gpu/drm/panthor/panthor_gpu.c > @@ -427,3 +427,43 @@ void panthor_gpu_resume(struct panthor_device *ptdev) > panthor_hw_l2_power_on(ptdev); > } > > +u64 panthor_gpu_get_timestap(struct panthor_device *ptdev) > +{ > + return gpu_read64_counter(ptdev->iomem, GPU_TIMESTAMP); > +} > + > +u64 panthor_gpu_get_timestap_offset(struct panthor_device *ptdev) > +{ > + return gpu_read64(ptdev->iomem, GPU_TIMESTAMP_OFFSET); > +} > + > +u64 panthor_gpu_get_cycle_count(struct panthor_device *ptdev) > +{ > + return gpu_read64_counter(ptdev->iomem, GPU_CYCLE_COUNT); > +} > + > +int panthor_gpu_coherency_init(struct panthor_device *ptdev) > +{ > + BUILD_BUG_ON(GPU_COHERENCY_NONE != DRM_PANTHOR_GPU_COHERENCY_NONE); > + BUILD_BUG_ON(GPU_COHERENCY_ACE_LITE != DRM_PANTHOR_GPU_COHERENCY_ACE_LITE); > + BUILD_BUG_ON(GPU_COHERENCY_ACE != DRM_PANTHOR_GPU_COHERENCY_ACE); > + > + /* Start with no coherency, and update it if the device is flagged coherent. */ > + ptdev->gpu_info.selected_coherency = GPU_COHERENCY_NONE; > + ptdev->coherent = device_get_dma_attr(ptdev->base.dev) == DEV_DMA_COHERENT; > + > + if (!ptdev->coherent) > + return 0; > + > + /* 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) & > + GPU_COHERENCY_PROT_BIT(ACE_LITE))) { > + ptdev->gpu_info.selected_coherency = GPU_COHERENCY_ACE_LITE; > + return 0; > + } > + > + drm_err(&ptdev->base, "Coherency not supported by the device"); > + return -ENOTSUPP; > +} > diff --git a/drivers/gpu/drm/panthor/panthor_gpu.h b/drivers/gpu/drm/panthor/panthor_gpu.h > index 12c263a39928..c22378e5ce48 100644 > --- a/drivers/gpu/drm/panthor/panthor_gpu.h > +++ b/drivers/gpu/drm/panthor/panthor_gpu.h > @@ -54,4 +54,10 @@ int panthor_gpu_soft_reset(struct panthor_device *ptdev); > void panthor_gpu_power_changed_off(struct panthor_device *ptdev); > int panthor_gpu_power_changed_on(struct panthor_device *ptdev); > > +u64 panthor_gpu_get_timestap(struct panthor_device *ptdev); > +u64 panthor_gpu_get_timestap_offset(struct panthor_device *ptdev); > +u64 panthor_gpu_get_cycle_count(struct panthor_device *ptdev); > + > +int panthor_gpu_coherency_init(struct panthor_device *ptdev); > + > #endif > diff --git a/drivers/gpu/drm/panthor/panthor_pwr.c b/drivers/gpu/drm/panthor/panthor_pwr.c > index 306592ff2227..aafb0c5c7d23 100644 > --- a/drivers/gpu/drm/panthor/panthor_pwr.c > +++ b/drivers/gpu/drm/panthor/panthor_pwr.c > @@ -199,7 +199,7 @@ static int panthor_pwr_domain_wait_transition(struct panthor_device *ptdev, u32 > > static void panthor_pwr_debug_info_show(struct panthor_device *ptdev) > { > - drm_info(&ptdev->base, "GPU_FEATURES: 0x%016llx", gpu_read64(ptdev->iomem, GPU_FEATURES)); > + drm_info(&ptdev->base, "GPU_FEATURES: 0x%016llx", ptdev->gpu_info.gpu_features); > drm_info(&ptdev->base, "PWR_STATUS: 0x%016llx", gpu_read64(ptdev->iomem, PWR_STATUS)); > drm_info(&ptdev->base, "L2_PRESENT: 0x%016llx", gpu_read64(ptdev->iomem, PWR_L2_PRESENT)); > drm_info(&ptdev->base, "L2_PWRTRANS: 0x%016llx", gpu_read64(ptdev->iomem, PWR_L2_PWRTRANS)); > diff --git a/drivers/gpu/drm/panthor/panthor_sched.c b/drivers/gpu/drm/panthor/panthor_sched.c > index 70b8a22b3ed7..60e7b4e20a13 100644 > --- a/drivers/gpu/drm/panthor/panthor_sched.c > +++ b/drivers/gpu/drm/panthor/panthor_sched.c > @@ -3373,7 +3373,7 @@ queue_run_job(struct drm_sched_job *sched_job) > if (resume_tick) > sched_resume_tick(ptdev); > > - gpu_write(ptdev->iomem, CSF_DOORBELL(queue->doorbell_id), 1); > + panthor_fw_ring_doorbell(ptdev, queue->doorbell_id); > if (!sched->pm.has_ref && > !(group->blocked_queues & BIT(job->queue_idx))) { > pm_runtime_get(ptdev->base.dev); > -- > 2.43.0 > -- ==================== | I would like to | | fix the world, | | but they're not | | giving me the | \ source code! / --------------- ¯\_(ツ)_/¯