From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from bali.collaboradmins.com (bali.collaboradmins.com [148.251.105.195]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id A10E63A16A8 for ; Thu, 30 Apr 2026 10:45:22 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=148.251.105.195 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1777545924; cv=none; b=MdjLeVRk1UcLdEnqTrWLOv3EwN4MXOVu12IRFRbpefgV/+yC3gwGE2RuuhrRoq39veWSpD86hZv6zqXsD34icKQzuBLrjJwydy3FcsMMctfa5eCeaDJ9geFYQZyolHQPv8CPp4mDMEUw1fieMUCJOAK23IdIugY+ctQ82IiPpp8= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1777545924; c=relaxed/simple; bh=SpLQSn+5rUGX26OZoolQ8wREsGXnphb9uQVa6v9dEaI=; h=Date:From:To:Cc:Subject:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=L/zjcrTCKvigxGvY6eelQwFVf6JOtxABghcGK8TTcVLB28xVNE36feSLHS7fTlz8VIIMnw1VlxaDWDwxh51K+2FMDOoaNG/Ry8SCuyznomXSQo11TMjQGglsBFsjCv/ws6zJvnTLYSgyM6gmIuIrW3ciwQQFh62JWO1GQkBP8No= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=collabora.com; spf=pass smtp.mailfrom=collabora.com; dkim=pass (2048-bit key) header.d=collabora.com header.i=@collabora.com header.b=eZ6HpAfi; arc=none smtp.client-ip=148.251.105.195 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=collabora.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=collabora.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=collabora.com header.i=@collabora.com header.b="eZ6HpAfi" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com; s=mail; t=1777545920; bh=SpLQSn+5rUGX26OZoolQ8wREsGXnphb9uQVa6v9dEaI=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=eZ6HpAfiKRo3+FoVxzOWhO23AIGDrltaVRPNPK2tborh7O94c/3LsoGSrogdzZ5+F DQQ49wOyp6Be9GdsMfhaHXFrITv58AZUGNqX2N83Pt829IwNESmrcfOyrU40n70Ni5 Th81kfHMUp1KZObF9plrNq0mwSsMfdKDVJArux7yI7QQYAbK6uwaFEcamAtrpSxrc1 1PIydN/NsedFAEgWILs8hFl+AQhQsPCbhl8Kc1KKNUsjDEtjLRBNbfER6mxlA2na3/ RG3ABCzf4uAxEeUb0cyVPEtSMoJ9P0c2A5FNcACYYJ4wDb+nA3XzyFWWWg3WZRujJF C3n4HWSRAh8MA== Received: from fedora (unknown [100.64.0.11]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (prime256v1) server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) (Authenticated sender: bbrezillon) by bali.collaboradmins.com (Postfix) with ESMTPSA id 6E1D517E0EBB; Thu, 30 Apr 2026 12:45:20 +0200 (CEST) Date: Thu, 30 Apr 2026 12:45:16 +0200 From: Boris Brezillon To: Karunika Choo Cc: dri-devel@lists.freedesktop.org, nd@arm.com, Steven Price , Liviu Dudau , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , David Airlie , Simona Vetter , linux-kernel@vger.kernel.org Subject: Re: [PATCH v3 3/8] drm/panthor: Replace cross-component register accesses with helpers Message-ID: <20260430124516.226aa4dc@fedora> In-Reply-To: <20260427155934.416502-4-karunika.choo@arm.com> References: <20260427155934.416502-1-karunika.choo@arm.com> <20260427155934.416502-4-karunika.choo@arm.com> Organization: Collabora X-Mailer: Claws Mail 4.4.0 (GTK 3.24.52; x86_64-redhat-linux-gnu) 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=US-ASCII Content-Transfer-Encoding: 7bit On Mon, 27 Apr 2026 16:59:29 +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. > > v3: > - Pick up Ack from Boris and R-bs from Liviu and Steve > v2: > - Fix incorrect spelling of timestamp helpers > - Fix unintended trailing backslash > > Reviewed-by: Steven Price > Reviewed-by: Liviu Dudau > Acked-by: Boris Brezillon > Signed-off-by: Karunika Choo Tested-by: Boris Brezillon > --- > drivers/gpu/drm/panthor/panthor_device.c | 27 ---------------- > drivers/gpu/drm/panthor/panthor_drv.c | 7 ++--- > drivers/gpu/drm/panthor/panthor_fw.c | 13 +++++--- > 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, 61 insertions(+), 37 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..66996c9147c2 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_timestamp_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_timestamp(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..4a0c23e987b6 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. > @@ -1414,7 +1419,7 @@ 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_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..747ac332be64 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_timestamp(struct panthor_device *ptdev) > +{ > + return gpu_read64_counter(ptdev->iomem, GPU_TIMESTAMP); > +} > + > +u64 panthor_gpu_get_timestamp_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..f615feb05609 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_timestamp(struct panthor_device *ptdev); > +u64 panthor_gpu_get_timestamp_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 530e1f0254b1..5b34032deff8 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);