Skip to contentSkip to conversation

clk subdev updates

PATCH Superseded Karol Herbst 3 participants 16 messages Mar 5, 2017 – Mar 6, 2017 7 messages not in the archive

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

cover 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(-)
Signature · 6 lines
2.12.0

_______________________________________________
Nouveau mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/nouveau

A message that is not in the archive (sent off-list or not yet received): 20170305163507.2002-1-karolherbst-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org

#2 lore raw

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

---
 drm/nouveau/include/nvkm/subdev/clk.h |  1 +
 drm/nouveau/nvkm/subdev/clk/base.c    | 26 ++++++++++++++++----------
 2 files changed, 17 insertions(+), 10 deletions(-)
drm/nouveau/include/nvkm/subdev/clk.h +1 -0
index e5275f74..b2c94cd5 100644
@@ -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 **);
drm/nouveau/nvkm/subdev/clk/base.c +16 -10
index e4c8d310..ecff3ff3 100644
@@ -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);
 
Signature · 6 lines
2.12.0

_______________________________________________
Nouveau mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/nouveau

A message that is not in the archive (sent off-list or not yet received): 20170305163507.2002-2-karolherbst-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org

#4 lore raw

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

Quoted 18 lines · Karol Herbst

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

---
 drm/nouveau/include/nvkm/subdev/clk.h |  1 +
 drm/nouveau/nvkm/subdev/clk/base.c    | 26 ++++++++++++++++----------
 2 files changed, 17 insertions(+), 10 deletions(-)
drm/nouveau/include/nvkm/subdev/clk.h +0 -0
index e5275f74..b2c94cd5 100644

drm/nouveau/include/nvkm/subdev/clk.h int nvkm_clk_ustate(struct nvkm_clk *, int req, int pwr); · line 126

Quoted 4 lines
drm/nouveau/include/nvkm/subdev/clk.h +1 -0
@@ -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!

Quoted 90 lines
drm/nouveau/include/nvkm/subdev/clk.h +0 -0
 int nv04_clk_new(struct nvkm_device *, int, struct nvkm_clk **);
 int nv40_clk_new(struct nvkm_device *, int, struct nvkm_clk **);
drm/nouveau/nvkm/subdev/clk/base.c +15 -9
index e4c8d310..ecff3ff3 100644
@@ -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;
 }
 

drm/nouveau/nvkm/subdev/clk/base.c nvkm_clk_ctor(const struct nvkm_clk_func *func, struct nvkm_device *device, · line 687

Quoted 8 lines
drm/nouveau/nvkm/subdev/clk/base.c +1 -1
@@ -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);
 

_______________________________________________
Nouveau mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/nouveau

#5 lore raw

PATCH 2/9 clk: Remove dstate

In reply to a message not in the archive

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.

---
 drm/nouveau/include/nvkm/subdev/clk.h |  2 --
 drm/nouveau/nvkm/subdev/clk/base.c    | 16 ++--------------
 2 files changed, 2 insertions(+), 16 deletions(-)
drm/nouveau/include/nvkm/subdev/clk.h +0 -2
index b2c94cd5..69942b14 100644
@@ -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);
 
drm/nouveau/nvkm/subdev/clk/base.c +2 -14
index ecff3ff3..07d530ed 100644
@@ -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);
Signature · 6 lines
2.12.0

_______________________________________________
Nouveau mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/nouveau

A message that is not in the archive (sent off-list or not yet received): 20170305163507.2002-3-karolherbst-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org

#7 lore raw

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

Quoted 2 lines · Karol Herbst

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"?

Quoted 81 lines

clocking logic.

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

---
 drm/nouveau/include/nvkm/subdev/clk.h |  2 --
 drm/nouveau/nvkm/subdev/clk/base.c    | 16 ++--------------
 2 files changed, 2 insertions(+), 16 deletions(-)
drm/nouveau/include/nvkm/subdev/clk.h +0 -2
index b2c94cd5..69942b14 100644
@@ -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);
 
drm/nouveau/nvkm/subdev/clk/base.c +2 -15
index ecff3ff3..07d530ed 100644
@@ -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

Quoted 4 lines

_______________________________________________
Nouveau mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/nouveau

_______________________________________________
Nouveau mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/nouveau

#8 lore raw

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

In reply to a message not in the archive

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

---
 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(-)
drm/nouveau/include/nvkm/subdev/clk.h +3 -1
index 69942b14..37263b7f 100644
@@ -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) */
drm/nouveau/nvkm/engine/device/ctrl.c +4 -1
index b0ece71a..da70626c 100644
@@ -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;
drm/nouveau/nvkm/subdev/clk/base.c +12 -5
index 07d530ed..0d4d9fdf 100644
@@ -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;
drm/nouveau/nvkm/subdev/pmu/gk20a.c +7 -11
index 9ca0db79..43a9a74a 100644
@@ -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);
Signature · 6 lines
2.12.0

_______________________________________________
Nouveau mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/nouveau

A message that is not in the archive (sent off-list or not yet received): 20170305163507.2002-4-karolherbst-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org

#10 lore raw

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

Quoted 1 line · Karol Herbst

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.

Quoted 146 lines

us some CPU cycles looking them up every second.

---
 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(-)
drm/nouveau/include/nvkm/subdev/clk.h +3 -1
index 69942b14..37263b7f 100644
@@ -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) */
drm/nouveau/nvkm/engine/device/ctrl.c +4 -1
index b0ece71a..da70626c 100644
@@ -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;
drm/nouveau/nvkm/subdev/clk/base.c +12 -5
index 07d530ed..0d4d9fdf 100644
@@ -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;
drm/nouveau/nvkm/subdev/pmu/gk20a.c +7 -9
index 9ca0db79..43a9a74a 100644
@@ -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));

drm/nouveau/nvkm/subdev/pmu/gk20a.c gk20a_pmu_dvfs_work(struct nvkm_alarm *alarm) · line 145

Quoted 8 lines
drm/nouveau/nvkm/subdev/pmu/gk20a.c +0 -2
@@ -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);

_______________________________________________
Nouveau mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/nouveau

#11 lore raw

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

In reply to a message not in the archive

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

---
 drm/nouveau/include/nvkm/subdev/clk.h |  2 ++
 drm/nouveau/nvkm/subdev/clk/base.c    | 32 +++++++++++++++++++++++++-------
 2 files changed, 27 insertions(+), 7 deletions(-)
drm/nouveau/include/nvkm/subdev/clk.h +2 -0
index 37263b7f..0e0af704 100644
@@ -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;
drm/nouveau/nvkm/subdev/clk/base.c +25 -7
index 0d4d9fdf..d37c13b7 100644
@@ -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);
Signature · 6 lines
2.12.0

_______________________________________________
Nouveau mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/nouveau
#12 lore raw

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

In reply to a message not in the archive

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.

---
 drm/nouveau/nouveau_debugfs.c      |  6 +--
 drm/nouveau/nvkm/subdev/clk/base.c | 78 +++++++++++++++++++-------------------
 2 files changed, 41 insertions(+), 43 deletions(-)
drm/nouveau/nouveau_debugfs.c +3 -3
index fd64dfdc..b114a429 100644
@@ -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)
drm/nouveau/nvkm/subdev/clk/base.c +38 -40

134 lines not on this page · raw message

Signature · 6 lines
2.12.0

_______________________________________________
Nouveau mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/nouveau

A message that is not in the archive (sent off-list or not yet received): 20170305163507.2002-6-karolherbst-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org

#14 lore raw

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

Quoted 2 lines · Karol Herbst

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.

Quoted 164 lines

list to match the index.

---
 drm/nouveau/nouveau_debugfs.c      |  6 +--
 drm/nouveau/nvkm/subdev/clk/base.c | 78 +++++++++++++++++++-------------------
 2 files changed, 41 insertions(+), 43 deletions(-)
drm/nouveau/nouveau_debugfs.c +3 -3
index fd64dfdc..b114a429 100644
@@ -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)
drm/nouveau/nvkm/subdev/clk/base.c +37 -39
index d37c13b7..1d71bf09 100644
@@ -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

drm/nouveau/nvkm/subdev/clk/base.c nvkm_clk_init(struct nvkm_subdev *subdev) · line 631

Quoted 8 lines
drm/nouveau/nvkm/subdev/clk/base.c +1 -1
@@ -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;

_______________________________________________
Nouveau mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/nouveau

#15 lore raw

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

In reply to a message not in the archive

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

---
 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(-)
drm/nouveau/nvkm/subdev/clk/base.c +6 -9
index 1d71bf09..b81b0258 100644
@@ -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);
drm/nouveau/nvkm/subdev/clk/g84.c +1 -0
index f97e3ec1..7b9b30d2 100644
@@ -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 },
drm/nouveau/nvkm/subdev/clk/gf100.c +1 -0
index 7f67f9f5..8a46bf80 100644
@@ -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 },
drm/nouveau/nvkm/subdev/clk/gk104.c +1 -0
index 0b37e3da..cae58b6a 100644
@@ -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 },
drm/nouveau/nvkm/subdev/clk/gk20a.c +1 -0
index 218893e3..ae40675b 100644
@@ -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 = {
drm/nouveau/nvkm/subdev/clk/gm20b.c +1 -0
index b284e949..74346c88 100644
@@ -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,
drm/nouveau/nvkm/subdev/clk/gt215.c +1 -0
index 96e0941c..7315b310 100644
@@ -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 },
drm/nouveau/nvkm/subdev/clk/mcp77.c +1 -0
index 1c21b8b5..e80b68e9 100644
@@ -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 },
drm/nouveau/nvkm/subdev/clk/nv40.c +18 -0
index 2ab9b9b8..15768996 100644
@@ -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 },
drm/nouveau/nvkm/subdev/clk/nv50.c +1 -0
index da1770e4..130cfee0 100644
@@ -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 },
drm/nouveau/nvkm/subdev/clk/priv.h +5 -0
index 51eafc00..958f5e35 100644
@@ -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
Signature · 6 lines
2.12.0

_______________________________________________
Nouveau mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/nouveau
#16 lore raw

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

In reply to a message not in the archive

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.

---
 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(-)
drm/nouveau/nvkm/subdev/clk/base.c +8 -3
index b81b0258..54a4b7fa 100644
@@ -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) {
drm/nouveau/nvkm/subdev/clk/gf100.c +63 -1

82 lines not on this page · raw message

drm/nouveau/nvkm/subdev/clk/gk104.c +1 -1
index cae58b6a..691f3fb4 100644
@@ -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 },
drm/nouveau/nvkm/subdev/clk/priv.h +8 -0
index 958f5e35..f4b5950d 100644
@@ -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
Signature · 6 lines
2.12.0

_______________________________________________
Nouveau mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/nouveau
#17 lore raw

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

In reply to a message not in the archive

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.

---
 drm/nouveau/nvkm/subdev/clk/base.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)
drm/nouveau/nvkm/subdev/clk/base.c +6 -3
index 54a4b7fa..bc65906e 100644
@@ -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);
Signature · 6 lines
2.12.0

_______________________________________________
Nouveau mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/nouveau

A message that is not in the archive (sent off-list or not yet received): 20170305163507.2002-9-karolherbst-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org

#19 lore raw

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

Quoted 15 lines · Karol Herbst

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.

---
 drm/nouveau/nvkm/subdev/clk/base.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)
drm/nouveau/nvkm/subdev/clk/base.c +0 -0
index 54a4b7fa..bc65906e 100644

drm/nouveau/nvkm/subdev/clk/base.c nvkm_clk_init(struct nvkm_subdev *subdev) · line 630

Quoted 5 lines
drm/nouveau/nvkm/subdev/clk/base.c +1 -1
@@ -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 /* */

Quoted 12 lines
drm/nouveau/nvkm/subdev/clk/base.c +0 -2
 	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;

drm/nouveau/nvkm/subdev/clk/base.c nvkm_clk_ctor(const struct nvkm_clk_func *func, struct nvkm_device *device, · line 695

Quoted 10 lines
drm/nouveau/nvkm/subdev/clk/base.c +5 -0
+
+	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);

_______________________________________________
Nouveau mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/nouveau

#20 lore raw

PATCH 9/9 clk: Check pm_runtime status before reclocking

In reply to a message not in the archive

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.

---
 drm/nouveau/nvkm/subdev/clk/base.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)
drm/nouveau/nvkm/subdev/clk/base.c +9 -1
index bc65906e..143ce0ea 100644
@@ -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);
Signature · 6 lines
2.12.0

_______________________________________________
Nouveau mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/nouveau

A message that is not in the archive (sent off-list or not yet received): 20170305163507.2002-10-karolherbst-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org

#22 lore raw

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

Quoted 24 lines · Karol Herbst

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.

---
 drm/nouveau/nvkm/subdev/clk/base.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)
drm/nouveau/nvkm/subdev/clk/base.c +1 -0
index bc65906e..143ce0ea 100644
@@ -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;
 	}

drm/nouveau/nvkm/subdev/clk/base.c nvkm_clk_update_work(struct work_struct *work) · line 356

Quoted 10 lines
drm/nouveau/nvkm/subdev/clk/base.c +8 -1
 
-	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.

drm/nouveau/nvkm/subdev/clk/base.c nvkm_clk_update_work(struct work_struct *work)

Quoted 4 lines
drm/nouveau/nvkm/subdev/clk/base.c +0 -0
 	wake_up_all(&clk->wait);
 	nvkm_notify_get(&clk->pwrsrc_ntfy);
 

_______________________________________________
Nouveau mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/nouveau