thr3ads.net - Nouveau - [Nouveau] [RFC PATCH 4/5] subdev/clk: print the base clocks [Dec 2015]

If this information is useful, please help other people find it:
Share via:

Karol Herbst

2015-Dec-01 16:42 UTC

[Nouveau] [RFC PATCH 0/5] stabilize kepler reclocking

this series solves different issues we encounter on kepler cards while
reclocking:

1. core clock doesn't change at all and produces a volting error (patch 1)
	this can happen when the voltage table has only 0ed values in the header
	so we have to parse the entries itself, which contain the right voltages
2. kepler won't clock to highest cstates (patch 2)
	this happens, because there are entries in the cstep table with too high
voltages attached
	in this case we should simply drop those cstates, because they can't even
be used by the gpu
	the voltage limit is found in the voltage map table
3. heat issues after clocking to highest pstate (patch 3-5)
	sometimes the gpu is able to handle the highest cstate, but the gpu isn't
build for that high voltages
	in that case we should just parse the baseclock table, which tells us the
clocks the gpu is intented to run with

the last patches also introduce a new config option: NvBoost

0: highest clock available is the base clock (default)
1: highest clock available is the boost clock
else: all cstates are available

because this will regress performance on some cards, the new option should be
advertised later on, but I think
it is a better idea to be safe here, because otherwise nouveau can easily go
above the TDP of the gpu and this
leads to complete different issues.

After this, we can then try to understand which factors are important for
boosting and implement it in a better way, but for this we need:
1. support for power consumption sensors
2. better understanding on which power budget is the right one
3. in which situation we can boost how far

Because this series can mess up reclocking for a wide range of cards, I really
want to have it tested on several systems, thanks

Karol Herbst (5):
  bios/volt: handle voltage table version 0x50 with 0ed header
  clk: drop cstates with too high voltage
  nvbios: add parsing of BASE CLOCK table
  subdev/clk: print the base clocks
  clk: allow boosting only when NvBoost is set

 drm/nouveau/include/nvkm/subdev/bios/baseclock.h | 23 +++++++
 drm/nouveau/include/nvkm/subdev/clk.h            | 10 ++-
 drm/nouveau/include/nvkm/subdev/volt.h           |  4 ++
 drm/nouveau/nvkm/subdev/bios/Kbuild              |  1 +
 drm/nouveau/nvkm/subdev/bios/baseclock.c         | 79 ++++++++++++++++++++++++
 drm/nouveau/nvkm/subdev/bios/volt.c              |  3 +
 drm/nouveau/nvkm/subdev/clk/base.c               | 37 +++++++++++
 drm/nouveau/nvkm/subdev/clk/gf100.c              |  2 +-
 drm/nouveau/nvkm/subdev/clk/gk104.c              |  2 +-
 drm/nouveau/nvkm/subdev/volt/base.c              | 18 +++++-
 10 files changed, 174 insertions(+), 5 deletions(-)
 create mode 100644 drm/nouveau/include/nvkm/subdev/bios/baseclock.h
 create mode 100644 drm/nouveau/nvkm/subdev/bios/baseclock.c

-- 
2.6.3

Karol Herbst

2015-Dec-01 16:42 UTC

head link

[Nouveau] [RFC PATCH 1/5] bios/volt: handle voltage table version 0x50 with 0ed header

Some Kepler cards have no usefull header in the voltage table, which means
nouveau has to read the voltages out of the entries directly.

This patch fixes volting issues on those cards enabling them to switch cstates
---
 drm/nouveau/nvkm/subdev/bios/volt.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drm/nouveau/nvkm/subdev/bios/volt.c
b/drm/nouveau/nvkm/subdev/bios/volt.c
index 6e0a336..fd2776b 100644
--- a/drm/nouveau/nvkm/subdev/bios/volt.c
+++ b/drm/nouveau/nvkm/subdev/bios/volt.c
@@ -142,7 +142,10 @@ nvbios_volt_entry_parse(struct nvkm_bios *bios, int idx, u8
*ver, u8 *len,
 		info->vid     = nvbios_rd08(bios, volt + 0x01) >> 2;
 		break;
 	case 0x40:
+		break;
 	case 0x50:
+		info->voltage = nvbios_rd32(bios, volt) & 0x001fffff;
+		info->vid     = idx;
 		break;
 	}
 	return volt;
-- 
2.6.3

Karol Herbst

2015-Dec-01 16:42 UTC

head link

[Nouveau] [RFC PATCH 2/5] clk: drop cstates with too high voltage

To support boosting, vbios' seems to have fasters cstate than the actual
base clock.
We should definitely drop all those with a higher voltage than the max voltage
stated in the vbios.

This fixes reclocking issues mostly on high-end kepler cards where the highest
cstates goes way beyond the max voltage supported
---
 drm/nouveau/include/nvkm/subdev/volt.h |  4 ++++
 drm/nouveau/nvkm/subdev/clk/base.c     |  6 ++++++
 drm/nouveau/nvkm/subdev/volt/base.c    | 18 ++++++++++++++++--
 3 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/drm/nouveau/include/nvkm/subdev/volt.h
b/drm/nouveau/include/nvkm/subdev/volt.h
index b458d04..32c1a22 100644
--- a/drm/nouveau/include/nvkm/subdev/volt.h
+++ b/drm/nouveau/include/nvkm/subdev/volt.h
@@ -12,8 +12,12 @@ struct nvkm_volt {
 		u32 uv;
 		u8 vid;
 	} vid[256];
+
+	u32 max_voltage;
+	u32 min_voltage;
 };
 
+int nvkm_volt_map(struct nvkm_volt *volt, u8 id);
 int nvkm_volt_get(struct nvkm_volt *);
 int nvkm_volt_set_id(struct nvkm_volt *, u8 id, int condition);
 
diff --git a/drm/nouveau/nvkm/subdev/clk/base.c
b/drm/nouveau/nvkm/subdev/clk/base.c
index dc8682c..d731bc3 100644
--- a/drm/nouveau/nvkm/subdev/clk/base.c
+++ b/drm/nouveau/nvkm/subdev/clk/base.c
@@ -138,16 +138,22 @@ static int
 nvkm_cstate_new(struct nvkm_clk *clk, int idx, struct nvkm_pstate *pstate)
 {
 	struct nvkm_bios *bios = clk->subdev.device->bios;
+	struct nvkm_volt *volt = clk->subdev.device->volt;
 	const struct nvkm_domain *domain = clk->domains;
 	struct nvkm_cstate *cstate = NULL;
 	struct nvbios_cstepX cstepX;
 	u8  ver, hdr;
 	u16 data;
+	int voltage;
 
 	data = nvbios_cstepXp(bios, idx, &ver, &hdr, &cstepX);
 	if (!data)
 		return -ENOENT;
 
+	voltage = nvkm_volt_map(volt, cstepX.voltage);
+	if (volt && (voltage > volt->max_voltage || voltage <
volt->min_voltage))
+		return -EINVAL;
+
 	cstate = kzalloc(sizeof(*cstate), GFP_KERNEL);
 	if (!cstate)
 		return -ENOMEM;
diff --git a/drm/nouveau/nvkm/subdev/volt/base.c
b/drm/nouveau/nvkm/subdev/volt/base.c
index 50b5649..7104168 100644
--- a/drm/nouveau/nvkm/subdev/volt/base.c
+++ b/drm/nouveau/nvkm/subdev/volt/base.c
@@ -65,7 +65,7 @@ nvkm_volt_set(struct nvkm_volt *volt, u32 uv)
 	return ret;
 }
 
-static int
+int
 nvkm_volt_map(struct nvkm_volt *volt, u8 id)
 {
 	struct nvkm_bios *bios = volt->subdev.device->bios;
@@ -120,6 +120,9 @@ nvkm_volt_parse_bios(struct nvkm_bios *bios, struct
nvkm_volt *volt)
 
 	data = nvbios_volt_parse(bios, &ver, &hdr, &cnt, &len,
&info);
 	if (data && info.vidmask && info.base && info.step) {
+		volt->min_voltage = info.min;
+		volt->max_voltage = info.max;
+
 		for (i = 0; i < info.vidmask + 1; i++) {
 			if (info.base >= info.min &&
 				info.base <= info.max) {
@@ -138,9 +141,18 @@ nvkm_volt_parse_bios(struct nvkm_bios *bios, struct
nvkm_volt *volt)
 				volt->vid[volt->vid_nr].uv = ivid.voltage;
 				volt->vid[volt->vid_nr].vid = ivid.vid;
 				volt->vid_nr++;
+
+				if (volt->min_voltage == 0)
+					volt->min_voltage = ivid.voltage;
+				else
+					volt->min_voltage = min(volt->min_voltage, ivid.voltage);
+				volt->max_voltage = max(volt->max_voltage, ivid.voltage);
 			}
 		}
 		volt->vid_mask = info.vidmask;
+	} else if (data && info.type == NVBIOS_VOLT_PWM) {
+		volt->min_voltage = info.base;
+		volt->max_voltage = info.base + info.pwm_range;
 	}
 }
 
@@ -181,8 +193,10 @@ nvkm_volt_ctor(const struct nvkm_volt_func *func, struct
nvkm_device *device,
 	volt->func = func;
 
 	/* Assuming the non-bios device should build the voltage table later */
-	if (bios)
+	if (bios) {
 		nvkm_volt_parse_bios(bios, volt);
+		nvkm_debug(&volt->subdev, "min: %iuv max: %iuv\n",
volt->min_voltage, volt->max_voltage);
+	}
 
 	if (volt->vid_nr) {
 		for (i = 0; i < volt->vid_nr; i++) {
-- 
2.6.3

Karol Herbst

2015-Dec-01 16:42 UTC

head link

[Nouveau] [RFC PATCH 3/5] nvbios: add parsing of BASE CLOCK table

this table is found mainly on kepler+ cards. Also some fermi cards have this
table.
There are some fermi with a currupt one (strange version number), which is
unusable though.

---
 drm/nouveau/include/nvkm/subdev/bios/baseclock.h | 23 +++++++
 drm/nouveau/nvkm/subdev/bios/Kbuild              |  1 +
 drm/nouveau/nvkm/subdev/bios/baseclock.c         | 79 ++++++++++++++++++++++++
 3 files changed, 103 insertions(+)
 create mode 100644 drm/nouveau/include/nvkm/subdev/bios/baseclock.h
 create mode 100644 drm/nouveau/nvkm/subdev/bios/baseclock.c

diff --git a/drm/nouveau/include/nvkm/subdev/bios/baseclock.h
b/drm/nouveau/include/nvkm/subdev/bios/baseclock.h
new file mode 100644
index 0000000..07ee355
--- /dev/null
+++ b/drm/nouveau/include/nvkm/subdev/bios/baseclock.h
@@ -0,0 +1,23 @@
+#ifndef __NVBIOS_BASECLOCK_H__
+#define __NVBIOS_BASECLOCK_H__
+struct nvbios_baseclock_header {
+	u16 offset;
+
+	u8 version;
+	u8 hlen;
+	u8 ecount;
+	u8 elen;
+	u8 scount;
+	u8 slen;
+
+	u8 base_entry;
+	u8 boost_entry;
+	u8 tdp_entry;
+};
+struct nvbios_baseclock_entry {
+	u8  pstate;
+	u16 clock_mhz;
+};
+int nvbios_baseclock_parse(struct nvkm_bios *, struct nvbios_baseclock_header
*);
+int nvbios_baseclock_get_entry(struct nvkm_bios *, struct
nvbios_baseclock_header *h, u8 idx, struct nvbios_baseclock_entry *);
+#endif
diff --git a/drm/nouveau/nvkm/subdev/bios/Kbuild
b/drm/nouveau/nvkm/subdev/bios/Kbuild
index 64730d5..a402755 100644
--- a/drm/nouveau/nvkm/subdev/bios/Kbuild
+++ b/drm/nouveau/nvkm/subdev/bios/Kbuild
@@ -1,4 +1,5 @@
 nvkm-y += nvkm/subdev/bios/base.o
+nvkm-y += nvkm/subdev/bios/baseclock.o
 nvkm-y += nvkm/subdev/bios/bit.o
 nvkm-y += nvkm/subdev/bios/boost.o
 nvkm-y += nvkm/subdev/bios/conn.o
diff --git a/drm/nouveau/nvkm/subdev/bios/baseclock.c
b/drm/nouveau/nvkm/subdev/bios/baseclock.c
new file mode 100644
index 0000000..31609e0
--- /dev/null
+++ b/drm/nouveau/nvkm/subdev/bios/baseclock.c
@@ -0,0 +1,79 @@
+/*
+ * Copyright 2015 Nouveau Community
+ *
+ * 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 COPYRIGHT HOLDER(S) OR AUTHOR(S) 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.
+ *
+ * Authors: Karol Herbst
+ */
+#include <subdev/bios.h>
+#include <subdev/bios/bit.h>
+#include <subdev/bios/baseclock.h>
+
+static u16
+nvbios_baseclock_offset(struct nvkm_bios *b)
+{
+	struct bit_entry bit_P;
+
+	if (!bit_entry(b, 'P', &bit_P)) {
+		if (bit_P.version == 2)
+			return nvbios_rd16(b, bit_P.offset + 0x38);
+	}
+
+	return 0x0000;
+}
+
+int nvbios_baseclock_parse(struct nvkm_bios *b, struct nvbios_baseclock_header
*h)
+{
+	if (!h)
+		return -EINVAL;
+
+	h->offset = nvbios_baseclock_offset(b);
+	if (!h->offset)
+		return -ENODEV;
+
+	h->version = nvbios_rd08(b, h->offset);
+	switch (h->version) {
+	case 0x10:
+		h->hlen    = nvbios_rd08(b, h->offset + 0x1);
+		h->elen    = nvbios_rd08(b, h->offset + 0x2);
+		h->slen    = nvbios_rd08(b, h->offset + 0x3);
+		h->scount  = nvbios_rd08(b, h->offset + 0x4);
+		h->ecount  = nvbios_rd08(b, h->offset + 0x5);
+
+		h->base_entry  = nvbios_rd08(b, h->offset + 0x0f);
+		h->boost_entry = nvbios_rd08(b, h->offset + 0x10);
+		h->tdp_entry   = nvbios_rd08(b, h->offset + 0x11);
+		return 0;
+	default:
+		return -EINVAL;
+	}
+}
+
+int nvbios_baseclock_get_entry(struct nvkm_bios *b, struct
nvbios_baseclock_header *h, u8 idx, struct nvbios_baseclock_entry *e)
+{
+	u16 offset;
+
+	if (!e || !h)
+		return -EINVAL;
+
+	offset = h->offset + h->hlen + idx * (h->elen + (h->slen *
h->scount));
+	e->pstate    = nvbios_rd08(b, offset);
+	e->clock_mhz = nvbios_rd16(b, offset + 0x5);
+	return 0;
+}
-- 
2.6.3

Karol Herbst

2015-Dec-01 16:42 UTC

head link

[Nouveau] [RFC PATCH 4/5] subdev/clk: print the base clocks

this is just a nice thing to know and there is no harm in printing them
---
 drm/nouveau/nvkm/subdev/clk/base.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drm/nouveau/nvkm/subdev/clk/base.c
b/drm/nouveau/nvkm/subdev/clk/base.c
index d731bc3..df9173e 100644
--- a/drm/nouveau/nvkm/subdev/clk/base.c
+++ b/drm/nouveau/nvkm/subdev/clk/base.c
@@ -24,6 +24,7 @@
 #include "priv.h"
 
 #include <subdev/bios.h>
+#include <subdev/bios/baseclock.h>
 #include <subdev/bios/boost.h>
 #include <subdev/bios/cstep.h>
 #include <subdev/bios/perf.h>
@@ -562,10 +563,25 @@ int
 nvkm_clk_ctor(const struct nvkm_clk_func *func, struct nvkm_device *device,
 	      int index, bool allow_reclock, struct nvkm_clk *clk)
 {
+	struct nvkm_bios *bios;
 	int ret, idx, arglen;
 	const char *mode;
+	struct nvbios_baseclock_header header;
 
 	nvkm_subdev_ctor(&nvkm_clk, device, index, 0, &clk->subdev);
+	bios = device->bios;
+
+	if (bios && !nvbios_baseclock_parse(bios, &header)) {
+		struct nvbios_baseclock_entry base_entry, boost_entry;
+		if (nvbios_baseclock_get_entry(bios, &header, header.base_entry,
&base_entry))
+			nvkm_error(&clk->subdev, "couldn't parse base
clock\n");
+		else if (nvbios_baseclock_get_entry(bios, &header, header.boost_entry,
&boost_entry))
+			nvkm_error(&clk->subdev, "couldn't parse boost
clock\n");
+		else
+			nvkm_info(&clk->subdev, "base: %i MHz, boost: %i MHz\n",
+				base_entry.clock_mhz / 2, boost_entry.clock_mhz / 2);
+	}
+
 	clk->func = func;
 	INIT_LIST_HEAD(&clk->states);
 	clk->domains = func->domains;
-- 
2.6.3

Karol Herbst

2015-Dec-01 16:42 UTC

head link

[Nouveau] [RFC PATCH 5/5] clk: allow boosting only when NvBoost is set

0: disable boosting (cap to base clock from the vbios)
1: boost only to boost clock from the vbios
2: boost to max clock available
---
 drm/nouveau/include/nvkm/subdev/clk.h | 10 +++++++++-
 drm/nouveau/nvkm/subdev/clk/base.c    | 17 ++++++++++++++++-
 drm/nouveau/nvkm/subdev/clk/gf100.c   |  2 +-
 drm/nouveau/nvkm/subdev/clk/gk104.c   |  2 +-
 4 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/drm/nouveau/include/nvkm/subdev/clk.h
b/drm/nouveau/include/nvkm/subdev/clk.h
index 8708f0a..8085d81 100644
--- a/drm/nouveau/include/nvkm/subdev/clk.h
+++ b/drm/nouveau/include/nvkm/subdev/clk.h
@@ -64,7 +64,8 @@ struct nvkm_pstate {
 struct nvkm_domain {
 	enum nv_clk_src name;
 	u8 bios; /* 0xff for none */
-#define NVKM_CLK_DOM_FLAG_CORE 0x01
+#define NVKM_CLK_DOM_FLAG_CORE            0x01
+#define NVKM_CLK_DOM_FLAG_BASE_CLOCK_CORE 0x02
 	u8 flags;
 	const char *mname;
 	int mdiv;
@@ -94,6 +95,13 @@ struct nvkm_clk {
 	int dstate; /* display adjustment (min+) */
 
 	bool allow_reclock;
+#define NVKM_CLK_BOOST_MODE_NONE 0x0
+#define NVKM_CLK_BOOST_MODE_AVG  0x1
+#define NVKM_CLK_BOOST_MODE_FULL 0x2
+	u8 boost_mode;
+
+	u32 base_clock;
+	u32 boost_clock;
 
 	/*XXX: die, these are here *only* to support the completely
 	 *     bat-shit insane what-was-nouveau_hw.c code
diff --git a/drm/nouveau/nvkm/subdev/clk/base.c
b/drm/nouveau/nvkm/subdev/clk/base.c
index df9173e..ae76601 100644
--- a/drm/nouveau/nvkm/subdev/clk/base.c
+++ b/drm/nouveau/nvkm/subdev/clk/base.c
@@ -166,6 +166,12 @@ nvkm_cstate_new(struct nvkm_clk *clk, int idx, struct
nvkm_pstate *pstate)
 		if (domain->flags & NVKM_CLK_DOM_FLAG_CORE) {
 			u32 freq = nvkm_clk_adjust(clk, true, pstate->pstate,
 						   domain->bios, cstepX.freq);
+			if (domain->flags & NVKM_CLK_DOM_FLAG_BASE_CLOCK_CORE) {
+				if (clk->boost_mode == 0 && freq > clk->base_clock)
+					goto err;
+				if (clk->boost_mode == 1 && freq > clk->boost_clock)
+					goto err;
+			}
 			cstate->domain[domain->name] = freq;
 		}
 		domain++;
@@ -173,6 +179,9 @@ nvkm_cstate_new(struct nvkm_clk *clk, int idx, struct
nvkm_pstate *pstate)
 
 	list_add(&cstate->head, &pstate->list);
 	return 0;
+err:
+	kfree(cstate);
+	return -EINVAL;
 }
 
 /******************************************************************************
@@ -573,13 +582,19 @@ nvkm_clk_ctor(const struct nvkm_clk_func *func, struct
nvkm_device *device,
 
 	if (bios && !nvbios_baseclock_parse(bios, &header)) {
 		struct nvbios_baseclock_entry base_entry, boost_entry;
+		clk->boost_mode = nvkm_longopt(device->cfgopt, "NvBoost", 0);
 		if (nvbios_baseclock_get_entry(bios, &header, header.base_entry,
&base_entry))
 			nvkm_error(&clk->subdev, "couldn't parse base
clock\n");
 		else if (nvbios_baseclock_get_entry(bios, &header, header.boost_entry,
&boost_entry))
 			nvkm_error(&clk->subdev, "couldn't parse boost
clock\n");
-		else
+		else {
+			clk->base_clock = base_entry.clock_mhz * 1000;
+			clk->boost_clock = boost_entry.clock_mhz * 1000;
 			nvkm_info(&clk->subdev, "base: %i MHz, boost: %i MHz\n",
 				base_entry.clock_mhz / 2, boost_entry.clock_mhz / 2);
+		}
+	} else {
+		clk->boost_mode = -1;
 	}
 
 	clk->func = func;
diff --git a/drm/nouveau/nvkm/subdev/clk/gf100.c
b/drm/nouveau/nvkm/subdev/clk/gf100.c
index a52b7e7..eaf4f83 100644
--- a/drm/nouveau/nvkm/subdev/clk/gf100.c
+++ b/drm/nouveau/nvkm/subdev/clk/gf100.c
@@ -443,7 +443,7 @@ gf100_clk = {
 		{ nv_clk_src_hubk06 , 0x00 },
 		{ nv_clk_src_hubk01 , 0x01 },
 		{ nv_clk_src_copy   , 0x02 },
-		{ nv_clk_src_gpc    , 0x03, 0, "core", 2000 },
+		{ nv_clk_src_gpc    , 0x03, NVKM_CLK_DOM_FLAG_BASE_CLOCK_CORE,
"core", 2000 },
 		{ nv_clk_src_rop    , 0x04 },
 		{ nv_clk_src_mem    , 0x05, 0, "memory", 1000 },
 		{ nv_clk_src_vdec   , 0x06 },
diff --git a/drm/nouveau/nvkm/subdev/clk/gk104.c
b/drm/nouveau/nvkm/subdev/clk/gk104.c
index 396f7e4..f194112 100644
--- a/drm/nouveau/nvkm/subdev/clk/gk104.c
+++ b/drm/nouveau/nvkm/subdev/clk/gk104.c
@@ -485,7 +485,7 @@ gk104_clk = {
 	.domains = {
 		{ nv_clk_src_crystal, 0xff },
 		{ nv_clk_src_href   , 0xff },
-		{ nv_clk_src_gpc    , 0x00, NVKM_CLK_DOM_FLAG_CORE, "core", 2000 },
+		{ nv_clk_src_gpc    , 0x00, NVKM_CLK_DOM_FLAG_CORE |
NVKM_CLK_DOM_FLAG_BASE_CLOCK_CORE, "core", 2000 },
 		{ nv_clk_src_hubk07 , 0x01, NVKM_CLK_DOM_FLAG_CORE },
 		{ nv_clk_src_rop    , 0x02, NVKM_CLK_DOM_FLAG_CORE },
 		{ nv_clk_src_mem    , 0x03, 0, "memory", 500 },
-- 
2.6.3

Pierre Moreau

2015-Dec-02 01:26 UTC

head link

[Nouveau] [RFC PATCH 5/5] clk: allow boosting only when NvBoost is set

Hi Karol,

I have some comments below.

On 05:42 PM - Dec 01 2015, Karol Herbst wrote:> 0: disable boosting (cap to base clock from the vbios)
> 1: boost only to boost clock from the vbios
> 2: boost to max clock available
> ---
>  drm/nouveau/include/nvkm/subdev/clk.h | 10 +++++++++-
>  drm/nouveau/nvkm/subdev/clk/base.c    | 17 ++++++++++++++++-
>  drm/nouveau/nvkm/subdev/clk/gf100.c   |  2 +-
>  drm/nouveau/nvkm/subdev/clk/gk104.c   |  2 +-
>  4 files changed, 27 insertions(+), 4 deletions(-)
> 
> diff --git a/drm/nouveau/include/nvkm/subdev/clk.h
b/drm/nouveau/include/nvkm/subdev/clk.h
> index 8708f0a..8085d81 100644
> --- a/drm/nouveau/include/nvkm/subdev/clk.h
> +++ b/drm/nouveau/include/nvkm/subdev/clk.h
> @@ -64,7 +64,8 @@ struct nvkm_pstate {
>  struct nvkm_domain {
>  	enum nv_clk_src name;
>  	u8 bios; /* 0xff for none */
> -#define NVKM_CLK_DOM_FLAG_CORE 0x01
> +#define NVKM_CLK_DOM_FLAG_CORE            0x01
> +#define NVKM_CLK_DOM_FLAG_BASE_CLOCK_CORE 0x02
>  	u8 flags;
>  	const char *mname;
>  	int mdiv;
> @@ -94,6 +95,13 @@ struct nvkm_clk {
>  	int dstate; /* display adjustment (min+) */
>  
>  	bool allow_reclock;
> +#define NVKM_CLK_BOOST_MODE_NONE 0x0
> +#define NVKM_CLK_BOOST_MODE_AVG  0x1
> +#define NVKM_CLK_BOOST_MODE_FULL 0x2
> +	u8 boost_mode;
> +
> +	u32 base_clock;
> +	u32 boost_clock;
>  
>  	/*XXX: die, these are here *only* to support the completely
>  	 *     bat-shit insane what-was-nouveau_hw.c code
> diff --git a/drm/nouveau/nvkm/subdev/clk/base.c
b/drm/nouveau/nvkm/subdev/clk/base.c
> index df9173e..ae76601 100644
> --- a/drm/nouveau/nvkm/subdev/clk/base.c
> +++ b/drm/nouveau/nvkm/subdev/clk/base.c
> @@ -166,6 +166,12 @@ nvkm_cstate_new(struct nvkm_clk *clk, int idx, struct
nvkm_pstate *pstate)
>  		if (domain->flags & NVKM_CLK_DOM_FLAG_CORE) {
>  			u32 freq = nvkm_clk_adjust(clk, true, pstate->pstate,
>  						   domain->bios, cstepX.freq);
> +			if (domain->flags & NVKM_CLK_DOM_FLAG_BASE_CLOCK_CORE) {
> +				if (clk->boost_mode == 0 && freq > clk->base_clock)
> +					goto err;
> +				if (clk->boost_mode == 1 && freq > clk->boost_clock)
> +					goto err;
> +			}
>  			cstate->domain[domain->name] = freq;
>  		}
>  		domain++;
> @@ -173,6 +179,9 @@ nvkm_cstate_new(struct nvkm_clk *clk, int idx, struct
nvkm_pstate *pstate)
>  
>  	list_add(&cstate->head, &pstate->list);
>  	return 0;
> +err:
> +	kfree(cstate);
> +	return -EINVAL;
>  }
>  
> 
/******************************************************************************
> @@ -573,13 +582,19 @@ nvkm_clk_ctor(const struct nvkm_clk_func *func,
struct nvkm_device *device,
>  
>  	if (bios && !nvbios_baseclock_parse(bios, &header)) {
>  		struct nvbios_baseclock_entry base_entry, boost_entry;
> +		clk->boost_mode = nvkm_longopt(device->cfgopt,
"NvBoost", 0);
>  		if (nvbios_baseclock_get_entry(bios, &header, header.base_entry,
&base_entry))
If `boost_mode == -1` is some "went wrong, don't use" value,
shouldn't you set
`clk->boost_mode = -1;` here too?
>  			nvkm_error(&clk->subdev, "couldn't parse base
clock\n");
>  		else if (nvbios_baseclock_get_entry(bios, &header,
header.boost_entry, &boost_entry))
Same comment as above
>  			nvkm_error(&clk->subdev, "couldn't parse boost
clock\n");
> -		else
> +		else {
> +			clk->base_clock = base_entry.clock_mhz * 1000;
> +			clk->boost_clock = boost_entry.clock_mhz * 1000;
>  			nvkm_info(&clk->subdev, "base: %i MHz, boost: %i
MHz\n",
>  				base_entry.clock_mhz / 2, boost_entry.clock_mhz / 2);
> +		}
> +	} else {
> +		clk->boost_mode = -1;
You only listed values 0, 1, 2 in the commit message, so what is -1 for?
Disabling nvboost is already taken care by 0, so it's something else. runpm
uses it for auto mode, but here you set it if the bios wasn't found or
couldn't
be parsed, so not an auto mode. :-D


Regards,
Pierre

>  	}
>  
>  	clk->func = func;
> diff --git a/drm/nouveau/nvkm/subdev/clk/gf100.c
b/drm/nouveau/nvkm/subdev/clk/gf100.c
> index a52b7e7..eaf4f83 100644
> --- a/drm/nouveau/nvkm/subdev/clk/gf100.c
> +++ b/drm/nouveau/nvkm/subdev/clk/gf100.c
> @@ -443,7 +443,7 @@ gf100_clk = {
>  		{ nv_clk_src_hubk06 , 0x00 },
>  		{ nv_clk_src_hubk01 , 0x01 },
>  		{ nv_clk_src_copy   , 0x02 },
> -		{ nv_clk_src_gpc    , 0x03, 0, "core", 2000 },
> +		{ nv_clk_src_gpc    , 0x03, NVKM_CLK_DOM_FLAG_BASE_CLOCK_CORE,
"core", 2000 },
>  		{ nv_clk_src_rop    , 0x04 },
>  		{ nv_clk_src_mem    , 0x05, 0, "memory", 1000 },
>  		{ nv_clk_src_vdec   , 0x06 },
> diff --git a/drm/nouveau/nvkm/subdev/clk/gk104.c
b/drm/nouveau/nvkm/subdev/clk/gk104.c
> index 396f7e4..f194112 100644
> --- a/drm/nouveau/nvkm/subdev/clk/gk104.c
> +++ b/drm/nouveau/nvkm/subdev/clk/gk104.c
> @@ -485,7 +485,7 @@ gk104_clk = {
>  	.domains = {
>  		{ nv_clk_src_crystal, 0xff },
>  		{ nv_clk_src_href   , 0xff },
> -		{ nv_clk_src_gpc    , 0x00, NVKM_CLK_DOM_FLAG_CORE, "core",
2000 },
> +		{ nv_clk_src_gpc    , 0x00, NVKM_CLK_DOM_FLAG_CORE |
NVKM_CLK_DOM_FLAG_BASE_CLOCK_CORE, "core", 2000 },
>  		{ nv_clk_src_hubk07 , 0x01, NVKM_CLK_DOM_FLAG_CORE },
>  		{ nv_clk_src_rop    , 0x02, NVKM_CLK_DOM_FLAG_CORE },
>  		{ nv_clk_src_mem    , 0x03, 0, "memory", 500 },
> -- 
> 2.6.3
> 
> _______________________________________________
> Nouveau mailing list
> Nouveau at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/nouveau

Pierre Moreau

2015-Dec-02 01:34 UTC

head link

[Nouveau] [RFC PATCH 4/5] subdev/clk: print the base clocks

Hi,

On 05:42 PM - Dec 01 2015, Karol Herbst wrote:> this is just a nice thing to know and there is no harm in printing them
> ---
>  drm/nouveau/nvkm/subdev/clk/base.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/drm/nouveau/nvkm/subdev/clk/base.c
b/drm/nouveau/nvkm/subdev/clk/base.c
> index d731bc3..df9173e 100644
> --- a/drm/nouveau/nvkm/subdev/clk/base.c
> +++ b/drm/nouveau/nvkm/subdev/clk/base.c
> @@ -24,6 +24,7 @@
>  #include "priv.h"
>  
>  #include <subdev/bios.h>
> +#include <subdev/bios/baseclock.h>
>  #include <subdev/bios/boost.h>
>  #include <subdev/bios/cstep.h>
>  #include <subdev/bios/perf.h>
> @@ -562,10 +563,25 @@ int
>  nvkm_clk_ctor(const struct nvkm_clk_func *func, struct nvkm_device
*device,
>  	      int index, bool allow_reclock, struct nvkm_clk *clk)
>  {
> +	struct nvkm_bios *bios;
>  	int ret, idx, arglen;
>  	const char *mode;
> +	struct nvbios_baseclock_header header;
>  
>  	nvkm_subdev_ctor(&nvkm_clk, device, index, 0, &clk->subdev);
> +	bios = device->bios;
> +
> +	if (bios && !nvbios_baseclock_parse(bios, &header)) {
> +		struct nvbios_baseclock_entry base_entry, boost_entry;
> +		if (nvbios_baseclock_get_entry(bios, &header, header.base_entry,
&base_entry))
> +			nvkm_error(&clk->subdev, "couldn't parse base
clock\n");
> +		else if (nvbios_baseclock_get_entry(bios, &header,
header.boost_entry, &boost_entry))
> +			nvkm_error(&clk->subdev, "couldn't parse boost
clock\n");
> +		else
> +			nvkm_info(&clk->subdev, "base: %i MHz, boost: %i
MHz\n",
> +				base_entry.clock_mhz / 2, boost_entry.clock_mhz / 2);
This is probably just me missing some elementary electronic knowledge about
clocks, but why do you divide the clock frequency by two?

Regards,
Pierre
> +	}
> +
>  	clk->func = func;
>  	INIT_LIST_HEAD(&clk->states);
>  	clk->domains = func->domains;
> -- 
> 2.6.3
> 
> _______________________________________________
> Nouveau mailing list
> Nouveau at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/nouveau

Maybe Matching Threads

Search for more reasonably related threads

Nouveau - Dec 2015 - [RFC PATCH 4/5] subdev/clk: print the base clocks

[Nouveau] [RFC PATCH 0/5] stabilize kepler reclocking

[Nouveau] [RFC PATCH 1/5] bios/volt: handle voltage table version 0x50 with 0ed header

[Nouveau] [RFC PATCH 2/5] clk: drop cstates with too high voltage

[Nouveau] [RFC PATCH 3/5] nvbios: add parsing of BASE CLOCK table

[Nouveau] [RFC PATCH 4/5] subdev/clk: print the base clocks

[Nouveau] [RFC PATCH 5/5] clk: allow boosting only when NvBoost is set

[Nouveau] [RFC PATCH 5/5] clk: allow boosting only when NvBoost is set

[Nouveau] [RFC PATCH 4/5] subdev/clk: print the base clocks

Maybe Matching Threads