Skip to contentSkip to conversation

gpu + venus + iommu dt nodes for msm8916

PATCH In review Rob Clark 2 participants 8 messages May 25, 2017 – May 31, 2017

Subsystems (3)
lore
Share & tools
lore

Rob Clark

author
#0 lore raw

cover gpu + venus + iommu dt nodes for msm8916

I don't think Stanimir already sent the venus DT node. But since venus is queued up for 4.13, the gpu bindings are already upstream, and the qcom_iommu will hopefully land someday (and bindings doc already has Rob H's r-b), I figure it makes sense to land the dt nodes.

Sending together since 3/3 touches both venus and gpu nodes to wire up the iommu.

Rob Clark (2):
ARM64: DT: add gpu for msm8916
ARM64: DT: add iommu for msm8916

Stanimir Varbanov (1):
ARM64: DT: add video codec devicetree node

 arch/arm64/boot/dts/qcom/msm8916.dtsi | 120 ++++++++++++++++++++++++++++++++++
 1 file changed, 120 insertions(+)
Signature · 1 line
2.9.4
author
#1 lore raw

PATCH 1/3 ARM64: DT: add gpu for msm8916

---
 arch/arm64/boot/dts/qcom/msm8916.dtsi | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)
arch/arm64/boot/dts/qcom/msm8916.dtsi +35 -0
index ab30939..f9b8a28 100644
@@ -698,6 +698,41 @@
 			#thermal-sensor-cells = <1>;
 		};
 
+		gpu_opp_table: opp_table {
+			compatible = "operating-points-v2";
+
+			opp@400000000 {
+				opp-hz = /bits/ 64 <400000000>;
+			};
+			opp@19200000 {
+				opp-hz = /bits/ 64 <19200000>;
+			};
+		};
+
+		gpu@01c00000 {
+			compatible = "qcom,adreno-306.0", "qcom,adreno";
+			reg = <0x01c00000 0x20000>;
+			reg-names = "kgsl_3d0_reg_memory";
+			interrupts = <0 33 0>;
+			interrupt-names = "kgsl_3d0_irq";
+			clock-names =
+			    "core",
+			    "iface",
+			    "mem",
+			    "mem_iface",
+			    "alt_mem_iface",
+			    "gfx3d_clk";
+			clocks =
+			    <&gcc GCC_OXILI_GFX3D_CLK>,
+			    <&gcc GCC_OXILI_AHB_CLK>,
+			    <&gcc GCC_OXILI_GMEM_CLK>,
+			    <&gcc GCC_BIMC_GFX_CLK>,
+			    <&gcc GCC_BIMC_GPU_CLK>,
+			    <&gcc GFX3D_CLK_SRC>;
+			power-domains = <&gcc OXILI_GDSC>;
+			operating-points-v2 = <&gpu_opp_table>;
+		};
+
 		mdss: mdss@1a00000 {
 			compatible = "qcom,mdss";
 			reg = <0x1a00000 0x1000>,
Signature · 1 line
2.9.4
#2 lore raw

Re: [PATCH 1/3] ARM64: DT: add gpu for msm8916

Quoted 10 lines · Rob Clark
---
 arch/arm64/boot/dts/qcom/msm8916.dtsi | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)
arch/arm64/boot/dts/qcom/msm8916.dtsi +0 -0
index ab30939..f9b8a28 100644

arch/arm64/boot/dts/qcom/msm8916.dtsi line 704

Quoted 7 lines
arch/arm64/boot/dts/qcom/msm8916.dtsi +4 -0
@@ -698,6 +698,41 @@
 			#thermal-sensor-cells = <1>;
 		};
 
+		gpu_opp_table: opp_table {
+			compatible = "operating-points-v2";
+
+			opp@400000000 {

We recently changed the binding to have dash instead of at sign:

			opp-400000000 {

arch/arm64/boot/dts/qcom/msm8916.dtsi line 710

Quoted 6 lines
arch/arm64/boot/dts/qcom/msm8916.dtsi +6 -0
+				opp-hz = /bits/ 64 <400000000>;
+			};
+			opp@19200000 {
+				opp-hz = /bits/ 64 <19200000>;
+			};
+		};

This should be outside of soc node.

arch/arm64/boot/dts/qcom/msm8916.dtsi line 712

Quoted 2 lines
arch/arm64/boot/dts/qcom/msm8916.dtsi +2 -0
+
+		gpu@01c00000 {

Drop leading 0 on unit addresses please.

arch/arm64/boot/dts/qcom/msm8916.dtsi line 716

Quoted 4 lines
arch/arm64/boot/dts/qcom/msm8916.dtsi +4 -0
+			compatible = "qcom,adreno-306.0", "qcom,adreno";
+			reg = <0x01c00000 0x20000>;
+			reg-names = "kgsl_3d0_reg_memory";
+			interrupts = <0 33 0>;

Can we get an irq trigger type?

interrupts = <GIC_SPI 33 IRQ_TYPE_LEVEL_HIGH>

arch/arm64/boot/dts/qcom/msm8916.dtsi line 724

Quoted 8 lines
arch/arm64/boot/dts/qcom/msm8916.dtsi +8 -0
+			interrupt-names = "kgsl_3d0_irq";
+			clock-names =
+			    "core",
+			    "iface",
+			    "mem",
+			    "mem_iface",
+			    "alt_mem_iface",
+			    "gfx3d_clk";

It's still called gfx3d_clk? Too bad we couldn't get rid of that last _clk one.

Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
author
#3 lore raw

PATCH 2/3 ARM64: DT: add video codec devicetree node

In reply to Rob Clark

From: Stanimir Varbanov <[email protected]>

---
 arch/arm64/boot/dts/qcom/msm8916.dtsi | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)
arch/arm64/boot/dts/qcom/msm8916.dtsi +28 -0
index f9b8a28..2ef8f53 100644
@@ -88,6 +88,13 @@
 			no-map;
 		};
 
+		venus_mem: venus@89900000 {
+			compatible = "shared-dma-pool";
+			reg = <0x0 0x89900000 0x0 0x800000>;
+			alignment = <0x1000>;
+			no-map;
+		};
+
 		mba_mem: mba@8ea00000 {
 			no-map;
 			reg = <0 0x8ea00000 0 0x100000>;
@@ -1214,6 +1221,27 @@
 				};
 			};
 		};
+
+		venus: video-codec@1d00000 {
+			compatible = "qcom,msm8916-venus";
+			reg = <0x01d00000 0xff000>;
+			interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_HIGH>;
+			power-domains = <&gcc VENUS_GDSC>;
+			clocks = <&gcc GCC_VENUS0_VCODEC0_CLK>,
+				 <&gcc GCC_VENUS0_AHB_CLK>,
+				 <&gcc GCC_VENUS0_AXI_CLK>;
+			clock-names = "core", "iface", "bus";
+			memory-region = <&venus_mem>;
+			status = "okay";
+
+			video-decoder {
+				compatible = "venus-decoder";
+			};
+
+			video-encoder {
+				compatible = "venus-encoder";
+			};
+		};
 	};
 
 	smd {
Signature · 1 line
2.9.4
author
#4 lore raw

PATCH 3/3 ARM64: DT: add iommu for msm8916

In reply to Rob Clark
---
 arch/arm64/boot/dts/qcom/msm8916.dtsi | 57 +++++++++++++++++++++++++++++++++++
 1 file changed, 57 insertions(+)
arch/arm64/boot/dts/qcom/msm8916.dtsi +57 -0
index 2ef8f53..13bb079 100644
@@ -705,6 +705,59 @@
 			#thermal-sensor-cells = <1>;
 		};
 
+		apps_iommu: iommu@1ef0000 {
+			#address-cells = <1>;
+			#size-cells = <1>;
+			#iommu-cells = <1>;
+			compatible = "qcom,msm8916-iommu", "qcom,msm-iommu-v1";
+			ranges = <0 0x1e20000 0x40000>;
+			reg = <0x1ef0000 0x3000>;
+			clocks = <&gcc GCC_SMMU_CFG_CLK>,
+				 <&gcc GCC_APSS_TCU_CLK>;
+			clock-names = "iface", "bus";
+			qcom,iommu-secure-id = <17>;
+
+			// mdp_0:
+			iommu-ctx@4000 {
+				compatible = "qcom,msm-iommu-v1-ns";
+				reg = <0x4000 0x1000>;
+				interrupts = <GIC_SPI 70 0>;
+			};
+
+			// venus_ns:
+			iommu-ctx@5000 {
+				compatible = "qcom,msm-iommu-v1-sec";
+				reg = <0x5000 0x1000>;
+				interrupts = <GIC_SPI 70 0>;
+			};
+		};
+
+		gpu_iommu: iommu@1f08000 {
+			#address-cells = <1>;
+			#size-cells = <1>;
+			#iommu-cells = <1>;
+			compatible = "qcom,msm8916-iommu", "qcom,msm-iommu-v1";
+			ranges = <0 0x1f08000 0x10000>;
+			clocks = <&gcc GCC_SMMU_CFG_CLK>,
+				 <&gcc GCC_GFX_TCU_CLK>;
+			clock-names = "iface", "bus";
+			qcom,iommu-secure-id = <18>;
+
+			// gfx3d_user:
+			iommu-ctx@1000 {
+				compatible = "qcom,msm-iommu-v1-ns";
+				reg = <0x1000 0x1000>;
+				interrupts = <GIC_SPI 241 0>;
+			};
+
+			// gfx3d_priv:
+			iommu-ctx@2000 {
+				compatible = "qcom,msm-iommu-v1-ns";
+				reg = <0x2000 0x1000>;
+				interrupts = <GIC_SPI 242 0>;
+			};
+		};
+
 		gpu_opp_table: opp_table {
 			compatible = "operating-points-v2";
 
@@ -738,6 +791,7 @@
 			    <&gcc GFX3D_CLK_SRC>;
 			power-domains = <&gcc OXILI_GDSC>;
 			operating-points-v2 = <&gpu_opp_table>;
+			iommus = <&gpu_iommu 1>, <&gpu_iommu 2>;
 		};
 
 		mdss: mdss@1a00000 {
@@ -781,6 +835,8 @@
 					      "core_clk",
 					      "vsync_clk";
 
+				iommus = <&apps_iommu 4>;
+
 				ports {
 					#address-cells = <1>;
 					#size-cells = <0>;
@@ -1231,6 +1287,7 @@
 				 <&gcc GCC_VENUS0_AHB_CLK>,
 				 <&gcc GCC_VENUS0_AXI_CLK>;
 			clock-names = "core", "iface", "bus";
+			iommus = <&apps_iommu 5>;
 			memory-region = <&venus_mem>;
 			status = "okay";
 
Signature · 1 line
2.9.4
#5 lore raw

Re: [PATCH 3/3] ARM64: DT: add iommu for msm8916

Quoted 7 lines · Rob Clark
arch/arm64/boot/dts/qcom/msm8916.dtsi +7 -0
+		apps_iommu: iommu@1ef0000 {
+			#address-cells = <1>;
+			#size-cells = <1>;
+			#iommu-cells = <1>;
+			compatible = "qcom,msm8916-iommu", "qcom,msm-iommu-v1";
+			ranges = <0 0x1e20000 0x40000>;
+			reg = <0x1ef0000 0x3000>;

arch/arm64/boot/dts/qcom/msm8916.dtsi line 724

Quoted 10 lines
arch/arm64/boot/dts/qcom/msm8916.dtsi +10 -0
+			clocks = <&gcc GCC_SMMU_CFG_CLK>,
+				 <&gcc GCC_APSS_TCU_CLK>;
+			clock-names = "iface", "bus";
+			qcom,iommu-secure-id = <17>;
+
+			// mdp_0:
+			iommu-ctx@4000 {
+				compatible = "qcom,msm-iommu-v1-ns";
+				reg = <0x4000 0x1000>;
+				interrupts = <GIC_SPI 70 0>;

s/0/IRQ_TYPE_LEVEL_HIGH/

arch/arm64/boot/dts/qcom/msm8916.dtsi line 731

Quoted 7 lines
arch/arm64/boot/dts/qcom/msm8916.dtsi +7 -0
+			};
+
+			// venus_ns:
+			iommu-ctx@5000 {
+				compatible = "qcom,msm-iommu-v1-sec";
+				reg = <0x5000 0x1000>;
+				interrupts = <GIC_SPI 70 0>;

s/0/IRQ_TYPE_LEVEL_HIGH/

Is it the same interrupt number (70) twice? Not 71 or something?

Quoted 9 lines
arch/arm64/boot/dts/qcom/msm8916.dtsi +9 -0
+			};
+		};
+
+		gpu_iommu: iommu@1f08000 {
+			#address-cells = <1>;
+			#size-cells = <1>;
+			#iommu-cells = <1>;
+			compatible = "qcom,msm8916-iommu", "qcom,msm-iommu-v1";
+			ranges = <0 0x1f08000 0x10000>;

arch/arm64/boot/dts/qcom/msm8916.dtsi line 750

Quoted 10 lines
arch/arm64/boot/dts/qcom/msm8916.dtsi +10 -0
+			clocks = <&gcc GCC_SMMU_CFG_CLK>,
+				 <&gcc GCC_GFX_TCU_CLK>;
+			clock-names = "iface", "bus";
+			qcom,iommu-secure-id = <18>;
+
+			// gfx3d_user:
+			iommu-ctx@1000 {
+				compatible = "qcom,msm-iommu-v1-ns";
+				reg = <0x1000 0x1000>;
+				interrupts = <GIC_SPI 241 0>;

s/0/IRQ_TYPE_LEVEL_HIGH/

arch/arm64/boot/dts/qcom/msm8916.dtsi line 757

Quoted 7 lines
arch/arm64/boot/dts/qcom/msm8916.dtsi +7 -0
+			};
+
+			// gfx3d_priv:
+			iommu-ctx@2000 {
+				compatible = "qcom,msm-iommu-v1-ns";
+				reg = <0x2000 0x1000>;
+				interrupts = <GIC_SPI 242 0>;

s/0/IRQ_TYPE_LEVEL_HIGH/

Quoted 6 lines
arch/arm64/boot/dts/qcom/msm8916.dtsi +3 -0
+			};
+		};
+
 		gpu_opp_table: opp_table {
 			compatible = "operating-points-v2";
Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
author
#6 lore raw

Re: [PATCH 3/3] ARM64: DT: add iommu for msm8916

Quoted 19 lines · Stephen Boyd
Quoted 7 lines · Rob Clark
arch/arm64/boot/dts/qcom/msm8916.dtsi +7 -0
+             apps_iommu: iommu@1ef0000 {
+                     #address-cells = <1>;
+                     #size-cells = <1>;
+                     #iommu-cells = <1>;
+                     compatible = "qcom,msm8916-iommu", "qcom,msm-iommu-v1";
+                     ranges = <0 0x1e20000 0x40000>;
+                     reg = <0x1ef0000 0x3000>;

arch/arm64/boot/dts/qcom/msm8916.dtsi line 724

Quoted 10 lines
arch/arm64/boot/dts/qcom/msm8916.dtsi +10 -0
+                     clocks = <&gcc GCC_SMMU_CFG_CLK>,
+                              <&gcc GCC_APSS_TCU_CLK>;
+                     clock-names = "iface", "bus";
+                     qcom,iommu-secure-id = <17>;
+
+                     // mdp_0:
+                     iommu-ctx@4000 {
+                             compatible = "qcom,msm-iommu-v1-ns";
+                             reg = <0x4000 0x1000>;
+                             interrupts = <GIC_SPI 70 0>;
Quoted 1 line

s/0/IRQ_TYPE_LEVEL_HIGH/

0 actually seems to be _NONE.. so using _HIGH would change how irq is configured (according to gic_configure_irq()). Do you expect that to work? I'm probably going based on what was in downstream dt, and some of that is more a pain to retest so I'd rather not change the type unless at least one of us knows what they are doing.

Quoted 11 lines

arch/arm64/boot/dts/qcom/msm8916.dtsi line 731

Quoted 7 lines
arch/arm64/boot/dts/qcom/msm8916.dtsi +7 -0
+                     };
+
+                     // venus_ns:
+                     iommu-ctx@5000 {
+                             compatible = "qcom,msm-iommu-v1-sec";
+                             reg = <0x5000 0x1000>;
+                             interrupts = <GIC_SPI 70 0>;

s/0/IRQ_TYPE_LEVEL_HIGH/

Is it the same interrupt number (70) twice? Not 71 or something?

According to downstream. Not *entirely* sure how that is supposed to work as far as dispatching fault callback to the right driver, but at least unlike with the gpu, if we get a fault for mdp, that is entirely the kernels fault. So meh?

If you know better, let me know.

BR,
-R

Quoted 43 lines
Quoted 10 lines
arch/arm64/boot/dts/qcom/msm8916.dtsi +9 -0
+                     };
+             };
+
+             gpu_iommu: iommu@1f08000 {
+                     #address-cells = <1>;
+                     #size-cells = <1>;
+                     #iommu-cells = <1>;
+                     compatible = "qcom,msm8916-iommu", "qcom,msm-iommu-v1";
+                     ranges = <0 0x1f08000 0x10000>;

arch/arm64/boot/dts/qcom/msm8916.dtsi line 750

Quoted 10 lines
arch/arm64/boot/dts/qcom/msm8916.dtsi +10 -0
+                     clocks = <&gcc GCC_SMMU_CFG_CLK>,
+                              <&gcc GCC_GFX_TCU_CLK>;
+                     clock-names = "iface", "bus";
+                     qcom,iommu-secure-id = <18>;
+
+                     // gfx3d_user:
+                     iommu-ctx@1000 {
+                             compatible = "qcom,msm-iommu-v1-ns";
+                             reg = <0x1000 0x1000>;
+                             interrupts = <GIC_SPI 241 0>;

s/0/IRQ_TYPE_LEVEL_HIGH/

arch/arm64/boot/dts/qcom/msm8916.dtsi line 757

Quoted 7 lines
arch/arm64/boot/dts/qcom/msm8916.dtsi +7 -0
+                     };
+
+                     // gfx3d_priv:
+                     iommu-ctx@2000 {
+                             compatible = "qcom,msm-iommu-v1-ns";
+                             reg = <0x2000 0x1000>;
+                             interrupts = <GIC_SPI 242 0>;

s/0/IRQ_TYPE_LEVEL_HIGH/

arch/arm64/boot/dts/qcom/msm8916.dtsi line 762

Quoted 6 lines
arch/arm64/boot/dts/qcom/msm8916.dtsi +3 -0
+                     };
+             };
+
              gpu_opp_table: opp_table {
                      compatible = "operating-points-v2";

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project

#7 lore raw

Re: [PATCH 3/3] ARM64: DT: add iommu for msm8916

Quoted 20 lines · Rob Clark
Quoted 19 lines · Stephen Boyd
Quoted 17 lines · Rob Clark
arch/arm64/boot/dts/qcom/msm8916.dtsi +17 -0
+             apps_iommu: iommu@1ef0000 {
+                     #address-cells = <1>;
+                     #size-cells = <1>;
+                     #iommu-cells = <1>;
+                     compatible = "qcom,msm8916-iommu", "qcom,msm-iommu-v1";
+                     ranges = <0 0x1e20000 0x40000>;
+                     reg = <0x1ef0000 0x3000>;
+                     clocks = <&gcc GCC_SMMU_CFG_CLK>,
+                              <&gcc GCC_APSS_TCU_CLK>;
+                     clock-names = "iface", "bus";
+                     qcom,iommu-secure-id = <17>;
+
+                     // mdp_0:
+                     iommu-ctx@4000 {
+                             compatible = "qcom,msm-iommu-v1-ns";
+                             reg = <0x4000 0x1000>;
+                             interrupts = <GIC_SPI 70 0>;
Quoted 7 lines
Quoted 1 line

s/0/IRQ_TYPE_LEVEL_HIGH/

0 actually seems to be _NONE.. so using _HIGH would change how irq is configured (according to gic_configure_irq()). Do you expect that to work? I'm probably going based on what was in downstream dt, and some of that is more a pain to retest so I'd rather not change the type unless at least one of us knows what they are doing.

I believe the default setting for all these interrupts are IRQ_TYPE_LEVEL_HIGH already, so there shouldn't be any change to behavior and things should still work. Would be worth double checking what the GIC has it configured as with IRQ_TYPE_NONE here by looking at /proc/interrupts. I suspect it's level high.

Quoted 11 lines
Quoted 11 lines

arch/arm64/boot/dts/qcom/msm8916.dtsi line 731

Quoted 8 lines
arch/arm64/boot/dts/qcom/msm8916.dtsi +7 -0
+                     };
+
+                     // venus_ns:
+                     iommu-ctx@5000 {
+                             compatible = "qcom,msm-iommu-v1-sec";
+                             reg = <0x5000 0x1000>;
+                             interrupts = <GIC_SPI 70 0>;

s/0/IRQ_TYPE_LEVEL_HIGH/

Quoted 9 lines
Quoted 1 line

Is it the same interrupt number (70) twice? Not 71 or something?

According to downstream. Not *entirely* sure how that is supposed to work as far as dispatching fault callback to the right driver, but at least unlike with the gpu, if we get a fault for mdp, that is entirely the kernels fault. So meh?

If you know better, let me know.

Ok, I see what's going on. This design still seems odd to me. We have an interrupt per-context bank in DT so that we can map the top-level aggregated context bank interrupt (70) to each context bank that has their interrupt routed there. Depending on the bootloader configuration we could have many context banks route their fault interrupt to the same interrupt at the GIC or they could go to different top-level interrupts. In the GFX IOMMU case, hw folks decided to _not_ aggregate context bank interrupts at all so each context bank has a dedicated interrupt at the GIC.

Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project