From: Alyssa Rosenzweig alyssa.rosenzweig@collabora.com
This patch series adds preliminary support for Mali "Valhall" GPUs into the Panfrost kernel driver. The series has been tested on the Mali-G57 on a MediaTek MT8192 system. However, that system requires additional MediaTek-specific patches [1] as well as core mainlining for MediaTek. I'll post the MT8192-specific Panfrost patches soon; they depend on this core series.
On the userspace side, pre-CSF Valhall (what is supported here) uses an identical UABI as Bifrost. Mesa support for Valhall is being worked on in parallel [2]. I'm hoping basic support for Valhall will be available in Mesa 22.1.
[1] https://gitlab.freedesktop.org/panfrost/linux/-/tree/mt8192-branch [2] https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/14795
Alyssa Rosenzweig (9): dt-bindings: Add arm,mali-valhall compatible drm/panfrost: Handle HW_ISSUE_TTRX_2968_TTRX_3162 drm/panfrost: Constify argument to has_hw_issue drm/panfrost: Handle HW_ISSUE_TTRX_3076 drm/panfrost: Add HW_ISSUE_TTRX_3485 quirk drm/panfrost: Add "clean only safe" feature bit drm/panfrost: Don't set L2_MMU_CONFIG quirks drm/panfrost: Add Mali-G57 "Natt" support drm/panfrost: Handle arm,mali-valhall compatible
.../bindings/gpu/arm,mali-bifrost.yaml | 1 + drivers/gpu/drm/panfrost/panfrost_device.c | 9 +++++++-- drivers/gpu/drm/panfrost/panfrost_drv.c | 1 + drivers/gpu/drm/panfrost/panfrost_features.h | 13 +++++++++++++ drivers/gpu/drm/panfrost/panfrost_gpu.c | 17 +++++------------ drivers/gpu/drm/panfrost/panfrost_issues.h | 17 ++++++++++++++++- drivers/gpu/drm/panfrost/panfrost_regs.h | 1 + 7 files changed, 44 insertions(+), 15 deletions(-)
From: Alyssa Rosenzweig alyssa.rosenzweig@collabora.com
From the kernel's perspective, pre-CSF Valhall is more or less
compatible with Bifrost, although they differ to userspace. Add a compatible for Valhall to the existing Bifrost bindings documentation.
Signed-off-by: Alyssa Rosenzweig alyssa.rosenzweig@collabora.com Cc: devicetree@vger.kernel.org --- Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml | 1 + 1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml index 63a08f3f321d..48aeabd2ed68 100644 --- a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml +++ b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml @@ -23,6 +23,7 @@ properties: - rockchip,px30-mali - rockchip,rk3568-mali - const: arm,mali-bifrost # Mali Bifrost GPU model/revision is fully discoverable + - const: arm,mali-valhall # Mali Valhall GPU model/revision is fully discoverable
reg: maxItems: 1
On 11/02/2022 20:27, alyssa.rosenzweig@collabora.com wrote:
From: Alyssa Rosenzweig alyssa.rosenzweig@collabora.com
From the kernel's perspective, pre-CSF Valhall is more or less compatible with Bifrost, although they differ to userspace. Add a compatible for Valhall to the existing Bifrost bindings documentation.
Signed-off-by: Alyssa Rosenzweig alyssa.rosenzweig@collabora.com Cc: devicetree@vger.kernel.org
Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml | 1 + 1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml index 63a08f3f321d..48aeabd2ed68 100644 --- a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml +++ b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml @@ -23,6 +23,7 @@ properties: - rockchip,px30-mali - rockchip,rk3568-mali - const: arm,mali-bifrost # Mali Bifrost GPU model/revision is fully discoverable
- const: arm,mali-valhall # Mali Valhall GPU model/revision is fully discoverable
It might be worth spelling out here that this is *pre-CSF* Valhall. I'm pretty sure we're going to need different bindings for CSF GPUs.
Steve
reg: maxItems: 1
diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml index 63a08f3f321d..48aeabd2ed68 100644 --- a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml +++ b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml @@ -23,6 +23,7 @@ properties: - rockchip,px30-mali - rockchip,rk3568-mali - const: arm,mali-bifrost # Mali Bifrost GPU model/revision is fully discoverable
- const: arm,mali-valhall # Mali Valhall GPU model/revision is fully discoverableIt might be worth spelling out here that this is *pre-CSF* Valhall. I'm pretty sure we're going to need different bindings for CSF GPUs.
Yes, agreed, will make a note for v2.
On Mon, 14 Feb 2022 at 16:23, Steven Price steven.price@arm.com wrote:
On 11/02/2022 20:27, alyssa.rosenzweig@collabora.com wrote:
From the kernel's perspective, pre-CSF Valhall is more or less compatible with Bifrost, although they differ to userspace. Add a compatible for Valhall to the existing Bifrost bindings documentation.
diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml index 63a08f3f321d..48aeabd2ed68 100644 --- a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml +++ b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml @@ -23,6 +23,7 @@ properties: - rockchip,px30-mali - rockchip,rk3568-mali - const: arm,mali-bifrost # Mali Bifrost GPU model/revision is fully discoverable
- const: arm,mali-valhall # Mali Valhall GPU model/revision is fully discoverableIt might be worth spelling out here that this is *pre-CSF* Valhall. I'm pretty sure we're going to need different bindings for CSF GPUs.
Good point - maybe either make it arm,mali-valhall-jm then?
Cheers, Daniel
On 2022-02-11 20:27, alyssa.rosenzweig@collabora.com wrote:
From: Alyssa Rosenzweig alyssa.rosenzweig@collabora.com
From the kernel's perspective, pre-CSF Valhall is more or less compatible with Bifrost, although they differ to userspace. Add a compatible for Valhall to the existing Bifrost bindings documentation.
Signed-off-by: Alyssa Rosenzweig alyssa.rosenzweig@collabora.com Cc: devicetree@vger.kernel.org
Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml | 1 + 1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml index 63a08f3f321d..48aeabd2ed68 100644 --- a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml +++ b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml @@ -23,6 +23,7 @@ properties: - rockchip,px30-mali - rockchip,rk3568-mali - const: arm,mali-bifrost # Mali Bifrost GPU model/revision is fully discoverable
- const: arm,mali-valhall # Mali Valhall GPU model/revision is fully discoverable
This requires all existing Bifrost users to add the Valhall compatible as well - I don't think that's what you want. From what we tossed about on IRC the other week, I'd imagined something more in the shape of:
compatible: oneOf: - items: - enum: - vendor,soc-mali - ... - const: arm,mali-bifrost - items: - enum: - vendor,soc-mali - ... - const: arm,mali-valhall - const: arm,mali-bifrost #or not, depending on forward-compatibility preferences
Cheers, Robin.
On Fri, 11 Feb 2022 15:27:20 -0500, alyssa.rosenzweig@collabora.com wrote:
From: Alyssa Rosenzweig alyssa.rosenzweig@collabora.com
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
dtschema/dtc warnings/errors: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.example.dt.yaml: gpu@ffe40000: compatible: ['amlogic,meson-g12a-mali', 'arm,mali-bifrost'] is too short From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/patch/1591823
This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1.
If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date:
pip3 install dtschema --upgrade
Please check and re-submit.
From: Alyssa Rosenzweig alyssa.rosenzweig@collabora.com
Add handling for the HW_ISSUE_TTRX_2968_TTRX_3162 quirk. Logic ported from kbase. kbase lists this workaround as used on Mali-G57.
Signed-off-by: Alyssa Rosenzweig alyssa.rosenzweig@collabora.com --- drivers/gpu/drm/panfrost/panfrost_gpu.c | 3 +++ drivers/gpu/drm/panfrost/panfrost_issues.h | 3 +++ drivers/gpu/drm/panfrost/panfrost_regs.h | 1 + 3 files changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/panfrost/panfrost_gpu.c b/drivers/gpu/drm/panfrost/panfrost_gpu.c index 50c8922694d7..1c1e2017aa80 100644 --- a/drivers/gpu/drm/panfrost/panfrost_gpu.c +++ b/drivers/gpu/drm/panfrost/panfrost_gpu.c @@ -108,6 +108,9 @@ static void panfrost_gpu_init_quirks(struct panfrost_device *pfdev) quirks |= SC_LS_ALLOW_ATTR_TYPES; }
+ if (panfrost_has_hw_issue(pfdev, HW_ISSUE_TTRX_2968_TTRX_3162)) + quirks |= SC_VAR_ALGORITHM; + if (panfrost_has_hw_feature(pfdev, HW_FEATURE_TLS_HASHING)) quirks |= SC_TLS_HASH_ENABLE;
diff --git a/drivers/gpu/drm/panfrost/panfrost_issues.h b/drivers/gpu/drm/panfrost/panfrost_issues.h index 8e59d765bf19..3af7d723377e 100644 --- a/drivers/gpu/drm/panfrost/panfrost_issues.h +++ b/drivers/gpu/drm/panfrost/panfrost_issues.h @@ -125,6 +125,9 @@ enum panfrost_hw_issue { * kernel must fiddle with L2 caches to prevent data leakage */ HW_ISSUE_TGOX_R1_1234,
+ /* Must set SC_VAR_ALGORITHM */ + HW_ISSUE_TTRX_2968_TTRX_3162, + HW_ISSUE_END };
diff --git a/drivers/gpu/drm/panfrost/panfrost_regs.h b/drivers/gpu/drm/panfrost/panfrost_regs.h index 16e776cc82ea..fa1e1af56e17 100644 --- a/drivers/gpu/drm/panfrost/panfrost_regs.h +++ b/drivers/gpu/drm/panfrost/panfrost_regs.h @@ -195,6 +195,7 @@ #define SC_TLS_HASH_ENABLE BIT(17) #define SC_LS_ATTR_CHECK_DISABLE BIT(18) #define SC_ENABLE_TEXGRD_FLAGS BIT(25) +#define SC_VAR_ALGORITHM BIT(29) /* End SHADER_CONFIG register */
/* TILER_CONFIG register */
On 11/02/2022 20:27, alyssa.rosenzweig@collabora.com wrote:
From: Alyssa Rosenzweig alyssa.rosenzweig@collabora.com
Add handling for the HW_ISSUE_TTRX_2968_TTRX_3162 quirk. Logic ported from kbase. kbase lists this workaround as used on Mali-G57.
Signed-off-by: Alyssa Rosenzweig alyssa.rosenzweig@collabora.com
Reviewed-by: Steven Price steven.price@arm.com
drivers/gpu/drm/panfrost/panfrost_gpu.c | 3 +++ drivers/gpu/drm/panfrost/panfrost_issues.h | 3 +++ drivers/gpu/drm/panfrost/panfrost_regs.h | 1 + 3 files changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/panfrost/panfrost_gpu.c b/drivers/gpu/drm/panfrost/panfrost_gpu.c index 50c8922694d7..1c1e2017aa80 100644 --- a/drivers/gpu/drm/panfrost/panfrost_gpu.c +++ b/drivers/gpu/drm/panfrost/panfrost_gpu.c @@ -108,6 +108,9 @@ static void panfrost_gpu_init_quirks(struct panfrost_device *pfdev) quirks |= SC_LS_ALLOW_ATTR_TYPES; }
- if (panfrost_has_hw_issue(pfdev, HW_ISSUE_TTRX_2968_TTRX_3162))
quirks |= SC_VAR_ALGORITHM;- if (panfrost_has_hw_feature(pfdev, HW_FEATURE_TLS_HASHING)) quirks |= SC_TLS_HASH_ENABLE;
diff --git a/drivers/gpu/drm/panfrost/panfrost_issues.h b/drivers/gpu/drm/panfrost/panfrost_issues.h index 8e59d765bf19..3af7d723377e 100644 --- a/drivers/gpu/drm/panfrost/panfrost_issues.h +++ b/drivers/gpu/drm/panfrost/panfrost_issues.h @@ -125,6 +125,9 @@ enum panfrost_hw_issue { * kernel must fiddle with L2 caches to prevent data leakage */ HW_ISSUE_TGOX_R1_1234,
- /* Must set SC_VAR_ALGORITHM */
- HW_ISSUE_TTRX_2968_TTRX_3162,
- HW_ISSUE_END
};
diff --git a/drivers/gpu/drm/panfrost/panfrost_regs.h b/drivers/gpu/drm/panfrost/panfrost_regs.h index 16e776cc82ea..fa1e1af56e17 100644 --- a/drivers/gpu/drm/panfrost/panfrost_regs.h +++ b/drivers/gpu/drm/panfrost/panfrost_regs.h @@ -195,6 +195,7 @@ #define SC_TLS_HASH_ENABLE BIT(17) #define SC_LS_ATTR_CHECK_DISABLE BIT(18) #define SC_ENABLE_TEXGRD_FLAGS BIT(25) +#define SC_VAR_ALGORITHM BIT(29) /* End SHADER_CONFIG register */
/* TILER_CONFIG register */
From: Alyssa Rosenzweig alyssa.rosenzweig@collabora.com
Logically, this function is free of side effects, so any pointers it takes should be const. Needed to avoid a warning in the next patch.
Signed-off-by: Alyssa Rosenzweig alyssa.rosenzweig@collabora.com --- drivers/gpu/drm/panfrost/panfrost_issues.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/panfrost/panfrost_issues.h b/drivers/gpu/drm/panfrost/panfrost_issues.h index 3af7d723377e..a66692663833 100644 --- a/drivers/gpu/drm/panfrost/panfrost_issues.h +++ b/drivers/gpu/drm/panfrost/panfrost_issues.h @@ -251,7 +251,7 @@ enum panfrost_hw_issue {
#define hw_issues_g76 0
-static inline bool panfrost_has_hw_issue(struct panfrost_device *pfdev, +static inline bool panfrost_has_hw_issue(const struct panfrost_device *pfdev, enum panfrost_hw_issue issue) { return test_bit(issue, pfdev->features.hw_issues);
On 11/02/2022 20:27, alyssa.rosenzweig@collabora.com wrote:
From: Alyssa Rosenzweig alyssa.rosenzweig@collabora.com
Logically, this function is free of side effects, so any pointers it takes should be const. Needed to avoid a warning in the next patch.
Signed-off-by: Alyssa Rosenzweig alyssa.rosenzweig@collabora.com
Reviewed-by: Steven Price steven.price@arm.com
drivers/gpu/drm/panfrost/panfrost_issues.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/panfrost/panfrost_issues.h b/drivers/gpu/drm/panfrost/panfrost_issues.h index 3af7d723377e..a66692663833 100644 --- a/drivers/gpu/drm/panfrost/panfrost_issues.h +++ b/drivers/gpu/drm/panfrost/panfrost_issues.h @@ -251,7 +251,7 @@ enum panfrost_hw_issue {
#define hw_issues_g76 0
-static inline bool panfrost_has_hw_issue(struct panfrost_device *pfdev, +static inline bool panfrost_has_hw_issue(const struct panfrost_device *pfdev, enum panfrost_hw_issue issue) { return test_bit(issue, pfdev->features.hw_issues);
From: Alyssa Rosenzweig alyssa.rosenzweig@collabora.com
Some Valhall GPUs require resets when encountering bus faults due to occlusion query writes. Add the issue bit for this and handle it.
Signed-off-by: Alyssa Rosenzweig alyssa.rosenzweig@collabora.com --- drivers/gpu/drm/panfrost/panfrost_device.c | 9 +++++++-- drivers/gpu/drm/panfrost/panfrost_issues.h | 4 ++++ 2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c b/drivers/gpu/drm/panfrost/panfrost_device.c index 7f51a4682ccb..ee612303f076 100644 --- a/drivers/gpu/drm/panfrost/panfrost_device.c +++ b/drivers/gpu/drm/panfrost/panfrost_device.c @@ -11,6 +11,7 @@ #include "panfrost_device.h" #include "panfrost_devfreq.h" #include "panfrost_features.h" +#include "panfrost_issues.h" #include "panfrost_gpu.h" #include "panfrost_job.h" #include "panfrost_mmu.h" @@ -380,9 +381,13 @@ const char *panfrost_exception_name(u32 exception_code) bool panfrost_exception_needs_reset(const struct panfrost_device *pfdev, u32 exception_code) { - /* Right now, none of the GPU we support need a reset, but this - * might change. + /* If an occlusion query write causes a bus fault on affected GPUs, + * future fragment jobs may hang. Reset to workaround. */ + if (exception_code == DRM_PANFROST_EXCEPTION_JOB_BUS_FAULT) + return panfrost_has_hw_issue(pfdev, HW_ISSUE_TTRX_3076); + + /* No other GPUs we support need a reset */ return false; }
diff --git a/drivers/gpu/drm/panfrost/panfrost_issues.h b/drivers/gpu/drm/panfrost/panfrost_issues.h index a66692663833..058f6a4c8435 100644 --- a/drivers/gpu/drm/panfrost/panfrost_issues.h +++ b/drivers/gpu/drm/panfrost/panfrost_issues.h @@ -128,6 +128,10 @@ enum panfrost_hw_issue { /* Must set SC_VAR_ALGORITHM */ HW_ISSUE_TTRX_2968_TTRX_3162,
+ /* Bus fault from occlusion query write may cause future fragment jobs + * to hang */ + HW_ISSUE_TTRX_3076, + HW_ISSUE_END };
On 11/02/2022 20:27, alyssa.rosenzweig@collabora.com wrote:
From: Alyssa Rosenzweig alyssa.rosenzweig@collabora.com
Some Valhall GPUs require resets when encountering bus faults due to occlusion query writes. Add the issue bit for this and handle it.
Signed-off-by: Alyssa Rosenzweig alyssa.rosenzweig@collabora.com
Reviewed-by: Steven Price steven.price@arm.com (although one nit below)
Just in case any one is wondering - these bus faults occur when switching the GPU's MMU to unmapped - it's not a normal "bus fault" from the external bus. This is triggered by an attempt to read unmapped memory which is completed by the driver by switching the entire MMU to unmapped.
drivers/gpu/drm/panfrost/panfrost_device.c | 9 +++++++-- drivers/gpu/drm/panfrost/panfrost_issues.h | 4 ++++ 2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c b/drivers/gpu/drm/panfrost/panfrost_device.c index 7f51a4682ccb..ee612303f076 100644 --- a/drivers/gpu/drm/panfrost/panfrost_device.c +++ b/drivers/gpu/drm/panfrost/panfrost_device.c @@ -11,6 +11,7 @@ #include "panfrost_device.h" #include "panfrost_devfreq.h" #include "panfrost_features.h" +#include "panfrost_issues.h" #include "panfrost_gpu.h" #include "panfrost_job.h" #include "panfrost_mmu.h" @@ -380,9 +381,13 @@ const char *panfrost_exception_name(u32 exception_code) bool panfrost_exception_needs_reset(const struct panfrost_device *pfdev, u32 exception_code) {
- /* Right now, none of the GPU we support need a reset, but this
* might change.
- /* If an occlusion query write causes a bus fault on affected GPUs,
* future fragment jobs may hang. Reset to workaround.- if (exception_code == DRM_PANFROST_EXCEPTION_JOB_BUS_FAULT)
return panfrost_has_hw_issue(pfdev, HW_ISSUE_TTRX_3076);- /* No other GPUs we support need a reset */ return false;
}
diff --git a/drivers/gpu/drm/panfrost/panfrost_issues.h b/drivers/gpu/drm/panfrost/panfrost_issues.h index a66692663833..058f6a4c8435 100644 --- a/drivers/gpu/drm/panfrost/panfrost_issues.h +++ b/drivers/gpu/drm/panfrost/panfrost_issues.h @@ -128,6 +128,10 @@ enum panfrost_hw_issue { /* Must set SC_VAR_ALGORITHM */ HW_ISSUE_TTRX_2968_TTRX_3162,
- /* Bus fault from occlusion query write may cause future fragment jobs
* to hang */
NIT: Kernel comment style has the "/*" and "*/" on lines by themselves for multi-line comments. checkpatch will complain!
- HW_ISSUE_TTRX_3076,
- HW_ISSUE_END
};
On Mon, Feb 14, 2022 at 04:23:18PM +0000, Steven Price wrote:
On 11/02/2022 20:27, alyssa.rosenzweig@collabora.com wrote:
From: Alyssa Rosenzweig alyssa.rosenzweig@collabora.com
Some Valhall GPUs require resets when encountering bus faults due to occlusion query writes. Add the issue bit for this and handle it.
Signed-off-by: Alyssa Rosenzweig alyssa.rosenzweig@collabora.com
Reviewed-by: Steven Price steven.price@arm.com (although one nit below)
Just in case any one is wondering - these bus faults occur when switching the GPU's MMU to unmapped - it's not a normal "bus fault" from the external bus. This is triggered by an attempt to read unmapped memory which is completed by the driver by switching the entire MMU to unmapped.
Ouch, that's subtle.
diff --git a/drivers/gpu/drm/panfrost/panfrost_issues.h b/drivers/gpu/drm/panfrost/panfrost_issues.h index a66692663833..058f6a4c8435 100644 --- a/drivers/gpu/drm/panfrost/panfrost_issues.h +++ b/drivers/gpu/drm/panfrost/panfrost_issues.h @@ -128,6 +128,10 @@ enum panfrost_hw_issue { /* Must set SC_VAR_ALGORITHM */ HW_ISSUE_TTRX_2968_TTRX_3162,
- /* Bus fault from occlusion query write may cause future fragment jobs
* to hang */NIT: Kernel comment style has the "/*" and "*/" on lines by themselves for multi-line comments. checkpatch will complain!
Yes, I am aware (and checkpatch did complain). The existing multi-line comments in that file do not have the extra lines. Consistency within the file seemed like the lesser evil. If you think it's better to appease checkpatch, I can reformat for v2.
(I can also throw in a patch fixing the rest of that file's multiline comments but that seems a bit extra.)
On 14/02/2022 17:06, Alyssa Rosenzweig wrote:
On Mon, Feb 14, 2022 at 04:23:18PM +0000, Steven Price wrote:
On 11/02/2022 20:27, alyssa.rosenzweig@collabora.com wrote:
From: Alyssa Rosenzweig alyssa.rosenzweig@collabora.com
Some Valhall GPUs require resets when encountering bus faults due to occlusion query writes. Add the issue bit for this and handle it.
Signed-off-by: Alyssa Rosenzweig alyssa.rosenzweig@collabora.com
Reviewed-by: Steven Price steven.price@arm.com (although one nit below)
Just in case any one is wondering - these bus faults occur when switching the GPU's MMU to unmapped - it's not a normal "bus fault" from the external bus. This is triggered by an attempt to read unmapped memory which is completed by the driver by switching the entire MMU to unmapped.
Ouch, that's subtle.
diff --git a/drivers/gpu/drm/panfrost/panfrost_issues.h b/drivers/gpu/drm/panfrost/panfrost_issues.h index a66692663833..058f6a4c8435 100644 --- a/drivers/gpu/drm/panfrost/panfrost_issues.h +++ b/drivers/gpu/drm/panfrost/panfrost_issues.h @@ -128,6 +128,10 @@ enum panfrost_hw_issue { /* Must set SC_VAR_ALGORITHM */ HW_ISSUE_TTRX_2968_TTRX_3162,
- /* Bus fault from occlusion query write may cause future fragment jobs
* to hang */NIT: Kernel comment style has the "/*" and "*/" on lines by themselves for multi-line comments. checkpatch will complain!
Yes, I am aware (and checkpatch did complain). The existing multi-line comments in that file do not have the extra lines. Consistency within the file seemed like the lesser evil. If you think it's better to appease checkpatch, I can reformat for v2.
(I can also throw in a patch fixing the rest of that file's multiline comments but that seems a bit extra.)
Nah! I've never been very keen on that style rule myself, but I usually try to keep checkpatch happy when working on the kernel. Lets just follow the rest of the file for now.
Thanks,
Steve
From: Alyssa Rosenzweig alyssa.rosenzweig@collabora.com
TTRX_3485 requires the infamous "dummy job" workaround. I have this workaround implemented in a local branch, but I have not yet hit a case that requires it so I cannot test whether the implementation is correct. In the mean time, add the quirk bit so we can document which platforms may need it in the future.
Signed-off-by: Alyssa Rosenzweig alyssa.rosenzweig@collabora.com --- drivers/gpu/drm/panfrost/panfrost_issues.h | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/panfrost/panfrost_issues.h b/drivers/gpu/drm/panfrost/panfrost_issues.h index 058f6a4c8435..b8865fc9efce 100644 --- a/drivers/gpu/drm/panfrost/panfrost_issues.h +++ b/drivers/gpu/drm/panfrost/panfrost_issues.h @@ -132,6 +132,9 @@ enum panfrost_hw_issue { * to hang */ HW_ISSUE_TTRX_3076,
+ /* Must issue a dummy job before starting real work to prevent hangs */ + HW_ISSUE_TTRX_3485, + HW_ISSUE_END };
On 11/02/2022 20:27, alyssa.rosenzweig@collabora.com wrote:
From: Alyssa Rosenzweig alyssa.rosenzweig@collabora.com
TTRX_3485 requires the infamous "dummy job" workaround. I have this workaround implemented in a local branch, but I have not yet hit a case that requires it so I cannot test whether the implementation is correct. In the mean time, add the quirk bit so we can document which platforms may need it in the future.
This one is hideous ;) Although to me this isn't the 'infamous' one as it's not the earliest example of a dummy job.
However... I believe as Panfrost currently stands this is probably not very possible to hit. It requires a job to be stopped (soft or hard) at a critical point during submission - which at the moment Panfrost basically never does (the exception is if you close the fd immediately while a job is in progress). And of course the timing has to be 'just right' to hit the bug.
That said I think we should probably add pre-emption support sometime at which point this could become an issue.
Signed-off-by: Alyssa Rosenzweig alyssa.rosenzweig@collabora.com
Reviewed-by: Steven Price steven.price@arm.com
drivers/gpu/drm/panfrost/panfrost_issues.h | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/panfrost/panfrost_issues.h b/drivers/gpu/drm/panfrost/panfrost_issues.h index 058f6a4c8435..b8865fc9efce 100644 --- a/drivers/gpu/drm/panfrost/panfrost_issues.h +++ b/drivers/gpu/drm/panfrost/panfrost_issues.h @@ -132,6 +132,9 @@ enum panfrost_hw_issue { * to hang */ HW_ISSUE_TTRX_3076,
- /* Must issue a dummy job before starting real work to prevent hangs */
- HW_ISSUE_TTRX_3485,
- HW_ISSUE_END
};
TTRX_3485 requires the infamous "dummy job" workaround. I have this workaround implemented in a local branch, but I have not yet hit a case that requires it so I cannot test whether the implementation is correct. In the mean time, add the quirk bit so we can document which platforms may need it in the future.
This one is hideous ;) Although to me this isn't the 'infamous' one as it's not the earliest example of a dummy job.
Terrifying. I guess we narrowly avoided the 'replay' workaround which was far worse than this one...
However... I believe as Panfrost currently stands this is probably not very possible to hit. It requires a job to be stopped (soft or hard) at a critical point during submission - which at the moment Panfrost basically never does (the exception is if you close the fd immediately while a job is in progress). And of course the timing has to be 'just right' to hit the bug.
OK, that's good to know. Still "should" be fixed but that definitely lowers the priority of it. Frankly the multithreading bugs we have on the CPU side would hang the machine sooner...
From: Alyssa Rosenzweig alyssa.rosenzweig@collabora.com
Add the HW_FEATURE_CLEAN_ONLY_SAFE bit based on kbase. When I actually tried to port the logic from kbase, trivial jobs raised Data Invalid Faults, so this may depend on other coherency details. It's still useful to have the bit to record the feature bit when adding new models.
Signed-off-by: Alyssa Rosenzweig alyssa.rosenzweig@collabora.com --- drivers/gpu/drm/panfrost/panfrost_features.h | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/panfrost/panfrost_features.h b/drivers/gpu/drm/panfrost/panfrost_features.h index 36fadcf9634e..1a8bdebc86a3 100644 --- a/drivers/gpu/drm/panfrost/panfrost_features.h +++ b/drivers/gpu/drm/panfrost/panfrost_features.h @@ -21,6 +21,7 @@ enum panfrost_hw_feature { HW_FEATURE_TLS_HASHING, HW_FEATURE_THREAD_GROUP_SPLIT, HW_FEATURE_IDVS_GROUP_SIZE, + HW_FEATURE_CLEAN_ONLY_SAFE, HW_FEATURE_3BIT_EXT_RW_L2_MMU_CONFIG, };
On 11/02/2022 20:27, alyssa.rosenzweig@collabora.com wrote:
From: Alyssa Rosenzweig alyssa.rosenzweig@collabora.com
Add the HW_FEATURE_CLEAN_ONLY_SAFE bit based on kbase. When I actually tried to port the logic from kbase, trivial jobs raised Data Invalid Faults, so this may depend on other coherency details. It's still useful to have the bit to record the feature bit when adding new models.
Signed-off-by: Alyssa Rosenzweig alyssa.rosenzweig@collabora.com
Reviewed-by: Steven Price steven.price@arm.com
Sadly I don't have the hardware to try this out on, but it should be a simple case of the below (untested):
----8<---- diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c b/drivers/gpu/drm/panfrost/panfrost_job.c index 908d79520853..602e51c4966e 100644 --- a/drivers/gpu/drm/panfrost/panfrost_job.c +++ b/drivers/gpu/drm/panfrost/panfrost_job.c @@ -212,9 +212,13 @@ static void panfrost_job_hw_submit(struct panfrost_job *job, int js) * start */ cfg |= JS_CONFIG_THREAD_PRI(8) | JS_CONFIG_START_FLUSH_CLEAN_INVALIDATE | - JS_CONFIG_END_FLUSH_CLEAN_INVALIDATE | panfrost_get_job_chain_flag(job);
+ if (panfrost_has_hw_feature(pfdev, HW_FEATURE_CLEAN_ONLY_SAFE)) + cfg |= JS_CONFIG_END_FLUSH_CLEAN; + else + cfg |= JS_CONFIG_END_FLUSH_CLEAN_INVALIDATE; + if (panfrost_has_hw_feature(pfdev, HW_FEATURE_FLUSH_REDUCTION)) cfg |= JS_CONFIG_ENABLE_FLUSH_REDUCTION;
----8<----
Steve
drivers/gpu/drm/panfrost/panfrost_features.h | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/panfrost/panfrost_features.h b/drivers/gpu/drm/panfrost/panfrost_features.h index 36fadcf9634e..1a8bdebc86a3 100644 --- a/drivers/gpu/drm/panfrost/panfrost_features.h +++ b/drivers/gpu/drm/panfrost/panfrost_features.h @@ -21,6 +21,7 @@ enum panfrost_hw_feature { HW_FEATURE_TLS_HASHING, HW_FEATURE_THREAD_GROUP_SPLIT, HW_FEATURE_IDVS_GROUP_SIZE,
- HW_FEATURE_CLEAN_ONLY_SAFE, HW_FEATURE_3BIT_EXT_RW_L2_MMU_CONFIG,
};
Add the HW_FEATURE_CLEAN_ONLY_SAFE bit based on kbase. When I actually tried to port the logic from kbase, trivial jobs raised Data Invalid Faults, so this may depend on other coherency details. It's still useful to have the bit to record the feature bit when adding new models.
Signed-off-by: Alyssa Rosenzweig alyssa.rosenzweig@collabora.com
Reviewed-by: Steven Price steven.price@arm.com
Sadly I don't have the hardware to try this out on, but it should be a simple case of the below (untested):
----8<---- diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c b/drivers/gpu/drm/panfrost/panfrost_job.c index 908d79520853..602e51c4966e 100644 --- a/drivers/gpu/drm/panfrost/panfrost_job.c +++ b/drivers/gpu/drm/panfrost/panfrost_job.c @@ -212,9 +212,13 @@ static void panfrost_job_hw_submit(struct panfrost_job *job, int js) * start */ cfg |= JS_CONFIG_THREAD_PRI(8) | JS_CONFIG_START_FLUSH_CLEAN_INVALIDATE |
JS_CONFIG_END_FLUSH_CLEAN_INVALIDATE | panfrost_get_job_chain_flag(job);
if (panfrost_has_hw_feature(pfdev, HW_FEATURE_CLEAN_ONLY_SAFE))cfg |= JS_CONFIG_END_FLUSH_CLEAN;elsecfg |= JS_CONFIG_END_FLUSH_CLEAN_INVALIDATE;if (panfrost_has_hw_feature(pfdev, HW_FEATURE_FLUSH_REDUCTION)) cfg |= JS_CONFIG_ENABLE_FLUSH_REDUCTION;
Yes, this is the patch I typed out... causes DATA_INVALID_FAULTs for me with Mesa. Which makes me wonder if userspace needs to respect some extra rules for this to be safe.
On 14/02/2022 17:01, Alyssa Rosenzweig wrote:
Add the HW_FEATURE_CLEAN_ONLY_SAFE bit based on kbase. When I actually tried to port the logic from kbase, trivial jobs raised Data Invalid Faults, so this may depend on other coherency details. It's still useful to have the bit to record the feature bit when adding new models.
Signed-off-by: Alyssa Rosenzweig alyssa.rosenzweig@collabora.com
Reviewed-by: Steven Price steven.price@arm.com
Sadly I don't have the hardware to try this out on, but it should be a simple case of the below (untested):
----8<---- diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c b/drivers/gpu/drm/panfrost/panfrost_job.c index 908d79520853..602e51c4966e 100644 --- a/drivers/gpu/drm/panfrost/panfrost_job.c +++ b/drivers/gpu/drm/panfrost/panfrost_job.c @@ -212,9 +212,13 @@ static void panfrost_job_hw_submit(struct panfrost_job *job, int js) * start */ cfg |= JS_CONFIG_THREAD_PRI(8) | JS_CONFIG_START_FLUSH_CLEAN_INVALIDATE |
JS_CONFIG_END_FLUSH_CLEAN_INVALIDATE | panfrost_get_job_chain_flag(job);
if (panfrost_has_hw_feature(pfdev, HW_FEATURE_CLEAN_ONLY_SAFE))cfg |= JS_CONFIG_END_FLUSH_CLEAN;elsecfg |= JS_CONFIG_END_FLUSH_CLEAN_INVALIDATE;if (panfrost_has_hw_feature(pfdev, HW_FEATURE_FLUSH_REDUCTION)) cfg |= JS_CONFIG_ENABLE_FLUSH_REDUCTION;Yes, this is the patch I typed out... causes DATA_INVALID_FAULTs for me with Mesa. Which makes me wonder if userspace needs to respect some extra rules for this to be safe.
Odd - the invalidate at the end of the job shouldn't be needed to read the job descriptors from userspace only the one at the beginning.
However I'm wondering if there's something fishy happening with the flush reduction. That allows skipping the cache maintenance at the beginning of a job if there has already been one for other reasons. But I can't immediately see any difference in the way kbase handles this.
Steve
From: Alyssa Rosenzweig alyssa.rosenzweig@collabora.com
L2_MMU_CONFIG is an implementation-defined register. Different Mali GPUs define slightly different MAX_READS and MAX_WRITES fields, which throttle outstanding reads and writes when set to non-zero values. When left as zero, reads and writes are not throttled.
Both kbase and panfrost always zero these registers. Per discussion with Steven Price, there are two reasons these quirks may be used:
1. Simulating slower memory subsystems. This use case is only of interest to system-on-chip designers; it is not relevant to mainline.
2. Working around broken memory subsystems. Hopefully we never see this case in mainline. If we do, we'll need to set this register based on an SoC-compatible, rather than generally matching on the GPU model.
To the best of our knowledge, these fields are zero at reset, so the write is not necessary. Let's remove the write to aid porting to new Mali GPUs, which have different layouts for the L2_MMU_CONFIG register.
Signed-off-by: Alyssa Rosenzweig alyssa.rosenzweig@collabora.com Suggested-by: Steven Price steven.price@arm.com --- drivers/gpu/drm/panfrost/panfrost_gpu.c | 12 ------------ 1 file changed, 12 deletions(-)
diff --git a/drivers/gpu/drm/panfrost/panfrost_gpu.c b/drivers/gpu/drm/panfrost/panfrost_gpu.c index 1c1e2017aa80..73e5774f01c1 100644 --- a/drivers/gpu/drm/panfrost/panfrost_gpu.c +++ b/drivers/gpu/drm/panfrost/panfrost_gpu.c @@ -127,18 +127,6 @@ static void panfrost_gpu_init_quirks(struct panfrost_device *pfdev) gpu_write(pfdev, GPU_TILER_CONFIG, quirks);
- quirks = gpu_read(pfdev, GPU_L2_MMU_CONFIG); - - /* Limit read & write ID width for AXI */ - if (panfrost_has_hw_feature(pfdev, HW_FEATURE_3BIT_EXT_RW_L2_MMU_CONFIG)) - quirks &= ~(L2_MMU_CONFIG_3BIT_LIMIT_EXTERNAL_READS | - L2_MMU_CONFIG_3BIT_LIMIT_EXTERNAL_WRITES); - else - quirks &= ~(L2_MMU_CONFIG_LIMIT_EXTERNAL_READS | - L2_MMU_CONFIG_LIMIT_EXTERNAL_WRITES); - - gpu_write(pfdev, GPU_L2_MMU_CONFIG, quirks); - quirks = 0; if ((panfrost_model_eq(pfdev, 0x860) || panfrost_model_eq(pfdev, 0x880)) && pfdev->features.revision >= 0x2000)
On 11/02/2022 20:27, alyssa.rosenzweig@collabora.com wrote:
From: Alyssa Rosenzweig alyssa.rosenzweig@collabora.com
L2_MMU_CONFIG is an implementation-defined register. Different Mali GPUs define slightly different MAX_READS and MAX_WRITES fields, which throttle outstanding reads and writes when set to non-zero values. When left as zero, reads and writes are not throttled.
Both kbase and panfrost always zero these registers. Per discussion with Steven Price, there are two reasons these quirks may be used:
Simulating slower memory subsystems. This use case is only of interest to system-on-chip designers; it is not relevant to mainline.
Working around broken memory subsystems. Hopefully we never see this case in mainline. If we do, we'll need to set this register based on an SoC-compatible, rather than generally matching on the GPU model.
To the best of our knowledge, these fields are zero at reset, so the write is not necessary. Let's remove the write to aid porting to new Mali GPUs, which have different layouts for the L2_MMU_CONFIG register.
Signed-off-by: Alyssa Rosenzweig alyssa.rosenzweig@collabora.com Suggested-by: Steven Price steven.price@arm.com
Reviewed-by: Steven Price steven.price@arm.com
drivers/gpu/drm/panfrost/panfrost_gpu.c | 12 ------------ 1 file changed, 12 deletions(-)
diff --git a/drivers/gpu/drm/panfrost/panfrost_gpu.c b/drivers/gpu/drm/panfrost/panfrost_gpu.c index 1c1e2017aa80..73e5774f01c1 100644 --- a/drivers/gpu/drm/panfrost/panfrost_gpu.c +++ b/drivers/gpu/drm/panfrost/panfrost_gpu.c @@ -127,18 +127,6 @@ static void panfrost_gpu_init_quirks(struct panfrost_device *pfdev) gpu_write(pfdev, GPU_TILER_CONFIG, quirks);
- quirks = gpu_read(pfdev, GPU_L2_MMU_CONFIG);
- /* Limit read & write ID width for AXI */
- if (panfrost_has_hw_feature(pfdev, HW_FEATURE_3BIT_EXT_RW_L2_MMU_CONFIG))
quirks &= ~(L2_MMU_CONFIG_3BIT_LIMIT_EXTERNAL_READS |L2_MMU_CONFIG_3BIT_LIMIT_EXTERNAL_WRITES);- else
quirks &= ~(L2_MMU_CONFIG_LIMIT_EXTERNAL_READS |L2_MMU_CONFIG_LIMIT_EXTERNAL_WRITES);- gpu_write(pfdev, GPU_L2_MMU_CONFIG, quirks);
- quirks = 0; if ((panfrost_model_eq(pfdev, 0x860) || panfrost_model_eq(pfdev, 0x880)) && pfdev->features.revision >= 0x2000)
From: Alyssa Rosenzweig alyssa.rosenzweig@collabora.com
Add the features, issues, and GPU ID for Mali-G57, a first-generation Valhall GPU. Other first- and second-generation Valhall GPUs should be similar.
Signed-off-by: Alyssa Rosenzweig alyssa.rosenzweig@collabora.com --- drivers/gpu/drm/panfrost/panfrost_features.h | 12 ++++++++++++ drivers/gpu/drm/panfrost/panfrost_gpu.c | 2 ++ drivers/gpu/drm/panfrost/panfrost_issues.h | 5 +++++ 3 files changed, 19 insertions(+)
diff --git a/drivers/gpu/drm/panfrost/panfrost_features.h b/drivers/gpu/drm/panfrost/panfrost_features.h index 1a8bdebc86a3..7ed0cd3ea2d4 100644 --- a/drivers/gpu/drm/panfrost/panfrost_features.h +++ b/drivers/gpu/drm/panfrost/panfrost_features.h @@ -106,6 +106,18 @@ enum panfrost_hw_feature { BIT_ULL(HW_FEATURE_TLS_HASHING) | \ BIT_ULL(HW_FEATURE_3BIT_EXT_RW_L2_MMU_CONFIG))
+#define hw_features_g57 (\ + BIT_ULL(HW_FEATURE_JOBCHAIN_DISAMBIGUATION) | \ + BIT_ULL(HW_FEATURE_PWRON_DURING_PWROFF_TRANS) | \ + BIT_ULL(HW_FEATURE_XAFFINITY) | \ + BIT_ULL(HW_FEATURE_FLUSH_REDUCTION) | \ + BIT_ULL(HW_FEATURE_PROTECTED_MODE) | \ + BIT_ULL(HW_FEATURE_PROTECTED_DEBUG_MODE) | \ + BIT_ULL(HW_FEATURE_COHERENCY_REG) | \ + BIT_ULL(HW_FEATURE_AARCH64_MMU) | \ + BIT_ULL(HW_FEATURE_IDVS_GROUP_SIZE) | \ + BIT_ULL(HW_FEATURE_CLEAN_ONLY_SAFE)) + static inline bool panfrost_has_hw_feature(struct panfrost_device *pfdev, enum panfrost_hw_feature feat) { diff --git a/drivers/gpu/drm/panfrost/panfrost_gpu.c b/drivers/gpu/drm/panfrost/panfrost_gpu.c index 73e5774f01c1..08d657527099 100644 --- a/drivers/gpu/drm/panfrost/panfrost_gpu.c +++ b/drivers/gpu/drm/panfrost/panfrost_gpu.c @@ -201,6 +201,8 @@ static const struct panfrost_model gpu_models[] = { GPU_MODEL(g52, 0x7002), GPU_MODEL(g31, 0x7003, GPU_REV(g31, 1, 0)), + + GPU_MODEL(g57, 0x9001), };
static void panfrost_gpu_init_features(struct panfrost_device *pfdev) diff --git a/drivers/gpu/drm/panfrost/panfrost_issues.h b/drivers/gpu/drm/panfrost/panfrost_issues.h index b8865fc9efce..1a0dc7f7f857 100644 --- a/drivers/gpu/drm/panfrost/panfrost_issues.h +++ b/drivers/gpu/drm/panfrost/panfrost_issues.h @@ -258,6 +258,11 @@ enum panfrost_hw_issue {
#define hw_issues_g76 0
+#define hw_issues_g57 (\ + BIT_ULL(HW_ISSUE_TTRX_2968_TTRX_3162) | \ + BIT_ULL(HW_ISSUE_TTRX_3076) | \ + BIT_ULL(HW_ISSUE_TTRX_3485)) + static inline bool panfrost_has_hw_issue(const struct panfrost_device *pfdev, enum panfrost_hw_issue issue) {
On 11/02/2022 20:27, alyssa.rosenzweig@collabora.com wrote:
From: Alyssa Rosenzweig alyssa.rosenzweig@collabora.com
Add the features, issues, and GPU ID for Mali-G57, a first-generation Valhall GPU. Other first- and second-generation Valhall GPUs should be similar.
Signed-off-by: Alyssa Rosenzweig alyssa.rosenzweig@collabora.com
drivers/gpu/drm/panfrost/panfrost_features.h | 12 ++++++++++++ drivers/gpu/drm/panfrost/panfrost_gpu.c | 2 ++ drivers/gpu/drm/panfrost/panfrost_issues.h | 5 +++++ 3 files changed, 19 insertions(+)
diff --git a/drivers/gpu/drm/panfrost/panfrost_features.h b/drivers/gpu/drm/panfrost/panfrost_features.h index 1a8bdebc86a3..7ed0cd3ea2d4 100644 --- a/drivers/gpu/drm/panfrost/panfrost_features.h +++ b/drivers/gpu/drm/panfrost/panfrost_features.h @@ -106,6 +106,18 @@ enum panfrost_hw_feature { BIT_ULL(HW_FEATURE_TLS_HASHING) | \ BIT_ULL(HW_FEATURE_3BIT_EXT_RW_L2_MMU_CONFIG))
+#define hw_features_g57 (\
- BIT_ULL(HW_FEATURE_JOBCHAIN_DISAMBIGUATION) | \
- BIT_ULL(HW_FEATURE_PWRON_DURING_PWROFF_TRANS) | \
- BIT_ULL(HW_FEATURE_XAFFINITY) | \
- BIT_ULL(HW_FEATURE_FLUSH_REDUCTION) | \
- BIT_ULL(HW_FEATURE_PROTECTED_MODE) | \
- BIT_ULL(HW_FEATURE_PROTECTED_DEBUG_MODE) | \
- BIT_ULL(HW_FEATURE_COHERENCY_REG) | \
- BIT_ULL(HW_FEATURE_AARCH64_MMU) | \
- BIT_ULL(HW_FEATURE_IDVS_GROUP_SIZE) | \
- BIT_ULL(HW_FEATURE_CLEAN_ONLY_SAFE))
static inline bool panfrost_has_hw_feature(struct panfrost_device *pfdev, enum panfrost_hw_feature feat) { diff --git a/drivers/gpu/drm/panfrost/panfrost_gpu.c b/drivers/gpu/drm/panfrost/panfrost_gpu.c index 73e5774f01c1..08d657527099 100644 --- a/drivers/gpu/drm/panfrost/panfrost_gpu.c +++ b/drivers/gpu/drm/panfrost/panfrost_gpu.c @@ -201,6 +201,8 @@ static const struct panfrost_model gpu_models[] = { GPU_MODEL(g52, 0x7002), GPU_MODEL(g31, 0x7003, GPU_REV(g31, 1, 0)),
- GPU_MODEL(g57, 0x9001),
};
static void panfrost_gpu_init_features(struct panfrost_device *pfdev) diff --git a/drivers/gpu/drm/panfrost/panfrost_issues.h b/drivers/gpu/drm/panfrost/panfrost_issues.h index b8865fc9efce..1a0dc7f7f857 100644 --- a/drivers/gpu/drm/panfrost/panfrost_issues.h +++ b/drivers/gpu/drm/panfrost/panfrost_issues.h @@ -258,6 +258,11 @@ enum panfrost_hw_issue {
#define hw_issues_g76 0
+#define hw_issues_g57 (\
- BIT_ULL(HW_ISSUE_TTRX_2968_TTRX_3162) | \
- BIT_ULL(HW_ISSUE_TTRX_3076) | \
- BIT_ULL(HW_ISSUE_TTRX_3485))
Do you know whether you have an r0p0 or an r0p1 Natt? Only the r0p0 has the 3485 issue, and we might be lucky and it's the r0p1 that's "in the wild".
It would be good to annotate these lists with the hardware revisions when there is a difference.
Steve
static inline bool panfrost_has_hw_issue(const struct panfrost_device *pfdev, enum panfrost_hw_issue issue) {
index b8865fc9efce..1a0dc7f7f857 100644 --- a/drivers/gpu/drm/panfrost/panfrost_issues.h +++ b/drivers/gpu/drm/panfrost/panfrost_issues.h @@ -258,6 +258,11 @@ enum panfrost_hw_issue {
#define hw_issues_g76 0
+#define hw_issues_g57 (\
- BIT_ULL(HW_ISSUE_TTRX_2968_TTRX_3162) | \
- BIT_ULL(HW_ISSUE_TTRX_3076) | \
- BIT_ULL(HW_ISSUE_TTRX_3485))
Do you know whether you have an r0p0 or an r0p1 Natt? Only the r0p0 has the 3485 issue, and we might be lucky and it's the r0p1 that's "in the wild".
Sadly, I believe I have an r0p0. I don't know if future spins of the same SoC would be bumped up, but I'm skeptical.
It would be good to annotate these lists with the hardware revisions when there is a difference.
Sure.
From: Alyssa Rosenzweig alyssa.rosenzweig@collabora.com
The most important Valhall-specific quirks have been handled, so add the Valhall compatible and probe.
Signed-off-by: Alyssa Rosenzweig alyssa.rosenzweig@collabora.com --- drivers/gpu/drm/panfrost/panfrost_drv.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c index 96bb5a465627..12977454af75 100644 --- a/drivers/gpu/drm/panfrost/panfrost_drv.c +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c @@ -663,6 +663,7 @@ static const struct of_device_id dt_match[] = { { .compatible = "arm,mali-t860", .data = &default_data, }, { .compatible = "arm,mali-t880", .data = &default_data, }, { .compatible = "arm,mali-bifrost", .data = &default_data, }, + { .compatible = "arm,mali-valhall", .data = &default_data, }, { .compatible = "mediatek,mt8183-mali", .data = &mediatek_mt8183_data }, {} };
On 11/02/2022 20:27, alyssa.rosenzweig@collabora.com wrote:
From: Alyssa Rosenzweig alyssa.rosenzweig@collabora.com
The most important Valhall-specific quirks have been handled, so add the Valhall compatible and probe.
Signed-off-by: Alyssa Rosenzweig alyssa.rosenzweig@collabora.com
Reviewed-by: Steven Price steven.price@arm.com
drivers/gpu/drm/panfrost/panfrost_drv.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c index 96bb5a465627..12977454af75 100644 --- a/drivers/gpu/drm/panfrost/panfrost_drv.c +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c @@ -663,6 +663,7 @@ static const struct of_device_id dt_match[] = { { .compatible = "arm,mali-t860", .data = &default_data, }, { .compatible = "arm,mali-t880", .data = &default_data, }, { .compatible = "arm,mali-bifrost", .data = &default_data, },
- { .compatible = "arm,mali-valhall", .data = &default_data, }, { .compatible = "mediatek,mt8183-mali", .data = &mediatek_mt8183_data }, {}
};
dri-devel@lists.freedesktop.org
-
Alyssa Rosenzweig -
alyssa.rosenzweig＠collabora.com -
Daniel Stone -
Rob Herring -
Robin Murphy -
Steven Price