On 19/12/2024 17:05, Karunika Choo wrote:
> This patch adds parsing of GPU register fields on initialization instead of
> parsing the fields each time it is needed.
Why? ;)
The commit message should ideally explain the reason behind a change
rather than the change itself (that should ideally be obvious from the
patch). (See below for more).
Also from a reviewer's perspective it's hard to review patches which
both move code between files and change it. So splitting into an initial
patch which just moves code into the new panthor_props.c and a follow up
patch would make the review easier.
> Signed-off-by: Karunika Choo <karunika.choo@xxxxxxx>
> ---
> drivers/gpu/drm/panthor/Makefile | 1 +
> drivers/gpu/drm/panthor/panthor_device.c | 1 +
> drivers/gpu/drm/panthor/panthor_device.h | 4 +
> drivers/gpu/drm/panthor/panthor_fw.c | 5 +-
> drivers/gpu/drm/panthor/panthor_gpu.c | 105 ++--------------
> drivers/gpu/drm/panthor/panthor_heap.c | 6 +-
> drivers/gpu/drm/panthor/panthor_mmu.c | 21 +---
> drivers/gpu/drm/panthor/panthor_props.c | 151 +++++++++++++++++++++++
> drivers/gpu/drm/panthor/panthor_props.h | 70 +++++++++++
> drivers/gpu/drm/panthor/panthor_regs.h | 5 +
> drivers/gpu/drm/panthor/panthor_sched.c | 6 +-
> 11 files changed, 252 insertions(+), 123 deletions(-)
> create mode 100644 drivers/gpu/drm/panthor/panthor_props.c
> create mode 100644 drivers/gpu/drm/panthor/panthor_props.h
>
[...]
> diff --git a/drivers/gpu/drm/panthor/panthor_props.c b/drivers/gpu/drm/panthor/panthor_props.c
> new file mode 100644
> index 000000000000..0a379feaf12d
> --- /dev/null
> +++ b/drivers/gpu/drm/panthor/panthor_props.c
> @@ -0,0 +1,151 @@
> +// SPDX-License-Identifier: GPL-2.0 or MIT
> +/* Copyright 2024 ARM Limited. All rights reserved. */
> +
> +#include <drm/drm_managed.h>
> +
> +#include "panthor_device.h"
> +#include "panthor_props.h"
> +#include "panthor_regs.h"
> +
> +static void panthor_props_arch_10_8_init_info(struct panthor_device *ptdev)
> +{
> + unsigned int i;
> +
> + ptdev->gpu_info.csf_id = gpu_read(ptdev, GPU_CSF_ID);
> + ptdev->gpu_info.gpu_rev = gpu_read(ptdev, GPU_REVID);
> + ptdev->gpu_info.core_features = gpu_read(ptdev, GPU_CORE_FEATURES);
> + ptdev->gpu_info.l2_features = gpu_read(ptdev, GPU_L2_FEATURES);
> + ptdev->gpu_info.tiler_features = gpu_read(ptdev, GPU_TILER_FEATURES);
> + ptdev->gpu_info.mem_features = gpu_read(ptdev, GPU_MEM_FEATURES);
> + ptdev->gpu_info.mmu_features = gpu_read(ptdev, GPU_MMU_FEATURES);
> + ptdev->gpu_info.thread_features = gpu_read(ptdev, GPU_THREAD_FEATURES);
> + ptdev->gpu_info.max_threads = gpu_read(ptdev, GPU_THREAD_MAX_THREADS);
> + ptdev->gpu_info.thread_max_workgroup_size = gpu_read(ptdev, GPU_THREAD_MAX_WORKGROUP_SIZE);
> + ptdev->gpu_info.thread_max_barrier_size = gpu_read(ptdev, GPU_THREAD_MAX_BARRIER_SIZE);
> + ptdev->gpu_info.coherency_features = gpu_read(ptdev, GPU_COHERENCY_FEATURES);
> + for (i = 0; i < 4; i++)
> + ptdev->gpu_info.texture_features[i] = gpu_read(ptdev, GPU_TEXTURE_FEATURES(i));
> +}
> +
> +static void panthor_props_arch_10_8_parse_props(struct panthor_device *ptdev)
> +{
> + struct panthor_props *props = ptdev->props;
> + struct drm_panthor_gpu_info *info = &ptdev->gpu_info;
> +
> + props->shader_core_count = hweight64(info->shader_present);
> + props->mmu_va_bits = GPU_MMU_FEATURES_VA_BITS(info->mmu_features);
> + props->mmu_pa_bits = GPU_MMU_FEATURES_PA_BITS(info->mmu_features);
> + props->mmu_as_count = hweight32(info->as_present);
> + props->l2_line_size = GPU_L2_FEATURES_LINE_SIZE(info->l2_features);
