[PATCH 0/6] drm/msm: New features for 4.12

[PATCH 0/6] drm/msm: New features for 4.12

[Jordan Crouse]

Hey Rob - here are a handful of new features and more extensive bug fixes that
might be suitable for 4.12.

Of note is the reference count for address spaces which is a pre-requisite for
per-instance pagetables and the move to OPP tables which is a stepping stone for
all sorts of clock related shenanigans.

Thanks!
Jordan

Jordan Crouse (6):
  drm/msm: Don't allow zero sized buffer objects
  drm/msm: Reference count address spaces
  drm/msm: Add MSM_PARAM_GMEM_BASE
  drm/msm: Hard code the GPU "slow frequency"
  drm/msm: gpu: Use OPP tables if we can
  msm/drm: gpu: Dynamically locate the clocks from the device tree

 drivers/gpu/drm/msm/adreno/adreno_device.c | 90 ++++++++++++++++++++++--------
 drivers/gpu/drm/msm/adreno/adreno_gpu.c    | 10 ++--
 drivers/gpu/drm/msm/adreno/adreno_gpu.h    |  2 +-
 drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c    |  2 +-
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c    |  2 +-
 drivers/gpu/drm/msm/msm_drv.h              |  3 +-
 drivers/gpu/drm/msm/msm_gem.c              |  6 ++
 drivers/gpu/drm/msm/msm_gem.h              |  2 +
 drivers/gpu/drm/msm/msm_gem_vma.c          | 35 +++++++++---
 drivers/gpu/drm/msm/msm_gpu.c              | 82 +++++++++++++++++++--------
 drivers/gpu/drm/msm/msm_gpu.h              |  6 +-
 include/uapi/drm/msm_drm.h                 |  1 +
 12 files changed, 176 insertions(+), 65 deletions(-)

-- 
1.9.1

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[PATCH 1/6] drm/msm: Don't allow zero sized buffer objects

[Jordan Crouse]

Zero sized buffer objects tend to make various bits of the GEM
infrastructure complain:

 WARNING: CPU: 1 PID: 2323 at drivers/gpu/drm/drm_mm.c:389 drm_mm_insert_node_generic+0x258/0x2f0
 Modules linked in:

 CPU: 1 PID: 2323 Comm: drm-api-test Tainted: G        W 4.9.0-rc4-00906-g693af44 #213
 Hardware name: Qualcomm Technologies, Inc. DB820c (DT)
 task: ffff8000d7353400 task.stack: ffff8000d7720000
 PC is at drm_mm_insert_node_generic+0x258/0x2f0
 LR is at drm_vma_offset_add+0x4c/0x70

Zero sized buffers serve no appreciable value to the user so disallow
them at create time.

Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
---
 drivers/gpu/drm/msm/msm_gem.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
index 4c3e6ef..e819b83 100644
--- a/drivers/gpu/drm/msm/msm_gem.c
+++ b/drivers/gpu/drm/msm/msm_gem.c
@@ -874,6 +874,12 @@ struct drm_gem_object *msm_gem_new(struct drm_device *dev,
 
 	size = PAGE_ALIGN(size);
 
+	/* Disallow zero sized objects as they make the underlying
+	 * infrastructure grumpy
+	 */
+	if (size == 0)
+		return ERR_PTR(-EINVAL);
+
 	ret = msm_gem_new_impl(dev, size, flags, NULL, &obj);
 	if (ret)
 		goto fail;
-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/6] drm/msm: Reference count address spaces

[Jordan Crouse]

There are reasons for a memory object to outlive the file descriptor
that created it and so the address space that a buffer object is
attached to must also outlive the file descriptor. Reference count
the address space so that it can remain viable until all the objects
have released their addresses.

Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
---
 drivers/gpu/drm/msm/adreno/adreno_gpu.c |  2 +-
 drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c |  2 +-
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c |  2 +-
 drivers/gpu/drm/msm/msm_drv.h           |  3 ++-
 drivers/gpu/drm/msm/msm_gem.h           |  2 ++
 drivers/gpu/drm/msm/msm_gem_vma.c       | 35 ++++++++++++++++++++++++---------
 6 files changed, 33 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index 2debc76..18a4f74 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -498,6 +498,6 @@ void adreno_gpu_cleanup(struct adreno_gpu *adreno_gpu)
 	if (gpu->aspace) {
 		gpu->aspace->mmu->funcs->detach(gpu->aspace->mmu,
 			iommu_ports, ARRAY_SIZE(iommu_ports));
-		msm_gem_address_space_destroy(gpu->aspace);
+		msm_gem_address_space_put(gpu->aspace);
 	}
 }
diff --git a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c
index 239a202..ea1dab7 100644
--- a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c
+++ b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c
@@ -168,7 +168,7 @@ static void mdp4_destroy(struct msm_kms *kms)
 	if (aspace) {
 		aspace->mmu->funcs->detach(aspace->mmu,
 				iommu_ports, ARRAY_SIZE(iommu_ports));
-		msm_gem_address_space_destroy(aspace);
+		msm_gem_address_space_put(aspace);
 	}
 
 	drm_gem_object_unreference_unlocked(mdp4_kms->blank_cursor_bo);
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
index 889dd5d..fa4096a 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
@@ -168,7 +168,7 @@ static void mdp5_kms_destroy(struct msm_kms *kms)
 	if (aspace) {
 		aspace->mmu->funcs->detach(aspace->mmu,
 				iommu_ports, ARRAY_SIZE(iommu_ports));
-		msm_gem_address_space_destroy(aspace);
+		msm_gem_address_space_put(aspace);
 	}
 
 	/*
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index 08ad9e4..e740fa5 100644
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -190,7 +190,8 @@ void msm_gem_unmap_vma(struct msm_gem_address_space *aspace,
 int msm_gem_map_vma(struct msm_gem_address_space *aspace,
 		struct msm_gem_vma *vma, struct sg_table *sgt, int npages);
 
-void msm_gem_address_space_destroy(struct msm_gem_address_space *aspace);
+void msm_gem_address_space_put(struct msm_gem_address_space *aspace);
+
 struct msm_gem_address_space *
 msm_gem_address_space_create(struct device *dev, struct iommu_domain *domain,
 		const char *name);
diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h
index 633b5c1..c57b54e 100644
--- a/drivers/gpu/drm/msm/msm_gem.h
+++ b/drivers/gpu/drm/msm/msm_gem.h
@@ -18,6 +18,7 @@
 #ifndef __MSM_GEM_H__
 #define __MSM_GEM_H__
 
+#include <linux/kref.h>
 #include <linux/reservation.h>
 #include "msm_drv.h"
 
@@ -31,6 +32,7 @@ struct msm_gem_address_space {
 	 */
 	struct drm_mm mm;
 	struct msm_mmu *mmu;
+	struct kref kref;
 };
 
 struct msm_gem_vma {
diff --git a/drivers/gpu/drm/msm/msm_gem_vma.c b/drivers/gpu/drm/msm/msm_gem_vma.c
index a311d26..2b1d2cb 100644
--- a/drivers/gpu/drm/msm/msm_gem_vma.c
+++ b/drivers/gpu/drm/msm/msm_gem_vma.c
@@ -19,6 +19,25 @@
 #include "msm_gem.h"
 #include "msm_mmu.h"
 
+static void
+msm_gem_address_space_destroy(struct kref *kref)
+{
+	struct msm_gem_address_space *aspace = container_of(kref,
+			struct msm_gem_address_space, kref);
+
+	drm_mm_takedown(&aspace->mm);
+	if (aspace->mmu)
+		aspace->mmu->funcs->destroy(aspace->mmu);
+	kfree(aspace);
+}
+
+
+void msm_gem_address_space_put(struct msm_gem_address_space *aspace)
+{
+	if (aspace)
+		kref_put(&aspace->kref, msm_gem_address_space_destroy);
+}
+
 void
 msm_gem_unmap_vma(struct msm_gem_address_space *aspace,
 		struct msm_gem_vma *vma, struct sg_table *sgt)
@@ -34,6 +53,8 @@
 	drm_mm_remove_node(&vma->node);
 
 	vma->iova = 0;
+
+	msm_gem_address_space_put(aspace);
 }
 
 int
@@ -58,16 +79,10 @@
 				size, IOMMU_READ | IOMMU_WRITE);
 	}
 
-	return ret;
-}
+	/* Get a reference to the aspace to keep it around */
+	kref_get(&aspace->kref);
 
-void
-msm_gem_address_space_destroy(struct msm_gem_address_space *aspace)
-{
-	drm_mm_takedown(&aspace->mm);
-	if (aspace->mmu)
-		aspace->mmu->funcs->destroy(aspace->mmu);
-	kfree(aspace);
+	return ret;
 }
 
 struct msm_gem_address_space *
@@ -86,5 +101,7 @@ struct msm_gem_address_space *
 	drm_mm_init(&aspace->mm, (domain->geometry.aperture_start >> PAGE_SHIFT),
 			(domain->geometry.aperture_end >> PAGE_SHIFT) - 1);
 
+	kref_init(&aspace->kref);
+
 	return aspace;
 }
-- 
1.9.1

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[PATCH 3/6] drm/msm: Add MSM_PARAM_GMEM_BASE

[Jordan Crouse]

User space needs to know where the GMEM whole starts so that they
can set up the addressing correctly.

Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
---
 drivers/gpu/drm/msm/adreno/adreno_gpu.c | 3 +++
 include/uapi/drm/msm_drm.h              | 1 +
 2 files changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index 18a4f74..dcbf3ea 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -33,6 +33,9 @@ int adreno_get_param(struct msm_gpu *gpu, uint32_t param, uint64_t *value)
 	case MSM_PARAM_GMEM_SIZE:
 		*value = adreno_gpu->gmem;
 		return 0;
+	case MSM_PARAM_GMEM_BASE:
+		*value = 0x100000;
+		return 0;
 	case MSM_PARAM_CHIP_ID:
 		*value = adreno_gpu->rev.patchid |
 				(adreno_gpu->rev.minor << 8) |
diff --git a/include/uapi/drm/msm_drm.h b/include/uapi/drm/msm_drm.h
index 915634b..c8244f4 100644
--- a/include/uapi/drm/msm_drm.h
+++ b/include/uapi/drm/msm_drm.h
@@ -72,6 +72,7 @@ struct drm_msm_timespec {
 #define MSM_PARAM_CHIP_ID    0x03
 #define MSM_PARAM_MAX_FREQ   0x04
 #define MSM_PARAM_TIMESTAMP  0x05
+#define MSM_PARAM_GMEM_BASE  0x06
 
 struct drm_msm_param {
 	__u32 pipe;           /* in, MSM_PIPE_x */
-- 
1.9.1

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[PATCH 4/6] drm/msm: Hard code the GPU "slow frequency"

[Jordan Crouse]

Some A3XX and A4XX GPU targets required that the GPU clock be
programmed to a non zero value when it was disabled so
27Mhz was chosen as the "invalid" frequency.

Even though newer targets do not have the same clock restrictions
we still write 27Mhz on clock disable and expect the clock subsystem
to round down to zero.

For unknown reasons even though the slow clock speed is always
27Mhz and it isn't actually a functional level the legacy device tree
frequency tables always defined it and then did gymnastics to work
around it.

Instead of playing the same silly games just hard code the "slow" clock
speed in the code as 27MHz and save ourselves a bit of infrastructure.

Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
---
 drivers/gpu/drm/msm/adreno/adreno_device.c | 5 +----
 drivers/gpu/drm/msm/adreno/adreno_gpu.c    | 5 ++---
 drivers/gpu/drm/msm/adreno/adreno_gpu.h    | 2 +-
 drivers/gpu/drm/msm/msm_gpu.c              | 8 ++++++--
 drivers/gpu/drm/msm/msm_gpu.h              | 2 +-
 5 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c
index 6e060e7..8374e9a 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_device.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
@@ -2,7 +2,7 @@
  * Copyright (C) 2013-2014 Red Hat
  * Author: Rob Clark <robdclark@gmail.com>
  *
- * Copyright (c) 2014 The Linux Foundation. All rights reserved.
+ * Copyright (c) 2014,2017 The Linux Foundation. All rights reserved.
  *
  * This program is free software; you can redistribute it and/or modify it
  * under the terms of the GNU General Public License version 2 as published by
@@ -238,7 +238,6 @@ static int adreno_bind(struct device *dev, struct device *master, void *data)
 
 	/* find clock rates: */
 	config.fast_rate = 0;
-	config.slow_rate = ~0;
 	for_each_child_of_node(node, child) {
 		if (of_device_is_compatible(child, "qcom,gpu-pwrlevels")) {
 			struct device_node *pwrlvl;
@@ -249,7 +248,6 @@ static int adreno_bind(struct device *dev, struct device *master, void *data)
 					return ret;
 				}
 				config.fast_rate = max(config.fast_rate, val);
-				config.slow_rate = min(config.slow_rate, val);
 			}
 		}
 	}
@@ -258,7 +256,6 @@ static int adreno_bind(struct device *dev, struct device *master, void *data)
 		dev_warn(dev, "could not find clk rates\n");
 		/* This is a safe low speed for all devices: */
 		config.fast_rate = 200000000;
-		config.slow_rate = 27000000;
 	}
 
 	dev->platform_data = &config;
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index dcbf3ea..b41bd88 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -407,14 +407,13 @@ int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev,
 	adreno_gpu->rev = config->rev;
 
 	gpu->fast_rate = config->fast_rate;
-	gpu->slow_rate = config->slow_rate;
 	gpu->bus_freq  = config->bus_freq;
 #ifdef DOWNSTREAM_CONFIG_MSM_BUS_SCALING
 	gpu->bus_scale_table = config->bus_scale_table;
 #endif
 
-	DBG("fast_rate=%u, slow_rate=%u, bus_freq=%u",
-			gpu->fast_rate, gpu->slow_rate, gpu->bus_freq);
+	DBG("fast_rate=%u, slow_rate=27000000, bus_freq=%u",
+			gpu->fast_rate, gpu->bus_freq);
 
 	adreno_gpu_config.ioname = "kgsl_3d0_reg_memory";
 	adreno_gpu_config.irqname = "kgsl_3d0_irq";
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
index 4a3630f..841ec30 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
@@ -128,7 +128,7 @@ struct adreno_gpu {
 /* platform config data (ie. from DT, or pdata) */
 struct adreno_platform_config {
 	struct adreno_rev rev;
-	uint32_t fast_rate, slow_rate, bus_freq;
+	uint32_t fast_rate, bus_freq;
 #ifdef DOWNSTREAM_CONFIG_MSM_BUS_SCALING
 	struct msm_bus_scale_pdata *bus_scale_table;
 #endif
diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
index c7969f5..2b731e7 100644
--- a/drivers/gpu/drm/msm/msm_gpu.c
+++ b/drivers/gpu/drm/msm/msm_gpu.c
@@ -123,8 +123,12 @@ static int disable_clk(struct msm_gpu *gpu)
 		if (gpu->grp_clks[i])
 			clk_unprepare(gpu->grp_clks[i]);
 
-	if (gpu->grp_clks[0] && gpu->slow_rate)
-		clk_set_rate(gpu->grp_clks[0], gpu->slow_rate);
+	/*
+	 * Set the clock to a deliberately low rate. On older targets the clock
+	 * speed had to be non zero to avoid problems. On newer targets this
+	 * will be rounded down to zero anyway so it all works out.
+	 */
+	clk_set_rate(gpu->grp_clks[0], 27000000);
 
 	if (gpu->grp_clks[2])
 		clk_set_rate(gpu->grp_clks[2], 0);
diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
index 50fef27..bc6f163 100644
--- a/drivers/gpu/drm/msm/msm_gpu.h
+++ b/drivers/gpu/drm/msm/msm_gpu.h
@@ -111,7 +111,7 @@ struct msm_gpu {
 	/* Power Control: */
 	struct regulator *gpu_reg, *gpu_cx;
 	struct clk *ebi1_clk, *grp_clks[6];
-	uint32_t fast_rate, slow_rate, bus_freq;
+	uint32_t fast_rate, bus_freq;
 
 #ifdef DOWNSTREAM_CONFIG_MSM_BUS_SCALING
 	struct msm_bus_scale_pdata *bus_scale_table;
-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 5/6] drm/msm: gpu: Use OPP tables if we can

[Jordan Crouse]

If a OPP table is defined for the GPU device in the device tree use
that in lieu of the downstream style GPU frequency table. If we do
use the downstream table convert it to a OPP table so that we can
take advantage of the OPP lookup facilities later.

Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
---
 drivers/gpu/drm/msm/adreno/adreno_device.c | 85 +++++++++++++++++++++++-------
 1 file changed, 66 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c
index 8374e9a..24da7f6 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_device.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
@@ -17,6 +17,7 @@
  * this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include <linux/pm_opp.h>
 #include "adreno_gpu.h"
 
 #define ANY_ID 0xff
@@ -220,10 +221,71 @@ static int find_chipid(struct device *dev, u32 *chipid)
 	return 0;
 }
 
+/* Get legacy powerlevels from qcom,gpu-pwrlevels and populate the opp table */
+static int adreno_get_legacy_pwrlevels(struct device *dev)
+{
+	struct device_node *child, *node;
+	int ret;
+
+	node = of_find_compatible_node(dev->of_node, NULL,
+		"qcom,gpu-pwrlevels");
+	if (!node) {
+		dev_err(dev, "Could not find the GPU powerlevels\n");
+		return -ENXIO;
+	}
+
+	for_each_child_of_node(node, child) {
+		unsigned int val;
+
+		ret = of_property_read_u32(child, "qcom,gpu-freq", &val);
+		if (ret)
+			continue;
+
+		/*
+		 * Skip the intentionally bogus clock value found at the bottom
+		 * of most legacy frequency tables
+		 */
+		if (val != 27000000)
+			dev_pm_opp_add(dev, val, 0);
+	}
+
+	return 0;
+}
+
+static int adreno_get_pwrlevels(struct device *dev,
+		struct adreno_platform_config *config)
+{
+	unsigned long freq = ULONG_MAX;
+	struct dev_pm_opp *opp;
+	int ret;
+
+	/* You down with OPP? */
+	if (!of_find_property(dev->of_node, "operating-points-v2", NULL))
+		ret = adreno_get_legacy_pwrlevels(dev);
+	else
+		ret = dev_pm_opp_of_add_table(dev);
+
+	if (ret)
+		return ret;
+
+	/* Find the fastest defined rate */
+	opp = dev_pm_opp_find_freq_floor(dev, &freq);
+	if (!IS_ERR(opp))
+		config->fast_rate = dev_pm_opp_get_freq(opp);
+
+	if (!config->fast_rate) {
+		DRM_DEV_INFO(dev,
+			"Could not find clock rate. Using default\n");
+		/* Pick a suitably safe clock speed for any target */
+		config->fast_rate = 200000000;
+	}
+
+	return 0;
+}
+
 static int adreno_bind(struct device *dev, struct device *master, void *data)
 {
 	static struct adreno_platform_config config = {};
-	struct device_node *child, *node = dev->of_node;
 	u32 val;
 	int ret;
 
@@ -238,25 +300,10 @@ static int adreno_bind(struct device *dev, struct device *master, void *data)
 
 	/* find clock rates: */
 	config.fast_rate = 0;
-	for_each_child_of_node(node, child) {
-		if (of_device_is_compatible(child, "qcom,gpu-pwrlevels")) {
-			struct device_node *pwrlvl;
-			for_each_child_of_node(child, pwrlvl) {
-				ret = of_property_read_u32(pwrlvl, "qcom,gpu-freq", &val);
-				if (ret) {
-					dev_err(dev, "could not find gpu-freq: %d\n", ret);
-					return ret;
-				}
-				config.fast_rate = max(config.fast_rate, val);
-			}
-		}
-	}
 
-	if (!config.fast_rate) {
-		dev_warn(dev, "could not find clk rates\n");
-		/* This is a safe low speed for all devices: */
-		config.fast_rate = 200000000;
-	}
+	ret = adreno_get_pwrlevels(dev, &config);
+	if (ret)
+		return ret;
 
 	dev->platform_data = &config;
 	set_gpu_pdev(dev_get_drvdata(master), to_platform_device(dev));
-- 
1.9.1

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[PATCH 6/6] msm/drm: gpu: Dynamically locate the clocks from the device tree

[Jordan Crouse]

Instead of using a fixed list of clock names use the clock-names
list in the device tree to discover and get the list of clocks
that we need.

Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
---
 drivers/gpu/drm/msm/msm_gpu.c | 76 ++++++++++++++++++++++++++++++-------------
 drivers/gpu/drm/msm/msm_gpu.h |  4 ++-
 2 files changed, 57 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
index 2b731e7..25de46f 100644
--- a/drivers/gpu/drm/msm/msm_gpu.c
+++ b/drivers/gpu/drm/msm/msm_gpu.c
@@ -93,18 +93,18 @@ static int enable_clk(struct msm_gpu *gpu)
 {
 	int i;
 
-	if (gpu->grp_clks[0] && gpu->fast_rate)
-		clk_set_rate(gpu->grp_clks[0], gpu->fast_rate);
+	if (gpu->core_clk && gpu->fast_rate)
+		clk_set_rate(gpu->core_clk, gpu->fast_rate);
 
 	/* Set the RBBM timer rate to 19.2Mhz */
-	if (gpu->grp_clks[2])
-		clk_set_rate(gpu->grp_clks[2], 19200000);
+	if (gpu->rbbmtimer_clk)
+		clk_set_rate(gpu->rbbmtimer_clk, 19200000);
 
-	for (i = ARRAY_SIZE(gpu->grp_clks) - 1; i >= 0; i--)
+	for (i = gpu->nr_clocks - 1; i >= 0; i--)
 		if (gpu->grp_clks[i])
 			clk_prepare(gpu->grp_clks[i]);
 
-	for (i = ARRAY_SIZE(gpu->grp_clks) - 1; i >= 0; i--)
+	for (i = gpu->nr_clocks - 1; i >= 0; i--)
 		if (gpu->grp_clks[i])
 			clk_enable(gpu->grp_clks[i]);
 
@@ -115,11 +115,11 @@ static int disable_clk(struct msm_gpu *gpu)
 {
 	int i;
 
-	for (i = ARRAY_SIZE(gpu->grp_clks) - 1; i >= 0; i--)
+	for (i = gpu->nr_clocks - 1; i >= 0; i--)
 		if (gpu->grp_clks[i])
 			clk_disable(gpu->grp_clks[i]);
 
-	for (i = ARRAY_SIZE(gpu->grp_clks) - 1; i >= 0; i--)
+	for (i = gpu->nr_clocks - 1; i >= 0; i--)
 		if (gpu->grp_clks[i])
 			clk_unprepare(gpu->grp_clks[i]);
 
@@ -128,10 +128,11 @@ static int disable_clk(struct msm_gpu *gpu)
 	 * speed had to be non zero to avoid problems. On newer targets this
 	 * will be rounded down to zero anyway so it all works out.
 	 */
-	clk_set_rate(gpu->grp_clks[0], 27000000);
+	if (gpu->core_clk)
+		clk_set_rate(gpu->core_clk, 27000000);
 
-	if (gpu->grp_clks[2])
-		clk_set_rate(gpu->grp_clks[2], 0);
+	if (gpu->rbbmtimer_clk)
+		clk_set_rate(gpu->rbbmtimer_clk, 0);
 
 	return 0;
 }
@@ -586,9 +587,45 @@ static irqreturn_t irq_handler(int irq, void *data)
 	return gpu->funcs->irq(gpu);
 }
 
-static const char *clk_names[] = {
-	"core", "iface", "rbbmtimer", "mem", "mem_iface", "alt_mem_iface",
-};
+static struct clk *get_clock(struct device *dev, const char *name)
+{
+	struct clk *clk = devm_clk_get(dev, name);
+
+	return IS_ERR(clk) ? NULL : clk;
+}
+
+static int get_clocks(struct platform_device *pdev, struct msm_gpu *gpu)
+{
+	struct device *dev = &pdev->dev;
+	struct property *prop;
+	const char *name;
+	int i = 0;
+
+	gpu->nr_clocks = of_property_count_strings(dev->of_node, "clock-names");
+	if (gpu->nr_clocks < 1) {
+		gpu->nr_clocks = 0;
+		return 0;
+	}
+
+	gpu->grp_clks = devm_kcalloc(dev, sizeof(struct clk *), gpu->nr_clocks,
+		GFP_KERNEL);
+	if (!gpu->grp_clks)
+		return -ENOMEM;
+
+	of_property_for_each_string(dev->of_node, "clock-names", prop, name) {
+		gpu->grp_clks[i] = get_clock(dev, name);
+
+		/* Remember the key clocks that we need to control later */
+		if (!strcmp(name, "core"))
+			gpu->core_clk = gpu->grp_clks[i];
+		else if (!strcmp(name, "rbbmtimer"))
+			gpu->rbbmtimer_clk = gpu->grp_clks[i];
+
+		++i;
+	}
+
+	return 0;
+}
 
 int msm_gpu_init(struct drm_device *drm, struct platform_device *pdev,
 		struct msm_gpu *gpu, const struct msm_gpu_funcs *funcs,
@@ -625,7 +662,6 @@ int msm_gpu_init(struct drm_device *drm, struct platform_device *pdev,
 
 	spin_lock_init(&gpu->perf_lock);
 
-	BUG_ON(ARRAY_SIZE(clk_names) != ARRAY_SIZE(gpu->grp_clks));
 
 	/* Map registers: */
 	gpu->mmio = msm_ioremap(pdev, config->ioname, name);
@@ -649,13 +685,9 @@ int msm_gpu_init(struct drm_device *drm, struct platform_device *pdev,
 		goto fail;
 	}
 
-	/* Acquire clocks: */
-	for (i = 0; i < ARRAY_SIZE(clk_names); i++) {
-		gpu->grp_clks[i] = msm_clk_get(pdev, clk_names[i]);
-		DBG("grp_clks[%s]: %p", clk_names[i], gpu->grp_clks[i]);
-		if (IS_ERR(gpu->grp_clks[i]))
-			gpu->grp_clks[i] = NULL;
-	}
+	ret = get_clocks(pdev, gpu);
+	if (ret)
+		goto fail;
 
 	gpu->ebi1_clk = msm_clk_get(pdev, "bus");
 	DBG("ebi1_clk: %p", gpu->ebi1_clk);
diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
index bc6f163..4162028 100644
--- a/drivers/gpu/drm/msm/msm_gpu.h
+++ b/drivers/gpu/drm/msm/msm_gpu.h
@@ -110,7 +110,9 @@ struct msm_gpu {
 
 	/* Power Control: */
 	struct regulator *gpu_reg, *gpu_cx;
-	struct clk *ebi1_clk, *grp_clks[6];
+	struct clk **grp_clks;
+	int nr_clocks;
+	struct clk *ebi1_clk, *core_clk, *rbbmtimer_clk;
 	uint32_t fast_rate, bus_freq;
 
 #ifdef DOWNSTREAM_CONFIG_MSM_BUS_SCALING
-- 
1.9.1

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH 5/6] drm/msm: gpu: Use OPP tables if we can

[Rob Clark]

On Tue, Mar 7, 2017 at 12:02 PM, Jordan Crouse <jcrouse@codeaurora.org> wrote:
> If a OPP table is defined for the GPU device in the device tree use
> that in lieu of the downstream style GPU frequency table. If we do
> use the downstream table convert it to a OPP table so that we can
> take advantage of the OPP lookup facilities later.

so this one should probably be accompanied by an update to
Documentation/devicetree/bindings/display/msm/gpu.txt (and cc'd to
devicetree list, although I think it should not be controversial to
use the proper bindings for this ;-))

otherwise, looks good.. 1/6 looks like fixes material so I'll pull
that one in immediately..

BR,
-R

> Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
> ---
>  drivers/gpu/drm/msm/adreno/adreno_device.c | 85 +++++++++++++++++++++++-------
>  1 file changed, 66 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c
> index 8374e9a..24da7f6 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_device.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
> @@ -17,6 +17,7 @@
>   * this program.  If not, see <http://www.gnu.org/licenses/>.
>   */
>
> +#include <linux/pm_opp.h>
>  #include "adreno_gpu.h"
>
>  #define ANY_ID 0xff
> @@ -220,10 +221,71 @@ static int find_chipid(struct device *dev, u32 *chipid)
>         return 0;
>  }
>
> +/* Get legacy powerlevels from qcom,gpu-pwrlevels and populate the opp table */
> +static int adreno_get_legacy_pwrlevels(struct device *dev)
> +{
> +       struct device_node *child, *node;
> +       int ret;
> +
> +       node = of_find_compatible_node(dev->of_node, NULL,
> +               "qcom,gpu-pwrlevels");
> +       if (!node) {
> +               dev_err(dev, "Could not find the GPU powerlevels\n");
> +               return -ENXIO;
> +       }
> +
> +       for_each_child_of_node(node, child) {
> +               unsigned int val;
> +
> +               ret = of_property_read_u32(child, "qcom,gpu-freq", &val);
> +               if (ret)
> +                       continue;
> +
> +               /*
> +                * Skip the intentionally bogus clock value found at the bottom
> +                * of most legacy frequency tables
> +                */
> +               if (val != 27000000)
> +                       dev_pm_opp_add(dev, val, 0);
> +       }
> +
> +       return 0;
> +}
> +
> +static int adreno_get_pwrlevels(struct device *dev,
> +               struct adreno_platform_config *config)
> +{
> +       unsigned long freq = ULONG_MAX;
> +       struct dev_pm_opp *opp;
> +       int ret;
> +
> +       /* You down with OPP? */
> +       if (!of_find_property(dev->of_node, "operating-points-v2", NULL))
> +               ret = adreno_get_legacy_pwrlevels(dev);
> +       else
> +               ret = dev_pm_opp_of_add_table(dev);
> +
> +       if (ret)
> +               return ret;
> +
> +       /* Find the fastest defined rate */
> +       opp = dev_pm_opp_find_freq_floor(dev, &freq);
> +       if (!IS_ERR(opp))
> +               config->fast_rate = dev_pm_opp_get_freq(opp);
> +
> +       if (!config->fast_rate) {
> +               DRM_DEV_INFO(dev,
> +                       "Could not find clock rate. Using default\n");
> +               /* Pick a suitably safe clock speed for any target */
> +               config->fast_rate = 200000000;
> +       }
> +
> +       return 0;
> +}
> +
>  static int adreno_bind(struct device *dev, struct device *master, void *data)
>  {
>         static struct adreno_platform_config config = {};
> -       struct device_node *child, *node = dev->of_node;
>         u32 val;
>         int ret;
>
> @@ -238,25 +300,10 @@ static int adreno_bind(struct device *dev, struct device *master, void *data)
>
>         /* find clock rates: */
>         config.fast_rate = 0;
> -       for_each_child_of_node(node, child) {
> -               if (of_device_is_compatible(child, "qcom,gpu-pwrlevels")) {
> -                       struct device_node *pwrlvl;
> -                       for_each_child_of_node(child, pwrlvl) {
> -                               ret = of_property_read_u32(pwrlvl, "qcom,gpu-freq", &val);
> -                               if (ret) {
> -                                       dev_err(dev, "could not find gpu-freq: %d\n", ret);
> -                                       return ret;
> -                               }
> -                               config.fast_rate = max(config.fast_rate, val);
> -                       }
> -               }
> -       }
>
> -       if (!config.fast_rate) {
> -               dev_warn(dev, "could not find clk rates\n");
> -               /* This is a safe low speed for all devices: */
> -               config.fast_rate = 200000000;
> -       }
> +       ret = adreno_get_pwrlevels(dev, &config);
> +       if (ret)
> +               return ret;
>
>         dev->platform_data = &config;
>         set_gpu_pdev(dev_get_drvdata(master), to_platform_device(dev));
> --
> 1.9.1
>
> _______________________________________________
> Freedreno mailing list
> Freedreno@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/freedreno
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno