[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <35034ce0-46de-4417-9bd6-6ea90c5e9095@arm.com>
Date: Fri, 29 Aug 2025 14:41:02 +0100
From: Steven Price <steven.price@....com>
To: Chia-I Wu <olvaffe@...il.com>,
Boris Brezillon <boris.brezillon@...labora.com>,
Liviu Dudau <liviu.dudau@....com>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>,
David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, dri-devel@...ts.freedesktop.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] drm/panthor: add asn-hash support
On 28/08/2025 21:18, Chia-I Wu wrote:
> Parse asn-hash and enable custom ASN hash when the property exists.
> This is required on some socs such as mt8196.
>
> Signed-off-by: Chia-I Wu <olvaffe@...il.com>
This mostly looks fine, although there is a question of naming. This was
renamed in a later version of the architecture to be L2C_SLICE_HASH
(rather than ASN_HASH).
I'm honestly not sure whether to stick with asn-hash (as it's out in the
wild already) or try to align with the newer spec and whether that will
create or avoid confusion!
> ---
> drivers/gpu/drm/panthor/panthor_device.c | 28 ++++++++++++++++++++++++
> drivers/gpu/drm/panthor/panthor_device.h | 6 +++++
> drivers/gpu/drm/panthor/panthor_gpu.c | 17 ++++++++++++++
> drivers/gpu/drm/panthor/panthor_regs.h | 4 ++++
> 4 files changed, 55 insertions(+)
>
> diff --git a/drivers/gpu/drm/panthor/panthor_device.c b/drivers/gpu/drm/panthor/panthor_device.c
> index 81df49880bd87..19423c495d8d7 100644
> --- a/drivers/gpu/drm/panthor/panthor_device.c
> +++ b/drivers/gpu/drm/panthor/panthor_device.c
> @@ -41,6 +41,30 @@ static int panthor_gpu_coherency_init(struct panthor_device *ptdev)
> return -ENOTSUPP;
> }
>
> +static int panthor_gpu_asn_hash_init(struct panthor_device *ptdev)
> +{
> + int ret;
> +
> + ret = of_property_read_u32_array(ptdev->base.dev->of_node, "asn-hash",
> + ptdev->asn_hash,
> + ARRAY_SIZE(ptdev->asn_hash));
> + if (ret) {
> + if (ret == -EINVAL)
> + ret = 0;
> + return ret;
> + }
NIT: I think this would be neater written as:
if (ret == -EINVAL)
return 0;
else if (ret)
return ret
Thanks,
Steve
> +
> + if (GPU_ARCH_MAJOR(ptdev->gpu_info.gpu_id) < 11) {
> + drm_err(&ptdev->base,
> + "Custom ASN hash not supported by the device");
> + return -EOPNOTSUPP;
> + }
> +
> + ptdev->has_asn_hash = true;
> +
> + return 0;
> +}
> +
> static int panthor_clk_init(struct panthor_device *ptdev)
> {
> ptdev->clks.core = devm_clk_get(ptdev->base.dev, NULL);
> @@ -257,6 +281,10 @@ int panthor_device_init(struct panthor_device *ptdev)
> if (ret)
> goto err_unplug_gpu;
>
> + ret = panthor_gpu_asn_hash_init(ptdev);
> + if (ret)
> + goto err_unplug_gpu;
> +
> ret = panthor_mmu_init(ptdev);
> if (ret)
> goto err_unplug_gpu;
> diff --git a/drivers/gpu/drm/panthor/panthor_device.h b/drivers/gpu/drm/panthor/panthor_device.h
> index 4fc7cf2aeed57..6f8e2b3b037e5 100644
> --- a/drivers/gpu/drm/panthor/panthor_device.h
> +++ b/drivers/gpu/drm/panthor/panthor_device.h
> @@ -114,6 +114,12 @@ struct panthor_device {
> /** @coherent: True if the CPU/GPU are memory coherent. */
> bool coherent;
>
> + /** @has_asn_hash: True if custom ASN hash is enabled. */
> + bool has_asn_hash;
> +
> + /** @asn_hash: ASN_HASH values for custom ASN hash */
> + u32 asn_hash[3];
> +
> /** @gpu_info: GPU information. */
> struct drm_panthor_gpu_info gpu_info;
>
> diff --git a/drivers/gpu/drm/panthor/panthor_gpu.c b/drivers/gpu/drm/panthor/panthor_gpu.c
> index db69449a5be09..f9222b67f314d 100644
> --- a/drivers/gpu/drm/panthor/panthor_gpu.c
> +++ b/drivers/gpu/drm/panthor/panthor_gpu.c
> @@ -52,6 +52,22 @@ static void panthor_gpu_coherency_set(struct panthor_device *ptdev)
> ptdev->coherent ? GPU_COHERENCY_PROT_BIT(ACE_LITE) : GPU_COHERENCY_NONE);
> }
>
> +static void panthor_gpu_asn_hash_set(struct panthor_device *ptdev)
> +{
> + u32 l2_config;
> + u32 i;
> +
> + if (!ptdev->has_asn_hash)
> + return;
> +
> + for (i = 0; i < ARRAY_SIZE(ptdev->asn_hash); i++)
> + gpu_write(ptdev, ASN_HASH(i), ptdev->asn_hash[i]);
> +
> + l2_config = gpu_read(ptdev, L2_CONFIG);
> + l2_config |= L2_CONFIG_ASN_HASH_ENABLE;
> + gpu_write(ptdev, L2_CONFIG, l2_config);
> +}
> +
> static void panthor_gpu_irq_handler(struct panthor_device *ptdev, u32 status)
> {
> gpu_write(ptdev, GPU_INT_CLEAR, status);
> @@ -243,6 +259,7 @@ int panthor_gpu_l2_power_on(struct panthor_device *ptdev)
>
> /* Set the desired coherency mode before the power up of L2 */
> panthor_gpu_coherency_set(ptdev);
> + panthor_gpu_asn_hash_set(ptdev);
>
> return panthor_gpu_power_on(ptdev, L2, 1, 20000);
> }
> diff --git a/drivers/gpu/drm/panthor/panthor_regs.h b/drivers/gpu/drm/panthor/panthor_regs.h
> index 8bee76d01bf83..c9f795624e79b 100644
> --- a/drivers/gpu/drm/panthor/panthor_regs.h
> +++ b/drivers/gpu/drm/panthor/panthor_regs.h
> @@ -64,6 +64,8 @@
>
> #define GPU_FAULT_STATUS 0x3C
> #define GPU_FAULT_ADDR 0x40
> +#define L2_CONFIG 0x48
> +#define L2_CONFIG_ASN_HASH_ENABLE BIT(24)
>
> #define GPU_PWR_KEY 0x50
> #define GPU_PWR_KEY_UNLOCK 0x2968A819
> @@ -110,6 +112,8 @@
>
> #define GPU_REVID 0x280
>
> +#define ASN_HASH(n) (0x2C0 + ((n) * 4))
> +
> #define GPU_COHERENCY_FEATURES 0x300
> #define GPU_COHERENCY_PROT_BIT(name) BIT(GPU_COHERENCY_ ## name)
>
Powered by blists - more mailing lists