diff mbox series

[v2] arm64: dts: qcom: msm8998: enable adreno_smmu by default

Message ID be51d1a4-e8fc-48d1-9afb-a42b1d6ca478@freebox.fr (mailing list archive)
State Accepted
Headers show
Series [v2] arm64: dts: qcom: msm8998: enable adreno_smmu by default | expand

Commit Message

Marc Gonzalez May 15, 2024, 2:27 p.m. UTC 
15 qcom platform DTSI files define an adreno_smmu node.
msm8998 is the only one with adreno_smmu disabled by default.

There's no reason why this SMMU should be disabled by default,
it doesn't need any further configuration.

Bring msm8998 in line with the 14 other platforms.

This fixes GPU init failing with ENODEV:
msm_dpu c901000.display-controller: failed to load adreno gpu
msm_dpu c901000.display-controller: failed to bind 5000000.gpu (ops a3xx_ops): -19

Fixes: 87cd46d68aeac8 ("Configure Adreno GPU and related IOMMU")
Signed-off-by: Marc Gonzalez <mgonzalez@freebox.fr>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
New in v2: rewrote commit message with input from Martin, Bryan, Luca
Supersedes: <1ba7031f-c97c-41f1-8cbc-d99f1e848e76@freebox.fr>

Maintainers, feel free to drop the Fixes tag.

Failure log:
[    2.756363] [drm:adreno_bind] Found GPU: 5.4.0.1
[    2.767183] [drm:a5xx_gpu_init]
[    2.767422] [drm:adreno_gpu_init] fast_rate=710000097, slow_rate=27000000
[    3.003869] [drm:msm_gpu_init] ebi1_clk: fffffffffffffffe
[    3.004002] adreno 5000000.gpu: supply vdd not found, using dummy regulator
[    3.008463] [drm:msm_gpu_init] gpu_reg: ffff0000819e4000
[    3.015105] adreno 5000000.gpu: supply vddcx not found, using dummy regulator
[    3.020702] [drm:msm_gpu_init] gpu_cx: ffff0000819e4180
[    3.028173] [drm:adreno_iommu_create_address_space]
[    3.054552] [drm:msm_gpu_init] gpu->aspace=ffffffffffffffed
[    3.058112] [drm:a5xx_destroy] 5.4.0.1
[    3.065922] [drm:msm_gpu_cleanup] 5.4.0.1
[    3.074237] msm_dpu c901000.display-controller: failed to load adreno gpu
[    3.082412] msm_dpu c901000.display-controller: failed to bind 5000000.gpu (ops a3xx_ops): -19
[    3.088342] msm_dpu c901000.display-controller: [drm:drm_managed_release] drmres release begin
...
[    3.197694] [drm:drm_managed_release] drmres release end
[    3.204009] msm_dpu c901000.display-controller: adev bind failed: -19
---
 arch/arm64/boot/dts/qcom/msm8998.dtsi | 1 -
 1 file changed, 1 deletion(-)

Comments

Marijn Suijten May 15, 2024, 2:48 p.m. UTC | #1 
On 2024-05-15 16:27:44, Marc Gonzalez wrote:
> 15 qcom platform DTSI files define an adreno_smmu node.
> msm8998 is the only one with adreno_smmu disabled by default.
> 
> There's no reason why this SMMU should be disabled by default,
> it doesn't need any further configuration.
> 
> Bring msm8998 in line with the 14 other platforms.
> 
> This fixes GPU init failing with ENODEV:

Nit: I'd specialize this to:

"This saves every MSM8998 board DTS from having to enable adreno_smmu when
enabling the Adreno GPU node, which leads to the following unclear probe failure
when forgotten about:"

But no need to send a v3 for that.

> msm_dpu c901000.display-controller: failed to load adreno gpu
> msm_dpu c901000.display-controller: failed to bind 5000000.gpu (ops a3xx_ops): -19

And as a more general discussion, which is not really relevant to have your
commit message, we could have a separate patch adding an error message detailing
**where** this -19 came from.  In this case create_address_space().

> Fixes: 87cd46d68aeac8 ("Configure Adreno GPU and related IOMMU")
> Signed-off-by: Marc Gonzalez <mgonzalez@freebox.fr>
> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>

> ---
> New in v2: rewrote commit message with input from Martin, Bryan, Luca

Thanks!

> Supersedes: <1ba7031f-c97c-41f1-8cbc-d99f1e848e76@freebox.fr>
> 
> Maintainers, feel free to drop the Fixes tag.

For context, I don't think the original patch was necessarily wrong in disabling
this node, as any node using it (adreno_gpu) is also disabled by default.  It is
however less consistent with other DTSI, which don't require every board DTS to
re-enable adreno_smmu.

- Marijn

> Failure log:
> [    2.756363] [drm:adreno_bind] Found GPU: 5.4.0.1
> [    2.767183] [drm:a5xx_gpu_init]
> [    2.767422] [drm:adreno_gpu_init] fast_rate=710000097, slow_rate=27000000
> [    3.003869] [drm:msm_gpu_init] ebi1_clk: fffffffffffffffe
> [    3.004002] adreno 5000000.gpu: supply vdd not found, using dummy regulator
> [    3.008463] [drm:msm_gpu_init] gpu_reg: ffff0000819e4000
> [    3.015105] adreno 5000000.gpu: supply vddcx not found, using dummy regulator
> [    3.020702] [drm:msm_gpu_init] gpu_cx: ffff0000819e4180
> [    3.028173] [drm:adreno_iommu_create_address_space]
> [    3.054552] [drm:msm_gpu_init] gpu->aspace=ffffffffffffffed
> [    3.058112] [drm:a5xx_destroy] 5.4.0.1
> [    3.065922] [drm:msm_gpu_cleanup] 5.4.0.1
> [    3.074237] msm_dpu c901000.display-controller: failed to load adreno gpu
> [    3.082412] msm_dpu c901000.display-controller: failed to bind 5000000.gpu (ops a3xx_ops): -19
> [    3.088342] msm_dpu c901000.display-controller: [drm:drm_managed_release] drmres release begin
> ...
> [    3.197694] [drm:drm_managed_release] drmres release end
> [    3.204009] msm_dpu c901000.display-controller: adev bind failed: -19
> ---
>  arch/arm64/boot/dts/qcom/msm8998.dtsi | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi b/arch/arm64/boot/dts/qcom/msm8998.dtsi
> index 3d3b1f61c0690..edf379c28e1e1 100644
> --- a/arch/arm64/boot/dts/qcom/msm8998.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi
> @@ -1580,7 +1580,6 @@ adreno_smmu: iommu@5040000 {
>  			 * SoC VDDMX RPM Power Domain in the Adreno driver.
>  			 */
>  			power-domains = <&gpucc GPU_GX_GDSC>;
> -			status = "disabled";
>  		};
>  
>  		gpucc: clock-controller@5065000 {
> -- 
> 2.34.1
>
Jeffrey Hugo May 17, 2024, 3:09 p.m. UTC | #2 
On 5/15/2024 8:27 AM, Marc Gonzalez wrote:
> 15 qcom platform DTSI files define an adreno_smmu node.
> msm8998 is the only one with adreno_smmu disabled by default.
> 
> There's no reason why this SMMU should be disabled by default,
> it doesn't need any further configuration.
> 
> Bring msm8998 in line with the 14 other platforms.
> 
> This fixes GPU init failing with ENODEV:
> msm_dpu c901000.display-controller: failed to load adreno gpu
> msm_dpu c901000.display-controller: failed to bind 5000000.gpu (ops a3xx_ops): -19
> 
> Fixes: 87cd46d68aeac8 ("Configure Adreno GPU and related IOMMU")
> Signed-off-by: Marc Gonzalez <mgonzalez@freebox.fr>
> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

Reviewed-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
Bjorn Andersson May 28, 2024, 3:32 a.m. UTC | #3 
On Wed, 15 May 2024 16:27:44 +0200, Marc Gonzalez wrote:
> 15 qcom platform DTSI files define an adreno_smmu node.
> msm8998 is the only one with adreno_smmu disabled by default.
> 
> There's no reason why this SMMU should be disabled by default,
> it doesn't need any further configuration.
> 
> Bring msm8998 in line with the 14 other platforms.
> 
> [...]

Applied, thanks!

[1/1] arm64: dts: qcom: msm8998: enable adreno_smmu by default
      commit: 98a0c4f2278b4d6c1c7722735c20b2247de6293f

Best regards,
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi b/arch/arm64/boot/dts/qcom/msm8998.dtsi
index 3d3b1f61c0690..edf379c28e1e1 100644
--- a/arch/arm64/boot/dts/qcom/msm8998.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi
@@ -1580,7 +1580,6 @@  adreno_smmu: iommu@5040000 {
 			 * SoC VDDMX RPM Power Domain in the Adreno driver.
 			 */
 			power-domains = <&gpucc GPU_GX_GDSC>;
-			status = "disabled";
 		};
 
 		gpucc: clock-controller@5065000 {