as it says on the can
drm/msm patches for Rob
arm64 patches for linux-arm-msm
for use with https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/25408
tested on QCM6490 (SC7280-IOT) Fairphone FP5
Signed-off-by: Konrad Dybcio <[email protected]>
---
Konrad Dybcio (7):
drm/msm/a6xx: Fix unknown speedbin case
drm/msm/adreno: Add ZAP firmware name to A635
drm/msm/adreno: Add A635 speedbin 0xac (A643)
arm64: dts: qcom: sc7280: Add ZAP shader support
arm64: dts: qcom: sc7280: Fix up GPU SIDs
arm64: dts: qcom: sc7280: Mark Adreno SMMU as DMA coherent
arm64: dts: qcom: sc7280: Add 0xac Adreno speed bin
arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi | 2 ++
arch/arm64/boot/dts/qcom/sc7280.dtsi | 26 ++++++++++++++++------
drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 2 +-
drivers/gpu/drm/msm/adreno/adreno_device.c | 2 ++
4 files changed, 24 insertions(+), 8 deletions(-)
---
base-commit: 4ae73bba62a367f2314f6ce69e3085a941983d8b
change-id: 20230926-topic-a643-a7ec9a08a3a1
Best regards,
--
Konrad Dybcio <[email protected]>
When opp-supported-hw is present under an OPP node, but no form of
opp_set_supported_hw() has been called, that OPP is ignored by the API
and marked as unsupported.
Before Commit c928a05e4415 ("drm/msm/adreno: Move speedbin mapping to
device table"), an unknown speedbin would result in marking all OPPs
as available, but it's better to avoid potentially overclocking the
silicon - the GMU will simply refuse to power up the chip.
Currently, the Adreno speedbin code does just that (AND returns an
invalid error, (int)UINT_MAX). Fix that by defaulting to speedbin 0
(which is conveniently always bound to fuseval == 0).
Fixes: c928a05e4415 ("drm/msm/adreno: Move speedbin mapping to device table")
Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index d4e85e24002f..522ca7fe6762 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -2237,7 +2237,7 @@ static int a6xx_set_supported_hw(struct device *dev, const struct adreno_info *i
DRM_DEV_ERROR(dev,
"missing support for speed-bin: %u. Some OPPs may not be supported by hardware\n",
speedbin);
- return UINT_MAX;
+ supp_hw = BIT(0); /* Default */
}
ret = devm_pm_opp_set_supported_hw(dev, &supp_hw, 1);
--
2.42.0
Downstream calls this the "speedbin 1", but that number is already
occupied. Use index two.
Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/gpu/drm/msm/adreno/adreno_device.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c
index 16527fe8584d..4977fd759b5b 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_device.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
@@ -460,6 +460,7 @@ static const struct adreno_info gpulist[] = {
.speedbins = ADRENO_SPEEDBINS(
{ 0, 0 },
{ 117, 0 },
+ { 172, 2 }, /* Called speedbin 1 downstream, but let's not break things! */
{ 190, 1 },
),
}, {
--
2.42.0
A643 (A635 speedbin 0xac) tops out at 812 MHz. Fill in the
opp-supported-hw appropriately.
Note that fuseval 0xac is referred to as speedbin 1 downstream, but
that was already in use upstream, so 2 was chosen instead.
Signed-off-by: Konrad Dybcio <[email protected]>
---
arch/arm64/boot/dts/qcom/sc7280.dtsi | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index edaca6c2cf8c..ccc2dd6c45de 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -2626,14 +2626,14 @@ opp-315000000 {
opp-hz = /bits/ 64 <315000000>;
opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS>;
opp-peak-kBps = <1804000>;
- opp-supported-hw = <0x03>;
+ opp-supported-hw = <0x07>;
};
opp-450000000 {
opp-hz = /bits/ 64 <450000000>;
opp-level = <RPMH_REGULATOR_LEVEL_SVS>;
opp-peak-kBps = <4068000>;
- opp-supported-hw = <0x03>;
+ opp-supported-hw = <0x07>;
};
/* Only applicable for SKUs which has 550Mhz as Fmax */
@@ -2648,28 +2648,28 @@ opp-550000000-1 {
opp-hz = /bits/ 64 <550000000>;
opp-level = <RPMH_REGULATOR_LEVEL_SVS_L1>;
opp-peak-kBps = <6832000>;
- opp-supported-hw = <0x02>;
+ opp-supported-hw = <0x06>;
};
opp-608000000 {
opp-hz = /bits/ 64 <608000000>;
opp-level = <RPMH_REGULATOR_LEVEL_SVS_L2>;
opp-peak-kBps = <8368000>;
- opp-supported-hw = <0x02>;
+ opp-supported-hw = <0x06>;
};
opp-700000000 {
opp-hz = /bits/ 64 <700000000>;
opp-level = <RPMH_REGULATOR_LEVEL_NOM>;
opp-peak-kBps = <8532000>;
- opp-supported-hw = <0x02>;
+ opp-supported-hw = <0x06>;
};
opp-812000000 {
opp-hz = /bits/ 64 <812000000>;
opp-level = <RPMH_REGULATOR_LEVEL_NOM_L1>;
opp-peak-kBps = <8532000>;
- opp-supported-hw = <0x02>;
+ opp-supported-hw = <0x06>;
};
opp-840000000 {
--
2.42.0
Some (many?) devices with A635 expect a ZAP shader to be loaded.
Set the file name to allow for that.
Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/gpu/drm/msm/adreno/adreno_device.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c
index fa527935ffd4..16527fe8584d 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_device.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
@@ -454,6 +454,7 @@ static const struct adreno_info gpulist[] = {
.quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT |
ADRENO_QUIRK_HAS_HW_APRIV,
.init = a6xx_gpu_init,
+ .zapfw = "a660_zap.mbn",
.hwcg = a660_hwcg,
.address_space_size = SZ_16G,
.speedbins = ADRENO_SPEEDBINS(
--
2.42.0
Non-Chrome SC7280-family platforms ship a ZAP shader with the Adreno GPU.
Describe that and make sure it doesn't interfere with Chrome devices.
Signed-off-by: Konrad Dybcio <[email protected]>
---
arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi | 2 ++
arch/arm64/boot/dts/qcom/sc7280.dtsi | 10 ++++++++++
2 files changed, 12 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi b/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
index 5d462ae14ba1..88fc67c3646e 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
@@ -17,6 +17,8 @@
* required by the setup for Chrome boards.
*/
+/delete-node/ &gpu_zap_mem;
+/delete-node/ &gpu_zap_shader;
/delete-node/ &hyp_mem;
/delete-node/ &xbl_mem;
/delete-node/ &reserved_xbl_uefi_log;
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 66f1eb83cca7..c38ddf267ef5 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -152,6 +152,11 @@ ipa_fw_mem: memory@8b700000 {
no-map;
};
+ gpu_zap_mem: zap@8b71a000 {
+ reg = <0 0x8b71a000 0 0x2000>;
+ no-map;
+ };
+
rmtfs_mem: memory@9c900000 {
compatible = "qcom,rmtfs-mem";
reg = <0x0 0x9c900000 0x0 0x280000>;
@@ -2608,6 +2613,11 @@ gpu: gpu@3d00000 {
nvmem-cells = <&gpu_speed_bin>;
nvmem-cell-names = "speed_bin";
+ gpu_zap_shader: zap-shader {
+ memory-region = <&gpu_zap_mem>;
+ firmware-name = "qcom/a660_zap.mdt";
+ };
+
gpu_opp_table: opp-table {
compatible = "operating-points-v2";
--
2.42.0
GPU_SMMU SID 1 is meant for Adreno LPAC (Low Priority Async Compute).
On platforms that support it (in firmware), it is necessary to
describe that link, or Adreno register access will hang the board.
Add that and fix up the SMR mask of SID 0, which seems to have been
copypasted from another SoC.
Fixes: 96c471970b7b ("arm64: dts: qcom: sc7280: Add gpu support")
Signed-off-by: Konrad Dybcio <[email protected]>
---
arch/arm64/boot/dts/qcom/sc7280.dtsi | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index c38ddf267ef5..0d96d1454c49 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -2603,7 +2603,8 @@ gpu: gpu@3d00000 {
"cx_mem",
"cx_dbgc";
interrupts = <GIC_SPI 300 IRQ_TYPE_LEVEL_HIGH>;
- iommus = <&adreno_smmu 0 0x401>;
+ iommus = <&adreno_smmu 0 0x400>,
+ <&adreno_smmu 1 0x400>;
operating-points-v2 = <&gpu_opp_table>;
qcom,gmu = <&gmu>;
interconnects = <&gem_noc MASTER_GFX3D 0 &mc_virt SLAVE_EBI1 0>;
--
2.42.0
On 26.09.2023 20:24, Konrad Dybcio wrote:
> Non-Chrome SC7280-family platforms ship a ZAP shader with the Adreno GPU.
> Describe that and make sure it doesn't interfere with Chrome devices.
>
> Signed-off-by: Konrad Dybcio <[email protected]>
> ---
> arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi | 2 ++
> arch/arm64/boot/dts/qcom/sc7280.dtsi | 10 ++++++++++
> 2 files changed, 12 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi b/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
> index 5d462ae14ba1..88fc67c3646e 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
> @@ -17,6 +17,8 @@
> * required by the setup for Chrome boards.
> */
>
> +/delete-node/ &gpu_zap_mem;
> +/delete-node/ &gpu_zap_shader;
> /delete-node/ &hyp_mem;
> /delete-node/ &xbl_mem;
> /delete-node/ &reserved_xbl_uefi_log;
> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> index 66f1eb83cca7..c38ddf267ef5 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> @@ -152,6 +152,11 @@ ipa_fw_mem: memory@8b700000 {
> no-map;
> };
>
> + gpu_zap_mem: zap@8b71a000 {
> + reg = <0 0x8b71a000 0 0x2000>;
> + no-map;
> + };
> +
> rmtfs_mem: memory@9c900000 {
> compatible = "qcom,rmtfs-mem";
> reg = <0x0 0x9c900000 0x0 0x280000>;
> @@ -2608,6 +2613,11 @@ gpu: gpu@3d00000 {
> nvmem-cells = <&gpu_speed_bin>;
> nvmem-cell-names = "speed_bin";
>
> + gpu_zap_shader: zap-shader {
> + memory-region = <&gpu_zap_mem>;
> + firmware-name = "qcom/a660_zap.mdt";
Gah. This line shouldn't have been there.
Considering it's not the only oops, I'll resend.
Konrad
The SMMUs on sc7280 are cache-coherent. APPS_SMMU is marked as such,
mark the GPU one as well.
Signed-off-by: Konrad Dybcio <[email protected]>
---
arch/arm64/boot/dts/qcom/sc7280.dtsi | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 0d96d1454c49..edaca6c2cf8c 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -2783,6 +2783,7 @@ adreno_smmu: iommu@3da0000 {
"gpu_cc_hub_aon_clk";
power-domains = <&gpucc GPU_CC_CX_GDSC>;
+ dma-coherent;
};
remoteproc_mpss: remoteproc@4080000 {
--
2.42.0
On Tue, Sep 26, 2023 at 08:24:36PM +0200, Konrad Dybcio wrote:
>
> When opp-supported-hw is present under an OPP node, but no form of
> opp_set_supported_hw() has been called, that OPP is ignored by the API
> and marked as unsupported.
>
> Before Commit c928a05e4415 ("drm/msm/adreno: Move speedbin mapping to
> device table"), an unknown speedbin would result in marking all OPPs
> as available, but it's better to avoid potentially overclocking the
> silicon - the GMU will simply refuse to power up the chip.
>
> Currently, the Adreno speedbin code does just that (AND returns an
> invalid error, (int)UINT_MAX). Fix that by defaulting to speedbin 0
> (which is conveniently always bound to fuseval == 0).
Wish we documented somewhere that we should reserve BIT(0) for fuse
val=0 always and assume that would be the super SKU.
Reviewed-by: Akhil P Oommen <[email protected]>
-Akhil
>
> Fixes: c928a05e4415 ("drm/msm/adreno: Move speedbin mapping to device table")
> Signed-off-by: Konrad Dybcio <[email protected]>
> ---
> drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> index d4e85e24002f..522ca7fe6762 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> @@ -2237,7 +2237,7 @@ static int a6xx_set_supported_hw(struct device *dev, const struct adreno_info *i
> DRM_DEV_ERROR(dev,
> "missing support for speed-bin: %u. Some OPPs may not be supported by hardware\n",
> speedbin);
> - return UINT_MAX;
> + supp_hw = BIT(0); /* Default */
> }
>
> ret = devm_pm_opp_set_supported_hw(dev, &supp_hw, 1);
>
> --
> 2.42.0
>
On Tue, Sep 26, 2023 at 08:24:37PM +0200, Konrad Dybcio wrote:
>
> Some (many?) devices with A635 expect a ZAP shader to be loaded.
>
> Set the file name to allow for that.
>
> Signed-off-by: Konrad Dybcio <[email protected]>
> ---
> drivers/gpu/drm/msm/adreno/adreno_device.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c
> index fa527935ffd4..16527fe8584d 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_device.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
> @@ -454,6 +454,7 @@ static const struct adreno_info gpulist[] = {
> .quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT |
> ADRENO_QUIRK_HAS_HW_APRIV,
> .init = a6xx_gpu_init,
> + .zapfw = "a660_zap.mbn",
sc7280 doesn't have a TZ and so no zap shader support. Can we handle
this using "firmware-name" property in your top level platform dt? Zap
firmwares are signed with different keys for each OEMs. So there is
cross-compatibility anyway.
-Ahil.
> .hwcg = a660_hwcg,
> .address_space_size = SZ_16G,
> .speedbins = ADRENO_SPEEDBINS(
>
> --
> 2.42.0
>
On Tue, Sep 26, 2023 at 08:24:40PM +0200, Konrad Dybcio wrote:
>
> GPU_SMMU SID 1 is meant for Adreno LPAC (Low Priority Async Compute).
> On platforms that support it (in firmware), it is necessary to
> describe that link, or Adreno register access will hang the board.
>
> Add that and fix up the SMR mask of SID 0, which seems to have been
> copypasted from another SoC.
>
> Fixes: 96c471970b7b ("arm64: dts: qcom: sc7280: Add gpu support")
> Signed-off-by: Konrad Dybcio <[email protected]>
> ---
> arch/arm64/boot/dts/qcom/sc7280.dtsi | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> index c38ddf267ef5..0d96d1454c49 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> @@ -2603,7 +2603,8 @@ gpu: gpu@3d00000 {
> "cx_mem",
> "cx_dbgc";
> interrupts = <GIC_SPI 300 IRQ_TYPE_LEVEL_HIGH>;
> - iommus = <&adreno_smmu 0 0x401>;
> + iommus = <&adreno_smmu 0 0x400>,
> + <&adreno_smmu 1 0x400>;
Aren't both functionally same? 401 works fine on sc7280. You might be
having issue due to Qcom TZ policies on your platform. I am okay with the change, but can
you please reword the commit text?
-Akhil.
> operating-points-v2 = <&gpu_opp_table>;
> qcom,gmu = <&gmu>;
> interconnects = <&gem_noc MASTER_GFX3D 0 &mc_virt SLAVE_EBI1 0>;
>
> --
> 2.42.0
>
On Tue, Sep 26, 2023 at 08:24:41PM +0200, Konrad Dybcio wrote:
>
> The SMMUs on sc7280 are cache-coherent. APPS_SMMU is marked as such,
> mark the GPU one as well.
>
> Signed-off-by: Konrad Dybcio <[email protected]>
Reviewed-by: Akhil P Oommen <[email protected]>
-Akhil
> ---
> arch/arm64/boot/dts/qcom/sc7280.dtsi | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> index 0d96d1454c49..edaca6c2cf8c 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> @@ -2783,6 +2783,7 @@ adreno_smmu: iommu@3da0000 {
> "gpu_cc_hub_aon_clk";
>
> power-domains = <&gpucc GPU_CC_CX_GDSC>;
> + dma-coherent;
> };
>
> remoteproc_mpss: remoteproc@4080000 {
>
> --
> 2.42.0
>
On Mon, Oct 16, 2023 at 1:12 PM Akhil P Oommen <[email protected]> wrote:
>
> On Tue, Sep 26, 2023 at 08:24:37PM +0200, Konrad Dybcio wrote:
> >
> > Some (many?) devices with A635 expect a ZAP shader to be loaded.
> >
> > Set the file name to allow for that.
> >
> > Signed-off-by: Konrad Dybcio <[email protected]>
> > ---
> > drivers/gpu/drm/msm/adreno/adreno_device.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c
> > index fa527935ffd4..16527fe8584d 100644
> > --- a/drivers/gpu/drm/msm/adreno/adreno_device.c
> > +++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
> > @@ -454,6 +454,7 @@ static const struct adreno_info gpulist[] = {
> > .quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT |
> > ADRENO_QUIRK_HAS_HW_APRIV,
> > .init = a6xx_gpu_init,
> > + .zapfw = "a660_zap.mbn",
>
> sc7280 doesn't have a TZ and so no zap shader support. Can we handle
> this using "firmware-name" property in your top level platform dt? Zap
> firmwares are signed with different keys for each OEMs. So there is
> cross-compatibility anyway.
I think this ends up working out because the version of sc7280 that
doesn't have TZ also doesn't have the associated mem-region/etc.. but
maybe we should deprecate the zapfw field as in practice it isn't
useful (ie. always overriden by firmware-name).
Fwiw there are windows laptops with sc7180/sc7280 which do use zap fw.
BR,
-R
>
> -Ahil.
>
> > .hwcg = a660_hwcg,
> > .address_space_size = SZ_16G,
> > .speedbins = ADRENO_SPEEDBINS(
> >
> > --
> > 2.42.0
> >
On 10/17/23 09:33, Rob Clark wrote:
> On Mon, Oct 16, 2023 at 1:12 PM Akhil P Oommen <[email protected]> wrote:
>>
>> On Tue, Sep 26, 2023 at 08:24:37PM +0200, Konrad Dybcio wrote:
>>>
>>> Some (many?) devices with A635 expect a ZAP shader to be loaded.
>>>
>>> Set the file name to allow for that.
>>>
>>> Signed-off-by: Konrad Dybcio <[email protected]>
>>> ---
>>> drivers/gpu/drm/msm/adreno/adreno_device.c | 1 +
>>> 1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c
>>> index fa527935ffd4..16527fe8584d 100644
>>> --- a/drivers/gpu/drm/msm/adreno/adreno_device.c
>>> +++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
>>> @@ -454,6 +454,7 @@ static const struct adreno_info gpulist[] = {
>>> .quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT |
>>> ADRENO_QUIRK_HAS_HW_APRIV,
>>> .init = a6xx_gpu_init,
>>> + .zapfw = "a660_zap.mbn",
>>
>> sc7280 doesn't have a TZ and so no zap shader support. Can we handle
>> this using "firmware-name" property in your top level platform dt? Zap
>> firmwares are signed with different keys for each OEMs. So there is
>> cross-compatibility anyway.
>
> I think this ends up working out because the version of sc7280 that
> doesn't have TZ also doesn't have the associated mem-region/etc..
Yes
> but
> maybe we should deprecate the zapfw field as in practice it isn't
> useful (ie. always overriden by firmware-name).
Also yes, we've discussed that on IRC once
>
> Fwiw there are windows laptops with sc7180/sc7280 which do use zap fw.
Correct, e.g. the SC7180 Acer Aspire 1 that is supported upstream.
Konrad
Konrad
On Tue, Oct 17, 2023 at 12:33:45AM -0700, Rob Clark wrote:
>
> On Mon, Oct 16, 2023 at 1:12 PM Akhil P Oommen <[email protected]> wrote:
> >
> > On Tue, Sep 26, 2023 at 08:24:37PM +0200, Konrad Dybcio wrote:
> > >
> > > Some (many?) devices with A635 expect a ZAP shader to be loaded.
> > >
> > > Set the file name to allow for that.
> > >
> > > Signed-off-by: Konrad Dybcio <[email protected]>
> > > ---
> > > drivers/gpu/drm/msm/adreno/adreno_device.c | 1 +
> > > 1 file changed, 1 insertion(+)
> > >
> > > diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c
> > > index fa527935ffd4..16527fe8584d 100644
> > > --- a/drivers/gpu/drm/msm/adreno/adreno_device.c
> > > +++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
> > > @@ -454,6 +454,7 @@ static const struct adreno_info gpulist[] = {
> > > .quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT |
> > > ADRENO_QUIRK_HAS_HW_APRIV,
> > > .init = a6xx_gpu_init,
> > > + .zapfw = "a660_zap.mbn",
> >
> > sc7280 doesn't have a TZ and so no zap shader support. Can we handle
> > this using "firmware-name" property in your top level platform dt? Zap
> > firmwares are signed with different keys for each OEMs. So there is
> > cross-compatibility anyway.
I had a typo here. I meant "no cross compatibility".
>
> I think this ends up working out because the version of sc7280 that
> doesn't have TZ also doesn't have the associated mem-region/etc.. but
> maybe we should deprecate the zapfw field as in practice it isn't
> useful (ie. always overriden by firmware-name).
Sounds good.
>
> Fwiw there are windows laptops with sc7180/sc7280 which do use zap fw.
Aah! right.
>
> BR,
> -R
>
> >
> > -Ahil.
> >
> > > .hwcg = a660_hwcg,
> > > .address_space_size = SZ_16G,
> > > .speedbins = ADRENO_SPEEDBINS(
> > >
> > > --
> > > 2.42.0
> > >
On Tue, Oct 17, 2023 at 01:22:27AM +0530, Akhil P Oommen wrote:
>
> On Tue, Sep 26, 2023 at 08:24:36PM +0200, Konrad Dybcio wrote:
> >
> > When opp-supported-hw is present under an OPP node, but no form of
> > opp_set_supported_hw() has been called, that OPP is ignored by the API
> > and marked as unsupported.
> >
> > Before Commit c928a05e4415 ("drm/msm/adreno: Move speedbin mapping to
> > device table"), an unknown speedbin would result in marking all OPPs
> > as available, but it's better to avoid potentially overclocking the
> > silicon - the GMU will simply refuse to power up the chip.
> >
> > Currently, the Adreno speedbin code does just that (AND returns an
> > invalid error, (int)UINT_MAX). Fix that by defaulting to speedbin 0
> > (which is conveniently always bound to fuseval == 0).
>
> Wish we documented somewhere that we should reserve BIT(0) for fuse
> val=0 always and assume that would be the super SKU.
Aah! I got this backward. Fuseval=0 is the supersku and it is not safe
to fallback to that blindly. Ideally, we should fallback to the lowest
denominator SKU, but it is difficult to predict that upfront and assign
BIT(0).
Anyway, I can't see a better way to handle this.
-Akhil
>
> Reviewed-by: Akhil P Oommen <[email protected]>
>
> -Akhil
>
> >
> > Fixes: c928a05e4415 ("drm/msm/adreno: Move speedbin mapping to device table")
> > Signed-off-by: Konrad Dybcio <[email protected]>
> > ---
> > drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> > index d4e85e24002f..522ca7fe6762 100644
> > --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> > @@ -2237,7 +2237,7 @@ static int a6xx_set_supported_hw(struct device *dev, const struct adreno_info *i
> > DRM_DEV_ERROR(dev,
> > "missing support for speed-bin: %u. Some OPPs may not be supported by hardware\n",
> > speedbin);
> > - return UINT_MAX;
> > + supp_hw = BIT(0); /* Default */
> > }
> >
> > ret = devm_pm_opp_set_supported_hw(dev, &supp_hw, 1);
> >
> > --
> > 2.42.0
> >
On 10/16/23 22:22, Akhil P Oommen wrote:
> On Tue, Sep 26, 2023 at 08:24:40PM +0200, Konrad Dybcio wrote:
>>
>> GPU_SMMU SID 1 is meant for Adreno LPAC (Low Priority Async Compute).
>> On platforms that support it (in firmware), it is necessary to
>> describe that link, or Adreno register access will hang the board.
>>
>> Add that and fix up the SMR mask of SID 0, which seems to have been
>> copypasted from another SoC.
>>
>> Fixes: 96c471970b7b ("arm64: dts: qcom: sc7280: Add gpu support")
>> Signed-off-by: Konrad Dybcio <[email protected]>
>> ---
>> arch/arm64/boot/dts/qcom/sc7280.dtsi | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
>> index c38ddf267ef5..0d96d1454c49 100644
>> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
>> @@ -2603,7 +2603,8 @@ gpu: gpu@3d00000 {
>> "cx_mem",
>> "cx_dbgc";
>> interrupts = <GIC_SPI 300 IRQ_TYPE_LEVEL_HIGH>;
>> - iommus = <&adreno_smmu 0 0x401>;
>> + iommus = <&adreno_smmu 0 0x400>,
>> + <&adreno_smmu 1 0x400>;
> Aren't both functionally same? 401 works fine on sc7280. You might be
> having issue due to Qcom TZ policies on your platform. I am okay with the change, but can
> you please reword the commit text?
Hm, looking at what the SMR registers represent, it looks like
they should do the same thing and it may indeed be down to the
TZ being picky.. I'll rephrase.
Konrad