[RFC] drm/msm/adreno: clean up gpu bindings

Rob Clark robdclark at gmail.com
Tue Jan 24 17:11:32 UTC 2017


So, cleaning up the GPU bindings is something that has been on my TODO
list for a while, but always $bigger_fires.  Existing bindings are a bit
ugly, but served a purpose when too many of the other drivers the GPU
depends on where still working their way upstream.  But now enough of
that is in place for a few devices, that we should start trying to get
the GPU nodes into the upstream dts files.

This drops the "qcom,chipid" property and parses the GPU revision out of
the compat string.  It does mean you need to list both "qcom,adreno" and
the more specific string, for example "qcom,adreno-530.2".  I'm not sure
if that is "weird" or anyone has better ideas (hence this RFC).

It also drops "qcom,gpu-pwrlevels".  Jordan tells me we can probably
replace that w/ OPP bindings, which seems more sane.  For now, the code
just falls back to a super-slow safe clock until we get the OPP bindings
sorted.

In both cases, the code is still compatible with the old bindings, so
dts files that exist on top of upstream will still work.  But it is
removed from the documentation, and for merging GPU nodes in upstream
dts files, we should use the new bindings.

(I'll ofc split out the .dtsi changes, but figured for an RFC it was
easier to include them in this patch for discussion.)

Signed-off-by: Rob Clark <robdclark at gmail.com>
---
 .../devicetree/bindings/display/msm/gpu.txt        | 24 +++----------
 arch/arm64/boot/dts/qcom/msm8916.dtsi              |  3 +-
 arch/arm64/boot/dts/qcom/msm8996.dtsi              |  8 +----
 drivers/gpu/drm/msm/adreno/adreno_device.c         | 42 ++++++++++++++++++++--
 drivers/gpu/drm/msm/msm_drv.c                      |  1 +
 5 files changed, 47 insertions(+), 31 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/msm/gpu.txt b/Documentation/devicetree/bindings/display/msm/gpu.txt
index 67d0a58..760194d 100644
--- a/Documentation/devicetree/bindings/display/msm/gpu.txt
+++ b/Documentation/devicetree/bindings/display/msm/gpu.txt
@@ -1,7 +1,11 @@
 Qualcomm adreno/snapdragon GPU
 
 Required properties:
-- compatible: "qcom,adreno-3xx"
+- compatible: "qcom,adreno-XYZ.W", "qcom,adreno"
+    for example: "qcom,adreno-306.0", "qcom,adreno"
+  Note that you need to list the less specific "qcom,adreno" (since this
+  is what the device is matched on), in addition to the more specific
+  with the chip-id.
 - reg: Physical base address and length of the controller's registers.
 - interrupts: The interrupt signal from the gpu.
 - clocks: device clocks
@@ -10,14 +14,6 @@ Required properties:
   * "core_clk"
   * "iface_clk"
   * "mem_iface_clk"
-- qcom,chipid: gpu chip-id.  Note this may become optional for future
-  devices if we can reliably read the chipid from hw
-- qcom,gpu-pwrlevels: list of operating points
-  - compatible: "qcom,gpu-pwrlevels"
-  - for each qcom,gpu-pwrlevel:
-    - qcom,gpu-freq: requested gpu clock speed
-    - NOTE: downstream android driver defines additional parameters to
-      configure memory bandwidth scaling per OPP.
 
 Example:
 
@@ -38,15 +34,5 @@ Example:
 		    <&mmcc GFX3D_CLK>,
 		    <&mmcc GFX3D_AHB_CLK>,
 		    <&mmcc MMSS_IMEM_AHB_CLK>;
-		qcom,chipid = <0x03020100>;
-		qcom,gpu-pwrlevels {
-			compatible = "qcom,gpu-pwrlevels";
-			qcom,gpu-pwrlevel at 0 {
-				qcom,gpu-freq = <450000000>;
-			};
-			qcom,gpu-pwrlevel at 1 {
-				qcom,gpu-freq = <27000000>;
-			};
-		};
 	};
 };
diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi
index 681486c..6dcbda6 100644
--- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
@@ -758,7 +758,7 @@
 		};
 
 		adreno-3xx at 01c00000 {
-			compatible = "qcom,adreno-3xx";
+			compatible = "qcom,adreno-306.0", "qcom,adreno";
 			#stream-id-cells = <16>;
 			reg = <0x01c00000 0x20000>;
 			reg-names = "kgsl_3d0_reg_memory";
@@ -779,7 +779,6 @@
 			    <&gcc GCC_BIMC_GPU_CLK>,
 			    <&gcc GFX3D_CLK_SRC>;
 			power-domains = <&gcc OXILI_GDSC>;
-			qcom,chipid = <0x03000600>;
 			qcom,gpu-pwrlevels {
 				compatible = "qcom,gpu-pwrlevels";
 				qcom,gpu-pwrlevel at 0 {
diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
index c5226a7..941493f 100644
--- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
@@ -873,7 +873,7 @@
 		};
 
 		adreno-3xx at b00000 {
-			compatible = "qcom,adreno-3xx";
+			compatible = "qcom,adreno-530.2", "qcom,adreno";
 			#stream-id-cells = <16>;
 
 			reg = <0xb00000 0x3f000>;
@@ -899,12 +899,6 @@
 			power-domains = <&mmcc GPU_GDSC>;
 			iommus = <&adreno_smmu 0>;
 
-			/* There are patchlevel 3 chips in the world (Snapdragon
-			 * (820) but they are functionally similar to the 821 in
-			 * the code so we can safely set the chipset as
-			 * patchlevel 4. */
-			qcom,chipid = <0x05030002>;
-
 			qcom,gpu-quirk-two-pass-use-wfi;
 			qcom,gpu-quirk-fault-detect-mask;
 
diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c
index e130b5e..71300d0 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_device.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
@@ -189,6 +189,40 @@ static const struct {
 	{ "qcom,gpu-quirk-fault-detect-mask", ADRENO_QUIRK_FAULT_DETECT_MASK },
 };
 
+static int find_chipid(struct device_node *node, u32 *chipid)
+{
+	struct property *prop;
+	const char *compat;
+	int ret;
+
+	/* first search the compat strings for qcom,adreno-XYZ.W: */
+	prop = of_find_property(node, "compatible", NULL);
+	for (compat = of_prop_next_string(prop, NULL); compat;
+	     compat = of_prop_next_string(prop, compat)) {
+		unsigned rev, patch;
+
+		if (sscanf(compat, "qcom,adreno-%u.%u", &rev, &patch) != 2)
+			continue;
+
+		*chipid = 0;
+		*chipid |= (rev / 100) << 24;  /* core */
+		rev %= 100;
+		*chipid |= (rev / 10) << 16;   /* major */
+		rev %= 10;
+		*chipid |= rev << 8;           /* minor */
+		*chipid |= patch;
+
+		return 0;
+	}
+
+	/* and if that fails, fall back to legacy "qcom,chipid" property: */
+	ret = of_property_read_u32(node, "qcom,chipid", chipid);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
 static int adreno_bind(struct device *dev, struct device *master, void *data)
 {
 	static struct adreno_platform_config config = {};
@@ -196,7 +230,7 @@ static int adreno_bind(struct device *dev, struct device *master, void *data)
 	u32 val;
 	int ret, i;
 
-	ret = of_property_read_u32(node, "qcom,chipid", &val);
+	ret = find_chipid(node, &val);
 	if (ret) {
 		dev_err(dev, "could not find chipid: %d\n", ret);
 		return ret;
@@ -224,8 +258,9 @@ static int adreno_bind(struct device *dev, struct device *master, void *data)
 	}
 
 	if (!config.fast_rate) {
-		dev_err(dev, "could not find clk rates\n");
-		return -ENXIO;
+		dev_warn(dev, "could not find clk rates\n");
+		/* This is a safe low speed for all devices: */
+		config.fast_rate = config.slow_rate = 27000000;
 	}
 
 	for (i = 0; i < ARRAY_SIZE(quirks); i++)
@@ -263,6 +298,7 @@ static int adreno_remove(struct platform_device *pdev)
 }
 
 static const struct of_device_id dt_match[] = {
+	{ .compatible = "qcom,adreno" },
 	{ .compatible = "qcom,adreno-3xx" },
 	/* for backwards compat w/ downstream kgsl DT files: */
 	{ .compatible = "qcom,kgsl-3d0" },
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index e29bb66..6b85c41 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -985,6 +985,7 @@ static int add_display_components(struct device *dev,
  * as components.
  */
 static const struct of_device_id msm_gpu_match[] = {
+	{ .compatible = "qcom,adreno" },
 	{ .compatible = "qcom,adreno-3xx" },
 	{ .compatible = "qcom,kgsl-3d0" },
 	{ },
-- 
2.9.3



More information about the dri-devel mailing list