thr3ads.net - Nouveau - [Nouveau] [PATCH 9/9] clk: Check pm_runtime status before reclocking [Mar 2017]

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

Karol Herbst

2017-Mar-05 16:34 UTC

[Nouveau] [PATCH 0/9] clk subdev updates

This series addresses various issues inside the reclocking code:
1. after resume the set clocks are reset
2. reclocking not possible while GPU is suspended
3. nouveau always does full reclocks even if only a change of the voltage is
   required

Some of the patches were part of the bigger reclocking series I sent months
ago, some things have changed though.

This is also preparation work of changing the clock state due to temperature
changes and dynamic reclocking.

Karol Herbst (9):
  clk: Rename nvkm_pstate_calc to nvkm_clk_update and export it
  clk: Remove dstate
  clk: Make pstate a pointer to nvkm_pstate
  clk: Hold information about the current cstate status
  clk: We should pass the pstate id around not the index in the list
  clk: Split out update code to nv40
  clk: Only do partial reclocks as required
  clk: Set clocks to pre suspend state after suspend
  clk: Check pm_runtime status before reclocking

 drm/nouveau/include/nvkm/subdev/clk.h |   9 +-
 drm/nouveau/nouveau_debugfs.c         |   6 +-
 drm/nouveau/nvkm/engine/device/ctrl.c |   5 +-
 drm/nouveau/nvkm/subdev/clk/base.c    | 192 ++++++++++++++++++++--------------
 drm/nouveau/nvkm/subdev/clk/g84.c     |   1 +
 drm/nouveau/nvkm/subdev/clk/gf100.c   |  63 +++++++++++
 drm/nouveau/nvkm/subdev/clk/gk104.c   |   1 +
 drm/nouveau/nvkm/subdev/clk/gk20a.c   |   1 +
 drm/nouveau/nvkm/subdev/clk/gm20b.c   |   1 +
 drm/nouveau/nvkm/subdev/clk/gt215.c   |   1 +
 drm/nouveau/nvkm/subdev/clk/mcp77.c   |   1 +
 drm/nouveau/nvkm/subdev/clk/nv40.c    |  18 ++++
 drm/nouveau/nvkm/subdev/clk/nv50.c    |   1 +
 drm/nouveau/nvkm/subdev/clk/priv.h    |  13 +++
 drm/nouveau/nvkm/subdev/pmu/gk20a.c   |  18 ++--
 15 files changed, 232 insertions(+), 99 deletions(-)

-- 
2.12.0

Karol Herbst

2017-Mar-05 16:34 UTC

head link

[Nouveau] [PATCH 1/9] clk: Rename nvkm_pstate_calc to nvkm_clk_update and export it

This function will be used to update the current clock state.

This will happen for various reasons:
  * Temperature changes
  * User changes clocking state
  * Load changes

Signed-off-by: Karol Herbst <karolherbst at gmail.com>
---
 drm/nouveau/include/nvkm/subdev/clk.h |  1 +
 drm/nouveau/nvkm/subdev/clk/base.c    | 26 ++++++++++++++++----------
 2 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/drm/nouveau/include/nvkm/subdev/clk.h
b/drm/nouveau/include/nvkm/subdev/clk.h
index e5275f74..b2c94cd5 100644
--- a/drm/nouveau/include/nvkm/subdev/clk.h
+++ b/drm/nouveau/include/nvkm/subdev/clk.h
@@ -123,6 +123,7 @@ int nvkm_clk_ustate(struct nvkm_clk *, int req, int pwr);
 int nvkm_clk_astate(struct nvkm_clk *, int req, int rel, bool wait);
 int nvkm_clk_dstate(struct nvkm_clk *, int req, int rel);
 int nvkm_clk_tstate(struct nvkm_clk *, u8 temperature);
+int nvkm_clk_update(struct nvkm_clk *clk, bool wait);
 
 int nv04_clk_new(struct nvkm_device *, int, struct nvkm_clk **);
 int nv40_clk_new(struct nvkm_device *, int, struct nvkm_clk **);
diff --git a/drm/nouveau/nvkm/subdev/clk/base.c
b/drm/nouveau/nvkm/subdev/clk/base.c
index e4c8d310..ecff3ff3 100644
--- a/drm/nouveau/nvkm/subdev/clk/base.c
+++ b/drm/nouveau/nvkm/subdev/clk/base.c
@@ -296,7 +296,7 @@ nvkm_pstate_prog(struct nvkm_clk *clk, int pstatei)
 }
 
 static void
-nvkm_pstate_work(struct work_struct *work)
+nvkm_clk_update_work(struct work_struct *work)
 {
 	struct nvkm_clk *clk = container_of(work, typeof(*clk), work);
 	struct nvkm_subdev *subdev = &clk->subdev;
@@ -332,9 +332,15 @@ nvkm_pstate_work(struct work_struct *work)
 	nvkm_notify_get(&clk->pwrsrc_ntfy);
 }
 
-static int
-nvkm_pstate_calc(struct nvkm_clk *clk, bool wait)
+int
+nvkm_clk_update(struct nvkm_clk *clk, bool wait)
 {
+	if (!clk)
+		return -EINVAL;
+
+	if (!clk->allow_reclock)
+		return -ENODEV;
+
 	atomic_set(&clk->waiting, 1);
 	schedule_work(&clk->work);
 	if (wait)
@@ -524,7 +530,7 @@ nvkm_clk_ustate(struct nvkm_clk *clk, int req, int pwr)
 	if (ret >= 0) {
 		if (ret -= 2, pwr) clk->ustate_ac = ret;
 		else		   clk->ustate_dc = ret;
-		return nvkm_pstate_calc(clk, true);
+		return nvkm_clk_update(clk, true);
 	}
 	return ret;
 }
@@ -536,7 +542,7 @@ nvkm_clk_astate(struct nvkm_clk *clk, int req, int rel, bool
wait)
 	if ( rel) clk->astate += rel;
 	clk->astate = min(clk->astate, clk->state_nr - 1);
 	clk->astate = max(clk->astate, 0);
-	return nvkm_pstate_calc(clk, wait);
+	return nvkm_clk_update(clk, wait);
 }
 
 int
@@ -545,7 +551,7 @@ nvkm_clk_tstate(struct nvkm_clk *clk, u8 temp)
 	if (clk->temp == temp)
 		return 0;
 	clk->temp = temp;
-	return nvkm_pstate_calc(clk, false);
+	return nvkm_clk_update(clk, false);
 }
 
 int
@@ -555,7 +561,7 @@ nvkm_clk_dstate(struct nvkm_clk *clk, int req, int rel)
 	if ( rel) clk->dstate += rel;
 	clk->dstate = min(clk->dstate, clk->state_nr - 1);
 	clk->dstate = max(clk->dstate, 0);
-	return nvkm_pstate_calc(clk, true);
+	return nvkm_clk_update(clk, true);
 }
 
 static int
@@ -563,7 +569,7 @@ nvkm_clk_pwrsrc(struct nvkm_notify *notify)
 {
 	struct nvkm_clk *clk  		container_of(notify, typeof(*clk), pwrsrc_ntfy);
-	nvkm_pstate_calc(clk, false);
+	nvkm_clk_update(clk, false);
 	return NVKM_NOTIFY_DROP;
 }
 
@@ -618,7 +624,7 @@ nvkm_clk_init(struct nvkm_subdev *subdev)
 	clk->dstate = 0;
 	clk->pstate = -1;
 	clk->temp = 90; /* reasonable default value */
-	nvkm_pstate_calc(clk, true);
+	nvkm_clk_update(clk, true);
 	return 0;
 }
 
@@ -675,7 +681,7 @@ nvkm_clk_ctor(const struct nvkm_clk_func *func, struct
nvkm_device *device,
 	clk->ustate_dc = -1;
 	clk->allow_reclock = allow_reclock;
 
-	INIT_WORK(&clk->work, nvkm_pstate_work);
+	INIT_WORK(&clk->work, nvkm_clk_update_work);
 	init_waitqueue_head(&clk->wait);
 	atomic_set(&clk->waiting, 0);
 
-- 
2.12.0

Karol Herbst

2017-Mar-05 16:35 UTC

head link

[Nouveau] [PATCH 2/9] clk: Remove dstate

We won't need it now, because we will adjust the clocks depending on engine
loads later on anyway or a static lockup table. It also simplifies the
clocking logic.

This code was nowhere used anyway and just a mock up.

Signed-off-by: Karol Herbst <karolherbst at gmail.com>
Reviewed-by: Martin Peres <martin.peres at free.fr>
---
 drm/nouveau/include/nvkm/subdev/clk.h |  2 --
 drm/nouveau/nvkm/subdev/clk/base.c    | 16 ++--------------
 2 files changed, 2 insertions(+), 16 deletions(-)

diff --git a/drm/nouveau/include/nvkm/subdev/clk.h
b/drm/nouveau/include/nvkm/subdev/clk.h
index b2c94cd5..69942b14 100644
--- a/drm/nouveau/include/nvkm/subdev/clk.h
+++ b/drm/nouveau/include/nvkm/subdev/clk.h
@@ -99,7 +99,6 @@ struct nvkm_clk {
 	int ustate_ac; /* user-requested (-1 disabled, -2 perfmon) */
 	int ustate_dc; /* user-requested (-1 disabled, -2 perfmon) */
 	int astate; /* perfmon adjustment (base) */
-	int dstate; /* display adjustment (min+) */
 	u8  temp;
 
 	bool allow_reclock;
@@ -121,7 +120,6 @@ struct nvkm_clk {
 int nvkm_clk_read(struct nvkm_clk *, enum nv_clk_src);
 int nvkm_clk_ustate(struct nvkm_clk *, int req, int pwr);
 int nvkm_clk_astate(struct nvkm_clk *, int req, int rel, bool wait);
-int nvkm_clk_dstate(struct nvkm_clk *, int req, int rel);
 int nvkm_clk_tstate(struct nvkm_clk *, u8 temperature);
 int nvkm_clk_update(struct nvkm_clk *clk, bool wait);
 
diff --git a/drm/nouveau/nvkm/subdev/clk/base.c
b/drm/nouveau/nvkm/subdev/clk/base.c
index ecff3ff3..07d530ed 100644
--- a/drm/nouveau/nvkm/subdev/clk/base.c
+++ b/drm/nouveau/nvkm/subdev/clk/base.c
@@ -306,15 +306,14 @@ nvkm_clk_update_work(struct work_struct *work)
 		return;
 	clk->pwrsrc = power_supply_is_system_supplied();
 
-	nvkm_trace(subdev, "P %d PWR %d U(AC) %d U(DC) %d A %d T %d°C D
%d\n",
+	nvkm_trace(subdev, "P %d PWR %d U(AC) %d U(DC) %d A %d T %d°C\n",
 		   clk->pstate, clk->pwrsrc, clk->ustate_ac, clk->ustate_dc,
-		   clk->astate, clk->temp, clk->dstate);
+		   clk->astate, clk->temp);
 
 	pstate = clk->pwrsrc ? clk->ustate_ac : clk->ustate_dc;
 	if (clk->state_nr && pstate != -1) {
 		pstate = (pstate < 0) ? clk->astate : pstate;
 		pstate = min(pstate, clk->state_nr - 1);
-		pstate = max(pstate, clk->dstate);
 	} else {
 		pstate = clk->pstate = -1;
 	}
@@ -554,16 +553,6 @@ nvkm_clk_tstate(struct nvkm_clk *clk, u8 temp)
 	return nvkm_clk_update(clk, false);
 }
 
-int
-nvkm_clk_dstate(struct nvkm_clk *clk, int req, int rel)
-{
-	if (!rel) clk->dstate  = req;
-	if ( rel) clk->dstate += rel;
-	clk->dstate = min(clk->dstate, clk->state_nr - 1);
-	clk->dstate = max(clk->dstate, 0);
-	return nvkm_clk_update(clk, true);
-}
-
 static int
 nvkm_clk_pwrsrc(struct nvkm_notify *notify)
 {
@@ -621,7 +610,6 @@ nvkm_clk_init(struct nvkm_subdev *subdev)
 		return clk->func->init(clk);
 
 	clk->astate = clk->state_nr - 1;
-	clk->dstate = 0;
 	clk->pstate = -1;
 	clk->temp = 90; /* reasonable default value */
 	nvkm_clk_update(clk, true);
-- 
2.12.0

Karol Herbst

2017-Mar-05 16:35 UTC

head link

[Nouveau] [PATCH 3/9] clk: Make pstate a pointer to nvkm_pstate

We will access the current set cstate at least every second and this safes
us some CPU cycles looking them up every second.

Signed-off-by: Karol Herbst <karolherbst at gmail.com>
Reviewed-by: Martin Peres <martin.peres at free.fr>
---
 drm/nouveau/include/nvkm/subdev/clk.h |  4 +++-
 drm/nouveau/nvkm/engine/device/ctrl.c |  5 ++++-
 drm/nouveau/nvkm/subdev/clk/base.c    | 17 ++++++++++++-----
 drm/nouveau/nvkm/subdev/pmu/gk20a.c   | 18 +++++++-----------
 4 files changed, 26 insertions(+), 18 deletions(-)

diff --git a/drm/nouveau/include/nvkm/subdev/clk.h
b/drm/nouveau/include/nvkm/subdev/clk.h
index 69942b14..37263b7f 100644
--- a/drm/nouveau/include/nvkm/subdev/clk.h
+++ b/drm/nouveau/include/nvkm/subdev/clk.h
@@ -10,6 +10,8 @@ struct nvkm_pll_vals;
 #define NVKM_CLK_CSTATE_BASE    -2 /* pstate base */
 #define NVKM_CLK_CSTATE_HIGHEST -3 /* highest possible */
 
+#define NVKM_CLK_PSTATE_DEFAULT -1
+
 enum nv_clk_src {
 	nv_clk_src_crystal,
 	nv_clk_src_href,
@@ -95,7 +97,7 @@ struct nvkm_clk {
 
 	struct nvkm_notify pwrsrc_ntfy;
 	int pwrsrc;
-	int pstate; /* current */
+	struct nvkm_pstate *pstate; /* current */
 	int ustate_ac; /* user-requested (-1 disabled, -2 perfmon) */
 	int ustate_dc; /* user-requested (-1 disabled, -2 perfmon) */
 	int astate; /* perfmon adjustment (base) */
diff --git a/drm/nouveau/nvkm/engine/device/ctrl.c
b/drm/nouveau/nvkm/engine/device/ctrl.c
index b0ece71a..da70626c 100644
--- a/drm/nouveau/nvkm/engine/device/ctrl.c
+++ b/drm/nouveau/nvkm/engine/device/ctrl.c
@@ -52,7 +52,10 @@ nvkm_control_mthd_pstate_info(struct nvkm_control *ctrl, void
*data, u32 size)
 		args->v0.ustate_ac = clk->ustate_ac;
 		args->v0.ustate_dc = clk->ustate_dc;
 		args->v0.pwrsrc = clk->pwrsrc;
-		args->v0.pstate = clk->pstate;
+		if (clk->pstate)
+			args->v0.pstate = clk->pstate->pstate;
+		else
+			args->v0.pstate = NVKM_CLK_PSTATE_DEFAULT;
 	} else {
 		args->v0.count = 0;
 		args->v0.ustate_ac = NVIF_CONTROL_PSTATE_INFO_V0_USTATE_DISABLE;
diff --git a/drm/nouveau/nvkm/subdev/clk/base.c
b/drm/nouveau/nvkm/subdev/clk/base.c
index 07d530ed..0d4d9fdf 100644
--- a/drm/nouveau/nvkm/subdev/clk/base.c
+++ b/drm/nouveau/nvkm/subdev/clk/base.c
@@ -271,13 +271,16 @@ nvkm_pstate_prog(struct nvkm_clk *clk, int pstatei)
 	struct nvkm_pstate *pstate;
 	int ret, idx = 0;
 
+	if (pstatei == NVKM_CLK_PSTATE_DEFAULT)
+		return 0;
+
 	list_for_each_entry(pstate, &clk->states, head) {
 		if (idx++ == pstatei)
 			break;
 	}
 
 	nvkm_debug(subdev, "setting performance state %d\n", pstatei);
-	clk->pstate = pstatei;
+	clk->pstate = pstate;
 
 	nvkm_pcie_set_link(pci, pstate->pcie_speed, pstate->pcie_width);
 
@@ -306,8 +309,12 @@ nvkm_clk_update_work(struct work_struct *work)
 		return;
 	clk->pwrsrc = power_supply_is_system_supplied();
 
+	if (clk->pstate)
+		pstate = clk->pstate->pstate;
+	else
+		pstate = NVKM_CLK_PSTATE_DEFAULT;
 	nvkm_trace(subdev, "P %d PWR %d U(AC) %d U(DC) %d A %d T %d°C\n",
-		   clk->pstate, clk->pwrsrc, clk->ustate_ac, clk->ustate_dc,
+		   pstate, clk->pwrsrc, clk->ustate_ac, clk->ustate_dc,
 		   clk->astate, clk->temp);
 
 	pstate = clk->pwrsrc ? clk->ustate_ac : clk->ustate_dc;
@@ -315,11 +322,11 @@ nvkm_clk_update_work(struct work_struct *work)
 		pstate = (pstate < 0) ? clk->astate : pstate;
 		pstate = min(pstate, clk->state_nr - 1);
 	} else {
-		pstate = clk->pstate = -1;
+		pstate = NVKM_CLK_PSTATE_DEFAULT;
 	}
 
 	nvkm_trace(subdev, "-> %d\n", pstate);
-	if (pstate != clk->pstate) {
+	if (!clk->pstate || pstate != clk->pstate->pstate) {
 		int ret = nvkm_pstate_prog(clk, pstate);
 		if (ret) {
 			nvkm_error(subdev, "error setting pstate %d: %d\n",
@@ -610,7 +617,7 @@ nvkm_clk_init(struct nvkm_subdev *subdev)
 		return clk->func->init(clk);
 
 	clk->astate = clk->state_nr - 1;
-	clk->pstate = -1;
+	clk->pstate = NULL;
 	clk->temp = 90; /* reasonable default value */
 	nvkm_clk_update(clk, true);
 	return 0;
diff --git a/drm/nouveau/nvkm/subdev/pmu/gk20a.c
b/drm/nouveau/nvkm/subdev/pmu/gk20a.c
index 9ca0db79..43a9a74a 100644
--- a/drm/nouveau/nvkm/subdev/pmu/gk20a.c
+++ b/drm/nouveau/nvkm/subdev/pmu/gk20a.c
@@ -55,24 +55,22 @@ gk20a_pmu_dvfs_target(struct gk20a_pmu *pmu, int *state)
 	return nvkm_clk_astate(clk, *state, 0, false);
 }
 
-static void
-gk20a_pmu_dvfs_get_cur_state(struct gk20a_pmu *pmu, int *state)
-{
-	struct nvkm_clk *clk = pmu->base.subdev.device->clk;
-
-	*state = clk->pstate;
-}
-
 static int
 gk20a_pmu_dvfs_get_target_state(struct gk20a_pmu *pmu,
 				int *state, int load)
 {
 	struct gk20a_pmu_dvfs_data *data = pmu->data;
 	struct nvkm_clk *clk = pmu->base.subdev.device->clk;
+	struct nvkm_pstate *pstate = clk->pstate;
 	int cur_level, level;
 
+	if (!pstate) {
+		*state = 0;
+		return 1;
+	}
+
 	/* For GK20A, the performance level is directly mapped to pstate */
-	level = cur_level = clk->pstate;
+	level = cur_level = clk->pstate->pstate;
 
 	if (load > data->p_load_max) {
 		level = min(clk->state_nr - 1, level + (clk->state_nr / 3));
@@ -142,8 +140,6 @@ gk20a_pmu_dvfs_work(struct nvkm_alarm *alarm)
 	nvkm_trace(subdev, "utilization = %d %%, avg_load = %d %%\n",
 		   utilization, data->avg_load);
 
-	gk20a_pmu_dvfs_get_cur_state(pmu, &state);
-
 	if (gk20a_pmu_dvfs_get_target_state(pmu, &state, data->avg_load)) {
 		nvkm_trace(subdev, "set new state to %d\n", state);
 		gk20a_pmu_dvfs_target(pmu, &state);
-- 
2.12.0

Karol Herbst

2017-Mar-05 16:35 UTC

head link

[Nouveau] [PATCH 4/9] clk: Hold information about the current cstate status

Later we will have situations where the expected and the current state
isn't the same.

Signed-off-by: Karol Herbst <karolherbst at gmail.com>
Reviewed-by: Martin Peres <martin.peres at free.fr>
---
 drm/nouveau/include/nvkm/subdev/clk.h |  2 ++
 drm/nouveau/nvkm/subdev/clk/base.c    | 32 +++++++++++++++++++++++++-------
 2 files changed, 27 insertions(+), 7 deletions(-)

diff --git a/drm/nouveau/include/nvkm/subdev/clk.h
b/drm/nouveau/include/nvkm/subdev/clk.h
index 37263b7f..0e0af704 100644
--- a/drm/nouveau/include/nvkm/subdev/clk.h
+++ b/drm/nouveau/include/nvkm/subdev/clk.h
@@ -101,6 +101,8 @@ struct nvkm_clk {
 	int ustate_ac; /* user-requested (-1 disabled, -2 perfmon) */
 	int ustate_dc; /* user-requested (-1 disabled, -2 perfmon) */
 	int astate; /* perfmon adjustment (base) */
+	struct nvkm_cstate *cstate;
+	int exp_cstateid;
 	u8  temp;
 
 	bool allow_reclock;
diff --git a/drm/nouveau/nvkm/subdev/clk/base.c
b/drm/nouveau/nvkm/subdev/clk/base.c
index 0d4d9fdf..d37c13b7 100644
--- a/drm/nouveau/nvkm/subdev/clk/base.c
+++ b/drm/nouveau/nvkm/subdev/clk/base.c
@@ -146,9 +146,14 @@ static struct nvkm_cstate *
 nvkm_cstate_get(struct nvkm_clk *clk, struct nvkm_pstate *pstate, int cstatei)
 {
 	struct nvkm_cstate *cstate;
-	if (cstatei == NVKM_CLK_CSTATE_HIGHEST)
+	switch (cstatei) {
+	case NVKM_CLK_CSTATE_HIGHEST:
 		return list_last_entry(&pstate->list, typeof(*cstate), head);
-	else {
+	case NVKM_CLK_CSTATE_BASE:
+		return &pstate->base;
+	case NVKM_CLK_CSTATE_DEFAULT:
+		return NULL;
+	default:
 		list_for_each_entry(cstate, &pstate->list, head) {
 			if (cstate->id == cstatei)
 				return cstate;
@@ -167,6 +172,9 @@ nvkm_cstate_prog(struct nvkm_clk *clk, struct nvkm_pstate
*pstate, int cstatei)
 	struct nvkm_cstate *cstate;
 	int ret;
 
+	if (cstatei == NVKM_CLK_CSTATE_DEFAULT)
+		return 0;
+
 	if (!list_empty(&pstate->list)) {
 		cstate = nvkm_cstate_get(clk, pstate, cstatei);
 		cstate = nvkm_cstate_find_best(clk, pstate, cstate);
@@ -193,6 +201,7 @@ nvkm_cstate_prog(struct nvkm_clk *clk, struct nvkm_pstate
*pstate, int cstatei)
 
 	ret = clk->func->calc(clk, cstate);
 	if (ret == 0) {
+		clk->cstate = cstate;
 		ret = clk->func->prog(clk);
 		clk->func->tidy(clk);
 	}
@@ -295,7 +304,7 @@ nvkm_pstate_prog(struct nvkm_clk *clk, int pstatei)
 		ram->func->tidy(ram);
 	}
 
-	return nvkm_cstate_prog(clk, pstate, NVKM_CLK_CSTATE_HIGHEST);
+	return nvkm_cstate_prog(clk, pstate, clk->exp_cstateid);
 }
 
 static void
@@ -313,9 +322,9 @@ nvkm_clk_update_work(struct work_struct *work)
 		pstate = clk->pstate->pstate;
 	else
 		pstate = NVKM_CLK_PSTATE_DEFAULT;
-	nvkm_trace(subdev, "P %d PWR %d U(AC) %d U(DC) %d A %d T %d°C\n",
+	nvkm_trace(subdev, "P %d PWR %d U(AC) %d U(DC) %d A %d C %d T
%d°C\n",
 		   pstate, clk->pwrsrc, clk->ustate_ac, clk->ustate_dc,
-		   clk->astate, clk->temp);
+		   clk->astate, clk->exp_cstateid, clk->temp);
 
 	pstate = clk->pwrsrc ? clk->ustate_ac : clk->ustate_dc;
 	if (clk->state_nr && pstate != -1) {
@@ -536,6 +545,7 @@ nvkm_clk_ustate(struct nvkm_clk *clk, int req, int pwr)
 	if (ret >= 0) {
 		if (ret -= 2, pwr) clk->ustate_ac = ret;
 		else		   clk->ustate_dc = ret;
+		clk->exp_cstateid = NVKM_CLK_CSTATE_HIGHEST;
 		return nvkm_clk_update(clk, true);
 	}
 	return ret;
@@ -548,6 +558,7 @@ nvkm_clk_astate(struct nvkm_clk *clk, int req, int rel, bool
wait)
 	if ( rel) clk->astate += rel;
 	clk->astate = min(clk->astate, clk->state_nr - 1);
 	clk->astate = max(clk->astate, 0);
+	clk->exp_cstateid = NVKM_CLK_CSTATE_BASE;
 	return nvkm_clk_update(clk, wait);
 }
 
@@ -618,6 +629,8 @@ nvkm_clk_init(struct nvkm_subdev *subdev)
 
 	clk->astate = clk->state_nr - 1;
 	clk->pstate = NULL;
+	clk->exp_cstateid = NVKM_CLK_CSTATE_DEFAULT;
+	clk->cstate = NULL;
 	clk->temp = 90; /* reasonable default value */
 	nvkm_clk_update(clk, true);
 	return 0;
@@ -701,15 +714,20 @@ nvkm_clk_ctor(const struct nvkm_clk_func *func, struct
nvkm_device *device,
 	if (mode) {
 		clk->ustate_ac = nvkm_clk_nstate(clk, mode, arglen);
 		clk->ustate_dc = nvkm_clk_nstate(clk, mode, arglen);
+		clk->exp_cstateid = NVKM_CLK_CSTATE_HIGHEST;
 	}
 
 	mode = nvkm_stropt(device->cfgopt, "NvClkModeAC", &arglen);
-	if (mode)
+	if (mode) {
 		clk->ustate_ac = nvkm_clk_nstate(clk, mode, arglen);
+		clk->exp_cstateid = NVKM_CLK_CSTATE_HIGHEST;
+	}
 
 	mode = nvkm_stropt(device->cfgopt, "NvClkModeDC", &arglen);
-	if (mode)
+	if (mode) {
 		clk->ustate_dc = nvkm_clk_nstate(clk, mode, arglen);
+		clk->exp_cstateid = NVKM_CLK_CSTATE_HIGHEST;
+	}
 
 	clk->boost_mode = nvkm_longopt(device->cfgopt, "NvBoost",
 				       NVKM_CLK_BOOST_NONE);
-- 
2.12.0

Karol Herbst

2017-Mar-05 16:35 UTC

head link

[Nouveau] [PATCH 5/9] clk: We should pass the pstate id around not the index in the list

This makes the code easier, because we can compare the id with
pstate->pstate and safe us the trouble iterating over the entire pstate
list to match the index.

Signed-off-by: Karol Herbst <karolherbst at gmail.com>
Reviewed-by: Martin Peres <martin.peres at free.fr>
---
 drm/nouveau/nouveau_debugfs.c      |  6 +--
 drm/nouveau/nvkm/subdev/clk/base.c | 78 +++++++++++++++++++-------------------
 2 files changed, 41 insertions(+), 43 deletions(-)

diff --git a/drm/nouveau/nouveau_debugfs.c b/drm/nouveau/nouveau_debugfs.c
index fd64dfdc..b114a429 100644
--- a/drm/nouveau/nouveau_debugfs.c
+++ b/drm/nouveau/nouveau_debugfs.c
@@ -96,11 +96,11 @@ nouveau_debugfs_pstate_get(struct seq_file *m, void *data)
 		} while (attr.index);
 
 		if (state >= 0) {
-			if (info.ustate_ac == state)
+			if (info.ustate_ac == attr.state)
 				seq_printf(m, " AC");
-			if (info.ustate_dc == state)
+			if (info.ustate_dc == attr.state)
 				seq_printf(m, " DC");
-			if (info.pstate == state)
+			if (info.pstate == attr.state)
 				seq_printf(m, " *");
 		} else {
 			if (info.ustate_ac < -1)
diff --git a/drm/nouveau/nvkm/subdev/clk/base.c
b/drm/nouveau/nvkm/subdev/clk/base.c
index d37c13b7..1d71bf09 100644
--- a/drm/nouveau/nvkm/subdev/clk/base.c
+++ b/drm/nouveau/nvkm/subdev/clk/base.c
@@ -272,23 +272,26 @@ nvkm_cstate_new(struct nvkm_clk *clk, int idx, struct
nvkm_pstate *pstate)
  * P-States
  *****************************************************************************/
 static int
-nvkm_pstate_prog(struct nvkm_clk *clk, int pstatei)
+nvkm_pstate_prog(struct nvkm_clk *clk, int pstateid)
 {
 	struct nvkm_subdev *subdev = &clk->subdev;
 	struct nvkm_fb *fb = subdev->device->fb;
 	struct nvkm_pci *pci = subdev->device->pci;
 	struct nvkm_pstate *pstate;
-	int ret, idx = 0;
+	int ret;
 
-	if (pstatei == NVKM_CLK_PSTATE_DEFAULT)
+	if (pstateid == NVKM_CLK_PSTATE_DEFAULT)
 		return 0;
 
 	list_for_each_entry(pstate, &clk->states, head) {
-		if (idx++ == pstatei)
+		if (pstate->pstate == pstateid)
 			break;
 	}
 
-	nvkm_debug(subdev, "setting performance state %d\n", pstatei);
+	if (!pstate)
+		return -EINVAL;
+
+	nvkm_debug(subdev, "setting performance state %x\n", pstateid);
 	clk->pstate = pstate;
 
 	nvkm_pcie_set_link(pci, pstate->pcie_speed, pstate->pcie_width);
@@ -329,7 +332,6 @@ nvkm_clk_update_work(struct work_struct *work)
 	pstate = clk->pwrsrc ? clk->ustate_ac : clk->ustate_dc;
 	if (clk->state_nr && pstate != -1) {
 		pstate = (pstate < 0) ? clk->astate : pstate;
-		pstate = min(pstate, clk->state_nr - 1);
 	} else {
 		pstate = NVKM_CLK_PSTATE_DEFAULT;
 	}
@@ -491,32 +493,9 @@ nvkm_pstate_new(struct nvkm_clk *clk, int idx)
  * Adjustment triggers
  *****************************************************************************/
 static int
-nvkm_clk_ustate_update(struct nvkm_clk *clk, int req)
-{
-	struct nvkm_pstate *pstate;
-	int i = 0;
-
-	if (!clk->allow_reclock)
-		return -ENOSYS;
-
-	if (req != -1 && req != -2) {
-		list_for_each_entry(pstate, &clk->states, head) {
-			if (pstate->pstate == req)
-				break;
-			i++;
-		}
-
-		if (pstate->pstate != req)
-			return -EINVAL;
-		req = i;
-	}
-
-	return req + 2;
-}
-
-static int
 nvkm_clk_nstate(struct nvkm_clk *clk, const char *mode, int arglen)
 {
+	struct nvkm_pstate *pstate;
 	int ret = 1;
 
 	if (clk->allow_reclock && !strncasecmpz(mode, "auto",
arglen))
@@ -528,27 +507,46 @@ nvkm_clk_nstate(struct nvkm_clk *clk, const char *mode,
int arglen)
 
 		((char *)mode)[arglen] = '\0';
 		if (!kstrtol(mode, 0, &v)) {
-			ret = nvkm_clk_ustate_update(clk, v);
+			ret = v;
 			if (ret < 0)
 				ret = 1;
 		}
 		((char *)mode)[arglen] = save;
 	}
 
-	return ret - 2;
+	if (ret < 0)
+		return ret;
+
+	list_for_each_entry(pstate, &clk->states, head) {
+		if (pstate->pstate == ret)
+			return ret;
+	}
+	return -EINVAL;
 }
 
 int
 nvkm_clk_ustate(struct nvkm_clk *clk, int req, int pwr)
 {
-	int ret = nvkm_clk_ustate_update(clk, req);
-	if (ret >= 0) {
-		if (ret -= 2, pwr) clk->ustate_ac = ret;
-		else		   clk->ustate_dc = ret;
-		clk->exp_cstateid = NVKM_CLK_CSTATE_HIGHEST;
-		return nvkm_clk_update(clk, true);
+	struct nvkm_pstate *pstate;
+	bool valid = false;
+
+	list_for_each_entry(pstate, &clk->states, head) {
+		if (pstate->pstate == req) {
+			valid = true;
+			break;
+		}
 	}
-	return ret;
+
+	if (!valid)
+		return -EINVAL;
+
+	if (pwr)
+		clk->ustate_ac = req;
+	else
+		clk->ustate_dc = req;
+
+	clk->exp_cstateid = NVKM_CLK_CSTATE_HIGHEST;
+	return nvkm_clk_update(clk, true);
 }
 
 int
@@ -627,7 +625,7 @@ nvkm_clk_init(struct nvkm_subdev *subdev)
 	if (clk->func->init)
 		return clk->func->init(clk);
 
-	clk->astate = clk->state_nr - 1;
+	clk->astate = NVKM_CLK_PSTATE_DEFAULT;
 	clk->pstate = NULL;
 	clk->exp_cstateid = NVKM_CLK_CSTATE_DEFAULT;
 	clk->cstate = NULL;
-- 
2.12.0

Karol Herbst

2017-Mar-05 16:35 UTC

head link

[Nouveau] [PATCH 6/9] clk: Split out update code to nv40

This code will change for gf100 and newer for partial reclocks.

Signed-off-by: Karol Herbst <karolherbst at gmail.com>
Reviewed-by: Martin Peres <martin.peres at free.fr>
---
 drm/nouveau/nvkm/subdev/clk/base.c  | 15 ++++++---------
 drm/nouveau/nvkm/subdev/clk/g84.c   |  1 +
 drm/nouveau/nvkm/subdev/clk/gf100.c |  1 +
 drm/nouveau/nvkm/subdev/clk/gk104.c |  1 +
 drm/nouveau/nvkm/subdev/clk/gk20a.c |  1 +
 drm/nouveau/nvkm/subdev/clk/gm20b.c |  1 +
 drm/nouveau/nvkm/subdev/clk/gt215.c |  1 +
 drm/nouveau/nvkm/subdev/clk/mcp77.c |  1 +
 drm/nouveau/nvkm/subdev/clk/nv40.c  | 18 ++++++++++++++++++
 drm/nouveau/nvkm/subdev/clk/nv50.c  |  1 +
 drm/nouveau/nvkm/subdev/clk/priv.h  |  5 +++++
 11 files changed, 37 insertions(+), 9 deletions(-)

diff --git a/drm/nouveau/nvkm/subdev/clk/base.c
b/drm/nouveau/nvkm/subdev/clk/base.c
index 1d71bf09..b81b0258 100644
--- a/drm/nouveau/nvkm/subdev/clk/base.c
+++ b/drm/nouveau/nvkm/subdev/clk/base.c
@@ -271,7 +271,7 @@ nvkm_cstate_new(struct nvkm_clk *clk, int idx, struct
nvkm_pstate *pstate)
 /******************************************************************************
  * P-States
  *****************************************************************************/
-static int
+int
 nvkm_pstate_prog(struct nvkm_clk *clk, int pstateid)
 {
 	struct nvkm_subdev *subdev = &clk->subdev;
@@ -319,6 +319,10 @@ nvkm_clk_update_work(struct work_struct *work)
 
 	if (!atomic_xchg(&clk->waiting, 0))
 		return;
+
+	if (!clk->func->update)
+		return;
+
 	clk->pwrsrc = power_supply_is_system_supplied();
 
 	if (clk->pstate)
@@ -336,14 +340,7 @@ nvkm_clk_update_work(struct work_struct *work)
 		pstate = NVKM_CLK_PSTATE_DEFAULT;
 	}
 
-	nvkm_trace(subdev, "-> %d\n", pstate);
-	if (!clk->pstate || pstate != clk->pstate->pstate) {
-		int ret = nvkm_pstate_prog(clk, pstate);
-		if (ret) {
-			nvkm_error(subdev, "error setting pstate %d: %d\n",
-				   pstate, ret);
-		}
-	}
+	clk->func->update(clk, pstate);
 
 	wake_up_all(&clk->wait);
 	nvkm_notify_get(&clk->pwrsrc_ntfy);
diff --git a/drm/nouveau/nvkm/subdev/clk/g84.c
b/drm/nouveau/nvkm/subdev/clk/g84.c
index f97e3ec1..7b9b30d2 100644
--- a/drm/nouveau/nvkm/subdev/clk/g84.c
+++ b/drm/nouveau/nvkm/subdev/clk/g84.c
@@ -29,6 +29,7 @@ g84_clk = {
 	.calc = nv50_clk_calc,
 	.prog = nv50_clk_prog,
 	.tidy = nv50_clk_tidy,
+	.update = nv40_clk_update,
 	.domains = {
 		{ nv_clk_src_crystal, 0xff },
 		{ nv_clk_src_href   , 0xff },
diff --git a/drm/nouveau/nvkm/subdev/clk/gf100.c
b/drm/nouveau/nvkm/subdev/clk/gf100.c
index 7f67f9f5..8a46bf80 100644
--- a/drm/nouveau/nvkm/subdev/clk/gf100.c
+++ b/drm/nouveau/nvkm/subdev/clk/gf100.c
@@ -451,6 +451,7 @@ gf100_clk = {
 	.calc = gf100_clk_calc,
 	.prog = gf100_clk_prog,
 	.tidy = gf100_clk_tidy,
+	.update = nv40_clk_update,
 	.domains = {
 		{ nv_clk_src_crystal, 0xff },
 		{ nv_clk_src_href   , 0xff },
diff --git a/drm/nouveau/nvkm/subdev/clk/gk104.c
b/drm/nouveau/nvkm/subdev/clk/gk104.c
index 0b37e3da..cae58b6a 100644
--- a/drm/nouveau/nvkm/subdev/clk/gk104.c
+++ b/drm/nouveau/nvkm/subdev/clk/gk104.c
@@ -488,6 +488,7 @@ gk104_clk = {
 	.calc = gk104_clk_calc,
 	.prog = gk104_clk_prog,
 	.tidy = gk104_clk_tidy,
+	.update = nv40_clk_update,
 	.domains = {
 		{ nv_clk_src_crystal, 0xff },
 		{ nv_clk_src_href   , 0xff },
diff --git a/drm/nouveau/nvkm/subdev/clk/gk20a.c
b/drm/nouveau/nvkm/subdev/clk/gk20a.c
index 218893e3..ae40675b 100644
--- a/drm/nouveau/nvkm/subdev/clk/gk20a.c
+++ b/drm/nouveau/nvkm/subdev/clk/gk20a.c
@@ -600,6 +600,7 @@ gk20a_clk = {
 	.calc = gk20a_clk_calc,
 	.prog = gk20a_clk_prog,
 	.tidy = gk20a_clk_tidy,
+	.update = nv40_clk_update,
 	.pstates = gk20a_pstates,
 	.nr_pstates = ARRAY_SIZE(gk20a_pstates),
 	.domains = {
diff --git a/drm/nouveau/nvkm/subdev/clk/gm20b.c
b/drm/nouveau/nvkm/subdev/clk/gm20b.c
index b284e949..74346c88 100644
--- a/drm/nouveau/nvkm/subdev/clk/gm20b.c
+++ b/drm/nouveau/nvkm/subdev/clk/gm20b.c
@@ -880,6 +880,7 @@ gm20b_clk_speedo0 = {
 	.calc = gk20a_clk_calc,
 	.prog = gk20a_clk_prog,
 	.tidy = gk20a_clk_tidy,
+	.update = nv40_clk_update,
 	.pstates = gm20b_pstates,
 	/* Speedo 0 only supports 12 voltages */
 	.nr_pstates = ARRAY_SIZE(gm20b_pstates) - 1,
diff --git a/drm/nouveau/nvkm/subdev/clk/gt215.c
b/drm/nouveau/nvkm/subdev/clk/gt215.c
index 96e0941c..7315b310 100644
--- a/drm/nouveau/nvkm/subdev/clk/gt215.c
+++ b/drm/nouveau/nvkm/subdev/clk/gt215.c
@@ -520,6 +520,7 @@ gt215_clk = {
 	.calc = gt215_clk_calc,
 	.prog = gt215_clk_prog,
 	.tidy = gt215_clk_tidy,
+	.update = nv40_clk_update,
 	.domains = {
 		{ nv_clk_src_crystal  , 0xff },
 		{ nv_clk_src_core     , 0x00, 0, "core", 1000 },
diff --git a/drm/nouveau/nvkm/subdev/clk/mcp77.c
b/drm/nouveau/nvkm/subdev/clk/mcp77.c
index 1c21b8b5..e80b68e9 100644
--- a/drm/nouveau/nvkm/subdev/clk/mcp77.c
+++ b/drm/nouveau/nvkm/subdev/clk/mcp77.c
@@ -400,6 +400,7 @@ mcp77_clk = {
 	.calc = mcp77_clk_calc,
 	.prog = mcp77_clk_prog,
 	.tidy = mcp77_clk_tidy,
+	.update = nv40_clk_update,
 	.domains = {
 		{ nv_clk_src_crystal, 0xff },
 		{ nv_clk_src_href   , 0xff },
diff --git a/drm/nouveau/nvkm/subdev/clk/nv40.c
b/drm/nouveau/nvkm/subdev/clk/nv40.c
index 2ab9b9b8..15768996 100644
--- a/drm/nouveau/nvkm/subdev/clk/nv40.c
+++ b/drm/nouveau/nvkm/subdev/clk/nv40.c
@@ -201,12 +201,30 @@ nv40_clk_tidy(struct nvkm_clk *obj)
 {
 }
 
+void
+nv40_clk_update(struct nvkm_clk *clk, int pstate)
+{
+	struct nvkm_subdev *subdev = &clk->subdev;
+	int ret;
+
+	if (clk->pstate && pstate == clk->pstate->pstate)
+		return;
+
+	nvkm_trace(subdev, "-> %d\n", pstate);
+	ret = nvkm_pstate_prog(clk, pstate);
+	if (ret) {
+		nvkm_error(subdev, "error setting pstate %d: %d\n",
+			   pstate, ret);
+	}
+}
+
 static const struct nvkm_clk_func
 nv40_clk = {
 	.read = nv40_clk_read,
 	.calc = nv40_clk_calc,
 	.prog = nv40_clk_prog,
 	.tidy = nv40_clk_tidy,
+	.update = nv40_clk_update,
 	.domains = {
 		{ nv_clk_src_crystal, 0xff },
 		{ nv_clk_src_href   , 0xff },
diff --git a/drm/nouveau/nvkm/subdev/clk/nv50.c
b/drm/nouveau/nvkm/subdev/clk/nv50.c
index da1770e4..130cfee0 100644
--- a/drm/nouveau/nvkm/subdev/clk/nv50.c
+++ b/drm/nouveau/nvkm/subdev/clk/nv50.c
@@ -544,6 +544,7 @@ nv50_clk = {
 	.calc = nv50_clk_calc,
 	.prog = nv50_clk_prog,
 	.tidy = nv50_clk_tidy,
+	.update = nv40_clk_update,
 	.domains = {
 		{ nv_clk_src_crystal, 0xff },
 		{ nv_clk_src_href   , 0xff },
diff --git a/drm/nouveau/nvkm/subdev/clk/priv.h
b/drm/nouveau/nvkm/subdev/clk/priv.h
index 51eafc00..958f5e35 100644
--- a/drm/nouveau/nvkm/subdev/clk/priv.h
+++ b/drm/nouveau/nvkm/subdev/clk/priv.h
@@ -10,6 +10,7 @@ struct nvkm_clk_func {
 	int (*calc)(struct nvkm_clk *, struct nvkm_cstate *);
 	int (*prog)(struct nvkm_clk *);
 	void (*tidy)(struct nvkm_clk *);
+	void (*update)(struct nvkm_clk *, int pstate);
 	struct nvkm_pstate *pstates;
 	int nr_pstates;
 	struct nvkm_domain domains[];
@@ -20,7 +21,11 @@ int nvkm_clk_ctor(const struct nvkm_clk_func *, struct
nvkm_device *, int,
 int nvkm_clk_new_(const struct nvkm_clk_func *, struct nvkm_device *, int,
 		  bool allow_reclock, struct nvkm_clk **);
 
+int nvkm_pstate_prog(struct nvkm_clk *, int pstateid);
+
 int nv04_clk_pll_calc(struct nvkm_clk *, struct nvbios_pll *, int clk,
 		      struct nvkm_pll_vals *);
 int nv04_clk_pll_prog(struct nvkm_clk *, u32 reg1, struct nvkm_pll_vals *);
+
+void nv40_clk_update(struct nvkm_clk *, int pstate);
 #endif
-- 
2.12.0

Karol Herbst

2017-Mar-05 16:35 UTC

head link

[Nouveau] [PATCH 7/9] clk: Only do partial reclocks as required

We don't want to reclock to the same pstate or cstate over and over again,
so only do things we actually have to do.

This will become usefull later if we have a temperature daemon notifying
the clk subdev about a temperature change, because this may only change
the voltage or the cstate. And also for dynamic reclocking.

Signed-off-by: Karol Herbst <karolherbst at gmail.com>
Reviewed-by: Martin Peres <martin.peres at free.fr>
---
 drm/nouveau/nvkm/subdev/clk/base.c  | 11 +++++--
 drm/nouveau/nvkm/subdev/clk/gf100.c | 64 ++++++++++++++++++++++++++++++++++++-
 drm/nouveau/nvkm/subdev/clk/gk104.c |  2 +-
 drm/nouveau/nvkm/subdev/clk/priv.h  |  8 +++++
 4 files changed, 80 insertions(+), 5 deletions(-)

diff --git a/drm/nouveau/nvkm/subdev/clk/base.c
b/drm/nouveau/nvkm/subdev/clk/base.c
index b81b0258..54a4b7fa 100644
--- a/drm/nouveau/nvkm/subdev/clk/base.c
+++ b/drm/nouveau/nvkm/subdev/clk/base.c
@@ -107,7 +107,7 @@ nvkm_cstate_valid(struct nvkm_clk *clk, struct nvkm_cstate
*cstate,
 	return voltage <= min(max_volt, volt->max_uv);
 }
 
-static struct nvkm_cstate *
+struct nvkm_cstate *
 nvkm_cstate_find_best(struct nvkm_clk *clk, struct nvkm_pstate *pstate,
 		      struct nvkm_cstate *start)
 {
@@ -142,7 +142,7 @@ nvkm_cstate_find_best(struct nvkm_clk *clk, struct
nvkm_pstate *pstate,
 	return cstate;
 }
 
-static struct nvkm_cstate *
+struct nvkm_cstate *
 nvkm_cstate_get(struct nvkm_clk *clk, struct nvkm_pstate *pstate, int cstatei)
 {
 	struct nvkm_cstate *cstate;
@@ -162,7 +162,7 @@ nvkm_cstate_get(struct nvkm_clk *clk, struct nvkm_pstate
*pstate, int cstatei)
 	return NULL;
 }
 
-static int
+int
 nvkm_cstate_prog(struct nvkm_clk *clk, struct nvkm_pstate *pstate, int cstatei)
 {
 	struct nvkm_subdev *subdev = &clk->subdev;
@@ -182,6 +182,11 @@ nvkm_cstate_prog(struct nvkm_clk *clk, struct nvkm_pstate
*pstate, int cstatei)
 		cstate = &pstate->base;
 	}
 
+	if (!cstate) {
+		nvkm_error(subdev, "failed to set cstate %d\n", cstatei);
+		return -EINVAL;
+	}
+
 	if (therm) {
 		ret = nvkm_therm_cstate(therm, pstate->fanspeed, +1);
 		if (ret && ret != -ENODEV) {
diff --git a/drm/nouveau/nvkm/subdev/clk/gf100.c
b/drm/nouveau/nvkm/subdev/clk/gf100.c
index 8a46bf80..3444ef84 100644
--- a/drm/nouveau/nvkm/subdev/clk/gf100.c
+++ b/drm/nouveau/nvkm/subdev/clk/gf100.c
@@ -28,6 +28,7 @@
 #include <subdev/bios.h>
 #include <subdev/bios/pll.h>
 #include <subdev/timer.h>
+#include <subdev/volt.h>
 
 struct gf100_clk_info {
 	u32 freq;
@@ -445,13 +446,74 @@ gf100_clk_tidy(struct nvkm_clk *base)
 	memset(clk->eng, 0x00, sizeof(clk->eng));
 }
 
+static int
+gf100_clk_update_volt(struct nvkm_clk *clk)
+{
+	struct nvkm_subdev *subdev = &clk->subdev;
+	struct nvkm_volt *volt = subdev->device->volt;
+	struct nvkm_therm *therm = subdev->device->therm;
+
+	if (!volt || !therm || !clk->pstate || !clk->cstate)
+		return -EINVAL;
+
+	return nvkm_volt_set_id(volt, clk->cstate->voltage,
+				clk->pstate->base.voltage, clk->temp, 0);
+}
+
+void
+gf100_clk_update(struct nvkm_clk *clk, int pstate)
+{
+	struct nvkm_subdev *subdev = &clk->subdev;
+	int ret;
+
+	if (!clk->pstate || clk->pstate->pstate != pstate) {
+		nvkm_trace(subdev, "-> P %d\n", pstate);
+		ret = nvkm_pstate_prog(clk, pstate);
+		if (ret) {
+			nvkm_error(subdev, "error setting pstate %d: %d\n",
+				   pstate, ret);
+		}
+	} else if (!clk->cstate || clk->cstate->id != clk->exp_cstateid) {
+
+		struct nvkm_cstate *cstate +			nvkm_cstate_get(clk, clk->pstate,
clk->exp_cstateid);
+
+		if (!cstate) {
+			nvkm_error(subdev, "can't find cstate %i\n",
+				   clk->exp_cstateid);
+			return;
+		}
+
+		cstate = nvkm_cstate_find_best(clk, clk->pstate, cstate);
+		if (!cstate) {
+			nvkm_error(subdev, "can't find best cstate for %i\n",
+				   cstate->id);
+			return;
+		}
+
+		if (cstate != clk->cstate) {
+			nvkm_trace(subdev, "-> C %d\n", cstate->id);
+			ret = nvkm_cstate_prog(clk, clk->pstate, cstate->id);
+			if (ret) {
+				nvkm_error(subdev,
+					   "error setting cstate %d: %d\n",
+					   cstate->id, ret);
+			}
+		} else {
+			gf100_clk_update_volt(clk);
+		}
+	} else {
+		gf100_clk_update_volt(clk);
+	}
+}
+
 static const struct nvkm_clk_func
 gf100_clk = {
 	.read = gf100_clk_read,
 	.calc = gf100_clk_calc,
 	.prog = gf100_clk_prog,
 	.tidy = gf100_clk_tidy,
-	.update = nv40_clk_update,
+	.update = gf100_clk_update,
 	.domains = {
 		{ nv_clk_src_crystal, 0xff },
 		{ nv_clk_src_href   , 0xff },
diff --git a/drm/nouveau/nvkm/subdev/clk/gk104.c
b/drm/nouveau/nvkm/subdev/clk/gk104.c
index cae58b6a..691f3fb4 100644
--- a/drm/nouveau/nvkm/subdev/clk/gk104.c
+++ b/drm/nouveau/nvkm/subdev/clk/gk104.c
@@ -488,7 +488,7 @@ gk104_clk = {
 	.calc = gk104_clk_calc,
 	.prog = gk104_clk_prog,
 	.tidy = gk104_clk_tidy,
-	.update = nv40_clk_update,
+	.update = gf100_clk_update,
 	.domains = {
 		{ nv_clk_src_crystal, 0xff },
 		{ nv_clk_src_href   , 0xff },
diff --git a/drm/nouveau/nvkm/subdev/clk/priv.h
b/drm/nouveau/nvkm/subdev/clk/priv.h
index 958f5e35..f4b5950d 100644
--- a/drm/nouveau/nvkm/subdev/clk/priv.h
+++ b/drm/nouveau/nvkm/subdev/clk/priv.h
@@ -22,10 +22,18 @@ int nvkm_clk_new_(const struct nvkm_clk_func *, struct
nvkm_device *, int,
 		  bool allow_reclock, struct nvkm_clk **);
 
 int nvkm_pstate_prog(struct nvkm_clk *, int pstateid);
+int nvkm_cstate_prog(struct nvkm_clk *, struct nvkm_pstate *, int cstatei);
+
+struct nvkm_cstate *nvkm_cstate_get(struct nvkm_clk *, struct nvkm_pstate *,
+				    int cstatei);
+struct nvkm_cstate *nvkm_cstate_find_best(struct nvkm_clk *,
+					  struct nvkm_pstate *,
+					  struct nvkm_cstate *start);
 
 int nv04_clk_pll_calc(struct nvkm_clk *, struct nvbios_pll *, int clk,
 		      struct nvkm_pll_vals *);
 int nv04_clk_pll_prog(struct nvkm_clk *, u32 reg1, struct nvkm_pll_vals *);
 
 void nv40_clk_update(struct nvkm_clk *, int pstate);
+void gf100_clk_update(struct nvkm_clk *, int pstate);
 #endif
-- 
2.12.0

Karol Herbst

2017-Mar-05 16:35 UTC

head link

[Nouveau] [PATCH 8/9] clk: Set clocks to pre suspend state after suspend

The idea is to clear out the saved state, because after a resume we can't
know what the GPU is clocked to. The reclock is triggered by the call to
nvkm_clk_update later in nvkm_clk_init.

Signed-off-by: Karol Herbst <karolherbst at gmail.com>
Reviewed-by: Martin Peres <martin.peres at free.fr>
---
 drm/nouveau/nvkm/subdev/clk/base.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drm/nouveau/nvkm/subdev/clk/base.c
b/drm/nouveau/nvkm/subdev/clk/base.c
index 54a4b7fa..bc65906e 100644
--- a/drm/nouveau/nvkm/subdev/clk/base.c
+++ b/drm/nouveau/nvkm/subdev/clk/base.c
@@ -627,11 +627,9 @@ nvkm_clk_init(struct nvkm_subdev *subdev)
 	if (clk->func->init)
 		return clk->func->init(clk);
 
-	clk->astate = NVKM_CLK_PSTATE_DEFAULT;
+	// after a resume we have no idea what clocks are set, reset the state
 	clk->pstate = NULL;
-	clk->exp_cstateid = NVKM_CLK_CSTATE_DEFAULT;
 	clk->cstate = NULL;
-	clk->temp = 90; /* reasonable default value */
 	nvkm_clk_update(clk, true);
 	return 0;
 }
@@ -685,8 +683,13 @@ nvkm_clk_ctor(const struct nvkm_clk_func *func, struct
nvkm_device *device,
 	clk->func = func;
 	INIT_LIST_HEAD(&clk->states);
 	clk->domains = func->domains;
+
+	clk->astate = NVKM_CLK_PSTATE_DEFAULT;
 	clk->ustate_ac = -1;
 	clk->ustate_dc = -1;
+	clk->exp_cstateid = NVKM_CLK_CSTATE_DEFAULT;
+	clk->temp = 90; /* reasonable default value */
+
 	clk->allow_reclock = allow_reclock;
 
 	INIT_WORK(&clk->work, nvkm_clk_update_work);
-- 
2.12.0

Karol Herbst

2017-Mar-05 16:35 UTC

head link

[Nouveau] [PATCH 9/9] clk: Check pm_runtime status before reclocking

We don't want to change anything on the GPU if it's suspended. Also we
need to increase the refcount on the pm_runtime counter so that the GPU
won't be suspended while reclocking.

Signed-off-by: Karol Herbst <karolherbst at gmail.com>
---
 drm/nouveau/nvkm/subdev/clk/base.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drm/nouveau/nvkm/subdev/clk/base.c
b/drm/nouveau/nvkm/subdev/clk/base.c
index bc65906e..143ce0ea 100644
--- a/drm/nouveau/nvkm/subdev/clk/base.c
+++ b/drm/nouveau/nvkm/subdev/clk/base.c
@@ -320,6 +320,7 @@ nvkm_clk_update_work(struct work_struct *work)
 {
 	struct nvkm_clk *clk = container_of(work, typeof(*clk), work);
 	struct nvkm_subdev *subdev = &clk->subdev;
+	struct device *dev = subdev->device->dev;
 	int pstate;
 
 	if (!atomic_xchg(&clk->waiting, 0))
@@ -345,7 +346,14 @@ nvkm_clk_update_work(struct work_struct *work)
 		pstate = NVKM_CLK_PSTATE_DEFAULT;
 	}
 
-	clk->func->update(clk, pstate);
+	// only call into the code if the GPU is powered on
+	if (!pm_runtime_suspended(dev)) {
+		// it would be a shame if the GPU goes into suspend
+		// while doing the reclock
+		pm_runtime_get_sync(dev);
+		clk->func->update(clk, pstate);
+		pm_runtime_put(dev);
+	}
 
 	wake_up_all(&clk->wait);
 	nvkm_notify_get(&clk->pwrsrc_ntfy);
-- 
2.12.0

Ilia Mirkin

2017-Mar-05 16:40 UTC

head link

[Nouveau] [PATCH 2/9] clk: Remove dstate

On Sun, Mar 5, 2017 at 11:35 AM, Karol Herbst <karolherbst at gmail.com>
wrote:> We won't need it now, because we will adjust the clocks depending on
engine
> loads later on anyway or a static lockup table. It also simplifies the
Presumably "lookup"?
> clocking logic.
>
> This code was nowhere used anyway and just a mock up.
>
> Signed-off-by: Karol Herbst <karolherbst at gmail.com>
> Reviewed-by: Martin Peres <martin.peres at free.fr>
> ---
>  drm/nouveau/include/nvkm/subdev/clk.h |  2 --
>  drm/nouveau/nvkm/subdev/clk/base.c    | 16 ++--------------
>  2 files changed, 2 insertions(+), 16 deletions(-)
>
> diff --git a/drm/nouveau/include/nvkm/subdev/clk.h
b/drm/nouveau/include/nvkm/subdev/clk.h
> index b2c94cd5..69942b14 100644
> --- a/drm/nouveau/include/nvkm/subdev/clk.h
> +++ b/drm/nouveau/include/nvkm/subdev/clk.h
> @@ -99,7 +99,6 @@ struct nvkm_clk {
>         int ustate_ac; /* user-requested (-1 disabled, -2 perfmon) */
>         int ustate_dc; /* user-requested (-1 disabled, -2 perfmon) */
>         int astate; /* perfmon adjustment (base) */
> -       int dstate; /* display adjustment (min+) */
>         u8  temp;
>
>         bool allow_reclock;
> @@ -121,7 +120,6 @@ struct nvkm_clk {
>  int nvkm_clk_read(struct nvkm_clk *, enum nv_clk_src);
>  int nvkm_clk_ustate(struct nvkm_clk *, int req, int pwr);
>  int nvkm_clk_astate(struct nvkm_clk *, int req, int rel, bool wait);
> -int nvkm_clk_dstate(struct nvkm_clk *, int req, int rel);
>  int nvkm_clk_tstate(struct nvkm_clk *, u8 temperature);
>  int nvkm_clk_update(struct nvkm_clk *clk, bool wait);
>
> diff --git a/drm/nouveau/nvkm/subdev/clk/base.c
b/drm/nouveau/nvkm/subdev/clk/base.c
> index ecff3ff3..07d530ed 100644
> --- a/drm/nouveau/nvkm/subdev/clk/base.c
> +++ b/drm/nouveau/nvkm/subdev/clk/base.c
> @@ -306,15 +306,14 @@ nvkm_clk_update_work(struct work_struct *work)
>                 return;
>         clk->pwrsrc = power_supply_is_system_supplied();
>
> -       nvkm_trace(subdev, "P %d PWR %d U(AC) %d U(DC) %d A %d T %d°C
D %d\n",
> +       nvkm_trace(subdev, "P %d PWR %d U(AC) %d U(DC) %d A %d T
%d°C\n",
>                    clk->pstate, clk->pwrsrc, clk->ustate_ac,
clk->ustate_dc,
> -                  clk->astate, clk->temp, clk->dstate);
> +                  clk->astate, clk->temp);
>
>         pstate = clk->pwrsrc ? clk->ustate_ac : clk->ustate_dc;
>         if (clk->state_nr && pstate != -1) {
>                 pstate = (pstate < 0) ? clk->astate : pstate;
>                 pstate = min(pstate, clk->state_nr - 1);
> -               pstate = max(pstate, clk->dstate);
>         } else {
>                 pstate = clk->pstate = -1;
>         }
> @@ -554,16 +553,6 @@ nvkm_clk_tstate(struct nvkm_clk *clk, u8 temp)
>         return nvkm_clk_update(clk, false);
>  }
>
> -int
> -nvkm_clk_dstate(struct nvkm_clk *clk, int req, int rel)
> -{
> -       if (!rel) clk->dstate  = req;
> -       if ( rel) clk->dstate += rel;
> -       clk->dstate = min(clk->dstate, clk->state_nr - 1);
> -       clk->dstate = max(clk->dstate, 0);
> -       return nvkm_clk_update(clk, true);
> -}
> -
>  static int
>  nvkm_clk_pwrsrc(struct nvkm_notify *notify)
>  {
> @@ -621,7 +610,6 @@ nvkm_clk_init(struct nvkm_subdev *subdev)
>                 return clk->func->init(clk);
>
>         clk->astate = clk->state_nr - 1;
> -       clk->dstate = 0;
>         clk->pstate = -1;
>         clk->temp = 90; /* reasonable default value */
>         nvkm_clk_update(clk, true);
> --
> 2.12.0
>
> _______________________________________________
> Nouveau mailing list
> Nouveau at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau

Martin Peres

2017-Mar-06 23:09 UTC

head link

[Nouveau] [PATCH 1/9] clk: Rename nvkm_pstate_calc to nvkm_clk_update and export it

On 05/03/17 18:34, Karol Herbst wrote:> This function will be used to update the current clock state.
>
> This will happen for various reasons:
>   * Temperature changes
>   * User changes clocking state
>   * Load changes
>
> Signed-off-by: Karol Herbst <karolherbst at gmail.com>
> ---
>  drm/nouveau/include/nvkm/subdev/clk.h |  1 +
>  drm/nouveau/nvkm/subdev/clk/base.c    | 26 ++++++++++++++++----------
>  2 files changed, 17 insertions(+), 10 deletions(-)
>
> diff --git a/drm/nouveau/include/nvkm/subdev/clk.h
b/drm/nouveau/include/nvkm/subdev/clk.h
> index e5275f74..b2c94cd5 100644
> --- a/drm/nouveau/include/nvkm/subdev/clk.h
> +++ b/drm/nouveau/include/nvkm/subdev/clk.h
> @@ -123,6 +123,7 @@ int nvkm_clk_ustate(struct nvkm_clk *, int req, int
pwr);
>  int nvkm_clk_astate(struct nvkm_clk *, int req, int rel, bool wait);
>  int nvkm_clk_dstate(struct nvkm_clk *, int req, int rel);
>  int nvkm_clk_tstate(struct nvkm_clk *, u8 temperature);
> +int nvkm_clk_update(struct nvkm_clk *clk, bool wait);
To keep in line with the rest, please get rid of 'clk'.

Otherwise, looks good to me!
>
>  int nv04_clk_new(struct nvkm_device *, int, struct nvkm_clk **);
>  int nv40_clk_new(struct nvkm_device *, int, struct nvkm_clk **);
> diff --git a/drm/nouveau/nvkm/subdev/clk/base.c
b/drm/nouveau/nvkm/subdev/clk/base.c
> index e4c8d310..ecff3ff3 100644
> --- a/drm/nouveau/nvkm/subdev/clk/base.c
> +++ b/drm/nouveau/nvkm/subdev/clk/base.c
> @@ -296,7 +296,7 @@ nvkm_pstate_prog(struct nvkm_clk *clk, int pstatei)
>  }
>
>  static void
> -nvkm_pstate_work(struct work_struct *work)
> +nvkm_clk_update_work(struct work_struct *work)
>  {
>  	struct nvkm_clk *clk = container_of(work, typeof(*clk), work);
>  	struct nvkm_subdev *subdev = &clk->subdev;
> @@ -332,9 +332,15 @@ nvkm_pstate_work(struct work_struct *work)
>  	nvkm_notify_get(&clk->pwrsrc_ntfy);
>  }
>
> -static int
> -nvkm_pstate_calc(struct nvkm_clk *clk, bool wait)
> +int
> +nvkm_clk_update(struct nvkm_clk *clk, bool wait)
>  {
> +	if (!clk)
> +		return -EINVAL;
> +
> +	if (!clk->allow_reclock)
> +		return -ENODEV;
> +
>  	atomic_set(&clk->waiting, 1);
>  	schedule_work(&clk->work);
>  	if (wait)
> @@ -524,7 +530,7 @@ nvkm_clk_ustate(struct nvkm_clk *clk, int req, int pwr)
>  	if (ret >= 0) {
>  		if (ret -= 2, pwr) clk->ustate_ac = ret;
>  		else		   clk->ustate_dc = ret;
> -		return nvkm_pstate_calc(clk, true);
> +		return nvkm_clk_update(clk, true);
>  	}
>  	return ret;
>  }
> @@ -536,7 +542,7 @@ nvkm_clk_astate(struct nvkm_clk *clk, int req, int rel,
bool wait)
>  	if ( rel) clk->astate += rel;
>  	clk->astate = min(clk->astate, clk->state_nr - 1);
>  	clk->astate = max(clk->astate, 0);
> -	return nvkm_pstate_calc(clk, wait);
> +	return nvkm_clk_update(clk, wait);
>  }
>
>  int
> @@ -545,7 +551,7 @@ nvkm_clk_tstate(struct nvkm_clk *clk, u8 temp)
>  	if (clk->temp == temp)
>  		return 0;
>  	clk->temp = temp;
> -	return nvkm_pstate_calc(clk, false);
> +	return nvkm_clk_update(clk, false);
>  }
>
>  int
> @@ -555,7 +561,7 @@ nvkm_clk_dstate(struct nvkm_clk *clk, int req, int rel)
>  	if ( rel) clk->dstate += rel;
>  	clk->dstate = min(clk->dstate, clk->state_nr - 1);
>  	clk->dstate = max(clk->dstate, 0);
> -	return nvkm_pstate_calc(clk, true);
> +	return nvkm_clk_update(clk, true);
>  }
>
>  static int
> @@ -563,7 +569,7 @@ nvkm_clk_pwrsrc(struct nvkm_notify *notify)
>  {
>  	struct nvkm_clk *clk >  		container_of(notify, typeof(*clk),
pwrsrc_ntfy);
> -	nvkm_pstate_calc(clk, false);
> +	nvkm_clk_update(clk, false);
>  	return NVKM_NOTIFY_DROP;
>  }
>
> @@ -618,7 +624,7 @@ nvkm_clk_init(struct nvkm_subdev *subdev)
>  	clk->dstate = 0;
>  	clk->pstate = -1;
>  	clk->temp = 90; /* reasonable default value */
> -	nvkm_pstate_calc(clk, true);
> +	nvkm_clk_update(clk, true);
>  	return 0;
>  }
>
> @@ -675,7 +681,7 @@ nvkm_clk_ctor(const struct nvkm_clk_func *func, struct
nvkm_device *device,
>  	clk->ustate_dc = -1;
>  	clk->allow_reclock = allow_reclock;
>
> -	INIT_WORK(&clk->work, nvkm_pstate_work);
> +	INIT_WORK(&clk->work, nvkm_clk_update_work);
>  	init_waitqueue_head(&clk->wait);
>  	atomic_set(&clk->waiting, 0);
>
>

Martin Peres

2017-Mar-06 23:25 UTC

head link

[Nouveau] [PATCH 3/9] clk: Make pstate a pointer to nvkm_pstate

On 05/03/17 18:35, Karol Herbst wrote:> We will access the current set cstate at least every second and this safes
Either "the current ctstate" or "the currently-set cstate",
but not a
mix of both :D I prefer the first.

safes -> saves.
> us some CPU cycles looking them up every second.
>
> Signed-off-by: Karol Herbst <karolherbst at gmail.com>
> Reviewed-by: Martin Peres <martin.peres at free.fr>
> ---
>  drm/nouveau/include/nvkm/subdev/clk.h |  4 +++-
>  drm/nouveau/nvkm/engine/device/ctrl.c |  5 ++++-
>  drm/nouveau/nvkm/subdev/clk/base.c    | 17 ++++++++++++-----
>  drm/nouveau/nvkm/subdev/pmu/gk20a.c   | 18 +++++++-----------
>  4 files changed, 26 insertions(+), 18 deletions(-)
>
> diff --git a/drm/nouveau/include/nvkm/subdev/clk.h
b/drm/nouveau/include/nvkm/subdev/clk.h
> index 69942b14..37263b7f 100644
> --- a/drm/nouveau/include/nvkm/subdev/clk.h
> +++ b/drm/nouveau/include/nvkm/subdev/clk.h
> @@ -10,6 +10,8 @@ struct nvkm_pll_vals;
>  #define NVKM_CLK_CSTATE_BASE    -2 /* pstate base */
>  #define NVKM_CLK_CSTATE_HIGHEST -3 /* highest possible */
>
> +#define NVKM_CLK_PSTATE_DEFAULT -1
> +
>  enum nv_clk_src {
>  	nv_clk_src_crystal,
>  	nv_clk_src_href,
> @@ -95,7 +97,7 @@ struct nvkm_clk {
>
>  	struct nvkm_notify pwrsrc_ntfy;
>  	int pwrsrc;
> -	int pstate; /* current */
> +	struct nvkm_pstate *pstate; /* current */
>  	int ustate_ac; /* user-requested (-1 disabled, -2 perfmon) */
>  	int ustate_dc; /* user-requested (-1 disabled, -2 perfmon) */
>  	int astate; /* perfmon adjustment (base) */
> diff --git a/drm/nouveau/nvkm/engine/device/ctrl.c
b/drm/nouveau/nvkm/engine/device/ctrl.c
> index b0ece71a..da70626c 100644
> --- a/drm/nouveau/nvkm/engine/device/ctrl.c
> +++ b/drm/nouveau/nvkm/engine/device/ctrl.c
> @@ -52,7 +52,10 @@ nvkm_control_mthd_pstate_info(struct nvkm_control *ctrl,
void *data, u32 size)
>  		args->v0.ustate_ac = clk->ustate_ac;
>  		args->v0.ustate_dc = clk->ustate_dc;
>  		args->v0.pwrsrc = clk->pwrsrc;
> -		args->v0.pstate = clk->pstate;
> +		if (clk->pstate)
> +			args->v0.pstate = clk->pstate->pstate;
> +		else
> +			args->v0.pstate = NVKM_CLK_PSTATE_DEFAULT;
>  	} else {
>  		args->v0.count = 0;
>  		args->v0.ustate_ac = NVIF_CONTROL_PSTATE_INFO_V0_USTATE_DISABLE;
> diff --git a/drm/nouveau/nvkm/subdev/clk/base.c
b/drm/nouveau/nvkm/subdev/clk/base.c
> index 07d530ed..0d4d9fdf 100644
> --- a/drm/nouveau/nvkm/subdev/clk/base.c
> +++ b/drm/nouveau/nvkm/subdev/clk/base.c
> @@ -271,13 +271,16 @@ nvkm_pstate_prog(struct nvkm_clk *clk, int pstatei)
>  	struct nvkm_pstate *pstate;
>  	int ret, idx = 0;
>
> +	if (pstatei == NVKM_CLK_PSTATE_DEFAULT)
> +		return 0;
> +
>  	list_for_each_entry(pstate, &clk->states, head) {
>  		if (idx++ == pstatei)
>  			break;
>  	}
>
>  	nvkm_debug(subdev, "setting performance state %d\n", pstatei);
> -	clk->pstate = pstatei;
> +	clk->pstate = pstate;
>
>  	nvkm_pcie_set_link(pci, pstate->pcie_speed, pstate->pcie_width);
>
> @@ -306,8 +309,12 @@ nvkm_clk_update_work(struct work_struct *work)
>  		return;
>  	clk->pwrsrc = power_supply_is_system_supplied();
>
> +	if (clk->pstate)
> +		pstate = clk->pstate->pstate;
> +	else
> +		pstate = NVKM_CLK_PSTATE_DEFAULT;
>  	nvkm_trace(subdev, "P %d PWR %d U(AC) %d U(DC) %d A %d T
%d°C\n",
> -		   clk->pstate, clk->pwrsrc, clk->ustate_ac, clk->ustate_dc,
> +		   pstate, clk->pwrsrc, clk->ustate_ac, clk->ustate_dc,
>  		   clk->astate, clk->temp);
>
>  	pstate = clk->pwrsrc ? clk->ustate_ac : clk->ustate_dc;
> @@ -315,11 +322,11 @@ nvkm_clk_update_work(struct work_struct *work)
>  		pstate = (pstate < 0) ? clk->astate : pstate;
>  		pstate = min(pstate, clk->state_nr - 1);
>  	} else {
> -		pstate = clk->pstate = -1;
> +		pstate = NVKM_CLK_PSTATE_DEFAULT;
>  	}
>
>  	nvkm_trace(subdev, "-> %d\n", pstate);
> -	if (pstate != clk->pstate) {
> +	if (!clk->pstate || pstate != clk->pstate->pstate) {
>  		int ret = nvkm_pstate_prog(clk, pstate);
>  		if (ret) {
>  			nvkm_error(subdev, "error setting pstate %d: %d\n",
> @@ -610,7 +617,7 @@ nvkm_clk_init(struct nvkm_subdev *subdev)
>  		return clk->func->init(clk);
>
>  	clk->astate = clk->state_nr - 1;
> -	clk->pstate = -1;
> +	clk->pstate = NULL;
>  	clk->temp = 90; /* reasonable default value */
>  	nvkm_clk_update(clk, true);
>  	return 0;
> diff --git a/drm/nouveau/nvkm/subdev/pmu/gk20a.c
b/drm/nouveau/nvkm/subdev/pmu/gk20a.c
> index 9ca0db79..43a9a74a 100644
> --- a/drm/nouveau/nvkm/subdev/pmu/gk20a.c
> +++ b/drm/nouveau/nvkm/subdev/pmu/gk20a.c
> @@ -55,24 +55,22 @@ gk20a_pmu_dvfs_target(struct gk20a_pmu *pmu, int
*state)
>  	return nvkm_clk_astate(clk, *state, 0, false);
>  }
>
> -static void
> -gk20a_pmu_dvfs_get_cur_state(struct gk20a_pmu *pmu, int *state)
> -{
> -	struct nvkm_clk *clk = pmu->base.subdev.device->clk;
> -
> -	*state = clk->pstate;
> -}
> -
>  static int
>  gk20a_pmu_dvfs_get_target_state(struct gk20a_pmu *pmu,
>  				int *state, int load)
>  {
>  	struct gk20a_pmu_dvfs_data *data = pmu->data;
>  	struct nvkm_clk *clk = pmu->base.subdev.device->clk;
> +	struct nvkm_pstate *pstate = clk->pstate;
>  	int cur_level, level;
>
> +	if (!pstate) {
> +		*state = 0;
> +		return 1;
> +	}
> +
>  	/* For GK20A, the performance level is directly mapped to pstate */
> -	level = cur_level = clk->pstate;
> +	level = cur_level = clk->pstate->pstate;
>
>  	if (load > data->p_load_max) {
>  		level = min(clk->state_nr - 1, level + (clk->state_nr / 3));
> @@ -142,8 +140,6 @@ gk20a_pmu_dvfs_work(struct nvkm_alarm *alarm)
>  	nvkm_trace(subdev, "utilization = %d %%, avg_load = %d %%\n",
>  		   utilization, data->avg_load);
>
> -	gk20a_pmu_dvfs_get_cur_state(pmu, &state);
> -
>  	if (gk20a_pmu_dvfs_get_target_state(pmu, &state, data->avg_load))
{
>  		nvkm_trace(subdev, "set new state to %d\n", state);
>  		gk20a_pmu_dvfs_target(pmu, &state);
>

Martin Peres

2017-Mar-06 23:26 UTC

head link

[Nouveau] [PATCH 5/9] clk: We should pass the pstate id around not the index in the list

On 05/03/17 18:35, Karol Herbst wrote:> This makes the code easier, because we can compare the id with
> pstate->pstate and safe us the trouble iterating over the entire pstate
saves us from the trouble of iterating over the pstates.
> list to match the index.
>
> Signed-off-by: Karol Herbst <karolherbst at gmail.com>
> Reviewed-by: Martin Peres <martin.peres at free.fr>
> ---
>  drm/nouveau/nouveau_debugfs.c      |  6 +--
>  drm/nouveau/nvkm/subdev/clk/base.c | 78
+++++++++++++++++++-------------------
>  2 files changed, 41 insertions(+), 43 deletions(-)
>
> diff --git a/drm/nouveau/nouveau_debugfs.c b/drm/nouveau/nouveau_debugfs.c
> index fd64dfdc..b114a429 100644
> --- a/drm/nouveau/nouveau_debugfs.c
> +++ b/drm/nouveau/nouveau_debugfs.c
> @@ -96,11 +96,11 @@ nouveau_debugfs_pstate_get(struct seq_file *m, void
*data)
>  		} while (attr.index);
>
>  		if (state >= 0) {
> -			if (info.ustate_ac == state)
> +			if (info.ustate_ac == attr.state)
>  				seq_printf(m, " AC");
> -			if (info.ustate_dc == state)
> +			if (info.ustate_dc == attr.state)
>  				seq_printf(m, " DC");
> -			if (info.pstate == state)
> +			if (info.pstate == attr.state)
>  				seq_printf(m, " *");
>  		} else {
>  			if (info.ustate_ac < -1)
> diff --git a/drm/nouveau/nvkm/subdev/clk/base.c
b/drm/nouveau/nvkm/subdev/clk/base.c
> index d37c13b7..1d71bf09 100644
> --- a/drm/nouveau/nvkm/subdev/clk/base.c
> +++ b/drm/nouveau/nvkm/subdev/clk/base.c
> @@ -272,23 +272,26 @@ nvkm_cstate_new(struct nvkm_clk *clk, int idx, struct
nvkm_pstate *pstate)
>   * P-States
>  
*****************************************************************************/
>  static int
> -nvkm_pstate_prog(struct nvkm_clk *clk, int pstatei)
> +nvkm_pstate_prog(struct nvkm_clk *clk, int pstateid)
>  {
>  	struct nvkm_subdev *subdev = &clk->subdev;
>  	struct nvkm_fb *fb = subdev->device->fb;
>  	struct nvkm_pci *pci = subdev->device->pci;
>  	struct nvkm_pstate *pstate;
> -	int ret, idx = 0;
> +	int ret;
>
> -	if (pstatei == NVKM_CLK_PSTATE_DEFAULT)
> +	if (pstateid == NVKM_CLK_PSTATE_DEFAULT)
>  		return 0;
>
>  	list_for_each_entry(pstate, &clk->states, head) {
> -		if (idx++ == pstatei)
> +		if (pstate->pstate == pstateid)
>  			break;
>  	}
>
> -	nvkm_debug(subdev, "setting performance state %d\n", pstatei);
> +	if (!pstate)
> +		return -EINVAL;
> +
> +	nvkm_debug(subdev, "setting performance state %x\n", pstateid);
>  	clk->pstate = pstate;
>
>  	nvkm_pcie_set_link(pci, pstate->pcie_speed, pstate->pcie_width);
> @@ -329,7 +332,6 @@ nvkm_clk_update_work(struct work_struct *work)
>  	pstate = clk->pwrsrc ? clk->ustate_ac : clk->ustate_dc;
>  	if (clk->state_nr && pstate != -1) {
>  		pstate = (pstate < 0) ? clk->astate : pstate;
> -		pstate = min(pstate, clk->state_nr - 1);
>  	} else {
>  		pstate = NVKM_CLK_PSTATE_DEFAULT;
>  	}
> @@ -491,32 +493,9 @@ nvkm_pstate_new(struct nvkm_clk *clk, int idx)
>   * Adjustment triggers
>  
*****************************************************************************/
>  static int
> -nvkm_clk_ustate_update(struct nvkm_clk *clk, int req)
> -{
> -	struct nvkm_pstate *pstate;
> -	int i = 0;
> -
> -	if (!clk->allow_reclock)
> -		return -ENOSYS;
> -
> -	if (req != -1 && req != -2) {
> -		list_for_each_entry(pstate, &clk->states, head) {
> -			if (pstate->pstate == req)
> -				break;
> -			i++;
> -		}
> -
> -		if (pstate->pstate != req)
> -			return -EINVAL;
> -		req = i;
> -	}
> -
> -	return req + 2;
> -}
> -
> -static int
>  nvkm_clk_nstate(struct nvkm_clk *clk, const char *mode, int arglen)
>  {
> +	struct nvkm_pstate *pstate;
>  	int ret = 1;
>
>  	if (clk->allow_reclock && !strncasecmpz(mode,
"auto", arglen))
> @@ -528,27 +507,46 @@ nvkm_clk_nstate(struct nvkm_clk *clk, const char
*mode, int arglen)
>
>  		((char *)mode)[arglen] = '\0';
>  		if (!kstrtol(mode, 0, &v)) {
> -			ret = nvkm_clk_ustate_update(clk, v);
> +			ret = v;
>  			if (ret < 0)
>  				ret = 1;
>  		}
>  		((char *)mode)[arglen] = save;
>  	}
>
> -	return ret - 2;
> +	if (ret < 0)
> +		return ret;
> +
> +	list_for_each_entry(pstate, &clk->states, head) {
> +		if (pstate->pstate == ret)
> +			return ret;
> +	}
> +	return -EINVAL;
>  }
>
>  int
>  nvkm_clk_ustate(struct nvkm_clk *clk, int req, int pwr)
>  {
> -	int ret = nvkm_clk_ustate_update(clk, req);
> -	if (ret >= 0) {
> -		if (ret -= 2, pwr) clk->ustate_ac = ret;
> -		else		   clk->ustate_dc = ret;
> -		clk->exp_cstateid = NVKM_CLK_CSTATE_HIGHEST;
> -		return nvkm_clk_update(clk, true);
> +	struct nvkm_pstate *pstate;
> +	bool valid = false;
> +
> +	list_for_each_entry(pstate, &clk->states, head) {
> +		if (pstate->pstate == req) {
> +			valid = true;
> +			break;
> +		}
>  	}
> -	return ret;
> +
> +	if (!valid)
> +		return -EINVAL;
> +
> +	if (pwr)
> +		clk->ustate_ac = req;
> +	else
> +		clk->ustate_dc = req;
> +
> +	clk->exp_cstateid = NVKM_CLK_CSTATE_HIGHEST;
> +	return nvkm_clk_update(clk, true);
>  }
>
>  int
> @@ -627,7 +625,7 @@ nvkm_clk_init(struct nvkm_subdev *subdev)
>  	if (clk->func->init)
>  		return clk->func->init(clk);
>
> -	clk->astate = clk->state_nr - 1;
> +	clk->astate = NVKM_CLK_PSTATE_DEFAULT;
>  	clk->pstate = NULL;
>  	clk->exp_cstateid = NVKM_CLK_CSTATE_DEFAULT;
>  	clk->cstate = NULL;
>

Martin Peres

2017-Mar-06 23:45 UTC

head link

[Nouveau] [PATCH 8/9] clk: Set clocks to pre suspend state after suspend

On 05/03/17 18:35, Karol Herbst wrote:> The idea is to clear out the saved state, because after a resume we
can't
> know what the GPU is clocked to. The reclock is triggered by the call to
> nvkm_clk_update later in nvkm_clk_init.
>
> Signed-off-by: Karol Herbst <karolherbst at gmail.com>
> Reviewed-by: Martin Peres <martin.peres at free.fr>
> ---
>  drm/nouveau/nvkm/subdev/clk/base.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drm/nouveau/nvkm/subdev/clk/base.c
b/drm/nouveau/nvkm/subdev/clk/base.c
> index 54a4b7fa..bc65906e 100644
> --- a/drm/nouveau/nvkm/subdev/clk/base.c
> +++ b/drm/nouveau/nvkm/subdev/clk/base.c
> @@ -627,11 +627,9 @@ nvkm_clk_init(struct nvkm_subdev *subdev)
>  	if (clk->func->init)
>  		return clk->func->init(clk);
>
> -	clk->astate = NVKM_CLK_PSTATE_DEFAULT;
> +	// after a resume we have no idea what clocks are set, reset the state
No c++ comments, please use /* */
>  	clk->pstate = NULL;
> -	clk->exp_cstateid = NVKM_CLK_CSTATE_DEFAULT;
>  	clk->cstate = NULL;
> -	clk->temp = 90; /* reasonable default value */
>  	nvkm_clk_update(clk, true);
>  	return 0;
>  }
> @@ -685,8 +683,13 @@ nvkm_clk_ctor(const struct nvkm_clk_func *func, struct
nvkm_device *device,
>  	clk->func = func;
>  	INIT_LIST_HEAD(&clk->states);
>  	clk->domains = func->domains;
> +
> +	clk->astate = NVKM_CLK_PSTATE_DEFAULT;
>  	clk->ustate_ac = -1;
>  	clk->ustate_dc = -1;
> +	clk->exp_cstateid = NVKM_CLK_CSTATE_DEFAULT;
> +	clk->temp = 90; /* reasonable default value */
> +
>  	clk->allow_reclock = allow_reclock;
>
>  	INIT_WORK(&clk->work, nvkm_clk_update_work);
>

Martin Peres

2017-Mar-06 23:46 UTC

head link

[Nouveau] [PATCH 9/9] clk: Check pm_runtime status before reclocking

On 05/03/17 18:35, Karol Herbst wrote:> We don't want to change anything on the GPU if it's suspended. Also
we
> need to increase the refcount on the pm_runtime counter so that the GPU
> won't be suspended while reclocking.
>
> Signed-off-by: Karol Herbst <karolherbst at gmail.com>
> ---
>  drm/nouveau/nvkm/subdev/clk/base.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drm/nouveau/nvkm/subdev/clk/base.c
b/drm/nouveau/nvkm/subdev/clk/base.c
> index bc65906e..143ce0ea 100644
> --- a/drm/nouveau/nvkm/subdev/clk/base.c
> +++ b/drm/nouveau/nvkm/subdev/clk/base.c
> @@ -320,6 +320,7 @@ nvkm_clk_update_work(struct work_struct *work)
>  {
>  	struct nvkm_clk *clk = container_of(work, typeof(*clk), work);
>  	struct nvkm_subdev *subdev = &clk->subdev;
> +	struct device *dev = subdev->device->dev;
>  	int pstate;
>
>  	if (!atomic_xchg(&clk->waiting, 0))
> @@ -345,7 +346,14 @@ nvkm_clk_update_work(struct work_struct *work)
>  		pstate = NVKM_CLK_PSTATE_DEFAULT;
>  	}
>
> -	clk->func->update(clk, pstate);
> +	// only call into the code if the GPU is powered on
> +	if (!pm_runtime_suspended(dev)) {
> +		// it would be a shame if the GPU goes into suspend
> +		// while doing the reclock
> +		pm_runtime_get_sync(dev);
> +		clk->func->update(clk, pstate);
> +		pm_runtime_put(dev);
> +	}
Good catch! But Java has corrupted your mind :D Please use /* */ for 
comments.
>
>  	wake_up_all(&clk->wait);
>  	nvkm_notify_get(&clk->pwrsrc_ntfy);
>

Possibly Parallel Threads

Search for more apparently analagous threads

Nouveau - Mar 2017 - [PATCH 9/9] clk: Check pm_runtime status before reclocking

[Nouveau] [PATCH 0/9] clk subdev updates

[Nouveau] [PATCH 1/9] clk: Rename nvkm_pstate_calc to nvkm_clk_update and export it

[Nouveau] [PATCH 2/9] clk: Remove dstate

[Nouveau] [PATCH 3/9] clk: Make pstate a pointer to nvkm_pstate

[Nouveau] [PATCH 4/9] clk: Hold information about the current cstate status

[Nouveau] [PATCH 5/9] clk: We should pass the pstate id around not the index in the list

[Nouveau] [PATCH 6/9] clk: Split out update code to nv40

[Nouveau] [PATCH 7/9] clk: Only do partial reclocks as required

[Nouveau] [PATCH 8/9] clk: Set clocks to pre suspend state after suspend

[Nouveau] [PATCH 9/9] clk: Check pm_runtime status before reclocking

[Nouveau] [PATCH 2/9] clk: Remove dstate

[Nouveau] [PATCH 1/9] clk: Rename nvkm_pstate_calc to nvkm_clk_update and export it

[Nouveau] [PATCH 3/9] clk: Make pstate a pointer to nvkm_pstate

[Nouveau] [PATCH 5/9] clk: We should pass the pstate id around not the index in the list

[Nouveau] [PATCH 8/9] clk: Set clocks to pre suspend state after suspend

[Nouveau] [PATCH 9/9] clk: Check pm_runtime status before reclocking

Possibly Parallel Threads