drm/panthor: add custom ASN hash support
Subsystems (5)
cover drm/panthor: add custom ASN hash support
Some socs such as mt8196 require custom ASN hash.
Chia-I Wu (2):
dt-bindings: gpu: mali-valhall-csf: add asn-hash drm/panthor: add asn-hash support
.../bindings/gpu/arm,mali-valhall-csf.yaml | 8 ++++++ 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 +++ 5 files changed, 63 insertions(+)
Signature · 1 line
2.51.0.318.gd7df087d1a-goog
PATCH 1/2 dt-bindings: gpu: mali-valhall-csf: add asn-hash
The values are written to ASN_HASH[0..2] registers. The property is called "l2-hash-values" in the downstream driver.
--- .../devicetree/bindings/gpu/arm,mali-valhall-csf.yaml | 8 ++++++++ 1 file changed, 8 insertions(+)
Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml +8 -0
@@ -85,6 +85,14 @@ properties: dma-coherent: true + asn-hash: + $ref: /schemas/types.yaml#/definitions/uint32-array + description: + The values are written to ASN_HASH[0..2] registers. They affect how + physical addresses are mapped to L2 cache slices. + minItems: 3 + maxItems: 3 + required: - compatible - reg
Signature · 1 line
2.51.0.318.gd7df087d1a-goog
Boris Brezillon
maintainerRe: [PATCH 1/2] dt-bindings: gpu: mali-valhall-csf: add asn-hash
Quoted 13 lines · On Thu, 28 Aug 2025 13:18:05 -0700 Chia-I Wu <[email protected]> wrote:
The values are written to ASN_HASH[0..2] registers. The property is called "l2-hash-values" in the downstream driver.
Signed-off-by Chia-I Wu <[email protected]>--- .../devicetree/bindings/gpu/arm,mali-valhall-csf.yaml | 8 ++++++++ 1 file changed, 8 insertions(+)Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml +0
-0
Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml properties: · line 92
Quoted 8 lines
Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml +5
-0@@ -85,6 +85,14 @@ properties: dma-coherent: true + asn-hash: + $ref: /schemas/types.yaml#/definitions/uint32-array + description: + The values are written to ASN_HASH[0..2] registers. They affect how + physical addresses are mapped to L2 cache slices.
If this is per-SoC integration details, I would hide that behind the compatible string and have some panthor_soc_data attached to the of_device_id entries.
Quoted 6 lines
Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml +3
-0+ minItems: 3 + maxItems: 3 + required: - compatible - reg
Re: [PATCH 1/2] dt-bindings: gpu: mali-valhall-csf: add asn-hash
In reply to Chia-I WuQuoted 2 lines · Chia-I Wu
The values are written to ASN_HASH[0..2] registers. The property is called "l2-hash-values" in the downstream driver.
We don't add properties based on downstream drivers. Please provide some sort of rationale, including explanation why this is not SoC/compatible specific.
Quoted 11 lines
Signed-off-by Chia-I Wu <[email protected]>--- .../devicetree/bindings/gpu/arm,mali-valhall-csf.yaml | 8 ++++++++ 1 file changed, 8 insertions(+)Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml +0
-0
Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml properties: · line 88
Quoted 4 lines
Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml +1
-0@@ -85,6 +85,14 @@ properties: dma-coherent: true + asn-hash:
Missing vendor prefix or please explain which common schema has it already defined.
Best regards,
Krzysztof
PATCH 2/2 drm/panthor: add asn-hash support
In reply to Chia-I WuParse asn-hash and enable custom ASN hash when the property exists. This is required on some socs such as mt8196.
--- 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(+)
drivers/gpu/drm/panthor/panthor_device.c +28 -0
@@ -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; + } + +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;
drivers/gpu/drm/panthor/panthor_device.h +6 -0
@@ -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;
drivers/gpu/drm/panthor/panthor_gpu.c +17 -0
@@ -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 ); }
drivers/gpu/drm/panthor/panthor_regs.h +4 -0
@@ -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)
Signature · 1 line
2.51.0.318.gd7df087d1a-goog
Steven Price
maintainerRe: [PATCH 2/2] drm/panthor: add asn-hash support
Quoted 4 lines · Chia-I Wu
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 <[email protected]>
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!
Quoted 17 lines
--- 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(+)drivers/gpu/drm/panthor/panthor_device.c +2
-0@@ -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) +{
drivers/gpu/drm/panthor/panthor_device.c static int panthor_gpu_coherency_init(struct panthor_device *ptdev) · line 55
Quoted 10 lines
drivers/gpu/drm/panthor/panthor_device.c +10
-0+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
Quoted 100 lines
drivers/gpu/drm/panthor/panthor_device.c +16
-0+ +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;drivers/gpu/drm/panthor/panthor_device.h +6
-0@@ -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;drivers/gpu/drm/panthor/panthor_gpu.c +17
-0@@ -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 ); }drivers/gpu/drm/panthor/panthor_regs.h +4
-0@@ -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)
Steven Price
maintainerQuoted 1 line · Chia-I Wu
Some socs such as mt8196 require custom ASN hash.
I don't know the full details of this, but I'm puzzled by the "require" here.
AIUI the "custom ASN hash support" (or L2C_SLICE_HASH as it was renamed) affects the efficiency of the L2 caches in the GPU. It basically determines how addresses are striped over the individual caches.
So (unless there is a specific errata) not setting a custom hash will work just fine, but the performance might be slightly reduced.
kbase provides both a DT option and a module parameter which can be used to override the defaults (although the parameter descriptions say "for testing" which I think is somewhat telling).
How we should describe this somewhat depends on whether this is a specific workaround for a SoC - in which case Boris's suggestion of using a different compatible string and panthor_soc_data seems like a good choice. Or if this is exposed as a general "tuning" parameter, in which case this might be appropriate.
I believe the tuning is related to more than just a SoC (the external memory system has an impact). So I guess a DT level knob makes most sense here.
Steve
Quoted 11 lines
Chia-I Wu (2):
dt-bindings: gpu: mali-valhall-csf: add asn-hash drm/panthor: add asn-hash support.../bindings/gpu/arm,mali-valhall-csf.yaml | 8 ++++++ 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 +++ 5 files changed, 63 insertions(+)
Quoted 7 lines · Steven Price
Quoted 1 line · Chia-I Wu
Some socs such as mt8196 require custom ASN hash.
I don't know the full details of this, but I'm puzzled by the "require" here.
Quoted 6 lines
AIUI the "custom ASN hash support" (or
L2C_SLICE_HASHas it was renamed) affects the efficiency of the L2 caches in the GPU. It basically determines how addresses are striped over the individual caches.So (unless there is a specific errata) not setting a custom hash will work just fine, but the performance might be slightly reduced.
All memory access results in faults unless custom ASN hash is enabled on mt8196.
It sounds like this is a soc-specific quirk. I will check with the vendor, and likely turn this into a quirk.
Quoted 29 lines
kbase provides both a DT option and a module parameter which can be used to override the defaults (although the parameter descriptions say "for testing" which I think is somewhat telling).
How we should describe this somewhat depends on whether this is a specific workaround for a SoC - in which case Boris's suggestion of using a different compatible string and
panthor_soc_dataseems like a good choice. Or if this is exposed as a general "tuning" parameter, in which case this might be appropriate.I believe the tuning is related to more than just a SoC (the external memory system has an impact). So I guess a DT level knob makes most sense here.
Steve
Quoted 11 lines
Chia-I Wu (2):
dt-bindings: gpu: mali-valhall-csf: add asn-hash drm/panthor: add asn-hash support.../bindings/gpu/arm,mali-valhall-csf.yaml | 8 ++++++ 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 +++ 5 files changed, 63 insertions(+)