[PATCH 0/3] drm/gk20a: support for reclocking

[PATCH 0/3] drm/gk20a: support for reclocking

[Alexandre Courbot]

This series adds support for reclocking on GK20A. The first two patches touch
the clock subsystem to allow GK20A to operate, by making the presence of the
thermal and voltage devices optional, and allowing pstates to be provided
directly instead of being probed using the BIOS (which Tegra does not have).

The last patch adds the GK20A clock device. Arguably the clock can be seen as a
stripped-down version of what is seen on NVE0, however instead of using NVE0
support has been written from scratch using the ChromeOS kernel as a basis.
There are several reasons for this:

- The ChromeOS driver uses a lookup table for the P coefficient which I could
  not find in the NVE0 driver,
- Some registers that NVE0 expects to find are not present on GK20A (e.g.
  0x137120 and 0x137140),
- Calculation of MNP is done differently from what is performed in
  nva3_pll_calc(), and it might be interesting to compare the two methods,
- All the same, the programming sequence is done differently in the ChromeOS
  driver and NVE0 could possibly benefit from it (?)

It would be interesting to try and merge both, but for now I prefer to have the
two coexisting to ensure proper operation on GK20A and besure I don't break
dGPU support. :)

Regarding the first patch, one might argue that I could as well add thermal
and voltage devices to GK20A. The reason this is not done is because these
currently depend heavily on the presence of a BIOS, and will require a rework
similar to that done in patch 2 for clocks. I would like to make sure this
approach is approved because applying it to other subdevs.

Alexandre Courbot (3):
  drm/nouveau/clk: make therm and volt devices optional
  drm/nouveau/clk: support for non-BIOS pstates
  drm/gk20a: reclocking support

 drivers/gpu/drm/nouveau/Makefile                   |   1 +
 drivers/gpu/drm/nouveau/core/engine/device/nve0.c  |   1 +
 .../gpu/drm/nouveau/core/include/subdev/clock.h    |   9 +-
 drivers/gpu/drm/nouveau/core/subdev/clock/base.c   |  52 +-
 drivers/gpu/drm/nouveau/core/subdev/clock/gk20a.c  | 670 +++++++++++++++++++++
 drivers/gpu/drm/nouveau/core/subdev/clock/nv04.c   |   4 +-
 drivers/gpu/drm/nouveau/core/subdev/clock/nv40.c   |   4 +-
 drivers/gpu/drm/nouveau/core/subdev/clock/nv50.c   |   2 +-
 drivers/gpu/drm/nouveau/core/subdev/clock/nva3.c   |   4 +-
 drivers/gpu/drm/nouveau/core/subdev/clock/nvaa.c   |   4 +-
 drivers/gpu/drm/nouveau/core/subdev/clock/nvc0.c   |   4 +-
 drivers/gpu/drm/nouveau/core/subdev/clock/nve0.c   |   4 +-
 12 files changed, 725 insertions(+), 34 deletions(-)
 create mode 100644 drivers/gpu/drm/nouveau/core/subdev/clock/gk20a.c

-- 
2.0.0

[PATCH 1/3] drm/nouveau/clk: make therm and volt devices optional

[Alexandre Courbot]

Allow the clock subsystem to operate even if voltage and thermal devices
are not set for the device (for people with watercooling! ;))

Signed-off-by: Alexandre Courbot <acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/gpu/drm/nouveau/core/subdev/clock/base.c | 36 +++++++++++++++---------
 1 file changed, 22 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/core/subdev/clock/base.c b/drivers/gpu/drm/nouveau/core/subdev/clock/base.c
index 22351f594d2a..63b314a033cb 100644
--- a/drivers/gpu/drm/nouveau/core/subdev/clock/base.c
+++ b/drivers/gpu/drm/nouveau/core/subdev/clock/base.c
@@ -90,16 +90,20 @@ nouveau_cstate_prog(struct nouveau_clock *clk,
 		cstate = &pstate->base;
 	}
 
-	ret = nouveau_therm_cstate(ptherm, pstate->fanspeed, +1);
-	if (ret && ret != -ENODEV) {
-		nv_error(clk, "failed to raise fan speed: %d\n", ret);
-		return ret;
+	if (ptherm) {
+		ret = nouveau_therm_cstate(ptherm, pstate->fanspeed, +1);
+		if (ret && ret != -ENODEV) {
+			nv_error(clk, "failed to raise fan speed: %d\n", ret);
+			return ret;
+		}
 	}
 
-	ret = volt->set_id(volt, cstate->voltage, +1);
-	if (ret && ret != -ENODEV) {
-		nv_error(clk, "failed to raise voltage: %d\n", ret);
-		return ret;
+	if (volt) {
+		ret = volt->set_id(volt, cstate->voltage, +1);
+		if (ret && ret != -ENODEV) {
+			nv_error(clk, "failed to raise voltage: %d\n", ret);
+			return ret;
+		}
 	}
 
 	ret = clk->calc(clk, cstate);
@@ -108,13 +112,17 @@ nouveau_cstate_prog(struct nouveau_clock *clk,
 		clk->tidy(clk);
 	}
 
-	ret = volt->set_id(volt, cstate->voltage, -1);
-	if (ret && ret != -ENODEV)
-		nv_error(clk, "failed to lower voltage: %d\n", ret);
+	if (volt) {
+		ret = volt->set_id(volt, cstate->voltage, -1);
+		if (ret && ret != -ENODEV)
+			nv_error(clk, "failed to lower voltage: %d\n", ret);
+	}
 
-	ret = nouveau_therm_cstate(ptherm, pstate->fanspeed, -1);
-	if (ret && ret != -ENODEV)
-		nv_error(clk, "failed to lower fan speed: %d\n", ret);
+	if (ptherm) {
+		ret = nouveau_therm_cstate(ptherm, pstate->fanspeed, -1);
+		if (ret && ret != -ENODEV)
+			nv_error(clk, "failed to lower fan speed: %d\n", ret);
+	}
 
 	return 0;
 }
-- 
2.0.0

[PATCH 2/3] drm/nouveau/clk: support for non-BIOS pstates

[Alexandre Courbot]

Make nouveau_clock_create() take new two optional arguments: an array
of pstates and its size. When these are specified,
nouveau_clock_create() will use the provided pstates instead of
probing them using the BIOS.

This is useful for platforms which do not provide a BIOS, like Tegra.

Signed-off-by: Alexandre Courbot <acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/gpu/drm/nouveau/core/include/subdev/clock.h |  8 +++++---
 drivers/gpu/drm/nouveau/core/subdev/clock/base.c    | 16 ++++++++++++----
 drivers/gpu/drm/nouveau/core/subdev/clock/nv04.c    |  4 ++--
 drivers/gpu/drm/nouveau/core/subdev/clock/nv40.c    |  4 ++--
 drivers/gpu/drm/nouveau/core/subdev/clock/nv50.c    |  2 +-
 drivers/gpu/drm/nouveau/core/subdev/clock/nva3.c    |  4 ++--
 drivers/gpu/drm/nouveau/core/subdev/clock/nvaa.c    |  4 ++--
 drivers/gpu/drm/nouveau/core/subdev/clock/nvc0.c    |  4 ++--
 drivers/gpu/drm/nouveau/core/subdev/clock/nve0.c    |  4 ++--
 9 files changed, 30 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/core/include/subdev/clock.h b/drivers/gpu/drm/nouveau/core/include/subdev/clock.h
index c01e29c9f89a..c0fe191c9787 100644
--- a/drivers/gpu/drm/nouveau/core/include/subdev/clock.h
+++ b/drivers/gpu/drm/nouveau/core/include/subdev/clock.h
@@ -108,8 +108,9 @@ struct nouveau_clocks {
 	int mdiv;
 };
 
-#define nouveau_clock_create(p,e,o,i,r,d)                                      \
-	nouveau_clock_create_((p), (e), (o), (i), (r), sizeof(**d), (void **)d)
+#define nouveau_clock_create(p,e,o,i,r,s,n,d)                                  \
+	nouveau_clock_create_((p), (e), (o), (i), (r), (s), (n), sizeof(**d),  \
+			      (void **)d)
 #define nouveau_clock_destroy(p) ({                                            \
 	struct nouveau_clock *clk = (p);                                       \
 	_nouveau_clock_dtor(nv_object(clk));                                   \
@@ -123,7 +124,8 @@ struct nouveau_clocks {
 
 int  nouveau_clock_create_(struct nouveau_object *, struct nouveau_object *,
 			   struct nouveau_oclass *,
-			   struct nouveau_clocks *, bool, int, void **);
+			   struct nouveau_clocks *, struct nouveau_pstate *,
+			   int, bool, int, void **);
 void _nouveau_clock_dtor(struct nouveau_object *);
 int _nouveau_clock_init(struct nouveau_object *);
 #define _nouveau_clock_fini _nouveau_subdev_fini
diff --git a/drivers/gpu/drm/nouveau/core/subdev/clock/base.c b/drivers/gpu/drm/nouveau/core/subdev/clock/base.c
index 63b314a033cb..e5798e74d616 100644
--- a/drivers/gpu/drm/nouveau/core/subdev/clock/base.c
+++ b/drivers/gpu/drm/nouveau/core/subdev/clock/base.c
@@ -464,6 +464,7 @@ nouveau_clock_create_(struct nouveau_object *parent,
 		      struct nouveau_object *engine,
 		      struct nouveau_oclass *oclass,
 		      struct nouveau_clocks *clocks,
+		      struct nouveau_pstate *pstates, int nb_pstates,
 		      bool allow_reclock,
 		      int length, void **object)
 {
@@ -482,10 +483,17 @@ nouveau_clock_create_(struct nouveau_object *parent,
 	clk->domains = clocks;
 	clk->ustate = -1;
 
-	idx = 0;
-	do {
-		ret = nouveau_pstate_new(clk, idx++);
-	} while (ret == 0);
+	/* If no pstates are provided, try and fetch them from the BIOS */
+	if (!pstates) {
+		idx = 0;
+		do {
+			ret = nouveau_pstate_new(clk, idx++);
+		} while (ret == 0);
+	} else {
+		for (idx = 0; idx < nb_pstates; idx++)
+			list_add_tail(&pstates[idx].head, &clk->states);
+		clk->state_nr = nb_pstates;
+	}
 
 	clk->allow_reclock = allow_reclock;
 
diff --git a/drivers/gpu/drm/nouveau/core/subdev/clock/nv04.c b/drivers/gpu/drm/nouveau/core/subdev/clock/nv04.c
index eb2d4425a49e..4c48232686be 100644
--- a/drivers/gpu/drm/nouveau/core/subdev/clock/nv04.c
+++ b/drivers/gpu/drm/nouveau/core/subdev/clock/nv04.c
@@ -82,8 +82,8 @@ nv04_clock_ctor(struct nouveau_object *parent, struct nouveau_object *engine,
 	struct nv04_clock_priv *priv;
 	int ret;
 
-	ret = nouveau_clock_create(parent, engine, oclass, nv04_domain, false,
-				   &priv);
+	ret = nouveau_clock_create(parent, engine, oclass, nv04_domain, NULL, 0,
+				   false, &priv);
 	*pobject = nv_object(priv);
 	if (ret)
 		return ret;
diff --git a/drivers/gpu/drm/nouveau/core/subdev/clock/nv40.c b/drivers/gpu/drm/nouveau/core/subdev/clock/nv40.c
index 8a9e16839791..08368fe97029 100644
--- a/drivers/gpu/drm/nouveau/core/subdev/clock/nv40.c
+++ b/drivers/gpu/drm/nouveau/core/subdev/clock/nv40.c
@@ -213,8 +213,8 @@ nv40_clock_ctor(struct nouveau_object *parent, struct nouveau_object *engine,
 	struct nv40_clock_priv *priv;
 	int ret;
 
-	ret = nouveau_clock_create(parent, engine, oclass, nv40_domain, true,
-				   &priv);
+	ret = nouveau_clock_create(parent, engine, oclass, nv40_domain, NULL, 0,
+				   true, &priv);
 	*pobject = nv_object(priv);
 	if (ret)
 		return ret;
diff --git a/drivers/gpu/drm/nouveau/core/subdev/clock/nv50.c b/drivers/gpu/drm/nouveau/core/subdev/clock/nv50.c
index 8c132772ba9e..5070ebc260f8 100644
--- a/drivers/gpu/drm/nouveau/core/subdev/clock/nv50.c
+++ b/drivers/gpu/drm/nouveau/core/subdev/clock/nv50.c
@@ -507,7 +507,7 @@ nv50_clock_ctor(struct nouveau_object *parent, struct nouveau_object *engine,
 	int ret;
 
 	ret = nouveau_clock_create(parent, engine, oclass, pclass->domains,
-				   false, &priv);
+				   NULL, 0, false, &priv);
 	*pobject = nv_object(priv);
 	if (ret)
 		return ret;
diff --git a/drivers/gpu/drm/nouveau/core/subdev/clock/nva3.c b/drivers/gpu/drm/nouveau/core/subdev/clock/nva3.c
index 9fb58354a80b..087012b18956 100644
--- a/drivers/gpu/drm/nouveau/core/subdev/clock/nva3.c
+++ b/drivers/gpu/drm/nouveau/core/subdev/clock/nva3.c
@@ -302,8 +302,8 @@ nva3_clock_ctor(struct nouveau_object *parent, struct nouveau_object *engine,
 	struct nva3_clock_priv *priv;
 	int ret;
 
-	ret = nouveau_clock_create(parent, engine, oclass, nva3_domain, false,
-				   &priv);
+	ret = nouveau_clock_create(parent, engine, oclass, nva3_domain, NULL, 0,
+				   false, &priv);
 	*pobject = nv_object(priv);
 	if (ret)
 		return ret;
diff --git a/drivers/gpu/drm/nouveau/core/subdev/clock/nvaa.c b/drivers/gpu/drm/nouveau/core/subdev/clock/nvaa.c
index 6a65fc9e9663..74e19731b1b7 100644
--- a/drivers/gpu/drm/nouveau/core/subdev/clock/nvaa.c
+++ b/drivers/gpu/drm/nouveau/core/subdev/clock/nvaa.c
@@ -421,8 +421,8 @@ nvaa_clock_ctor(struct nouveau_object *parent, struct nouveau_object *engine,
 	struct nvaa_clock_priv *priv;
 	int ret;
 
-	ret = nouveau_clock_create(parent, engine, oclass, nvaa_domains, true,
-				   &priv);
+	ret = nouveau_clock_create(parent, engine, oclass, nvaa_domains, NULL,
+				   0, true, &priv);
 	*pobject = nv_object(priv);
 	if (ret)
 		return ret;
diff --git a/drivers/gpu/drm/nouveau/core/subdev/clock/nvc0.c b/drivers/gpu/drm/nouveau/core/subdev/clock/nvc0.c
index dbf8517f54da..1234abaab2db 100644
--- a/drivers/gpu/drm/nouveau/core/subdev/clock/nvc0.c
+++ b/drivers/gpu/drm/nouveau/core/subdev/clock/nvc0.c
@@ -437,8 +437,8 @@ nvc0_clock_ctor(struct nouveau_object *parent, struct nouveau_object *engine,
 	struct nvc0_clock_priv *priv;
 	int ret;
 
-	ret = nouveau_clock_create(parent, engine, oclass, nvc0_domain, false,
-				   &priv);
+	ret = nouveau_clock_create(parent, engine, oclass, nvc0_domain, NULL, 0,
+				   false, &priv);
 	*pobject = nv_object(priv);
 	if (ret)
 		return ret;
diff --git a/drivers/gpu/drm/nouveau/core/subdev/clock/nve0.c b/drivers/gpu/drm/nouveau/core/subdev/clock/nve0.c
index 0e62a3240144..7eccad57512e 100644
--- a/drivers/gpu/drm/nouveau/core/subdev/clock/nve0.c
+++ b/drivers/gpu/drm/nouveau/core/subdev/clock/nve0.c
@@ -475,8 +475,8 @@ nve0_clock_ctor(struct nouveau_object *parent, struct nouveau_object *engine,
 	struct nve0_clock_priv *priv;
 	int ret;
 
-	ret = nouveau_clock_create(parent, engine, oclass, nve0_domain, true,
-				   &priv);
+	ret = nouveau_clock_create(parent, engine, oclass, nve0_domain, NULL, 0,
+				   true, &priv);
 	*pobject = nv_object(priv);
 	if (ret)
 		return ret;
-- 
2.0.0

[PATCH 3/3] drm/gk20a: reclocking support

[Alexandre Courbot]

Add support for reclocking on GK20A, using a statically-defined pstates
table. The algorithms for calculating the coefficients and setting the
clocks are directly taken from the ChromeOS kernel.

Signed-off-by: Alexandre Courbot <acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/gpu/drm/nouveau/Makefile                   |   1 +
 drivers/gpu/drm/nouveau/core/engine/device/nve0.c  |   1 +
 .../gpu/drm/nouveau/core/include/subdev/clock.h    |   1 +
 drivers/gpu/drm/nouveau/core/subdev/clock/gk20a.c  | 670 +++++++++++++++++++++
 4 files changed, 673 insertions(+)
 create mode 100644 drivers/gpu/drm/nouveau/core/subdev/clock/gk20a.c

diff --git a/drivers/gpu/drm/nouveau/Makefile b/drivers/gpu/drm/nouveau/Makefile
index a882ca0f3819..205d1ae7dd03 100644
--- a/drivers/gpu/drm/nouveau/Makefile
+++ b/drivers/gpu/drm/nouveau/Makefile
@@ -65,6 +65,7 @@ nouveau-y += core/subdev/clock/nva3.o
 nouveau-y += core/subdev/clock/nvaa.o
 nouveau-y += core/subdev/clock/nvc0.o
 nouveau-y += core/subdev/clock/nve0.o
+nouveau-y += core/subdev/clock/gk20a.o
 nouveau-y += core/subdev/clock/pllnv04.o
 nouveau-y += core/subdev/clock/pllnva3.o
 nouveau-y += core/subdev/devinit/base.o
diff --git a/drivers/gpu/drm/nouveau/core/engine/device/nve0.c b/drivers/gpu/drm/nouveau/core/engine/device/nve0.c
index c75e9fc9b25f..a8b5184088b5 100644
--- a/drivers/gpu/drm/nouveau/core/engine/device/nve0.c
+++ b/drivers/gpu/drm/nouveau/core/engine/device/nve0.c
@@ -158,6 +158,7 @@ nve0_identify(struct nouveau_device *device)
 		break;
 	case 0xea:
 		device->cname = "GK20A";
+		device->oclass[NVDEV_SUBDEV_CLOCK  ] = &gk20a_clock_oclass;
 		device->oclass[NVDEV_SUBDEV_MC     ] =  nvc3_mc_oclass;
 		device->oclass[NVDEV_SUBDEV_BUS    ] =  nvc0_bus_oclass;
 		device->oclass[NVDEV_SUBDEV_TIMER  ] = &gk20a_timer_oclass;
diff --git a/drivers/gpu/drm/nouveau/core/include/subdev/clock.h b/drivers/gpu/drm/nouveau/core/include/subdev/clock.h
index c0fe191c9787..9fed2834f25e 100644
--- a/drivers/gpu/drm/nouveau/core/include/subdev/clock.h
+++ b/drivers/gpu/drm/nouveau/core/include/subdev/clock.h
@@ -138,6 +138,7 @@ extern struct nouveau_oclass *nvaa_clock_oclass;
 extern struct nouveau_oclass nva3_clock_oclass;
 extern struct nouveau_oclass nvc0_clock_oclass;
 extern struct nouveau_oclass nve0_clock_oclass;
+extern struct nouveau_oclass gk20a_clock_oclass;
 
 int nv04_clock_pll_set(struct nouveau_clock *, u32 type, u32 freq);
 int nv04_clock_pll_calc(struct nouveau_clock *, struct nvbios_pll *,
diff --git a/drivers/gpu/drm/nouveau/core/subdev/clock/gk20a.c b/drivers/gpu/drm/nouveau/core/subdev/clock/gk20a.c
new file mode 100644
index 000000000000..e175cfda0a48
--- /dev/null
+++ b/drivers/gpu/drm/nouveau/core/subdev/clock/gk20a.c
@@ -0,0 +1,670 @@
+/*
+ * Copyright (c) 2014, NVIDIA CORPORATION. All rights reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ *
+ * Shamelessly ripped off from ChromeOS's gk20a/clk_pllg.c
+ *
+ */
+
+#define MHZ (1000 * 1000)
+
+#define MASK(w)	((1 << w) - 1)
+
+#define SYS_GPCPLL_CFG_BASE			0x00137000
+#define GPC_BCASE_GPCPLL_CFG_BASE		0x00132800
+
+#define GPCPLL_CFG		(SYS_GPCPLL_CFG_BASE + 0)
+#define GPCPLL_CFG_ENABLE	BIT(0)
+#define GPCPLL_CFG_IDDQ		BIT(1)
+#define GPCPLL_CFG_LOCK_DET_OFF	BIT(4)
+#define GPCPLL_CFG_LOCK		BIT(17)
+
+#define GPCPLL_COEFF		(SYS_GPCPLL_CFG_BASE + 4)
+#define GPCPLL_COEFF_M_SHIFT	0
+#define GPCPLL_COEFF_M_WIDTH	8
+#define GPCPLL_COEFF_N_SHIFT	8
+#define GPCPLL_COEFF_N_WIDTH	8
+#define GPCPLL_COEFF_P_SHIFT	16
+#define GPCPLL_COEFF_P_WIDTH	6
+
+#define GPCPLL_CFG2			(SYS_GPCPLL_CFG_BASE + 0xc)
+#define GPCPLL_CFG2_SETUP2_SHIFT	16
+#define GPCPLL_CFG2_PLL_STEPA_SHIFT	24
+
+#define GPCPLL_CFG3			(SYS_GPCPLL_CFG_BASE + 0x18)
+#define GPCPLL_CFG3_PLL_STEPB_SHIFT	16
+
+#define GPCPLL_NDIV_SLOWDOWN			(SYS_GPCPLL_CFG_BASE + 0x1c)
+#define GPCPLL_NDIV_SLOWDOWN_NDIV_LO_SHIFT	0
+#define GPCPLL_NDIV_SLOWDOWN_NDIV_MID_SHIFT	8
+#define GPCPLL_NDIV_SLOWDOWN_STEP_SIZE_LO2MID_SHIFT	16
+#define GPCPLL_NDIV_SLOWDOWN_SLOWDOWN_USING_PLL_SHIFT	22
+#define GPCPLL_NDIV_SLOWDOWN_EN_DYNRAMP_SHIFT	31
+
+#define SEL_VCO				(SYS_GPCPLL_CFG_BASE + 0x100)
+#define SEL_VCO_GPC2CLK_OUT_SHIFT	0
+
+#define GPC2CLK_OUT			(SYS_GPCPLL_CFG_BASE + 0x250)
+#define GPC2CLK_OUT_SDIV14_INDIV4_WIDTH	1
+#define GPC2CLK_OUT_SDIV14_INDIV4_SHIFT	31
+#define GPC2CLK_OUT_SDIV14_INDIV4_MODE	1
+#define GPC2CLK_OUT_VCODIV_WIDTH	6
+#define GPC2CLK_OUT_VCODIV_SHIFT	8
+#define GPC2CLK_OUT_VCODIV1		0
+#define GPC2CLK_OUT_VCODIV_MASK		(MASK(GPC2CLK_OUT_VCODIV_WIDTH) << \
+					GPC2CLK_OUT_VCODIV_SHIFT)
+#define	GPC2CLK_OUT_BYPDIV_WIDTH	6
+#define GPC2CLK_OUT_BYPDIV_SHIFT	0
+#define GPC2CLK_OUT_BYPDIV31		0x3c
+#define GPC2CLK_OUT_INIT_MASK	((MASK(GPC2CLK_OUT_SDIV14_INDIV4_WIDTH) << \
+		GPC2CLK_OUT_SDIV14_INDIV4_SHIFT)\
+		| (MASK(GPC2CLK_OUT_VCODIV_WIDTH) << GPC2CLK_OUT_VCODIV_SHIFT)\
+		| (MASK(GPC2CLK_OUT_BYPDIV_WIDTH) << GPC2CLK_OUT_BYPDIV_SHIFT))
+#define GPC2CLK_OUT_INIT_VAL	((GPC2CLK_OUT_SDIV14_INDIV4_MODE << \
+		GPC2CLK_OUT_SDIV14_INDIV4_SHIFT) \
+		| (GPC2CLK_OUT_VCODIV1 << GPC2CLK_OUT_VCODIV_SHIFT) \
+		| (GPC2CLK_OUT_BYPDIV31 << GPC2CLK_OUT_BYPDIV_SHIFT))
+
+#define GPC_BCAST_NDIV_SLOWDOWN_DEBUG	(GPC_BCASE_GPCPLL_CFG_BASE + 0xa0)
+#define GPC_BCAST_NDIV_SLOWDOWN_DEBUG_PLL_DYNRAMP_DONE_SYNCED_SHIFT	24
+#define GPC_BCAST_NDIV_SLOWDOWN_DEBUG_PLL_DYNRAMP_DONE_SYNCED_MASK \
+	    (0x1 << GPC_BCAST_NDIV_SLOWDOWN_DEBUG_PLL_DYNRAMP_DONE_SYNCED_SHIFT)
+
+#include <linux/types.h>
+#include <linux/clk.h>
+#include <linux/delay.h>
+
+#include <subdev/clock.h>
+#include <subdev/timer.h>
+
+#include <nouveau_platform.h>
+
+static const u8 pl_to_div[] = {
+/* PL:   0, 1, 2, 3, 4, 5, 6,  7,  8,  9, 10, 11, 12, 13, 14 */
+/* p: */ 1, 2, 3, 4, 5, 6, 8, 10, 12, 16, 12, 16, 20, 24, 32,
+};
+
+/* All frequencies in Mhz */
+struct gk20a_clk_pllg_params {
+	u32 min_freq, max_freq;
+	u32 min_vco, max_vco;
+	u32 min_u, max_u;
+	u32 min_m, max_m;
+	u32 min_n, max_n;
+	u32 min_pl, max_pl;
+};
+
+static const struct gk20a_clk_pllg_params gk20a_pllg_params = {
+	.min_freq = 144, .max_freq = 2064,
+	.min_vco = 1000, .max_vco = 2064,
+	.min_u = 12, .max_u = 38,
+	.min_m = 1, .max_m = 255,
+	.min_n = 8, .max_n = 255,
+	.min_pl = 1, .max_pl = 32,
+};
+
+struct gk20a_clock_priv {
+	struct nouveau_clock base;
+	const struct gk20a_clk_pllg_params *params;
+	u32 m, n, pl;
+	unsigned long parent_rate;
+};
+#define to_gk20a_clock(base) container_of(base, struct gk20a_clock_priv, base)
+
+static void
+gk20a_pllg_read_mnp(struct gk20a_clock_priv *priv)
+{
+	u32 val;
+
+	val = nv_rd32(priv, GPCPLL_COEFF);
+	priv->m = (val >> GPCPLL_COEFF_M_SHIFT) & MASK(GPCPLL_COEFF_M_WIDTH);
+	priv->n = (val >> GPCPLL_COEFF_N_SHIFT) & MASK(GPCPLL_COEFF_N_WIDTH);
+	priv->pl = (val >> GPCPLL_COEFF_P_SHIFT) & MASK(GPCPLL_COEFF_P_WIDTH);
+}
+
+static unsigned long
+gk20a_pllg_calc_rate(struct gk20a_clock_priv *priv)
+{
+	unsigned long rate;
+	unsigned long divider;
+
+	rate = priv->parent_rate * priv->n;
+	divider = priv->m * pl_to_div[priv->pl];
+	do_div(rate, divider);
+
+	return rate / 2;
+}
+
+static int
+gk20a_pllg_calc_mnp(struct gk20a_clock_priv *priv, unsigned long rate)
+{
+	unsigned int target_clk_f, ref_clk_f, target_freq;
+	unsigned int min_vco_f, max_vco_f;
+	u32 low_pl, high_pl, best_pl;
+	unsigned int target_vco_f, vco_f;
+	u32 best_m, best_n;
+	unsigned int u_f;
+	u32 m, n, n2;
+	u32 delta, lwv, best_delta = ~0;
+	int pl;
+
+	target_clk_f = rate * 2 / MHZ;
+	ref_clk_f = priv->parent_rate / MHZ;
+
+	max_vco_f = priv->params->max_vco;
+	min_vco_f = priv->params->min_vco;
+	best_m = priv->params->max_m;
+	best_n = priv->params->min_n;
+	best_pl = priv->params->min_pl;
+
+	target_vco_f = target_clk_f + target_clk_f / 50;
+	if (max_vco_f < target_vco_f)
+		max_vco_f = target_vco_f;
+
+	/* min_pl <= high_pl <= max_pl */
+	high_pl = (max_vco_f + target_vco_f - 1) / target_vco_f;
+	high_pl = min(high_pl, priv->params->max_pl);
+	high_pl = max(high_pl, priv->params->min_pl);
+
+	/* min_pl <= low_pl <= max_pl */
+	low_pl = min_vco_f / target_vco_f;
+	low_pl = min(low_pl, priv->params->max_pl);
+	low_pl = max(low_pl, priv->params->min_pl);
+
+	/* Find Indices of high_pl and low_pl */
+	for (pl = 0; pl < ARRAY_SIZE(pl_to_div) - 1; pl++) {
+		if (pl_to_div[pl] >= low_pl) {
+			low_pl = pl;
+			break;
+		}
+	}
+	for (pl = 0; pl < ARRAY_SIZE(pl_to_div) - 1; pl++) {
+		if (pl_to_div[pl] >= high_pl) {
+			high_pl = pl;
+			break;
+		}
+	}
+
+	/* Select lowest possible VCO */
+	for (pl = low_pl; pl <= high_pl; pl++) {
+		target_vco_f = target_clk_f * pl_to_div[pl];
+		for (m = priv->params->min_m; m <= priv->params->max_m; m++) {
+			u_f = ref_clk_f / m;
+
+			if (u_f < priv->params->min_u)
+				break;
+			if (u_f > priv->params->max_u)
+				continue;
+
+			n = (target_vco_f * m) / ref_clk_f;
+			n2 = ((target_vco_f * m) + (ref_clk_f - 1)) / ref_clk_f;
+
+			if (n > priv->params->max_n)
+				break;
+
+			for (; n <= n2; n++) {
+				if (n < priv->params->min_n)
+					continue;
+				if (n > priv->params->max_n)
+					break;
+
+				vco_f = ref_clk_f * n / m;
+
+				if (vco_f >= min_vco_f && vco_f <= max_vco_f) {
+					lwv = (vco_f + (pl_to_div[pl] / 2))
+						/ pl_to_div[pl];
+					delta = abs(lwv - target_clk_f);
+
+					if (delta < best_delta) {
+						best_delta = delta;
+						best_m = m;
+						best_n = n;
+						best_pl = pl;
+
+						if (best_delta == 0)
+							goto found_match;
+					}
+				}
+			}
+		}
+	}
+
+found_match:
+	WARN_ON(best_delta == ~0);
+
+	if (best_delta != 0)
+		nv_debug(priv, "no best match for target @ %dMHz on gpc_pll",
+			 target_clk_f);
+
+	priv->m = best_m;
+	priv->n = best_n;
+	priv->pl = best_pl;
+
+	target_freq = gk20a_pllg_calc_rate(priv) / MHZ;
+
+	nv_debug(priv, "actual target freq %d MHz, M %d, N %d, PL %d(div%d)\n",
+		 target_freq, priv->m, priv->n, priv->pl, pl_to_div[priv->pl]);
+
+	return 0;
+}
+
+static int
+gk20a_pllg_slide(struct gk20a_clock_priv *priv, u32 n)
+{
+	u32 val;
+	int ramp_timeout;
+
+	/* get old coefficients */
+	val = nv_rd32(priv, GPCPLL_COEFF);
+	/* do nothing if NDIV is the same */
+	if (n == ((val >> GPCPLL_COEFF_N_SHIFT) & MASK(GPCPLL_COEFF_N_WIDTH)))
+		return 0;
+
+	/* setup */
+	nv_mask(priv, GPCPLL_CFG2, 0xff << GPCPLL_CFG2_PLL_STEPA_SHIFT,
+		0x2b << GPCPLL_CFG2_PLL_STEPA_SHIFT);
+	nv_mask(priv, GPCPLL_CFG3, 0xff << GPCPLL_CFG3_PLL_STEPB_SHIFT,
+		0xb << GPCPLL_CFG3_PLL_STEPB_SHIFT);
+
+	/* pll slowdown mode */
+	nv_mask(priv, GPCPLL_NDIV_SLOWDOWN,
+		BIT(GPCPLL_NDIV_SLOWDOWN_SLOWDOWN_USING_PLL_SHIFT),
+		BIT(GPCPLL_NDIV_SLOWDOWN_SLOWDOWN_USING_PLL_SHIFT));
+
+	/* new ndiv ready for ramp */
+	val = nv_rd32(priv, GPCPLL_COEFF);
+	val &= ~(MASK(GPCPLL_COEFF_N_WIDTH) << GPCPLL_COEFF_N_SHIFT);
+	val |= (n & MASK(GPCPLL_COEFF_N_WIDTH)) << GPCPLL_COEFF_N_SHIFT;
+	udelay(1);
+	nv_wr32(priv, GPCPLL_COEFF, val);
+
+	/* dynamic ramp to new ndiv */
+	val = nv_rd32(priv, GPCPLL_NDIV_SLOWDOWN);
+	val |= 0x1 << GPCPLL_NDIV_SLOWDOWN_EN_DYNRAMP_SHIFT;
+	udelay(1);
+	nv_wr32(priv, GPCPLL_NDIV_SLOWDOWN, val);
+
+	for (ramp_timeout = 500; ramp_timeout > 0; ramp_timeout--) {
+		udelay(1);
+		val = nv_rd32(priv, GPC_BCAST_NDIV_SLOWDOWN_DEBUG);
+		if (val & GPC_BCAST_NDIV_SLOWDOWN_DEBUG_PLL_DYNRAMP_DONE_SYNCED_MASK)
+			break;
+	}
+
+	/* exit slowdown mode */
+	nv_mask(priv, GPCPLL_NDIV_SLOWDOWN,
+		BIT(GPCPLL_NDIV_SLOWDOWN_SLOWDOWN_USING_PLL_SHIFT) |
+		BIT(GPCPLL_NDIV_SLOWDOWN_EN_DYNRAMP_SHIFT), 0);
+	nv_rd32(priv, GPCPLL_NDIV_SLOWDOWN);
+
+	if (ramp_timeout <= 0) {
+		nv_error(priv, "gpcpll dynamic ramp timeout\n");
+		return -ETIMEDOUT;
+	}
+
+	return 0;
+}
+
+static void
+_gk20a_pllg_enable(struct gk20a_clock_priv *priv)
+{
+	nv_mask(priv, GPCPLL_CFG, GPCPLL_CFG_ENABLE, GPCPLL_CFG_ENABLE);
+	nv_rd32(priv, GPCPLL_CFG);
+}
+
+static void
+_gk20a_pllg_disable(struct gk20a_clock_priv *priv)
+{
+	nv_mask(priv, GPCPLL_CFG, GPCPLL_CFG_ENABLE, 0);
+	nv_rd32(priv, GPCPLL_CFG);
+}
+
+static int
+_gk20a_pllg_program_mnp(struct gk20a_clock_priv *priv, bool allow_slide)
+{
+	u32 val, cfg;
+	u32 m_old, pl_old, n_lo;
+
+	/* get old coefficients */
+	val = nv_rd32(priv, GPCPLL_COEFF);
+	m_old = (val >> GPCPLL_COEFF_M_SHIFT) & MASK(GPCPLL_COEFF_M_WIDTH);
+	pl_old = (val >> GPCPLL_COEFF_P_SHIFT) & MASK(GPCPLL_COEFF_P_WIDTH);
+
+	/* do NDIV slide if there is no change in M and PL */
+	cfg = nv_rd32(priv, GPCPLL_CFG);
+	if (allow_slide && priv->m == m_old && priv->pl == pl_old &&
+	    (cfg & GPCPLL_CFG_ENABLE)) {
+		return gk20a_pllg_slide(priv, priv->n);
+	}
+
+	/* slide down to NDIV_LO */
+	n_lo = DIV_ROUND_UP(m_old * priv->params->min_vco,
+			    priv->parent_rate / MHZ);
+	if (allow_slide && (cfg & GPCPLL_CFG_ENABLE)) {
+		int ret = gk20a_pllg_slide(priv, n_lo);
+
+		if (ret)
+			return ret;
+	}
+
+	/* split FO-to-bypass jump in halfs by setting out divider 1:2 */
+	nv_mask(priv, GPC2CLK_OUT, GPC2CLK_OUT_VCODIV_MASK,
+		0x2 << GPC2CLK_OUT_VCODIV_SHIFT);
+
+	/* put PLL in bypass before programming it */
+	val = nv_rd32(priv, SEL_VCO);
+	val &= ~(BIT(SEL_VCO_GPC2CLK_OUT_SHIFT));
+	udelay(2);
+	nv_wr32(priv, SEL_VCO, val);
+
+	/* get out from IDDQ */
+	val = nv_rd32(priv, GPCPLL_CFG);
+	if (val & GPCPLL_CFG_IDDQ) {
+		val &= ~GPCPLL_CFG_IDDQ;
+		nv_wr32(priv, GPCPLL_CFG, val);
+		nv_rd32(priv, GPCPLL_CFG);
+		udelay(2);
+	}
+
+	_gk20a_pllg_disable(priv);
+
+	nv_debug(priv, "%s: m=%d n=%d pl=%d\n", __func__, priv->m, priv->n,
+		 priv->pl);
+
+	n_lo = DIV_ROUND_UP(priv->m * priv->params->min_vco,
+			    priv->parent_rate / MHZ);
+	val = priv->m << GPCPLL_COEFF_M_SHIFT;
+	val |= (allow_slide ? n_lo : priv->n) << GPCPLL_COEFF_N_SHIFT;
+	val |= priv->pl << GPCPLL_COEFF_P_SHIFT;
+	nv_wr32(priv, GPCPLL_COEFF, val);
+
+	_gk20a_pllg_enable(priv);
+
+	val = nv_rd32(priv, GPCPLL_CFG);
+	if (val & GPCPLL_CFG_LOCK_DET_OFF) {
+		val &= ~GPCPLL_CFG_LOCK_DET_OFF;
+		nv_wr32(priv, GPCPLL_CFG, val);
+	}
+
+	if (!nouveau_timer_wait_eq(priv, 300000, GPCPLL_CFG, GPCPLL_CFG_LOCK,
+				   GPCPLL_CFG_LOCK)) {
+		nv_error(priv, "%s: timeout waiting for pllg lock\n", __func__);
+		return -ETIMEDOUT;
+	}
+
+	/* switch to VCO mode */
+	nv_mask(priv, SEL_VCO, 0, BIT(SEL_VCO_GPC2CLK_OUT_SHIFT));
+
+	/* restore out divider 1:1 */
+	val = nv_rd32(priv, GPC2CLK_OUT);
+	val &= ~GPC2CLK_OUT_VCODIV_MASK;
+	udelay(2);
+	nv_wr32(priv, GPC2CLK_OUT, val);
+
+	/* slide up to new NDIV */
+	return allow_slide ? gk20a_pllg_slide(priv, priv->n) : 0;
+}
+
+static int
+gk20a_pllg_program_mnp(struct gk20a_clock_priv *priv)
+{
+	int err;
+
+	err = _gk20a_pllg_program_mnp(priv, true);
+	if (err)
+		err = _gk20a_pllg_program_mnp(priv, false);
+
+	return err;
+}
+
+static void
+gk20a_pllg_disable(struct gk20a_clock_priv *priv)
+{
+	u32 val;
+
+	/* slide to VCO min */
+	val = nv_rd32(priv, GPCPLL_CFG);
+	if (val & GPCPLL_CFG_ENABLE) {
+		u32 coeff, m, n_lo;
+
+		coeff = nv_rd32(priv, GPCPLL_COEFF);
+		m = (coeff >> GPCPLL_COEFF_M_SHIFT) & MASK(GPCPLL_COEFF_M_WIDTH);
+		n_lo = DIV_ROUND_UP(m * priv->params->min_vco,
+				    priv->parent_rate / MHZ);
+		gk20a_pllg_slide(priv, n_lo);
+	}
+
+	/* put PLL in bypass before disabling it */
+	nv_mask(priv, SEL_VCO, BIT(SEL_VCO_GPC2CLK_OUT_SHIFT), 0);
+
+	_gk20a_pllg_disable(priv);
+}
+
+#define GK20A_CLK_GPC_MDIV 1000
+
+static struct nouveau_clocks
+gk20a_domains[] = {
+	{ nv_clk_src_crystal, 0xff },
+	{ nv_clk_src_gpc, 0xff, 0, "core", GK20A_CLK_GPC_MDIV },
+	{ nv_clk_src_max }
+};
+
+static struct nouveau_pstate
+gk20a_pstates[] = {
+	{
+		.base = {
+			.domain[nv_clk_src_gpc] = 72000,
+		},
+	},
+	{
+		.base = {
+			.domain[nv_clk_src_gpc] = 108000,
+		},
+	},
+	{
+		.base = {
+			.domain[nv_clk_src_gpc] = 180000,
+		},
+	},
+	{
+		.base = {
+			.domain[nv_clk_src_gpc] = 252000,
+		},
+	},
+	{
+		.base = {
+			.domain[nv_clk_src_gpc] = 324000,
+		},
+	},
+	{
+		.base = {
+			.domain[nv_clk_src_gpc] = 396000,
+		},
+	},
+	{
+		.base = {
+			.domain[nv_clk_src_gpc] = 468000,
+		},
+	},
+	{
+		.base = {
+			.domain[nv_clk_src_gpc] = 540000,
+		},
+	},
+	{
+		.base = {
+			.domain[nv_clk_src_gpc] = 612000,
+		},
+	},
+	{
+		.base = {
+			.domain[nv_clk_src_gpc] = 648000,
+		},
+	},
+	{
+		.base = {
+			.domain[nv_clk_src_gpc] = 684000,
+		},
+	},
+	{
+		.base = {
+			.domain[nv_clk_src_gpc] = 708000,
+		},
+	},
+	{
+		.base = {
+			.domain[nv_clk_src_gpc] = 756000,
+		},
+	},
+	{
+		.base = {
+			.domain[nv_clk_src_gpc] = 804000,
+		},
+	},
+	{
+		.base = {
+			.domain[nv_clk_src_gpc] = 852000,
+		},
+	},
+};
+
+static int
+gk20a_clock_read(struct nouveau_clock *clk, enum nv_clk_src src)
+{
+	struct gk20a_clock_priv *priv = (void *)clk;
+
+	switch (src) {
+	case nv_clk_src_crystal:
+		return nv_device(clk)->crystal;
+	case nv_clk_src_gpc:
+		gk20a_pllg_read_mnp(priv);
+		return gk20a_pllg_calc_rate(priv) / GK20A_CLK_GPC_MDIV;
+	default:
+		nv_error(clk, "invalid clock source %d\n", src);
+		return -EINVAL;
+	}
+}
+
+static int
+gk20a_clock_calc(struct nouveau_clock *clk, struct nouveau_cstate *cstate)
+{
+	struct gk20a_clock_priv *priv = (void *)clk;
+
+	return gk20a_pllg_calc_mnp(priv, cstate->domain[nv_clk_src_gpc] *
+					 GK20A_CLK_GPC_MDIV);
+}
+
+static int
+gk20a_clock_prog(struct nouveau_clock *clk)
+{
+	struct gk20a_clock_priv *priv = (void *)clk;
+
+	return gk20a_pllg_program_mnp(priv);
+}
+
+static void
+gk20a_clock_tidy(struct nouveau_clock *clk)
+{
+}
+
+static int
+gk20a_clock_fini(struct nouveau_object *object, bool suspend)
+{
+	struct gk20a_clock_priv *priv = (void *)object;
+	int ret;
+
+	ret = nouveau_clock_fini(&priv->base, false);
+
+	gk20a_pllg_disable(priv);
+
+	return ret;
+}
+
+static int
+gk20a_clock_init(struct nouveau_object *object)
+{
+	struct gk20a_clock_priv *priv = (void *)object;
+	int ret;
+
+	nv_mask(priv, GPC2CLK_OUT, GPC2CLK_OUT_INIT_MASK, GPC2CLK_OUT_INIT_VAL);
+
+	ret = gk20a_clock_calc(&priv->base, &gk20a_pstates[0].base);
+	if (ret) {
+		nv_error(priv, "cannot compute clock parameters\n");
+		return ret;
+	}
+
+	ret = gk20a_clock_prog(&priv->base);
+	if (ret) {
+		nv_error(priv, "cannot initialize PLLG\n");
+		return ret;
+	}
+
+	nouveau_clock_init(&priv->base);
+
+	return 0;
+}
+
+static int
+gk20a_clock_ctor(struct nouveau_object *parent,  struct nouveau_object *engine,
+		 struct nouveau_oclass *oclass, void *data, u32 size,
+		 struct nouveau_object **pobject)
+{
+	struct gk20a_clock_priv *priv;
+	struct nouveau_platform_device *plat;
+	int ret;
+	int i;
+
+	/* Finish initializing the pstates */
+	for (i = 0; i < ARRAY_SIZE(gk20a_pstates); i++) {
+		INIT_LIST_HEAD(&gk20a_pstates[i].list);
+		gk20a_pstates[i].pstate = i + 1;
+	}
+
+	ret = nouveau_clock_create(parent, engine, oclass, gk20a_domains,
+			gk20a_pstates, ARRAY_SIZE(gk20a_pstates), true, &priv);
+	*pobject = nv_object(priv);
+	if (ret)
+		return ret;
+
+	priv->params = &gk20a_pllg_params;
+
+	plat = nv_device_to_platform(nv_device(parent));
+	priv->parent_rate = clk_get_rate(plat->gpu->clk);
+	nv_info(priv, "parent clock rate: %ld Mhz\n", priv->parent_rate / MHZ);
+
+	priv->base.read = gk20a_clock_read;
+	priv->base.calc = gk20a_clock_calc;
+	priv->base.prog = gk20a_clock_prog;
+	priv->base.tidy = gk20a_clock_tidy;
+
+	return 0;
+}
+
+struct nouveau_oclass
+gk20a_clock_oclass = {
+	.handle = NV_SUBDEV(CLOCK, 0xea),
+	.ofuncs = &(struct nouveau_ofuncs) {
+		.ctor = gk20a_clock_ctor,
+		.dtor = _nouveau_subdev_dtor,
+		.init = gk20a_clock_init,
+		.fini = gk20a_clock_fini,
+	},
+};
-- 
2.0.0

Re: [PATCH 0/3] drm/gk20a: support for reclocking

[Peter De Schrijver]

On Thu, Jul 10, 2014 at 09:34:34AM +0200, Alexandre Courbot wrote:
> This series adds support for reclocking on GK20A. The first two patches touch
> the clock subsystem to allow GK20A to operate, by making the presence of the
> thermal and voltage devices optional, and allowing pstates to be provided
> directly instead of being probed using the BIOS (which Tegra does not have).
> 
> The last patch adds the GK20A clock device. Arguably the clock can be seen as a
> stripped-down version of what is seen on NVE0, however instead of using NVE0
> support has been written from scratch using the ChromeOS kernel as a basis.
> There are several reasons for this:
> 
> - The ChromeOS driver uses a lookup table for the P coefficient which I could
>   not find in the NVE0 driver,
> - Some registers that NVE0 expects to find are not present on GK20A (e.g.
>   0x137120 and 0x137140),
> - Calculation of MNP is done differently from what is performed in
>   nva3_pll_calc(), and it might be interesting to compare the two methods,
> - All the same, the programming sequence is done differently in the ChromeOS
>   driver and NVE0 could possibly benefit from it (?)
> 
> It would be interesting to try and merge both, but for now I prefer to have the
> two coexisting to ensure proper operation on GK20A and besure I don't break
> dGPU support. :)
> 
> Regarding the first patch, one might argue that I could as well add thermal
> and voltage devices to GK20A. The reason this is not done is because these
> currently depend heavily on the presence of a BIOS, and will require a rework
> similar to that done in patch 2 for clocks. I would like to make sure this
> approach is approved because applying it to other subdevs.

I think this should use CCF so we can use pre and post rate change notifiers
to hookup vdd_gpu DVS.

Thanks,

Peter.

Re: [PATCH 0/3] drm/gk20a: support for reclocking

[Alexandre Courbot]

On 07/10/2014 06:43 PM, Peter De Schrijver wrote:
> On Thu, Jul 10, 2014 at 09:34:34AM +0200, Alexandre Courbot wrote:
>> This series adds support for reclocking on GK20A. The first two patches touch
>> the clock subsystem to allow GK20A to operate, by making the presence of the
>> thermal and voltage devices optional, and allowing pstates to be provided
>> directly instead of being probed using the BIOS (which Tegra does not have).
>>
>> The last patch adds the GK20A clock device. Arguably the clock can be seen as a
>> stripped-down version of what is seen on NVE0, however instead of using NVE0
>> support has been written from scratch using the ChromeOS kernel as a basis.
>> There are several reasons for this:
>>
>> - The ChromeOS driver uses a lookup table for the P coefficient which I could
>>    not find in the NVE0 driver,
>> - Some registers that NVE0 expects to find are not present on GK20A (e.g.
>>    0x137120 and 0x137140),
>> - Calculation of MNP is done differently from what is performed in
>>    nva3_pll_calc(), and it might be interesting to compare the two methods,
>> - All the same, the programming sequence is done differently in the ChromeOS
>>    driver and NVE0 could possibly benefit from it (?)
>>
>> It would be interesting to try and merge both, but for now I prefer to have the
>> two coexisting to ensure proper operation on GK20A and besure I don't break
>> dGPU support. :)
>>
>> Regarding the first patch, one might argue that I could as well add thermal
>> and voltage devices to GK20A. The reason this is not done is because these
>> currently depend heavily on the presence of a BIOS, and will require a rework
>> similar to that done in patch 2 for clocks. I would like to make sure this
>> approach is approved because applying it to other subdevs.
>
> I think this should use CCF so we can use pre and post rate change notifiers
> to hookup vdd_gpu DVS.

Do you mean that we should turn the Nouveau gk20a clock driver into a 
consumer of this CCF clock? I have nothing against this, but note that 
Nouveau can also perform DVS on its own, as the pstates can also contain 
a voltage to be applied to the volt device (not yet implemented in this 
series).

The question then becomes whether we want an additional layer of 
abstraction on these devices and whether the pre/post rate change 
notifiers give us any advantage compared to what Nouveau currently proposes.

Re: [PATCH 0/3] drm/gk20a: support for reclocking

[Ben Skeggs]

On Fri, Jul 11, 2014 at 11:49 AM, Alexandre Courbot <acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> On 07/10/2014 06:43 PM, Peter De Schrijver wrote:
>>
>> On Thu, Jul 10, 2014 at 09:34:34AM +0200, Alexandre Courbot wrote:
>>>
>>> This series adds support for reclocking on GK20A. The first two patches
>>> touch
>>> the clock subsystem to allow GK20A to operate, by making the presence of
>>> the
>>> thermal and voltage devices optional, and allowing pstates to be provided
>>> directly instead of being probed using the BIOS (which Tegra does not
>>> have).
>>>
>>> The last patch adds the GK20A clock device. Arguably the clock can be
>>> seen as a
>>> stripped-down version of what is seen on NVE0, however instead of using
>>> NVE0
>>> support has been written from scratch using the ChromeOS kernel as a
>>> basis.
>>> There are several reasons for this:
>>>
>>> - The ChromeOS driver uses a lookup table for the P coefficient which I
>>> could
>>>    not find in the NVE0 driver,
>>> - Some registers that NVE0 expects to find are not present on GK20A (e.g.
>>>    0x137120 and 0x137140),
>>> - Calculation of MNP is done differently from what is performed in
>>>    nva3_pll_calc(), and it might be interesting to compare the two
>>> methods,
>>> - All the same, the programming sequence is done differently in the
>>> ChromeOS
>>>    driver and NVE0 could possibly benefit from it (?)
>>>
>>> It would be interesting to try and merge both, but for now I prefer to
>>> have the
>>> two coexisting to ensure proper operation on GK20A and besure I don't
>>> break
>>> dGPU support. :)
>>>
>>> Regarding the first patch, one might argue that I could as well add
>>> thermal
>>> and voltage devices to GK20A. The reason this is not done is because
>>> these
>>> currently depend heavily on the presence of a BIOS, and will require a
>>> rework
>>> similar to that done in patch 2 for clocks. I would like to make sure
>>> this
>>> approach is approved because applying it to other subdevs.
>>
>>
>> I think this should use CCF so we can use pre and post rate change
>> notifiers
>> to hookup vdd_gpu DVS.
>
>
> Do you mean that we should turn the Nouveau gk20a clock driver into a
> consumer of this CCF clock? I have nothing against this, but note that
> Nouveau can also perform DVS on its own, as the pstates can also contain a
> voltage to be applied to the volt device (not yet implemented in this
> series).
>
> The question then becomes whether we want an additional layer of abstraction
> on these devices and whether the pre/post rate change notifiers give us any
> advantage compared to what Nouveau currently proposes.
I had a brief look at this, and personally I don't think the CCF is a
very good match at all for how we're *supposed* to manage clock
frequencies as described by a discrete GPU VBIOS, and especially for
when we get to the point of using the PMU falcon to coordinate all the
various bits and pieces that go towards power management.

>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/3] drm/gk20a: support for reclocking

[Peter De Schrijver]

On Fri, Jul 11, 2014 at 04:01:02AM +0200, Ben Skeggs wrote:
> On Fri, Jul 11, 2014 at 11:49 AM, Alexandre Courbot <acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> > On 07/10/2014 06:43 PM, Peter De Schrijver wrote:
> >>
> >> On Thu, Jul 10, 2014 at 09:34:34AM +0200, Alexandre Courbot wrote:
> >>>
> >>> This series adds support for reclocking on GK20A. The first two patches
> >>> touch
> >>> the clock subsystem to allow GK20A to operate, by making the presence of
> >>> the
> >>> thermal and voltage devices optional, and allowing pstates to be provided
> >>> directly instead of being probed using the BIOS (which Tegra does not
> >>> have).
> >>>
> >>> The last patch adds the GK20A clock device. Arguably the clock can be
> >>> seen as a
> >>> stripped-down version of what is seen on NVE0, however instead of using
> >>> NVE0
> >>> support has been written from scratch using the ChromeOS kernel as a
> >>> basis.
> >>> There are several reasons for this:
> >>>
> >>> - The ChromeOS driver uses a lookup table for the P coefficient which I
> >>> could
> >>>    not find in the NVE0 driver,
> >>> - Some registers that NVE0 expects to find are not present on GK20A (e.g.
> >>>    0x137120 and 0x137140),
> >>> - Calculation of MNP is done differently from what is performed in
> >>>    nva3_pll_calc(), and it might be interesting to compare the two
> >>> methods,
> >>> - All the same, the programming sequence is done differently in the
> >>> ChromeOS
> >>>    driver and NVE0 could possibly benefit from it (?)
> >>>
> >>> It would be interesting to try and merge both, but for now I prefer to
> >>> have the
> >>> two coexisting to ensure proper operation on GK20A and besure I don't
> >>> break
> >>> dGPU support. :)
> >>>
> >>> Regarding the first patch, one might argue that I could as well add
> >>> thermal
> >>> and voltage devices to GK20A. The reason this is not done is because
> >>> these
> >>> currently depend heavily on the presence of a BIOS, and will require a
> >>> rework
> >>> similar to that done in patch 2 for clocks. I would like to make sure
> >>> this
> >>> approach is approved because applying it to other subdevs.
> >>
> >>
> >> I think this should use CCF so we can use pre and post rate change
> >> notifiers
> >> to hookup vdd_gpu DVS.
> >
> >
> > Do you mean that we should turn the Nouveau gk20a clock driver into a
> > consumer of this CCF clock? I have nothing against this, but note that
> > Nouveau can also perform DVS on its own, as the pstates can also contain a
> > voltage to be applied to the volt device (not yet implemented in this
> > series).
> >
> > The question then becomes whether we want an additional layer of abstraction
> > on these devices and whether the pre/post rate change notifiers give us any
> > advantage compared to what Nouveau currently proposes.
> I had a brief look at this, and personally I don't think the CCF is a
> very good match at all for how we're *supposed* to manage clock
> frequencies as described by a discrete GPU VBIOS, and especially for
> when we get to the point of using the PMU falcon to coordinate all the
> various bits and pieces that go towards power management.
> 

For all I can see, the PMU is not involved in the mechanics of GPU frequency
scaling on Tegra.

Cheers,

Peter.

Re: [PATCH 0/3] drm/gk20a: support for reclocking

[Peter De Schrijver]

On Fri, Jul 11, 2014 at 03:49:06AM +0200, Alex Courbot wrote:
> On 07/10/2014 06:43 PM, Peter De Schrijver wrote:
> > On Thu, Jul 10, 2014 at 09:34:34AM +0200, Alexandre Courbot wrote:
> >> This series adds support for reclocking on GK20A. The first two patches touch
> >> the clock subsystem to allow GK20A to operate, by making the presence of the
> >> thermal and voltage devices optional, and allowing pstates to be provided
> >> directly instead of being probed using the BIOS (which Tegra does not have).
> >>
> >> The last patch adds the GK20A clock device. Arguably the clock can be seen as a
> >> stripped-down version of what is seen on NVE0, however instead of using NVE0
> >> support has been written from scratch using the ChromeOS kernel as a basis.
> >> There are several reasons for this:
> >>
> >> - The ChromeOS driver uses a lookup table for the P coefficient which I could
> >>    not find in the NVE0 driver,
> >> - Some registers that NVE0 expects to find are not present on GK20A (e.g.
> >>    0x137120 and 0x137140),
> >> - Calculation of MNP is done differently from what is performed in
> >>    nva3_pll_calc(), and it might be interesting to compare the two methods,
> >> - All the same, the programming sequence is done differently in the ChromeOS
> >>    driver and NVE0 could possibly benefit from it (?)
> >>
> >> It would be interesting to try and merge both, but for now I prefer to have the
> >> two coexisting to ensure proper operation on GK20A and besure I don't break
> >> dGPU support. :)
> >>
> >> Regarding the first patch, one might argue that I could as well add thermal
> >> and voltage devices to GK20A. The reason this is not done is because these
> >> currently depend heavily on the presence of a BIOS, and will require a rework
> >> similar to that done in patch 2 for clocks. I would like to make sure this
> >> approach is approved because applying it to other subdevs.
> >
> > I think this should use CCF so we can use pre and post rate change notifiers
> > to hookup vdd_gpu DVS.
> 
> Do you mean that we should turn the Nouveau gk20a clock driver into a 
> consumer of this CCF clock? I have nothing against this, but note that 
> Nouveau can also perform DVS on its own, as the pstates can also contain 
> a voltage to be applied to the volt device (not yet implemented in this 
> series).
> 

Yes. For Tegra I think it makes sense to move DVS out of the individual
drivers. Then we can share the code which has to deal with building the OPP
tables with other DVS rails (eg. vdd_core) for example. Often there are also
chip specific quirks to be dealt with (such as the maximum allowed voltage step
or voltage relationships between rails), which are easier to handle in common
code.

Cheers,

Peter.

Re: [PATCH 0/3] drm/gk20a: support for reclocking

[Alexandre Courbot]

On Fri, Jul 11, 2014 at 7:54 PM, Peter De Schrijver
<pdeschrijver-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> On Fri, Jul 11, 2014 at 03:49:06AM +0200, Alex Courbot wrote:
>> On 07/10/2014 06:43 PM, Peter De Schrijver wrote:
>> > On Thu, Jul 10, 2014 at 09:34:34AM +0200, Alexandre Courbot wrote:
>> >> This series adds support for reclocking on GK20A. The first two patches touch
>> >> the clock subsystem to allow GK20A to operate, by making the presence of the
>> >> thermal and voltage devices optional, and allowing pstates to be provided
>> >> directly instead of being probed using the BIOS (which Tegra does not have).
>> >>
>> >> The last patch adds the GK20A clock device. Arguably the clock can be seen as a
>> >> stripped-down version of what is seen on NVE0, however instead of using NVE0
>> >> support has been written from scratch using the ChromeOS kernel as a basis.
>> >> There are several reasons for this:
>> >>
>> >> - The ChromeOS driver uses a lookup table for the P coefficient which I could
>> >>    not find in the NVE0 driver,
>> >> - Some registers that NVE0 expects to find are not present on GK20A (e.g.
>> >>    0x137120 and 0x137140),
>> >> - Calculation of MNP is done differently from what is performed in
>> >>    nva3_pll_calc(), and it might be interesting to compare the two methods,
>> >> - All the same, the programming sequence is done differently in the ChromeOS
>> >>    driver and NVE0 could possibly benefit from it (?)
>> >>
>> >> It would be interesting to try and merge both, but for now I prefer to have the
>> >> two coexisting to ensure proper operation on GK20A and besure I don't break
>> >> dGPU support. :)
>> >>
>> >> Regarding the first patch, one might argue that I could as well add thermal
>> >> and voltage devices to GK20A. The reason this is not done is because these
>> >> currently depend heavily on the presence of a BIOS, and will require a rework
>> >> similar to that done in patch 2 for clocks. I would like to make sure this
>> >> approach is approved because applying it to other subdevs.
>> >
>> > I think this should use CCF so we can use pre and post rate change notifiers
>> > to hookup vdd_gpu DVS.
>>
>> Do you mean that we should turn the Nouveau gk20a clock driver into a
>> consumer of this CCF clock? I have nothing against this, but note that
>> Nouveau can also perform DVS on its own, as the pstates can also contain
>> a voltage to be applied to the volt device (not yet implemented in this
>> series).
>>
>
> Yes. For Tegra I think it makes sense to move DVS out of the individual
> drivers. Then we can share the code which has to deal with building the OPP
> tables with other DVS rails (eg. vdd_core) for example. Often there are also
> chip specific quirks to be dealt with (such as the maximum allowed voltage step
> or voltage relationships between rails), which are easier to handle in common
> code.

I see, thanks for the explanation. It should be relatively easy to
convert the clock to CCF if/when we need it - for now I will keep the
direct Nouveau implementation for simplicity purposes.

Re: [PATCH 0/3] drm/gk20a: support for reclocking

[Mikko Perttunen]

Does GK20A itself have any kind of thermal protection capabilities? 
Upstream SOCTHERM support is not yet available (though I have a driver 
in my tree), so we are thinking of disabling CPU DVFS on boards that 
don't have always-on active cooling for now. Same might be necessary for 
GPU as well.

On 10/07/14 10:34, Alexandre Courbot wrote:
> This series adds support for reclocking on GK20A. The first two patches touch
> the clock subsystem to allow GK20A to operate, by making the presence of the
> thermal and voltage devices optional, and allowing pstates to be provided
> directly instead of being probed using the BIOS (which Tegra does not have).
>
> The last patch adds the GK20A clock device. Arguably the clock can be seen as a
> stripped-down version of what is seen on NVE0, however instead of using NVE0
> support has been written from scratch using the ChromeOS kernel as a basis.
> There are several reasons for this:
>
> - The ChromeOS driver uses a lookup table for the P coefficient which I could
>    not find in the NVE0 driver,
> - Some registers that NVE0 expects to find are not present on GK20A (e.g.
>    0x137120 and 0x137140),
> - Calculation of MNP is done differently from what is performed in
>    nva3_pll_calc(), and it might be interesting to compare the two methods,
> - All the same, the programming sequence is done differently in the ChromeOS
>    driver and NVE0 could possibly benefit from it (?)
>
> It would be interesting to try and merge both, but for now I prefer to have the
> two coexisting to ensure proper operation on GK20A and besure I don't break
> dGPU support. :)
>
> Regarding the first patch, one might argue that I could as well add thermal
> and voltage devices to GK20A. The reason this is not done is because these
> currently depend heavily on the presence of a BIOS, and will require a rework
> similar to that done in patch 2 for clocks. I would like to make sure this
> approach is approved because applying it to other subdevs.
>
> Alexandre Courbot (3):
>    drm/nouveau/clk: make therm and volt devices optional
>    drm/nouveau/clk: support for non-BIOS pstates
>    drm/gk20a: reclocking support
>
>   drivers/gpu/drm/nouveau/Makefile                   |   1 +
>   drivers/gpu/drm/nouveau/core/engine/device/nve0.c  |   1 +
>   .../gpu/drm/nouveau/core/include/subdev/clock.h    |   9 +-
>   drivers/gpu/drm/nouveau/core/subdev/clock/base.c   |  52 +-
>   drivers/gpu/drm/nouveau/core/subdev/clock/gk20a.c  | 670 +++++++++++++++++++++
>   drivers/gpu/drm/nouveau/core/subdev/clock/nv04.c   |   4 +-
>   drivers/gpu/drm/nouveau/core/subdev/clock/nv40.c   |   4 +-
>   drivers/gpu/drm/nouveau/core/subdev/clock/nv50.c   |   2 +-
>   drivers/gpu/drm/nouveau/core/subdev/clock/nva3.c   |   4 +-
>   drivers/gpu/drm/nouveau/core/subdev/clock/nvaa.c   |   4 +-
>   drivers/gpu/drm/nouveau/core/subdev/clock/nvc0.c   |   4 +-
>   drivers/gpu/drm/nouveau/core/subdev/clock/nve0.c   |   4 +-
>   12 files changed, 725 insertions(+), 34 deletions(-)
>   create mode 100644 drivers/gpu/drm/nouveau/core/subdev/clock/gk20a.c
>

Re: [PATCH 0/3] drm/gk20a: support for reclocking

[Alexandre Courbot]

On 07/10/2014 06:50 PM, Mikko Perttunen wrote:
> Does GK20A itself have any kind of thermal protection capabilities?
> Upstream SOCTHERM support is not yet available (though I have a driver
> in my tree), so we are thinking of disabling CPU DVFS on boards that
> don't have always-on active cooling for now. Same might be necessary for
> GPU as well.

There is a small thermal driver ( 
https://android.googlesource.com/kernel/tegra/+/b445e5296764d18861a6450f6851f25b9ca59dee/drivers/video/tegra/host/gk20a/therm_gk20a.c 
) but it doesn't seem to do much. I believe that for Tegra we rely in 
SOCTHERM instead, but maybe Ken could confirm?

Re: [PATCH 0/3] drm/gk20a: support for reclocking

[Martin Peres]

On 11/07/2014 03:42, Alexandre Courbot wrote:
> On 07/10/2014 06:50 PM, Mikko Perttunen wrote:
>> Does GK20A itself have any kind of thermal protection capabilities?
>> Upstream SOCTHERM support is not yet available (though I have a driver
>> in my tree), so we are thinking of disabling CPU DVFS on boards that
>> don't have always-on active cooling for now. Same might be necessary for
>> GPU as well.
>
> There is a small thermal driver ( 
> https://android.googlesource.com/kernel/tegra/+/b445e5296764d18861a6450f6851f25b9ca59dee/drivers/video/tegra/host/gk20a/therm_gk20a.c 
> ) but it doesn't seem to do much. I believe that for Tegra we rely in 
> SOCTHERM instead, but maybe Ken could confirm?

Unless it changed a great deal, I reverse engineered most of the 
registers in this area (for the nv50), but some stuff didn't change that 
much and could be used straight away (temperature reading, sending IRQs 
on thresholds, auto downclocking when overheating). We are not really 
using those features on nouveau right now, we just poll on the 
temperature every second.

If we are moving to using the PMU for thermal monitoring, we can do the 
polling there and warn the userspace when the temperature is too high or 
if performance is insufficient / too much. I have part of the code for 
performance counters in PMU, it's dead simple.

Martin

Re: [Nouveau] [PATCH 0/3] drm/gk20a: support for reclocking

[Ben Skeggs]

On Thu, Jul 10, 2014 at 5:34 PM, Alexandre Courbot <acourbot@nvidia.com> wrote:
> This series adds support for reclocking on GK20A. The first two patches touch
> the clock subsystem to allow GK20A to operate, by making the presence of the
> thermal and voltage devices optional, and allowing pstates to be provided
> directly instead of being probed using the BIOS (which Tegra does not have).
Hey Alex,

I mentioned a while back I had some stuff in-progress to make
implementing this a bit cleaner for you, but as you can probably tell,
I didn't get to it yet.  It's likely I won't manage to before the next
merge window either.  So, I'll likely take these patches as-is
(assuming no objections on reviews here) and rebase my stuff on top.

>
> The last patch adds the GK20A clock device. Arguably the clock can be seen as a
> stripped-down version of what is seen on NVE0, however instead of using NVE0
> support has been written from scratch using the ChromeOS kernel as a basis.
> There are several reasons for this:
>
> - The ChromeOS driver uses a lookup table for the P coefficient which I could
>   not find in the NVE0 driver,
Interesting.  Can you give more details on how "PL" works exactly,
we'd been operating on the assumption (mainly inherited from code that
appeared in xf86-video-nv) that it was always a straight division.

> - Some registers that NVE0 expects to find are not present on GK20A (e.g.
>   0x137120 and 0x137140),
> - Calculation of MNP is done differently from what is performed in
>   nva3_pll_calc(), and it might be interesting to compare the two methods,
> - All the same, the programming sequence is done differently in the ChromeOS
>   driver and NVE0 could possibly benefit from it (?)
>
> It would be interesting to try and merge both, but for now I prefer to have the
> two coexisting to ensure proper operation on GK20A and besure I don't break
> dGPU support. :)
It's something we can look to achieving down the road, but won't block
merging the support.

>
> Regarding the first patch, one might argue that I could as well add thermal
> and voltage devices to GK20A. The reason this is not done is because these
> currently depend heavily on the presence of a BIOS, and will require a rework
> similar to that done in patch 2 for clocks. I would like to make sure this
> approach is approved because applying it to other subdevs.
They don't *need* to depend on the BIOS, you can opt for an
implementation that doesn't use the base classes that the rest of the
dGPU implementations do.  But, it's fine to take the approach you've
taken.

Thanks!
Ben.

>
> Alexandre Courbot (3):
>   drm/nouveau/clk: make therm and volt devices optional
>   drm/nouveau/clk: support for non-BIOS pstates
>   drm/gk20a: reclocking support
>
>  drivers/gpu/drm/nouveau/Makefile                   |   1 +
>  drivers/gpu/drm/nouveau/core/engine/device/nve0.c  |   1 +
>  .../gpu/drm/nouveau/core/include/subdev/clock.h    |   9 +-
>  drivers/gpu/drm/nouveau/core/subdev/clock/base.c   |  52 +-
>  drivers/gpu/drm/nouveau/core/subdev/clock/gk20a.c  | 670 +++++++++++++++++++++
>  drivers/gpu/drm/nouveau/core/subdev/clock/nv04.c   |   4 +-
>  drivers/gpu/drm/nouveau/core/subdev/clock/nv40.c   |   4 +-
>  drivers/gpu/drm/nouveau/core/subdev/clock/nv50.c   |   2 +-
>  drivers/gpu/drm/nouveau/core/subdev/clock/nva3.c   |   4 +-
>  drivers/gpu/drm/nouveau/core/subdev/clock/nvaa.c   |   4 +-
>  drivers/gpu/drm/nouveau/core/subdev/clock/nvc0.c   |   4 +-
>  drivers/gpu/drm/nouveau/core/subdev/clock/nve0.c   |   4 +-
>  12 files changed, 725 insertions(+), 34 deletions(-)
>  create mode 100644 drivers/gpu/drm/nouveau/core/subdev/clock/gk20a.c
>
> --
> 2.0.0
>
> _______________________________________________
> Nouveau mailing list
> Nouveau@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [Nouveau] [PATCH 0/3] drm/gk20a: support for reclocking

[Alexandre Courbot]

Hi Ben,

On 07/11/2014 10:07 AM, Ben Skeggs wrote:
> On Thu, Jul 10, 2014 at 5:34 PM, Alexandre Courbot <acourbot@nvidia.com> wrote:
>> This series adds support for reclocking on GK20A. The first two patches touch
>> the clock subsystem to allow GK20A to operate, by making the presence of the
>> thermal and voltage devices optional, and allowing pstates to be provided
>> directly instead of being probed using the BIOS (which Tegra does not have).
> Hey Alex,
>
> I mentioned a while back I had some stuff in-progress to make
> implementing this a bit cleaner for you, but as you can probably tell,
> I didn't get to it yet.  It's likely I won't manage to before the next
> merge window either.  So, I'll likely take these patches as-is
> (assuming no objections on reviews here) and rebase my stuff on top.

Thanks. You will probably notice that these patches won't apply to your 
tree and require some tweaking. I will probably end up sending a v2 
anyway, so maybe you should wait for it. If you want to try this version 
anyway, I have fixed-up patches against your tree.

Note that your tree currently won't build against -next because it uses 
drm_sysfs_connector_add/remove which are not available anymore (replaced 
by drm_connector_register/unregister I believe).

Oh and while I'm at it, there seems to be a typo in line 131 of clock.h, 
which should read _nouveau_clock_fini and not _nouveau_clock_init.

>>
>> The last patch adds the GK20A clock device. Arguably the clock can be seen as a
>> stripped-down version of what is seen on NVE0, however instead of using NVE0
>> support has been written from scratch using the ChromeOS kernel as a basis.
>> There are several reasons for this:
>>
>> - The ChromeOS driver uses a lookup table for the P coefficient which I could
>>    not find in the NVE0 driver,
> Interesting.  Can you give more details on how "PL" works exactly,
> we'd been operating on the assumption (mainly inherited from code that
> appeared in xf86-video-nv) that it was always a straight division.

The pl_to_div table in clock/gk20a.c should give the right coefficients, 
but I have seen contradictory information in our docs. Let me ask the 
right people so we can get to the bottom of this.

>
>> - Some registers that NVE0 expects to find are not present on GK20A (e.g.
>>    0x137120 and 0x137140),
>> - Calculation of MNP is done differently from what is performed in
>>    nva3_pll_calc(), and it might be interesting to compare the two methods,
>> - All the same, the programming sequence is done differently in the ChromeOS
>>    driver and NVE0 could possibly benefit from it (?)
>>
>> It would be interesting to try and merge both, but for now I prefer to have the
>> two coexisting to ensure proper operation on GK20A and besure I don't break
>> dGPU support. :)
> It's something we can look to achieving down the road, but won't block
> merging the support.

Great, thanks!

>
>>
>> Regarding the first patch, one might argue that I could as well add thermal
>> and voltage devices to GK20A. The reason this is not done is because these
>> currently depend heavily on the presence of a BIOS, and will require a rework
>> similar to that done in patch 2 for clocks. I would like to make sure this
>> approach is approved because applying it to other subdevs.
> They don't *need* to depend on the BIOS, you can opt for an
> implementation that doesn't use the base classes that the rest of the
> dGPU implementations do.  But, it's fine to take the approach you've
> taken.

At first I did not use the base classes for the gk20a clock 
implementation, but it resulted in replicating some init code and I was 
concerned that this might be a source of bugs in the future (e.g. clock 
base clock init gets updated but not the gk20a init). So I preferred the 
current approach which keeps everything in the same place.

Since you have no concern with it I will apply the same to volt and 
therm, and we can then get rid of patch 1. Then I should probably send 
you a v2 once the PL thing is cleared.

Cheers,
Alex.

Re: [PATCH 0/3] drm/gk20a: support for reclocking

[Alexandre Courbot]

On Fri, Jul 11, 2014 at 10:38 AM, Alexandre Courbot <acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> Hi Ben,
>
>
> On 07/11/2014 10:07 AM, Ben Skeggs wrote:
>>
>> On Thu, Jul 10, 2014 at 5:34 PM, Alexandre Courbot <acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>> wrote:
>>>
>>> This series adds support for reclocking on GK20A. The first two patches
>>> touch
>>> the clock subsystem to allow GK20A to operate, by making the presence of
>>> the
>>> thermal and voltage devices optional, and allowing pstates to be provided
>>> directly instead of being probed using the BIOS (which Tegra does not
>>> have).
>>
>> Hey Alex,
>>
>> I mentioned a while back I had some stuff in-progress to make
>> implementing this a bit cleaner for you, but as you can probably tell,
>> I didn't get to it yet.  It's likely I won't manage to before the next
>> merge window either.  So, I'll likely take these patches as-is
>> (assuming no objections on reviews here) and rebase my stuff on top.
>
>
> Thanks. You will probably notice that these patches won't apply to your tree
> and require some tweaking. I will probably end up sending a v2 anyway, so
> maybe you should wait for it. If you want to try this version anyway, I have
> fixed-up patches against your tree.
>
> Note that your tree currently won't build against -next because it uses
> drm_sysfs_connector_add/remove which are not available anymore (replaced by
> drm_connector_register/unregister I believe).
>
> Oh and while I'm at it, there seems to be a typo in line 131 of clock.h,
> which should read _nouveau_clock_fini and not _nouveau_clock_init.
>
>
>>>
>>> The last patch adds the GK20A clock device. Arguably the clock can be
>>> seen as a
>>> stripped-down version of what is seen on NVE0, however instead of using
>>> NVE0
>>> support has been written from scratch using the ChromeOS kernel as a
>>> basis.
>>> There are several reasons for this:
>>>
>>> - The ChromeOS driver uses a lookup table for the P coefficient which I
>>> could
>>>    not find in the NVE0 driver,
>>
>> Interesting.  Can you give more details on how "PL" works exactly,
>> we'd been operating on the assumption (mainly inherited from code that
>> appeared in xf86-video-nv) that it was always a straight division.
>
>
> The pl_to_div table in clock/gk20a.c should give the right coefficients, but
> I have seen contradictory information in our docs. Let me ask the right
> people so we can get to the bottom of this.

So as it turns out this seems to be gk20a-specific. Other Kepler GPUs
use it as a straight division, but gk20a uses a narrower range and
thus requires this table.