[PATCH 0/9] clk subdev updates

[PATCH 0/9] clk subdev updates

[Karol Herbst]

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

_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

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

[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

Signed-off-by: Karol Herbst <karolherbst@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

_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

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

[Martin Peres]

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@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);
>
>
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

[PATCH 2/9] clk: Remove dstate

[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
clocking logic.

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

Signed-off-by: Karol Herbst <karolherbst@gmail.com>
Reviewed-by: Martin Peres <martin.peres@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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

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

[Ilia Mirkin]

On Sun, Mar 5, 2017 at 11:35 AM, Karol Herbst <karolherbst@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@gmail.com>
> Reviewed-by: Martin Peres <martin.peres@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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

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

[Karol Herbst]

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@gmail.com>
Reviewed-by: Martin Peres <martin.peres@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

_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

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

[Martin Peres]

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@gmail.com>
> Reviewed-by: Martin Peres <martin.peres@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);
>
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

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

[Karol Herbst]

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

Signed-off-by: Karol Herbst <karolherbst@gmail.com>
Reviewed-by: Martin Peres <martin.peres@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

_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

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

[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
list to match the index.

Signed-off-by: Karol Herbst <karolherbst@gmail.com>
Reviewed-by: Martin Peres <martin.peres@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

_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

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

[Martin Peres]

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@gmail.com>
> Reviewed-by: Martin Peres <martin.peres@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;
>
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

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

[Karol Herbst]

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

Signed-off-by: Karol Herbst <karolherbst@gmail.com>
Reviewed-by: Martin Peres <martin.peres@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

_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

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

[Karol Herbst]

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@gmail.com>
Reviewed-by: Martin Peres <martin.peres@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

_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

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

[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.

Signed-off-by: Karol Herbst <karolherbst@gmail.com>
Reviewed-by: Martin Peres <martin.peres@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

_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

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

[Martin Peres]

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@gmail.com>
> Reviewed-by: Martin Peres <martin.peres@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);
>
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

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

[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.

Signed-off-by: Karol Herbst <karolherbst@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

_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

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

[Martin Peres]

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@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);
>
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau